ARCHIVE/rippled
1
0
Fork 0
mirror of https://github.com/XRPLF/rippled.git synced 2025-11-20 19:15:54 +00:00
Code Issues Packages Projects Releases Wiki Activity

Decouple CredentialHelpers from xrpld/app/tx (#5487)

Browse Source 
This PR refactors `CredentialHelpers` and removes some unnecessary dependencies as a step of modularization.

The ledger component is almost independent except that it references `MPTokenAuthorize` and `CredentialHelpers.h`, and the latter further references `Transactor.h`. This PR partially clears the path to modularizing the ledger component and decouples `CredentialHelpers` from xrpld.
This commit is contained in:
Jingchen
2025-07-03 15:27:37 +01:00
committed by GitHub
parent c2f3e2e263
commit 9874d47d7f
6 changed files with 87 additions and 43 deletions
Download Patch File Download Diff File Expand all files Collapse all files

45
src/xrpld/app/misc/CredentialHelpers.cpp
View File

@@ -120,15 +120,15 @@ deleteSLE(
} }
NotTEC NotTEC
checkFields(PreflightContext const& ctx) checkFields(STTx const& tx, beast::Journal j)
{ {
if (!ctx.tx.isFieldPresent(sfCredentialIDs)) if (!tx.isFieldPresent(sfCredentialIDs))
return tesSUCCESS; return tesSUCCESS;
auto const& credentials = ctx.tx.getFieldV256(sfCredentialIDs); auto const& credentials = tx.getFieldV256(sfCredentialIDs);
if (credentials.empty() || (credentials.size() > maxCredentialsArraySize)) if (credentials.empty() || (credentials.size() > maxCredentialsArraySize))
{ {
JLOG(ctx.j.trace()) JLOG(j.trace())
<< "Malformed transaction: Credentials array size is invalid: " << "Malformed transaction: Credentials array size is invalid: "
<< credentials.size(); << credentials.size();
return temMALFORMED; return temMALFORMED;
@@ -140,7 +140,7 @@ checkFields(PreflightContext const& ctx)
auto [it, ins] = duplicates.insert(cred); auto [it, ins] = duplicates.insert(cred);
if (!ins) if (!ins)
{ {
JLOG(ctx.j.trace()) JLOG(j.trace())
<< "Malformed transaction: duplicates in credentials."; << "Malformed transaction: duplicates in credentials.";
return temMALFORMED; return temMALFORMED;
} }
@@ -150,24 +150,28 @@ checkFields(PreflightContext const& ctx)
} }
TER TER
valid(PreclaimContext const& ctx, AccountID const& src) valid(
STTx const& tx,
ReadView const& view,
AccountID const& src,
beast::Journal j)
{ {
if (!ctx.tx.isFieldPresent(sfCredentialIDs)) if (!tx.isFieldPresent(sfCredentialIDs))
return tesSUCCESS; return tesSUCCESS;
auto const& credIDs(ctx.tx.getFieldV256(sfCredentialIDs)); auto const& credIDs(tx.getFieldV256(sfCredentialIDs));
for (auto const& h : credIDs) for (auto const& h : credIDs)
{ {
auto const sleCred = ctx.view.read(keylet::credential(h)); auto const sleCred = view.read(keylet::credential(h));
if (!sleCred) if (!sleCred)
{ {
JLOG(ctx.j.trace()) << "Credential doesn't exist. Cred: " << h; JLOG(j.trace()) << "Credential doesn't exist. Cred: " << h;
return tecBAD_CREDENTIALS; return tecBAD_CREDENTIALS;
} }
if (sleCred->getAccountID(sfSubject) != src) if (sleCred->getAccountID(sfSubject) != src)
{ {
JLOG(ctx.j.trace()) JLOG(j.trace())
<< "Credential doesn't belong to the source account. Cred: " << "Credential doesn't belong to the source account. Cred: "
<< h; << h;
return tecBAD_CREDENTIALS; return tecBAD_CREDENTIALS;
@@ -175,7 +179,7 @@ valid(PreclaimContext const& ctx, AccountID const& src)
if (!(sleCred->getFlags() & lsfAccepted)) if (!(sleCred->getFlags() & lsfAccepted))
{ {
JLOG(ctx.j.trace()) << "Credential isn't accepted. Cred: " << h; JLOG(j.trace()) << "Credential isn't accepted. Cred: " << h;
return tecBAD_CREDENTIALS; return tecBAD_CREDENTIALS;
} }
@@ -352,10 +356,12 @@ verifyValidDomain(
TER TER
verifyDepositPreauth( verifyDepositPreauth(
ApplyContext& ctx, STTx const& tx,
ApplyView& view,
AccountID const& src, AccountID const& src,
AccountID const& dst, AccountID const& dst,
std::shared_ptr<SLE> const& sleDst) std::shared_ptr<SLE> const& sleDst,
beast::Journal j)
{ {
// If depositPreauth is enabled, then an account that requires // If depositPreauth is enabled, then an account that requires
// authorization has at least two ways to get a payment in: // authorization has at least two ways to get a payment in:
@@ -363,24 +369,21 @@ verifyDepositPreauth(
// 2. If src is deposit preauthorized by dst (either by account or by // 2. If src is deposit preauthorized by dst (either by account or by
// credentials). // credentials).
bool const credentialsPresent = ctx.tx.isFieldPresent(sfCredentialIDs); bool const credentialsPresent = tx.isFieldPresent(sfCredentialIDs);
if (credentialsPresent && if (credentialsPresent &&
credentials::removeExpired( credentials::removeExpired(view, tx.getFieldV256(sfCredentialIDs), j))
ctx.view(), ctx.tx.getFieldV256(sfCredentialIDs), ctx.journal))
return tecEXPIRED; return tecEXPIRED;
if (sleDst && (sleDst->getFlags() & lsfDepositAuth)) if (sleDst && (sleDst->getFlags() & lsfDepositAuth))
{ {
if (src != dst) if (src != dst)
{ {
if (!ctx.view().exists(keylet::depositPreauth(dst, src))) if (!view.exists(keylet::depositPreauth(dst, src)))
return !credentialsPresent return !credentialsPresent
? tecNO_PERMISSION ? tecNO_PERMISSION
: credentials::authorizedDepositPreauth( : credentials::authorizedDepositPreauth(
ctx.view(), view, tx.getFieldV256(sfCredentialIDs), dst);
ctx.tx.getFieldV256(sfCredentialIDs),
dst);
} }
} }

25
src/xrpld/app/misc/CredentialHelpers.h
View File

@@ -20,7 +20,16 @@
#ifndef RIPPLE_APP_MISC_CREDENTIALHELPERS_H_INCLUDED #ifndef RIPPLE_APP_MISC_CREDENTIALHELPERS_H_INCLUDED
#define RIPPLE_APP_MISC_CREDENTIALHELPERS_H_INCLUDED #define RIPPLE_APP_MISC_CREDENTIALHELPERS_H_INCLUDED
#include <xrpld/app/tx/detail/Transactor.h> #include <xrpld/ledger/ApplyView.h>
#include <xrpld/ledger/ReadView.h>
#include <xrpl/basics/Log.h>
#include <xrpl/basics/base_uint.h>
#include <xrpl/beast/utility/Journal.h>
#include <xrpl/protocol/AccountID.h>
#include <xrpl/protocol/STArray.h>
#include <xrpl/protocol/STTx.h>
#include <xrpl/protocol/TER.h>
namespace ripple { namespace ripple {
namespace credentials { namespace credentials {
@@ -48,13 +57,17 @@ deleteSLE(
// Amendment and parameters checks for sfCredentialIDs field // Amendment and parameters checks for sfCredentialIDs field
NotTEC NotTEC
checkFields(PreflightContext const& ctx); checkFields(STTx const& tx, beast::Journal j);
// Accessing the ledger to check if provided credentials are valid. Do not use // Accessing the ledger to check if provided credentials are valid. Do not use
// in doApply (only in preclaim) since it does not remove expired credentials. // in doApply (only in preclaim) since it does not remove expired credentials.
// If you call it in prelaim, you also must call verifyDepositPreauth in doApply // If you call it in prelaim, you also must call verifyDepositPreauth in doApply
TER TER
valid(PreclaimContext const& ctx, AccountID const& src); valid(
STTx const& tx,
ReadView const& view,
AccountID const& src,
beast::Journal j);
// Check if subject has any credential maching the given domain. If you call it // Check if subject has any credential maching the given domain. If you call it
// in preclaim and it returns tecEXPIRED, you should call verifyValidDomain in // in preclaim and it returns tecEXPIRED, you should call verifyValidDomain in
@@ -93,10 +106,12 @@ verifyValidDomain(
// Check expired credentials and for existing DepositPreauth ledger object // Check expired credentials and for existing DepositPreauth ledger object
TER TER
verifyDepositPreauth( verifyDepositPreauth(
ApplyContext& ctx, STTx const& tx,
ApplyView& view,
AccountID const& src, AccountID const& src,
AccountID const& dst, AccountID const& dst,
std::shared_ptr<SLE> const& sleDst); std::shared_ptr<SLE> const& sleDst,
beast::Journal j);
} // namespace ripple } // namespace ripple

9
src/xrpld/app/tx/detail/DeleteAccount.cpp
View File

@@ -58,7 +58,8 @@ DeleteAccount::preflight(PreflightContext const& ctx)
// An account cannot be deleted and give itself the resulting XRP. // An account cannot be deleted and give itself the resulting XRP.
return temDST_IS_SRC; return temDST_IS_SRC;
if (auto const err = credentials::checkFields(ctx); !isTesSuccess(err)) if (auto const err = credentials::checkFields(ctx.tx, ctx.j);
!isTesSuccess(err))
return err; return err;
return preflight2(ctx); return preflight2(ctx);
@@ -241,7 +242,8 @@ DeleteAccount::preclaim(PreclaimContext const& ctx)
return tecDST_TAG_NEEDED; return tecDST_TAG_NEEDED;
// If credentials are provided - check them anyway // If credentials are provided - check them anyway
if (auto const err = credentials::valid(ctx, account); !isTesSuccess(err)) if (auto const err = credentials::valid(ctx.tx, ctx.view, account, ctx.j);
!isTesSuccess(err))
return err; return err;
// if credentials then postpone auth check to doApply, to check for expired // if credentials then postpone auth check to doApply, to check for expired
@@ -376,7 +378,8 @@ DeleteAccount::doApply()
if (ctx_.view().rules().enabled(featureDepositAuth) && if (ctx_.view().rules().enabled(featureDepositAuth) &&
ctx_.tx.isFieldPresent(sfCredentialIDs)) ctx_.tx.isFieldPresent(sfCredentialIDs))
{ {
if (auto err = verifyDepositPreauth(ctx_, account_, dstID, dst); if (auto err = verifyDepositPreauth(
ctx_.tx, ctx_.view(), account_, dstID, dst, ctx_.journal);
!isTesSuccess(err)) !isTesSuccess(err))
return err; return err;
} }

9
src/xrpld/app/tx/detail/Escrow.cpp
View File

@@ -672,7 +672,8 @@ EscrowFinish::preflight(PreflightContext const& ctx)
} }
} }
if (auto const err = credentials::checkFields(ctx); !isTesSuccess(err)) if (auto const err = credentials::checkFields(ctx.tx, ctx.j);
!isTesSuccess(err))
return err; return err;
return tesSUCCESS; return tesSUCCESS;
@@ -761,7 +762,8 @@ EscrowFinish::preclaim(PreclaimContext const& ctx)
{ {
if (ctx.view.rules().enabled(featureCredentials)) if (ctx.view.rules().enabled(featureCredentials))
{ {
if (auto const err = credentials::valid(ctx, ctx.tx[sfAccount]); if (auto const err =
credentials::valid(ctx.tx, ctx.view, ctx.tx[sfAccount], ctx.j);
!isTesSuccess(err)) !isTesSuccess(err))
return err; return err;
} }
@@ -1107,7 +1109,8 @@ EscrowFinish::doApply()
if (ctx_.view().rules().enabled(featureDepositAuth)) if (ctx_.view().rules().enabled(featureDepositAuth))
{ {
if (auto err = verifyDepositPreauth(ctx_, account_, destID, sled); if (auto err = verifyDepositPreauth(
ctx_.tx, ctx_.view(), account_, destID, sled, ctx_.journal);
!isTesSuccess(err)) !isTesSuccess(err))
return err; return err;
} }

9
src/xrpld/app/tx/detail/PayChan.cpp
View File

@@ -473,7 +473,8 @@ PayChanClaim::preflight(PreflightContext const& ctx)
return temBAD_SIGNATURE; return temBAD_SIGNATURE;
} }
if (auto const err = credentials::checkFields(ctx); !isTesSuccess(err)) if (auto const err = credentials::checkFields(ctx.tx, ctx.j);
!isTesSuccess(err))
return err; return err;
return preflight2(ctx); return preflight2(ctx);
@@ -485,7 +486,8 @@ PayChanClaim::preclaim(PreclaimContext const& ctx)
if (!ctx.view.rules().enabled(featureCredentials)) if (!ctx.view.rules().enabled(featureCredentials))
return Transactor::preclaim(ctx); return Transactor::preclaim(ctx);
if (auto const err = credentials::valid(ctx, ctx.tx[sfAccount]); if (auto const err =
credentials::valid(ctx.tx, ctx.view, ctx.tx[sfAccount], ctx.j);
!isTesSuccess(err)) !isTesSuccess(err))
return err; return err;
@@ -554,7 +556,8 @@ PayChanClaim::doApply()
if (depositAuth) if (depositAuth)
{ {
if (auto err = verifyDepositPreauth(ctx_, txAccount, dst, sled); if (auto err = verifyDepositPreauth(
ctx_.tx, ctx_.view(), txAccount, dst, sled, ctx_.journal);
!isTesSuccess(err)) !isTesSuccess(err))
return err; return err;
} }

33
src/xrpld/app/tx/detail/Payment.cpp
View File

@@ -238,7 +238,8 @@ Payment::preflight(PreflightContext const& ctx)
} }
} }
if (auto const err = credentials::checkFields(ctx); !isTesSuccess(err)) if (auto const err = credentials::checkFields(ctx.tx, ctx.j);
!isTesSuccess(err))
return err; return err;
return preflight2(ctx); return preflight2(ctx);
@@ -358,7 +359,8 @@ Payment::preclaim(PreclaimContext const& ctx)
} }
} }
if (auto const err = credentials::valid(ctx, ctx.tx[sfAccount]); if (auto const err =
credentials::valid(ctx.tx, ctx.view, ctx.tx[sfAccount], ctx.j);
!isTesSuccess(err)) !isTesSuccess(err))
return err; return err;
@@ -450,8 +452,13 @@ Payment::doApply()
// 1. If Account == Destination, or // 1. If Account == Destination, or
// 2. If Account is deposit preauthorized by destination. // 2. If Account is deposit preauthorized by destination.
if (auto err = if (auto err = verifyDepositPreauth(
verifyDepositPreauth(ctx_, account_, dstAccountID, sleDst); ctx_.tx,
ctx_.view(),
account_,
dstAccountID,
sleDst,
ctx_.journal);
!isTesSuccess(err)) !isTesSuccess(err))
return err; return err;
} }
@@ -521,8 +528,13 @@ Payment::doApply()
ter != tesSUCCESS) ter != tesSUCCESS)
return ter; return ter;
if (auto err = if (auto err = verifyDepositPreauth(
verifyDepositPreauth(ctx_, account_, dstAccountID, sleDst); ctx_.tx,
ctx_.view(),
account_,
dstAccountID,
sleDst,
ctx_.journal);
!isTesSuccess(err)) !isTesSuccess(err))
return err; return err;
@@ -644,8 +656,13 @@ Payment::doApply()
if (dstAmount > dstReserve || if (dstAmount > dstReserve ||
sleDst->getFieldAmount(sfBalance) > dstReserve) sleDst->getFieldAmount(sfBalance) > dstReserve)
{ {
if (auto err = if (auto err = verifyDepositPreauth(
verifyDepositPreauth(ctx_, account_, dstAccountID, sleDst); ctx_.tx,
ctx_.view(),
account_,
dstAccountID,
sleDst,
ctx_.journal);
!isTesSuccess(err)) !isTesSuccess(err))
return err; return err;
} }