From fb5d94bbefcd09bf64be711c8bf534b42850edd4 Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Sat, 17 May 2025 15:12:54 +0100 Subject: [PATCH] Refactor 4: Transactor extra signing support --- include/xrpl/protocol/SField.h | 4 +- src/test/jtx/Account.h | 4 ++ src/test/jtx/impl/Account.cpp | 8 +++ src/xrpld/app/tx/detail/Transactor.cpp | 77 +++++++++++++++++--------- src/xrpld/app/tx/detail/Transactor.h | 26 ++++++++- 5 files changed, 90 insertions(+), 29 deletions(-) diff --git a/include/xrpl/protocol/SField.h b/include/xrpl/protocol/SField.h index a72332b888..c8e5d6dca3 100644 --- a/include/xrpl/protocol/SField.h +++ b/include/xrpl/protocol/SField.h @@ -149,8 +149,8 @@ public: sMD_ChangeNew = 0x02, // new value when it changes sMD_DeleteFinal = 0x04, // final value when it is deleted sMD_Create = 0x08, // value when it's created - sMD_Always = 0x10, // value when node containing it is affected at all - sMD_BaseTen = 0x20, // value is treated as base 10, overriding behavior + sMD_Always = 0x10, // value when node containing it is affected at all + sMD_BaseTen = 0x20, // value is treated as base 10, overriding behavior sMD_PseudoAccount = 0x40, // if this field is set in an ACCOUNT_ROOT // _only_, then it is a pseudo-account sMD_Default = diff --git a/src/test/jtx/Account.h b/src/test/jtx/Account.h index d91bb4a383..2e6987d32b 100644 --- a/src/test/jtx/Account.h +++ b/src/test/jtx/Account.h @@ -74,6 +74,10 @@ public: /** @} */ + /** Create an amount from an account ID. Should only be used when the secret + * key is unavailable, such as for pseudo-accounts. */ + explicit Account(std::string name, AccountID const& id); + enum AcctStringType { base58Seed, other }; /** Create an account from a base58 seed string. Throws on invalid seed. */ Account(AcctStringType stringType, std::string base58SeedStr); diff --git a/src/test/jtx/impl/Account.cpp b/src/test/jtx/impl/Account.cpp index b61048e66f..fe901848f8 100644 --- a/src/test/jtx/impl/Account.cpp +++ b/src/test/jtx/impl/Account.cpp @@ -86,6 +86,14 @@ Account::Account(AcctStringType stringType, std::string base58SeedStr) { } +Account::Account(std::string name, AccountID const& id) + : Account(name, randomKeyPair(KeyType::secp256k1), privateCtorTag{}) +{ + // override the randomly generated values + id_ = id; + human_ = toBase58(id_); +} + IOU Account::operator[](std::string const& s) const { diff --git a/src/xrpld/app/tx/detail/Transactor.cpp b/src/xrpld/app/tx/detail/Transactor.cpp index 33b899d823..1a93476e0d 100644 --- a/src/xrpld/app/tx/detail/Transactor.cpp +++ b/src/xrpld/app/tx/detail/Transactor.cpp @@ -160,26 +160,28 @@ preflight1(PreflightContext const& ctx, std::uint32_t flagMask) return tesSUCCESS; } -/** Checks whether the signature appears valid */ -NotTEC -preflight2(PreflightContext const& ctx) +std::optional +preflightCheckSimulateKeys( + ApplyFlags flags, + STObject const& sigObject, + beast::Journal j) { - if (ctx.flags & tapDRY_RUN) // simulation + if (flags & tapDRY_RUN) // simulation { - if (!ctx.tx.getSignature().empty()) + if (!sigObject.getFieldVL(sfTxnSignature).empty()) { // NOTE: This code should never be hit because it's checked in the // `simulate` RPC return temINVALID; // LCOV_EXCL_LINE } - if (!ctx.tx.isFieldPresent(sfSigners)) + if (!sigObject.isFieldPresent(sfSigners)) { // no signers, no signature - a valid simulation return tesSUCCESS; } - for (auto const& signer : ctx.tx.getFieldArray(sfSigners)) + for (auto const& signer : sigObject.getFieldArray(sfSigners)) { if (signer.isFieldPresent(sfTxnSignature) && !signer[sfTxnSignature].empty()) @@ -191,6 +193,17 @@ preflight2(PreflightContext const& ctx) } return tesSUCCESS; } + return {}; +} + +/** Checks whether the signature appears valid */ +NotTEC +preflight2(PreflightContext const& ctx) +{ + if (auto const ret = preflightCheckSimulateKeys(ctx.flags, ctx.tx, ctx.j)) + // Skips following checks if the transaction is being simulated, + // regardless of success or failure + return *ret; auto const sigValid = checkValidity( ctx.app.getHashRouter(), ctx.tx, ctx.rules, ctx.app.config()); @@ -609,28 +622,44 @@ Transactor::apply() } NotTEC -Transactor::checkSign(PreclaimContext const& ctx) +Transactor::checkSign( + PreclaimContext const& ctx, + AccountID const& id, + STObject const& sigObject) { if (ctx.flags & tapDRY_RUN) { // This code must be different for `simulate` // Since the public key may be empty even for single signing - if (ctx.tx.isFieldPresent(sfSigners)) - return checkMultiSign(ctx); - return checkSingleSign(ctx); + if (sigObject.isFieldPresent(sfSigners)) + return checkMultiSign(ctx, id, sigObject); + return checkSingleSign(ctx, id, sigObject); } // If the pk is empty, then we must be multi-signing. - if (ctx.tx.getSigningPubKey().empty()) - return checkMultiSign(ctx); + if (sigObject.getFieldVL(sfSigningPubKey).empty()) + return checkMultiSign(ctx, id, sigObject); - return checkSingleSign(ctx); + return checkSingleSign(ctx, id, sigObject); } NotTEC -Transactor::checkSingleSign(PreclaimContext const& ctx) +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); +} + +// TODO generalize +NotTEC +Transactor::checkSingleSign( + PreclaimContext const& ctx, + AccountID const& idAccount, + STObject const& sigObject) { // Check that the value in the signing key slot is a public key. - auto const pkSigner = ctx.tx.getSigningPubKey(); + auto const pkSigner = sigObject.getFieldVL(sfSigningPubKey); if (!(ctx.flags & tapDRY_RUN) && !publicKeyType(makeSlice(pkSigner))) { JLOG(ctx.j.trace()) @@ -639,9 +668,6 @@ Transactor::checkSingleSign(PreclaimContext const& ctx) } // Look up the account. - auto const idAccount = ctx.tx.isFieldPresent(sfDelegate) - ? ctx.tx.getAccountID(sfDelegate) - : ctx.tx.getAccountID(sfAccount); auto const sleAccount = ctx.view.read(keylet::account(idAccount)); if (!sleAccount) return terNO_ACCOUNT; @@ -708,13 +734,14 @@ Transactor::checkSingleSign(PreclaimContext const& ctx) return tesSUCCESS; } +// TODO generalize NotTEC -Transactor::checkMultiSign(PreclaimContext const& ctx) +Transactor::checkMultiSign( + PreclaimContext const& ctx, + AccountID const& id, + STObject const& sigObject) { - auto const id = ctx.tx.isFieldPresent(sfDelegate) - ? ctx.tx.getAccountID(sfDelegate) - : ctx.tx.getAccountID(sfAccount); - // Get mTxnAccountID's SignerList and Quorum. + // Get id's SignerList and Quorum. std::shared_ptr sleAccountSigners = ctx.view.read(keylet::signers(id)); // If the signer list doesn't exist the account is not multi-signing. @@ -740,7 +767,7 @@ Transactor::checkMultiSign(PreclaimContext const& ctx) return accountSigners.error(); // Get the array of transaction signers. - STArray const& txSigners(ctx.tx.getFieldArray(sfSigners)); + STArray const& txSigners(sigObject.getFieldArray(sfSigners)); // Walk the accountSigners performing a variety of checks and see if // the quorum is met. diff --git a/src/xrpld/app/tx/detail/Transactor.h b/src/xrpld/app/tx/detail/Transactor.h index ba33e0cd13..cf57017e3e 100644 --- a/src/xrpld/app/tx/detail/Transactor.h +++ b/src/xrpld/app/tx/detail/Transactor.h @@ -217,6 +217,12 @@ protected: static XRPAmount calculateOwnerReserveFee(ReadView const& view, STTx const& tx); + static NotTEC + checkSign( + PreclaimContext const& ctx, + AccountID const& id, + STObject const& sigObject); + // Base class always returns true static bool isEnabled(PreflightContext const& ctx); @@ -248,9 +254,15 @@ private: TER payFee(); static NotTEC - checkSingleSign(PreclaimContext const& ctx); + checkSingleSign( + PreclaimContext const& ctx, + AccountID const& id, + STObject const& sigObject); static NotTEC - checkMultiSign(PreclaimContext const& ctx); + checkMultiSign( + PreclaimContext const& ctx, + AccountID const& id, + STObject const& sigObject); void trapTransaction(uint256) const; }; @@ -281,6 +293,16 @@ preflightCheckSigningKey(STObject const& sigObject, beast::Journal j); NotTEC preflight1(PreflightContext const& ctx, std::uint32_t flagMask); +/** Checks the special signing key state needed for simulation + * + * Normally called from preflight2 with ctx.tx. + */ +std::optional +preflightCheckSimulateKeys( + ApplyFlags flags, + STObject const& sigObject, + beast::Journal j); + /** Checks whether the signature appears valid */ NotTEC preflight2(PreflightContext const& ctx);