diff --git a/src/ripple/app/tx/impl/Transactor.cpp b/src/ripple/app/tx/impl/Transactor.cpp index 80646172a0..1543d784dc 100644 --- a/src/ripple/app/tx/impl/Transactor.cpp +++ b/src/ripple/app/tx/impl/Transactor.cpp @@ -289,8 +289,40 @@ Transactor::calculateBaseFee(ReadView const& view, STTx const& tx) // Each signer adds one more baseFee to the minimum required fee // for the transaction. - std::size_t const signerCount = - tx.isFieldPresent(sfSigners) ? tx.getFieldArray(sfSigners).size() : 0; + std::size_t signerCount = 0; + if (tx.isFieldPresent(sfSigners)) + { + // Define recursive lambda to count all leaf signers + std::function countSigners; + + countSigners = [&](STArray const& signers) -> std::size_t { + std::size_t count = 0; + + for (auto const& signer : signers) + { + if (signer.isFieldPresent(sfSigners)) + { + // This is a nested signer - recursively count its signers + count += countSigners(signer.getFieldArray(sfSigners)); + } + else + { + // This is a leaf signer (one who actually signs) + // Count it only if it has signing fields (not just a + // placeholder) + if (signer.isFieldPresent(sfSigningPubKey) && + signer.isFieldPresent(sfTxnSignature)) + { + count += 1; + } + } + } + + return count; + }; + + signerCount = countSigners(tx.getFieldArray(sfSigners)); + } XRPAmount hookExecutionFee{0}; uint64_t burden{1}; @@ -923,157 +955,282 @@ NotTEC Transactor::checkMultiSign(PreclaimContext const& ctx) { auto const id = ctx.tx.getAccountID(sfAccount); - // Get mTxnAccountID'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. - if (!sleAccountSigners) - { - JLOG(ctx.j.trace()) - << "applyTransaction: Invalid: Not a multi-signing account."; - return tefNOT_MULTI_SIGNING; - } - - // We have plans to support multiple SignerLists in the future. The - // presence and defaulted value of the SignerListID field will enable that. - assert(sleAccountSigners->isFieldPresent(sfSignerListID)); - assert(sleAccountSigners->getFieldU32(sfSignerListID) == 0); - - auto accountSigners = - SignerEntries::deserialize(*sleAccountSigners, ctx.j, "ledger"); - if (!accountSigners) - return accountSigners.error(); - // Get the array of transaction signers. - STArray const& txSigners(ctx.tx.getFieldArray(sfSigners)); - - // Walk the accountSigners performing a variety of checks and see if - // the quorum is met. - - // Both the multiSigners and accountSigners are sorted by account. So - // matching multi-signers to account signers should be a simple - // linear walk. *All* signers must be valid or the transaction fails. - std::uint32_t weightSum = 0; - auto iter = accountSigners->begin(); - for (auto const& txSigner : txSigners) - { - AccountID const txSignerAcctID = txSigner.getAccountID(sfAccount); + // Set max depth based on feature flag + bool const allowNested = ctx.view.rules().enabled(featureNestedMultiSign); + int const maxDepth = allowNested ? 4 : 1; + + // Define recursive lambda for checking signers at any depth + // ancestors tracks the signing chain to detect cycles + std::function)> + validateSigners; + + validateSigners = [&](AccountID const& acc, + STArray const& signers, + int depth, + std::set ancestors) -> NotTEC { + // Cycle detection: if we're already validating this account up the + // chain it cannot contribute - but this isn't an error, just + // unavailable weight + if (ancestors.count(acc)) + { + JLOG(ctx.j.trace()) + << "checkMultiSign: Cyclic signer detected: " << acc; + return tesSUCCESS; + } - // Attempt to match the SignerEntry with a Signer; - while (iter->account < txSignerAcctID) + // Check depth limit + if (depth > maxDepth) { - if (++iter == accountSigners->end()) + if (allowNested) { JLOG(ctx.j.trace()) - << "applyTransaction: Invalid SigningAccount.Account."; + << "checkMultiSign: Multi-signing depth limit exceeded."; return tefBAD_SIGNATURE; } + + JLOG(ctx.j.warn()) + << "checkMultiSign: Nested multisigning disabled."; + return temMALFORMED; } - if (iter->account != txSignerAcctID) + + ancestors.insert(acc); + + // Get the SignerList for the account we're validating signers for + std::shared_ptr sleAllowedSigners = + ctx.view.read(keylet::signers(acc)); + + if (!sleAllowedSigners) { - // The SigningAccount is not in the SignerEntries. - JLOG(ctx.j.trace()) - << "applyTransaction: Invalid SigningAccount.Account."; - return tefBAD_SIGNATURE; + JLOG(ctx.j.trace()) << "checkMultiSign: Account " << acc + << " not set up for multi-signing."; + return tefNOT_MULTI_SIGNING; } - // We found the SigningAccount in the list of valid signers. Now we - // need to compute the accountID that is associated with the signer's - // public key. - auto const spk = txSigner.getFieldVL(sfSigningPubKey); + uint32_t const quorum = sleAllowedSigners->getFieldU32(sfSignerQuorum); + uint32_t sum{0}; - if (!publicKeyType(makeSlice(spk))) + auto allowedSigners = + SignerEntries::deserialize(*sleAllowedSigners, ctx.j, "ledger"); + if (!allowedSigners) + return allowedSigners.error(); + + // Build lookup map for O(1) signer validation and weight retrieval + std::map signerWeights; + uint32_t totalWeight{0}, cyclicWeight{0}; + for (auto const& entry : *allowedSigners) { - JLOG(ctx.j.trace()) - << "checkMultiSign: signing public key type is unknown"; - return tefBAD_SIGNATURE; + signerWeights[entry.account] = entry.weight; + totalWeight += entry.weight; + if (ancestors.count(entry.account)) + cyclicWeight += entry.weight; } - AccountID const signingAcctIDFromPubKey = - calcAccountID(PublicKey(makeSlice(spk))); - - // Verify that the signingAcctID and the signingAcctIDFromPubKey - // belong together. Here is are the rules: - // - // 1. "Phantom account": an account that is not in the ledger - // A. If signingAcctID == signingAcctIDFromPubKey and the - // signingAcctID is not in the ledger then we have a phantom - // account. - // B. Phantom accounts are always allowed as multi-signers. - // - // 2. "Master Key" - // A. signingAcctID == signingAcctIDFromPubKey, and signingAcctID - // is in the ledger. - // B. If the signingAcctID in the ledger does not have the - // asfDisableMaster flag set, then the signature is allowed. - // - // 3. "Regular Key" - // A. signingAcctID != signingAcctIDFromPubKey, and signingAcctID - // is in the ledger. - // B. If signingAcctIDFromPubKey == signingAcctID.RegularKey (from - // ledger) then the signature is allowed. - // - // No other signatures are allowed. (January 2015) - - // In any of these cases we need to know whether the account is in - // the ledger. Determine that now. - auto sleTxSignerRoot = ctx.view.read(keylet::account(txSignerAcctID)); - - if (signingAcctIDFromPubKey == txSignerAcctID) + // Walk the signers array, validating each signer + // Signers must be in strict ascending order for consensus + std::optional prevSigner; + + for (auto const& signerEntry : signers) { - // Either Phantom or Master. Phantoms automatically pass. - if (sleTxSignerRoot) + AccountID const signer = signerEntry.getAccountID(sfAccount); + bool const isNested = signerEntry.isFieldPresent(sfSigners); + + // Enforce strict ascending order (required for consensus) + if (prevSigner && signer <= *prevSigner) { - // Master Key. Account may not have asfDisableMaster set. - std::uint32_t const signerAccountFlags = - sleTxSignerRoot->getFieldU32(sfFlags); + JLOG(ctx.j.trace()) + << "checkMultiSign: Signers not in strict ascending order: " + << signer << " <= " << *prevSigner; + return temMALFORMED; + } + prevSigner = signer; - if (signerAccountFlags & lsfDisableMaster) - { - JLOG(ctx.j.trace()) - << "applyTransaction: Signer:Account lsfDisableMaster."; - return tefMASTER_DISABLED; - } + // Skip cyclic signers - they cannot contribute at this level + if (ancestors.count(signer)) + { + JLOG(ctx.j.trace()) + << "checkMultiSign: Skipping cyclic signer: " << signer; + continue; } - } - else - { - // May be a Regular Key. Let's find out. - // Public key must hash to the account's regular key. - if (!sleTxSignerRoot) + + // Lookup signer in authorized set + auto const weightIt = signerWeights.find(signer); + if (weightIt == signerWeights.end()) { - JLOG(ctx.j.trace()) << "applyTransaction: Non-phantom signer " - "lacks account root."; + JLOG(ctx.j.trace()) + << "checkMultiSign: Invalid signer " << signer + << " not in signer list for " << acc; return tefBAD_SIGNATURE; } + uint16_t const weight = weightIt->second; - if (!sleTxSignerRoot->isFieldPresent(sfRegularKey)) + // Check if this signer has nested signers (delegation) + if (isNested) { + // This is a nested multi-signer that delegates to sub-signers + if (signerEntry.isFieldPresent(sfSigningPubKey) || + signerEntry.isFieldPresent(sfTxnSignature)) + { + JLOG(ctx.j.trace()) << "checkMultiSign: Signer " << signer + << " cannot have both nested signers " + "and signature fields."; + return tefBAD_SIGNATURE; + } + + // Recursively validate the nested signers against signer's + // signer list + STArray const& nestedSigners = + signerEntry.getFieldArray(sfSigners); + NotTEC result = validateSigners( + signer, nestedSigners, depth + 1, ancestors); + if (!isTesSuccess(result)) + return result; + + // Nested signers met their quorum - add this signer's weight + sum += weight; + JLOG(ctx.j.trace()) - << "applyTransaction: Account lacks RegularKey."; - return tefBAD_SIGNATURE; + << "checkMultiSign: Nested signer " << signer + << " validated, weight=" << weight << ", depth=" << depth + << ", sum=" << sum << "/" << quorum; } - if (signingAcctIDFromPubKey != - sleTxSignerRoot->getAccountID(sfRegularKey)) + else { + // This is a leaf signer - validate signature + if (!signerEntry.isFieldPresent(sfSigningPubKey) || + !signerEntry.isFieldPresent(sfTxnSignature)) + { + JLOG(ctx.j.trace()) + << "checkMultiSign: Leaf signer " << signer + << " must have SigningPubKey and TxnSignature."; + return tefBAD_SIGNATURE; + } + + auto const spk = signerEntry.getFieldVL(sfSigningPubKey); + + if (!publicKeyType(makeSlice(spk))) + { + JLOG(ctx.j.trace()) + << "checkMultiSign: Unknown public key type for signer " + << signer; + return tefBAD_SIGNATURE; + } + + AccountID const signingAcctIDFromPubKey = + calcAccountID(PublicKey(makeSlice(spk))); + + auto sleTxSignerRoot = ctx.view.read(keylet::account(signer)); + + if (signingAcctIDFromPubKey == signer) + { + if (sleTxSignerRoot) + { + std::uint32_t const signerAccountFlags = + sleTxSignerRoot->getFieldU32(sfFlags); + + if (signerAccountFlags & lsfDisableMaster) + { + JLOG(ctx.j.trace()) + << "checkMultiSign: Signer " << signer + << " has lsfDisableMaster set."; + return tefMASTER_DISABLED; + } + } + } + else + { + if (!sleTxSignerRoot) + { + JLOG(ctx.j.trace()) + << "checkMultiSign: Non-phantom signer " << signer + << " lacks account root."; + return tefBAD_SIGNATURE; + } + + if (!sleTxSignerRoot->isFieldPresent(sfRegularKey)) + { + JLOG(ctx.j.trace()) << "checkMultiSign: Signer " + << signer << " lacks RegularKey."; + return tefBAD_SIGNATURE; + } + if (signingAcctIDFromPubKey != + sleTxSignerRoot->getAccountID(sfRegularKey)) + { + JLOG(ctx.j.trace()) + << "checkMultiSign: Signer " << signer + << " pubkey doesn't match RegularKey."; + return tefBAD_SIGNATURE; + } + } + + // Valid leaf signer - add their weight + sum += weight; + JLOG(ctx.j.trace()) - << "applyTransaction: Account doesn't match RegularKey."; - return tefBAD_SIGNATURE; + << "checkMultiSign: Leaf signer " << signer + << " validated, weight=" << weight << ", depth=" << depth + << ", sum=" << sum << "/" << quorum; } } - // The signer is legitimate. Add their weight toward the quorum. - weightSum += iter->weight; - } - // Cannot perform transaction if quorum is not met. - if (weightSum < sleAccountSigners->getFieldU32(sfSignerQuorum)) + // Calculate effective quorum, relaxing for cyclic lockout scenarios + // Sanity check: cyclicWeight must not exceed totalWeight (underflow + // guard) + if (cyclicWeight > totalWeight) + { + JLOG(ctx.j.error()) << "checkMultiSign: Invariant violation for " + << acc << ": cyclicWeight (" << cyclicWeight + << ") > totalWeight (" << totalWeight << ")"; + return tefINTERNAL; + } + + uint32_t effectiveQuorum = quorum; + uint32_t const maxAchievable = totalWeight - cyclicWeight; + + if (cyclicWeight > 0 && maxAchievable < quorum) + { + JLOG(ctx.j.warn()) + << "checkMultiSign: Cyclic lockout detected for " << acc + << ": relaxing quorum from " << quorum << " to " + << maxAchievable << " (total=" << totalWeight + << ", cyclic=" << cyclicWeight << ")"; + effectiveQuorum = maxAchievable; + } + + // Sanity check: effectiveQuorum of 0 means all signers are cyclic - + // this is an irrecoverable misconfiguration + if (effectiveQuorum == 0) + { + JLOG(ctx.j.warn()) << "checkMultiSign: All signers for " << acc + << " are cyclic - no valid signing path exists."; + return tefBAD_QUORUM; + } + + // Check if accumulated weight meets required quorum + if (sum < effectiveQuorum) + { + JLOG(ctx.j.trace()) << "checkMultiSign: Quorum not met for " << acc + << " at depth " << depth << " (sum=" << sum + << ", required=" << effectiveQuorum << ")"; + return tefBAD_QUORUM; + } + + return tesSUCCESS; + }; + + STArray const& entries(ctx.tx.getFieldArray(sfSigners)); + + // Initial call with empty ancestor set - the function inserts acc after + // cycle check + NotTEC result = validateSigners(id, entries, 1, {}); + if (!isTesSuccess(result)) { JLOG(ctx.j.trace()) - << "applyTransaction: Signers failed to meet quorum."; - return tefBAD_QUORUM; + << "checkMultiSign: Validation failed with " << transToken(result); + return result; } - // Met the quorum. Continue. return tesSUCCESS; } diff --git a/src/ripple/protocol/Feature.h b/src/ripple/protocol/Feature.h index 2766859dc7..63dc9f5ac2 100644 --- a/src/ripple/protocol/Feature.h +++ b/src/ripple/protocol/Feature.h @@ -74,7 +74,7 @@ namespace detail { // Feature.cpp. Because it's only used to reserve storage, and determine how // large to make the FeatureBitset, it MAY be larger. It MUST NOT be less than // the actual number of amendments. A LogicError on startup will verify this. -static constexpr std::size_t numFeatures = 90; +static constexpr std::size_t numFeatures = 91; /** Amendments that this server supports and the default voting behavior. Whether they are enabled depends on the Rules defined in the validated @@ -378,6 +378,7 @@ extern uint256 const fixInvalidTxFlags; extern uint256 const featureExtendedHookState; extern uint256 const fixCronStacking; extern uint256 const fixHookAPI20251128; +extern uint256 const featureNestedMultiSign; } // namespace ripple #endif diff --git a/src/ripple/protocol/impl/Feature.cpp b/src/ripple/protocol/impl/Feature.cpp index 24383f896c..47b71b484a 100644 --- a/src/ripple/protocol/impl/Feature.cpp +++ b/src/ripple/protocol/impl/Feature.cpp @@ -484,6 +484,7 @@ REGISTER_FIX (fixInvalidTxFlags, Supported::yes, VoteBehavior::De REGISTER_FEATURE(ExtendedHookState, Supported::yes, VoteBehavior::DefaultNo); REGISTER_FIX (fixCronStacking, Supported::yes, VoteBehavior::DefaultYes); REGISTER_FIX (fixHookAPI20251128, Supported::yes, VoteBehavior::DefaultYes); +REGISTER_FEATURE(NestedMultiSign, Supported::yes, VoteBehavior::DefaultNo); // The following amendments are obsolete, but must remain supported // because they could potentially get enabled. diff --git a/src/ripple/protocol/impl/InnerObjectFormats.cpp b/src/ripple/protocol/impl/InnerObjectFormats.cpp index 4c2500ff21..9ba6c33e0d 100644 --- a/src/ripple/protocol/impl/InnerObjectFormats.cpp +++ b/src/ripple/protocol/impl/InnerObjectFormats.cpp @@ -44,8 +44,9 @@ InnerObjectFormats::InnerObjectFormats() sfSigner.getCode(), { {sfAccount, soeREQUIRED}, - {sfSigningPubKey, soeREQUIRED}, - {sfTxnSignature, soeREQUIRED}, + {sfSigningPubKey, soeOPTIONAL}, + {sfTxnSignature, soeOPTIONAL}, + {sfSigners, soeOPTIONAL}, }); add(sfMajority.jsonName.c_str(), diff --git a/src/ripple/protocol/impl/STTx.cpp b/src/ripple/protocol/impl/STTx.cpp index 2fbb194218..86549ce6a7 100644 --- a/src/ripple/protocol/impl/STTx.cpp +++ b/src/ripple/protocol/impl/STTx.cpp @@ -369,64 +369,124 @@ STTx::checkMultiSign( bool const fullyCanonical = (getFlags() & tfFullyCanonicalSig) || (requireCanonicalSig == RequireFullyCanonicalSig::yes); - // Signers must be in sorted order by AccountID. - AccountID lastAccountID(beast::zero); - bool const isWildcardNetwork = isFieldPresent(sfNetworkID) && getFieldU32(sfNetworkID) == 65535; - for (auto const& signer : signers) - { - auto const accountID = signer.getAccountID(sfAccount); + // Set max depth based on feature flag + int const maxDepth = rules.enabled(featureNestedMultiSign) ? 4 : 1; - // The account owner may not multisign for themselves. - if (accountID == txnAccountID) - return Unexpected("Invalid multisigner."); + // Define recursive lambda for checking signatures at any depth + std::function( + STArray const&, AccountID const&, int)> + checkSignersArray; - // No duplicate signers allowed. - if (lastAccountID == accountID) - return Unexpected("Duplicate Signers not allowed."); + checkSignersArray = [&](STArray const& signersArray, + AccountID const& parentAccountID, + int depth) -> Expected { + // Check depth limit + if (depth > maxDepth) + return Unexpected("Multi-signing depth limit exceeded."); - // Accounts must be in order by account ID. No duplicates allowed. - if (lastAccountID > accountID) - return Unexpected("Unsorted Signers array."); + // There are well known bounds that the number of signers must be + // within. + if (signersArray.size() < minMultiSigners || + signersArray.size() > maxMultiSigners(&rules)) + return Unexpected("Invalid Signers array size."); - // The next signature must be greater than this one. - lastAccountID = accountID; + // Signers must be in sorted order by AccountID. + AccountID lastAccountID(beast::zero); - // Verify the signature. - bool validSig = false; - try + for (auto const& signer : signersArray) { - Serializer s = dataStart; - finishMultiSigningData(accountID, s); + auto const accountID = signer.getAccountID(sfAccount); + + // The account owner may not multisign for themselves. + if (accountID == txnAccountID) + return Unexpected("Invalid multisigner."); + + // No duplicate signers allowed. + if (lastAccountID == accountID) + return Unexpected("Duplicate Signers not allowed."); + + // Accounts must be in order by account ID. No duplicates allowed. + if (lastAccountID > accountID) + return Unexpected("Unsorted Signers array."); - auto spk = signer.getFieldVL(sfSigningPubKey); + // The next signature must be greater than this one. + lastAccountID = accountID; - if (publicKeyType(makeSlice(spk))) + // Check if this signer has nested signers + if (signer.isFieldPresent(sfSigners)) { - Blob const signature = signer.getFieldVL(sfTxnSignature); - - // wildcard network gets a free pass - validSig = isWildcardNetwork || - verify(PublicKey(makeSlice(spk)), - s.slice(), - makeSlice(signature), - fullyCanonical); + // This is a nested multi-signer + if (maxDepth == 1) + { + // amendment is not enabled, this is an error + return Unexpected("FeatureNestedMultiSign is disabled"); + } + + // Ensure it doesn't also have signature fields + if (signer.isFieldPresent(sfSigningPubKey) || + signer.isFieldPresent(sfTxnSignature)) + return Unexpected( + "Signer cannot have both nested signers and signature " + "fields."); + + // Recursively check nested signers + STArray const& nestedSigners = signer.getFieldArray(sfSigners); + auto result = + checkSignersArray(nestedSigners, accountID, depth + 1); + if (!result) + return result; + } + else + { + // This is a leaf node - must have signature + if (!signer.isFieldPresent(sfSigningPubKey) || + !signer.isFieldPresent(sfTxnSignature)) + return Unexpected( + "Leaf signer must have SigningPubKey and " + "TxnSignature."); + + // Verify the signature + bool validSig = false; + try + { + Serializer s = dataStart; + finishMultiSigningData(accountID, s); + + auto spk = signer.getFieldVL(sfSigningPubKey); + + if (publicKeyType(makeSlice(spk))) + { + Blob const signature = + signer.getFieldVL(sfTxnSignature); + + // wildcard network gets a free pass + validSig = isWildcardNetwork || + verify(PublicKey(makeSlice(spk)), + s.slice(), + makeSlice(signature), + fullyCanonical); + } + } + catch (std::exception const&) + { + // We assume any problem lies with the signature. + validSig = false; + } + if (!validSig) + return Unexpected( + std::string("Invalid signature on account ") + + toBase58(accountID) + "."); } } - catch (std::exception const&) - { - // We assume any problem lies with the signature. - validSig = false; - } - if (!validSig) - return Unexpected( - std::string("Invalid signature on account ") + - toBase58(accountID) + "."); - } - // All signatures verified. - return {}; + + return {}; + }; + + // Start the recursive check at depth 1 + return checkSignersArray(signers, txnAccountID, 1); } //------------------------------------------------------------------------------ diff --git a/src/ripple/rpc/impl/TransactionSign.cpp b/src/ripple/rpc/impl/TransactionSign.cpp index c903c26f8e..37bf12acb4 100644 --- a/src/ripple/rpc/impl/TransactionSign.cpp +++ b/src/ripple/rpc/impl/TransactionSign.cpp @@ -1183,12 +1183,21 @@ transactionSubmitMultiSigned( // The Signers array may only contain Signer objects. if (std::find_if_not( signers.begin(), signers.end(), [](STObject const& obj) { - return ( - // A Signer object always contains these fields and no - // others. - obj.isFieldPresent(sfAccount) && - obj.isFieldPresent(sfSigningPubKey) && - obj.isFieldPresent(sfTxnSignature) && obj.getCount() == 3); + if (obj.getCount() != 4 || !obj.isFieldPresent(sfAccount)) + return false; + // leaf signer + if (obj.isFieldPresent(sfSigningPubKey) && + obj.isFieldPresent(sfTxnSignature) && + !obj.isFieldPresent(sfSigners)) + return true; + + // nested signer + if (!obj.isFieldPresent(sfSigningPubKey) && + !obj.isFieldPresent(sfTxnSignature) && + obj.isFieldPresent(sfSigners)) + return true; + + return false; }) != signers.end()) { return RPC::make_param_error( diff --git a/src/test/app/MultiSign_test.cpp b/src/test/app/MultiSign_test.cpp index 7dfbfdd78b..075fd2ce63 100644 --- a/src/test/app/MultiSign_test.cpp +++ b/src/test/app/MultiSign_test.cpp @@ -1659,6 +1659,800 @@ class MultiSign_test : public beast::unit_test::suite BEAST_EXPECT(env.seq(alice) == aliceSeq + 1); } + void + test_nestedMultiSign(FeatureBitset features) + { + testcase("Nested MultiSign"); + +#define STRINGIFY(x) #x +#define TOSTRING(x) STRINGIFY(x) + +#define LINE_TO_HEX_STRING \ + []() -> std::string { \ + const char* line = TOSTRING(__LINE__); \ + int len = 0; \ + while (line[len]) \ + len++; \ + std::string result; \ + if (len % 2 == 1) \ + { \ + result += (char)(0x00 * 16 + (line[0] - '0')); \ + line++; \ + } \ + for (int i = 0; line[i]; i += 2) \ + { \ + result += (char)((line[i] - '0') * 16 + (line[i + 1] - '0')); \ + } \ + return result; \ + }() + +#define M(m) memo(m, "", "") +#define L() memo(LINE_TO_HEX_STRING, "", "") + + using namespace jtx; + Env env{*this, envconfig(), features}; + // Env env{*this, envconfig(), features, nullptr, + // beast::severities::kTrace}; + + Account const alice{"alice", KeyType::secp256k1}; + Account const becky{"becky", KeyType::ed25519}; + Account const cheri{"cheri", KeyType::secp256k1}; + Account const daria{"daria", KeyType::ed25519}; + Account const edgar{"edgar", KeyType::secp256k1}; + Account const fiona{"fiona", KeyType::ed25519}; + Account const grace{"grace", KeyType::secp256k1}; + Account const henry{"henry", KeyType::ed25519}; + Account const f1{"f1", KeyType::ed25519}; + Account const f2{"f2", KeyType::ed25519}; + Account const f3{"f3", KeyType::ed25519}; + env.fund( + XRP(1000), + alice, + becky, + cheri, + daria, + edgar, + fiona, + grace, + henry, + f1, + f2, + f3, + phase, + jinni, + acc10, + acc11, + acc12); + env.close(); + + auto const baseFee = env.current()->fees().base; + + if (!features[featureNestedMultiSign]) + { + // When feature is disabled, nested signing should fail + env(signers(f1, 1, {{f2, 1}})); + env(signers(f2, 1, {{f3, 1}})); + env.close(); + + std::uint32_t f1Seq = env.seq(f1); + env(noop(f1), + msig({msigner(f2, msigner(f3))}), + L(), + fee(3 * baseFee), + ter(temINVALID)); + env.close(); + BEAST_EXPECT(env.seq(f1) == f1Seq); + return; + } + + // Test Case 1: Basic 2-level nested signing with quorum + { + // Set up signer lists with quorum requirements + env(signers(becky, 2, {{bogie, 1}, {demon, 1}, {ghost, 1}})); + env(signers(cheri, 3, {{haunt, 2}, {jinni, 2}})); + env.close(); + + // Alice requires quorum of 3 with weighted signers + env(signers(alice, 3, {{becky, 2}, {cheri, 2}, {daria, 1}})); + env.close(); + + // Test 1a: becky alone (weight 2) doesn't meet alice's quorum + std::uint32_t aliceSeq = env.seq(alice); + env(noop(alice), + msig({msigner(becky, msigner(bogie), msigner(demon))}), + L(), + fee(4 * baseFee), + ter(tefBAD_QUORUM)); + env.close(); + BEAST_EXPECT(env.seq(alice) == aliceSeq); + + // Test 1b: becky (2) + daria (1) meets quorum of 3 + aliceSeq = env.seq(alice); + env(noop(alice), + msig( + {msigner(becky, msigner(bogie), msigner(demon)), + msigner(daria)}), + L(), + fee(5 * baseFee)); + env.close(); + BEAST_EXPECT(env.seq(alice) == aliceSeq + 1); + + // Test 1c: cheri's nested signers must meet her quorum + aliceSeq = env.seq(alice); + env(noop(alice), + msig( + {msigner( + becky, + msigner(bogie), + msigner(demon)), // becky has a satisfied quorum + msigner(cheri, msigner(haunt))}), // but cheri does not + // (needs jinni too) + L(), + fee(5 * baseFee), + ter(tefBAD_QUORUM)); + env.close(); + BEAST_EXPECT(env.seq(alice) == aliceSeq); + + // Test 1d: cheri with both signers meets her quorum + aliceSeq = env.seq(alice); + env(noop(alice), + msig( + {msigner(cheri, msigner(haunt), msigner(jinni)), + msigner(daria)}), + L(), + fee(5 * baseFee)); + env.close(); + BEAST_EXPECT(env.seq(alice) == aliceSeq + 1); + } + + // Test Case 2: 3-level maximum depth with quorum at each level + { + // Level 2: phase needs direct signatures (no deeper nesting) + env(signers(phase, 2, {{acc10, 1}, {acc11, 1}, {acc12, 1}})); + + // Level 1: jinni needs weighted signatures + env(signers(jinni, 3, {{phase, 2}, {shade, 2}, {spook, 1}})); + + // Level 0: edgar needs 2 from weighted signers + env(signers(edgar, 2, {{jinni, 1}, {bogie, 1}, {demon, 1}})); + + // Alice now requires edgar with weight 3 + env(signers(alice, 3, {{edgar, 3}, {fiona, 2}})); + env.close(); + + // Test 2a: 3-level signing with phase signing directly (not through + // nested signers) + std::uint32_t aliceSeq = env.seq(alice); + env(noop(alice), + msig({ + msigner( + edgar, + msigner( + jinni, + msigner(phase), // phase signs directly at level 3 + msigner(shade)) // jinni quorum: 2+2 = 4 >= 3 ✓ + ) // edgar quorum: 1+0 = 1 < 2 ✗ + }), + L(), + fee(4 * baseFee), + ter(tefBAD_QUORUM)); + env.close(); + BEAST_EXPECT(env.seq(alice) == aliceSeq); + + // Test 2b: Edgar needs to meet his quorum too + aliceSeq = env.seq(alice); + env(noop(alice), + msig({ + msigner( + edgar, + msigner( + jinni, + msigner(phase), // phase signs directly + msigner(shade)), + msigner(bogie)) // edgar quorum: 1+1 = 2 ✓ + }), + L(), + fee(5 * baseFee)); + env.close(); + BEAST_EXPECT(env.seq(alice) == aliceSeq + 1); + + // Test 2c: Use phase's signers (making it effectively 3-level from + // alice) + aliceSeq = env.seq(alice); + env(noop(alice), + msig({msigner( + edgar, + msigner( + jinni, + msigner(phase, msigner(acc10), msigner(acc11)), + msigner(spook)), + msigner(bogie))}), + L(), + fee(6 * baseFee)); + env.close(); + BEAST_EXPECT(env.seq(alice) == aliceSeq + 1); + } + + // Test Case 3: Mixed levels - some direct, some nested at different + // depths (max 3) + { + // Set up mixed-level signing for alice + // grace has direct signers + env(signers(grace, 2, {{bogie, 1}, {demon, 1}})); + + // henry has 2-level signers (henry -> becky -> bogie/demon) + env(signers(henry, 1, {{becky, 1}, {cheri, 1}})); + + // edgar can be signed for by bogie + env(signers(edgar, 1, {{bogie, 1}, {shade, 1}})); + + // Alice has mix of direct and nested signers at different weights + env(signers( + alice, + 5, + { + {daria, 1}, // direct signer + {edgar, 2}, // has 2-level signers + {fiona, 1}, // direct signer + {grace, 2}, // has direct signers + {henry, 2} // has 2-level signers + })); + env.close(); + + // Test 3a: Mix of all levels meeting quorum exactly + std::uint32_t aliceSeq = env.seq(alice); + env(noop(alice), + msig({ + msigner(daria), // weight 1, direct + msigner(edgar, msigner(bogie)), // weight 2, 2-level + msigner(grace, msigner(bogie), msigner(demon)) // weight 2, + // 2-level + }), + L(), + fee(6 * baseFee)); + env.close(); + BEAST_EXPECT(env.seq(alice) == aliceSeq + 1); + + // Test 3b: 3-level signing through henry + aliceSeq = env.seq(alice); + env(noop(alice), + msig( + {msigner(fiona), // weight 1, direct + msigner( + grace, msigner(bogie)), // weight 2, 2-level (partial) + msigner( + henry, // weight 2, 3-level + msigner(becky, msigner(bogie), msigner(demon)))}), + L(), + fee(6 * baseFee), + ter(tefBAD_QUORUM)); // grace didn't meet quorum + env.close(); + BEAST_EXPECT(env.seq(alice) == aliceSeq); + + // Test 3c: Correct version with all quorums met + aliceSeq = env.seq(alice); + env(noop(alice), + msig( + {msigner(fiona), // weight 1 + msigner( + edgar, msigner(bogie), msigner(shade)), // weight 2 + msigner( + henry, // weight 2 + msigner(becky, msigner(bogie), msigner(demon)))}), + L(), + fee(8 * baseFee)); // Total weight: 1+2+2 = 5 ✓ + env.close(); + BEAST_EXPECT(env.seq(alice) == aliceSeq + 1); + } + + // Test Case 4: Complex scenario with maximum signers at mixed depths + // (max 3) + { + // Create a signing tree that uses close to maximum signers + // and tests weight accumulation across all levels + + // Set up for alice: needs 15 out of possible 20 weight + env(signers( + alice, + 15, + { + {becky, 3}, // will use 2-level + {cheri, 3}, // will use 2-level + {daria, 3}, // will use direct + {edgar, 3}, // will use 2-level + {fiona, 3}, // will use direct + {grace, 3}, // will use direct + {henry, 2} // will use 2-level + })); + env.close(); + + // Complex multi-level transaction just meeting quorum + std::uint32_t aliceSeq = env.seq(alice); + env(noop(alice), + msig({ + msigner( + becky, // weight 3, 2-level + msigner(demon), + msigner(ghost)), + msigner( + cheri, // weight 3, 2-level + msigner(haunt), + msigner(jinni)), + msigner(daria), // weight 3, direct + msigner( + edgar, // weight 3, 2-level + msigner(bogie)), + msigner(grace) // weight 3, direct + }), + L(), + fee(10 * baseFee)); // Total weight: 3+3+3+3+3 = 15 ✓ + env.close(); + BEAST_EXPECT(env.seq(alice) == aliceSeq + 1); + + // Test 4b: Test with henry using 3-level depth (maximum) + // First set up henry's chain properly + env(signers(henry, 1, {{jinni, 1}})); + env(signers(jinni, 2, {{acc10, 1}, {acc11, 1}})); + env.close(); + + aliceSeq = env.seq(alice); + env(noop(alice), + msig( + {msigner( + becky, // weight 3 + msigner(demon)), // becky quorum not met! + msigner( + cheri, // weight 3 + msigner(haunt), + msigner(jinni)), + msigner(daria), // weight 3 + msigner( + henry, // weight 2, 3-level depth + msigner(jinni, msigner(acc10), msigner(acc11))), + msigner( + edgar, // weight 3 + msigner(bogie), + msigner(shade))}), + L(), + fee(10 * baseFee), + ter(tefBAD_QUORUM)); // becky's quorum not met + env.close(); + BEAST_EXPECT(env.seq(alice) == aliceSeq); + } + + // Test Case 5: Edge case - single signer with maximum nesting (depth 3) + { + // Alice needs just one signer, but that signer uses depth up to 3 + env(signers(alice, 1, {{becky, 1}})); + env.close(); + + std::uint32_t aliceSeq = env.seq(alice); + env(noop(alice), + msig({msigner(becky, msigner(demon), msigner(ghost))}), + L(), + fee(4 * baseFee)); + env.close(); + BEAST_EXPECT(env.seq(alice) == aliceSeq + 1); + + // Now with 3-level depth (maximum allowed) + // Structure: alice -> becky -> cheri -> jinni (jinni signs + // directly) + env(signers(becky, 1, {{cheri, 1}})); + env(signers(cheri, 1, {{jinni, 1}})); + // Note: We do NOT add signers to jinni to keep max depth at 3 + env.close(); + + aliceSeq = env.seq(alice); + env(noop(alice), + msig({msigner( + becky, + msigner( + cheri, + msigner(jinni)))}), // jinni signs directly (depth 3) + L(), + fee(4 * baseFee)); + env.close(); + BEAST_EXPECT(env.seq(alice) == aliceSeq + 1); + } + + // Test Case 6: Simple cycle detection (A -> B -> A) + { + testcase("Cycle Detection - Simple"); + + // Reset signer lists for clean state + env(signers(alice, jtx::none)); + env(signers(becky, jtx::none)); + env.close(); + + // becky's signer list includes alice + // alice's signer list includes becky + // This creates: alice -> becky -> alice (cycle) + env(signers(alice, 1, {{becky, 1}, {bogie, 1}})); + env(signers(becky, 1, {{alice, 1}, {demon, 1}})); + env.close(); + + // Without cycle relaxation this would fail because: + // - alice needs becky (weight 1) + // - becky needs alice, but alice is ancestor -> cycle + // - becky's effective quorum relaxes since alice is unavailable + // - demon can satisfy becky's relaxed quorum + std::uint32_t aliceSeq = env.seq(alice); + env(noop(alice), + msig({msigner(becky, msigner(demon))}), + L(), + fee(4 * baseFee)); + env.close(); + BEAST_EXPECT(env.seq(alice) == aliceSeq + 1); + + // Test that direct signer still works normally + aliceSeq = env.seq(alice); + env(noop(alice), msig({msigner(bogie)}), L(), fee(3 * baseFee)); + env.close(); + BEAST_EXPECT(env.seq(alice) == aliceSeq + 1); + } + + // Test Case 7: The specific lockout scenario + // onyx:{jade, nova:{ruby:{jade, nova}, jade}} + // All have quorum 2, only jade can actually sign + { + testcase("Cycle Detection - Complex Lockout"); + + Account const onyx{"onyx", KeyType::secp256k1}; + Account const nova{"nova", KeyType::ed25519}; + Account const ruby{"ruby", KeyType::secp256k1}; + Account const jade{"jade", KeyType::ed25519}; // phantom signer + + env.fund(XRP(1000), onyx, nova, ruby); + env.close(); + + // Set up signer lists FIRST (before disabling master keys) + // ruby: {jade, nova} with quorum 2 + env(signers(ruby, 2, {{jade, 1}, {nova, 1}})); + // nova: {ruby, jade} with quorum 2 + env(signers(nova, 2, {{jade, 1}, {ruby, 1}})); + // onyx: {jade, nova} with quorum 2 + env(signers(onyx, 2, {{jade, 1}, {nova, 1}})); + env.close(); + + // NOW disable master keys (signer lists provide alternative) + env(fset(onyx, asfDisableMaster), sig(onyx)); + env(fset(nova, asfDisableMaster), sig(nova)); + env(fset(ruby, asfDisableMaster), sig(ruby)); + env.close(); + + // The signing tree for onyx: + // onyx (quorum 2) -> jade (weight 1) + nova (weight 1) + // nova (quorum 2) -> jade (weight 1) + ruby (weight 1) + // ruby (quorum 2) -> jade (weight 1) + nova (weight 1, CYCLE!) + // + // Without cycle detection: ruby needs nova, but nova is ancestor -> + // stuck With cycle detection: + // - At ruby level: nova is cyclic, cyclicWeight=1, totalWeight=2 + // - maxAchievable = 2-1 = 1 < quorum(2), so effectiveQuorum -> 1 + // - jade alone can satisfy ruby's relaxed quorum + // - ruby satisfied -> nova gets ruby's weight + // - nova: jade(1) + ruby(1) = 2 >= quorum(2) ✓ + // - onyx: jade(1) + nova(1) = 2 >= quorum(2) ✓ + + std::uint32_t onyxSeq = env.seq(onyx); + env(noop(onyx), + msig( + {msigner(jade), + msigner( + nova, + msigner(jade), + msigner( + ruby, msigner(jade)))}), // nova is cyclic, + // skipped at ruby level + L(), + fee(6 * baseFee)); + env.close(); + BEAST_EXPECT(env.seq(onyx) == onyxSeq + 1); + } + + // Test Case 8: Cycle where all signers are cyclic (effectiveQuorum == + // 0) + { + testcase("Cycle Detection - Total Lockout"); + + Account const alpha{"alpha", KeyType::secp256k1}; + Account const beta{"beta", KeyType::ed25519}; + Account const gamma{"gamma", KeyType::secp256k1}; + + env.fund(XRP(1000), alpha, beta, gamma); + env.close(); + + // Set up pure cycle signer lists FIRST + env(signers(alpha, 1, {{beta, 1}})); + env(signers(beta, 1, {{gamma, 1}})); + env(signers(gamma, 1, {{alpha, 1}})); + env.close(); + + // NOW disable master keys + env(fset(alpha, asfDisableMaster), sig(alpha)); + env(fset(beta, asfDisableMaster), sig(beta)); + env(fset(gamma, asfDisableMaster), sig(gamma)); + env.close(); + + // This is a true lockout - no valid signing path exists. + // gamma appears as a leaf signer but has master disabled -> + // tefMASTER_DISABLED (The cycle detection would return + // tefBAD_QUORUM if gamma were nested, but there's no way to + // construct such a transaction since gamma's only signer is alpha, + // which is what we're trying to sign for) + std::uint32_t alphaSeq = env.seq(alpha); + env(noop(alpha), + msig({msigner( + beta, + msigner(gamma))}), // gamma can't sign - master disabled + L(), + fee(4 * baseFee), + ter(tefMASTER_DISABLED)); + env.close(); + BEAST_EXPECT(env.seq(alpha) == alphaSeq); + } + + // Test Case 9: Cycle at depth 3 (near max depth) + { + testcase("Cycle Detection - Deep Cycle"); + + // Reset signer lists + env(signers(alice, jtx::none)); + env(signers(becky, jtx::none)); + env(signers(cheri, jtx::none)); + env(signers(daria, jtx::none)); + env.close(); + + // Structure: alice -> becky -> cheri -> daria -> alice (cycle at + // depth 4) + env(signers(alice, 1, {{becky, 1}, {bogie, 1}})); + env(signers(becky, 1, {{cheri, 1}})); + env(signers(cheri, 1, {{daria, 1}})); + env(signers(daria, 1, {{alice, 1}, {demon, 1}})); + env.close(); + + // At depth 4, daria needs alice but alice is ancestor + // daria's quorum relaxes, demon can satisfy + std::uint32_t aliceSeq = env.seq(alice); + env(noop(alice), + msig({msigner( + becky, msigner(cheri, msigner(daria, msigner(demon))))}), + L(), + fee(6 * baseFee)); + env.close(); + BEAST_EXPECT(env.seq(alice) == aliceSeq + 1); + } + + // Test Case 10: Multiple independent cycles in same tree + { + testcase("Cycle Detection - Multiple Cycles"); + + // Reset signer lists + env(signers(alice, jtx::none)); + env(signers(becky, jtx::none)); + env(signers(cheri, jtx::none)); + env.close(); + + // alice -> {becky, cheri} + // becky -> {alice, bogie} (cycle back to alice) + // cheri -> {alice, demon} (another cycle back to alice) + env(signers(alice, 2, {{becky, 1}, {cheri, 1}})); + env(signers(becky, 2, {{alice, 1}, {bogie, 1}})); + env(signers(cheri, 2, {{alice, 1}, {demon, 1}})); + env.close(); + + // Both becky and cheri have cycles back to alice + // Both need their quorums relaxed + // bogie satisfies becky, demon satisfies cheri + std::uint32_t aliceSeq = env.seq(alice); + env(noop(alice), + msig( + {msigner(becky, msigner(bogie)), + msigner(cheri, msigner(demon))}), + L(), + fee(6 * baseFee)); + env.close(); + BEAST_EXPECT(env.seq(alice) == aliceSeq + 1); + } + + // Test Case 11: Cycle with sufficient non-cyclic weight (no relaxation + // needed) + { + testcase("Cycle Detection - No Relaxation Needed"); + + // Reset signer lists + env(signers(alice, jtx::none)); + env(signers(becky, jtx::none)); + env.close(); + + // becky has alice in signer list but also has enough other signers + env(signers(alice, 1, {{becky, 1}})); + env(signers(becky, 2, {{alice, 1}, {bogie, 1}, {demon, 1}})); + env.close(); + + // becky quorum is 2, alice is cyclic (weight 1) + // totalWeight = 3, cyclicWeight = 1, maxAchievable = 2 >= quorum + // No relaxation needed, bogie + demon satisfy quorum normally + std::uint32_t aliceSeq = env.seq(alice); + env(noop(alice), + msig({msigner(becky, msigner(bogie), msigner(demon))}), + L(), + fee(5 * baseFee)); + env.close(); + BEAST_EXPECT(env.seq(alice) == aliceSeq + 1); + + // Should fail if only one non-cyclic signer provided + aliceSeq = env.seq(alice); + env(noop(alice), + msig({msigner(becky, msigner(bogie))}), + L(), + fee(4 * baseFee), + ter(tefBAD_QUORUM)); + env.close(); + BEAST_EXPECT(env.seq(alice) == aliceSeq); + } + + // Test Case 12: Partial cycle - one branch cyclic, one not + { + testcase("Cycle Detection - Partial Cycle"); + + // Reset signer lists + env(signers(alice, jtx::none)); + env(signers(becky, jtx::none)); + env(signers(cheri, jtx::none)); + env.close(); + + // alice -> {becky, cheri} + // becky -> {alice, bogie} (cyclic) + // cheri -> {daria} (not cyclic) + env(signers(alice, 2, {{becky, 1}, {cheri, 1}})); + env(signers(becky, 1, {{alice, 1}, {bogie, 1}})); + env(signers(cheri, 1, {{daria, 1}})); + env.close(); + + // becky's branch has cycle, cheri's doesn't + // Both contribute to alice's quorum + std::uint32_t aliceSeq = env.seq(alice); + env(noop(alice), + msig( + {msigner(becky, msigner(bogie)), // relaxed quorum + msigner(cheri, msigner(daria))}), // normal quorum + L(), + fee(6 * baseFee)); + env.close(); + BEAST_EXPECT(env.seq(alice) == aliceSeq + 1); + } + + // Test Case 13: Diamond pattern with cycle + { + testcase("Cycle Detection - Diamond Pattern"); + + // Reset signer lists + env(signers(alice, jtx::none)); + env(signers(becky, jtx::none)); + env(signers(cheri, jtx::none)); + env(signers(daria, jtx::none)); + env.close(); + + // alice -> {becky, cheri} + // becky -> {daria} + // cheri -> {daria} + // daria -> {alice, bogie} (cycle through both paths) + env(signers(alice, 2, {{becky, 1}, {cheri, 1}})); + env(signers(becky, 1, {{daria, 1}})); + env(signers(cheri, 1, {{daria, 1}})); + env(signers(daria, 1, {{alice, 1}, {bogie, 1}})); + env.close(); + + // Both paths converge at daria, which cycles back to alice + std::uint32_t aliceSeq = env.seq(alice); + env(noop(alice), + msig( + {msigner(becky, msigner(daria, msigner(bogie))), + msigner(cheri, msigner(daria, msigner(bogie)))}), + L(), + fee(7 * baseFee)); + env.close(); + BEAST_EXPECT(env.seq(alice) == aliceSeq + 1); + } + + // Test Case 14: Cycle requiring maximum quorum relaxation + { + testcase("Cycle Detection - Maximum Relaxation"); + + Account const omega{"omega", KeyType::secp256k1}; + Account const sigma{"sigma", KeyType::ed25519}; + + env.fund(XRP(1000), omega, sigma); + env.close(); + + // Reset alice and becky signer lists + env(signers(alice, jtx::none)); + env(signers(becky, jtx::none)); + env.close(); + + // Set up signer lists FIRST + env(signers(sigma, 1, {{omega, 1}, {bogie, 1}})); + env(signers(omega, 3, {{sigma, 2}, {alice, 1}, {becky, 1}})); + env(signers(alice, 1, {{omega, 1}, {demon, 1}})); + env(signers(becky, 1, {{omega, 1}, {ghost, 1}})); + env.close(); + + // NOW disable master keys + env(fset(omega, asfDisableMaster), sig(omega)); + env(fset(sigma, asfDisableMaster), sig(sigma)); + env.close(); + + // From omega's perspective when signing for omega: + // - sigma: needs omega (cyclic), so relaxes to bogie only + // - alice: needs omega (cyclic), so relaxes to demon only + // - becky: needs omega (cyclic), so relaxes to ghost only + // All signers need relaxation but can be satisfied + std::uint32_t omegaSeq = env.seq(omega); + env(noop(omega), + msig( + {msigner(alice, msigner(demon)), + msigner(becky, msigner(ghost)), + msigner(sigma, msigner(bogie))}), + L(), + fee(7 * baseFee)); + env.close(); + BEAST_EXPECT(env.seq(omega) == omegaSeq + 1); + } + + // Test Case 15: Cycle at exact max depth boundary + { + testcase("Cycle Detection - Max Depth Boundary"); + + // Reset signer lists + env(signers(alice, jtx::none)); + env(signers(becky, jtx::none)); + env(signers(cheri, jtx::none)); + env(signers(daria, jtx::none)); + env(signers(edgar, jtx::none)); + env.close(); + + // Depth 4 is max: alice(1) -> becky(2) -> cheri(3) -> daria(4) + // daria cycles back but we're at max depth + env(signers(alice, 1, {{becky, 1}})); + env(signers(becky, 1, {{cheri, 1}})); + env(signers(cheri, 1, {{daria, 1}})); + env(signers(daria, 1, {{alice, 1}, {bogie, 1}})); + env.close(); + + // This should work - cycle detected and relaxed at depth 4 + std::uint32_t aliceSeq = env.seq(alice); + env(noop(alice), + msig({msigner( + becky, msigner(cheri, msigner(daria, msigner(bogie))))}), + L(), + fee(6 * baseFee)); + env.close(); + BEAST_EXPECT(env.seq(alice) == aliceSeq + 1); + + // Now try to exceed depth (add edgar at depth 5) + env(signers(daria, 1, {{edgar, 1}})); + env(signers(edgar, 1, {{bogie, 1}})); + env.close(); + + // Transaction structure is rejected at preflight for exceeding + // nesting limits + aliceSeq = env.seq(alice); + env(noop(alice), + msig({msigner( + becky, + msigner( + cheri, + msigner(daria, msigner(edgar, msigner(bogie)))))}), + L(), + fee(7 * baseFee), + ter(temMALFORMED)); // Rejected at preflight for excessive + // nesting + env.close(); + BEAST_EXPECT(env.seq(alice) == aliceSeq); + } + } + void test_signerListSetFlags(FeatureBitset features) { @@ -1709,6 +2503,7 @@ class MultiSign_test : public beast::unit_test::suite test_signForHash(features); test_signersWithTickets(features); test_signersWithTags(features); + test_nestedMultiSign(features); } void @@ -1721,8 +2516,11 @@ class MultiSign_test : public beast::unit_test::suite // featureMultiSignReserve. Limits on the number of signers // changes based on featureExpandedSignerList. Test both with and // without. - testAll(all - featureMultiSignReserve - featureExpandedSignerList); - testAll(all - featureExpandedSignerList); + testAll( + all - featureMultiSignReserve - featureExpandedSignerList - + featureNestedMultiSign); + testAll(all - featureExpandedSignerList - featureNestedMultiSign); + testAll(all - featureNestedMultiSign); testAll(all); test_signerListSetFlags(all); diff --git a/src/test/jtx/impl/multisign.cpp b/src/test/jtx/impl/multisign.cpp index 1e1f514179..e22fda574e 100644 --- a/src/test/jtx/impl/multisign.cpp +++ b/src/test/jtx/impl/multisign.cpp @@ -66,15 +66,45 @@ signers(Account const& account, none_t) //------------------------------------------------------------------------------ -msig::msig(std::vector signers_) : signers(std::move(signers_)) +// Helper function to recursively sort nested signers +void +sortSignersRecursive(std::vector& signers) { - // Signatures must be applied in sorted order. + // Sort current level by account ID std::sort( signers.begin(), signers.end(), - [](msig::Reg const& lhs, msig::Reg const& rhs) { - return lhs.acct.id() < rhs.acct.id(); + [](msig::SignerPtr const& lhs, msig::SignerPtr const& rhs) { + return lhs->id() < rhs->id(); }); + + // Recursively sort nested signers for each signer at this level + for (auto& signer : signers) + { + if (signer->isNested() && !signer->nested.empty()) + { + sortSignersRecursive(signer->nested); + } + } +} + +msig::msig(std::vector signers_) : signers(std::move(signers_)) +{ + // Recursively sort all signers at all nesting levels + // This ensures account IDs are in strictly ascending order at each level + sortSignersRecursive(signers); +} + +msig::msig(std::vector signers_) +{ + // Convert Reg vector to SignerPtr vector for backward compatibility + signers.reserve(signers_.size()); + for (auto const& s : signers_) + signers.push_back(s.toSigner()); + + // Recursively sort all signers at all nesting levels + // This ensures account IDs are in strictly ascending order at each level + sortSignersRecursive(signers); } void @@ -93,19 +123,47 @@ msig::operator()(Env& env, JTx& jt) const env.test.log << pretty(jtx.jv) << std::endl; Rethrow(); } + + // Recursive function to build signer JSON + std::function buildSignerJson; + buildSignerJson = [&](SignerPtr const& signer) -> Json::Value { + Json::Value jo; + jo[jss::Account] = signer->acct.human(); + + if (signer->isNested()) + { + // For nested signers, we use the already-sorted nested vector + // (sorted during construction via sortSignersRecursive) + // This ensures account IDs are in strictly ascending order + auto& subJs = jo[sfSigners.getJsonName()]; + for (std::size_t i = 0; i < signer->nested.size(); ++i) + { + auto& subJo = subJs[i][sfSigner.getJsonName()]; + subJo = buildSignerJson(signer->nested[i]); + } + } + else + { + // This is a leaf signer - add signature + jo[jss::SigningPubKey] = strHex(signer->sig.pk().slice()); + + Serializer ss{buildMultiSigningData(*st, signer->acct.id())}; + auto const sig = ripple::sign( + *publicKeyType(signer->sig.pk().slice()), + signer->sig.sk(), + ss.slice()); + jo[sfTxnSignature.getJsonName()] = + strHex(Slice{sig.data(), sig.size()}); + } + + return jo; + }; + auto& js = jtx[sfSigners.getJsonName()]; for (std::size_t i = 0; i < mySigners.size(); ++i) { - auto const& e = mySigners[i]; auto& jo = js[i][sfSigner.getJsonName()]; - jo[jss::Account] = e.acct.human(); - jo[jss::SigningPubKey] = strHex(e.sig.pk().slice()); - - Serializer ss{buildMultiSigningData(*st, e.acct.id())}; - auto const sig = ripple::sign( - *publicKeyType(e.sig.pk().slice()), e.sig.sk(), ss.slice()); - jo[sfTxnSignature.getJsonName()] = - strHex(Slice{sig.data(), sig.size()}); + jo = buildSignerJson(mySigners[i]); } }; } diff --git a/src/test/jtx/multisign.h b/src/test/jtx/multisign.h index b5ade20749..d1e8363c6d 100644 --- a/src/test/jtx/multisign.h +++ b/src/test/jtx/multisign.h @@ -21,6 +21,7 @@ #define RIPPLE_TEST_JTX_MULTISIGN_H_INCLUDED #include +#include #include #include #include @@ -65,6 +66,48 @@ signers(Account const& account, none_t); class msig { public: + // Recursive signer structure + struct Signer + { + Account acct; + Account sig; // For leaf signers (same as acct for master key) + std::vector> nested; // For nested signers + + // Leaf signer constructor (regular signing) + Signer(Account const& masterSig) : acct(masterSig), sig(masterSig) + { + } + + // Leaf signer constructor (with different signing key) + Signer(Account const& acct_, Account const& regularSig) + : acct(acct_), sig(regularSig) + { + } + + // Nested signer constructor + Signer( + Account const& acct_, + std::vector> nested_) + : acct(acct_), nested(std::move(nested_)) + { + } + + bool + isNested() const + { + return !nested.empty(); + } + + AccountID + id() const + { + return acct.id(); + } + }; + + using SignerPtr = std::shared_ptr; + + // For backward compatibility struct Reg { Account acct; @@ -73,16 +116,13 @@ class msig Reg(Account const& masterSig) : acct(masterSig), sig(masterSig) { } - Reg(Account const& acct_, Account const& regularSig) : acct(acct_), sig(regularSig) { } - Reg(char const* masterSig) : acct(masterSig), sig(masterSig) { } - Reg(char const* acct_, char const* regularSig) : acct(acct_), sig(regularSig) { @@ -93,13 +133,32 @@ class msig { return acct < rhs.acct; } + + // Convert to Signer + SignerPtr + toSigner() const + { + return std::make_shared(acct, sig); + } }; - std::vector signers; + std::vector signers; public: + // Initializer list constructor - resolves brace-init ambiguity + msig(std::initializer_list signers_) + : msig(std::vector(signers_)) + { + // handled by : + } + + // Direct constructor with SignerPtr vector + explicit msig(std::vector signers_); + + // Backward compatibility constructor msig(std::vector signers_); + // Variadic constructor for backward compatibility template explicit msig(AccountType&& a0, Accounts&&... aN) : msig{std::vector{ @@ -112,6 +171,30 @@ class msig operator()(Env&, JTx& jt) const; }; +// Helper functions to create signers - renamed to avoid conflict with sig() +// transaction modifier +inline msig::SignerPtr +msigner(Account const& acct) +{ + return std::make_shared(acct); +} + +inline msig::SignerPtr +msigner(Account const& acct, Account const& signingKey) +{ + return std::make_shared(acct, signingKey); +} + +// Create nested signer with initializer list +template +inline msig::SignerPtr +msigner(Account const& acct, Args&&... args) +{ + std::vector nested; + (nested.push_back(std::forward(args)), ...); + return std::make_shared(acct, std::move(nested)); +} + //------------------------------------------------------------------------------ /** The number of signer lists matches. */ diff --git a/src/test/protocol/STTx_test.cpp b/src/test/protocol/STTx_test.cpp index 3956726662..be818ec7b3 100644 --- a/src/test/protocol/STTx_test.cpp +++ b/src/test/protocol/STTx_test.cpp @@ -1757,30 +1757,32 @@ class InnerObjectFormatsSerializer_test : public beast::unit_test::suite // This lambda contains the bulk of the test code. auto testMalformedSigningAccount = - [this, &txn](STObject const& signer, bool expectPass) { - // Create SigningAccounts array. - STArray signers(sfSigners, 1); - signers.push_back(signer); + [this, &txn]( + STObject const& signer, bool expectPass) -> bool /* passed */ { + // Create SigningAccounts array. + STArray signers(sfSigners, 1); + signers.push_back(signer); - // Insert signers into transaction. - STTx tempTxn(txn); - tempTxn.setFieldArray(sfSigners, signers); + // Insert signers into transaction. + STTx tempTxn(txn); + tempTxn.setFieldArray(sfSigners, signers); - Serializer rawTxn; - tempTxn.add(rawTxn); - SerialIter sit(rawTxn.slice()); - bool serialized = false; - try - { - STTx copy(sit); - serialized = true; - } - catch (std::exception const&) - { - ; // If it threw then serialization failed. - } - BEAST_EXPECT(serialized == expectPass); - }; + Serializer rawTxn; + tempTxn.add(rawTxn); + SerialIter sit(rawTxn.slice()); + bool serialized = false; + try + { + STTx copy(sit); + serialized = true; + } + catch (std::exception const&) + { + ; // If it threw then serialization failed. + } + BEAST_EXPECT(serialized == expectPass); + return serialized == expectPass; + }; { // Test case 1. Make a valid Signer object. @@ -1790,13 +1792,15 @@ class InnerObjectFormatsSerializer_test : public beast::unit_test::suite soTest1.setFieldVL(sfTxnSignature, saMultiSignature); testMalformedSigningAccount(soTest1, true); } - { + + /*{ // RHNOTE: featureNestedMultiSign covers this in the + checkMultiSign() // Test case 2. Omit sfSigningPubKey from SigningAccount. STObject soTest2(sfSigner); soTest2.setAccountID(sfAccount, id2); soTest2.setFieldVL(sfTxnSignature, saMultiSignature); testMalformedSigningAccount(soTest2, false); - } + }*/ { // Test case 3. Extra sfAmount in SigningAccount. STObject soTest3(sfSigner); diff --git a/src/test/unit_test/multi_runner.h b/src/test/unit_test/multi_runner.h index 57d6d33f9e..2265aeca95 100644 --- a/src/test/unit_test/multi_runner.h +++ b/src/test/unit_test/multi_runner.h @@ -332,6 +332,7 @@ multi_runner_child::run_multi(Pred pred) { if (!pred(*t)) continue; + try { failed = run(*t) || failed;