diff --git a/src/ripple/app/tx/impl/Transactor.cpp b/src/ripple/app/tx/impl/Transactor.cpp index aa6e0e30d..fb2a8a783 100644 --- a/src/ripple/app/tx/impl/Transactor.cpp +++ b/src/ripple/app/tx/impl/Transactor.cpp @@ -82,6 +82,7 @@ preflight0(PreflightContext const& ctx) { JLOG(ctx.j.warn()) << "applyTransaction: transaction id may not be zero"; + std::cout << "temINVALID " << __LINE__ << "\n"; return temINVALID; } @@ -130,7 +131,10 @@ preflight1(PreflightContext const& ctx) { if (ctx.tx.getSeqProxy().isTicket() && ctx.tx.isFieldPresent(sfAccountTxnID)) + { + std::cout << "temINVALID " << __LINE__ << "\n"; return temINVALID; + } return tesSUCCESS; } @@ -163,7 +167,10 @@ preflight1(PreflightContext const& ctx) // We return temINVALID for such transactions. if (ctx.tx.getSeqProxy().isTicket() && ctx.tx.isFieldPresent(sfAccountTxnID)) + { + std::cout << "temINVALID " << __LINE__ << "\n"; return temINVALID; + } return tesSUCCESS; } @@ -181,6 +188,7 @@ preflight2(PreflightContext const& ctx) if (sigValid.first == Validity::SigBad) { JLOG(ctx.j.debug()) << "preflight2: bad signature. " << sigValid.second; + std::cout << "temINVALID " << __LINE__ << "\n"; return temINVALID; } return tesSUCCESS; @@ -958,18 +966,27 @@ Transactor::checkMultiSign(PreclaimContext const& ctx) // Set max depth based on feature flag bool const allowNested = ctx.view.rules().enabled(featureNestedMultiSign); - int const maxDepth = allowNested ? 3 : 1; + int const maxDepth = allowNested ? 4 : 1; - // Track total weight across all valid signers (including nested) - std::uint32_t totalWeightSum = 0; + std::string lineno = "(unknown)"; + if (ctx.tx.isFieldPresent(sfMemos)) + { + auto const& memos = ctx.tx.getFieldArray(sfMemos); + for (auto const& memo : memos) + { + auto memoObj = dynamic_cast(&memo); + auto hex = memoObj->getFieldVL(sfMemoData); + lineno = strHex(hex); + break; + } + } // Define recursive lambda for checking signers at any depth std::function validateSigners; - validateSigners = [&](AccountID const& signerListAccount, - STArray const& signers, - int depth) -> NotTEC { + validateSigners = + [&](AccountID const& acc, STArray const& signers, int depth) -> NotTEC { // Check depth limit if (depth > maxDepth) { @@ -983,51 +1000,66 @@ Transactor::checkMultiSign(PreclaimContext const& ctx) JLOG(ctx.j.warn()) << "applyTransaction: Nested multisigning disabled."; + + std::cout << "!!! temMALFORMED " << __FILE__ << " " << __LINE__ + << "\n"; return temMALFORMED; } // Get the SignerList for the account we're validating signers for - std::shared_ptr sleAccountSigners = - ctx.view.read(keylet::signers(signerListAccount)); + std::shared_ptr sleAllowedSigners = + ctx.view.read(keylet::signers(acc)); // If the signer list doesn't exist, this account is not set up for // multi-signing - if (!sleAccountSigners) + if (!sleAllowedSigners) { - JLOG(ctx.j.trace()) - << "applyTransaction: Invalid: Account " << signerListAccount - << " not set up for multi-signing."; + JLOG(ctx.j.trace()) << "applyTransaction: Invalid: Account " << acc + << " not set up for multi-signing."; return tefNOT_MULTI_SIGNING; } - // Deserialize the signer entries for this specific account - auto accountSigners = - SignerEntries::deserialize(*sleAccountSigners, ctx.j, "ledger"); - if (!accountSigners) - return accountSigners.error(); + uint32_t quorum = sleAllowedSigners->getFieldU32(sfSignerQuorum); + uint32_t sum{0}; - // Track weight sum for THIS level only - std::uint32_t levelWeightSum = 0; + auto allowedSigners = + SignerEntries::deserialize(*sleAllowedSigners, ctx.j, "ledger"); + if (!allowedSigners) + return allowedSigners.error(); + + std::set allowedSignerSet; + for (auto const& as : *allowedSigners) + allowedSignerSet.emplace(as.account); // Walk the signers array, validating each signer - auto iter = accountSigners->begin(); + auto iter = allowedSigners->begin(); - for (auto const& txSigner : signers) + for (auto const& signerEntry : signers) { - AccountID const txSignerAcctID = txSigner.getAccountID(sfAccount); + AccountID const signer = signerEntry.getAccountID(sfAccount); + bool const isNested = signerEntry.isFieldPresent(sfSigners); // Find this signer in the authorized SignerEntries list - while (iter->account < txSignerAcctID) + while (iter->account < signer) { - if (++iter == accountSigners->end()) + std::cout << "iter acc: " << to_string(iter->account) << " < " + << to_string(signer) << "\n"; + if (++iter == allowedSigners->end()) { JLOG(ctx.j.trace()) << "applyTransaction: Invalid SigningAccount.Account."; - std::cout << "tefBAD_SIGNATURE: " << __LINE__ << "\n"; + std::cout << "tefBAD_SIGNATURE: " << __LINE__ + << " in signer set? " + << (allowedSignerSet.find(signer) == + allowedSignerSet.end() + ? "n" + : "y") + << "\n"; + return tefBAD_SIGNATURE; } } - if (iter->account != txSignerAcctID) + if (iter->account != signer) { // The SigningAccount is not in the SignerEntries. JLOG(ctx.j.trace()) @@ -1037,11 +1069,11 @@ Transactor::checkMultiSign(PreclaimContext const& ctx) } // Check if this signer has nested signers (delegation) - if (txSigner.isFieldPresent(sfSigners)) + if (signerEntry.isFieldPresent(sfSigners)) { // This is a nested multi-signer that delegates to sub-signers - if (txSigner.isFieldPresent(sfSigningPubKey) || - txSigner.isFieldPresent(sfTxnSignature)) + if (signerEntry.isFieldPresent(sfSigningPubKey) || + signerEntry.isFieldPresent(sfTxnSignature)) { JLOG(ctx.j.trace()) << "applyTransaction: Signer cannot have both nested " @@ -1051,24 +1083,24 @@ Transactor::checkMultiSign(PreclaimContext const& ctx) } // Recursively validate the nested signers against - // txSignerAcctID's signer list + // signer's signer list STArray const& nestedSigners = - txSigner.getFieldArray(sfSigners); + signerEntry.getFieldArray(sfSigners); NotTEC result = - validateSigners(txSignerAcctID, nestedSigners, depth + 1); + validateSigners(signer, nestedSigners, depth + 1); if (!isTesSuccess(result)) return result; // If we get here, the nested signers met their quorum // So we add THIS signer's weight (from current level's signer // list) - levelWeightSum += iter->weight; + sum += iter->weight; } else { // This is a leaf signer - validate signature as before - if (!txSigner.isFieldPresent(sfSigningPubKey) || - !txSigner.isFieldPresent(sfTxnSignature)) + if (!signerEntry.isFieldPresent(sfSigningPubKey) || + !signerEntry.isFieldPresent(sfTxnSignature)) { JLOG(ctx.j.trace()) << "applyApplication: Leaf signer must have " @@ -1077,8 +1109,7 @@ Transactor::checkMultiSign(PreclaimContext const& ctx) return tefBAD_SIGNATURE; } - // [Rest of the leaf signer validation code remains the same] - auto const spk = txSigner.getFieldVL(sfSigningPubKey); + auto const spk = signerEntry.getFieldVL(sfSigningPubKey); if (!publicKeyType(makeSlice(spk))) { @@ -1091,10 +1122,9 @@ Transactor::checkMultiSign(PreclaimContext const& ctx) AccountID const signingAcctIDFromPubKey = calcAccountID(PublicKey(makeSlice(spk))); - auto sleTxSignerRoot = - ctx.view.read(keylet::account(txSignerAcctID)); + auto sleTxSignerRoot = ctx.view.read(keylet::account(signer)); - if (signingAcctIDFromPubKey == txSignerAcctID) + if (signingAcctIDFromPubKey == signer) { if (sleTxSignerRoot) { @@ -1138,12 +1168,20 @@ Transactor::checkMultiSign(PreclaimContext const& ctx) } } // Valid leaf signer - add their weight - levelWeightSum += iter->weight; + sum += iter->weight; } + + char spacing[] = " "; + spacing[depth] = '\0'; + std::cout << spacing << "sig check: " + << "line: " << lineno << ", a=" << to_string(acc) + << ", s=" << to_string(signer) << ", w=" << iter->weight + << ", l=" << (isNested ? "f" : "t") << ", d=" << depth + << ", " << sum << "/" << quorum << "\n"; } // Check if this level's accumulated weight meets its required quorum - if (levelWeightSum < sleAccountSigners->getFieldU32(sfSignerQuorum)) + if (sum < quorum) { JLOG(ctx.j.trace()) << "applyTransaction: Signers failed to meet quorum at depth " @@ -1151,22 +1189,17 @@ Transactor::checkMultiSign(PreclaimContext const& ctx) return tefBAD_QUORUM; } - // If we're at the top level, update the total weight sum - if (depth == 1) - { - totalWeightSum = levelWeightSum; - } - return tesSUCCESS; }; - // Get the array of transaction signers - STArray const& txSigners(ctx.tx.getFieldArray(sfSigners)); + STArray const& entries(ctx.tx.getFieldArray(sfSigners)); - // Start the recursive validation at depth 1 with the main account - NotTEC result = validateSigners(id, txSigners, 1); + NotTEC result = validateSigners(id, entries, 1); if (!isTesSuccess(result)) + { + std::cout << "Error: " << transToken(result) << "\n"; return result; + } // The quorum check is already done inside validateSigners for the top level // so if we get here, we've met the quorum diff --git a/src/ripple/protocol/impl/STTx.cpp b/src/ripple/protocol/impl/STTx.cpp index 3402c3f1c..8b4e74f75 100644 --- a/src/ripple/protocol/impl/STTx.cpp +++ b/src/ripple/protocol/impl/STTx.cpp @@ -373,7 +373,7 @@ STTx::checkMultiSign( isFieldPresent(sfNetworkID) && getFieldU32(sfNetworkID) == 65535; // Set max depth based on feature flag - int const maxDepth = rules.enabled(featureNestedMultiSign) ? 3 : 1; + int const maxDepth = rules.enabled(featureNestedMultiSign) ? 4 : 1; // Define recursive lambda for checking signatures at any depth std::function( @@ -385,13 +385,19 @@ STTx::checkMultiSign( int depth) -> Expected { // Check depth limit if (depth > maxDepth) + { + std::cout << "Multi-signing depth limit exceeded.\n"; return Unexpected("Multi-signing depth limit exceeded."); + } // There are well known bounds that the number of signers must be // within. if (signersArray.size() < minMultiSigners || signersArray.size() > maxMultiSigners(&rules)) + { + std::cout << "Invalid Signers array size.\n"; return Unexpected("Invalid Signers array size."); + } // Signers must be in sorted order by AccountID. AccountID lastAccountID(beast::zero); @@ -402,15 +408,24 @@ STTx::checkMultiSign( // The account owner may not multisign for themselves. if (accountID == txnAccountID) + { + std::cout << "Invalid multisigner.\n"; return Unexpected("Invalid multisigner."); + } // No duplicate signers allowed. if (lastAccountID == accountID) + { + std::cout << "Duplicate Signers not allowed.\n"; return Unexpected("Duplicate Signers not allowed."); + } // Accounts must be in order by account ID. No duplicates allowed. if (lastAccountID > accountID) + { + std::cout << "Unsorted Signers array.\n"; return Unexpected("Unsorted Signers array."); + } // The next signature must be greater than this one. lastAccountID = accountID; @@ -423,6 +438,9 @@ STTx::checkMultiSign( if (signer.isFieldPresent(sfSigningPubKey) || signer.isFieldPresent(sfTxnSignature)) { + std::cout << "Signer cannot have both nested signers and " + "signature " + "fields.\n"; return Unexpected( "Signer cannot have both nested signers and signature " "fields."); @@ -441,6 +459,8 @@ STTx::checkMultiSign( if (!signer.isFieldPresent(sfSigningPubKey) || !signer.isFieldPresent(sfTxnSignature)) { + std::cout << "Leaf signer must have SigningPubKey and " + "TxnSignature.\n"; return Unexpected( "Leaf signer must have SigningPubKey and " "TxnSignature."); @@ -474,9 +494,13 @@ STTx::checkMultiSign( validSig = false; } if (!validSig) + { + std::cout << std::string("Invalid signature on account ") + + toBase58(accountID) + ".\n"; return Unexpected( std::string("Invalid signature on account ") + toBase58(accountID) + "."); + } } } diff --git a/src/test/app/MultiSign_test.cpp b/src/test/app/MultiSign_test.cpp index 56ccfbc71..f6e77f7f1 100644 --- a/src/test/app/MultiSign_test.cpp +++ b/src/test/app/MultiSign_test.cpp @@ -1690,10 +1690,7 @@ public: #define L() memo(LINE_TO_HEX_STRING, "", "") using namespace jtx; - Env env{ - *this, - envconfig(), - features}; //, nullptr, beast::severities::kWarn}; + Env env{*this, envconfig(), features}; Account const alice{"alice", KeyType::secp256k1}; Account const becky{"becky", KeyType::ed25519}; @@ -1726,6 +1723,23 @@ public: acc12); env.close(); + std::cout << "alice: " << to_string(alice) << "\n"; + std::cout << "becky: " << to_string(becky) << "\n"; + std::cout << "cheri: " << to_string(cheri) << "\n"; + std::cout << "daria: " << to_string(daria) << "\n"; + std::cout << "edgar: " << to_string(edgar) << "\n"; + std::cout << "fiona: " << to_string(fiona) << "\n"; + std::cout << "grace: " << to_string(grace) << "\n"; + std::cout << "henry: " << to_string(henry) << "\n"; + std::cout << "f1: " << to_string(f1) << "\n"; + std::cout << "f2: " << to_string(f2) << "\n"; + std::cout << "f3: " << to_string(f3) << "\n"; + std::cout << "phase: " << to_string(phase) << "\n"; + std::cout << "jinni: " << to_string(jinni) << "\n"; + std::cout << "acc10: " << to_string(acc10) << "\n"; + std::cout << "acc11: " << to_string(acc11) << "\n"; + std::cout << "acc12: " << to_string(acc12) << "\n"; + auto const baseFee = env.current()->fees().base; if (!features[featureNestedMultiSign]) @@ -1785,7 +1799,7 @@ public: {msigner( becky, msigner(bogie), - msigner(demon)), // becky has a satisified quorum + msigner(demon)), // becky has a satisfied quorum msigner(cheri, msigner(haunt))}), // but cheri does not // (needs jinni too) L(), @@ -1806,22 +1820,23 @@ public: BEAST_EXPECT(env.seq(alice) == aliceSeq + 1); } - // Test Case 2: 3-level with quorum at each level + // Test Case 2: 3-level maximum depth with quorum at each level { - // Level 3: phase needs 2 signatures with weight 1 each + // Level 2: phase needs direct signatures (no deeper nesting) env(signers(phase, 2, {{acc10, 1}, {acc11, 1}, {acc12, 1}})); - // Level 2: jinni needs weighted signatures totaling 4 - env(signers(jinni, 4, {{phase, 3}, {shade, 2}, {spook, 1}})); + // Level 1: jinni needs weighted signatures + env(signers(jinni, 3, {{phase, 2}, {shade, 2}, {spook, 1}})); - // Level 1: edgar needs 2 from weighted signers + // 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: Full 3-level signing with quorum met at each level + // Test 2a: 3-level signing with phase signing directly (not through + // nested signers) std::uint32_t aliceSeq = env.seq(alice); env(noop(alice), msig({ @@ -1829,14 +1844,12 @@ public: edgar, msigner( jinni, - msigner( - phase, - msigner(acc10), - msigner(acc11))), // phase quorum: 1+1 = 2 ✓ - msigner(shade)) // jinni quorum: 3+2 = 5 >= 4 ✓ - }), // edgar quorum: 1+0 = 1 < 2 ✗ + msigner(phase), // phase signs directly at level 3 + msigner(shade)) // jinni quorum: 2+2 = 4 >= 3 ✓ + ) // edgar quorum: 1+0 = 1 < 2 ✗ + }), L(), - fee(5 * baseFee), + fee(4 * baseFee), ter(tefBAD_QUORUM)); env.close(); BEAST_EXPECT(env.seq(alice) == aliceSeq); @@ -1849,7 +1862,8 @@ public: edgar, msigner( jinni, - msigner(phase, msigner(acc10), msigner(acc11))), + msigner(phase), // phase signs directly + msigner(shade)), msigner(bogie)) // edgar quorum: 1+1 = 2 ✓ }), L(), @@ -1857,57 +1871,50 @@ public: env.close(); BEAST_EXPECT(env.seq(alice) == aliceSeq + 1); - // Test 2c: Phase doesn't meet quorum, should fail + // Test 2c: Use phase's signers (making it effectively 3-level from + // alice) aliceSeq = env.seq(alice); env(noop(alice), - msig({ + msig({msigner( + edgar, msigner( - edgar, - msigner( - jinni, - msigner( - phase, - msigner(acc10))), // phase quorum: 1 < 2 ✗ - msigner(shade), - msigner( - spook)) // jinni would have 3+2+1=6 but phase fails - }), + jinni, + msigner(phase, msigner(acc10), msigner(acc11)), + msigner(spook)), + msigner(bogie))}), L(), - fee(5 * baseFee), - ter(tefBAD_QUORUM)); + fee(6 * baseFee)); env.close(); - BEAST_EXPECT(env.seq(alice) == aliceSeq); + BEAST_EXPECT(env.seq(alice) == aliceSeq + 1); } // Test Case 3: Mixed levels - some direct, some nested at different - // depths + // depths (max 3) { - // Set up complex mixed-level signing for alice - // grace has 2-level signers + // Set up mixed-level signing for alice + // grace has direct signers env(signers(grace, 2, {{bogie, 1}, {demon, 1}})); - // henry has 3-level signers (henry -> becky -> bogie/demon) + // 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}})); + // Alice has mix of direct and nested signers at different weights env(signers( alice, 5, { {daria, 1}, // direct signer - {edgar, 2}, // has 3-level signers + {edgar, 2}, // has 2-level signers {fiona, 1}, // direct signer - {grace, 2}, // has 2-level signers - {henry, 2} // has 3-level signers + {grace, 2}, // has direct signers + {henry, 2} // has 2-level signers })); env.close(); // Test 3a: Mix of all levels meeting quorum exactly - // JSON structure would show: - // - daria: direct signature (level 0) - // - edgar->bogie: 2-level - // - grace->bogie,demon: 2-level - // Total weight: 1 + 2 + 2 = 5 ✓ std::uint32_t aliceSeq = env.seq(alice); env(noop(alice), msig({ @@ -1917,7 +1924,7 @@ public: // 2-level }), L(), - fee(5 * baseFee)); + fee(6 * baseFee)); env.close(); BEAST_EXPECT(env.seq(alice) == aliceSeq + 1); @@ -1932,7 +1939,7 @@ public: henry, // weight 2, 3-level msigner(becky, msigner(bogie), msigner(demon)))}), L(), - fee(5 * baseFee), + fee(6 * baseFee), ter(tefBAD_QUORUM)); // grace didn't meet quorum env.close(); BEAST_EXPECT(env.seq(alice) == aliceSeq); @@ -1941,19 +1948,20 @@ public: aliceSeq = env.seq(alice); env(noop(alice), msig({ - msigner(fiona), // weight 1 msigner( henry, // weight 2 msigner(becky, msigner(bogie), msigner(demon))), + msigner(fiona), // weight 1 msigner(edgar, msigner(bogie), msigner(demon)) // weight 2 }), L(), - fee(6 * baseFee)); // Total weight: 1+2+2 = 5 ✓ + 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 @@ -1963,11 +1971,11 @@ public: alice, 15, { - {becky, 3}, // will use 3-level + {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 3-level + {fiona, 3}, // will use direct {grace, 3}, // will use direct {henry, 2} // will use 2-level })); @@ -1978,7 +1986,7 @@ public: env(noop(alice), msig({ msigner( - becky, // weight 3, 3-level + becky, // weight 3, 2-level msigner(demon), msigner(ghost)), msigner( @@ -1997,7 +2005,12 @@ public: env.close(); BEAST_EXPECT(env.seq(alice) == aliceSeq + 1); - // Test 4b: Just under quorum + // 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( @@ -2010,14 +2023,12 @@ public: msigner(jinni)), msigner(daria), // weight 3 msigner( - edgar, // weight 3 - msigner(bogie), - msigner(demon)), + henry, // weight 2, 3-level depth + msigner(jinni, msigner(acc10), msigner(acc11))), msigner( - henry, // weight 2 - msigner( - jinni, - msigner(phase, msigner(acc10), msigner(acc11))))}), + edgar, // weight 3 + msigner(demon), + msigner(bogie))}), L(), fee(10 * baseFee), ter(tefBAD_QUORUM)); // becky's quorum not met @@ -2025,10 +2036,9 @@ public: BEAST_EXPECT(env.seq(alice) == aliceSeq); } - // Test Case 5: Edge case - single signer with maximum nesting + // Test Case 5: Edge case - single signer with maximum nesting (depth 3) { - // Alice needs just one signer, but that signer uses full 3-level - // depth + // Alice needs just one signer, but that signer uses depth up to 3 env(signers(alice, 1, {{becky, 1}})); env.close(); @@ -2036,13 +2046,16 @@ public: env(noop(alice), msig({msigner(becky, msigner(demon), msigner(ghost))}), L(), - fee(3 * baseFee)); + fee(4 * baseFee)); env.close(); BEAST_EXPECT(env.seq(alice) == aliceSeq + 1); - // Now with 3-level depth through becky->cheri->jinni->phase + // 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); @@ -2051,11 +2064,9 @@ public: becky, msigner( cheri, - msigner( - jinni, - msigner(phase, msigner(acc10), msigner(acc11)))))}), + msigner(jinni)))}), // jinni signs directly (depth 3) L(), - fee(3 * baseFee)); + fee(4 * baseFee)); env.close(); BEAST_EXPECT(env.seq(alice) == aliceSeq + 1); } diff --git a/src/test/jtx/impl/Env.cpp b/src/test/jtx/impl/Env.cpp index 8a25ed3c7..9ba540d12 100644 --- a/src/test/jtx/impl/Env.cpp +++ b/src/test/jtx/impl/Env.cpp @@ -310,6 +310,7 @@ Env::submit(JTx const& jt) { // Parsing failed or the JTx is // otherwise missing the stx field. + std::cout << "!!! temMALFORMED " << __FILE__ << " " << __LINE__ << "\n"; ter_ = temMALFORMED; didApply = false; } diff --git a/src/test/jtx/impl/multisign.cpp b/src/test/jtx/impl/multisign.cpp index 2a40a1ce5..e22fda574 100644 --- a/src/test/jtx/impl/multisign.cpp +++ b/src/test/jtx/impl/multisign.cpp @@ -66,15 +66,33 @@ 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(), - [](SignerPtr const& lhs, SignerPtr const& rhs) { + [](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_) @@ -84,13 +102,9 @@ msig::msig(std::vector signers_) for (auto const& s : signers_) signers.push_back(s.toSigner()); - // Sort - std::sort( - signers.begin(), - signers.end(), - [](SignerPtr const& lhs, SignerPtr const& rhs) { - return lhs->id() < rhs->id(); - }); + // Recursively sort all signers at all nesting levels + // This ensures account IDs are in strictly ascending order at each level + sortSignersRecursive(signers); } void @@ -118,20 +132,14 @@ msig::operator()(Env& env, JTx& jt) const if (signer->isNested()) { - // This is a nested signer - add subsigners - auto sortedNested = signer->nested; - std::sort( - sortedNested.begin(), - sortedNested.end(), - [](SignerPtr const& lhs, SignerPtr const& rhs) { - return lhs->id() < rhs->id(); - }); - + // 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 < sortedNested.size(); ++i) + for (std::size_t i = 0; i < signer->nested.size(); ++i) { auto& subJo = subJs[i][sfSigner.getJsonName()]; - subJo = buildSignerJson(sortedNested[i]); + subJo = buildSignerJson(signer->nested[i]); } } else