Refactor 4: Transactor extra signing support

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 =

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);

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
 {

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<NotTEC>
+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<STLedgerEntry const> 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.

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<NotTEC>
+preflightCheckSimulateKeys(
+    ApplyFlags flags,
+    STObject const& sigObject,
+    beast::Journal j);
+
 /** Checks whether the signature appears valid */
 NotTEC
 preflight2(PreflightContext const& ctx);