ARCHIVE/rippled
1
0
Fork 0
mirror of https://github.com/XRPLF/rippled.git synced 2025-12-06 17:27:55 +00:00
Code Issues Packages Projects Releases Wiki Activity

refactor: Add support for extra transaction signatures (#5594)

Browse Source 
* Restructures Transactor signature checking code to be able to handle a `sigObject`, which may be the full transaction, or may be an object field containing a separate signature. Either way, the `sigObject` can be a single- or multi-sign signature.
This commit is contained in:
Ed Hennis
2025-09-29 18:11:53 -04:00
committed by GitHub
parent d67dcfe3c4
commit 550f90a75e
2 changed files with 64 additions and 54 deletions
Download Patch File Download Diff File Expand all files Collapse all files

99
src/xrpld/app/tx/detail/Transactor.cpp
View File

@@ -659,16 +659,19 @@ Transactor::apply()
} }
NotTEC NotTEC
Transactor::checkSign(PreclaimContext const& ctx) Transactor::checkSign(
PreclaimContext const& ctx,
AccountID const& idAccount,
STObject const& sigObject)
{ {
auto const pkSigner = ctx.tx.getSigningPubKey(); auto const pkSigner = sigObject.getFieldVL(sfSigningPubKey);
// Ignore signature check on batch inner transactions // Ignore signature check on batch inner transactions
if (ctx.tx.isFlag(tfInnerBatchTxn) && if (sigObject.isFlag(tfInnerBatchTxn) &&
ctx.view.rules().enabled(featureBatch)) ctx.view.rules().enabled(featureBatch))
{ {
// Defensive Check: These values are also checked in Batch::preflight // Defensive Check: These values are also checked in Batch::preflight
if (ctx.tx.isFieldPresent(sfTxnSignature) || !pkSigner.empty() || if (sigObject.isFieldPresent(sfTxnSignature) || !pkSigner.empty() ||
ctx.tx.isFieldPresent(sfSigners)) sigObject.isFieldPresent(sfSigners))
{ {
return temINVALID_FLAG; // LCOV_EXCL_LINE return temINVALID_FLAG; // LCOV_EXCL_LINE
} }
@@ -676,34 +679,31 @@ Transactor::checkSign(PreclaimContext const& ctx)
} }
if ((ctx.flags & tapDRY_RUN) && pkSigner.empty() && if ((ctx.flags & tapDRY_RUN) && pkSigner.empty() &&
!ctx.tx.isFieldPresent(sfSigners)) !sigObject.isFieldPresent(sfSigners))
{ {
// simulate: skip signature validation when neither SigningPubKey nor // simulate: skip signature validation when neither SigningPubKey nor
// Signers are provided // Signers are provided
return tesSUCCESS; return tesSUCCESS;
} }
auto const idAccount = ctx.tx[~sfDelegate].value_or(ctx.tx[sfAccount]);
// If the pk is empty and not simulate or simulate and signers, // If the pk is empty and not simulate or simulate and signers,
// then we must be multi-signing. // then we must be multi-signing.
if (ctx.tx.isFieldPresent(sfSigners)) if (ctx.tx.isFieldPresent(sfSigners))
{ {
STArray const& txSigners(ctx.tx.getFieldArray(sfSigners)); return checkMultiSign(ctx, idAccount, sigObject);
return checkMultiSign(ctx.view, idAccount, txSigners, ctx.flags, ctx.j);
} }
// Check Single Sign // Check Single Sign
XRPL_ASSERT( XRPL_ASSERT(
!pkSigner.empty(), !pkSigner.empty(), "ripple::Transactor::checkSign : non-empty signer");
"ripple::Transactor::checkSingleSign : non-empty signer or simulation");
if (!publicKeyType(makeSlice(pkSigner))) if (!publicKeyType(makeSlice(pkSigner)))
{ {
JLOG(ctx.j.trace()) JLOG(ctx.j.trace()) << "checkSign: signing public key type is unknown";
<< "checkSingleSign: signing public key type is unknown";
return tefBAD_AUTH; // FIXME: should be better error! return tefBAD_AUTH; // FIXME: should be better error!
} }
// Look up the account.
auto const idSigner = pkSigner.empty() auto const idSigner = pkSigner.empty()
? idAccount ? idAccount
: calcAccountID(PublicKey(makeSlice(pkSigner))); : calcAccountID(PublicKey(makeSlice(pkSigner)));
@@ -711,8 +711,16 @@ Transactor::checkSign(PreclaimContext const& ctx)
if (!sleAccount) if (!sleAccount)
return terNO_ACCOUNT; return terNO_ACCOUNT;
return checkSingleSign( return checkSingleSign(ctx, idSigner, idAccount, sleAccount);
idSigner, idAccount, sleAccount, ctx.view.rules(), ctx.j); }
NotTEC
Transactor::checkSign(PreclaimContext const& ctx)
{
auto const idAccount = ctx.tx.isFieldPresent(sfDelegate)
? ctx.tx.getAccountID(sfDelegate)
: ctx.tx.getAccountID(sfAccount);
return checkSign(ctx, idAccount, ctx.tx);
} }
NotTEC NotTEC
@@ -727,9 +735,7 @@ Transactor::checkBatchSign(PreclaimContext const& ctx)
Blob const& pkSigner = signer.getFieldVL(sfSigningPubKey); Blob const& pkSigner = signer.getFieldVL(sfSigningPubKey);
if (pkSigner.empty()) if (pkSigner.empty())
{ {
STArray const& txSigners(signer.getFieldArray(sfSigners)); if (ret = checkMultiSign(ctx, idAccount, signer);
if (ret = checkMultiSign(
ctx.view, idAccount, txSigners, ctx.flags, ctx.j);
!isTesSuccess(ret)) !isTesSuccess(ret))
return ret; return ret;
} }
@@ -753,8 +759,7 @@ Transactor::checkBatchSign(PreclaimContext const& ctx)
return tesSUCCESS; return tesSUCCESS;
} }
if (ret = checkSingleSign( if (ret = checkSingleSign(ctx, idSigner, idAccount, sleAccount);
idSigner, idAccount, sleAccount, ctx.view.rules(), ctx.j);
!isTesSuccess(ret)) !isTesSuccess(ret))
return ret; return ret;
} }
@@ -764,15 +769,14 @@ Transactor::checkBatchSign(PreclaimContext const& ctx)
NotTEC NotTEC
Transactor::checkSingleSign( Transactor::checkSingleSign(
PreclaimContext const& ctx,
AccountID const& idSigner, AccountID const& idSigner,
AccountID const& idAccount, AccountID const& idAccount,
std::shared_ptr<SLE const> sleAccount, std::shared_ptr<SLE const> sleAccount)
Rules const& rules,
beast::Journal j)
{ {
bool const isMasterDisabled = sleAccount->isFlag(lsfDisableMaster); bool const isMasterDisabled = sleAccount->isFlag(lsfDisableMaster);
if (rules.enabled(fixMasterKeyAsRegularKey)) if (ctx.view.rules().enabled(fixMasterKeyAsRegularKey))
{ {
// Signed with regular key. // Signed with regular key.
if ((*sleAccount)[~sfRegularKey] == idSigner) if ((*sleAccount)[~sfRegularKey] == idSigner)
@@ -809,14 +813,16 @@ Transactor::checkSingleSign(
else if (sleAccount->isFieldPresent(sfRegularKey)) else if (sleAccount->isFieldPresent(sfRegularKey))
{ {
// Signing key does not match master or regular key. // Signing key does not match master or regular key.
JLOG(j.trace()) << "checkSingleSign: Not authorized to use account."; JLOG(ctx.j.trace())
<< "checkSingleSign: Not authorized to use account.";
return tefBAD_AUTH; return tefBAD_AUTH;
} }
else else
{ {
// No regular key on account and signing key does not match master key. // No regular key on account and signing key does not match master key.
// FIXME: Why differentiate this case from tefBAD_AUTH? // FIXME: Why differentiate this case from tefBAD_AUTH?
JLOG(j.trace()) << "checkSingleSign: Not authorized to use account."; JLOG(ctx.j.trace())
<< "checkSingleSign: Not authorized to use account.";
return tefBAD_AUTH_MASTER; return tefBAD_AUTH_MASTER;
} }
@@ -825,19 +831,17 @@ Transactor::checkSingleSign(
NotTEC NotTEC
Transactor::checkMultiSign( Transactor::checkMultiSign(
ReadView const& view, PreclaimContext const& ctx,
AccountID const& id, AccountID const& id,
STArray const& txSigners, STObject const& sigObject)
ApplyFlags const& flags,
beast::Journal j)
{ {
// Get mTxnAccountID's SignerList and Quorum. // Get id's SignerList and Quorum.
std::shared_ptr<STLedgerEntry const> sleAccountSigners = std::shared_ptr<STLedgerEntry const> sleAccountSigners =
view.read(keylet::signers(id)); ctx.view.read(keylet::signers(id));
// If the signer list doesn't exist the account is not multi-signing. // If the signer list doesn't exist the account is not multi-signing.
if (!sleAccountSigners) if (!sleAccountSigners)
{ {
JLOG(j.trace()) JLOG(ctx.j.trace())
<< "applyTransaction: Invalid: Not a multi-signing account."; << "applyTransaction: Invalid: Not a multi-signing account.";
return tefNOT_MULTI_SIGNING; return tefNOT_MULTI_SIGNING;
} }
@@ -852,11 +856,12 @@ Transactor::checkMultiSign(
"ripple::Transactor::checkMultiSign : signer list ID is 0"); "ripple::Transactor::checkMultiSign : signer list ID is 0");
auto accountSigners = auto accountSigners =
SignerEntries::deserialize(*sleAccountSigners, j, "ledger"); SignerEntries::deserialize(*sleAccountSigners, ctx.j, "ledger");
if (!accountSigners) if (!accountSigners)
return accountSigners.error(); return accountSigners.error();
// Get the array of transaction signers. // Get the array of transaction signers.
STArray const& txSigners(sigObject.getFieldArray(sfSigners));
// Walk the accountSigners performing a variety of checks and see if // Walk the accountSigners performing a variety of checks and see if
// the quorum is met. // the quorum is met.
@@ -875,7 +880,7 @@ Transactor::checkMultiSign(
{ {
if (++iter == accountSigners->end()) if (++iter == accountSigners->end())
{ {
JLOG(j.trace()) JLOG(ctx.j.trace())
<< "applyTransaction: Invalid SigningAccount.Account."; << "applyTransaction: Invalid SigningAccount.Account.";
return tefBAD_SIGNATURE; return tefBAD_SIGNATURE;
} }
@@ -883,7 +888,7 @@ Transactor::checkMultiSign(
if (iter->account != txSignerAcctID) if (iter->account != txSignerAcctID)
{ {
// The SigningAccount is not in the SignerEntries. // The SigningAccount is not in the SignerEntries.
JLOG(j.trace()) JLOG(ctx.j.trace())
<< "applyTransaction: Invalid SigningAccount.Account."; << "applyTransaction: Invalid SigningAccount.Account.";
return tefBAD_SIGNATURE; return tefBAD_SIGNATURE;
} }
@@ -897,13 +902,13 @@ Transactor::checkMultiSign(
// STTx::checkMultiSign // STTx::checkMultiSign
if (!spk.empty() && !publicKeyType(makeSlice(spk))) if (!spk.empty() && !publicKeyType(makeSlice(spk)))
{ {
JLOG(j.trace()) JLOG(ctx.j.trace())
<< "checkMultiSign: signing public key type is unknown"; << "checkMultiSign: signing public key type is unknown";
return tefBAD_SIGNATURE; return tefBAD_SIGNATURE;
} }
XRPL_ASSERT( XRPL_ASSERT(
(flags & tapDRY_RUN) || !spk.empty(), (ctx.flags & tapDRY_RUN) || !spk.empty(),
"ripple::Transactor::checkMultiSign : non-empty signer or " "ripple::Transactor::checkMultiSign : non-empty signer or "
"simulation"); "simulation");
AccountID const signingAcctIDFromPubKey = spk.empty() AccountID const signingAcctIDFromPubKey = spk.empty()
@@ -935,7 +940,8 @@ Transactor::checkMultiSign(
// In any of these cases we need to know whether the account is in // In any of these cases we need to know whether the account is in
// the ledger. Determine that now. // the ledger. Determine that now.
auto const sleTxSignerRoot = view.read(keylet::account(txSignerAcctID)); auto const sleTxSignerRoot =
ctx.view.read(keylet::account(txSignerAcctID));
if (signingAcctIDFromPubKey == txSignerAcctID) if (signingAcctIDFromPubKey == txSignerAcctID)
{ {
@@ -948,7 +954,7 @@ Transactor::checkMultiSign(
if (signerAccountFlags & lsfDisableMaster) if (signerAccountFlags & lsfDisableMaster)
{ {
JLOG(j.trace()) JLOG(ctx.j.trace())
<< "applyTransaction: Signer:Account lsfDisableMaster."; << "applyTransaction: Signer:Account lsfDisableMaster.";
return tefMASTER_DISABLED; return tefMASTER_DISABLED;
} }
@@ -960,21 +966,21 @@ Transactor::checkMultiSign(
// Public key must hash to the account's regular key. // Public key must hash to the account's regular key.
if (!sleTxSignerRoot) if (!sleTxSignerRoot)
{ {
JLOG(j.trace()) << "applyTransaction: Non-phantom signer " JLOG(ctx.j.trace()) << "applyTransaction: Non-phantom signer "
"lacks account root."; "lacks account root.";
return tefBAD_SIGNATURE; return tefBAD_SIGNATURE;
} }
if (!sleTxSignerRoot->isFieldPresent(sfRegularKey)) if (!sleTxSignerRoot->isFieldPresent(sfRegularKey))
{ {
JLOG(j.trace()) JLOG(ctx.j.trace())
<< "applyTransaction: Account lacks RegularKey."; << "applyTransaction: Account lacks RegularKey.";
return tefBAD_SIGNATURE; return tefBAD_SIGNATURE;
} }
if (signingAcctIDFromPubKey != if (signingAcctIDFromPubKey !=
sleTxSignerRoot->getAccountID(sfRegularKey)) sleTxSignerRoot->getAccountID(sfRegularKey))
{ {
JLOG(j.trace()) JLOG(ctx.j.trace())
<< "applyTransaction: Account doesn't match RegularKey."; << "applyTransaction: Account doesn't match RegularKey.";
return tefBAD_SIGNATURE; return tefBAD_SIGNATURE;
} }
@@ -986,7 +992,8 @@ Transactor::checkMultiSign(
// Cannot perform transaction if quorum is not met. // Cannot perform transaction if quorum is not met.
if (weightSum < sleAccountSigners->getFieldU32(sfSignerQuorum)) if (weightSum < sleAccountSigners->getFieldU32(sfSignerQuorum))
{ {
JLOG(j.trace()) << "applyTransaction: Signers failed to meet quorum."; JLOG(ctx.j.trace())
<< "applyTransaction: Signers failed to meet quorum.";
return tefBAD_QUORUM; return tefBAD_QUORUM;
} }

19
src/xrpld/app/tx/detail/Transactor.h
View File

@@ -281,6 +281,12 @@ protected:
static XRPAmount static XRPAmount
calculateOwnerReserveFee(ReadView const& view, STTx const& tx); calculateOwnerReserveFee(ReadView const& view, STTx const& tx);
static NotTEC
checkSign(
PreclaimContext const& ctx,
AccountID const& id,
STObject const& sigObject);
// Base class always returns true // Base class always returns true
static bool static bool
checkExtraFeatures(PreflightContext const& ctx); checkExtraFeatures(PreflightContext const& ctx);
@@ -317,18 +323,15 @@ private:
payFee(); payFee();
static NotTEC static NotTEC
checkSingleSign( checkSingleSign(
PreclaimContext const& ctx,
AccountID const& idSigner, AccountID const& idSigner,
AccountID const& idAccount, AccountID const& idAccount,
std::shared_ptr<SLE const> sleAccount, std::shared_ptr<SLE const> sleAccount);
Rules const& rules,
beast::Journal j);
static NotTEC static NotTEC
checkMultiSign( checkMultiSign(
ReadView const& view, PreclaimContext const& ctx,
AccountID const& idAccount, AccountID const& id,
STArray const& txSigners, STObject const& sigObject);
ApplyFlags const& flags,
beast::Journal j);
void trapTransaction(uint256) const; void trapTransaction(uint256) const;