From 752dab8b308d6b526eb3a6c4bf307111bc7b89a6 Mon Sep 17 00:00:00 2001 From: Peter Chen <34582813+PeterChen13579@users.noreply.github.com> Date: Mon, 13 Jul 2026 11:44:59 -0700 Subject: [PATCH 01/13] feat: Add delegate filter param for account_tx RPC (#6126) --- API-CHANGELOG.md | 3 + include/xrpl/protocol/jss.h | 4 + include/xrpl/rdb/RelationalDatabase.h | 19 + .../suppressions/sanitizer-ignorelist.txt | 2 +- src/test/rpc/AccountTx_test.cpp | 426 ++++++++++++++++++ src/xrpld/app/misc/NetworkOPs.cpp | 3 +- src/xrpld/app/rdb/backend/detail/Node.cpp | 102 ++++- src/xrpld/rpc/handlers/account/AccountTx.cpp | 91 +++- 8 files changed, 646 insertions(+), 4 deletions(-) diff --git a/API-CHANGELOG.md b/API-CHANGELOG.md index 56a45c132a..a04f265328 100644 --- a/API-CHANGELOG.md +++ b/API-CHANGELOG.md @@ -28,6 +28,9 @@ This section contains changes targeting a future version. ### Additions +- `account_tx`: Added an optional `delegate` request object to filter delegated transactions. The object requires `delegate_filter`, which must be either `actor` for transactions owned by the requested account but signed by another account, or `authorizer` for transactions signed by the requested account on behalf of another account. The optional `counter_party` account narrows the results to a specific signer/delegate for `actor` or a specific owner/delegator for `authorizer`. Malformed `delegate`, `delegate_filter`, and `counter_party` values return standard invalid field errors, and invalid account IDs return `actMalformed`. + When paginating delegate-filtered queries, a marker from a delegate-filtered query includes a `delegate` flag and is only valid for follow-up requests that also supply `delegate` (mixing marker conventions returns `invalidParams`). Because filtering is applied after the ledger scan, a page may contain fewer results than `limit` (possibly zero) while still returning a marker, so callers must continue until no marker is present. + - `ledger_entry`, `account_objects`: The `Delegate` ledger entry now includes an optional `DestinationNode` field, which stores the index into the authorized account's owner directory. This field is present on entries created after bidirectional directory tracking was introduced and may appear in RPC responses for those entries. ([#6681](https://github.com/XRPLF/rippled/pull/6681)) - `server_definitions`: Added the following new sections to the response ([#6321](https://github.com/XRPLF/rippled/pull/6321)): diff --git a/include/xrpl/protocol/jss.h b/include/xrpl/protocol/jss.h index b20b71661a..63e877ca31 100644 --- a/include/xrpl/protocol/jss.h +++ b/include/xrpl/protocol/jss.h @@ -110,6 +110,7 @@ JSS(accounts); // in: LedgerEntry, Subscribe, handlers/Ledger JSS(accounts_proposed); // in: Subscribe, Unsubscribe JSS(action); // JSS(active); // out: OverlayImpl +JSS(actor); // in/out: AccountTx JSS(acquiring); // out: LedgerRequest JSS(address); // out: PeerImp JSS(affected); // out: AcceptedLedgerTx @@ -133,6 +134,7 @@ JSS(attestation_reward_account); // JSS(auction_slot); // out: amm_info JSS(authorized); // out: AccountLines JSS(authorize); // out: delegate +JSS(authorizer); // in/out: AccountTx JSS(authorized_credentials); // in: ledger_entry DepositPreauth JSS(auth_accounts); // out: amm_info JSS(auth_change); // out: AccountInfo @@ -191,6 +193,7 @@ JSS(converge_time); // out: NetworkOPs JSS(converge_time_s); // out: NetworkOPs JSS(cookie); // out: NetworkOPs JSS(count); // in: AccountTx*, ValidatorList +JSS(counter_party); // in/out: AccountTx JSS(counters); // in/out: retrieve counters JSS(credentials); // in: deposit_authorized JSS(credential_type); // in: LedgerEntry DepositPreauth @@ -270,6 +273,7 @@ JSS(freeze); // out: AccountLines JSS(freeze_peer); // out: AccountLines JSS(deep_freeze); // out: AccountLines JSS(deep_freeze_peer); // out: AccountLines +JSS(delegate_filter); // in/out: AccountTx JSS(frozen_balances); // out: GatewayBalances JSS(full); // in: LedgerClearer, handlers/Ledger JSS(full_reply); // out: PathFind diff --git a/include/xrpl/rdb/RelationalDatabase.h b/include/xrpl/rdb/RelationalDatabase.h index c77c8a7ed3..e5784c7418 100644 --- a/include/xrpl/rdb/RelationalDatabase.h +++ b/include/xrpl/rdb/RelationalDatabase.h @@ -46,6 +46,22 @@ struct LedgerRange uint32_t max; }; +/** + * @brief Enumeration of possible delegate types that can occur during filtering in account_tx + */ +enum class DelegateType { + Actor, ///< Another account signed and submitted transactions on behalf of this account (this + ///< account is the owner/delegator). + Authorizer ///< This account signed and submitted transactions on behalf of another account + ///< (this account is the signer/delegatee). +}; + +struct DelegateFilter +{ + DelegateType type = DelegateType::Actor; + std::optional counterparty; +}; + class RelationalDatabase { public: @@ -82,6 +98,7 @@ public: std::optional marker; std::uint32_t limit = 0; bool bAdmin = false; + std::optional delegate; }; using AccountTx = std::pair, std::shared_ptr>; @@ -101,6 +118,7 @@ public: bool forward = false; uint32_t limit = 0; std::optional marker; + std::optional delegate; }; struct AccountTxResult @@ -109,6 +127,7 @@ public: LedgerRange ledgerRange{}; uint32_t limit = 0; std::optional marker; + std::optional delegate; }; virtual ~RelationalDatabase() = default; diff --git a/sanitizers/suppressions/sanitizer-ignorelist.txt b/sanitizers/suppressions/sanitizer-ignorelist.txt index dc9a31f8e4..ffd9b4103a 100644 --- a/sanitizers/suppressions/sanitizer-ignorelist.txt +++ b/sanitizers/suppressions/sanitizer-ignorelist.txt @@ -29,7 +29,7 @@ src:test/beast/beast_PropertyStream_test.cpp src:src/test/app/Invariants_test.cpp # ASan false positive: stack-use-after-scope in ErrorCodes.h inline functions. -# When Clang inlines the StaticString overloads (e.g. invalid_field_error(StaticString)), +# When Clang inlines the StaticString overloads (e.g. invalidFieldError(StaticString)), # ASan scope-poisons the temporary std::string before the inlined callee finishes reading # through the const ref. This corrupts the coroutine stack and crashes the Simulate test. # See asan.supp comments for full explanation and planned fix. diff --git a/src/test/rpc/AccountTx_test.cpp b/src/test/rpc/AccountTx_test.cpp index a7f37d39b9..f1fbc2871b 100644 --- a/src/test/rpc/AccountTx_test.cpp +++ b/src/test/rpc/AccountTx_test.cpp @@ -5,6 +5,7 @@ #include #include // IWYU pragma: keep #include +#include #include #include #include @@ -28,6 +29,7 @@ #include #include #include +#include #include #include #include @@ -45,6 +47,7 @@ #include #include #include +#include #include #include @@ -889,6 +892,426 @@ class AccountTx_test : public beast::unit_test::Suite checkAliceAcctTx(9, jss::Payment); } + void + testDelegation() + { + testcase("Delegation Filtering"); + + using namespace test::jtx; + + Env env(*this); + Account const alice{"alice"}; + Account const bob{"bob"}; + Account const carol{"carol"}; + + env.fund(XRP(10000), alice, bob, carol); + env.close(); + + // Normal TX: Alice pays Carol (Signed by Alice's Master Key) + env(pay(alice, carol, XRP(10))); + env.close(); + + // Setup Delegation: Alice allows Bob to sign Payments for her + env(delegate::set(alice, bob, {"Payment"})); + env.close(); + + // Delegated TX: Alice pays Bob (Signed by Bob using Delegation) + env(pay(alice, bob, XRP(20)), delegate::As(bob)); + env.close(); + + // Normal TX: Bob pays Carol (Signed by Bob for himself) + env(pay(bob, carol, XRP(30))); + env.close(); + + auto const countTxs = [&](AccountID const& account, + json::Value const& delegateParams, + std::optional const limit = std::nullopt) -> int { + int count = 0; + json::Value marker; + bool haveMarker = false; + int pages = 0; + + while (true) + { + json::Value params; + params[jss::account] = toBase58(account); + params[jss::ledger_index_min] = -1; + params[jss::ledger_index_max] = -1; + + if (!delegateParams.isNull()) + params[jss::delegate] = delegateParams; + if (limit) + params[jss::limit] = *limit; + if (haveMarker) + params[jss::marker] = marker; + + auto const res = env.rpc("json", "account_tx", to_string(params)); + auto const& result = res[jss::result]; + + if (result.isMember(jss::transactions)) + count += result[jss::transactions].size(); + + if (!limit || !result.isMember(jss::marker)) + break; + + marker = result[jss::marker]; + haveMarker = true; + ++pages; + if (!BEAST_EXPECT(pages < 20)) + break; + } + + return count; + }; + + auto const checkError = [&](json::Value const& delegateParams, + std::string const& errToken) { + json::Value params; + params[jss::account] = alice.human(); + params[jss::delegate] = delegateParams; + auto res = env.rpc("json", "account_tx", to_string(params)); + BEAST_EXPECT(res[jss::result][jss::error] == errToken); + }; + + // Filter: Delegatee. Expects TX #2 (Signed by Bob) + { + json::Value p; + p[jss::delegate_filter] = "actor"; + BEAST_EXPECT(countTxs(alice.id(), p) == 1); + } + + // Filter: Delegatee + Counterparty Bob. Expects TX #2. + { + json::Value p; + p[jss::delegate_filter] = "actor"; + p[jss::counter_party] = bob.human(); + BEAST_EXPECT(countTxs(alice.id(), p) == 1); + } + + // Filter: Delegatee + Counterparty Carol. Expects 0. + { + json::Value p; + p[jss::delegate_filter] = "actor"; + p[jss::counter_party] = carol.human(); + BEAST_EXPECT(countTxs(alice.id(), p) == 0); + } + + // Filter: Delegator. Expects TX #2. + // (Bob signed it, but Alice is the owner). + { + json::Value p; + p[jss::delegate_filter] = "authorizer"; + BEAST_EXPECT(countTxs(bob.id(), p) == 1); + } + + // Filter: Delegator + Counterparty Alice. Expects TX #2. + { + json::Value p; + p[jss::delegate_filter] = "authorizer"; + p[jss::counter_party] = alice.human(); + BEAST_EXPECT(countTxs(bob.id(), p) == 1); + } + + // Filter: Authorizer. Expect: None. + // TX #2 has sfDelegate present, but Alice is the delegator/owner, not + // the delegate signer + { + json::Value p; + p[jss::delegate_filter] = "authorizer"; + BEAST_EXPECT(countTxs(alice.id(), p) == 0); + } + + // Query Bob (Signer), Filter: Delegator, Counterparty: Carol + // Expect: None (Alice is Owner, not Carol) + { + json::Value p; + p[jss::delegate_filter] = "authorizer"; + p[jss::counter_party] = carol.human(); + BEAST_EXPECT(countTxs(bob.id(), p) == 0); + } + + // Query Bob (Signer), Filter: Delegatee + // Expect: None. Bob did not employ a delegatee for his own TXs (TX C). + { + json::Value p; + p[jss::delegate_filter] = "actor"; + BEAST_EXPECT(countTxs(bob.id(), p) == 0); + } + + // "delegate" is not an object (e.g., string) + { + json::Value const p = "not_an_object"; + checkError(p, "invalidParams"); + } + + // Missing "delegate_filter" inside object + { + json::Value const p(json::ValueType::Object); + checkError(p, "invalidParams"); + } + + // "delegate_filter" is not a string (e.g., int) + { + json::Value p; + p[jss::delegate_filter] = 123; + checkError(p, "invalidParams"); + } + + // "delegate_filter" has invalid value + { + json::Value p; + p[jss::delegate_filter] = "random_string"; + checkError(p, "invalidParams"); + } + + // "counterparty" is not a string + { + json::Value p; + p[jss::delegate_filter] = "actor"; + p[jss::counter_party] = 123; + checkError(p, "invalidParams"); + } + + // "counterparty" is malformed base58 + { + json::Value p; + p[jss::delegate_filter] = "actor"; + p[jss::counter_party] = "not_an_account"; + checkError(p, "actMalformed"); + } + + // Multi-signed non-delegated TX: Alice pays Carol via multi-sig. + // sfDelegate is absent and sfSigningPubKey is empty — the filter + // must skip it without crashing. + { + Account const daria{"daria"}; + Account const edward{"edward"}; + env.fund(XRP(1000), daria, edward); + env.close(); + env(signers(alice, 2, {{daria, 1}, {edward, 1}})); + env.close(); + env(pay(alice, carol, XRP(1)), + Fee(drops(env.current()->fees().increment * 2)), + Msig(daria, edward)); + env.close(); + + // Alice's actor filter should still see only the 1 delegated tx, + // not the multi-signed one. + json::Value p; + p[jss::delegate_filter] = "actor"; + BEAST_EXPECT(countTxs(alice.id(), p) == 1); + } + + // Regular-key-signed non-delegated TX: Alice pays Bob, signed by Bob + // as Alice's regular key. This must not be treated as delegation. + { + env(regkey(alice, bob)); + env.close(); + env(pay(alice, bob, XRP(1))); + env.close(); + + json::Value actorFilter; + actorFilter[jss::delegate_filter] = "actor"; + BEAST_EXPECT(countTxs(alice.id(), actorFilter) == 1); + // limit: 1 forces pagination past newer non-delegated rows. + BEAST_EXPECT(countTxs(alice.id(), actorFilter, 1) == 1); + + actorFilter[jss::counter_party] = bob.human(); + BEAST_EXPECT(countTxs(alice.id(), actorFilter) == 1); + BEAST_EXPECT(countTxs(alice.id(), actorFilter, 1) == 1); + + json::Value authorizerFilter; + authorizerFilter[jss::delegate_filter] = "authorizer"; + BEAST_EXPECT(countTxs(bob.id(), authorizerFilter) == 1); + BEAST_EXPECT(countTxs(bob.id(), authorizerFilter, 1) == 1); + + authorizerFilter[jss::counter_party] = alice.human(); + BEAST_EXPECT(countTxs(bob.id(), authorizerFilter) == 1); + BEAST_EXPECT(countTxs(bob.id(), authorizerFilter, 1) == 1); + } + + // Pagination marker/delegate-filter consistency. A marker returned by a + // delegate-filtered query carries a `delegate` flag and is only valid + // for a follow-up request that repeats the filter; mixing the two + // marker conventions must be rejected with invalidParams. + { + json::Value actorFilter; + actorFilter[jss::delegate_filter] = "actor"; + + // Obtain a delegate-filtered marker (limit 1 forces pagination). + json::Value dp; + dp[jss::account] = alice.human(); + dp[jss::ledger_index_min] = -1; + dp[jss::ledger_index_max] = -1; + dp[jss::delegate] = actorFilter; + dp[jss::limit] = 1; + auto const dRes = env.rpc("json", "account_tx", to_string(dp)); + BEAST_EXPECT(dRes[jss::result].isMember(jss::marker)); + json::Value const delegateMarker = dRes[jss::result][jss::marker]; + BEAST_EXPECT( + delegateMarker.isMember(jss::delegate) && delegateMarker[jss::delegate].asBool()); + + // Reusing a delegate marker without the delegate filter is rejected. + { + json::Value p; + p[jss::account] = alice.human(); + p[jss::ledger_index_min] = -1; + p[jss::ledger_index_max] = -1; + p[jss::limit] = 1; + p[jss::marker] = delegateMarker; + auto const r = env.rpc("json", "account_tx", to_string(p)); + BEAST_EXPECT(r[jss::result][jss::error] == "invalidParams"); + } + + // Obtain a non-delegate marker; it must not carry the flag. + json::Value np; + np[jss::account] = alice.human(); + np[jss::ledger_index_min] = -1; + np[jss::ledger_index_max] = -1; + np[jss::limit] = 1; + auto const nRes = env.rpc("json", "account_tx", to_string(np)); + BEAST_EXPECT(nRes[jss::result].isMember(jss::marker)); + json::Value const normalMarker = nRes[jss::result][jss::marker]; + BEAST_EXPECT(!normalMarker.isMember(jss::delegate)); + + // Reusing a non-delegate marker with a delegate filter is rejected. + { + json::Value p; + p[jss::account] = alice.human(); + p[jss::ledger_index_min] = -1; + p[jss::ledger_index_max] = -1; + p[jss::limit] = 1; + p[jss::delegate] = actorFilter; + p[jss::marker] = normalMarker; + auto const r = env.rpc("json", "account_tx", to_string(p)); + BEAST_EXPECT(r[jss::result][jss::error] == "invalidParams"); + } + } + } + + void + testDelegationMultiSign() + { + testcase("Delegation filter with multi-signed delegatee"); + using namespace test::jtx; + + Env env(*this); + Account const alice{"alice"}; + Account const bob{"bob"}; + Account const carol{"carol"}; + Account const daria{"daria"}; + Account const edward{"edward"}; + + env.fund(XRP(10000), alice, bob, carol, daria, edward); + env.close(); + + // Bob's identity is established via multi-sig (daria + edward) + env(signers(bob, 2, {{daria, 1}, {edward, 1}})); + env.close(); + + env(delegate::set(alice, bob, {"Payment"})); + env.close(); + + // Delegated tx: Alice pays Carol, Bob signs via multi-sig + env(pay(alice, carol, XRP(10)), Fee(XRP(1)), delegate::As(bob), Msig(daria, edward)); + env.close(); + + auto const countTxs = [&](AccountID const& account, + json::Value const& delegateParams) -> int { + json::Value params; + params[jss::account] = toBase58(account); + params[jss::ledger_index_min] = -1; + params[jss::ledger_index_max] = -1; + params[jss::delegate] = delegateParams; + + auto const res = env.rpc("json", "account_tx", to_string(params)); + + if (res[jss::result].isMember(jss::transactions)) + return res[jss::result][jss::transactions].size(); + return 0; + }; + + // Alice (owner) finds the tx with actor filter + { + json::Value p; + p[jss::delegate_filter] = "actor"; + BEAST_EXPECT(countTxs(alice.id(), p) == 1); + } + + // Alice (owner) + counterparty Bob finds the tx + { + json::Value p; + p[jss::delegate_filter] = "actor"; + p[jss::counter_party] = bob.human(); + BEAST_EXPECT(countTxs(alice.id(), p) == 1); + } + + // Bob (delegatee) finds the tx with authorizer filter + { + json::Value p; + p[jss::delegate_filter] = "authorizer"; + BEAST_EXPECT(countTxs(bob.id(), p) == 1); + } + + // Bob (delegatee) + counterparty Alice finds the tx + { + json::Value p; + p[jss::delegate_filter] = "authorizer"; + p[jss::counter_party] = alice.human(); + BEAST_EXPECT(countTxs(bob.id(), p) == 1); + } + } + + void + testDelegationMarkerWithinPage() + { + testcase("Delegation filter marker within a single query page"); + + using namespace test::jtx; + + Env env(*this); + Account const alice{"alice"}; + Account const bob{"bob"}; + Account const carol{"carol"}; + + env.fund(XRP(10000), alice, bob, carol); + env.close(); + + env(delegate::set(alice, bob, {"Payment"})); + env.close(); + + auto const startLedger = env.closed()->header().seq + 1; + + env(pay(alice, carol, XRP(1)), delegate::As(bob)); + env.close(); + env(pay(alice, carol, XRP(1)), delegate::As(bob)); + env.close(); + + json::Value p; + p[jss::delegate_filter] = "actor"; + + json::Value params; + params[jss::account] = alice.human(); + params[jss::ledger_index_min] = startLedger; + params[jss::ledger_index_max] = -1; + params[jss::delegate] = p; + params[jss::limit] = 1; + + auto const res = env.rpc("json", "account_tx", to_string(params)); + auto const& result = res[jss::result]; + + // The first page emits only the first delegated payment. (as page limit is set to 1) + BEAST_EXPECT(result[jss::transactions].size() == 1); + BEAST_EXPECT(result.isMember(jss::marker)); + + // Following the marker resumes right after the first payment and + // returns the second one. + json::Value page2 = params; + page2[jss::marker] = result[jss::marker]; + auto const res2 = env.rpc("json", "account_tx", to_string(page2)); + BEAST_EXPECT(res2[jss::result][jss::transactions].size() == 1); + } + void testSponsorship() { @@ -973,6 +1396,9 @@ public: testContents(); testAccountDelete(); testMPT(); + testDelegation(); + testDelegationMultiSign(); + testDelegationMarkerWithinPage(); testSponsorship(); } }; diff --git a/src/xrpld/app/misc/NetworkOPs.cpp b/src/xrpld/app/misc/NetworkOPs.cpp index 14d23b26d5..12d63c6210 100644 --- a/src/xrpld/app/misc/NetworkOPs.cpp +++ b/src/xrpld/app/misc/NetworkOPs.cpp @@ -3880,7 +3880,8 @@ NetworkOPsImp::addAccountHistoryJob(SubAccountHistoryInfoWeak subInfo) .ledgerRange = {.min = minLedger, .max = maxLedger}, .marker = marker, .limit = 0, - .bAdmin = true}; + .bAdmin = true, + .delegate = std::nullopt}; return db.newestAccountTxPage(options); }; diff --git a/src/xrpld/app/rdb/backend/detail/Node.cpp b/src/xrpld/app/rdb/backend/detail/Node.cpp index f8dc4a6981..b2f14c71ea 100644 --- a/src/xrpld/app/rdb/backend/detail/Node.cpp +++ b/src/xrpld/app/rdb/backend/detail/Node.cpp @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -28,6 +29,7 @@ #include #include #include +#include #include #include #include @@ -990,6 +992,57 @@ getNewestAccountTxsB( return getAccountTxsB(session, app, options, true, j); } +/** + * @brief Determines whether a transaction should be included in account_tx + * results based on a delegation filter. + * @param rawData Serialized transaction blob. + * @param filter The delegate filter specifying the role of the queried account + * (Actor or Authorizer) and an optional counterparty to match against. + * @param contextAccount The account passed to account_tx (the queried account). + * @return True if the transaction passes the filter and should be included, + * false if it should be skipped. + */ +static bool +passesDelegateFilter( + Blob const& rawData, + DelegateFilter const& filter, + AccountID const& contextAccount) +{ + SerialIter sit{makeSlice(rawData)}; + STTx const tx{sit}; + + AccountID const txOwner = tx.getAccountID(sfAccount); + + if (!tx.isFieldPresent(sfDelegate)) + return false; + + AccountID const txSigner = tx.getAccountID(sfDelegate); + + switch (filter.type) + { + case DelegateType::Actor: { + // Keep txns where the queried account (A) is the owner but + // another account (C) was the delegatee that signed. + bool const isDelegated = (txOwner == contextAccount) && (txSigner != contextAccount); + if (!isDelegated) + return false; + return !filter.counterparty || (txSigner == *filter.counterparty); + } + + case DelegateType::Authorizer: { + // Keep txns where the queried account (C) is the signer acting + // on behalf of another account (A, the delegator/owner). + bool const isActingAsDelegate = + (txSigner == contextAccount) && (txOwner != contextAccount); + if (!isActingAsDelegate) + return false; + return !filter.counterparty || (txOwner == *filter.counterparty); + } + } + + return false; // LCOV_EXCL_LINE +} + /** * @brief accountTxPage Searches for the oldest or newest transactions for the * account that matches the given criteria starting from the provided @@ -1020,6 +1073,7 @@ accountTxPage( { int total = 0; + bool const hasDelegateFilter = options.delegate.has_value(); bool lookingForMarker = options.marker.has_value(); std::uint32_t numberOfResults = 0; @@ -1107,6 +1161,11 @@ accountTxPage( { Blob rawData; Blob rawMeta; + // Delegate filtering happens after SQL, so skipped rows need their own + // continuation marker accounting. + std::uint32_t fetchedRows = 0; + std::optional lastEmitted; + std::optional lastScanned; // SOCI requires boost::optional (not std::optional) as parameters. boost::optional ledgerSeq; @@ -1128,18 +1187,31 @@ accountTxPage( while (st.fetch()) { + if (hasDelegateFilter) + { + ++fetchedRows; + lastScanned = { + .ledgerSeq = rangeCheckedCast(ledgerSeq.value_or(0)), + .txnSeq = txnSeq.value_or(0)}; + } + if (lookingForMarker) { if (findLedger == ledgerSeq.value_or(0) && findSeq == txnSeq.value_or(0)) { lookingForMarker = false; + // Delegate markers are continuation cursors for the last + // scanned row, so resume after the marker row. + if (hasDelegateFilter) + continue; } else { continue; } } - else if (numberOfResults == 0) + + if (!hasDelegateFilter && numberOfResults == 0) { newmarker = { .ledgerSeq = rangeCheckedCast(ledgerSeq.value_or(0)), @@ -1165,6 +1237,23 @@ accountTxPage( rawMeta.clear(); } + if (hasDelegateFilter) + { + if (rawData.empty() || + !passesDelegateFilter(rawData, options.delegate.value(), options.account)) + { + rawData.clear(); + rawMeta.clear(); + continue; + } + + if (numberOfResults == 0) + { + newmarker = lastEmitted; + break; + } + } + // Work around a bug that could leave the metadata missing if (rawMeta.empty()) onUnsavedLedger(ledgerSeq.value_or(0)); @@ -1186,7 +1275,18 @@ accountTxPage( --numberOfResults; total++; + if (hasDelegateFilter) + { + lastEmitted = { + .ledgerSeq = rangeCheckedCast(ledgerSeq.value_or(0)), + .txnSeq = txnSeq.value_or(0)}; + } } + + // If this filtered page did not fill the requested number of results, + // still return a marker so the caller can continue scanning later rows. + if (hasDelegateFilter && !newmarker && !lookingForMarker && fetchedRows == queryLimit) + newmarker = lastScanned; } return {newmarker, total}; diff --git a/src/xrpld/rpc/handlers/account/AccountTx.cpp b/src/xrpld/rpc/handlers/account/AccountTx.cpp index 6c6d2bb6fd..c43f560861 100644 --- a/src/xrpld/rpc/handlers/account/AccountTx.cpp +++ b/src/xrpld/rpc/handlers/account/AccountTx.cpp @@ -30,6 +30,7 @@ #include #include +#include #include #include #include @@ -38,6 +39,48 @@ namespace xrpl { +static std::expected +parseDelegateFilter(json::Value const& delegateNode) +{ + if (!delegateNode.isObject()) + return std::unexpected(RPC::invalidFieldError(jss::delegate)); + + if (!delegateNode.isMember(jss::delegate_filter) || + !delegateNode[jss::delegate_filter].isString()) + return std::unexpected(RPC::invalidFieldError(jss::delegate_filter)); + + auto const& delegateFilterStr = delegateNode[jss::delegate_filter].asString(); + + auto typeResult = [&] -> std::expected { + if (delegateFilterStr == "actor") + return DelegateType::Actor; + + if (delegateFilterStr == "authorizer") + return DelegateType::Authorizer; + + return std::unexpected(RPC::invalidFieldError(jss::delegate_filter)); + }(); + + if (!typeResult) + return std::unexpected(typeResult.error()); + + DelegateType const type = *typeResult; + + std::optional counterparty; + if (delegateNode.isMember(jss::counter_party)) + { + if (!delegateNode[jss::counter_party].isString()) + return std::unexpected(RPC::invalidFieldError(jss::counter_party)); + + counterparty = parseBase58(delegateNode[jss::counter_party].asString()); + + if (!counterparty) + return std::unexpected(rpcError(RpcActMalformed)); + } + + return DelegateFilter{.type = type, .counterparty = counterparty}; +} + using TxnsData = RelationalDatabase::AccountTxs; using TxnsDataBinary = RelationalDatabase::MetaTxsList; using TxnDataBinary = RelationalDatabase::txnMetaLedgerType; @@ -231,7 +274,8 @@ doAccountTxHelp(RPC::Context& context, AccountTxArgs const& args) .ledgerRange = result.ledgerRange, .marker = result.marker, .limit = args.limit, - .bAdmin = isUnlimited(context.role)}; + .bAdmin = isUnlimited(context.role), + .delegate = args.delegate}; auto& db = context.app.getRelationalDatabase(); @@ -370,6 +414,9 @@ populateJsonResponse( response[jss::marker] = json::ValueType::Object; response[jss::marker][jss::ledger] = result.marker->ledgerSeq; response[jss::marker][jss::seq] = result.marker->txnSeq; + + if (args.delegate) + response[jss::marker][jss::delegate] = true; } } @@ -386,7 +433,17 @@ populateJsonResponse( // limit: integer, // optional // marker: object {ledger: ledger_index, seq: txn_sequence} // optional, // resume previous query +// delegate: object { // optional +// delegate_filter: string, // required; "actor" or "authorizer" +// counter_party: account // optional +// } // } +// +// Pagination note for delegate-filtered queries: the `delegate` object (both +// `delegate_filter` and `counter_party`) must be supplied unchanged on every +// paginated request until the query completes. A marker returned by a +// delegate-filtered query is only valid for a follow-up request that repeats +// the same `delegate` object json::Value doAccountTx(RPC::JsonContext& context) { @@ -454,6 +511,38 @@ doAccountTx(RPC::JsonContext& context) .ledgerSeq = token[jss::ledger].asUInt(), .txnSeq = token[jss::seq].asUInt()}; } + if (params.isMember(jss::delegate)) + { + if (auto const filter = parseDelegateFilter(params[jss::delegate]); filter.has_value()) + { + args.delegate = *filter; + } + else + { + return filter.error(); + } + } + + // A marker produced by a delegate-filtered query uses a different + // pagination cursor than a normal query, so it is only valid when the same + // `delegate` object is supplied again. Reject any mismatch so pagination + // cannot silently skip or duplicate results. + if (args.marker) + { + bool const markerFromDelegate = params[jss::marker].isMember(jss::delegate) && + params[jss::marker][jss::delegate].isBool() && + params[jss::marker][jss::delegate].asBool(); + if (markerFromDelegate != args.delegate.has_value()) + { + RPC::Status const status{ + RpcInvalidParams, + "Do not mix delegate and non-delegate pagination markers in account_tx; " + "repeat the same `delegate` object when using a delegate marker."}; + status.inject(response); + return response; + } + } + auto res = doAccountTxHelp(context, args); JLOG(context.j.debug()) << __func__ << " populating response"; return populateJsonResponse(res, args, context); From cd06ee221d69d5afc56513fb49bc7da9316aaf61 Mon Sep 17 00:00:00 2001 From: Ayaz Salikhov Date: Mon, 13 Jul 2026 20:19:18 +0100 Subject: [PATCH 02/13] chore: Run clang_tidy_check with `pass_filenames: false` from pre-commit (#7800) --- .cspell.config.yaml | 1 + .pre-commit-config.yaml | 5 ++ bin/pre-commit/clang_tidy_check.py | 128 +++++++++++++++++++++++------ 3 files changed, 107 insertions(+), 27 deletions(-) diff --git a/.cspell.config.yaml b/.cspell.config.yaml index 3c5191f5e1..19b3970865 100644 --- a/.cspell.config.yaml +++ b/.cspell.config.yaml @@ -65,6 +65,7 @@ words: - Btrfs - Buildx - canonicality + - canonicalised - changespq - checkme - choco diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index d3bf0209fb..8a689c0f8b 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -32,6 +32,11 @@ repos: # as standalone translation units, so they have no compile_commands.json # entry to lint (verify_headers checks them transitively). exclude: '^include/xrpl/protocol_autogen|\.ipp$' + # run-clang-tidy --fix may edit headers included by files it is not run on, + # so pre-commit must not split the files across parallel hook invocations. + # The script determines the staged files itself and lets run-clang-tidy + # handle parallelism internally. + pass_filenames: false - id: fix-include-style name: fix include style entry: ./bin/pre-commit/fix_include_style.py diff --git a/bin/pre-commit/clang_tidy_check.py b/bin/pre-commit/clang_tidy_check.py index 5b5792b405..cf4808d2ea 100755 --- a/bin/pre-commit/clang_tidy_check.py +++ b/bin/pre-commit/clang_tidy_check.py @@ -1,27 +1,46 @@ #!/usr/bin/env python3 -"""Pre-commit hook that runs clang-tidy on changed files using run-clang-tidy. +"""Pre-commit hook that runs clang-tidy on staged files using run-clang-tidy. -The set of files is chosen by pre-commit (see .pre-commit-config.yaml), which -filters to C/C++ sources and excludes `.ipp` fragments. Headers are linted -directly: the `verify_headers` build option (ON by default) compiles every -`.h`/`.hpp` on its own, so each header is the main file of its own -compile_commands.json entry and run-clang-tidy can analyse it just like a -`.cpp`. +The script determines the staged files itself (see `pass_filenames: false` in +.pre-commit-config.yaml) so run-clang-tidy is run once and handles parallelism +internally: pre-commit would otherwise split the files across parallel hook +invocations that race when fixes edit a shared header. + +Fixes are collected with `-export-fixes` and applied by clang-apply-replacements +in a separate step rather than with run-clang-tidy's `-fix`. The `add_module` +build isolates each module's headers behind a per-module symlink directory +(build/modules//...), so a header reachable from several translation +units is referenced through different paths that all resolve to the same source +file. clang-apply-replacements deduplicates identical replacements by their +literal path, so those paths must be canonicalised to the real source path +first; otherwise the same fix is applied once per path and corrupts the header. """ from __future__ import annotations import os +import re import shutil import subprocess import sys +import tempfile from pathlib import Path CLANG_TIDY_VERSION = 22 +# Extensions run-clang-tidy can analyse: `.cpp` translation units and, thanks to +# the `verify_headers` build option, `.h`/`.hpp` headers (each has its own +# compile_commands.json entry). `.ipp` fragments have no entry and are skipped. +TIDY_EXTENSIONS = {".cpp", ".h", ".hpp"} -def find_run_clang_tidy() -> str | None: - for candidate in (f"run-clang-tidy-{CLANG_TIDY_VERSION}", "run-clang-tidy"): +# A single-quoted `FilePath:` entry in an -export-fixes YAML file, allowing the +# `- ` marker that precedes it inside a `Replacements:` sequence. clang-tidy +# emits paths single-quoted and doubles any embedded quote per YAML rules. +FILEPATH_RE = re.compile(r"^(\s*(?:-\s+)?FilePath:\s*)'((?:[^']|'')*)'\s*$") + + +def find_tool(name: str) -> str | None: + for candidate in (f"{name}-{CLANG_TIDY_VERSION}", name): if path := shutil.which(candidate): return path return None @@ -35,23 +54,43 @@ def find_build_dir(repo_root: Path) -> Path | None: return None +def staged_files(repo_root: Path) -> list[Path]: + """Return absolute paths of staged, lint-able C/C++ files. + + `--diff-filter=d` excludes deletions so we never lint a removed file. + """ + output = subprocess.check_output( + ["git", "diff", "--staged", "--name-only", "--diff-filter=d", "--"] + + [f"*{ext}" for ext in TIDY_EXTENSIONS], + text=True, + cwd=repo_root, + ) + return [repo_root / rel for rel in output.splitlines() if rel] + + +def canonicalize_fix_paths(fixes_dir: Path) -> None: + """Rewrite every `FilePath` in the exported fixes to its real source path. + + A header included through a module's isolation symlink is recorded under that + symlink's path; collapsing all paths to the same real file lets + clang-apply-replacements recognise the per-translation-unit duplicates and + apply each fix once. + """ + for yaml in fixes_dir.glob("*.yaml"): + lines = [] + for line in yaml.read_text().splitlines(): + if m := FILEPATH_RE.match(line): + path = m.group(2).replace("''", "'") + real = os.path.realpath(path).replace("'", "''") + line = f"{m.group(1)}'{real}'" + lines.append(line) + yaml.write_text("\n".join(lines) + "\n") + + def main(): if not os.environ.get("TIDY"): return 0 - files = sys.argv[1:] - if not files: - return 0 - - run_clang_tidy = find_run_clang_tidy() - if not run_clang_tidy: - print( - f"clang-tidy check failed: TIDY is enabled but neither " - f"'run-clang-tidy-{CLANG_TIDY_VERSION}' nor 'run-clang-tidy' was found in PATH.", - file=sys.stderr, - ) - return 1 - repo_root = Path( subprocess.check_output( ["git", "rev-parse", "--show-toplevel"], @@ -59,6 +98,29 @@ def main(): text=True, ).strip() ) + + files = staged_files(repo_root) + if not files: + return 0 + + run_clang_tidy = find_tool("run-clang-tidy") + clang_apply_replacements = find_tool("clang-apply-replacements") + missing = [ + name + for name, path in ( + ("run-clang-tidy", run_clang_tidy), + ("clang-apply-replacements", clang_apply_replacements), + ) + if not path + ] + if missing: + print( + f"clang-tidy check failed: TIDY is enabled but {' and '.join(missing)} " + f"was not found in PATH (tried the '-{CLANG_TIDY_VERSION}' suffix too).", + file=sys.stderr, + ) + return 1 + build_dir = find_build_dir(repo_root) if not build_dir: print( @@ -68,11 +130,23 @@ def main(): ) return 1 - result = subprocess.run( - [run_clang_tidy, "-quiet", "-p", str(build_dir), "-fix", "-allow-no-checks"] - + files - ) - return result.returncode + with tempfile.TemporaryDirectory() as fixes_dir: + result = subprocess.run( + [ + run_clang_tidy, + "-quiet", + "-p", + build_dir, + "-export-fixes", + fixes_dir, + "-allow-no-checks", + ] + + files + ) + canonicalize_fix_paths(Path(fixes_dir)) + applied = subprocess.run([clang_apply_replacements, fixes_dir]) + + return result.returncode or applied.returncode if __name__ == "__main__": From acd54fd6275fdf04dd024900dde6693accd1d955 Mon Sep 17 00:00:00 2001 From: Bart Date: Tue, 14 Jul 2026 06:22:49 -0400 Subject: [PATCH 03/13] ci: Do not run conflict checker when label is applied (#7774) Co-authored-by: Bart <11445373+bthomee@users.noreply.github.com> --- .github/workflows/conflicting-pr.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/conflicting-pr.yml b/.github/workflows/conflicting-pr.yml index 772d46fd7d..cf65640954 100644 --- a/.github/workflows/conflicting-pr.yml +++ b/.github/workflows/conflicting-pr.yml @@ -14,6 +14,7 @@ permissions: jobs: main: + if: ${{ !contains(github.event.pull_request.labels.*.name, 'IgnoreConflicts') }} runs-on: ubuntu-latest steps: - name: Check if PRs are dirty From e1d4f357dc58641e2f4c1d41b286bbb9e2f38552 Mon Sep 17 00:00:00 2001 From: Ayaz Salikhov Date: Tue, 14 Jul 2026 13:21:40 +0100 Subject: [PATCH 04/13] chore: Enable most readability checks (#7772) --- .clang-tidy | 17 +- include/xrpl/beast/rfc2616.h | 13 +- include/xrpl/rdb/SociDB.h | 4 +- src/libxrpl/core/detail/Workers.cpp | 12 +- src/libxrpl/json/json_value.cpp | 3 +- src/libxrpl/ledger/helpers/MPTokenHelpers.cpp | 11 +- src/libxrpl/ledger/helpers/TokenHelpers.cpp | 10 +- src/libxrpl/protocol/STObject.cpp | 2 +- src/libxrpl/protocol/STPathSet.cpp | 12 +- src/libxrpl/protocol/UintTypes.cpp | 2 +- src/libxrpl/rdb/SociDB.cpp | 4 +- .../tx/invariants/DirectoryInvariant.cpp | 10 +- src/libxrpl/tx/invariants/FreezeInvariant.cpp | 20 +- .../tx/invariants/LoanBrokerInvariant.cpp | 10 +- src/test/app/CheckMPT_test.cpp | 24 - src/test/app/Invariants_test.cpp | 2 +- src/test/app/MPToken_test.cpp | 2 +- src/test/app/NFTokenDir_test.cpp | 505 ------------------ src/test/app/PayStrand_test.cpp | 8 +- src/test/app/PermissionedDEX_test.cpp | 11 +- src/test/app/ReducedOffer_test.cpp | 49 -- src/test/app/ValidatorList_test.cpp | 2 +- src/test/app/XChain_test.cpp | 10 +- src/test/consensus/Consensus_test.cpp | 10 - src/test/core/SociDB_test.cpp | 58 +- src/test/csf/Peer.h | 8 +- src/test/jtx/TestHelpers.h | 8 +- src/test/jtx/impl/AMM.cpp | 11 +- src/test/nodestore/Timing_test.cpp | 3 - src/test/overlay/cluster_test.cpp | 7 +- src/test/overlay/reduce_relay_test.cpp | 8 +- src/test/protocol/STAmount_test.cpp | 45 -- src/test/rpc/ServerDefinitions_test.cpp | 15 +- src/test/server/ServerStatus_test.cpp | 4 - src/tests/libxrpl/basics/Buffer.cpp | 4 +- src/tests/libxrpl/basics/base_uint_test.cpp | 19 - src/xrpld/app/misc/NetworkOPs.cpp | 10 +- src/xrpld/app/misc/SHAMapStoreImp.cpp | 6 +- src/xrpld/app/misc/detail/ValidatorSite.cpp | 2 +- src/xrpld/peerfinder/detail/Logic.h | 16 +- src/xrpld/rpc/detail/RPCCall.cpp | 10 +- .../server_info/ServerDefinitions.cpp | 2 +- 42 files changed, 102 insertions(+), 887 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index 88dd6f4e57..68fc9e75fc 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -56,32 +56,17 @@ Checks: "-*, readability-*, -readability-avoid-const-params-in-decls, - -readability-avoid-unconditional-preprocessor-if, -readability-container-data-pointer, - -readability-delete-null-pointer, -readability-function-cognitive-complexity, - -readability-function-size, -readability-identifier-length, -readability-inconsistent-declaration-parameter-name, -readability-isolate-declaration, -readability-magic-numbers, - -readability-misplaced-array-index, -readability-named-parameter, - -readability-operators-representation, -readability-qualified-auto, -readability-redundant-access-specifiers, - -readability-redundant-control-flow, - -readability-redundant-function-ptr-dereference, - -readability-redundant-preprocessor, - -readability-redundant-smartptr-get, - -readability-redundant-string-cstr, - -readability-simplify-subscript-expr, -readability-static-accessed-through-instance, - -readability-string-compare, - -readability-uniqueptr-delete-release, - -readability-uppercase-literal-suffix, - -readability-use-anyofallof, - -readability-use-concise-preprocessor-directives + -readability-uppercase-literal-suffix " # --- # bugprone-narrowing-conversions, # This will break a lot of code but we should enable it in the future because it can eliminate a lot of bugs diff --git a/include/xrpl/beast/rfc2616.h b/include/xrpl/beast/rfc2616.h index bd9a78fddb..1986568553 100644 --- a/include/xrpl/beast/rfc2616.h +++ b/include/xrpl/beast/rfc2616.h @@ -358,12 +358,13 @@ template bool tokenInList(boost::string_ref const& value, boost::string_ref const& token) { - for (auto const& item : makeList(value)) - { - if (ciEqual(item, token)) - return true; - } - return false; + auto const list = makeList(value); + // ListIterator is not default-constructible, so it does not model a std::ranges + // sentinel/range; the classic std::any_of (which only needs an input iterator) + // is used instead. + // NOLINTNEXTLINE(modernize-use-ranges) + return std::any_of( + list.begin(), list.end(), [&token](auto const& item) { return ciEqual(item, token); }); } template diff --git a/include/xrpl/rdb/SociDB.h b/include/xrpl/rdb/SociDB.h index 61edcf2263..80d83f0f75 100644 --- a/include/xrpl/rdb/SociDB.h +++ b/include/xrpl/rdb/SociDB.h @@ -13,7 +13,7 @@ #include #include -#if defined(__clang__) +#ifdef __clang__ #pragma clang diagnostic push #pragma clang diagnostic ignored "-Wdeprecated" #endif @@ -120,6 +120,6 @@ makeCheckpointer(std::uintptr_t id, std::weak_ptr, JobQueue&, Ser } // namespace xrpl -#if defined(__clang__) +#ifdef __clang__ #pragma clang diagnostic pop #endif diff --git a/src/libxrpl/core/detail/Workers.cpp b/src/libxrpl/core/detail/Workers.cpp index 0d9c1afd26..abbbb2f25c 100644 --- a/src/libxrpl/core/detail/Workers.cpp +++ b/src/libxrpl/core/detail/Workers.cpp @@ -127,15 +127,11 @@ Workers::deleteWorkers(beast::LockFreeStack& stack) { Worker const* const worker = stack.popFront(); - if (worker != nullptr) - { - // This call blocks until the thread orderly exits - delete worker; - } - else - { + if (worker == nullptr) break; - } + + // This call blocks until the thread orderly exits + delete worker; } } diff --git a/src/libxrpl/json/json_value.cpp b/src/libxrpl/json/json_value.cpp index cf95d873fb..e7ebb04495 100644 --- a/src/libxrpl/json/json_value.cpp +++ b/src/libxrpl/json/json_value.cpp @@ -314,8 +314,7 @@ Value::~Value() case ValueType::Array: case ValueType::Object: - if (value_.mapVal != nullptr) - delete value_.mapVal; + delete value_.mapVal; break; // LCOV_EXCL_START diff --git a/src/libxrpl/ledger/helpers/MPTokenHelpers.cpp b/src/libxrpl/ledger/helpers/MPTokenHelpers.cpp index ac14c3b66d..6fe7328fa7 100644 --- a/src/libxrpl/ledger/helpers/MPTokenHelpers.cpp +++ b/src/libxrpl/ledger/helpers/MPTokenHelpers.cpp @@ -28,6 +28,7 @@ #include #include +#include #include #include #include @@ -80,13 +81,9 @@ isAnyFrozen( return true; } - for (auto const& account : accounts) - { - if (isVaultPseudoAccountFrozen(view, account, mptIssue, depth)) - return true; - } - - return false; + return std::ranges::any_of(accounts, [&](auto const& account) { + return isVaultPseudoAccountFrozen(view, account, mptIssue, depth); + }); } Rate diff --git a/src/libxrpl/ledger/helpers/TokenHelpers.cpp b/src/libxrpl/ledger/helpers/TokenHelpers.cpp index 06e366847d..79e10cdf79 100644 --- a/src/libxrpl/ledger/helpers/TokenHelpers.cpp +++ b/src/libxrpl/ledger/helpers/TokenHelpers.cpp @@ -28,6 +28,7 @@ #include #include +#include #include #include #include @@ -105,12 +106,9 @@ isAnyFrozen( std::initializer_list const& accounts, Issue const& issue) { - for (auto const& account : accounts) - { - if (isFrozen(view, account, issue.currency, issue.account)) - return true; - } - return false; + return std::ranges::any_of(accounts, [&](auto const& account) { + return isFrozen(view, account, issue.currency, issue.account); + }); } bool diff --git a/src/libxrpl/protocol/STObject.cpp b/src/libxrpl/protocol/STObject.cpp index e86b3fb6e6..ab9ae7d4d7 100644 --- a/src/libxrpl/protocol/STObject.cpp +++ b/src/libxrpl/protocol/STObject.cpp @@ -710,7 +710,7 @@ STObject::getFieldNumber(SField const& field) const void STObject::set(std::unique_ptr v) { - set(std::move(*v.get())); + set(std::move(*v)); } void diff --git a/src/libxrpl/protocol/STPathSet.cpp b/src/libxrpl/protocol/STPathSet.cpp index d61f17ecc6..8987d05f1e 100644 --- a/src/libxrpl/protocol/STPathSet.cpp +++ b/src/libxrpl/protocol/STPathSet.cpp @@ -13,6 +13,7 @@ #include #include +#include #include #include #include @@ -159,13 +160,10 @@ STPathSet::isDefault() const bool STPath::hasSeen(AccountID const& account, PathAsset const& asset, AccountID const& issuer) const { - for (auto& p : path_) - { - if (p.getAccountID() == account && p.getPathAsset() == asset && p.getIssuerID() == issuer) - return true; - } - - return false; + return std::ranges::any_of(path_, [&](auto& p) { + return p.getAccountID() == account && p.getPathAsset() == asset && + p.getIssuerID() == issuer; + }); } json::Value diff --git a/src/libxrpl/protocol/UintTypes.cpp b/src/libxrpl/protocol/UintTypes.cpp index 486c11ba45..e1b8895f44 100644 --- a/src/libxrpl/protocol/UintTypes.cpp +++ b/src/libxrpl/protocol/UintTypes.cpp @@ -64,7 +64,7 @@ to_string(Currency const& currency) bool toCurrency(Currency& currency, std::string const& code) { - if (code.empty() || (code.compare(systemCurrencyCode()) == 0)) + if (code.empty() || code == systemCurrencyCode()) { currency = beast::kZero; return true; diff --git a/src/libxrpl/rdb/SociDB.cpp b/src/libxrpl/rdb/SociDB.cpp index 7354ef9b23..2c3fb1bde1 100644 --- a/src/libxrpl/rdb/SociDB.cpp +++ b/src/libxrpl/rdb/SociDB.cpp @@ -17,7 +17,7 @@ #include #include #include -#if defined(__clang__) +#ifdef __clang__ #pragma clang diagnostic push #pragma clang diagnostic ignored "-Wdeprecated" #endif @@ -342,6 +342,6 @@ makeCheckpointer( } // namespace xrpl -#if defined(__clang__) +#ifdef __clang__ #pragma clang diagnostic pop #endif diff --git a/src/libxrpl/tx/invariants/DirectoryInvariant.cpp b/src/libxrpl/tx/invariants/DirectoryInvariant.cpp index 1624a19830..b850f02ab5 100644 --- a/src/libxrpl/tx/invariants/DirectoryInvariant.cpp +++ b/src/libxrpl/tx/invariants/DirectoryInvariant.cpp @@ -5,6 +5,7 @@ #include #include #include +#include #include #include #include @@ -12,6 +13,7 @@ #include #include +#include #include namespace xrpl { @@ -88,17 +90,15 @@ ValidBookDirectory::finalize( return false; } - for (auto const& rootIndex : rootIndexes_) - { + return std::ranges::all_of(rootIndexes_, [&](auto const& rootIndex) { auto const root = view.read(Keylet(ltDIR_NODE, rootIndex)); if (!root) { JLOG(j.fatal()) << "Invariant failed: book directory root missing"; return false; } - } - - return true; + return true; + }); } } // namespace xrpl diff --git a/src/libxrpl/tx/invariants/FreezeInvariant.cpp b/src/libxrpl/tx/invariants/FreezeInvariant.cpp index 0293d42e97..0a604d4c39 100644 --- a/src/libxrpl/tx/invariants/FreezeInvariant.cpp +++ b/src/libxrpl/tx/invariants/FreezeInvariant.cpp @@ -16,6 +16,7 @@ #include #include +#include #include namespace xrpl { @@ -73,8 +74,8 @@ TransfersNotFrozen::finalize( */ [[maybe_unused]] bool const enforce = view.rules().enabled(featureDeepFreeze); - for (auto const& [issue, changes] : balanceChanges_) - { + return std::ranges::all_of(balanceChanges_, [&](auto const& entry) { + auto const& [issue, changes] = entry; auto const issuerSle = findIssuer(issue.account, view); // It should be impossible for the issuer to not be found, but check // just in case so xrpld doesn't crash in release. @@ -86,20 +87,11 @@ TransfersNotFrozen::finalize( enforce, "xrpl::TransfersNotFrozen::finalize : enforce " "invariant."); - if (enforce) - { - return false; - } - continue; + return !enforce; } - if (!validateIssuerChanges(issuerSle, changes, tx, j, enforce)) - { - return false; - } - } - - return true; + return validateIssuerChanges(issuerSle, changes, tx, j, enforce); + }); } bool diff --git a/src/libxrpl/tx/invariants/LoanBrokerInvariant.cpp b/src/libxrpl/tx/invariants/LoanBrokerInvariant.cpp index d239acd417..b70c02947f 100644 --- a/src/libxrpl/tx/invariants/LoanBrokerInvariant.cpp +++ b/src/libxrpl/tx/invariants/LoanBrokerInvariant.cpp @@ -15,6 +15,8 @@ #include #include +#include + namespace xrpl { void @@ -127,8 +129,8 @@ ValidLoanBroker::finalize( } } - for (auto const& [brokerID, broker] : brokers_) - { + return std::ranges::all_of(brokers_, [&](auto const& entry) { + auto const& [brokerID, broker] = entry; auto const& after = broker.brokerAfter ? broker.brokerAfter : view.read(keylet::loanBroker(brokerID)); @@ -204,8 +206,8 @@ ValidLoanBroker::finalize( return false; } } - } - return true; + return true; + }); } } // namespace xrpl diff --git a/src/test/app/CheckMPT_test.cpp b/src/test/app/CheckMPT_test.cpp index 6370be7b6f..66cc582201 100644 --- a/src/test/app/CheckMPT_test.cpp +++ b/src/test/app/CheckMPT_test.cpp @@ -793,15 +793,6 @@ class CheckMPT_test : public beast::unit_test::Suite env(check::create(alice, bob, usd(125))); env.close(); - // alice writes another check that won't get cashed until the transfer - // rate changes so we can see the rate applies when the check is - // cashed, not when it is created. -#if 0 - uint256 const chkId120{getCheckIndex(alice, env.Seq(alice))}; - env(check::create(alice, bob, USD(120))); - env.close(); -#endif - // bob attempts to cash the check for face value. Should fail. env(check::cash(bob, chkId125, usd(125)), Ter(tecPATH_PARTIAL)); env.close(); @@ -816,21 +807,6 @@ class CheckMPT_test : public beast::unit_test::Suite env.require(Balance(bob, usd(0 + 100))); BEAST_EXPECT(checksOnAccount(env, alice).empty()); BEAST_EXPECT(checksOnAccount(env, bob).empty()); - -#if 0 - // Adjust gw's rate... - env(rate(gw, 1.2)); - env.close(); - - // bob cashes the second check for less than the face value. The new - // rate applies to the actual value transferred. - env(check::cash(bob, chkId120, USD(50))); - env.close(); - env.Require(Balance(alice, USD(1000 - 125 - 60))); - env.Require(Balance(bob, USD(0 + 100 + 50))); - BEAST_EXPECT(checksOnAccount(env, alice).size() == 0); - BEAST_EXPECT(checksOnAccount(env, bob).size() == 0); -#endif } void diff --git a/src/test/app/Invariants_test.cpp b/src/test/app/Invariants_test.cpp index 461a151d8e..569eecb27b 100644 --- a/src/test/app/Invariants_test.cpp +++ b/src/test/app/Invariants_test.cpp @@ -2741,7 +2741,7 @@ class Invariants_test : public beast::unit_test::Suite } void - testVault() + testVault() // NOLINT(readability-function-size) { using namespace test::jtx; diff --git a/src/test/app/MPToken_test.cpp b/src/test/app/MPToken_test.cpp index 7543db7364..befc46e2ae 100644 --- a/src/test/app/MPToken_test.cpp +++ b/src/test/app/MPToken_test.cpp @@ -2115,7 +2115,7 @@ class MPToken_test : public beast::unit_test::Suite jv[jss::Account] = alice.human(); jv[sfSponsee.fieldName] = carol.human(); jv[sfFeeAmount.fieldName] = mpt.getJson(JsonOptions::Values::None); - test(jv, sfFeeAmount.fieldName.c_str()); + test(jv, sfFeeAmount.fieldName); } } BEAST_EXPECT(txWithAmounts.empty()); diff --git a/src/test/app/NFTokenDir_test.cpp b/src/test/app/NFTokenDir_test.cpp index 19bf58f247..7dd0b14fe5 100644 --- a/src/test/app/NFTokenDir_test.cpp +++ b/src/test/app/NFTokenDir_test.cpp @@ -1052,508 +1052,3 @@ BEAST_DEFINE_TESTSUITE_PRIO(NFTokenDir, app, xrpl, 1); // // sp6JS7f14BuwFY8MwFe95Vpi9Znjs // - -// Sets of related accounts. -// -// Identifying the seeds of accounts that generate account IDs with the -// same low 32 bits takes a while. However several sets of accounts with -// that relationship have been located. In case these sets of accounts are -// needed for future testing scenarios they are recorded below. -#if 0 -34 account seeds that produce account IDs with low 32-bits 0x399187e9: - sp6JS7f14BuwFY8Mw5EYu5z86hKDL - sp6JS7f14BuwFY8Mw5PUAMwc5ygd7 - sp6JS7f14BuwFY8Mw5R3xUBcLSeTs - sp6JS7f14BuwFY8Mw5W6oS5sdC3oF - sp6JS7f14BuwFY8Mw5pYc3D9iuLcw - sp6JS7f14BuwFY8Mw5pfGVnhcdp3b - sp6JS7f14BuwFY8Mw6jS6RdEqXqrN - sp6JS7f14BuwFY8Mw6krt6AKbvRXW - sp6JS7f14BuwFY8Mw6mnVBQq7cAN2 - sp6JS7f14BuwFY8Mw8ECJxPjmkufQ - sp6JS7f14BuwFY8Mw8asgzcceGWYm - sp6JS7f14BuwFY8MwF6J3FXnPCgL8 - sp6JS7f14BuwFY8MwFEud2w5czv5q - sp6JS7f14BuwFY8MwFNxKVqJnx8P5 - sp6JS7f14BuwFY8MwFnTCXg3eRidL - sp6JS7f14BuwFY8Mwj47hv1vrDge6 - sp6JS7f14BuwFY8Mwj6TYekeeyukh - sp6JS7f14BuwFY8MwjFjsRDerz7jb - sp6JS7f14BuwFY8Mwjrj9mHTLBrcX - sp6JS7f14BuwFY8MwkKcJi3zMzAea - sp6JS7f14BuwFY8MwkYTDdnYRm9z4 - sp6JS7f14BuwFY8Mwkq8ei4D8uPNd - sp6JS7f14BuwFY8Mwm2pFruxbnJRd - sp6JS7f14BuwFY8MwmJV2ZnAjpC2g - sp6JS7f14BuwFY8MwmTFMPHQHfVYF - sp6JS7f14BuwFY8MwmkG2jXEgqiud - sp6JS7f14BuwFY8Mwms3xEh5tMDTw - sp6JS7f14BuwFY8MwmtipW4D8giZ9 - sp6JS7f14BuwFY8MwoRQBZm4KUUeE - sp6JS7f14BuwFY8MwoVey94QpXcrc - sp6JS7f14BuwFY8MwoZiuUoUTo3VG - sp6JS7f14BuwFY8MwonFFDLT4bHAZ - sp6JS7f14BuwFY8MwooGphD4hefBQ - sp6JS7f14BuwFY8MwoxDp3dmX6q5N - -34 account seeds that produce account IDs with low 32-bits 0x473f2c9a: - sp6JS7f14BuwFY8Mw53ktgqmv5Bmz - sp6JS7f14BuwFY8Mw5KPb2Kz7APFX - sp6JS7f14BuwFY8Mw5Xx4A6HRTPEE - sp6JS7f14BuwFY8Mw5y6qZFNAo358 - sp6JS7f14BuwFY8Mw6kdaBg1QrZfn - sp6JS7f14BuwFY8Mw8QmTfLMAZ5K1 - sp6JS7f14BuwFY8Mw8cbRRVcCEELr - sp6JS7f14BuwFY8Mw8gQvJebmxvDG - sp6JS7f14BuwFY8Mw8qPQurwu3P7Y - sp6JS7f14BuwFY8MwFS4PEVKmuPy5 - sp6JS7f14BuwFY8MwFUQM1rAsQ8tS - sp6JS7f14BuwFY8MwjJBZCkuwsRnM - sp6JS7f14BuwFY8MwjTdS8vZhX5E9 - sp6JS7f14BuwFY8MwjhSmWCbNhd25 - sp6JS7f14BuwFY8MwjwkpqwZsDBw9 - sp6JS7f14BuwFY8MwjyET4p6eqd5J - sp6JS7f14BuwFY8MwkMNAe4JhnG7E - sp6JS7f14BuwFY8MwkRRpnT93UWWS - sp6JS7f14BuwFY8MwkY9CvB22RvUe - sp6JS7f14BuwFY8Mwkhw9VxXqmTr7 - sp6JS7f14BuwFY8MwkmgaTat7eFa7 - sp6JS7f14BuwFY8Mwkq5SxGGv1oLH - sp6JS7f14BuwFY8MwmCBM5p5bTg6y - sp6JS7f14BuwFY8MwmmmXaVah64dB - sp6JS7f14BuwFY8Mwo7R7Cn614v9V - sp6JS7f14BuwFY8MwoCAG1na7GR2M - sp6JS7f14BuwFY8MwoDuPvJS4gG7C - sp6JS7f14BuwFY8MwoMMowSyPQLfy - sp6JS7f14BuwFY8MwoRqDiwTNsTBm - sp6JS7f14BuwFY8MwoWbBWtjpB7pg - sp6JS7f14BuwFY8Mwoi1AEeELGecF - sp6JS7f14BuwFY8MwopGP6Lo5byuj - sp6JS7f14BuwFY8MwoufkXGHp2VW8 - sp6JS7f14BuwFY8MwowGeagFQY32k - -34 account seeds that produce account IDs with low 32-bits 0x4d59f0d1: - sp6JS7f14BuwFY8Mw5CsNgH64zxK7 - sp6JS7f14BuwFY8Mw5Dg4wi2E344h - sp6JS7f14BuwFY8Mw5ErV949Zh2PX - sp6JS7f14BuwFY8Mw5p4nsQvEUE1s - sp6JS7f14BuwFY8Mw8LGnkbaP68Gn - sp6JS7f14BuwFY8Mw8aq6RCBc3iHo - sp6JS7f14BuwFY8Mw8bkWaGoKYT6e - sp6JS7f14BuwFY8Mw8qrCuXnzAXVj - sp6JS7f14BuwFY8MwFDKcPAHPHJTm - sp6JS7f14BuwFY8MwFUXJs4unfgNu - sp6JS7f14BuwFY8MwFj9Yv5LjshD9 - sp6JS7f14BuwFY8Mwj3H73nmq5UaC - sp6JS7f14BuwFY8MwjHSYShis1Yhk - sp6JS7f14BuwFY8MwjpfE1HVo8UP1 - sp6JS7f14BuwFY8Mwk6JE1SXUuiNc - sp6JS7f14BuwFY8MwkASgxEjEnFmU - sp6JS7f14BuwFY8MwkGNY8kg7R6RK - sp6JS7f14BuwFY8MwkHinNZ8SYBQu - sp6JS7f14BuwFY8MwkXLCW1hbhGya - sp6JS7f14BuwFY8MwkZ7mWrYK9YtU - sp6JS7f14BuwFY8MwkdFSqNB5DbKL - sp6JS7f14BuwFY8Mwm3jdBaCAx8H6 - sp6JS7f14BuwFY8Mwm3rk5hEwDRtY - sp6JS7f14BuwFY8Mwm77a2ULuwxu4 - sp6JS7f14BuwFY8MwmJpY7braKLaN - sp6JS7f14BuwFY8MwmKHQjG4XiZ6g - sp6JS7f14BuwFY8Mwmmv8Y3wyUDzs - sp6JS7f14BuwFY8MwmucFe1WgqtwG - sp6JS7f14BuwFY8Mwo1EjdU1bznZR - sp6JS7f14BuwFY8MwoJiqankkU5uR - sp6JS7f14BuwFY8MwoLnvQ6zdqbKw - sp6JS7f14BuwFY8MwoUGeJ319eu48 - sp6JS7f14BuwFY8MwoYf135tQjHP4 - sp6JS7f14BuwFY8MwogeF6M6SAyid - -34 account seeds that produce account IDs with low 32-bits 0xabb11898: - sp6JS7f14BuwFY8Mw5DgiYaNVSb1G - sp6JS7f14BuwFY8Mw5k6e94TMvuox - sp6JS7f14BuwFY8Mw5tTSN7KzYxiT - sp6JS7f14BuwFY8Mw61XV6m33utif - sp6JS7f14BuwFY8Mw87jKfrjiENCb - sp6JS7f14BuwFY8Mw8AFtxxFiRtJG - sp6JS7f14BuwFY8Mw8cosAVExzbeE - sp6JS7f14BuwFY8Mw8fmkQ63zE8WQ - sp6JS7f14BuwFY8Mw8iYSsxNbDN6D - sp6JS7f14BuwFY8Mw8wTZdGRJyyM1 - sp6JS7f14BuwFY8Mw8z7xEh3qBGr7 - sp6JS7f14BuwFY8MwFL5gpKQWZj7g - sp6JS7f14BuwFY8MwFPeZchXQnRZ5 - sp6JS7f14BuwFY8MwFSPxWSJVoU29 - sp6JS7f14BuwFY8MwFYyVkqX8kvRm - sp6JS7f14BuwFY8MwFcbVikUEwJvk - sp6JS7f14BuwFY8MwjF7NcZk1NctK - sp6JS7f14BuwFY8MwjJCwYr9zSfAv - sp6JS7f14BuwFY8MwjYa5yLkgCLuT - sp6JS7f14BuwFY8MwjenxuJ3TH2Bc - sp6JS7f14BuwFY8MwjriN7Ui11NzB - sp6JS7f14BuwFY8Mwk3AuoJNSEo34 - sp6JS7f14BuwFY8MwkT36hnRv8hTo - sp6JS7f14BuwFY8MwkTQixEXfi1Cr - sp6JS7f14BuwFY8MwkYJaZM1yTJBF - sp6JS7f14BuwFY8Mwkc4k1uo85qp2 - sp6JS7f14BuwFY8Mwkf7cFhF1uuxx - sp6JS7f14BuwFY8MwmCK2un99wb4e - sp6JS7f14BuwFY8MwmETztNHYu2Bx - sp6JS7f14BuwFY8MwmJws9UwRASfR - sp6JS7f14BuwFY8MwoH5PQkGK8tEb - sp6JS7f14BuwFY8MwoVXtP2yCzjJV - sp6JS7f14BuwFY8MwobxRXA9vsTeX - sp6JS7f14BuwFY8Mwos3pc5Gb3ihU - -34 account seeds that produce account IDs with low 32-bits 0xce627322: - sp6JS7f14BuwFY8Mw5Ck6i83pGNh3 - sp6JS7f14BuwFY8Mw5FKuwTxjAdH1 - sp6JS7f14BuwFY8Mw5FVKkEn6TkLH - sp6JS7f14BuwFY8Mw5NbQwLwHDd5v - sp6JS7f14BuwFY8Mw5X1dbz3msZaZ - sp6JS7f14BuwFY8Mw6qv6qaXNeP74 - sp6JS7f14BuwFY8Mw81SXagUeutCw - sp6JS7f14BuwFY8Mw84Ph7Qa8kwwk - sp6JS7f14BuwFY8Mw8Hp4gFyU3Qko - sp6JS7f14BuwFY8Mw8Kt8bAKredSx - sp6JS7f14BuwFY8Mw8XHK3VKRQ7v7 - sp6JS7f14BuwFY8Mw8eGyWxZGHY6v - sp6JS7f14BuwFY8Mw8iU5CLyHVcD2 - sp6JS7f14BuwFY8Mw8u3Zr26Ar914 - sp6JS7f14BuwFY8MwF2Kcdxtjzjv8 - sp6JS7f14BuwFY8MwFLmPWb6rbxNg - sp6JS7f14BuwFY8MwFUu8s7UVuxuJ - sp6JS7f14BuwFY8MwFYBaatwHxAJ8 - sp6JS7f14BuwFY8Mwjg6hFkeHwoqG - sp6JS7f14BuwFY8MwjjycJojy2ufk - sp6JS7f14BuwFY8MwkEWoxcSKGPXv - sp6JS7f14BuwFY8MwkMe7wLkEUsQT - sp6JS7f14BuwFY8MwkvyKLaPUc4FS - sp6JS7f14BuwFY8Mwm8doqXPKZmVQ - sp6JS7f14BuwFY8Mwm9r3No8yQ8Tx - sp6JS7f14BuwFY8Mwm9w6dks68W9B - sp6JS7f14BuwFY8MwmMPrv9sCdbpS - sp6JS7f14BuwFY8MwmPAvs3fcQNja - sp6JS7f14BuwFY8MwmS5jasapfcnJ - sp6JS7f14BuwFY8MwmU2L3qJEhnuA - sp6JS7f14BuwFY8MwoAQYmiBnW7fM - sp6JS7f14BuwFY8MwoBkkkXrPmkKF - sp6JS7f14BuwFY8MwonfmxPo6tkvC - sp6JS7f14BuwFY8MwouZFwhiNcYq6 - -34 account seeds that produce account IDs with low 32-bits 0xe29643e8: - sp6JS7f14BuwFY8Mw5EfAavcXAh2k - sp6JS7f14BuwFY8Mw5LhFjLkFSCVF - sp6JS7f14BuwFY8Mw5bRfEv5HgdBh - sp6JS7f14BuwFY8Mw5d6sPcKzypKN - sp6JS7f14BuwFY8Mw5rcqDtk1fACP - sp6JS7f14BuwFY8Mw5xkxRq1Notzv - sp6JS7f14BuwFY8Mw66fbkdw5WYmt - sp6JS7f14BuwFY8Mw6diEG8sZ7Fx7 - sp6JS7f14BuwFY8Mw6v2r1QhG7xc1 - sp6JS7f14BuwFY8Mw6zP6DHCTx2Fd - sp6JS7f14BuwFY8Mw8B3n39JKuFkk - sp6JS7f14BuwFY8Mw8FmBvqYw7uqn - sp6JS7f14BuwFY8Mw8KEaftb1eRwu - sp6JS7f14BuwFY8Mw8WJ1qKkegj9N - sp6JS7f14BuwFY8Mw8r8cAZEkq2BS - sp6JS7f14BuwFY8MwFKPxxwF65gZh - sp6JS7f14BuwFY8MwFKhaF8APcN5H - sp6JS7f14BuwFY8MwFN2buJn4BgYC - sp6JS7f14BuwFY8MwFUTe175MjP3x - sp6JS7f14BuwFY8MwFZhmRDb53NNb - sp6JS7f14BuwFY8MwFa2Azn5nU2WS - sp6JS7f14BuwFY8MwjNNt91hwgkn7 - sp6JS7f14BuwFY8MwjdiYt6ChACe7 - sp6JS7f14BuwFY8Mwk5qFVQ48Mmr9 - sp6JS7f14BuwFY8MwkGvCj7pNf1zG - sp6JS7f14BuwFY8MwkY9UcN2D2Fzs - sp6JS7f14BuwFY8MwkpGvSk9G9RyT - sp6JS7f14BuwFY8MwmGQ7nJf1eEzV - sp6JS7f14BuwFY8MwmQLjGsYdyAmV - sp6JS7f14BuwFY8MwmZ8usztKvikT - sp6JS7f14BuwFY8MwobyMLC2hQdFR - sp6JS7f14BuwFY8MwoiRtwUecZeJ5 - sp6JS7f14BuwFY8MwojHjKsUzj1KJ - sp6JS7f14BuwFY8Mwop29anGAjidU - -33 account seeds that produce account IDs with low 32-bits 0x115d0525: - sp6JS7f14BuwFY8Mw56vZeiBuhePx - sp6JS7f14BuwFY8Mw5BodF9tGuTUe - sp6JS7f14BuwFY8Mw5EnhC1cg84J7 - sp6JS7f14BuwFY8Mw5P913Cunr2BK - sp6JS7f14BuwFY8Mw5Pru7eLo1XzT - sp6JS7f14BuwFY8Mw61SLUC8UX2m8 - sp6JS7f14BuwFY8Mw6AsBF9TpeMpq - sp6JS7f14BuwFY8Mw84XqrBZkU2vE - sp6JS7f14BuwFY8Mw89oSU6dBk3KB - sp6JS7f14BuwFY8Mw89qUKCyDmyzj - sp6JS7f14BuwFY8Mw8GfqQ9VRZ8tm - sp6JS7f14BuwFY8Mw8LtW3VqrqMks - sp6JS7f14BuwFY8Mw8ZrAkJc2sHew - sp6JS7f14BuwFY8Mw8jpkYSNrD3ah - sp6JS7f14BuwFY8MwF2mshd786m3V - sp6JS7f14BuwFY8MwFHfXq9x5NbPY - sp6JS7f14BuwFY8MwFrjWq5LAB8NT - sp6JS7f14BuwFY8Mwj4asgSh6hQZd - sp6JS7f14BuwFY8Mwj7ipFfqBSRrE - sp6JS7f14BuwFY8MwjHqtcvGav8uW - sp6JS7f14BuwFY8MwjLp4sk5fmzki - sp6JS7f14BuwFY8MwjioHuYb3Ytkx - sp6JS7f14BuwFY8MwkRjHPXWi7fGN - sp6JS7f14BuwFY8MwkdVdPV3LjNN1 - sp6JS7f14BuwFY8MwkxUtVY5AXZFk - sp6JS7f14BuwFY8Mwm4jQzdfTbY9F - sp6JS7f14BuwFY8MwmCucYAqNp4iF - sp6JS7f14BuwFY8Mwo2bgdFtxBzpF - sp6JS7f14BuwFY8MwoGwD7v4U6qBh - sp6JS7f14BuwFY8MwoUczqFADMoXi - sp6JS7f14BuwFY8MwoY1xZeGd3gAr - sp6JS7f14BuwFY8MwomVCbfkv4kYZ - sp6JS7f14BuwFY8MwoqbrPSr4z13F - -33 account seeds that produce account IDs with low 32-bits 0x304033aa: - sp6JS7f14BuwFY8Mw5DaUP9agF5e1 - sp6JS7f14BuwFY8Mw5ohbtmPN4yGN - sp6JS7f14BuwFY8Mw5rRsA5fcoTAQ - sp6JS7f14BuwFY8Mw6zpYHMY3m6KT - sp6JS7f14BuwFY8Mw86BzQq4sTnoW - sp6JS7f14BuwFY8Mw8CCpnfvmGdV7 - sp6JS7f14BuwFY8Mw8DRjUDaBcFco - sp6JS7f14BuwFY8Mw8cL7GPo3zZN7 - sp6JS7f14BuwFY8Mw8y6aeYVtH6qt - sp6JS7f14BuwFY8MwFZR3PtVTCdUH - sp6JS7f14BuwFY8MwFcdcdbgz7m3s - sp6JS7f14BuwFY8MwjdnJDiUxEBRR - sp6JS7f14BuwFY8MwjhxWgSntqrFe - sp6JS7f14BuwFY8MwjrSHEhZ8CUM1 - sp6JS7f14BuwFY8MwjzkEeSTc9ZYf - sp6JS7f14BuwFY8MwkBZSk9JhaeCB - sp6JS7f14BuwFY8MwkGfwNY4i2iiU - sp6JS7f14BuwFY8MwknjtZd2oU2Ff - sp6JS7f14BuwFY8Mwkszsqd3ok9NE - sp6JS7f14BuwFY8Mwm58A81MAMvgZ - sp6JS7f14BuwFY8MwmiPTWysuDJCH - sp6JS7f14BuwFY8MwmxhiNeLfD76r - sp6JS7f14BuwFY8Mwo7SPdkwpGrFH - sp6JS7f14BuwFY8MwoANq4F1Sj3qH - sp6JS7f14BuwFY8MwoVjcHufAkd6L - sp6JS7f14BuwFY8MwoVxHBXdaxzhm - sp6JS7f14BuwFY8MwoZ2oTjBNfLpm - sp6JS7f14BuwFY8Mwoc9swzyotFVD - sp6JS7f14BuwFY8MwogMqVRwVEcQ9 - sp6JS7f14BuwFY8MwohMm7WxwnFqH - sp6JS7f14BuwFY8MwopUcpZHuF8BH - sp6JS7f14BuwFY8Mwor6rW6SS7tiB - sp6JS7f14BuwFY8MwoxyaqYz4Ngsb - -33 account seeds that produce account IDs with low 32-bits 0x42d4e09c: - sp6JS7f14BuwFY8Mw58NSZH9EaUxQ - sp6JS7f14BuwFY8Mw5JByk1pgPpL7 - sp6JS7f14BuwFY8Mw5YrJJuXnkHVB - sp6JS7f14BuwFY8Mw5kZe2ZzNSnKR - sp6JS7f14BuwFY8Mw6eXHTsbwi1U7 - sp6JS7f14BuwFY8Mw6gqN7HHDDKSh - sp6JS7f14BuwFY8Mw6zw8L1sSSR53 - sp6JS7f14BuwFY8Mw8E4WqSKKbksy - sp6JS7f14BuwFY8MwF3V9gemqJtND - sp6JS7f14BuwFY8Mwj4j46LHWZuY6 - sp6JS7f14BuwFY8MwjF5i8vh4Ezjy - sp6JS7f14BuwFY8MwjJZpEKgMpUAt - sp6JS7f14BuwFY8MwjWL7LfnzNUuh - sp6JS7f14BuwFY8Mwk7Y1csGuqAhX - sp6JS7f14BuwFY8MwkB1HVH17hN5W - sp6JS7f14BuwFY8MwkBntH7BZZupu - sp6JS7f14BuwFY8MwkEy4rMbNHG9P - sp6JS7f14BuwFY8MwkKz4LYesZeiN - sp6JS7f14BuwFY8MwkUrXyo9gMDPM - sp6JS7f14BuwFY8MwkV2hySsxej1G - sp6JS7f14BuwFY8MwkozhTVN12F9C - sp6JS7f14BuwFY8MwkpkzGB3sFJw5 - sp6JS7f14BuwFY8Mwks3zDZLGrhdn - sp6JS7f14BuwFY8MwktG1KCS7L2wW - sp6JS7f14BuwFY8Mwm1jVFsafwcYx - sp6JS7f14BuwFY8Mwm8hmrU6g5Wd6 - sp6JS7f14BuwFY8MwmFvstfRF7e2f - sp6JS7f14BuwFY8MwmeRohi6m5fs8 - sp6JS7f14BuwFY8MwmmU96RHUaRZL - sp6JS7f14BuwFY8MwoDFzteYqaUh4 - sp6JS7f14BuwFY8MwoPkTf5tDykPF - sp6JS7f14BuwFY8MwoSbMaDtiMoDN - sp6JS7f14BuwFY8MwoVL1vY1CysjR - -33 account seeds that produce account IDs with low 32-bits 0x9a8ebed3: - sp6JS7f14BuwFY8Mw5FnqmbciPvH6 - sp6JS7f14BuwFY8Mw5MBGbyMSsXLp - sp6JS7f14BuwFY8Mw5S4PnDyBdKKm - sp6JS7f14BuwFY8Mw6kcXpM2enE35 - sp6JS7f14BuwFY8Mw6tuuSMMwyJ44 - sp6JS7f14BuwFY8Mw8E8JWLQ1P8pt - sp6JS7f14BuwFY8Mw8WwdgWkCHhEx - sp6JS7f14BuwFY8Mw8XDUYvU6oGhQ - sp6JS7f14BuwFY8Mw8ceVGL4M1zLQ - sp6JS7f14BuwFY8Mw8fdSwLCZWDFd - sp6JS7f14BuwFY8Mw8zuF6Fg65i1E - sp6JS7f14BuwFY8MwF2k7bihVfqes - sp6JS7f14BuwFY8MwF6X24WXGn557 - sp6JS7f14BuwFY8MwFMpn7strjekg - sp6JS7f14BuwFY8MwFSdy9sYVrwJs - sp6JS7f14BuwFY8MwFdMcLy9UkrXn - sp6JS7f14BuwFY8MwFdbwFm1AAboa - sp6JS7f14BuwFY8MwFdr5AhKThVtU - sp6JS7f14BuwFY8MwjFc3Q9YatvAw - sp6JS7f14BuwFY8MwjRXcNs1ozEXn - sp6JS7f14BuwFY8MwkQGUKL7v1FBt - sp6JS7f14BuwFY8Mwkamsoxx1wECt - sp6JS7f14BuwFY8Mwm3hus1dG6U8y - sp6JS7f14BuwFY8Mwm589M8vMRpXF - sp6JS7f14BuwFY8MwmJTRJ4Fqz1A3 - sp6JS7f14BuwFY8MwmRfy8fer4QbL - sp6JS7f14BuwFY8MwmkkFx1HtgWRx - sp6JS7f14BuwFY8MwmwP9JFdKa4PS - sp6JS7f14BuwFY8MwoXWJLB3ciHfo - sp6JS7f14BuwFY8MwoYc1gTtT2mWL - sp6JS7f14BuwFY8MwogXtHH7FNVoo - sp6JS7f14BuwFY8MwoqYoA9P8gf3r - sp6JS7f14BuwFY8MwoujwMJofGnsA - -33 account seeds that produce account IDs with low 32-bits 0xa1dcea4a: - sp6JS7f14BuwFY8Mw5Ccov2N36QTy - sp6JS7f14BuwFY8Mw5CuSemVb5p7w - sp6JS7f14BuwFY8Mw5Ep8wpsTfpSz - sp6JS7f14BuwFY8Mw5WtutJc2H45M - sp6JS7f14BuwFY8Mw6vsDeaSKeUJZ - sp6JS7f14BuwFY8Mw83t5BPWUAzzF - sp6JS7f14BuwFY8Mw8FYGnK35mgkV - sp6JS7f14BuwFY8Mw8huo1x5pfKKJ - sp6JS7f14BuwFY8Mw8mPStxfMDrZa - sp6JS7f14BuwFY8Mw8yC3A7aQJytK - sp6JS7f14BuwFY8MwFCWCDmo9o3t8 - sp6JS7f14BuwFY8MwFjapa4gKxPhR - sp6JS7f14BuwFY8Mwj8CWtG29uw71 - sp6JS7f14BuwFY8MwjHyU5KpEMLVT - sp6JS7f14BuwFY8MwjMZSN7LZuWD8 - sp6JS7f14BuwFY8Mwja2TXJNBhKHU - sp6JS7f14BuwFY8Mwjf3xNTopHKTF - sp6JS7f14BuwFY8Mwjn5RAhedPeuM - sp6JS7f14BuwFY8MwkJdr4d6QoE8K - sp6JS7f14BuwFY8MwkmBryo3SUoLm - sp6JS7f14BuwFY8MwkrPdsc4tR8yw - sp6JS7f14BuwFY8Mwkttjcw2a65Fi - sp6JS7f14BuwFY8Mwm19n3rSaNx5S - sp6JS7f14BuwFY8Mwm3ryr4Xp2aQX - sp6JS7f14BuwFY8MwmBnDmgnJLB6B - sp6JS7f14BuwFY8MwmHgPjzrYjthq - sp6JS7f14BuwFY8MwmeV55DAnWKdd - sp6JS7f14BuwFY8Mwo49hK6BGrauT - sp6JS7f14BuwFY8Mwo56vfKY9aoWu - sp6JS7f14BuwFY8MwoU7tTTXLQTrh - sp6JS7f14BuwFY8MwoXpogSF2KaZB - sp6JS7f14BuwFY8MwoY9JYQAR16pc - sp6JS7f14BuwFY8MwoozLzKNAEXKM - -33 account seeds that produce account IDs with low 32-bits 0xbd2116db: - sp6JS7f14BuwFY8Mw5GrpkmPuA3Bw - sp6JS7f14BuwFY8Mw5r1sLoQJZDc6 - sp6JS7f14BuwFY8Mw68zzRmezLdd6 - sp6JS7f14BuwFY8Mw6jDSyaiF1mRp - sp6JS7f14BuwFY8Mw813wU9u5D6Uh - sp6JS7f14BuwFY8Mw8BBvpf2JFGoJ - sp6JS7f14BuwFY8Mw8F7zXxAiT263 - sp6JS7f14BuwFY8Mw8XG7WuVGHP2N - sp6JS7f14BuwFY8Mw8eyWrcz91cz6 - sp6JS7f14BuwFY8Mw8yNVKFVYyk9u - sp6JS7f14BuwFY8MwF2oA6ePqvZWP - sp6JS7f14BuwFY8MwF9VkcSNh3keq - sp6JS7f14BuwFY8MwFYsMWajgEf2j - sp6JS7f14BuwFY8Mwj3Gu43jYoJ4n - sp6JS7f14BuwFY8MwjJ5iRmYDHrW4 - sp6JS7f14BuwFY8MwjaUSSga93CiM - sp6JS7f14BuwFY8MwjxgLh2FY4Lvt - sp6JS7f14BuwFY8Mwk9hQdNZUgmTB - sp6JS7f14BuwFY8MwkcMXqtFp1sMx - sp6JS7f14BuwFY8MwkzZCDc56jsUB - sp6JS7f14BuwFY8Mwm5Zz7fP24Qym - sp6JS7f14BuwFY8MwmDWqizXSoJRG - sp6JS7f14BuwFY8MwmKHmkNYdMqqi - sp6JS7f14BuwFY8MwmRfAWHxWpGNK - sp6JS7f14BuwFY8MwmjCdXwyhphZ1 - sp6JS7f14BuwFY8MwmmukDAm1w6FL - sp6JS7f14BuwFY8Mwmmz2SzaR9TRH - sp6JS7f14BuwFY8Mwmz2z5mKHXzfn - sp6JS7f14BuwFY8Mwo2xNe5629r5k - sp6JS7f14BuwFY8MwoKy8tZxZrfJw - sp6JS7f14BuwFY8MwoLyQ9aMsq8Dm - sp6JS7f14BuwFY8MwoqqYkewuyZck - sp6JS7f14BuwFY8MwouvvhREVp6Pp - -33 account seeds that produce account IDs with low 32-bits 0xd80df065: - sp6JS7f14BuwFY8Mw5B7ERyhAfgHA - sp6JS7f14BuwFY8Mw5VuW3cF7bm2v - sp6JS7f14BuwFY8Mw5py3t1j7YbFT - sp6JS7f14BuwFY8Mw5qc84SzB6RHr - sp6JS7f14BuwFY8Mw5vGHW1G1hAy8 - sp6JS7f14BuwFY8Mw6gVa8TYukws6 - sp6JS7f14BuwFY8Mw8K9w1RoUAv1w - sp6JS7f14BuwFY8Mw8KvKtB7787CA - sp6JS7f14BuwFY8Mw8Y7WhRbuFzRq - sp6JS7f14BuwFY8Mw8cipw7inRmMn - sp6JS7f14BuwFY8MwFM5fAUNLNB13 - sp6JS7f14BuwFY8MwFSe1zAsht3X3 - sp6JS7f14BuwFY8MwFYNdigqQuHZM - sp6JS7f14BuwFY8MwjWkejj7V4V5Q - sp6JS7f14BuwFY8Mwjd2JGpsjvynq - sp6JS7f14BuwFY8Mwjg1xkducn751 - sp6JS7f14BuwFY8Mwjsp6LnaJvL1W - sp6JS7f14BuwFY8MwjvSbLc9593yH - sp6JS7f14BuwFY8Mwjw2h5wx7U6vZ - sp6JS7f14BuwFY8MwjxKUjtRsmPLH - sp6JS7f14BuwFY8Mwk1Yy8ginDfqv - sp6JS7f14BuwFY8Mwk2HrWhWwZP12 - sp6JS7f14BuwFY8Mwk4SsqiexvpWs - sp6JS7f14BuwFY8Mwk66zCs5ACpE6 - sp6JS7f14BuwFY8MwkCwx6vY97Nwh - sp6JS7f14BuwFY8MwknrbjnhTTWU8 - sp6JS7f14BuwFY8MwkokDy2ShRzQx - sp6JS7f14BuwFY8Mwm3BxnRPNxsuu - sp6JS7f14BuwFY8MwmY9EWdQQsFVr - sp6JS7f14BuwFY8MwmYTWjrDhmk8S - sp6JS7f14BuwFY8Mwo9skXt9Y5BVS - sp6JS7f14BuwFY8MwoZYKZybJ1Crp - sp6JS7f14BuwFY8MwoyXqkhySfSmF - -33 account seeds that produce account IDs with low 32-bits 0xe2e44294: - sp6JS7f14BuwFY8Mw53dmvTgNtBwi - sp6JS7f14BuwFY8Mw5Wrxsqn6WrXW - sp6JS7f14BuwFY8Mw5fGDT31RCXgC - sp6JS7f14BuwFY8Mw5nKRkubwrLWM - sp6JS7f14BuwFY8Mw5nXMajwKjriB - sp6JS7f14BuwFY8Mw5xZybggrC9NG - sp6JS7f14BuwFY8Mw5xea8f6dBMV5 - sp6JS7f14BuwFY8Mw5zDGofAHy5Lb - sp6JS7f14BuwFY8Mw6eado41rQNVG - sp6JS7f14BuwFY8Mw6yqKXQsQJPuU - sp6JS7f14BuwFY8Mw83MSN4FDzSGH - sp6JS7f14BuwFY8Mw8B3pUbzQqHe2 - sp6JS7f14BuwFY8Mw8WwRLnhBRvfk - sp6JS7f14BuwFY8Mw8hDBpKbpJwJX - sp6JS7f14BuwFY8Mw8jggRSZACe7M - sp6JS7f14BuwFY8Mw8mJRpU3qWbwC - sp6JS7f14BuwFY8MwFDnVozykN21u - sp6JS7f14BuwFY8MwFGGRGY9fctgv - sp6JS7f14BuwFY8MwjKznfChH9DQb - sp6JS7f14BuwFY8MwjbC5GvngRCk6 - sp6JS7f14BuwFY8Mwk3Lb7FPe1629 - sp6JS7f14BuwFY8MwkCeS41BwVrBD - sp6JS7f14BuwFY8MwkDnnvRyuWJ7d - sp6JS7f14BuwFY8MwkbkRNnzDEFpf - sp6JS7f14BuwFY8MwkiNhaVhGNk6v - sp6JS7f14BuwFY8Mwm1X4UJXRZx3p - sp6JS7f14BuwFY8Mwm7da9q5vfq7J - sp6JS7f14BuwFY8MwmPLqfBPrHw5H - sp6JS7f14BuwFY8MwmbJpxvVjEwm2 - sp6JS7f14BuwFY8MwoAVeA7ka37cD - sp6JS7f14BuwFY8MwoTFFTAwFKmVM - sp6JS7f14BuwFY8MwoYsne51VpDE3 - sp6JS7f14BuwFY8MwohLVnU1VTk5h - -#endif // 0 diff --git a/src/test/app/PayStrand_test.cpp b/src/test/app/PayStrand_test.cpp index 967f84f275..2a4b884668 100644 --- a/src/test/app/PayStrand_test.cpp +++ b/src/test/app/PayStrand_test.cpp @@ -37,6 +37,7 @@ #include #include +#include #include #include #include @@ -126,12 +127,7 @@ class ElementComboIter [[nodiscard]] bool hasAny(std::initializer_list sb) const { - for (auto const s : sb) - { - if (has(s)) - return true; - } - return false; + return std::ranges::any_of(sb, [this](auto const s) { return has(s); }); } [[nodiscard]] size_t diff --git a/src/test/app/PermissionedDEX_test.cpp b/src/test/app/PermissionedDEX_test.cpp index 8578b8a9ba..998b7b1c7f 100644 --- a/src/test/app/PermissionedDEX_test.cpp +++ b/src/test/app/PermissionedDEX_test.cpp @@ -39,6 +39,7 @@ #include #include +#include #include #include #include @@ -82,13 +83,9 @@ class PermissionedDEX_test : public beast::unit_test::Suite return false; auto const& indexes = page->getFieldV256(sfIndexes); - for (auto const& index : indexes) - { - if (index == keylet::offer(account, offerSeq).key) - return true; - } - - return false; + return std::ranges::any_of(indexes, [&](auto const& index) { + return index == keylet::offer(account, offerSeq).key; + }); }; auto const sle = env.le(keylet::offer(account.id(), offerSeq)); diff --git a/src/test/app/ReducedOffer_test.cpp b/src/test/app/ReducedOffer_test.cpp index 47a2c1294c..dea197590c 100644 --- a/src/test/app/ReducedOffer_test.cpp +++ b/src/test/app/ReducedOffer_test.cpp @@ -144,20 +144,6 @@ public: Quality(Amounts{tweakedTakerPays, reducedTakerGets}).rate(); BEAST_EXPECT(tweakedRate > initialRate); } -#if 0 - std::cout << "Placed rate: " << initialRate - << "; in-ledger rate: " << inLedgerRate - << "; TakerPays: " << reducedTakerPays - << "; TakerGets: " << reducedTakerGets - << "; bob already got: " << bobGot << std::endl; -// #else - std::string_view filler = - inLedgerRate > initialRate ? "**" : " "; - std::cout << "| `" << reducedTakerGets << "` | `" - << reducedTakerPays << "` | `" << initialRate - << "` | " << filler << "`" << inLedgerRate << "`" - << filler << " |`" << std::endl; -#endif } // In preparation for the next iteration make sure the two @@ -275,21 +261,6 @@ public: Quality(Amounts{tweakedTakerPays, reducedTakerGets}).rate(); BEAST_EXPECT(tweakedRate > initialRate); } -#if 0 - std::cout << "Placed rate: " << initialRate - << "; in-ledger rate: " << inLedgerRate - << "; TakerPays: " << reducedTakerPays - << "; TakerGets: " << reducedTakerGets - << "; alice already got: " << aliceGot - << std::endl; -// #else - std::string_view filler = badRate ? "**" : " "; - std::cout << "| `" << reducedTakerGets << "` | `" - << reducedTakerPays << "` | `" << initialRate - << "` | " << filler << "`" << inLedgerRate << "`" - << filler << " | `" << aliceGot << "` |" - << std::endl; -#endif } // In preparation for the next iteration make sure the two @@ -463,13 +434,6 @@ public: { bool const bobOfferGone = !offerInLedger(env, bob, bobOfferSeq); STAmount const aliceBalanceUSD = env.balance(alice, usd); -#if 0 - std::cout - << "bob initial: " << initialBobUSD - << "; alice final: " << aliceBalanceUSD - << "; bob offer: " << bobOfferJson.toStyledString() - << std::endl; -#endif // Sanity check the ledger if alice got USD. if (aliceBalanceUSD.signum() > 0) { @@ -619,19 +583,6 @@ public: Quality(Amounts{aliceReducedOffer.in, tweakedTakerGets}).rate(); BEAST_EXPECT(tweakedRate > initialRate); } -#if 0 - std::cout << "Placed rate: " << initialRate - << "; in-ledger rate: " << inLedgerRate - << "; TakerPays: " << aliceReducedOffer.in - << "; TakerGets: " << aliceReducedOffer.out - << std::endl; -// #else - std::string_view filler = badRate ? "**" : " "; - std::cout << "| " << aliceReducedOffer.in << "` | `" - << aliceReducedOffer.out << "` | `" << initialRate - << "` | " << filler << "`" << inLedgerRate << "`" - << filler << std::endl; -#endif } // In preparation for the next iteration make sure all three diff --git a/src/test/app/ValidatorList_test.cpp b/src/test/app/ValidatorList_test.cpp index 8077c21863..60228f6723 100644 --- a/src/test/app/ValidatorList_test.cpp +++ b/src/test/app/ValidatorList_test.cpp @@ -2596,7 +2596,7 @@ private: } void - testQuorumDisabled() + testQuorumDisabled() // NOLINT(readability-function-size) { testcase("Test quorum disabled"); diff --git a/src/test/app/XChain_test.cpp b/src/test/app/XChain_test.cpp index 4b007eea13..d1f9cd2722 100644 --- a/src/test/app/XChain_test.cpp +++ b/src/test/app/XChain_test.cpp @@ -3923,12 +3923,10 @@ private: [[nodiscard]] bool verify() const { - for (auto const& [acct, state] : accounts) - { - if (!state.verify(env, acct)) - return false; - } - return true; + return std::ranges::all_of(accounts, [&](auto const& entry) { + auto const& [acct, state] = entry; + return state.verify(env, acct); + }); } struct BridgeCounters diff --git a/src/test/consensus/Consensus_test.cpp b/src/test/consensus/Consensus_test.cpp index 92a4c67e32..45f58d16ba 100644 --- a/src/test/consensus/Consensus_test.cpp +++ b/src/test/consensus/Consensus_test.cpp @@ -1034,16 +1034,6 @@ public: // slow ledger is generated UndoDelay undoDelay{behind}; sim.collectors.add(undoDelay); - -#if 0 - // Have all beast::journal output printed to stdout - for (Peer* p : network) - p->sink.threshold(beast::Severity::All); - - // Print ledger accept and fully validated events to stdout - StreamCollector sc{std::cout}; - sim.collectors.add(sc); -#endif // Run the simulation for 100 seconds of simulation time with std::chrono::nanoseconds const simDuration = 100s; diff --git a/src/test/core/SociDB_test.cpp b/src/test/core/SociDB_test.cpp index d37569d6cd..373ec66cd1 100644 --- a/src/test/core/SociDB_test.cpp +++ b/src/test/core/SociDB_test.cpp @@ -229,63 +229,7 @@ public: fail(); } // There are too many issues when working with soci::row and - // boost::tuple. DO NOT USE soci row! I had a set of workarounds to - // make soci row less error prone, I'm keeping these tests in case I - // try to add soci::row and boost::tuple back into soci. -#if 0 - try - { - std::int32_t ig = 0; - std::uint32_t uig = 0; - std::int64_t big = 0; - std::uint64_t ubig = 0; - soci::row r; - s << "SELECT I, UI, BI, UBI from STT", soci::into (r); - ig = r.get(0); - uig = r.get(1); - big = r.get(2); - ubig = r.get(3); - BEAST_EXPECT(ig == id[0] && uig == uid[0] && big == bid[0] && - ubig == ubid[0]); - } - catch (std::exception&) - { - fail (); - } - try - { - std::int32_t ig = 0; - std::uint32_t uig = 0; - std::int64_t big = 0; - std::uint64_t ubig = 0; - soci::row r; - s << "SELECT I, UI, BI, UBI from STT", soci::into (r); - ig = r.get("I"); - uig = r.get("UI"); - big = r.get("BI"); - ubig = r.get("UBI"); - BEAST_EXPECT(ig == id[0] && uig == uid[0] && big == bid[0] && - ubig == ubid[0]); - } - catch (std::exception&) - { - fail (); - } - try - { - boost::tuple d; - s << "SELECT I, UI, BI, UBI from STT", soci::into (d); - BEAST_EXPECT(get<0>(d) == id[0] && get<1>(d) == uid[0] && - get<2>(d) == bid[0] && get<3>(d) == ubid[0]); - } - catch (std::exception&) - { - fail (); - } -#endif + // boost::tuple. DO NOT USE soci row! } { namespace bfs = boost::filesystem; diff --git a/src/test/csf/Peer.h b/src/test/csf/Peer.h index 53b26d05cf..79bffec9cb 100644 --- a/src/test/csf/Peer.h +++ b/src/test/csf/Peer.h @@ -413,12 +413,8 @@ struct Peer bool trusts(PeerID const& oId) { - for (auto const p : trustGraph.trustedPeers(this)) - { - if (p->id == oId) - return true; - } - return false; + return std::ranges::any_of( + trustGraph.trustedPeers(this), [&oId](auto const p) { return p->id == oId; }); } /** diff --git a/src/test/jtx/TestHelpers.h b/src/test/jtx/TestHelpers.h index c4991c4ff3..e7a2808f07 100644 --- a/src/test/jtx/TestHelpers.h +++ b/src/test/jtx/TestHelpers.h @@ -464,12 +464,8 @@ same(STPathSet const& st1, Args const&... args) if (st1.size() != st2.size()) return false; - for (auto const& p : st2) - { - if (std::ranges::find(st1, p) == st1.end()) - return false; - } - return true; + return std::ranges::all_of( + st2, [&st1](auto const& p) { return std::ranges::find(st1, p) != st1.end(); }); } json::Value diff --git a/src/test/jtx/impl/AMM.cpp b/src/test/jtx/impl/AMM.cpp index 5c815d4226..8effce288e 100644 --- a/src/test/jtx/impl/AMM.cpp +++ b/src/test/jtx/impl/AMM.cpp @@ -326,13 +326,10 @@ AMM::expectAuctionSlot(std::vector const& authAccounts) const { return expectAuctionSlot( [&](std::uint32_t, std::optional, IOUAmount const&, STArray const& accounts) { - for (auto const& account : accounts) - { - if (std::ranges::find(authAccounts, account.getAccountID(sfAccount)) == - authAccounts.end()) - return false; - } - return true; + return std::ranges::all_of(accounts, [&](auto const& account) { + return std::ranges::find(authAccounts, account.getAccountID(sfAccount)) != + authAccounts.end(); + }); }); } diff --git a/src/test/nodestore/Timing_test.cpp b/src/test/nodestore/Timing_test.cpp index 1c296ca44c..1f282d9d7f 100644 --- a/src/test/nodestore/Timing_test.cpp +++ b/src/test/nodestore/Timing_test.cpp @@ -692,9 +692,6 @@ public: #if XRPL_ROCKSDB_AVAILABLE ";type=rocksdb,open_files=2000,filter_bits=12,cache_mb=256," "file_size_mb=8,file_size_mult=2" -#endif -#if 0 - ";type=memory|path=NodeStore" #endif ; diff --git a/src/test/overlay/cluster_test.cpp b/src/test/overlay/cluster_test.cpp index 6c2114b7de..0a51f98594 100644 --- a/src/test/overlay/cluster_test.cpp +++ b/src/test/overlay/cluster_test.cpp @@ -150,7 +150,7 @@ public: { auto member = c->member(node); BEAST_EXPECT(static_cast(member)); - BEAST_EXPECT(member->compare(name) == 0); // NOLINT(bugprone-unchecked-optional-access) + BEAST_EXPECT(*member == name); // NOLINT(bugprone-unchecked-optional-access) } // Updating the name (non-empty doesn't go to empty) @@ -159,7 +159,7 @@ public: { auto member = c->member(node); BEAST_EXPECT(static_cast(member)); - BEAST_EXPECT(member->compare(name) == 0); // NOLINT(bugprone-unchecked-optional-access) + BEAST_EXPECT(*member == name); // NOLINT(bugprone-unchecked-optional-access) } // Updating the name (non-empty updates to new non-empty) @@ -168,8 +168,7 @@ public: { auto member = c->member(node); BEAST_EXPECT(static_cast(member)); - BEAST_EXPECT( - member->compare("test") == 0); // NOLINT(bugprone-unchecked-optional-access) + BEAST_EXPECT(*member == "test"); // NOLINT(bugprone-unchecked-optional-access) } } diff --git a/src/test/overlay/reduce_relay_test.cpp b/src/test/overlay/reduce_relay_test.cpp index c55729a45a..2f42313037 100644 --- a/src/test/overlay/reduce_relay_test.cpp +++ b/src/test/overlay/reduce_relay_test.cpp @@ -858,12 +858,8 @@ public: bool isSelected(Peer::id_t id) { - for (auto& v : validators_) - { - if (overlay_.isSelected(v, id)) - return true; - } - return false; + return std::ranges::any_of( + validators_, [&](auto& v) { return overlay_.isSelected(v, id); }); } /** diff --git a/src/test/protocol/STAmount_test.cpp b/src/test/protocol/STAmount_test.cpp index 720fafa8f2..f6c5a94752 100644 --- a/src/test/protocol/STAmount_test.cpp +++ b/src/test/protocol/STAmount_test.cpp @@ -537,51 +537,6 @@ public: { // VFALCO TODO There are no actual tests here, just printed output? // Change this to actually do something. - -#if 0 - beginTestCase ("rounding "); - - std::uint64_t value = 25000000000000000ull; - int offset = -14; - canonicalizeRound (false, value, offset, true); - - STAmount one (noIssue(), 1); - STAmount two (noIssue(), 2); - STAmount three (noIssue(), 3); - - STAmount oneThird1 = divRound (one, three, noIssue(), false); - STAmount oneThird2 = divide (one, three, noIssue()); - STAmount oneThird3 = divRound (one, three, noIssue(), true); - log << oneThird1; - log << oneThird2; - log << oneThird3; - - STAmount twoThird1 = divRound (two, three, noIssue(), false); - STAmount twoThird2 = divide (two, three, noIssue()); - STAmount twoThird3 = divRound (two, three, noIssue(), true); - log << twoThird1; - log << twoThird2; - log << twoThird3; - - STAmount oneA = mulRound (oneThird1, three, noIssue(), false); - STAmount oneB = multiply (oneThird2, three, noIssue()); - STAmount oneC = mulRound (oneThird3, three, noIssue(), true); - log << oneA; - log << oneB; - log << oneC; - - STAmount fourThirdsB = twoThird2 + twoThird2; - log << fourThirdsA; - log << fourThirdsB; - log << fourThirdsC; - - STAmount dripTest1 = mulRound (twoThird2, two, xrpIssue (), false); - STAmount dripTest2 = multiply (twoThird2, two, xrpIssue ()); - STAmount dripTest3 = mulRound (twoThird2, two, xrpIssue (), true); - log << dripTest1; - log << dripTest2; - log << dripTest3; -#endif } void diff --git a/src/test/rpc/ServerDefinitions_test.cpp b/src/test/rpc/ServerDefinitions_test.cpp index bf345e6fdf..0cf6b315e0 100644 --- a/src/test/rpc/ServerDefinitions_test.cpp +++ b/src/test/rpc/ServerDefinitions_test.cpp @@ -8,6 +8,7 @@ #include #include +#include #include #include @@ -61,14 +62,12 @@ public: // check exception SFields { auto const fieldExists = [&](std::string name) { - for (auto& field : result[jss::result][jss::FIELDS]) - { - if (field[0u].asString() == name) - { - return true; - } - } - return false; + auto& fields = result[jss::result][jss::FIELDS]; + // json::Value is not a std::ranges range, so the iterator form is used. + // NOLINTNEXTLINE(modernize-use-ranges) + return std::any_of(fields.begin(), fields.end(), [&](auto& field) { + return field[0u].asString() == name; + }); }; BEAST_EXPECT(fieldExists("Generic")); BEAST_EXPECT(fieldExists("Invalid")); diff --git a/src/test/server/ServerStatus_test.cpp b/src/test/server/ServerStatus_test.cpp index 4a9c12c96b..60ea622616 100644 --- a/src/test/server/ServerStatus_test.cpp +++ b/src/test/server/ServerStatus_test.cpp @@ -202,8 +202,6 @@ class ServerStatus_test : public beast::unit_test::Suite, public beast::test::En if (ec) return; } - - return; } void @@ -218,7 +216,6 @@ class ServerStatus_test : public beast::unit_test::Suite, public beast::test::En auto ip = env.app().config()[Sections::kPortWs].get(Keys::kIp); // NOLINTNEXTLINE(bugprone-unchecked-optional-access) doRequest(yield, makeWSUpgrade(*ip, *port), *ip, *port, secure, resp, ec); - return; } void @@ -235,7 +232,6 @@ class ServerStatus_test : public beast::unit_test::Suite, public beast::test::En auto const ip = env.app().config()[Sections::kPortRpc].get(Keys::kIp); // NOLINTNEXTLINE(bugprone-unchecked-optional-access) doRequest(yield, makeHTTPRequest(*ip, *port, body, fields), *ip, *port, secure, resp, ec); - return; } static auto diff --git a/src/tests/libxrpl/basics/Buffer.cpp b/src/tests/libxrpl/basics/Buffer.cpp index 5b34585fcf..9cdf610282 100644 --- a/src/tests/libxrpl/basics/Buffer.cpp +++ b/src/tests/libxrpl/basics/Buffer.cpp @@ -83,7 +83,7 @@ TEST_F(BufferTest, buffer) x = b0; EXPECT_EQ(x, b0); EXPECT_TRUE(sane(x)); -#if defined(__clang__) +#ifdef __clang__ #pragma clang diagnostic push #pragma clang diagnostic ignored "-Wself-assign-overloaded" #endif @@ -95,7 +95,7 @@ TEST_F(BufferTest, buffer) EXPECT_EQ(y, b3); EXPECT_TRUE(sane(y)); -#if defined(__clang__) +#ifdef __clang__ #pragma clang diagnostic pop #endif } diff --git a/src/tests/libxrpl/basics/base_uint_test.cpp b/src/tests/libxrpl/basics/base_uint_test.cpp index 365b43930c..eefbff158d 100644 --- a/src/tests/libxrpl/basics/base_uint_test.cpp +++ b/src/tests/libxrpl/basics/base_uint_test.cpp @@ -271,25 +271,6 @@ TEST_F(BaseUintTest, base_uint) static_assert(BaseUInt96("000000000000000000000001").signum() == 1); static_assert(BaseUInt96("800000000000000000000000").signum() == 1); -// Everything within the #if should fail during compilation. -#if 0 - // Too few characters - static_assert(BaseUInt96("00000000000000000000000").signum() == 0); - - // Too many characters - static_assert(BaseUInt96("0000000000000000000000000").signum() == 0); - - // Non-hex characters - static_assert(BaseUInt96("00000000000000000000000 ").signum() == 1); - static_assert(BaseUInt96("00000000000000000000000/").signum() == 1); - static_assert(BaseUInt96("00000000000000000000000:").signum() == 1); - static_assert(BaseUInt96("00000000000000000000000@").signum() == 1); - static_assert(BaseUInt96("00000000000000000000000G").signum() == 1); - static_assert(BaseUInt96("00000000000000000000000`").signum() == 1); - static_assert(BaseUInt96("00000000000000000000000g").signum() == 1); - static_assert(BaseUInt96("00000000000000000000000~").signum() == 1); -#endif // 0 - // Using the constexpr constructor in a non-constexpr context // with an error in the parsing throws an exception. { diff --git a/src/xrpld/app/misc/NetworkOPs.cpp b/src/xrpld/app/misc/NetworkOPs.cpp index 12d63c6210..4b0091dff6 100644 --- a/src/xrpld/app/misc/NetworkOPs.cpp +++ b/src/xrpld/app/misc/NetworkOPs.cpp @@ -3819,10 +3819,9 @@ NetworkOPsImp::addAccountHistoryJob(SubAccountHistoryInfoWeak subInfo) return true; } - for (auto& node : meta->getNodes()) - { + return std::ranges::any_of(meta->getNodes(), [&](auto& node) { if (node.getFieldU16(sfLedgerEntryType) != ltACCOUNT_ROOT) - continue; + return false; if (node.isFieldPresent(sfNewFields)) { @@ -3836,9 +3835,8 @@ NetworkOPsImp::addAccountHistoryJob(SubAccountHistoryInfoWeak subInfo) } } } - } - - return false; + return false; + }); }; auto send = [&](json::Value const& jvObj, bool unsubscribe) -> bool { diff --git a/src/xrpld/app/misc/SHAMapStoreImp.cpp b/src/xrpld/app/misc/SHAMapStoreImp.cpp index 0e0099cdbf..6167b5d772 100644 --- a/src/xrpld/app/misc/SHAMapStoreImp.cpp +++ b/src/xrpld/app/misc/SHAMapStoreImp.cpp @@ -437,15 +437,15 @@ SHAMapStoreImp::dbPaths() it != boost::filesystem::directory_iterator(); ++it) { - if (state.writableDb.compare(it->path().string()) == 0) + if (state.writableDb == it->path().string()) { writableDbExists = true; } - else if (state.archiveDb.compare(it->path().string()) == 0) + else if (state.archiveDb == it->path().string()) { archiveDbExists = true; } - else if (dbPrefix_.compare(it->path().stem().string()) == 0) + else if (dbPrefix_ == it->path().stem().string()) { pathsToDelete.push_back(it->path()); } diff --git a/src/xrpld/app/misc/detail/ValidatorSite.cpp b/src/xrpld/app/misc/detail/ValidatorSite.cpp index 7b51fbd597..73ec3e0d6f 100644 --- a/src/xrpld/app/misc/detail/ValidatorSite.cpp +++ b/src/xrpld/app/misc/detail/ValidatorSite.cpp @@ -389,7 +389,7 @@ ValidatorSite::parseJsonResponse( json::Value const body = [&res, siteIdx, this]() { json::Reader r; json::Value body; - if (!r.parse(res.data(), body)) + if (!r.parse(res, body)) { JLOG(j_.warn()) << "Unable to parse JSON response from " << sites_[siteIdx].activeResource->uri; diff --git a/src/xrpld/peerfinder/detail/Logic.h b/src/xrpld/peerfinder/detail/Logic.h index b0047b1ea3..a7dbbf850d 100644 --- a/src/xrpld/peerfinder/detail/Logic.h +++ b/src/xrpld/peerfinder/detail/Logic.h @@ -965,12 +965,8 @@ public: bool fixed(beast::IP::Endpoint const& endpoint) const { - for (auto const& entry : fixed_) - { - if (entry.first == endpoint) - return true; - } - return false; + return std::ranges::any_of( + fixed_, [&endpoint](auto const& entry) { return entry.first == endpoint; }); } // Returns `true` if the address matches a fixed slot address @@ -979,12 +975,8 @@ public: bool fixed(beast::IP::Address const& address) const { - for (auto const& entry : fixed_) - { - if (entry.first.address() == address) - return true; - } - return false; + return std::ranges::any_of( + fixed_, [&address](auto const& entry) { return entry.first.address() == address; }); } //-------------------------------------------------------------------------- diff --git a/src/xrpld/rpc/detail/RPCCall.cpp b/src/xrpld/rpc/detail/RPCCall.cpp index af5677008d..b5d5c680cd 100644 --- a/src/xrpld/rpc/detail/RPCCall.cpp +++ b/src/xrpld/rpc/detail/RPCCall.cpp @@ -569,12 +569,10 @@ private: { if (jv.size() == 0) return false; - for (auto const& j : jv) - { - if (!isValidJson2(j)) - return false; - } - return true; + // json::Value is not a std::ranges range, so the iterator form is used. + // NOLINTNEXTLINE(modernize-use-ranges) + return std::all_of( + jv.begin(), jv.end(), [this](auto const& j) { return isValidJson2(j); }); } if (jv.isObject()) { diff --git a/src/xrpld/rpc/handlers/server_info/ServerDefinitions.cpp b/src/xrpld/rpc/handlers/server_info/ServerDefinitions.cpp index 1e9e0ddc14..cce1b3e07f 100644 --- a/src/xrpld/rpc/handlers/server_info/ServerDefinitions.cpp +++ b/src/xrpld/rpc/handlers/server_info/ServerDefinitions.cpp @@ -108,7 +108,7 @@ ServerDefinitions::translate(std::string const& inp) if (token.size() > 1) { boost::algorithm::to_lower(token); - token.data()[0] -= ('a' - 'A'); + token[0] -= ('a' - 'A'); out += token; } else From 73e97b8b843de76ee6810d7a30592022177d8256 Mon Sep 17 00:00:00 2001 From: Alex Kremer Date: Tue, 14 Jul 2026 13:35:04 +0100 Subject: [PATCH 05/13] test: Add JSON array size tests (#7592) --- src/test/protocol/STParsedJSON_test.cpp | 191 +++++++++++++++++++++++- 1 file changed, 189 insertions(+), 2 deletions(-) diff --git a/src/test/protocol/STParsedJSON_test.cpp b/src/test/protocol/STParsedJSON_test.cpp index 24981053f5..c2030627c7 100644 --- a/src/test/protocol/STParsedJSON_test.cpp +++ b/src/test/protocol/STParsedJSON_test.cpp @@ -19,6 +19,7 @@ #include #include +#include #include #include #include @@ -1979,7 +1980,7 @@ class STParsedJSON_test : public beast::unit_test::Suite json::Value j; json::Value obj(json::ValueType::Object); json::Value* current = &obj; - for (int i = 0; i < 63; ++i) + for (std::size_t i = 1; i < kMaxParsedJsonDepth; ++i) { json::Value const next(json::ValueType::Object); (*current)[sfTransactionMetaData] = next; @@ -1998,7 +1999,7 @@ class STParsedJSON_test : public beast::unit_test::Suite json::Value j; json::Value obj(json::ValueType::Object); json::Value* current = &obj; - for (int i = 0; i < 64; ++i) + for (std::size_t i = 1; i <= kMaxParsedJsonDepth; ++i) { json::Value const next(json::ValueType::Object); (*current)[sfTransactionMetaData] = next; @@ -2153,6 +2154,191 @@ class STParsedJSON_test : public beast::unit_test::Suite } } + void + testArrayBoundsChecking() + { + testcase("Array bounds checking"); + + auto const limitStr = std::to_string(kMaxParsedJsonArraySize) + " elements per field."; + + // parseArray rejects oversized STI_ARRAY (SignerEntries) + { + json::Value jv; + json::Value entries(json::ValueType::Array); + for (std::size_t i = 0; i <= kMaxParsedJsonArraySize; ++i) + { + json::Value entry; + entry["SignerEntry"]["Account"] = "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh"; + entry["SignerEntry"]["SignerWeight"] = 1; + entries.append(entry); + } + jv[sfSignerEntries] = entries; + + STParsedJSONObject parsed("test", jv); + BEAST_EXPECT(!parsed.object); + BEAST_EXPECT(parsed.error[jss::error] == "invalidParams"); + BEAST_EXPECT( + parsed.error[jss::error_message] == + "Field 'test.SignerEntries' exceeds allowed JSON array size of " + limitStr); + } + + // parseObject rejects oversized STI_VECTOR256 (Amendments) + { + json::Value jv; + json::Value amendments(json::ValueType::Array); + std::string const hash(64, '0'); + for (std::size_t i = 0; i <= kMaxParsedJsonArraySize; ++i) + amendments.append(hash); + jv[sfAmendments] = amendments; + + STParsedJSONObject parsed("test", jv); + BEAST_EXPECT(!parsed.object); + BEAST_EXPECT(parsed.error[jss::error] == "invalidParams"); + BEAST_EXPECT( + parsed.error[jss::error_message] == + "Field 'test.Amendments' exceeds allowed JSON array size of " + limitStr); + } + + // parseObject accepts exactly kMaxParsedJsonArraySize STI_VECTOR256 (Amendments) + { + json::Value jv; + json::Value amendments(json::ValueType::Array); + std::string const hash(64, '0'); + for (std::size_t i = 0; i < kMaxParsedJsonArraySize; ++i) + amendments.append(hash); + jv[sfAmendments] = amendments; + + STParsedJSONObject const parsed("test", jv); + BEAST_EXPECT(parsed.object); + // NOLINTNEXTLINE(bugprone-unchecked-optional-access) + auto const arrSize = parsed.object->getFieldV256(sfAmendments).size(); + BEAST_EXPECT(arrSize == kMaxParsedJsonArraySize); + } + + // parseObject rejects oversized STI_PATHSET (outer array) + { + json::Value jv; + json::Value paths(json::ValueType::Array); + for (std::size_t i = 0; i <= kMaxParsedJsonArraySize; ++i) + { + json::Value path(json::ValueType::Array); + json::Value hop; + hop["account"] = "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh"; + path.append(hop); + paths.append(path); + } + jv[sfPaths] = paths; + + STParsedJSONObject parsed("test", jv); + BEAST_EXPECT(!parsed.object); + BEAST_EXPECT(parsed.error[jss::error] == "invalidParams"); + BEAST_EXPECT( + parsed.error[jss::error_message] == + "Field 'test.Paths' exceeds allowed JSON array size of " + limitStr); + } + + // parseObject accepts exactly kMaxParsedJsonArraySize STI_PATHSET (outer array) + { + json::Value jv; + json::Value paths(json::ValueType::Array); + for (std::size_t i = 0; i < kMaxParsedJsonArraySize; ++i) + { + json::Value path(json::ValueType::Array); + json::Value hop; + hop["account"] = "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh"; + path.append(hop); + paths.append(path); + } + jv[sfPaths] = paths; + + STParsedJSONObject const parsed("test", jv); + BEAST_EXPECT(parsed.object); + // NOLINTNEXTLINE(bugprone-unchecked-optional-access) + auto const arrSize = parsed.object->getFieldPathSet(sfPaths).size(); + BEAST_EXPECT(arrSize == kMaxParsedJsonArraySize); + } + + // parseObject rejects oversized STI_PATHSET (inner path hop array) + { + json::Value jv; + json::Value paths(json::ValueType::Array); + json::Value path(json::ValueType::Array); + json::Value hop; + hop["account"] = "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh"; + for (std::size_t i = 0; i <= kMaxParsedJsonArraySize; ++i) + path.append(hop); + paths.append(path); + jv[sfPaths] = paths; + + STParsedJSONObject parsed("test", jv); + BEAST_EXPECT(!parsed.object); + BEAST_EXPECT(parsed.error[jss::error] == "invalidParams"); + BEAST_EXPECT( + parsed.error[jss::error_message] == + "Field 'test.Paths[0]' exceeds allowed JSON array size of " + limitStr); + } + + // parseObject accepts exactly kMaxParsedJsonArraySize hops in a single STI_PATHSET path + { + json::Value jv; + json::Value paths(json::ValueType::Array); + json::Value path(json::ValueType::Array); + json::Value hop; + hop["account"] = "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh"; + for (std::size_t i = 0; i < kMaxParsedJsonArraySize; ++i) + path.append(hop); + paths.append(path); + jv[sfPaths] = paths; + + STParsedJSONObject const parsed("test", jv); + BEAST_EXPECT(parsed.object); + // NOLINTNEXTLINE(bugprone-unchecked-optional-access) + auto const arrSize = parsed.object->getFieldPathSet(sfPaths)[0].size(); + BEAST_EXPECT(arrSize == kMaxParsedJsonArraySize); + } + + // parseArray accepts exactly kMaxParsedJsonArraySize Memos (boundary) + { + json::Value jv; + json::Value memos(json::ValueType::Array); + for (std::size_t i = 0; i < kMaxParsedJsonArraySize; ++i) + { + json::Value memo; + memo["Memo"] = json::Value(json::ValueType::Object); + memo["Memo"]["MemoData"] = "00"; + memos.append(memo); + } + jv[sfMemos] = memos; + + STParsedJSONObject const parsed("test", jv); + BEAST_EXPECT(parsed.object); + // NOLINTNEXTLINE(bugprone-unchecked-optional-access) + auto const arrSize = parsed.object->getFieldArray(sfMemos).size(); + BEAST_EXPECT(arrSize == kMaxParsedJsonArraySize); + } + + // parseArray rejects one more than kMaxParsedJsonArraySize Memos + { + json::Value jv; + json::Value memos(json::ValueType::Array); + for (std::size_t i = 0; i <= kMaxParsedJsonArraySize; ++i) + { + json::Value memo; + memo["Memo"] = json::Value(json::ValueType::Object); + memo["Memo"]["MemoData"] = "00"; + memos.append(memo); + } + jv[sfMemos] = memos; + + STParsedJSONObject parsed("test", jv); + BEAST_EXPECT(!parsed.object); + BEAST_EXPECT(parsed.error[jss::error] == "invalidParams"); + BEAST_EXPECT( + parsed.error[jss::error_message] == + "Field 'test.Memos' exceeds allowed JSON array size of " + limitStr); + } + } + void testEdgeCases() { @@ -2420,6 +2606,7 @@ class STParsedJSON_test : public beast::unit_test::Suite testNumber(); testObject(); testArray(); + testArrayBoundsChecking(); testEdgeCases(); } }; From ab3ff66cd9a4360c84849f40bc51f7f0d2c588a6 Mon Sep 17 00:00:00 2001 From: Ayaz Salikhov Date: Tue, 14 Jul 2026 13:55:34 +0100 Subject: [PATCH 06/13] docs: Add more information about pre-commit hooks and how to set them up (#7802) --- CONTRIBUTING.md | 85 ++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 70 insertions(+), 15 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index fc93223925..9929b2eb39 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -84,7 +84,9 @@ If you create new source files, they must be organized as follows: - All other non-test files must go under `src/xrpld`. - All test source files must go under `src/test`. -The source must be formatted according to the style guide below. +The source must be formatted according to the style guide below. The easiest +way to satisfy this is to install the [`pre-commit`](#pre-commit-hooks) hooks, +which format and lint your changes automatically on every commit. Header includes must be [levelized](.github/scripts/levelization). @@ -212,13 +214,61 @@ This is a non-exhaustive list of recommended style guidelines. These are not always strictly enforced and serve as a way to keep the codebase coherent rather than a set of _thou shalt not_ commandments. +## Pre-commit hooks + +We use the [`pre-commit`](https://pre-commit.com/) framework to run the +formatting and linting tools that keep the codebase consistent. `pre-commit` +runs each tool configured in +[`.pre-commit-config.yaml`](./.pre-commit-config.yaml) in its own isolated +environment, so you don't need to install most of the individual tools +yourself. The version of each hook sourced from an external repository +(`clang-format`, `gersemi`, etc.) is pinned in that file, so running the hooks +locally uses exactly the same versions as CI. A few `local` hooks — most notably +`clang-tidy` — run tools from your own environment; see +[Installing clang-tidy](#installing-clang-tidy) for how to get those. + +To get started, install `pre-commit` and enable the git hook scripts: + +```bash +pip install pre-commit +pre-commit install +``` + +Once installed, the hooks run automatically on your staged files every time you +`git commit`. You can also run them on demand: + +```bash +# Run all hooks against only the staged files +pre-commit run + +# Run all hooks against every file in the repository +pre-commit run --all-files + +# Run a single hook (e.g. clang-format) against all files +pre-commit run clang-format --all-files +``` + +The hooks configured in this repository include, among others: + +- `clang-format` — C++/proto formatting (see [Formatting](#formatting)) +- `clang-tidy` — C++ static analysis (see [Clang-tidy](#clang-tidy)); opt in with `TIDY=1` +- `fix-include-style`, `fix-pragma-once`, `check-doxygen-style` — C++ hygiene +- `gersemi` — CMake formatting +- `prettier`, `black`, `shfmt` — formatting for JavaScript/JSON/Markdown, Python, and shell +- `cspell` — spell checking + +The same hooks run in CI on every pull request, so running them locally before +you push helps you avoid CI failures. + ## Formatting -All code must conform to `clang-format` version 22, -according to the settings in [`.clang-format`](./.clang-format), -unless the result would be unreasonably difficult to read or maintain. -To demarcate lines that should be left as-is, surround them with comments like -this: +All code must conform to `clang-format`, according to the settings in +[`.clang-format`](./.clang-format), unless the result would be unreasonably +difficult to read or maintain. The `clang-format` version is pinned in +[`.pre-commit-config.yaml`](./.pre-commit-config.yaml), so the +[`pre-commit`](#pre-commit-hooks) hook always formats with the same version as +CI. To demarcate lines that should be left as-is, surround them with comments +like this: ``` // clang-format off @@ -226,9 +276,21 @@ this: // clang-format on ``` -You can format individual files in place by running `clang-format -i ...` +The easiest way to format your changes is to let the `pre-commit` hook run +automatically on commit, or to run it manually: + +```bash +pre-commit run clang-format --all-files +``` + +You can also format individual files in place by running `clang-format -i ...` from any directory within this project. +> [!NOTE] +> This uses whatever `clang-format` version is installed locally, which may +> differ from the pinned version used by `pre-commit` and CI, so the results +> can vary. + There is a Continuous Integration job that runs clang-format on pull requests. If the code doesn't comply, a patch file that corrects auto-fixable formatting issues is generated. To download the patch file: @@ -239,13 +301,6 @@ To download the patch file: 4. Download the zip file and extract it to your local git repository. Run `git apply [patch-file-name]`. 5. Commit and push. -You can install a pre-commit hook to automatically run `clang-format` before every commit: - -``` -pip3 install pre-commit -pre-commit install -``` - ## Clang-tidy All code must pass `clang-tidy` checks according to the settings in [`.clang-tidy`](./.clang-tidy). @@ -267,7 +322,7 @@ Before running clang-tidy, you must build the project to generate required files #### Via pre-commit (recommended) -If you have already installed the pre-commit hooks (see above), you can run clang-tidy on your staged files using: +If you have already installed the [`pre-commit`](#pre-commit-hooks) hooks, you can run clang-tidy on your staged files using: ``` TIDY=1 pre-commit run clang-tidy From 2e25435a4a16bd8310b62eceae010dd1abb3b12c Mon Sep 17 00:00:00 2001 From: Sergey Kuznetsov Date: Tue, 14 Jul 2026 14:28:55 +0100 Subject: [PATCH 07/13] ci: Add Rust to Nix docker image (#7571) --- .cspell.config.yaml | 2 + .../workflows/reusable-build-test-config.yml | 2 +- bin/check-tools.sh | 17 +++++ nix/docker/Dockerfile | 19 +++-- nix/docker/README.md | 21 ++++-- .../compile-sources.sh} | 0 .../run-binaries.sh} | 0 .../{cpp_sources => cpp/sources}/asan.cpp | 0 .../{cpp_sources => cpp/sources}/regular.cpp | 0 .../{cpp_sources => cpp/sources}/tsan.cpp | 0 .../{cpp_sources => cpp/sources}/ubsan.cpp | 0 nix/docker/test_files/rust/compile-sources.sh | 45 +++++++++++ nix/docker/test_files/rust/run-binaries.sh | 74 +++++++++++++++++++ nix/docker/test_files/rust/sources/hello.rs | 16 ++++ .../test_files/rust/sources/overflow.rs | 13 ++++ nix/docker/test_files/rust/sources/panic.rs | 5 ++ nix/packages.nix | 10 +++ 17 files changed, 209 insertions(+), 15 deletions(-) rename nix/docker/test_files/{compile-cpp-sources.sh => cpp/compile-sources.sh} (100%) rename nix/docker/test_files/{run-test-binaries.sh => cpp/run-binaries.sh} (100%) rename nix/docker/test_files/{cpp_sources => cpp/sources}/asan.cpp (100%) rename nix/docker/test_files/{cpp_sources => cpp/sources}/regular.cpp (100%) rename nix/docker/test_files/{cpp_sources => cpp/sources}/tsan.cpp (100%) rename nix/docker/test_files/{cpp_sources => cpp/sources}/ubsan.cpp (100%) create mode 100755 nix/docker/test_files/rust/compile-sources.sh create mode 100755 nix/docker/test_files/rust/run-binaries.sh create mode 100644 nix/docker/test_files/rust/sources/hello.rs create mode 100644 nix/docker/test_files/rust/sources/overflow.rs create mode 100644 nix/docker/test_files/rust/sources/panic.rs diff --git a/.cspell.config.yaml b/.cspell.config.yaml index 19b3970865..13da132b90 100644 --- a/.cspell.config.yaml +++ b/.cspell.config.yaml @@ -73,6 +73,7 @@ words: - citardauq - clawback - clawbacks + - clippy - cmaketoolchain - coeffs - coldwallet @@ -260,6 +261,7 @@ words: - rocksdb - Rohrs - roundings + - rustc - sahyadri - Satoshi - scons diff --git a/.github/workflows/reusable-build-test-config.yml b/.github/workflows/reusable-build-test-config.yml index e7a88a0e66..b2327f67ea 100644 --- a/.github/workflows/reusable-build-test-config.yml +++ b/.github/workflows/reusable-build-test-config.yml @@ -124,7 +124,7 @@ jobs: - name: Check tools env: CHECK_TOOLS_SKIP_CLONE: "1" - run: ./bin/check-tools.sh + run: ./bin/check-tools.sh || true - name: Print build environment uses: XRPLF/actions/print-build-env@59dec886e4afb05a1724443af08baccbc045b574 diff --git a/bin/check-tools.sh b/bin/check-tools.sh index 808f384d5b..7886bcf8b0 100755 --- a/bin/check-tools.sh +++ b/bin/check-tools.sh @@ -110,6 +110,23 @@ if [ "${os}" = "linux" ] || [ "${os}" = "macos" ]; then fi fi +# Rust toolchain. Part of the Nix commonPackages, so available on both Linux +# and macOS. The cargo plugins are invoked through cargo (`cargo `), which +# resolves the matching `cargo-` binary on PATH; `--version` is offline and +# does not need a Cargo project. +if [ "${os}" = "linux" ] || [ "${os}" = "macos" ]; then + echo + echo "Rust toolchain:" + check cargo + check cargo-audit cargo audit --version + check cargo-llvm-cov cargo llvm-cov --version + check cargo-nextest cargo nextest --version + check clippy clippy-driver --version + check rust-analyzer + check rustc + check rustfmt +fi + # GCC is the default compiler on Linux. macOS uses the system Apple Clang # instead, so GCC/g++/gcov are not expected there. if [ "${os}" = "linux" ]; then diff --git a/nix/docker/Dockerfile b/nix/docker/Dockerfile index 6d8980f897..7222cc8fa8 100644 --- a/nix/docker/Dockerfile +++ b/nix/docker/Dockerfile @@ -75,9 +75,13 @@ COPY bin/check-tools.sh /tmp/check-tools.sh RUN /tmp/check-tools.sh # Sanity-check that the g++/clang++ are able to build binaries, including sanitizer-instrumented ones. -COPY nix/docker/test_files/cpp_sources/ /tmp/cpp_sources/ -COPY nix/docker/test_files/compile-cpp-sources.sh /tmp/compile-cpp-sources.sh -RUN /tmp/compile-cpp-sources.sh /tmp/cpp_sources /tmp/bins +COPY nix/docker/test_files/cpp/ /tmp/test_files/cpp/ +RUN /tmp/test_files/cpp/compile-sources.sh /tmp/test_files/cpp/sources /tmp/cpp-bins + +# Sanity-check that rustc is able to build binaries, including ones that rely on +# the runtime overflow check. +COPY nix/docker/test_files/rust/ /tmp/test_files/rust/ +RUN /tmp/test_files/rust/compile-sources.sh /tmp/test_files/rust/sources /tmp/rust-bins # Tester: start from a clean BASE_IMAGE, install sanitizer runtime libraries, # and run the compiled test binaries to verify they execute correctly. @@ -94,15 +98,18 @@ SHELL ["/bin/bash", "-e", "-o", "pipefail", "-c"] # Sanity-check that the built binaries run correctly in the vanilla base image, with the necessary sanitizer runtime libraries installed. COPY bin/install-sanitizer-libs.sh /tmp/install-sanitizer-libs.sh -COPY nix/docker/test_files/run-test-binaries.sh /tmp/run-test-binaries.sh -COPY --from=final /tmp/bins /tmp/bins +COPY nix/docker/test_files/cpp/run-binaries.sh /tmp/test_files/cpp/run-binaries.sh +COPY nix/docker/test_files/rust/run-binaries.sh /tmp/test_files/rust/run-binaries.sh +COPY --from=final /tmp/cpp-bins /tmp/cpp-bins +COPY --from=final /tmp/rust-bins /tmp/rust-bins RUN < as for name in +# {hello,panic,overflow}. + +set -eo pipefail + +bins_dir="${1:?usage: $0 }" + +failed_binaries=() + +# Run a binary and verify its exit code and output. +# Usage: run +function run() { + local binary="${1}" + local expected_output="${2}" + local expected_rc="${3}" + + local out_file + out_file="$(mktemp)" + + echo "=== Run ${binary} ===" + set +e + "${binary}" >"${out_file}" 2>&1 + local rc=$? + set -e + + cat "${out_file}" + + local failed=0 + if [ "${expected_rc}" = "nonzero" ]; then + if [ "${rc}" -eq 0 ]; then + echo "ERROR: expected non-zero exit code from ${binary}, got ${rc}" >&2 + failed=1 + fi + elif [ "${rc}" -ne "${expected_rc}" ]; then + echo "ERROR: expected exit code ${expected_rc} from ${binary}, got ${rc}" >&2 + failed=1 + fi + + if ! grep -q "${expected_output}" "${out_file}"; then + echo "ERROR: expected '${expected_output}' from ${binary}" >&2 + failed=1 + fi + + if [ "${failed}" -eq 0 ]; then + echo "OK: '${expected_output}' detected" + else + failed_binaries+=("${binary}") + fi +} + +declare -A expect=( + [hello]="Hello from main thread" + [panic]="explicit panic from test" + [overflow]="attempt to add with overflow" +) + +for name in hello panic overflow; do + binary="${bins_dir}/${name}" + + if [ "${name}" = "hello" ]; then + expected_rc=0 + else + expected_rc=nonzero + fi + run "${binary}" "${expect[$name]}" "${expected_rc}" +done + +if [ "${#failed_binaries[@]}" -gt 0 ]; then + echo "ERROR: the following binaries failed:" >&2 + printf ' %s\n' "${failed_binaries[@]}" >&2 + exit 1 +fi diff --git a/nix/docker/test_files/rust/sources/hello.rs b/nix/docker/test_files/rust/sources/hello.rs new file mode 100644 index 0000000000..78e32c17f3 --- /dev/null +++ b/nix/docker/test_files/rust/sources/hello.rs @@ -0,0 +1,16 @@ +use std::thread; + +fn main() { + const NUM_THREADS: usize = 10; + let mut handles = Vec::with_capacity(NUM_THREADS); + for id in 0..NUM_THREADS { + handles.push(thread::spawn(move || { + println!("Hello from thread {id}"); + })); + } + for handle in handles { + handle.join().expect("worker thread panicked"); + } + + println!("Hello from main thread"); +} diff --git a/nix/docker/test_files/rust/sources/overflow.rs b/nix/docker/test_files/rust/sources/overflow.rs new file mode 100644 index 0000000000..2a4a54472c --- /dev/null +++ b/nix/docker/test_files/rust/sources/overflow.rs @@ -0,0 +1,13 @@ +use std::hint::black_box; + +// Rust analogue of the C++ UBSan check: with overflow checks enabled the +// compiler inserts a runtime check that panics on signed integer overflow. +// `black_box` keeps the operands opaque so the addition is evaluated at +// runtime rather than being rejected by the compile-time overflow lint. +fn main() { + let max = black_box(i32::MAX); + let one = black_box(1); + println!("Current max: {max}"); + let overflowed = max + one; + println!("Overflowed result: {overflowed}"); +} diff --git a/nix/docker/test_files/rust/sources/panic.rs b/nix/docker/test_files/rust/sources/panic.rs new file mode 100644 index 0000000000..38779ff515 --- /dev/null +++ b/nix/docker/test_files/rust/sources/panic.rs @@ -0,0 +1,5 @@ +fn main() { + // Verify the panic runtime works: a panic must print its message to stderr + // and exit with a non-zero status (Rust's default panic exit code is 101). + panic!("explicit panic from test"); +} diff --git a/nix/packages.nix b/nix/packages.nix index bf10ee3712..41d7e97328 100644 --- a/nix/packages.nix +++ b/nix/packages.nix @@ -62,5 +62,15 @@ in runClangTidy vim zip + # Rust packages + cargo + cargo-audit + cargo-llvm-cov + cargo-nextest + clippy + corrosion + rust-analyzer + rustc + rustfmt ]; } From c6211367483db5be0ffb0b9b79f59f1725d5cd3b Mon Sep 17 00:00:00 2001 From: Jingchen Date: Tue, 14 Jul 2026 15:29:18 +0100 Subject: [PATCH 08/13] test: Add unit tests for IP address related functions (#7744) Co-authored-by: Ayaz Salikhov Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Ed Hennis --- src/test/beast/IPEndpoint_test.cpp | 9 + src/test/peerfinder/PeerFinder_test.cpp | 259 +++++++++++++++++++++++- 2 files changed, 267 insertions(+), 1 deletion(-) diff --git a/src/test/beast/IPEndpoint_test.cpp b/src/test/beast/IPEndpoint_test.cpp index b1cd4709b4..bc04087891 100644 --- a/src/test/beast/IPEndpoint_test.cpp +++ b/src/test/beast/IPEndpoint_test.cpp @@ -305,6 +305,15 @@ public: BEAST_EXPECT(!isLoopback(ep)); BEAST_EXPECTS(to_string(ep) == "fd00::1", to_string(ep)); + // unspecified IPv6 (::) + ep = Endpoint(AddressV6{}); + BEAST_EXPECT(isUnspecified(ep)); + BEAST_EXPECT(!isPublic(ep)); + BEAST_EXPECT(!isPrivate(ep)); + BEAST_EXPECT(!isMulticast(ep)); + BEAST_EXPECT(!isLoopback(ep)); + BEAST_EXPECTS(to_string(ep) == "::", to_string(ep)); + { ep = Endpoint::fromString("192.0.2.112"); BEAST_EXPECT(!isUnspecified(ep)); diff --git a/src/test/peerfinder/PeerFinder_test.cpp b/src/test/peerfinder/PeerFinder_test.cpp index c4f129c1a3..cf91800951 100644 --- a/src/test/peerfinder/PeerFinder_test.cpp +++ b/src/test/peerfinder/PeerFinder_test.cpp @@ -21,6 +21,7 @@ #include #include #include +#include #include namespace xrpl::PeerFinder { @@ -64,8 +65,9 @@ public: void asyncConnect(beast::IP::Endpoint const& ep, Handler&& handler) { + // NOLINTNEXTLINE(misc-const-correctness) boost::system::error_code ec; - handler(ep, ep, ec); + handler(ec); } }; @@ -368,6 +370,259 @@ public: } } + void + testIsValidAddress() + { + testcase("is_valid_address"); + TestStore store; + TestChecker checker; + TestStopwatch clock; + Logic logic(clock, store, checker, journal_); + + auto const pass = [&](std::string const& s) { + BEAST_EXPECT(logic.isValidAddress(beast::IP::Endpoint::fromString(s))); + }; + auto const fail = [&](std::string const& s) { + BEAST_EXPECT(!logic.isValidAddress(beast::IP::Endpoint::fromString(s))); + }; + + // Invalid: port 0 + fail("65.0.0.1:0"); + + // --- IPv4 ranges --- + // For each range: 1 before (pass), first (fail), last (fail), + // 1 after (pass) + + // 0.0.0.0/8 - "This network" + // No "before" - nothing before 0.0.0.0 + fail("0.0.0.0:8080"); + fail("0.255.255.255:8080"); + pass("1.0.0.0:8080"); + + // 10.0.0.0/8 - Private (RFC 1918) + pass("9.255.255.255:8080"); + fail("10.0.0.0:8080"); + fail("10.255.255.255:8080"); + pass("11.0.0.0:8080"); + + // 100.64.0.0/10 - Shared Address Space / CGNAT (RFC 6598) + pass("100.63.255.255:8080"); + fail("100.64.0.0:8080"); + fail("100.127.255.255:8080"); + pass("100.128.0.0:8080"); + + // 127.0.0.0/8 - Loopback + pass("126.255.255.255:8080"); + fail("127.0.0.0:8080"); + fail("127.255.255.255:8080"); + pass("128.0.0.0:8080"); + + // 169.254.0.0/16 - Link-local + pass("169.253.255.255:8080"); + fail("169.254.0.0:8080"); + fail("169.254.255.255:8080"); + pass("169.255.0.0:8080"); + + // 172.16.0.0/12 - Private (RFC 1918) + pass("172.15.255.255:8080"); + fail("172.16.0.0:8080"); + fail("172.31.255.255:8080"); + pass("172.32.0.0:8080"); + + // 192.0.0.0/24 - IETF Protocol Assignments (RFC 6890) + pass("191.255.255.255:8080"); + fail("192.0.0.0:8080"); + fail("192.0.0.255:8080"); + pass("192.0.1.0:8080"); + + // 192.0.2.0/24 - TEST-NET-1 (RFC 5737) + pass("192.0.1.255:8080"); + fail("192.0.2.0:8080"); + fail("192.0.2.255:8080"); + pass("192.0.3.0:8080"); + + // 192.88.99.0/24 - 6to4 Relay Anycast (RFC 7526) + pass("192.88.98.255:8080"); + fail("192.88.99.0:8080"); + fail("192.88.99.255:8080"); + pass("192.88.100.0:8080"); + + // 192.168.0.0/16 - Private (RFC 1918) + pass("192.167.255.255:8080"); + fail("192.168.0.0:8080"); + fail("192.168.255.255:8080"); + pass("192.169.0.0:8080"); + + // 198.18.0.0/15 - Benchmarking (RFC 2544) + pass("198.17.255.255:8080"); + fail("198.18.0.0:8080"); + fail("198.19.255.255:8080"); + pass("198.20.0.0:8080"); + + // 198.51.100.0/24 - TEST-NET-2 (RFC 5737) + pass("198.51.99.255:8080"); + fail("198.51.100.0:8080"); + fail("198.51.100.255:8080"); + pass("198.51.101.0:8080"); + + // 203.0.113.0/24 - TEST-NET-3 (RFC 5737) + pass("203.0.112.255:8080"); + fail("203.0.113.0:8080"); + fail("203.0.113.255:8080"); + pass("203.0.114.0:8080"); + + // 224.0.0.0/4 - Multicast + pass("223.255.255.255:8080"); + fail("224.0.0.0:8080"); + fail("239.255.255.255:8080"); + // 240.0.0.0 (after multicast) is also blocked (reserved) + + // 240.0.0.0/4 - Reserved (RFC 1112) + // 239.255.255.255 (before reserved) is also blocked (multicast) + fail("240.0.0.0:8080"); + fail("255.255.255.255:8080"); + + // --- IPv6 ranges --- + + // ::1 - Loopback (single address) + fail("[::1]:8080"); + + // :: - Unspecified (single address) + fail("[::]:8080"); + + // fc00::/7 - Unique Local Address (ULA) + pass("[fb00::1]:8080"); + fail("[fc00::1]:8080"); + fail("[fdff::1]:8080"); + pass("[fe00::1]:8080"); + + // fe80::/10 - Link-local + pass("[fe7f::1]:8080"); + fail("[fe80::1]:8080"); + fail("[febf::1]:8080"); + pass("[fec0::1]:8080"); + + // ff00::/8 - Multicast + pass("[feff::1]:8080"); + fail("[ff00::1]:8080"); + fail("[ffff::1]:8080"); + // No "after" - ffff:... is the highest IPv6 range + + // 100::/64 - Discard prefix (RFC 6666) + pass("[ff::1]:8080"); + fail("[100::]:8080"); + fail("[100::ffff:ffff:ffff:ffff]:8080"); + pass("[100:0:0:1::1]:8080"); + + // 2001::/32 - IETF Protocol Assignments / Teredo (RFC 4380) + pass("[2000:ffff::1]:8080"); + fail("[2001::]:8080"); + fail("[2001:0:ffff::1]:8080"); + pass("[2001:1::1]:8080"); + + // 2001:20::/28 - ORCHIDv2 (RFC 7343) + pass("[2001:1f::1]:8080"); + fail("[2001:20::1]:8080"); + fail("[2001:2f::1]:8080"); + pass("[2001:30::1]:8080"); + + // 2001:db8::/32 - Documentation (RFC 3849) + pass("[2001:db7::1]:8080"); + fail("[2001:db8::1]:8080"); + fail("[2001:db8:ffff::1]:8080"); + pass("[2001:db9::1]:8080"); + + // 2002::/16 - 6to4 (RFC 3056, deprecated) + pass("[2001:ffff::1]:8080"); + fail("[2002::1]:8080"); + fail("[2002:ffff::1]:8080"); + pass("[2003::1]:8080"); + + // --- IPv6 v4-mapped (delegates to IPv4 checks) --- + fail("[::ffff:10.0.0.1]:8080"); + fail("[::ffff:100.64.0.1]:8080"); + fail("[::ffff:169.254.1.1]:8080"); + fail("[::ffff:192.0.2.1]:8080"); + fail("[::ffff:198.18.0.1]:8080"); + fail("[::ffff:224.0.0.1]:8080"); + fail("[::ffff:240.0.0.1]:8080"); + + // --- Valid public addresses --- + pass("8.8.8.8:443"); + pass("65.0.0.1:8080"); + pass("[2001:4860:4860::8888]:8080"); + pass("[2606:4700:4700::1111]:8080"); + } + + void + testVerifyEndpoints() + { + // Helper that sets up a Logic instance, creates and activates a slot, + // then calls on_endpoints with the given list and returns the + // livecache size afterwards. + auto run = [&](bool verifyEndpoints, Endpoints eps) -> std::size_t { + TestStore store; + TestChecker checker; + TestStopwatch clock; + Logic logic(clock, store, checker, journal_); + { + Config c; + c.autoConnect = false; + c.listeningPort = 1024; + c.ipLimit = 2; + c.verifyEndpoints = verifyEndpoints; + logic.config(c); + } + + auto const remote = beast::IP::Endpoint::fromString("65.0.0.1:5"); + auto const local = beast::IP::Endpoint::fromString("65.0.0.2:1024"); + + auto const [slot, r] = logic.newOutboundSlot(remote); + BEAST_EXPECT(slot != nullptr); + BEAST_EXPECT(r == Result::Success); + BEAST_EXPECT(logic.onConnected(slot, local)); + + PublicKey const pk(randomKeyPair(KeyType::Secp256k1).first); + BEAST_EXPECT(logic.activate(slot, pk, false) == Result::Success); + + logic.onEndpoints(slot, std::move(eps)); + + auto const size = logic.livecache.size(); + logic.onClosed(slot); + return size; + }; + + { + testcase("verify_endpoints enabled"); + + // Valid public addresses + Endpoints eps; + eps.emplace_back(beast::IP::Endpoint::fromString("44.0.0.1:5"), 1); + eps.emplace_back(beast::IP::Endpoint::fromString("44.0.0.2:6"), 1); + // Invalid: private address + eps.emplace_back(beast::IP::Endpoint::fromString("10.0.0.1:5"), 1); + // Invalid: port 0 + eps.emplace_back(beast::IP::Endpoint::fromString("44.0.0.3:0"), 1); + + // With verification enabled, only the 2 valid endpoints survive + BEAST_EXPECT(run(true, eps) == 2); + } + { + testcase("verify_endpoints disabled"); + + Endpoints eps; + eps.emplace_back(beast::IP::Endpoint::fromString("44.0.0.1:5"), 1); + eps.emplace_back(beast::IP::Endpoint::fromString("44.0.0.2:6"), 1); + // Private address — kept when verification is off + eps.emplace_back(beast::IP::Endpoint::fromString("10.0.0.1:5"), 1); + // Port 0 — kept when verification is off + eps.emplace_back(beast::IP::Endpoint::fromString("44.0.0.3:0"), 1); + + // Without verification, all 4 endpoints survive + BEAST_EXPECT(run(false, eps) == 4); + } + } + void testOnConnectedSelfConnection() { @@ -524,6 +779,8 @@ public: testActivateInboundDisabled(); testAddFixedPeerNoPort(); testOnConnectedSelfConnection(); + testIsValidAddress(); + testVerifyEndpoints(); } }; From 2403670da94eafe8d073111f6fc15f4a796115cf Mon Sep 17 00:00:00 2001 From: Gregory Tsipenyuk Date: Tue, 14 Jul 2026 10:31:06 -0400 Subject: [PATCH 09/13] fix: Strengthen Clawback invariant checks for MPT balances (#7285) --- include/xrpl/tx/invariants/InvariantCheck.h | 17 +- include/xrpl/tx/invariants/MPTInvariant.h | 2 +- src/libxrpl/tx/invariants/InvariantCheck.cpp | 149 +++++- src/libxrpl/tx/invariants/MPTInvariant.cpp | 4 +- src/test/app/Invariants_test.cpp | 519 +++++++++++++++++++ 5 files changed, 666 insertions(+), 25 deletions(-) diff --git a/include/xrpl/tx/invariants/InvariantCheck.h b/include/xrpl/tx/invariants/InvariantCheck.h index bb51105de2..150c0ed510 100644 --- a/include/xrpl/tx/invariants/InvariantCheck.h +++ b/include/xrpl/tx/invariants/InvariantCheck.h @@ -316,17 +316,26 @@ public: }; /** - * @brief Invariant: Token holder's trustline balance cannot be negative after - * Clawback. + * @brief Invariant: Token holder's trustline/MPT balance cannot be invalid + * after Clawback. * * We iterate all the trust lines affected by this transaction and ensure * that no more than one trustline is modified, and also holder's balance is - * non-negative. + * non-negative. When featureMPTokensV2 is enabled, also verify the holder's + * raw trustline/MPToken balance decreased by the clawed amount. */ class ValidClawback { + struct EntryChange + { + SLE::const_pointer before; + SLE::const_pointer after; + }; + std::uint32_t trustlinesChanged_ = 0; std::uint32_t mptokensChanged_ = 0; + EntryChange iou_; + EntryChange mpt_; public: void @@ -440,7 +449,7 @@ using InvariantChecks = std::tuple< ValidLoan, ValidVault, ValidConfidentialMPToken, - ValidMPTPayment, + ValidMPTBalanceChanges, ValidAmounts, ValidMPTTransfer, ObjectHasPseudoAccount, diff --git a/include/xrpl/tx/invariants/MPTInvariant.h b/include/xrpl/tx/invariants/MPTInvariant.h index 9a546fa400..5740cd5be2 100644 --- a/include/xrpl/tx/invariants/MPTInvariant.h +++ b/include/xrpl/tx/invariants/MPTInvariant.h @@ -87,7 +87,7 @@ public: * OutstandingAmount after application equals OutstandingAmount before * application plus the net holder balance delta. */ -class ValidMPTPayment +class ValidMPTBalanceChanges { enum class Order { Before = 0, After = 1 }; struct MPTData diff --git a/src/libxrpl/tx/invariants/InvariantCheck.cpp b/src/libxrpl/tx/invariants/InvariantCheck.cpp index 220a7a8e41..1032644132 100644 --- a/src/libxrpl/tx/invariants/InvariantCheck.cpp +++ b/src/libxrpl/tx/invariants/InvariantCheck.cpp @@ -799,14 +799,49 @@ ValidNewAccountRoot::finalize( //------------------------------------------------------------------------------ +static std::optional +clawbackTrustLineBalanceInHolderTerms( + SLE::const_pointer const& sle, + AccountID const& holder, + AccountID const& issuer, + Currency const& currency) +{ + if (!sle) + return STAmount{Issue{currency, issuer}}; + + if (sle->getType() != ltRIPPLE_STATE || + sle->key() != keylet::trustLine(holder, issuer, currency).key) + { + return std::nullopt; + } + + STAmount balance = sle->getFieldAmount(sfBalance); + if (holder > issuer) + balance.negate(); + balance.get().account = issuer; + return balance; +} + void -ValidClawback::visitEntry(bool, SLE::const_ref before, SLE::const_ref) +ValidClawback::visitEntry(bool isDelete, SLE::const_ref before, SLE::const_ref after) { if (before && before->getType() == ltRIPPLE_STATE) + { trustlinesChanged_++; + iou_.before = before; + } + + if (!isDelete && after && after->getType() == ltRIPPLE_STATE) + iou_.after = after; if (before && before->getType() == ltMPTOKEN) + { mptokensChanged_++; + mpt_.before = before; + } + + if (!isDelete && after && after->getType() == ltMPTOKEN) + mpt_.after = after; } bool @@ -835,31 +870,109 @@ ValidClawback::finalize( } bool const mptV2Enabled = view.rules().enabled(featureMPTokensV2); + if (trustlinesChanged_ != 0 && mptokensChanged_ != 0) + { + JLOG(j.fatal()) << "Invariant failed: trustline and MPToken both changed."; + if (mptV2Enabled) + return false; + } + if (trustlinesChanged_ == 1 || (mptV2Enabled && mptokensChanged_ == 1)) { - AccountID const issuer = tx.getAccountID(sfAccount); STAmount const& amount = tx.getFieldAmount(sfAmount); - AccountID const& holder = amount.getIssuer(); - STAmount const holderBalance = amount.asset().visit( + + return amount.asset().visit( [&](Issue const& issue) { - return accountHolds( + AccountID const issuer = tx.getAccountID(sfAccount); + AccountID const& holder = amount.getIssuer(); + STAmount const holderBalance = accountHolds( view, holder, issue.currency, issuer, FreezeHandling::IgnoreFreeze, j); + + if (holderBalance.signum() < 0) + { + JLOG(j.fatal()) << "Invariant failed: trustline or MPT balance is negative"; + return false; + } + + if (!iou_.before) + { + JLOG(j.fatal()) + << "Invariant failed: trustline clawback changed the wrong line"; + return !mptV2Enabled; + } + + auto const beforeBalance = clawbackTrustLineBalanceInHolderTerms( + iou_.before, holder, issuer, issue.currency); + auto const afterBalance = clawbackTrustLineBalanceInHolderTerms( + iou_.after, holder, issuer, issue.currency); + if (!beforeBalance || !afterBalance) + { + JLOG(j.fatal()) + << "Invariant failed: trustline clawback changed the wrong line"; + return !mptV2Enabled; + } + + STAmount clawAmount = amount; + clawAmount.get().account = issuer; + if (clawAmount <= beast::kZero) + { + JLOG(j.fatal()) << "Invariant failed: trustline clawback amount is invalid"; + return !mptV2Enabled; + } + + if (*afterBalance > *beforeBalance || + (*beforeBalance - *afterBalance) != std::min(*beforeBalance, clawAmount)) + { + JLOG(j.fatal()) + << "Invariant failed: trustline clawback balance change is invalid"; + return !mptV2Enabled; + } + + return true; }, [&](MPTIssue const& issue) { - return accountHolds( - view, - holder, - issue, - FreezeHandling::IgnoreFreeze, - AuthHandling::IgnoreAuth, - j); - }); + auto const holder = tx[~sfHolder]; + if (!holder) + { + JLOG(j.fatal()) << "Invariant failed: MPT clawback missing holder"; + return !mptV2Enabled; + } - if (holderBalance.signum() < 0) - { - JLOG(j.fatal()) << "Invariant failed: trustline or MPT balance is negative"; - return false; - } + if (!mpt_.before || !mpt_.after) + { + JLOG(j.fatal()) << "Invariant failed: MPT clawback token is missing"; + return !mptV2Enabled; + } + + if (mpt_.before->getAccountID(sfAccount) != *holder || + mpt_.after->getAccountID(sfAccount) != *holder || + (*mpt_.before)[sfMPTokenIssuanceID] != issue.getMptID() || + (*mpt_.after)[sfMPTokenIssuanceID] != issue.getMptID()) + { + JLOG(j.fatal()) << "Invariant failed: MPT clawback changed the wrong token"; + return !mptV2Enabled; + } + + auto const before = mpt_.before->getFieldU64(sfMPTAmount); + auto const after = mpt_.after->getFieldU64(sfMPTAmount); + if (amount.negative() || amount.mantissa() == 0) + { + JLOG(j.fatal()) << "Invariant failed: MPT clawback amount is invalid"; + return !mptV2Enabled; + } + auto const clawAmount = amount.mantissa(); + + // MPT balances are unsigned, so validate the raw holder + // debit instead of routing through accountHolds(). + if (after > before || (before - after) != std::min(before, clawAmount)) + { + JLOG(j.fatal()) + << "Invariant failed: MPT clawback balance change is invalid"; + return !mptV2Enabled; + } + + return true; + }); } } else diff --git a/src/libxrpl/tx/invariants/MPTInvariant.cpp b/src/libxrpl/tx/invariants/MPTInvariant.cpp index d8f7fcc27d..77c5ad781e 100644 --- a/src/libxrpl/tx/invariants/MPTInvariant.cpp +++ b/src/libxrpl/tx/invariants/MPTInvariant.cpp @@ -403,7 +403,7 @@ ValidMPTIssuance::finalize( } void -ValidMPTPayment::visitEntry(bool, SLE::const_ref before, SLE::const_ref after) +ValidMPTBalanceChanges::visitEntry(bool, SLE::const_ref before, SLE::const_ref after) { if (overflow_) return; @@ -465,7 +465,7 @@ ValidMPTPayment::visitEntry(bool, SLE::const_ref before, SLE::const_ref after) } bool -ValidMPTPayment::finalize( +ValidMPTBalanceChanges::finalize( STTx const& tx, TER const result, XRPAmount const, diff --git a/src/test/app/Invariants_test.cpp b/src/test/app/Invariants_test.cpp index 569eecb27b..9c90ade72f 100644 --- a/src/test/app/Invariants_test.cpp +++ b/src/test/app/Invariants_test.cpp @@ -4441,6 +4441,525 @@ class Invariants_test : public beast::unit_test::Suite return true; }); + // Invalid IOU clawback delta must fail once MPTokensV2 enforces before/after validation. + { + Env env(*this, defaultAmendments()); + Account const issuer{"issuer"}; + Account const holder{"holder"}; + Account const other{"other"}; + env.fund(XRP(1'000), issuer, holder, other); + auto const usd = issuer["USD"]; + env.trust(usd(100), holder); + env(pay(issuer, holder, usd(100))); + env.close(); + + doInvariantCheck( + std::move(env), + holder, + other, + {{"Invariant failed: trustline clawback balance change is invalid"}}, + [issuer, usd](Account const& holder, Account const&, ApplyContext& ac) { + auto sle = + ac.view().peek(keylet::trustLine(holder.id(), issuer.id(), usd.currency)); + if (!sle) + return false; + + STAmount balance{Issue{usd.currency, issuer.id()}, 80}; + if (holder.id() > issuer.id()) + balance.negate(); + sle->setFieldAmount(sfBalance, balance); + ac.view().update(sle); + return true; + }, + XRPAmount{}, + STTx{ + ttCLAWBACK, + [&](STObject& tx) { + tx[sfAccount] = issuer.id(); + tx[sfAmount] = STAmount{Issue{usd.currency, holder.id()}, 10}; + }}, + {tecINVARIANT_FAILED, tefINVARIANT_FAILED}); + } + + // Full IOU clawback may delete the trustline; missing after-SLE represents zero balance. + { + Env env(*this, defaultAmendments()); + Account const issuer{"issuer"}; + Account const holder{"holder"}; + Account const other{"other"}; + env.fund(XRP(1'000), issuer, holder, other); + auto const usd = issuer["USD"]; + env.trust(usd(100), holder); + env(pay(issuer, holder, usd(100))); + env.close(); + + doInvariantCheck( + std::move(env), + holder, + other, + {}, + [issuer, usd](Account const& holder, Account const&, ApplyContext& ac) { + auto const sle = + ac.view().peek(keylet::trustLine(holder.id(), issuer.id(), usd.currency)); + if (!sle) + return false; + + ac.view().erase(sle); + return true; + }, + XRPAmount{}, + STTx{ + ttCLAWBACK, + [&](STObject& tx) { + tx[sfAccount] = issuer.id(); + tx[sfAmount] = STAmount{Issue{usd.currency, holder.id()}, 100}; + }}, + {tesSUCCESS, tesSUCCESS}); + } + + // Pre-MPTokensV2 invalid IOU clawback delta logs but remains non-enforcing. + { + Env env(*this, defaultAmendments() - featureMPTokensV2); + Account const issuer{"issuer"}; + Account const holder{"holder"}; + Account const other{"other"}; + env.fund(XRP(1'000), issuer, holder, other); + auto const usd = issuer["USD"]; + env.trust(usd(100), holder); + env(pay(issuer, holder, usd(100))); + env.close(); + + doInvariantCheck( + std::move(env), + holder, + other, + {{"Invariant failed: trustline clawback balance change is invalid"}}, + [issuer, usd](Account const& holder, Account const&, ApplyContext& ac) { + auto sle = + ac.view().peek(keylet::trustLine(holder.id(), issuer.id(), usd.currency)); + if (!sle) + return false; + + STAmount balance{Issue{usd.currency, issuer.id()}, 80}; + if (holder.id() > issuer.id()) + balance.negate(); + sle->setFieldAmount(sfBalance, balance); + ac.view().update(sle); + return true; + }, + XRPAmount{}, + STTx{ + ttCLAWBACK, + [&](STObject& tx) { + tx[sfAccount] = issuer.id(); + tx[sfAmount] = STAmount{Issue{usd.currency, holder.id()}, 10}; + }}, + {tesSUCCESS, tesSUCCESS}); + } + + // Invalid MPT clawback delta must fail when raw MPToken debit mismatches sfAmount. + { + Env env(*this, defaultAmendments()); + Account const issuer{"issuer"}; + Account const holder{"holder"}; + Account const other{"other"}; + env.fund(XRP(1'000), issuer, holder, other); + MPTTester const mpt( + {.env = env, .issuer = issuer, .holders = {holder}, .pay = 100, .maxAmt = 100}); + auto const id = mpt.issuanceID(); + + doInvariantCheck( + std::move(env), + holder, + other, + {{"Invariant failed: MPT clawback balance change is invalid"}}, + [id](Account const& holder, Account const&, ApplyContext& ac) { + auto const sleToken = ac.view().peek(keylet::mptoken(id, holder)); + auto const sleIssuance = ac.view().peek(keylet::mptokenIssuance(id)); + if (!sleToken || !sleIssuance) + return false; + + sleToken->setFieldU64(sfMPTAmount, 80); + sleIssuance->setFieldU64(sfOutstandingAmount, 80); + ac.view().update(sleToken); + ac.view().update(sleIssuance); + return true; + }, + XRPAmount{}, + STTx{ + ttCLAWBACK, + [&](STObject& tx) { + tx[sfAccount] = issuer.id(); + tx[sfHolder] = holder.id(); + tx[sfAmount] = STAmount{MPTIssue{id}, 10}; + }}, + {tecINVARIANT_FAILED, tefINVARIANT_FAILED}); + } + + // A clawback that mutates both IOU and MPT entries must fail under MPTokensV2. + { + Env env(*this, defaultAmendments()); + Account const issuer{"issuer"}; + Account const holder{"holder"}; + Account const other{"other"}; + env.fund(XRP(1'000), issuer, holder, other); + auto const usd = issuer["USD"]; + env.trust(usd(100), holder); + env(pay(issuer, holder, usd(100))); + MPTTester const mpt( + {.env = env, .issuer = issuer, .holders = {holder}, .pay = 100, .maxAmt = 100}); + auto const id = mpt.issuanceID(); + + doInvariantCheck( + std::move(env), + holder, + other, + {{"Invariant failed: trustline and MPToken both changed"}}, + [issuer, usd, id](Account const& holder, Account const&, ApplyContext& ac) { + auto const sleLine = + ac.view().peek(keylet::trustLine(holder.id(), issuer.id(), usd.currency)); + auto const sleToken = ac.view().peek(keylet::mptoken(id, holder.id())); + auto const sleIssuance = ac.view().peek(keylet::mptokenIssuance(id)); + if (!sleLine || !sleToken || !sleIssuance) + return false; + + STAmount balance{Issue{usd.currency, issuer.id()}, 90}; + if (holder.id() > issuer.id()) + balance.negate(); + sleLine->setFieldAmount(sfBalance, balance); + sleToken->setFieldU64(sfMPTAmount, 90); + sleIssuance->setFieldU64(sfOutstandingAmount, 90); + ac.view().update(sleLine); + ac.view().update(sleToken); + ac.view().update(sleIssuance); + return true; + }, + XRPAmount{}, + STTx{ + ttCLAWBACK, + [&](STObject& tx) { + tx[sfAccount] = issuer.id(); + tx[sfHolder] = holder.id(); + tx[sfAmount] = STAmount{MPTIssue{id}, 10}; + }}, + {tecINVARIANT_FAILED, tefINVARIANT_FAILED}); + } + + // Clawback that modifies a trustline other than the one implied by the + // tx amount: clawbackTrustLineBalanceInHolderTerms returns nullopt for + // the mismatched line. + { + Env env(*this, defaultAmendments()); + Account const issuer{"issuer"}; + Account const holder{"holder"}; + Account const other{"other"}; + env.fund(XRP(1'000), issuer, holder, other); + auto const usd = issuer["USD"]; + auto const eur = issuer["EUR"]; + env.trust(eur(100), holder); + env(pay(issuer, holder, eur(100))); + env.close(); + + doInvariantCheck( + std::move(env), + holder, + other, + {{"Invariant failed: trustline clawback changed the wrong line"}}, + [issuer, eur](Account const& holder, Account const&, ApplyContext& ac) { + auto sle = + ac.view().peek(keylet::trustLine(holder.id(), issuer.id(), eur.currency)); + if (!sle) + return false; + STAmount balance{Issue{eur.currency, issuer.id()}, 90}; + if (holder.id() > issuer.id()) + balance.negate(); + sle->setFieldAmount(sfBalance, balance); + ac.view().update(sle); + return true; + }, + XRPAmount{}, + STTx{ + ttCLAWBACK, + [&](STObject& tx) { + tx[sfAccount] = issuer.id(); + tx[sfAmount] = STAmount{Issue{usd.currency, holder.id()}, 10}; + }}, + {tecINVARIANT_FAILED, tefINVARIANT_FAILED}); + } + + // Clawback leaving the holder's balance negative. + { + Env env(*this, defaultAmendments()); + Account const issuer{"issuer"}; + Account const holder{"holder"}; + Account const other{"other"}; + env.fund(XRP(1'000), issuer, holder, other); + auto const usd = issuer["USD"]; + env.trust(usd(100), holder); + env(pay(issuer, holder, usd(100))); + env.close(); + + doInvariantCheck( + std::move(env), + holder, + other, + {{"Invariant failed: trustline or MPT balance is negative"}}, + [issuer, usd](Account const& holder, Account const&, ApplyContext& ac) { + auto sle = + ac.view().peek(keylet::trustLine(holder.id(), issuer.id(), usd.currency)); + if (!sle) + return false; + // Make the holder's balance negative from their perspective. + STAmount balance{Issue{usd.currency, issuer.id()}, 80}; + if (holder.id() < issuer.id()) + balance.negate(); + sle->setFieldAmount(sfBalance, balance); + ac.view().update(sle); + return true; + }, + XRPAmount{}, + STTx{ + ttCLAWBACK, + [&](STObject& tx) { + tx[sfAccount] = issuer.id(); + tx[sfAmount] = STAmount{Issue{usd.currency, holder.id()}, 10}; + }}, + {tecINVARIANT_FAILED, tefINVARIANT_FAILED}); + } + + // IOU-amount clawback while only an MPToken changed: no trustline was + // recorded, so iou_.before is empty. + { + Env env(*this, defaultAmendments()); + Account const issuer{"issuer"}; + Account const holder{"holder"}; + Account const other{"other"}; + env.fund(XRP(1'000), issuer, holder, other); + auto const usd = issuer["USD"]; + MPTTester const mpt( + {.env = env, .issuer = issuer, .holders = {holder}, .pay = 100, .maxAmt = 100}); + auto const id = mpt.issuanceID(); + + doInvariantCheck( + std::move(env), + holder, + other, + {{"Invariant failed: trustline clawback changed the wrong line"}}, + [id](Account const& holder, Account const&, ApplyContext& ac) { + auto const sleToken = ac.view().peek(keylet::mptoken(id, holder)); + auto const sleIssuance = ac.view().peek(keylet::mptokenIssuance(id)); + if (!sleToken || !sleIssuance) + return false; + sleToken->setFieldU64(sfMPTAmount, 90); + sleIssuance->setFieldU64(sfOutstandingAmount, 90); + ac.view().update(sleToken); + ac.view().update(sleIssuance); + return true; + }, + XRPAmount{}, + STTx{ + ttCLAWBACK, + [&](STObject& tx) { + tx[sfAccount] = issuer.id(); + tx[sfAmount] = STAmount{Issue{usd.currency, holder.id()}, 10}; + }}, + {tecINVARIANT_FAILED, tefINVARIANT_FAILED}); + } + + // Valid trustline change but a zero clawback amount. + { + Env env(*this, defaultAmendments()); + Account const issuer{"issuer"}; + Account const holder{"holder"}; + Account const other{"other"}; + env.fund(XRP(1'000), issuer, holder, other); + auto const usd = issuer["USD"]; + env.trust(usd(100), holder); + env(pay(issuer, holder, usd(100))); + env.close(); + + doInvariantCheck( + std::move(env), + holder, + other, + {{"Invariant failed: trustline clawback amount is invalid"}}, + [issuer, usd](Account const& holder, Account const&, ApplyContext& ac) { + auto sle = + ac.view().peek(keylet::trustLine(holder.id(), issuer.id(), usd.currency)); + if (!sle) + return false; + STAmount balance{Issue{usd.currency, issuer.id()}, 90}; + if (holder.id() > issuer.id()) + balance.negate(); + sle->setFieldAmount(sfBalance, balance); + ac.view().update(sle); + return true; + }, + XRPAmount{}, + STTx{ + ttCLAWBACK, + [&](STObject& tx) { + tx[sfAccount] = issuer.id(); + tx[sfAmount] = STAmount{Issue{usd.currency, holder.id()}, 0}; + }}, + {tecINVARIANT_FAILED, tefINVARIANT_FAILED}); + } + + // MPT clawback tx missing the Holder field. + { + Env env(*this, defaultAmendments()); + Account const issuer{"issuer"}; + Account const holder{"holder"}; + Account const other{"other"}; + env.fund(XRP(1'000), issuer, holder, other); + MPTTester const mpt( + {.env = env, .issuer = issuer, .holders = {holder}, .pay = 100, .maxAmt = 100}); + auto const id = mpt.issuanceID(); + + doInvariantCheck( + std::move(env), + holder, + other, + {{"Invariant failed: MPT clawback missing holder"}}, + [id](Account const& holder, Account const&, ApplyContext& ac) { + auto const sleToken = ac.view().peek(keylet::mptoken(id, holder)); + auto const sleIssuance = ac.view().peek(keylet::mptokenIssuance(id)); + if (!sleToken || !sleIssuance) + return false; + sleToken->setFieldU64(sfMPTAmount, 90); + sleIssuance->setFieldU64(sfOutstandingAmount, 90); + ac.view().update(sleToken); + ac.view().update(sleIssuance); + return true; + }, + XRPAmount{}, + STTx{ + ttCLAWBACK, + [&](STObject& tx) { + tx[sfAccount] = issuer.id(); + tx[sfAmount] = STAmount{MPTIssue{id}, 10}; + }}, + {tecINVARIANT_FAILED, tefINVARIANT_FAILED}); + } + + // MPT clawback where the holder's MPToken was deleted (after is empty). + { + Env env(*this, defaultAmendments()); + Account const issuer{"issuer"}; + Account const holder{"holder"}; + Account const other{"other"}; + env.fund(XRP(1'000), issuer, holder, other); + MPTTester const mpt( + {.env = env, .issuer = issuer, .holders = {holder}, .pay = 100, .maxAmt = 100}); + auto const id = mpt.issuanceID(); + + doInvariantCheck( + std::move(env), + holder, + other, + {{"Invariant failed: MPT clawback token is missing"}}, + [id](Account const& holder, Account const&, ApplyContext& ac) { + auto const sleToken = ac.view().peek(keylet::mptoken(id, holder)); + auto const sleIssuance = ac.view().peek(keylet::mptokenIssuance(id)); + if (!sleToken || !sleIssuance) + return false; + // Keep the issuance consistent after removing the token. + sleIssuance->setFieldU64(sfOutstandingAmount, 0); + ac.view().update(sleIssuance); + ac.view().erase(sleToken); + return true; + }, + XRPAmount{}, + STTx{ + ttCLAWBACK, + [&](STObject& tx) { + tx[sfAccount] = issuer.id(); + tx[sfHolder] = holder.id(); + tx[sfAmount] = STAmount{MPTIssue{id}, 10}; + }}, + {tecINVARIANT_FAILED, tefINVARIANT_FAILED}); + } + + // MPT clawback that changed a different holder's MPToken. + { + Env env(*this, defaultAmendments()); + Account const issuer{"issuer"}; + Account const holder{"holder"}; + Account const other{"other"}; + env.fund(XRP(1'000), issuer, holder, other); + MPTTester const mpt( + {.env = env, + .issuer = issuer, + .holders = {holder, other}, + .pay = 100, + .maxAmt = 200}); + auto const id = mpt.issuanceID(); + + doInvariantCheck( + std::move(env), + holder, + other, + {{"Invariant failed: MPT clawback changed the wrong token"}}, + [id](Account const&, Account const& other, ApplyContext& ac) { + auto const sleToken = ac.view().peek(keylet::mptoken(id, other)); + auto const sleIssuance = ac.view().peek(keylet::mptokenIssuance(id)); + if (!sleToken || !sleIssuance) + return false; + sleToken->setFieldU64(sfMPTAmount, 90); + sleIssuance->setFieldU64(sfOutstandingAmount, 190); + ac.view().update(sleToken); + ac.view().update(sleIssuance); + return true; + }, + XRPAmount{}, + STTx{ + ttCLAWBACK, + [&](STObject& tx) { + tx[sfAccount] = issuer.id(); + tx[sfHolder] = holder.id(); + tx[sfAmount] = STAmount{MPTIssue{id}, 10}; + }}, + {tecINVARIANT_FAILED, tefINVARIANT_FAILED}); + } + + // Valid MPToken change but a zero MPT clawback amount. + { + Env env(*this, defaultAmendments()); + Account const issuer{"issuer"}; + Account const holder{"holder"}; + Account const other{"other"}; + env.fund(XRP(1'000), issuer, holder, other); + MPTTester const mpt( + {.env = env, .issuer = issuer, .holders = {holder}, .pay = 100, .maxAmt = 100}); + auto const id = mpt.issuanceID(); + + doInvariantCheck( + std::move(env), + holder, + other, + {{"Invariant failed: MPT clawback amount is invalid"}}, + [id](Account const& holder, Account const&, ApplyContext& ac) { + auto const sleToken = ac.view().peek(keylet::mptoken(id, holder)); + auto const sleIssuance = ac.view().peek(keylet::mptokenIssuance(id)); + if (!sleToken || !sleIssuance) + return false; + sleToken->setFieldU64(sfMPTAmount, 90); + sleIssuance->setFieldU64(sfOutstandingAmount, 90); + ac.view().update(sleToken); + ac.view().update(sleIssuance); + return true; + }, + XRPAmount{}, + STTx{ + ttCLAWBACK, + [&](STObject& tx) { + tx[sfAccount] = issuer.id(); + tx[sfHolder] = holder.id(); + tx[sfAmount] = STAmount{MPTIssue{id}, 0}; + }}, + {tecINVARIANT_FAILED, tefINVARIANT_FAILED}); + } + // More MPTokens created than expected std::array, 4> const tests = { std::make_pair(ttAMM_WITHDRAW, 2), From 0dc942508e207dc78292ac95ee092ca85a147e54 Mon Sep 17 00:00:00 2001 From: Ayaz Salikhov Date: Tue, 14 Jul 2026 17:09:20 +0100 Subject: [PATCH 10/13] ci: Run full matrix only on `Ready to merge` or `Full CI build` labeled PRs (#7689) Co-authored-by: Bart --- .github/scripts/strategy-matrix/generate.py | 62 +++++++------------ .github/scripts/strategy-matrix/linux.json | 31 ++++++++-- .github/scripts/strategy-matrix/macos.json | 5 +- .github/scripts/strategy-matrix/windows.json | 4 +- .github/workflows/on-pr.yml | 55 +++++++++++++--- .../workflows/reusable-strategy-matrix.yml | 7 ++- 6 files changed, 104 insertions(+), 60 deletions(-) diff --git a/.github/scripts/strategy-matrix/generate.py b/.github/scripts/strategy-matrix/generate.py index a269cb25d4..c783f32fb7 100755 --- a/.github/scripts/strategy-matrix/generate.py +++ b/.github/scripts/strategy-matrix/generate.py @@ -25,24 +25,16 @@ def get_cmake_args(build_type: str, extra_args: str) -> str: return " ".join(args) -def runs_on_event(exclude_event_types: list[str], event: str | None) -> bool: - """Whether a config should run for the current event. - - 'exclude_event_types' is a list of GitHub event names (e.g. - ["pull_request"]) on which the config should NOT run; an empty list means - the config runs on every event. When no event is given (event is None), no - filtering is applied. - """ - if event is None: - return True - return event not in exclude_event_types - - # --------------------------------------------------------------------------- # Input types — shapes of the JSON config files # --------------------------------------------------------------------------- +# Every config must declare 'minimal'. Minimal configs form the reduced matrix +# built for pull requests by default; the full matrix adds the rest. Packaging +# configs declare it too, but packaging is gated in the workflow, not by it. + + @dataclasses.dataclass class LinuxConfig: """One entry in linux.json's 'configs' or 'package_configs' arrays.""" @@ -50,13 +42,11 @@ class LinuxConfig: compiler: list[str] build_type: list[str] arch: list[str] + minimal: bool sanitizers: list[str] = dataclasses.field(default_factory=list) suffix: str = "" extra_cmake_args: str = "" image: str = "" # only used by package_configs entries - # List of GitHub event names (e.g. "pull_request") on which this config - # should NOT run. Empty means it runs on every event. - exclude_event_types: list[str] = dataclasses.field(default_factory=list) @dataclasses.dataclass @@ -89,11 +79,9 @@ class PlatformConfig: """One entry in macos.json's or windows.json's 'configs' array.""" build_type: list[str] + minimal: bool build_only: bool = False # if true, skip tests (e.g. macos/Windows Debug) extra_cmake_args: str = "" - # List of GitHub event names (e.g. "pull_request") on which this config - # should NOT run. Empty means it runs on every event. - exclude_event_types: list[str] = dataclasses.field(default_factory=list) def __post_init__(self) -> None: if isinstance(self.build_type, str): @@ -168,20 +156,18 @@ _ARCHS: dict[str, Architecture] = { } -def expand_linux_matrix( - linux: LinuxFile, event: str | None = None -) -> list[MatrixEntry]: +def expand_linux_matrix(linux: LinuxFile, minimal: bool) -> list[MatrixEntry]: """Expand a LinuxFile into a flat list of matrix entries. Each config entry is expanded over the cross-product of its - compiler, build_type, sanitizers, and architecture lists. Configs that - exclude the current event are skipped. + compiler, build_type, sanitizers, and architecture lists. When 'minimal' is + true, only configs flagged as minimal are included. """ entries: list[MatrixEntry] = [] for distro, configs in linux.configs.items(): for cfg in configs: - if not runs_on_event(cfg.exclude_event_types, event): + if minimal and not cfg.minimal: continue # An empty sanitizers list means "one entry with no sanitizer". effective_sanitizers = cfg.sanitizers or [""] @@ -240,19 +226,17 @@ def expand_linux_packaging(linux: LinuxFile) -> list[PackagingEntry]: return entries -def expand_platform_matrix( - pf: PlatformFile, event: str | None = None -) -> list[MatrixEntry]: +def expand_platform_matrix(pf: PlatformFile, minimal: bool) -> list[MatrixEntry]: """Expand a PlatformFile (macOS or Windows) into matrix entries. - Configs that exclude the current event are skipped. + When 'minimal' is true, only configs flagged as minimal are included. """ platform_name, arch = pf.platform.split("/") is_windows = platform_name == "windows" entries: list[MatrixEntry] = [] for cfg in pf.configs: - if not runs_on_event(cfg.exclude_event_types, event): + if minimal and not cfg.minimal: continue for build_type in cfg.build_type: entries.append( @@ -292,12 +276,12 @@ if __name__ == "__main__": action="store_true", ) parser.add_argument( - "-e", - "--event", - help="The GitHub event name that triggered the workflow (e.g. 'push', " - "'pull_request'). Configs are filtered by their 'event_type'. If " - "omitted, no filtering is applied.", - default=None, + "-m", + "--minimal", + help="Emit only the minimal matrix (the configs flagged 'minimal'), " + "used for pull requests by default. If omitted, the full matrix is " + "emitted.", + action="store_true", ) args = parser.parse_args() @@ -308,15 +292,15 @@ if __name__ == "__main__": else: if args.config in ("linux", None): matrix += expand_linux_matrix( - LinuxFile.load(THIS_DIR / "linux.json"), args.event + LinuxFile.load(THIS_DIR / "linux.json"), args.minimal ) if args.config in ("macos", None): matrix += expand_platform_matrix( - PlatformFile.load(THIS_DIR / "macos.json"), args.event + PlatformFile.load(THIS_DIR / "macos.json"), args.minimal ) if args.config in ("windows", None): matrix += expand_platform_matrix( - PlatformFile.load(THIS_DIR / "windows.json"), args.event + PlatformFile.load(THIS_DIR / "windows.json"), args.minimal ) print(f"matrix={json.dumps({'include': [dataclasses.asdict(e) for e in matrix]})}") diff --git a/.github/scripts/strategy-matrix/linux.json b/.github/scripts/strategy-matrix/linux.json index 863b910dda..03ac1c6334 100644 --- a/.github/scripts/strategy-matrix/linux.json +++ b/.github/scripts/strategy-matrix/linux.json @@ -2,16 +2,30 @@ "image_tag": "sha-e29b523", "configs": { "ubuntu": [ + { + "compiler": ["clang"], + "build_type": ["Release"], + "arch": ["amd64"], + "minimal": true + }, + { + "compiler": ["gcc"], + "build_type": ["Release"], + "arch": ["amd64"], + "minimal": false + }, { "compiler": ["gcc", "clang"], "build_type": ["Debug", "Release"], - "arch": ["amd64", "arm64"] + "arch": ["arm64"], + "minimal": false }, { "compiler": ["gcc", "clang"], "build_type": ["Debug", "Release"], "arch": ["amd64"], + "minimal": false, "sanitizers": ["address", "undefinedbehavior"] }, @@ -19,6 +33,7 @@ "compiler": ["gcc"], "build_type": ["Debug"], "arch": ["amd64"], + "minimal": true, "suffix": "coverage", "extra_cmake_args": "-DUNIT_TEST_REFERENCE_FEE=500 -Dcoverage=ON -Dcoverage_format=xml -DCODE_COVERAGE_VERBOSE=ON -DCMAKE_C_FLAGS=-O0 -DCMAKE_CXX_FLAGS=-O0" }, @@ -26,6 +41,7 @@ "compiler": ["clang"], "build_type": ["Debug"], "arch": ["amd64"], + "minimal": false, "suffix": "voidstar", "extra_cmake_args": "-Dvoidstar=ON" }, @@ -33,6 +49,7 @@ "compiler": ["clang"], "build_type": ["Release"], "arch": ["amd64"], + "minimal": false, "suffix": "reffee", "extra_cmake_args": "-DUNIT_TEST_REFERENCE_FEE=1000" }, @@ -40,9 +57,9 @@ "compiler": ["gcc"], "build_type": ["Debug"], "arch": ["amd64"], + "minimal": false, "suffix": "unity", - "extra_cmake_args": "-Dunity=ON", - "exclude_event_types": ["pull_request"] + "extra_cmake_args": "-Dunity=ON" } ], @@ -50,7 +67,8 @@ { "compiler": ["gcc"], "build_type": ["Release"], - "arch": ["amd64"] + "arch": ["amd64"], + "minimal": false } ], @@ -58,7 +76,8 @@ { "compiler": ["gcc"], "build_type": ["Release"], - "arch": ["amd64"] + "arch": ["amd64"], + "minimal": false } ] }, @@ -68,6 +87,7 @@ "compiler": ["gcc"], "build_type": ["Release"], "arch": ["amd64"], + "minimal": false, "image": "ghcr.io/xrplf/xrpld/packaging-debian:sha-577d745" } ], @@ -77,6 +97,7 @@ "compiler": ["gcc"], "build_type": ["Release"], "arch": ["amd64"], + "minimal": false, "image": "ghcr.io/xrplf/xrpld/packaging-rhel:sha-577d745" } ] diff --git a/.github/scripts/strategy-matrix/macos.json b/.github/scripts/strategy-matrix/macos.json index 2d3cc75c7b..98e0f13141 100644 --- a/.github/scripts/strategy-matrix/macos.json +++ b/.github/scripts/strategy-matrix/macos.json @@ -4,13 +4,14 @@ "configs": [ { "build_type": "Release", - "extra_cmake_args": "-DCMAKE_POLICY_VERSION_MINIMUM=3.5" + "extra_cmake_args": "-DCMAKE_POLICY_VERSION_MINIMUM=3.5", + "minimal": true }, { "build_type": "Debug", "extra_cmake_args": "-DCMAKE_POLICY_VERSION_MINIMUM=3.5", "build_only": true, - "exclude_event_types": ["pull_request"] + "minimal": false } ] } diff --git a/.github/scripts/strategy-matrix/windows.json b/.github/scripts/strategy-matrix/windows.json index 370e9f5bc7..6b926e85f5 100644 --- a/.github/scripts/strategy-matrix/windows.json +++ b/.github/scripts/strategy-matrix/windows.json @@ -2,11 +2,11 @@ "platform": "windows/amd64", "runner": ["self-hosted", "Windows", "dev-box-windows-2026"], "configs": [ - { "build_type": "Release" }, + { "build_type": "Release", "minimal": true }, { "build_type": "Debug", "build_only": true, - "exclude_event_types": ["pull_request"] + "minimal": false } ] } diff --git a/.github/workflows/on-pr.yml b/.github/workflows/on-pr.yml index 19fb170b92..bbf6f8c39e 100644 --- a/.github/workflows/on-pr.yml +++ b/.github/workflows/on-pr.yml @@ -1,7 +1,11 @@ -# This workflow runs all workflows to check, build and test the project on -# various Linux flavors, as well as on MacOS and Windows, on every push to a -# user branch. However, it will not run if the pull request is a draft unless it -# has the 'DraftRunCI' label. For commits to PRs that target a release branch, +# This workflow runs workflows to check, build and test the project +# on every meaningful change on pull_request. +# However, it will not run if the PR is a draft +# unless it has the 'DraftRunCI' or 'Full CI build' label. +# +# By default a PR builds only a minimal matrix. +# The full matrix runs once the PR is labeled "Ready to merge" or "Full CI build". +# For commits to PRs that target a release branch, # it also uploads the libxrpl recipe to the Conan remote. name: PR @@ -15,9 +19,24 @@ on: - reopened - synchronize - ready_for_review + # Trigger on label changes so toggling "Ready to merge" or "Full CI build" + # switches between the minimal and full matrix without needing a new push. + - labeled + - unlabeled concurrency: - group: ${{ github.workflow }}-${{ github.ref }} + # Use a per-ref group so a newer run (a push, or a change to a label below) + # supersedes the in-progress one for that ref. Label events we don't act on get + # their own unique group (per run id) instead, keeping them out of the shared + # group so real builds keep running. Keep this list in sync with `should-run`. + group: >- + ${{ github.workflow }}-${{ github.ref }}${{ + ((github.event.action == 'labeled' || github.event.action == 'unlabeled') + && github.event.label.name != 'Ready to merge' + && github.event.label.name != 'DraftRunCI' + && github.event.label.name != 'Full CI build') + && format('-{0}', github.run_id) || '' + }} cancel-in-progress: true defaults: @@ -26,10 +45,21 @@ defaults: jobs: # This job determines whether the rest of the workflow should run. It runs - # when the PR is not a draft (which should also cover merge-group) or - # has the 'DraftRunCI' label. + # when the PR is not a draft (which should also cover merge-group) or has the + # 'DraftRunCI' or 'Full CI build' label. For label events it only runs when the + # label added or removed is one we act on ('Ready to merge', 'DraftRunCI' or + # 'Full CI build'), so unrelated label changes do not trigger a redundant run. should-run: - if: ${{ !github.event.pull_request.draft || contains(github.event.pull_request.labels.*.name, 'DraftRunCI') }} + if: >- + ${{ + ((github.event.action != 'labeled' && github.event.action != 'unlabeled') + || github.event.label.name == 'Ready to merge' + || github.event.label.name == 'DraftRunCI' + || github.event.label.name == 'Full CI build') + && (!github.event.pull_request.draft + || contains(github.event.pull_request.labels.*.name, 'DraftRunCI') + || contains(github.event.pull_request.labels.*.name, 'Full CI build')) + }} runs-on: ubuntu-latest steps: - name: Checkout repository @@ -91,15 +121,17 @@ jobs: # least one of: # * Any of the files checked in the `changes` step were modified # * The PR is NOT a draft and is labeled "Ready to merge" + # * The PR is labeled "Full CI build" (draft or not) # * The workflow is running from the merge queue id: go env: FILES: ${{ steps.changes.outputs.any_changed }} DRAFT: ${{ github.event.pull_request.draft }} READY: ${{ contains(github.event.pull_request.labels.*.name, 'Ready to merge') }} + FULL: ${{ contains(github.event.pull_request.labels.*.name, 'Full CI build') }} MERGE: ${{ github.event_name == 'merge_group' }} run: | - echo "go=${{ (env.DRAFT != 'true' && env.READY == 'true') || env.FILES == 'true' || env.MERGE == 'true' }}" >>"${GITHUB_OUTPUT}" + echo "go=${{ (env.DRAFT != 'true' && env.READY == 'true') || env.FULL == 'true' || env.FILES == 'true' || env.MERGE == 'true' }}" >>"${GITHUB_OUTPUT}" cat "${GITHUB_OUTPUT}" outputs: go: ${{ steps.go.outputs.go == 'true' }} @@ -142,7 +174,10 @@ jobs: package: needs: [should-run, build-test] - if: ${{ needs.should-run.outputs.go == 'true' }} + # Packaging consumes the debian/rhel release binaries, which are only built + # by the full matrix. Skip it for pull requests that ran only the minimal + # matrix (i.e. not yet labeled "Ready to merge" or "Full CI build"). + if: ${{ needs.should-run.outputs.go == 'true' && (github.event_name != 'pull_request' || contains(github.event.pull_request.labels.*.name, 'Ready to merge') || contains(github.event.pull_request.labels.*.name, 'Full CI build')) }} uses: ./.github/workflows/reusable-package.yml upload-recipe: diff --git a/.github/workflows/reusable-strategy-matrix.yml b/.github/workflows/reusable-strategy-matrix.yml index 690aa3d423..b6091b99d9 100644 --- a/.github/workflows/reusable-strategy-matrix.yml +++ b/.github/workflows/reusable-strategy-matrix.yml @@ -35,5 +35,8 @@ jobs: id: generate env: GENERATE_CONFIG: ${{ inputs.os != '' && format('--config={0}', inputs.os) || '' }} - GENERATE_EVENT: ${{ github.event_name }} - run: ./generate.py ${GENERATE_CONFIG} --event="${GENERATE_EVENT}" >>"${GITHUB_OUTPUT}" + # Run only the minimal matrix for pull requests that are not yet + # labeled "Ready to merge" or "Full CI build". Any other event (merge + # queue, push, schedule, manual dispatch) runs the full matrix. + GENERATE_MINIMAL: ${{ (github.event_name == 'pull_request' && !contains(github.event.pull_request.labels.*.name, 'Ready to merge') && !contains(github.event.pull_request.labels.*.name, 'Full CI build')) && '--minimal' || '' }} + run: ./generate.py ${GENERATE_CONFIG} ${GENERATE_MINIMAL} >>"${GITHUB_OUTPUT}" From 0a4676d947d1e65a09c3002a2c4681895c69a413 Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Tue, 14 Jul 2026 14:16:46 -0400 Subject: [PATCH 11/13] fix: Document and assert "after" is never null in invariants (#7354) Co-authored-by: Bart --- include/xrpl/tx/invariants/InvariantCheck.h | 15 ++++-- src/libxrpl/tx/invariants/InvariantCheck.cpp | 56 +++++++++++--------- 2 files changed, 42 insertions(+), 29 deletions(-) diff --git a/include/xrpl/tx/invariants/InvariantCheck.h b/include/xrpl/tx/invariants/InvariantCheck.h index 150c0ed510..1239305e79 100644 --- a/include/xrpl/tx/invariants/InvariantCheck.h +++ b/include/xrpl/tx/invariants/InvariantCheck.h @@ -71,9 +71,18 @@ public: /** * @brief called for each ledger entry in the current transaction. * - * @param isDelete true if the SLE is being deleted - * @param before ledger entry before modification by the transaction - * @param after ledger entry after modification by the transaction + * @param isDelete true if the SLE is being deleted. + * @param before ledger entry before modification by the transaction. `before` will be null if + * the entry is new. + * @param after ledger entry after modification by the transaction. Always non-null. When + * deleting, `after` may differ from `before`. Whether that is important is up to the + * individual invariant check. + * + * @note `after` IS NEVER NULL. `isDelete` is the only correct way to check for deletions. + * Do not make logic or branching decisions on whether on `after` is set, because it will + * always be set. Treat a null `after` as a programming error (with XRPL_ASSERT). An + * invariant MAY check for null defensively, if it makes more sense, but an assertion is + * preferred for new invariants. */ void visitEntry(bool isDelete, SLE::const_ref before, SLE::const_ref after); diff --git a/src/libxrpl/tx/invariants/InvariantCheck.cpp b/src/libxrpl/tx/invariants/InvariantCheck.cpp index 1032644132..3615594d19 100644 --- a/src/libxrpl/tx/invariants/InvariantCheck.cpp +++ b/src/libxrpl/tx/invariants/InvariantCheck.cpp @@ -162,33 +162,37 @@ XRPNotCreated::visitEntry(bool isDelete, SLE::const_ref before, SLE::const_ref a } } - if (after) + if (!after) { - switch (after->getType()) - { - case ltACCOUNT_ROOT: - drops_ += (*after)[sfBalance].xrp().drops(); - break; - case ltPAYCHAN: - if (!isDelete) - drops_ += ((*after)[sfAmount] - (*after)[sfBalance]).xrp().drops(); - break; - case ltESCROW: - if (!isDelete && isXRP((*after)[sfAmount])) - drops_ += (*after)[sfAmount].xrp().drops(); - break; - case ltSPONSORSHIP: - if (!isDelete && after->isFieldPresent(sfFeeAmount)) - { - XRPL_ASSERT( - isXRP((*after)[sfFeeAmount]), - "XRPNotCreated::visitEntry : Sponsorship.FeeAmount is XRP"); - drops_ += (*after)[sfFeeAmount].xrp().drops(); - } - break; - default: - break; - } + // LCOV_EXCL_START + UNREACHABLE("xrpl::XRPNotCreated::visitEntry : after can't be null"); + return; + // LCOV_EXCL_STOP + } + switch (after->getType()) + { + case ltACCOUNT_ROOT: + drops_ += (*after)[sfBalance].xrp().drops(); + break; + case ltPAYCHAN: + if (!isDelete) + drops_ += ((*after)[sfAmount] - (*after)[sfBalance]).xrp().drops(); + break; + case ltESCROW: + if (!isDelete && isXRP((*after)[sfAmount])) + drops_ += (*after)[sfAmount].xrp().drops(); + break; + case ltSPONSORSHIP: + if (!isDelete && after->isFieldPresent(sfFeeAmount)) + { + XRPL_ASSERT( + isXRP((*after)[sfFeeAmount]), + "XRPNotCreated::visitEntry : Sponsorship.FeeAmount is XRP"); + drops_ += (*after)[sfFeeAmount].xrp().drops(); + } + break; + default: + break; } } From f10dd7b450b574109f70f651b7035e736cad7df1 Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Tue, 14 Jul 2026 14:47:41 -0400 Subject: [PATCH 12/13] fix: Handle rounding just above kMaxRep more accurately (#7389) Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: Vito Tumas <5780819+Tapanito@users.noreply.github.com> Co-authored-by: xrplf-ai-reviewer[bot] <266832837+xrplf-ai-reviewer[bot]@users.noreply.github.com> --- include/xrpl/basics/Number.h | 16 +- src/libxrpl/basics/Number.cpp | 207 +++++++--- src/libxrpl/protocol/Rules.cpp | 2 +- src/libxrpl/protocol/STNumber.cpp | 43 +- src/test/app/LoanBroker_test.cpp | 76 +++- src/test/app/Vault_test.cpp | 166 +++++--- src/test/protocol/STNumber_test.cpp | 54 ++- src/tests/libxrpl/basics/Number.cpp | 585 ++++++++++++++++++++++++---- 8 files changed, 946 insertions(+), 203 deletions(-) diff --git a/include/xrpl/basics/Number.h b/include/xrpl/basics/Number.h index 0026e7f006..f90800c715 100644 --- a/include/xrpl/basics/Number.h +++ b/include/xrpl/basics/Number.h @@ -364,6 +364,8 @@ public: static constexpr internalrep kMaxRep = std::numeric_limits::max(); static_assert(kMaxRep == 9'223'372'036'854'775'807); static_assert(-kMaxRep == std::numeric_limits::min() + 1); + static constexpr internalrep kMaxRepUp = ((kMaxRep / 10) + 1) * 10; + static_assert(kMaxRepUp == 9'223'372'036'854'775'810ULL); // May need to make unchecked private struct Unchecked @@ -591,6 +593,13 @@ public: std::pair normalizeToRange() const; + // Safely convert rep (int64) mantissa to internalrep (uint64). If the rep + // is negative, returns the positive value. This takes a little extra work + // because converting std::numeric_limits::min() flirts with + // UB, and can vary across compilers. + static internalrep + externalToInternal(rep mantissa); + private: static thread_local RoundingMode mode; // The available ranges for mantissa @@ -645,13 +654,6 @@ private: // exponent could go out of range, so it will be checked. [[nodiscard]] Number shiftExponent(int exponentDelta) const; - - // Safely convert rep (int64) mantissa to internalrep (uint64). If the rep - // is negative, returns the positive value. This takes a little extra work - // because converting std::numeric_limits::min() flirts with - // UB, and can vary across compilers. - static internalrep - externalToInternal(rep mantissa); }; constexpr Number::Number(bool negative, internalrep mantissa, int exponent, Unchecked) noexcept diff --git a/src/libxrpl/basics/Number.cpp b/src/libxrpl/basics/Number.cpp index d08fd23016..1f2c41809a 100644 --- a/src/libxrpl/basics/Number.cpp +++ b/src/libxrpl/basics/Number.cpp @@ -277,6 +277,25 @@ public: void doDropDigit(T& mantissa, int& exponent) noexcept; + // Modify the result to the correctly rounded value + template + void + doRoundUp(bool& negative, T& mantissa, int& exponent, std::string location); + + // Modify the result to the correctly rounded value + template + void + doRoundDown(bool& negative, T& mantissa, int& exponent) const; + + // Modify the result to the correctly rounded value + void + doRound(rep& drops, std::string location) const; + +private: + template + void + pushOverflow(T mantissa); + enum class Round { // The result is exact. No rounding is needed. Only used if cuspRoundingFix is Enabled330 or // higher. @@ -289,37 +308,22 @@ public: // The result was exactly half-way between two integers. This will round to even. Even = 0, // Round up. Always adds 1 (or subtracts 1 in some cases if cuspRoundingFix is not - // Enabled) + // Enabled330) Up = 1, }; - // Indicate round direction: 1 is up, -1 is down, 0 is even + // Indicate round direction. See Round enum above. // This enables the client to round towards nearest, and on // tie, round towards even. [[nodiscard]] Round round() const noexcept; - // Modify the result to the correctly rounded value - template - void - doRoundUp(bool& negative, T& mantissa, int& exponent, std::string location); - - // Modify the result to the correctly rounded value - template - void - doRoundDown(bool& negative, T& mantissa, int& exponent); - - // Modify the result to the correctly rounded value - void - doRound(rep& drops, std::string location) const; - -private: void doPush(unsigned d) noexcept; template void - bringIntoRange(bool& negative, T& mantissa, int& exponent); + bringIntoRange(bool& negative, T& mantissa, int& exponent) const; }; inline void @@ -349,6 +353,7 @@ Number::Guard::isNegative() const noexcept inline void Number::Guard::doPush(unsigned d) noexcept { + XRPL_ASSERT(d < 10, "xrpl::Number::Guard::doPush : valid digit"); xbit_ = xbit_ || ((digits_ & 0x0000'0000'0000'000F) != 0); digits_ >>= 4; digits_ |= (d & 0x0000'0000'0000'000FULL) << 60; @@ -396,10 +401,69 @@ Number::Guard::doDropDigit(uint128_t& mantissa, int& exponent) noexce ++exponent; } +template +void +Number::Guard::pushOverflow(T mantissa) +{ + XRPL_ASSERT(mantissa <= kMaxRepUp, "xrpl::Number::Guard::pushOverflow : valid mantissa"); + if (cuspRoundingFix >= MantissaRange::CuspRoundingFix::Enabled330 && mantissa >= kMaxRep && + mantissa < kMaxRepUp) + { + // Special case rounding rules for the values in the range [kMaxRep, kMaxRepUp). + + auto constexpr spread = kMaxRepUp - kMaxRep; + static_assert(spread == 3); + + // Round in two steps. + + // The first step uses the digits _already_ in the Guard to possibly round the mantissa up. + // Ultimately, the purpose of this step is to capture rounding where the stored digits would + // change the decision without those digits. (e.g. From just _below_ the midpoint to just + // _above_ the midpoint for ToNearest, or from kMaxRep into the in-between for Upward. Make + // an exception if the final digit is 9, because it can only get larger, and we don't want + // to bump up to kMaxRepUp. + if (mantissa % 10 < 9) + { + // Intentionally use integer math to get the largest value under the midpoint. + auto constexpr kMidpoint = kMaxRep + (spread / 2); + static_assert(kMidpoint == kMaxRep + 1); + auto const r = round(); + if (r == Round::Up || (r == Round::Even && mantissa == kMidpoint)) + { + ++mantissa; + } + } + + // The second step scales the final digit of the updated mantissa proportionally, converting + // from (kMaxRep, kMaxRepUp) to (0 to 9]. It then pushes that scaled digit onto the guard as + // if it was a digit that got removed, but doesn't actually remove it. This method should be + // future-proof in case the number of mantissa bits ever changes. (Though for integer values + // of the form 2^(2^x-1), the spread will always be the same.) Effects: + // * For round to nearest + // * if the updated mantissa is below the midpoint, it'll round "down" to kMaxRep + // * if above the midpoint, it'll round "up" to kMaxRepUp + // * it can never be exactly at the midpoint, because kMaxRepUp is always even, and + // kMaxRep is always odd, so don't worry about that case. + // * For round upward, will round up to kMaxRepUp for positive values, down to kMaxRep for + // negative. + // * For round downward, does the opposite of upward. + // * For round toward zero, always rounds down to kMaxRep. + + auto const diff = mantissa - kMaxRep; + auto const digit = static_cast((diff * 10) / spread); + XRPL_ASSERT( + digit < 10u && digit != 5, "xrpl::Number::Guard::pushOverflow : valid overflow digit"); + + // Don't remove the digit from the mantissa, but add it to the guard as if it was. + push(digit); + } +} + // Returns: -// -1 if Guard is less than half -// 0 if Guard is exactly half -// 1 if Guard is greater than half +// Exact if Guard is _zero_, and appropriate amendments are enabled +// Down if Guard is less than half +// Even if Guard is exactly half +// Up if Guard is greater than half Number::Guard::Round Number::Guard::round() const noexcept { @@ -445,17 +509,23 @@ Number::Guard::round() const noexcept template void -Number::Guard::bringIntoRange(bool& negative, T& mantissa, int& exponent) +Number::Guard::bringIntoRange(bool& negative, T& mantissa, int& exponent) const { // Bring mantissa back into the minMantissa / maxMantissa range AFTER - // rounding - if (mantissa < minMantissa) + // rounding. + if (mantissa < minMantissa && + (cuspRoundingFix < MantissaRange::CuspRoundingFix::Enabled330 || mantissa != 0)) { mantissa *= 10; --exponent; } - if (exponent < kMinExponent) + // mantissa should never be 0, but if it _is_ assert, but fall back to making the result kZero. + if (exponent < kMinExponent || + (cuspRoundingFix >= MantissaRange::CuspRoundingFix::Enabled330 && mantissa == 0)) { + // Engineers: If you hit this assert, you probably did something wrong in the operation + // leading up to the rounding work. + XRPL_ASSERT(mantissa != 0, "xrpl::Number::Guard::bringIntoRange : valid mantissa"); static constexpr Number kZero = Number{}; negative = kZero.negative_; @@ -468,7 +538,9 @@ template void Number::Guard::doRoundUp(bool& negative, T& mantissa, int& exponent, std::string location) { - auto r = round(); + pushOverflow(mantissa); + + auto const r = round(); if (r == Round::Up || (r == Round::Even && (mantissa & 1) == 1)) { auto const safeToIncrement = [this](auto const& mantissa) { @@ -485,18 +557,29 @@ Number::Guard::doRoundUp(bool& negative, T& mantissa, int& exponent, std::string } else { - // Incrementing the mantissa will require dividing, which will require rounding. So - // _don't_ increment the mantissa. Instead, divide and round recursively. It should - // be impossible to recurse more than once, because once the mantissa is divided by - // 10, it will be _well_ under maxMantissa and kMaxRep, so adding 1 will have no - // chance of bringing it back over. - doDropDigit(mantissa, exponent); - XRPL_ASSERT_PARTS( - safeToIncrement(mantissa), - "xrpl::Number::Guard::doRoundUp", - "can't recurse more than once"); - doRoundUp(negative, mantissa, exponent, location); - return; + if (cuspRoundingFix >= MantissaRange::CuspRoundingFix::Enabled330 && + mantissa > kMaxRep && mantissa < kMaxRepUp) + { + // When rounding up a value in between kMaxRep, and kMaxRepUp, round to + // kMaxRepUp. Note that the decision for this rounding is dominated by the + // results of pushOverflow. + mantissa = kMaxRepUp; + } + else + { + // Incrementing the mantissa will require dividing, which will require rounding. + // So _don't_ increment the mantissa. Instead, divide and round recursively. It + // should be impossible to recurse more than once, because once the mantissa is + // divided by 10, it will be _well_ under maxMantissa and kMaxRep, so adding 1 + // will have no chance of bringing it back over. + doDropDigit(mantissa, exponent); + XRPL_ASSERT_PARTS( + safeToIncrement(mantissa), + "xrpl::Number::Guard::doRoundUp", + "can't recurse more than once"); + doRoundUp(negative, mantissa, exponent, location); + return; + } } } else @@ -514,6 +597,14 @@ Number::Guard::doRoundUp(bool& negative, T& mantissa, int& exponent, std::string } } } + else if ( + cuspRoundingFix >= MantissaRange::CuspRoundingFix::Enabled330 && mantissa > kMaxRep && + mantissa < kMaxRepUp) + { + // When rounding down a value in between kMaxRep, and kMaxRepUp, round to kMaxRep. + // Note that the decision for this rounding is dominated by the results of pushOverflow. + mantissa = kMaxRep; + } bringIntoRange(negative, mantissa, exponent); if (exponent > kMaxExponent) Throw(std::string(location)); @@ -521,8 +612,10 @@ Number::Guard::doRoundUp(bool& negative, T& mantissa, int& exponent, std::string template void -Number::Guard::doRoundDown(bool& negative, T& mantissa, int& exponent) +Number::Guard::doRoundDown(bool& negative, T& mantissa, int& exponent) const { + // Do not pushOverflow here. + auto r = round(); if (cuspRoundingFix >= MantissaRange::CuspRoundingFix::Enabled330) { @@ -557,6 +650,8 @@ Number::Guard::doRoundDown(bool& negative, T& mantissa, int& exponent) void Number::Guard::doRound(rep& drops, std::string location) const { + // Do not pushOverflow here. + auto r = round(); if (r == Round::Up || (r == Round::Even && (drops & 1) == 1)) { @@ -573,6 +668,8 @@ Number::Guard::doRound(rep& drops, std::string location) const } ++drops; } + XRPL_ASSERT(drops >= 0, "xrpl::Number::Guard::doRound : positive magnitude"); + if (isNegative()) drops = -drops; } @@ -622,7 +719,9 @@ doNormalize( { static constexpr auto kMinExponent = Number::kMinExponent; static constexpr auto kMaxExponent = Number::kMaxExponent; - static constexpr auto kMaxRep = Number::kMaxRep; + auto const repLimit = cuspRoundingFix >= MantissaRange::CuspRoundingFix::Enabled330 + ? Number::kMaxRepUp + : Number::kMaxRep; using Guard = Number::Guard; @@ -672,17 +771,17 @@ doNormalize( // 9,900,000,000,000,123,450 or 9,900,000,000,000,123,460. // mantissa() will return mantissa / 10, and exponent() will return // exponent + 1. - if (m > kMaxRep) + if (m > repLimit) { if (exponent >= kMaxExponent) throw std::overflow_error("Number::normalize 1.5"); g.doDropDigit(m, exponent); } // Before modification, m should be within the min/max range. After - // modification, it must be less than kMaxRep. In other words, the original - // value should have been no more than kMaxRep * 10. - // (kMaxRep * 10 > maxMantissa) - XRPL_ASSERT_PARTS(m <= kMaxRep, "xrpl::doNormalize", "intermediate mantissa fits in int64"); + // modification, it must be less than repLimit. In other words, the original + // value should have been no more than repLimit * 10. + // (repLimit * 10 > maxMantissa) + XRPL_ASSERT_PARTS(m <= repLimit, "xrpl::doNormalize", "intermediate mantissa fits in limit"); mantissa = m; g.doRoundUp(negative, mantissa, exponent, "Number::normalize 2"); @@ -814,6 +913,9 @@ Number::operator+=(Number const& y) auto const& maxMantissa = g.maxMantissa; auto const cuspRoundingFix = g.cuspRoundingFix; + auto const repLimit = + cuspRoundingFix >= MantissaRange::CuspRoundingFix::Enabled330 ? kMaxRepUp : kMaxRep; + // Bring the exponents of both values into agreement, so the mantissas are on the same scale // and can be added directly together. @@ -898,7 +1000,7 @@ Number::operator+=(Number const& y) } else { - if (xm > maxMantissa || xm > kMaxRep) + if (xm > maxMantissa || xm > repLimit) { g.doDropDigit(xm, xe); } @@ -942,7 +1044,7 @@ Number::operator+=(Number const& y) { // Grow xm/xe and pull digits out of the Guard until it's back in the // minMantissa/maxMantissa range. - while (xm < minMantissa && xm * 10 <= kMaxRep) + while (xm < minMantissa && xm * 10 <= repLimit) { xm *= 10; xm -= g.pop(); @@ -1016,8 +1118,10 @@ Number::operator*=(Number const& y) g.setNegative(); auto const& maxMantissa = g.maxMantissa; + auto const repLimit = + g.cuspRoundingFix >= MantissaRange::CuspRoundingFix::Enabled330 ? kMaxRepUp : kMaxRep; - while (zm > maxMantissa || zm > kMaxRep) + while (zm > maxMantissa || zm > repLimit) { g.doDropDigit(zm, ze); } @@ -1282,8 +1386,11 @@ to_string(Number const& amount) } std::string ret = negative ? "-" : ""; ret.append(std::to_string(mantissa)); - ret.append(1, 'e'); - ret.append(std::to_string(exponent)); + if (exponent != 0) + { + ret.append(1, 'e'); + ret.append(std::to_string(exponent)); + } return ret; } diff --git a/src/libxrpl/protocol/Rules.cpp b/src/libxrpl/protocol/Rules.cpp index d71bb77f66..197139027a 100644 --- a/src/libxrpl/protocol/Rules.cpp +++ b/src/libxrpl/protocol/Rules.cpp @@ -45,7 +45,7 @@ setCurrentTransactionRules(std::optional r) // amendments must also be added to useRulesGuards. bool const enableLargeNumbers = !r || (r->enabled(featureSingleAssetVault) || r->enabled(featureLendingProtocol)); - // If enableLargeNumbers is true, then useRulesGuard must also return true. + // If enableLargeNumbers is true, then useRulesGuards must also return true. // However, the reverse is not true. Other amendments can cause the rules guard to be used, // even though large numbers are _not_ used. XRPL_ASSERT( diff --git a/src/libxrpl/protocol/STNumber.cpp b/src/libxrpl/protocol/STNumber.cpp index bd7f67649b..7bf98f270c 100644 --- a/src/libxrpl/protocol/STNumber.cpp +++ b/src/libxrpl/protocol/STNumber.cpp @@ -255,8 +255,47 @@ numberFromJson(SField const& field, json::Value const& value) Throw("not a number"); } - return STNumber{ - field, Number{parts.negative, parts.mantissa, parts.exponent, Number::Normalized{}}}; + Number const num{parts.negative, parts.mantissa, parts.exponent, Number::Normalized{}}; + + // Canonicalize "parts" and "num" with each other by getting rid of trailing 0s until either the + // exponents match, or there are no more 0s. If the two results don't match exactly, then the + // value has been rounded one way or another, and should not be used, because it may lead to an + // unexpected result. canonicalizeParts is not to be confused with Number::canonicalize, because + // they have completely different goals. + auto canonicalizeParts = [](NumberParts p, int otherExponent) { + if (p.mantissa == 0) + return NumberParts{}; + + while (p.exponent < otherExponent && p.mantissa % 10 == 0) + { + p.mantissa /= 10; + ++p.exponent; + } + + return p; + }; + + auto const numberMantissa = num.mantissa(); + auto const numberExponent = num.exponent(); + + auto const canonicalParts = canonicalizeParts(parts, numberExponent); + + auto const canonicalNum = canonicalizeParts( + NumberParts{ + .mantissa = Number::externalToInternal(numberMantissa), + .exponent = numberExponent, + .negative = numberMantissa < 0, + }, + canonicalParts.exponent); + + if (canonicalParts.mantissa != canonicalNum.mantissa || + canonicalParts.exponent != canonicalNum.exponent || + canonicalParts.negative != canonicalNum.negative) + { + Throw("number cannot be represented"); + } + + return STNumber{field, num}; } } // namespace xrpl diff --git a/src/test/app/LoanBroker_test.cpp b/src/test/app/LoanBroker_test.cpp index b0b0924c6e..f6f85a0cca 100644 --- a/src/test/app/LoanBroker_test.cpp +++ b/src/test/app/LoanBroker_test.cpp @@ -53,6 +53,7 @@ #include #include +#include #include #include #include @@ -1437,18 +1438,77 @@ class LoanBroker_test : public beast::unit_test::Suite env(tx2, Ter(temINVALID)); } + env.setParseFailureExpected(true); + try { - auto const dm = power(2, 63) - 1; - BEAST_EXPECTS(dm > kMaxMpTokenAmount, to_string(dm)); - tx2[sfDebtMaximum] = dm; + tx2[sfDebtMaximum] = "9223372036854775808"; env(tx2, Ter(temINVALID)); + // should throw in parser + fail(); } - + catch (std::exception const& e) { - auto const dm = power(2, 63) - 3; - BEAST_EXPECTS(dm == kMaxMpTokenAmount, to_string(dm)); - tx2[sfDebtMaximum] = dm; - env(tx2, Ter(tesSUCCESS)); + BEAST_EXPECT( + std::string(e.what()) == + "invalidParamsField 'tx_json.DebtMaximum' has invalid data."); + } + env.setParseFailureExpected(false); + + if (Number::getMantissaScale() >= MantissaRange::MantissaScale::Large330) + { + // For the Large330 scale, 2^63 rounds _down_ to Number::kMaxRep + { + auto const dm = power(2, 63); + BEAST_EXPECTS(dm == kMaxMpTokenAmount, to_string(dm)); + tx2[sfDebtMaximum] = dm; + env(tx2, Ter(tesSUCCESS)); + } + + { + auto const dm = power(2, 63) + Number{1, -1}; + BEAST_EXPECTS(dm == kMaxMpTokenAmount, to_string(dm)); + tx2[sfDebtMaximum] = dm; + env(tx2, Ter(tesSUCCESS)); + } + + { + auto const dm = power(2, 63) - 1; + BEAST_EXPECTS(dm < kMaxMpTokenAmount, to_string(dm)); + tx2[sfDebtMaximum] = dm; + env(tx2, Ter(tesSUCCESS)); + } + + { + auto const dm = power(2, 63) - 3; + BEAST_EXPECTS(dm < kMaxMpTokenAmount, to_string(dm)); + tx2[sfDebtMaximum] = dm; + env(tx2, Ter(tesSUCCESS)); + } + + { + auto const dm = power(2, 63) + 3; + BEAST_EXPECTS(dm > kMaxMpTokenAmount, to_string(dm)); + tx2[sfDebtMaximum] = dm; + env(tx2, Ter(temINVALID)); + } + } + else + { + // For other scales, 2^63 rounds _up_ to Number::kMaxRepUp. Subtracting 1 rounds up + // again. + { + auto const dm = power(2, 63) - 1; + BEAST_EXPECTS(dm > kMaxMpTokenAmount, to_string(dm)); + tx2[sfDebtMaximum] = dm; + env(tx2, Ter(temINVALID)); + } + + { + auto const dm = power(2, 63) - 3; + BEAST_EXPECTS(dm == kMaxMpTokenAmount, to_string(dm)); + tx2[sfDebtMaximum] = dm; + env(tx2, Ter(tesSUCCESS)); + } } { diff --git a/src/test/app/Vault_test.cpp b/src/test/app/Vault_test.cpp index 4de8c9c616..617820c89c 100644 --- a/src/test/app/Vault_test.cpp +++ b/src/test/app/Vault_test.cpp @@ -61,6 +61,7 @@ #include #include +#include #include #include #include @@ -5246,11 +5247,15 @@ class Vault_test : public beast::unit_test::Suite auto const maxInt64 = std::to_string(std::numeric_limits::max()); BEAST_EXPECT(maxInt64 == "9223372036854775807"); - // Naming things is hard auto const maxInt64Plus1 = std::to_string( static_cast(std::numeric_limits::max()) + 1); BEAST_EXPECT(maxInt64Plus1 == "9223372036854775808"); + // Naming things is hard + auto const maxInt64Plus2 = std::to_string( + static_cast(std::numeric_limits::max()) + 2); + BEAST_EXPECT(maxInt64Plus2 == "9223372036854775809"); + auto const initialXRP = to_string(kInitialXrp); BEAST_EXPECT(initialXRP == "100000000000000000"); @@ -5277,25 +5282,58 @@ class Vault_test : public beast::unit_test::Suite env(tx); env.close(); - tx[sfAssetsMaximum] = maxInt64Plus1; - env(tx, Ter(tefEXCEPTION)); - env.close(); + // There are several parse failures expected in this function, so just disable it once. + env.setParseFailureExpected(true); + try + { + tx[sfAssetsMaximum] = maxInt64Plus1; + env(tx, Ter(tefEXCEPTION)); + env.close(); + // should throw in parser + fail(); + } + catch (std::exception const& e) + { + BEAST_EXPECT( + std::string(e.what()) == + "invalidParamsField 'tx_json.AssetsMaximum' has invalid data."); + } + + try + { + tx[sfAssetsMaximum] = maxInt64Plus2; + env(tx, Ter(tefEXCEPTION)); + // should throw in parser + fail(); + } + catch (std::exception const& e) + { + BEAST_EXPECT( + std::string(e.what()) == + "invalidParamsField 'tx_json.AssetsMaximum' has invalid data."); + } - // This value will be rounded - auto const insertAt = maxInt64Plus1.size() - 3; - auto const decimalTest = maxInt64Plus1.substr(0, insertAt) + "." + - maxInt64Plus1.substr(insertAt); // (max int64+1) / 1000 - BEAST_EXPECT(decimalTest == "9223372036854775.808"); - tx[sfAssetsMaximum] = decimalTest; auto const newKeylet = keylet::vault(owner.id(), env.seq(owner)); - env(tx); - env.close(); + try + { + auto const insertAt = maxInt64Plus2.size() - 3; + auto const decimalTest = maxInt64Plus2.substr(0, insertAt) + "." + + maxInt64Plus2.substr(insertAt); // (max int64+2) / 1000 + BEAST_EXPECT(decimalTest == "9223372036854775.809"); + tx[sfAssetsMaximum] = decimalTest; + env(tx); + // should throw in parser + fail(); + } + catch (std::exception const& e) + { + BEAST_EXPECT( + std::string(e.what()) == + "invalidParamsField 'tx_json.AssetsMaximum' has invalid data."); + } auto const vaultSle = env.le(newKeylet); - if (!BEAST_EXPECT(vaultSle)) - return; - - BEAST_EXPECT(vaultSle->at(sfAssetsMaximum) == 9223372036854776); + BEAST_EXPECT(!vaultSle); } { @@ -5329,25 +5367,41 @@ class Vault_test : public beast::unit_test::Suite env(tx); env.close(); - tx[sfAssetsMaximum] = maxInt64Plus1; - env(tx, Ter(tefEXCEPTION)); - env.close(); + try + { + tx[sfAssetsMaximum] = maxInt64Plus2; + env(tx, Ter(tefEXCEPTION)); + // should throw in parser + fail(); + } + catch (std::exception const& e) + { + BEAST_EXPECT( + std::string(e.what()) == + "invalidParamsField 'tx_json.AssetsMaximum' has invalid data."); + } - // This value will be rounded - auto const insertAt = maxInt64Plus1.size() - 1; - auto const decimalTest = maxInt64Plus1.substr(0, insertAt) + "." + - maxInt64Plus1.substr(insertAt); // (max int64+1) / 10 - BEAST_EXPECT(decimalTest == "922337203685477580.8"); - tx[sfAssetsMaximum] = decimalTest; auto const newKeylet = keylet::vault(owner.id(), env.seq(owner)); - env(tx); - env.close(); + try + { + auto const insertAt = maxInt64Plus2.size() - 1; + auto const decimalTest = maxInt64Plus2.substr(0, insertAt) + "." + + maxInt64Plus2.substr(insertAt); // (max int64+2) / 10 + BEAST_EXPECT(decimalTest == "922337203685477580.9"); + tx[sfAssetsMaximum] = decimalTest; + env(tx); + // should throw in parser + fail(); + } + catch (std::exception const& e) + { + BEAST_EXPECT( + std::string(e.what()) == + "invalidParamsField 'tx_json.AssetsMaximum' has invalid data."); + } auto const vaultSle = env.le(newKeylet); - if (!BEAST_EXPECT(vaultSle)) - return; - - BEAST_EXPECT(vaultSle->at(sfAssetsMaximum) == 922337203685477581); + BEAST_EXPECT(!vaultSle); } { @@ -5374,9 +5428,22 @@ class Vault_test : public beast::unit_test::Suite env(tx); env.close(); - tx[sfAssetsMaximum] = maxInt64Plus1; - env(tx); - env.close(); + // Since several tests are expected to have parser failures, leave this flag set for the + // remainder of this function. + env.setParseFailureExpected(true); + try + { + tx[sfAssetsMaximum] = maxInt64Plus2; + env(tx); + // should throw in parser + fail(); + } + catch (std::exception const& e) + { + BEAST_EXPECT( + std::string(e.what()) == + "invalidParamsField 'tx_json.AssetsMaximum' has invalid data."); + } tx[sfAssetsMaximum] = "1000000000000000e80"; env.close(); @@ -5386,22 +5453,27 @@ class Vault_test : public beast::unit_test::Suite // These values will be rounded to 15 significant digits { - auto const insertAt = maxInt64Plus1.size() - 1; - auto const decimalTest = maxInt64Plus1.substr(0, insertAt) + "." + - maxInt64Plus1.substr(insertAt); // (max int64+1) / 10 - BEAST_EXPECT(decimalTest == "922337203685477580.8"); - tx[sfAssetsMaximum] = decimalTest; auto const newKeylet = keylet::vault(owner.id(), env.seq(owner)); - env(tx); - env.close(); + try + { + auto const insertAt = maxInt64Plus2.size() - 1; + auto const decimalTest = maxInt64Plus2.substr(0, insertAt) + "." + + maxInt64Plus2.substr(insertAt); // (max int64+2) / 10 + BEAST_EXPECT(decimalTest == "922337203685477580.9"); + tx[sfAssetsMaximum] = decimalTest; + env(tx); + // should throw in parser + fail(); + } + catch (std::exception const& e) + { + BEAST_EXPECT( + std::string(e.what()) == + "invalidParamsField 'tx_json.AssetsMaximum' has invalid data."); + } auto const vaultSle = env.le(newKeylet); - if (!BEAST_EXPECT(vaultSle)) - return; - - BEAST_EXPECT( - (vaultSle->at(sfAssetsMaximum) == - Number{9223372036854776, 2, Number::Normalized{}})); + BEAST_EXPECT(!vaultSle); } { tx[sfAssetsMaximum] = "9223372036854775807e40"; // max int64 * 10^40 diff --git a/src/test/protocol/STNumber_test.cpp b/src/test/protocol/STNumber_test.cpp index 5c9c3fd83c..74792e0a70 100644 --- a/src/test/protocol/STNumber_test.cpp +++ b/src/test/protocol/STNumber_test.cpp @@ -9,6 +9,7 @@ #include #include +#include #include #include #include @@ -101,6 +102,39 @@ struct STNumber_test : public beast::unit_test::Suite BEAST_EXPECT(numberFromJson(sfNumber, "-0.000e6") == STNumber(sfNumber, 0)); { + auto const parseNumber = [](std::string const& boundary) { + return numberFromJson(sfNumber, boundary); + }; + auto const expectParseThrows = [this, &parseNumber](std::string const& boundary) { + try + { + parseNumber(boundary); + fail(); + } + catch (std::exception const& e) + { + BEAST_EXPECT(std::string(e.what()) == "number cannot be represented"); + } + }; + + // Small rejects this; large scales parse it as 9223372036854775800e-1. + auto constexpr positiveBoundary = "922337203685477580"; + auto constexpr negativeBoundary = "-922337203685477580"; + if (Number::getMantissaScale() == MantissaRange::MantissaScale::Small) + { + expectParseThrows(positiveBoundary); + expectParseThrows(negativeBoundary); + } + else + { + BEAST_EXPECT( + parseNumber(positiveBoundary) == + STNumber(sfNumber, Number{922'337'203'685'477'580, 0})); + BEAST_EXPECT( + parseNumber(negativeBoundary) == + STNumber(sfNumber, Number{-922'337'203'685'477'580, 0})); + } + NumberRoundModeGuard const mg(Number::RoundingMode::TowardsZero); // maxint64 9,223,372,036,854,775,807 auto const maxInt = std::to_string(std::numeric_limits::max()); @@ -108,23 +142,19 @@ struct STNumber_test : public beast::unit_test::Suite auto const minInt = std::to_string(std::numeric_limits::min()); if (Number::getMantissaScale() == MantissaRange::MantissaScale::Small) { - BEAST_EXPECT( - numberFromJson(sfNumber, maxInt) == - STNumber(sfNumber, Number{9'223'372'036'854'775, 3})); - BEAST_EXPECT( - numberFromJson(sfNumber, minInt) == - STNumber(sfNumber, Number{-9'223'372'036'854'775, 3})); + // min/maxInt can't be exactly represented with the small mantissa, so they + // don't parse, and are expected to throw. + expectParseThrows(maxInt); + expectParseThrows(minInt); } else { + // with large mantissas, maxint is fine BEAST_EXPECT( - numberFromJson(sfNumber, maxInt) == + parseNumber(maxInt) == STNumber(sfNumber, Number{9'223'372'036'854'775'807, 0})); - BEAST_EXPECT( - numberFromJson(sfNumber, minInt) == - STNumber( - sfNumber, - Number{true, 9'223'372'036'854'775'808ULL, 0, Number::Normalized{}})); + // but minint's mantissa is > kMaxRep, and so rounds, and thus can't be parsed + expectParseThrows(minInt); } } diff --git a/src/tests/libxrpl/basics/Number.cpp b/src/tests/libxrpl/basics/Number.cpp index 70d6be2da5..36e1b4a700 100644 --- a/src/tests/libxrpl/basics/Number.cpp +++ b/src/tests/libxrpl/basics/Number.cpp @@ -182,6 +182,35 @@ TEST(NumberTest, limits) caught = true; } EXPECT_TRUE(caught); + + if (scale == MantissaRange::MantissaScale::Large330) + { + // Normalization with the other scales, including the older large mantissa scales, will + // overflow. + Number const bigNum{Number::kMaxRepUp, Number::kMaxExponent, Number::Normalized{}}; + // The display of large exponents won't go above kMaxExponent + EXPECT_EQ(to_string(bigNum), "9223372036854775810e32768") << bigNum; + // Perhaps surprisingly, this is ok, because the exponent range is related to when the + // number is _normalized_, and for mantissas > kMaxRep, the accessors return values that + // are not normalized. + EXPECT_EQ(bigNum.mantissa(), 922337203685477581ULL) << bigNum.mantissa(); + EXPECT_EQ(bigNum.exponent(), 32769) << bigNum.exponent(); + } + else + { + try + { + Number{Number::kMaxRepUp, Number::kMaxExponent, Number::Normalized{}}; + ADD_FAILURE(); + } + catch (std::overflow_error const& e) + { + std::string const expected = + (scale == MantissaRange::MantissaScale::Small ? "Number::normalize 1" + : "Number::normalize 1.5"); + EXPECT_EQ(e.what(), expected) << e.what(); + } + } } } @@ -193,7 +222,11 @@ TEST(NumberTest, add) auto const scale = Number::getMantissaScale(); + EXPECT_EQ(Number::getround(), Number::RoundingMode::ToNearest) + << to_string(Number::getround()); + using Case = std::tuple; + // TODO: Move these to the blocks where they're used auto const cSmall = std::to_array({ {Number{1'000'000'000'000'000, -15}, Number{6'555'555'555'555'555, -29}, @@ -304,12 +337,15 @@ TEST(NumberTest, add) auto const cLargeLegacy = std::to_array({ {Number{Number::kMaxRep}, Number{6, -1}, Number{Number::kMaxRep / 10, 1}, __LINE__}, }); - auto const cLargeCorrected = std::to_array({ + auto const cLarge320 = std::to_array({ {Number{Number::kMaxRep}, Number{6, -1}, Number{(Number::kMaxRep / 10) + 1, 1}, __LINE__}, }); + auto const cLargeCorrected = std::to_array({ + {Number{Number::kMaxRep}, Number{6, -1}, Number{Number::kMaxRep}, __LINE__}, + }); auto test = [](auto const& c) { for (auto const& [x, y, z, line] : c) { @@ -330,9 +366,28 @@ TEST(NumberTest, add) { test(cLargeLegacy); } + else if (scale == MantissaRange::MantissaScale::Large320) + { + test(cLarge320); + } else { test(cLargeCorrected); + + // This has to be created in this block, because normalization with the other + // scales, including the older large mantissa scales, will overflow. + Number const bigResult{ + Number::kMaxRepUp, Number::kMaxExponent, Number::Normalized{}}; + auto const cBigNums = std::to_array({ + { + // Add 3 to the mantissa to avoid rounding + Number::max(), + Number{3, Number::kMaxExponent}, + bigResult, + __LINE__, + }, + }); + test(cBigNums); } } { @@ -381,7 +436,7 @@ TEST(NumberTest, sub) Number{1'000'000'000'000'000, -15}, Number{1'000'000'000'000'000, -30}, __LINE__}}); - auto const cLarge = std::to_array( + auto const cLargeAll = std::to_array( // Note that items with extremely large mantissas need to be // calculated, because otherwise they overflow uint64. Items from C // with larger mantissa @@ -428,16 +483,55 @@ TEST(NumberTest, sub) Number{1'000'000'000'000'000'000, -36}, __LINE__}, {Number{Number::kMaxRep}, Number{6, -1}, Number{Number::kMaxRep - 1}, __LINE__}, - {Number{false, Number::kMaxRep + 1, 0, Number::Normalized{}}, - Number{1, 0}, - Number{(Number::kMaxRep / 10) + 1, 1}, - __LINE__}, - {Number{false, Number::kMaxRep + 1, 0, Number::Normalized{}}, - Number{3, 0}, - Number{Number::kMaxRep}, - __LINE__}, - {power(2, 63), Number{3, 0}, Number{Number::kMaxRep}, __LINE__}, }); + // Note that items with extremely large mantissas need to be + // calculated, because otherwise they overflow uint64. Items from C + // with larger mantissa + auto const cLarge = std::to_array({ + // Anything larger than kMaxRep rounds up + {Number{false, Number::kMaxRep + 1, 0, Number::Normalized{}}, + Number{1, 0}, + Number{(Number::kMaxRep / 10) + 1, 1}, + __LINE__}, + {Number{false, Number::kMaxRep + 1, 0, Number::Normalized{}}, + Number{3, 0}, + Number{Number::kMaxRep}, + __LINE__}, + {Number{false, Number::kMaxRep + 2, 0, Number::Normalized{}}, + Number{1, 0}, + Number{(Number::kMaxRep / 10) + 1, 1}, + __LINE__}, + {Number{false, Number::kMaxRep + 2, 0, Number::Normalized{}}, + Number{3, 0}, + Number{Number::kMaxRep}, + __LINE__}, + {power(2, 63), Number{3, 0}, Number{Number::kMaxRep}, __LINE__}, + }); + auto const cLarge330 = std::to_array({ + // kMaxRep + 1 is below the half-way point, so it rounds down to kMaxRep when the Number + // is created. + {Number{false, Number::kMaxRep + 1, 0, Number::Normalized{}}, + Number{1, 0}, + Number{Number::kMaxRep - 1}, + __LINE__}, + {Number{false, Number::kMaxRep + 1, 0, Number::Normalized{}}, + Number{3, 0}, + Number{Number::kMaxRep - 3}, + __LINE__}, + // kMaxRepUp -1 is above the half-way point, so it rounds up to kMaxRepUp when the + // Number is created. Subtracting 1 from that rounds up again. A little non-intuitive. + {Number{false, Number::kMaxRepUp - 1, 0, Number::Normalized{}}, + Number{1, 0}, + Number{(Number::kMaxRep / 10) + 1, 1}, + __LINE__}, + // Subtracting 3 gets back down to kMaxRep + {Number{false, Number::kMaxRepUp - 1, 0, Number::Normalized{}}, + Number{3, 0}, + Number{Number::kMaxRep}, + __LINE__}, + // 2^63 is the same as kMaxRep+1 + {power(2, 63), Number{3, 0}, Number{Number::kMaxRep - 3}, __LINE__}, + }); auto test = [](auto const& c) { for (auto const& [x, y, z, line] : c) { @@ -447,13 +541,23 @@ TEST(NumberTest, sub) EXPECT_EQ(result, z) << ss.str() << " Line: " << line; } }; - if (scale == MantissaRange::MantissaScale::Small) + switch (scale) { - test(cSmall); - } - else - { - test(cLarge); + case MantissaRange::MantissaScale::Small: + test(cSmall); + break; + case MantissaRange::MantissaScale::LargeLegacy: + case MantissaRange::MantissaScale::Large320: + test(cLargeAll); + test(cLarge); + break; + case MantissaRange::MantissaScale::Large330: + test(cLargeAll); + test(cLarge330); + break; + default: + ADD_FAILURE(); + break; } } } @@ -1370,38 +1474,39 @@ TEST(NumberTest, to_string) auto const scale = Number::getMantissaScale(); - auto test = [](Number const& n, std::string const& expected) { + auto test = [](Number const& n, std::string const& expected, int line) { auto const result = to_string(n); std::stringstream ss; ss << "to_string(" << result << "). Expected: " << expected; - EXPECT_EQ(result, expected) << ss.str(); + EXPECT_EQ(result, expected) << ss.str() << " Line: " << line; }; - test(Number(-2, 0), "-2"); - test(Number(0, 0), "0"); - test(Number(2, 0), "2"); - test(Number(25, -3), "0.025"); - test(Number(-25, -3), "-0.025"); - test(Number(25, 1), "250"); - test(Number(-25, 1), "-250"); - test(Number(2, 20), "2e20"); - test(Number(-2, -20), "-2e-20"); + test(Number(-2, 0), "-2", __LINE__); + test(Number(0, 0), "0", __LINE__); + test(Number(2, 0), "2", __LINE__); + test(Number(25, -3), "0.025", __LINE__); + test(Number(-25, -3), "-0.025", __LINE__); + test(Number(25, 1), "250", __LINE__); + test(Number(-25, 1), "-250", __LINE__); + test(Number(2, 20), "2e20", __LINE__); + test(Number(-2, -20), "-2e-20", __LINE__); // Test the edges // ((exponent < -(25)) || (exponent > -(5))))) // or ((exponent < -(28)) || (exponent > -(8))))) - test(Number(2, -10), "0.0000000002"); - test(Number(2, -11), "2e-11"); + test(Number(2, -10), "0.0000000002", __LINE__); + test(Number(2, -11), "2e-11", __LINE__); - test(Number(-2, 10), "-20000000000"); - test(Number(-2, 11), "-2e11"); + test(Number(-2, 10), "-20000000000", __LINE__); + test(Number(-2, 11), "-2e11", __LINE__); + test(Number(-2, 11) - 1, "-200000000001", __LINE__); switch (scale) { case MantissaRange::MantissaScale::Small: - test(Number::min(), "1e-32753"); - test(Number::max(), "9999999999999999e32768"); - test(Number::lowest(), "-9999999999999999e32768"); + test(Number::min(), "1e-32753", __LINE__); + test(Number::max(), "9999999999999999e32768", __LINE__); + test(Number::lowest(), "-9999999999999999e32768", __LINE__); { NumberRoundModeGuard const mg(Number::RoundingMode::TowardsZero); @@ -1409,61 +1514,131 @@ TEST(NumberTest, to_string) EXPECT_EQ(maxMantissa, (9'999'999'999'999'999)); test( Number{false, (maxMantissa * 1000) + 999, -3, Number::Normalized()}, - "9999999999999999"); + "9999999999999999", + __LINE__); test( Number{true, (maxMantissa * 1000) + 999, -3, Number::Normalized()}, - "-9999999999999999"); + "-9999999999999999", + __LINE__); - test(Number{std::numeric_limits::max(), -3}, "9223372036854775"); + test( + Number{std::numeric_limits::max(), -3}, + "9223372036854775", + __LINE__); test( -(Number{std::numeric_limits::max(), -3}), - "-9223372036854775"); + "-9223372036854775", + __LINE__); test( - Number{std::numeric_limits::min(), 0}, "-9223372036854775e3"); + Number{std::numeric_limits::min(), 0}, + "-9223372036854775e3", + __LINE__); test( -(Number{std::numeric_limits::min(), 0}), - "9223372036854775e3"); + "9223372036854775e3", + __LINE__); } break; default: // Test the edges // ((exponent < -(28)) || (exponent > -(8))))) - test(Number::min(), "1e-32750"); - test(Number::max(), "9223372036854775807e32768"); - test(Number::lowest(), "-9223372036854775807e32768"); + test(Number::min(), "1e-32750", __LINE__); + test(Number::max(), "9223372036854775807e32768", __LINE__); + test(Number::lowest(), "-9223372036854775807e32768", __LINE__); { NumberRoundModeGuard const mg(Number::RoundingMode::TowardsZero); auto const maxMantissa = Number::maxMantissa(); EXPECT_EQ((maxMantissa), (9'999'999'999'999'999'999ULL)); test( - Number{false, maxMantissa, 0, Number::Normalized{}}, "9999999999999999990"); + Number{false, maxMantissa, 0, Number::Normalized{}}, + "9999999999999999990", + __LINE__); test( - Number{true, maxMantissa, 0, Number::Normalized{}}, "-9999999999999999990"); + Number{true, maxMantissa, 0, Number::Normalized{}}, + "-9999999999999999990", + __LINE__); test( - Number{std::numeric_limits::max(), 0}, "9223372036854775807"); + Number{std::numeric_limits::max(), 0}, + "9223372036854775807", + __LINE__); test( -(Number{std::numeric_limits::max(), 0}), - "-9223372036854775807"); + "-9223372036854775807", + __LINE__); - // Because the absolute value of min is larger than max, it - // will be scaled down to fit under max. Since we're - // rounding towards zero, the 8 at the end is dropped. - test( - Number{std::numeric_limits::min(), 0}, - "-9223372036854775800"); - test( - -(Number{std::numeric_limits::min(), 0}), - "9223372036854775800"); + switch (scale) + { + case MantissaRange::MantissaScale::Large330: + // Because the absolute value of min() is larger than max(), it + // will be rounded down toward max() + test( + Number{std::numeric_limits::min(), 0}, + "-9223372036854775807", + __LINE__); + test( + -(Number{std::numeric_limits::min(), 0}), + "9223372036854775807", + __LINE__); + break; + default: + // Because the absolute value of min() is larger than max(), it + // will be scaled down to fit under max(). Since we're + // rounding towards zero, the 8 at the end is dropped. + test( + Number{std::numeric_limits::min(), 0}, + "-9223372036854775800", + __LINE__); + test( + -(Number{std::numeric_limits::min(), 0}), + "9223372036854775800", + __LINE__); + break; + } } + switch (scale) + { + case MantissaRange::MantissaScale::Large330: + // Rounding to nearest, since the mantissa is below the halfway point from + // kMaxRep to kMaxRepUp, it will be rounded down to kMaxRep + test( + Number{std::numeric_limits::max(), 0} + 1, + "9223372036854775807", + __LINE__); + test( + -(Number{std::numeric_limits::max(), 0} + 1), + "-9223372036854775807", + __LINE__); + break; + default: + // Rounding to nearest, since the mantissa is bigger than kMaxRep, the 8 + // will be dropped, and since that is bigger than 5, the result will be + // rounded up from 0 to 1. + test( + Number{std::numeric_limits::max(), 0} + 1, + "9223372036854775810", + __LINE__); + test( + -(Number{std::numeric_limits::max(), 0} + 1), + "-9223372036854775810", + __LINE__); + break; + } + // Rounding to nearest, will be rounded up to kMaxRepUp, but for different reasons + // depending on the scale. If older than "Large", it rounds up for the same reason + // "+1" rounds up. For "Large", since the mantissa is above the halfway point from + // kMaxRep to kMaxRepUp, it will be rounded up to kMaxRepUp. test( - Number{std::numeric_limits::max(), 0} + 1, "9223372036854775810"); + Number{std::numeric_limits::max(), 0} + 2, + "9223372036854775810", + __LINE__); test( - -(Number{std::numeric_limits::max(), 0} + 1), - "-9223372036854775810"); + -(Number{std::numeric_limits::max(), 0} + 2), + "-9223372036854775810", + __LINE__); break; } } @@ -1851,15 +2026,14 @@ TEST(NumberTest, upward_rounding_produces_value_not_below_exact_at_k_max_rep_cus auto const message = [&] { std::ostringstream os; - os << "\n" - << " a = " << fmt(BigInt(kAValue)) << "\n" + os << " a = " << fmt(BigInt(kAValue)) << "\n" << " b = " << fmt(BigInt(kBValue)) << "\n" << " exact a*b = " << fmt(exactProduct) << "\n" << " stored = " << fmt(storedValue) << "\n" << " stored - exact = " << fmt(signedDifference) << "\n" << " upward = " << (signedDifference >= 0 ? "held" : "VIOLATED") << "\n" << " stored.mantissa = " << product.mantissa() << "\n" - << " stored.exponent = " << product.exponent() << "\n"; + << " stored.exponent = " << product.exponent() << "\n\n"; return os.str(); }; @@ -1939,15 +2113,14 @@ TEST(NumberTest, upward_division_returns_value_not_below_exact_on_large_scale) auto const message = [&] { std::ostringstream os; - os << "\n" - << " a = " << kAValue << "\n" + os << " a = " << kAValue << "\n" << " b = " << kBValue << "\n" << " exact a/b = " << fmt(exact) << "\n" << " stored a/b = " << fmt(stored) << "\n" << " stored - exact = " << fmt(diff) << " (negative => Upward gave value BELOW truth)\n" << " quotient.mantissa = " << quotient.mantissa() << "\n" - << " quotient.exponent = " << quotient.exponent() << "\n"; + << " quotient.exponent = " << quotient.exponent() << "\n\n"; return os.str(); }; @@ -1997,15 +2170,14 @@ TEST(NumberTest, downward_division_returns_value_not_above_exact_on_large_scale) auto const message = [&] { std::ostringstream os; - os << "\n" - << " a = " << kAValue << "\n" + os << " a = " << kAValue << "\n" << " b = " << kBValue << "\n" << " exact a/b = " << fmt(exact) << "\n" << " stored a/b = " << fmt(stored) << "\n" << " stored - exact = " << fmt(diff) << " (positive => Downward gave value ABOVE truth)\n" << " quotient.mantissa = " << quotient.mantissa() << "\n" - << " quotient.exponent = " << quotient.exponent() << "\n"; + << " quotient.exponent = " << quotient.exponent() << "\n\n"; return os.str(); }; @@ -2065,15 +2237,14 @@ TEST(NumberTest, to_nearest_division_uses_dropped_digits_on_large_scale) auto const message = [&] { std::ostringstream os; - os << "\n" - << " a = " << kAValue << "\n" + os << " a = " << kAValue << "\n" << " b = " << kBValue << "\n" << " exact a/b = " << fmt(exact) << "\n" << " stored a/b = " << fmt(stored) << "\n" << " stored - exact = " << fmt(diff) << " (negative => ToNearest gave value BELOW truth)\n" << " quotient.mantissa = " << quotient.mantissa() << "\n" - << " quotient.exponent = " << quotient.exponent() << "\n"; + << " quotient.exponent = " << quotient.exponent() << "\n\n"; return os.str(); }; @@ -2169,14 +2340,13 @@ TEST(NumberTest, subtraction_rounding) auto const message = [&](auto const& r, auto const& sum) { std::ostringstream os; - os << "\n a = " << a << " (" << fmt(bigA) - << ")\n b = " << b << " (" << fmt(bigB) - << ")\n exact a + b = " << fmt(exact) << "\n"; + os << " a = " << a << " (" << fmt(bigA) << ")\n b = " << b + << " (" << fmt(bigB) << ")\n exact a + b = " << fmt(exact) << "\n"; auto const diff = sum.first - exact; auto const rLabel = to_string(r); os << std::string(15 - rLabel.length(), ' ') << rLabel << " = " << fmt(sum.first) - << "\n difference = " << fmt(diff) << "\n"; + << "\n difference = " << fmt(diff) << "\n\n"; return os.str(); }; @@ -2228,6 +2398,93 @@ TEST(NumberTest, subtraction_rounding) } } +TEST(NumberTest, normalization_cusp_tonearest_and_downward) +{ + for (auto const mantissaScale : MantissaRange::getAllScales()) + { + NumberMantissaScaleGuard const mg{mantissaScale}; + NumberRoundModeGuard const rg{Number::RoundingMode::ToNearest}; + + auto const scale = Number::getMantissaScale(); + + constexpr auto kMaxRep = Number::kMaxRep; + + // Both ToNearest and Downward should round to `below` + auto constexpr actual = static_cast(kMaxRep) + 1; + Number const below{static_cast(kMaxRep), 0}; + Number const above{false, static_cast(kMaxRep) + 3, 0, Number::Normalized{}}; + + auto construct = [](Number::RoundingMode mode) { + NumberRoundModeGuard const roundGuard{mode}; + return Number(false, actual, 0, Number::Normalized{}); + }; + Number const upward = construct(Number::RoundingMode::Upward); + + Number const toNearest = construct(Number::RoundingMode::ToNearest); + + Number const downward = construct(Number::RoundingMode::Downward); + + auto message = [&] { + std::ostringstream log; + log << " actual = " << actual << " (kMaxRep + 1)\n" + << " below = " << below << " (kMaxRep, distance 1)\n" + << " above = " << above << " (kMaxRep + 3, distance 2)\n" + << " Upward = " << upward << "\n" + << " ToNearest = " << toNearest << "\n" + << " Downward = " << downward << "\n\n"; + return log.str(); + }; + + switch (scale) + { + case MantissaRange::MantissaScale::Small: + // With the small mantissa, everything but Downward rounds UP, including the + // reference values, "above" and "below" + + EXPECT_EQ(below, above) << message(); + EXPECT_EQ(upward, above) << message(); + EXPECT_EQ(toNearest, above) << message(); + + EXPECT_LT(downward, below) << message(); + + break; + + case MantissaRange::MantissaScale::LargeLegacy: + case MantissaRange::MantissaScale::Large320: + // Upward round UP + EXPECT_EQ(upward, above) << message(); + + // ToNearest rounds UP when the DOWN neighbor is strictly closer + EXPECT_EQ(toNearest, above) << message(); + EXPECT_GT(toNearest, below) << message(); + + // Downward undershoots: it returns a value below `below` + EXPECT_LT(downward, below) << message(); + + // Both should have given the same answer, but they differ + EXPECT_GT(toNearest, downward) << message(); + + break; + default: + // Covers "Large" and any newly added scales + + // Upward round UP + EXPECT_EQ(upward, above) << message(); + + // ToNearest rounds to the strictly closer DOWN neighbor + EXPECT_NE(toNearest, above) << message(); + EXPECT_EQ(toNearest, below) << message(); + + // Downward also rounds to `below` + EXPECT_EQ(downward, below) << message(); + + // ToNearest rounds to downward + EXPECT_EQ(toNearest, downward) << message(); + break; + } + } +} + TEST(NumberTest, number_add_directed_sign_wrong) { for (auto const mantissaScale : MantissaRange::getAllScales()) @@ -2460,4 +2717,180 @@ TEST(NumberTest, number_add_to_nearest_picks_farther) } } +TEST(NumberTest, number_cusp_rounding_with_fractional_parts) +{ + for (auto const mantissaScale : MantissaRange::getAllScales()) + { + NumberMantissaScaleGuard const mg{mantissaScale}; + + auto const scale = Number::getMantissaScale(); + + Number const below{static_cast(Number::kMaxRep), 0}; + Number const above{false, Number::kMaxRepUp, 0, Number::Normalized{}}; + + auto header = [&] { + std::ostringstream log; + log << "Scale: " << to_string(mantissaScale) << ", Below: " << below + << ", Above: " << above << "\n"; + return log.str(); + }; + + auto const zeroPointFour = Number(4, -1); + auto const zeroPointFive = Number(5, -1); + auto const zeroPointSix = Number(6, -1); + auto const onePointFour = Number(14, -1); + auto const onePointFive = Number(15, -1); + auto const onePointSix = Number(16, -1); + auto const twoPointFour = Number(24, -1); + auto const twoPointFive = Number(25, -1); + auto const twoPointSix = Number(26, -1); + + auto const operands = std::to_array({ + zeroPointFour, + zeroPointFive, + zeroPointSix, + onePointFour, + onePointFive, + onePointSix, + twoPointFour, + twoPointFive, + twoPointSix, + }); + + auto const modes = std::to_array({ + Number::RoundingMode::ToNearest, + Number::RoundingMode::TowardsZero, + Number::RoundingMode::Downward, + Number::RoundingMode::Upward, + }); + + // Addition cases test kMaxRep + Operand + for (auto const& mode : modes) + { + for (auto const& operand : operands) + { + NumberRoundModeGuard const rg{mode}; + + auto const expectedValue = [&]() { + // Returns "above" by default. The checks here are for exceptions. + if (scale >= MantissaRange::MantissaScale::Large330) + { + if (mode == Number::RoundingMode::ToNearest && operand < onePointFive) + return below; + if (mode == Number::RoundingMode::TowardsZero || + mode == Number::RoundingMode::Downward) + return below; + } + if (scale == MantissaRange::MantissaScale::Large320) + { + if (mode == Number::RoundingMode::ToNearest) + { + if (operand < zeroPointFive) + return below; + } + if (mode == Number::RoundingMode::TowardsZero || + mode == Number::RoundingMode::Downward) + { + if (operand >= onePointFour) + return below - 7; + return below; + } + } + if (scale == MantissaRange::MantissaScale::LargeLegacy) + { + if (mode == Number::RoundingMode::ToNearest) + { + if (operand < zeroPointFive) + return below; + if (operand <= zeroPointSix) + return below - 7; + } + if (mode == Number::RoundingMode::TowardsZero || + mode == Number::RoundingMode::Downward) + { + if (operand >= onePointFour) + return below - 7; + return below; + } + if (mode == Number::RoundingMode::Upward && operand <= zeroPointSix) + return below - 7; + } + if (scale == MantissaRange::MantissaScale::Small && + mode == Number::RoundingMode::Upward) + return above + 1000; + return above; + }(); + + Number const actual = below + operand; + + auto message = [&] { + std::stringstream ss; + ss << header() << "kMaxRep + " << operand << " rounded " << to_string(mode) + << " to " << actual << ". Expected: " << expectedValue; + return ss.str(); + }; + EXPECT_EQ(actual, expectedValue) << message(); + } + } + + // Subtraction cases test kMaxRepUp - Operand + for (auto const& mode : modes) + { + for (auto const& operand : operands) + { + NumberRoundModeGuard const rg{mode}; + + auto const expectedValue = [&]() { + if (scale >= MantissaRange::MantissaScale::Large330) + { + if (mode == Number::RoundingMode::ToNearest && operand > onePointFive) + return below; + if (mode == Number::RoundingMode::TowardsZero || + mode == Number::RoundingMode::Downward) + return below; + } + if (scale == MantissaRange::MantissaScale::LargeLegacy || + scale == MantissaRange::MantissaScale::Large320) + { + if (mode == Number::RoundingMode::ToNearest) + { + if (operand >= twoPointSix) + return below; + } + if (mode == Number::RoundingMode::TowardsZero) + { + if (operand >= onePointFour) + return below - 7; + } + if (mode == Number::RoundingMode::Downward) + { + if (operand <= onePointSix) + return below - 7; + return below; + } + } + if (scale == MantissaRange::MantissaScale::Small) + { + if (mode == Number::RoundingMode::Downward) + return below - 1000; + if (mode == Number::RoundingMode::Upward) + return below; + } + return above; + }(); + + Number const actual = above - operand; + + auto message = [&] { + std::stringstream ss; + ss << header() << "kMaxRepUp - " << operand << " rounded " << to_string(mode) + << " to " << actual << ". Expected: " << expectedValue; + return ss.str(); + }; + EXPECT_EQ(actual, expectedValue) << message(); + } + } + } +} + } // namespace xrpl From 530e09dbe8982ade14b8d469ab6323d7a65f0594 Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Tue, 14 Jul 2026 14:48:10 -0400 Subject: [PATCH 13/13] fix: Update base_uint and test changes released in 3.1.3 (#7570) Co-authored-by: Sergey Kuznetsov Co-authored-by: xrplf-ai-reviewer[bot] <266832837+xrplf-ai-reviewer[bot]@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: Ayaz Salikhov --- .../scripts/levelization/results/ordering.txt | 1 + include/xrpl/basics/base_uint.h | 10 +- src/test/protocol/Issue_test.cpp | 100 +++++++++++++ src/test/protocol/STIssue_test.cpp | 140 ++++++++++++++++++ src/test/rpc/AccountObjects_test.cpp | 83 +++++++++++ src/test/rpc/Transaction_test.cpp | 122 +++++++++++++++ src/tests/libxrpl/basics/base_uint_test.cpp | 80 ++++++++++ 7 files changed, 534 insertions(+), 2 deletions(-) diff --git a/.github/scripts/levelization/results/ordering.txt b/.github/scripts/levelization/results/ordering.txt index aee6f4c579..b31e6ae961 100644 --- a/.github/scripts/levelization/results/ordering.txt +++ b/.github/scripts/levelization/results/ordering.txt @@ -159,6 +159,7 @@ test.peerfinder > xrpl.protocol test.protocol > test.jtx test.protocol > test.unit_test test.protocol > xrpl.basics +test.protocol > xrpld.core test.protocol > xrpl.json test.protocol > xrpl.protocol test.rpc > test.jtx diff --git a/include/xrpl/basics/base_uint.h b/include/xrpl/basics/base_uint.h index 96cfa343e3..bee8b8b945 100644 --- a/include/xrpl/basics/base_uint.h +++ b/include/xrpl/basics/base_uint.h @@ -308,7 +308,9 @@ public: XRPL_ASSERT( c.size() * sizeof(typename Container::value_type) == size(), "xrpl::BaseUInt::fromRaw(Container auto) : input size match"); - std::memcpy(result.data_.data(), c.data(), size()); + std::size_t const canCopy = + std::min(size(), c.size() * sizeof(typename Container::value_type)); + std::memcpy(result.data_.data(), c.data(), canCopy); return result; } @@ -322,7 +324,11 @@ public: XRPL_ASSERT( c.size() * sizeof(typename Container::value_type) == size(), "xrpl::BaseUInt::operator=(Container auto) : input size match"); - std::memcpy(data_.data(), c.data(), size()); + std::size_t const canCopy = + std::min(size(), c.size() * sizeof(typename Container::value_type)); + if (canCopy < size()) + *this = beast::kZero; + std::memcpy(data_.data(), c.data(), canCopy); return *this; } diff --git a/src/test/protocol/Issue_test.cpp b/src/test/protocol/Issue_test.cpp index 44a36b7dcf..a6a1fdd341 100644 --- a/src/test/protocol/Issue_test.cpp +++ b/src/test/protocol/Issue_test.cpp @@ -1,10 +1,12 @@ #include #include #include +#include #include #include #include #include +#include #include #include @@ -863,6 +865,101 @@ public: //-------------------------------------------------------------------------- + void + testIssueFromJson() + { + testcase("issueFromJson"); + + // Valid XRP — no issuer field + { + json::Value jv; + jv[jss::currency] = "XRP"; + auto const issue = issueFromJson(jv); + BEAST_EXPECT(isXRP(issue)); + } + + // Valid IOU — legitimate issuer + { + json::Value jv; + jv[jss::currency] = "USD"; + jv[jss::issuer] = "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh"; + auto const issue = issueFromJson(jv); + BEAST_EXPECT(!isXRP(issue)); + BEAST_EXPECT(issue.account != noAccount()); + } + + // noAccount() is the MPT sentinel in binary serialization - must be + // rejected + try + { + json::Value jv; + jv[jss::currency] = "USD"; + jv[jss::issuer] = to_string(noAccount()); + issueFromJson(jv); + fail("noAccount() accepted as IOU issuer"); + } + catch (...) + { + pass(); + } + + // xrpAccount() is the XRP sentinel (all zeros) - must be rejected + // as IOU issuer + try + { + json::Value jv; + jv[jss::currency] = "USD"; + jv[jss::issuer] = to_string(xrpAccount()); + issueFromJson(jv); + fail("xrpAccount() accepted as IOU issuer"); + } + catch (...) + { + pass(); + } + + // Invalid base58 — must be rejected + try + { + json::Value jv; + jv[jss::currency] = "USD"; + jv[jss::issuer] = "not_a_valid_address"; + issueFromJson(jv); + fail("invalid base58 accepted as IOU issuer"); + } + catch (...) + { + pass(); + } + + // Non-XRP currency with no issuer field — must be rejected + try + { + json::Value jv; + jv[jss::currency] = "USD"; + issueFromJson(jv); + fail("missing issuer accepted"); + } + catch (...) + { + pass(); + } + + // XRP with an issuer field — must be rejected + try + { + json::Value jv; + jv[jss::currency] = "XRP"; + jv[jss::issuer] = "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh"; + issueFromJson(jv); + fail("XRP with issuer accepted"); + } + catch (...) + { + pass(); + } + } + void run() override { @@ -897,6 +994,9 @@ public: // --- testIssueDomainSets(); testIssueDomainMaps(); + + // --- + testIssueFromJson(); } }; diff --git a/src/test/protocol/STIssue_test.cpp b/src/test/protocol/STIssue_test.cpp index 1d6d750355..b7cc944e6b 100644 --- a/src/test/protocol/STIssue_test.cpp +++ b/src/test/protocol/STIssue_test.cpp @@ -1,15 +1,24 @@ #include +#include #include // IWYU pragma: keep +#include + +#include #include #include +#include +#include #include #include #include #include #include #include +#include + +#include namespace xrpl::test { @@ -137,12 +146,143 @@ public: "000000000000000000000000000000000000000000000002"); } + void + testNoAccountIssuerRpc() + { + testcase("noAccount issuer rejected via RPC sign"); + + using namespace jtx; + Env env{*this, envconfig([](std::unique_ptr cfg) { + cfg->loadFromString("[signing_support]\ntrue"); + return cfg; + })}; + + Account const alice{"alice"}; + env.fund(XRP(10000), alice); + env.close(); + + json::Value txJson; + txJson[jss::TransactionType] = "AMMDelete"; + txJson[jss::Account] = alice.human(); + txJson[jss::Asset][jss::currency] = "USD"; + txJson[jss::Asset][jss::issuer] = to_string(noAccount()); + txJson[jss::Asset2][jss::currency] = "XRP"; + + json::Value req; + req[jss::tx_json] = txJson; + req[jss::secret] = alice.name(); + + auto const result = env.rpc("json", "sign", to_string(req))[jss::result]; + + BEAST_EXPECT(result[jss::error] == "invalidParams"); + BEAST_EXPECT(result[jss::error_message] == "Field 'tx_json.Asset' has invalid data."); + } + + void + testNoAccountIssuer() + { + testcase("noAccount issuer rejection"); + + { + json::Value jv; + jv[jss::currency] = "USD"; + jv[jss::issuer] = to_string(noAccount()); + + try + { + issueFromJson(sfAsset, jv); + fail("issueFromJson accepted noAccount() as IOU issuer"); + } + catch (...) + { + pass(); + } + } + + { + Serializer s; + s.addBitString(toCurrency("USD")); + s.addBitString(noAccount()); + SerialIter iter(s.slice()); + + try + { + STIssue const stissue(iter, sfAsset); + fail( + "STIssue deserialization of [USD][noAccount()] should " + "throw"); + } + catch (...) + { + pass(); + } + } + } + + void + testXrpAccountIssuerRpc() + { + testcase("xrpAccount issuer rejected via RPC sign"); + + using namespace jtx; + Env env{*this, envconfig([](std::unique_ptr cfg) { + cfg->loadFromString("[signing_support]\ntrue"); + return cfg; + })}; + + Account const alice{"alice"}; + env.fund(XRP(10000), alice); + env.close(); + + json::Value txJson; + txJson[jss::TransactionType] = "AMMDelete"; + txJson[jss::Account] = alice.human(); + txJson[jss::Asset][jss::currency] = "USD"; + txJson[jss::Asset][jss::issuer] = to_string(xrpAccount()); + txJson[jss::Asset2][jss::currency] = "XRP"; + + json::Value req; + req[jss::tx_json] = txJson; + req[jss::secret] = alice.name(); + + auto const result = env.rpc("json", "sign", to_string(req))[jss::result]; + + BEAST_EXPECT(result[jss::error] == "invalidParams"); + BEAST_EXPECT(result[jss::error_message] == "Field 'tx_json.Asset' has invalid data."); + } + + void + testXrpAccountIssuer() + { + testcase("xrpAccount issuer rejection"); + + { + json::Value jv; + jv[jss::currency] = "USD"; + jv[jss::issuer] = to_string(xrpAccount()); + + try + { + issueFromJson(sfAsset, jv); + fail("issueFromJson accepted xrpAccount() as IOU issuer"); + } + catch (...) + { + pass(); + } + } + } + void run() override { // compliments other unit tests to ensure complete coverage testConstructor(); testCompare(); + testNoAccountIssuerRpc(); + testNoAccountIssuer(); + testXrpAccountIssuerRpc(); + testXrpAccountIssuer(); } }; diff --git a/src/test/rpc/AccountObjects_test.cpp b/src/test/rpc/AccountObjects_test.cpp index 6ba6e7de9f..c656c97a4c 100644 --- a/src/test/rpc/AccountObjects_test.cpp +++ b/src/test/rpc/AccountObjects_test.cpp @@ -27,6 +27,7 @@ #include #include #include +#include #include #include #include @@ -36,6 +37,7 @@ #include #include #include +#include #include namespace xrpl::test { @@ -1616,6 +1618,86 @@ public: } } + void + testAccountObjectDoesntShowCancelledOffers() + { + testcase("AccountObjectDoesntShowCancelledOffers"); + + using namespace jtx; + Env env(*this); + + Account const alice{"alice"}; + Account const bob{"bob"}; + auto const eur = bob["EUR"]; + env.fund(XRP(10000), alice, bob); + env.close(); + + auto const rpcAccountObjects = [&](std::optional limit = std::nullopt) { + json::Value params; + params[jss::account] = alice.human(); + if (limit.has_value()) + { + params[jss::limit] = *limit; + } + return env.rpc("json", "account_objects", to_string(params)); + }; + + auto const numEntries = 33; + std::vector seqs; + seqs.reserve(numEntries); + for ([[maybe_unused]] auto _ : std::ranges::iota_view{0, numEntries}) + { + json::Value params; + params[jss::secret] = toBase58(generateSeed("alice")); + params[jss::tx_json] = offer(alice, eur(1), XRP(2)); + auto const res = env.rpc("json", "submit", to_string(params))[jss::result]; + BEAST_EXPECT(res[jss::engine_result].asString() == "tesSUCCESS"); + seqs.push_back(env.seq(alice)); + } + + auto res = rpcAccountObjects(); + BEAST_EXPECT(res[jss::result][jss::account_objects].size() == numEntries); + BEAST_EXPECT(not res[jss::result].isMember(jss::limit)); + BEAST_EXPECT(not res[jss::result].isMember(jss::marker)); + + for (auto const s : std::views::all(seqs) | std::views::take(numEntries - 1)) + { + json::Value params; + params[jss::secret] = toBase58(generateSeed("alice")); + params[jss::tx_json] = offerCancel(alice, s - 1); + auto const res = env.rpc("json", "submit", to_string(params))[jss::result]; + BEAST_EXPECT(res[jss::engine_result].asString() == "tesSUCCESS"); + } + + res = rpcAccountObjects(); + BEAST_EXPECT(res[jss::result][jss::account_objects].size() == 1); + BEAST_EXPECT(not res[jss::result].isMember(jss::limit)); + BEAST_EXPECT(not res[jss::result].isMember(jss::marker)); + + { + json::Value params; + params[jss::secret] = toBase58(generateSeed("alice")); + json::Value txJson; + txJson[jss::TransactionType] = jss::NFTokenMint; + txJson[jss::Account] = to_string(alice.id()); + txJson["NFTokenTaxon"] = 1; + params[jss::tx_json] = txJson; + auto const res = env.rpc("json", "submit", to_string(params))[jss::result]; + BEAST_EXPECT(res[jss::engine_result].asString() == "tesSUCCESS"); + } + env.close(); + + res = rpcAccountObjects(); + BEAST_EXPECT(res[jss::result][jss::account_objects].size() == 2); + BEAST_EXPECT(not res[jss::result].isMember(jss::limit)); + BEAST_EXPECT(not res[jss::result].isMember(jss::marker)); + + res = rpcAccountObjects(1); + BEAST_EXPECT(res[jss::result][jss::account_objects].size() == 1); + BEAST_EXPECT(res[jss::result][jss::limit].asUInt() == 1); + BEAST_EXPECT(res[jss::result].isMember(jss::marker)); + } + void run() override { @@ -1627,6 +1709,7 @@ public: testAccountNFTs(); testAccountObjectMarker(); testSponsoredFilter(); + testAccountObjectDoesntShowCancelledOffers(); } }; diff --git a/src/test/rpc/Transaction_test.cpp b/src/test/rpc/Transaction_test.cpp index c3bf707ec4..4dae475b63 100644 --- a/src/test/rpc/Transaction_test.cpp +++ b/src/test/rpc/Transaction_test.cpp @@ -2,6 +2,7 @@ #include #include #include +#include #include #include #include @@ -880,6 +881,125 @@ class Transaction_test : public beast::unit_test::Suite } } + void + testSignForNetworkIDValidation() + { + testcase("SignFor NetworkID validation"); + using namespace test::jtx; + + Account const owner{"owner"}; + Account const signer{"signer"}; + + auto makeConfig = [](std::uint32_t networkID) { + return envconfig([networkID](std::unique_ptr cfg) { + cfg->networkId = networkID; + return cfg; + }); + }; + + auto setupEnv = [&](Env& env) { + env.fund(XRP(10'000), owner, signer); + env.close(); + env(signers(owner, 1, {{signer, 1}})); + env.close(); + }; + + auto makeTx = [&](Env& env) { + json::Value tx; + tx[jss::TransactionType] = jss::AccountSet; + tx[jss::Account] = owner.human(); + tx[jss::Sequence] = env.seq(owner); + tx[jss::Fee] = "100"; + tx[jss::SigningPubKey] = ""; + return tx; + }; + + auto signFor = [&](Env& env, json::Value const& tx) { + json::Value signReq; + signReq[jss::tx_json] = tx; + signReq[jss::account] = signer.human(); + signReq[jss::secret] = signer.name(); + return env.rpc("json", "sign_for", to_string(signReq))[jss::result]; + }; + + // Test case: NetworkID < 1024 - field is not required + { + Env env{*this, makeConfig(500)}; + setupEnv(env); + + auto tx = makeTx(env); + auto result = signFor(env, tx); + + BEAST_EXPECT(result[jss::status] == "success"); + BEAST_EXPECT(!result[jss::tx_json].isMember(jss::NetworkID)); + } + + // Test case: NetworkID > 1024 - missing NetworkID field + { + Env env{*this, makeConfig(2040)}; + setupEnv(env); + + auto tx = makeTx(env); + auto result = signFor(env, tx); + + BEAST_EXPECT(result[jss::error] == "invalidParams"); + BEAST_EXPECT(result[jss::error_message] == "Missing field 'tx_json.NetworkID'."); + } + + // Test case: NetworkID > 1024 - NetworkID field is not a number + { + Env env{*this, makeConfig(2040)}; + setupEnv(env); + + auto tx = makeTx(env); + tx[jss::NetworkID] = "not_a_number"; + auto result = signFor(env, tx); + + BEAST_EXPECT(result[jss::error] == "invalidParams"); + BEAST_EXPECT(result[jss::error_message] == "Invalid field 'tx_json.NetworkID'."); + } + + // Test case: NetworkID > 1024 - NetworkID field is not integral + { + Env env{*this, makeConfig(2040)}; + setupEnv(env); + + auto tx = makeTx(env); + tx[jss::NetworkID] = 2040.1; + auto result = signFor(env, tx); + + BEAST_EXPECT(result[jss::error] == "invalidParams"); + BEAST_EXPECT(result[jss::error_message] == "Invalid field 'tx_json.NetworkID'."); + } + + // Test case: NetworkID > 1024 - NetworkID field is different from + // actual NetworkID + { + Env env{*this, makeConfig(2040)}; + setupEnv(env); + + auto tx = makeTx(env); + tx[jss::NetworkID] = 9999; + auto result = signFor(env, tx); + + BEAST_EXPECT(result[jss::error] == "invalidParams"); + BEAST_EXPECT(result[jss::error_message] == "Invalid field 'tx_json.NetworkID'."); + } + + // Test case: NetworkID > 1024 - NetworkID field is correct + { + Env env{*this, makeConfig(2040)}; + setupEnv(env); + + auto tx = makeTx(env); + tx[jss::NetworkID] = 2040; + auto result = signFor(env, tx); + + BEAST_EXPECT(result[jss::status] == "success"); + BEAST_EXPECT(result[jss::tx_json][jss::NetworkID].asUInt() == 2040); + } + } + public: void run() override @@ -889,6 +1009,8 @@ public: FeatureBitset const all{testableAmendments()}; testWithFeats(all); + + testSignForNetworkIDValidation(); } void diff --git a/src/tests/libxrpl/basics/base_uint_test.cpp b/src/tests/libxrpl/basics/base_uint_test.cpp index eefbff158d..c9bfc35c94 100644 --- a/src/tests/libxrpl/basics/base_uint_test.cpp +++ b/src/tests/libxrpl/basics/base_uint_test.cpp @@ -122,6 +122,73 @@ struct BaseUintTest : public ::testing::Test } }; +using BaseUintDeathTest = BaseUintTest; + +TEST_F(BaseUintDeathTest, fromRaw_size_mismatch) +{ + // ENABLE_VOIDSTAR is a debug build, but does not crash on failed asserts. Rather than twist + // these tests into knots to make them work, just skip them. +#ifdef ENABLE_VOIDSTAR + GTEST_SKIP() << "ENABLE_VOIDSTAR is a debug build, but does not crash on failed asserts."; +#else + auto smallConstruct = [] { + // Container smaller than the base_uint (8 bytes vs 12 bytes for + // test96). Only the first 8 bytes are copied; the remaining 4 bytes + // stay zero. + Blob const tooSmall{1, 2, 3, 4, 5, 6, 7, 8}; + BaseUInt96 const result = BaseUInt96::fromRaw(tooSmall); + auto const resultText = to_string(result); + EXPECT_EQ(resultText, "010203040506070800000000") << resultText; + }; + EXPECT_DEBUG_DEATH(smallConstruct(), "input size match"); + + auto largeConstruct = [] { + // Container larger than the base_uint (16 bytes vs 12 bytes for + // test96). Only the first 12 bytes are copied; the extra bytes are + // ignored. + Blob const tooBig{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16}; + BaseUInt96 const result = BaseUInt96::fromRaw(tooBig); + auto const resultText = to_string(result); + EXPECT_EQ(resultText, "0102030405060708090A0B0C") << resultText; + }; + EXPECT_DEBUG_DEATH(largeConstruct(), "input size match"); + + auto smallCopy = [] { + // Container smaller than the base_uint (8 bytes vs 12 bytes for + // test96). Only the first 8 bytes are copied; the remaining 4 bytes + // stay zero. + Blob const tooSmall{1, 2, 3, 4, 5, 6, 7, 8}; + BaseUInt96 result{}; + --result; + { + auto const originalText = to_string(result); + EXPECT_EQ(originalText, "FFFFFFFFFFFFFFFFFFFFFFFF") << originalText; + } + result = tooSmall; + auto const resultText = to_string(result); + EXPECT_EQ(resultText, "010203040506070800000000") << resultText; + }; + EXPECT_DEBUG_DEATH(smallCopy(), "input size match"); + + auto const largeCopy = [] { + // Container larger than the base_uint (16 bytes vs 12 bytes for + // test96). Only the first 12 bytes are copied; the extra bytes are + // ignored. + Blob const tooBig{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16}; + BaseUInt96 result{}; + --result; + { + auto const originalText = to_string(result); + EXPECT_EQ(originalText, "FFFFFFFFFFFFFFFFFFFFFFFF") << originalText; + } + result = tooBig; + auto const resultText = to_string(result); + EXPECT_EQ(resultText, "0102030405060708090A0B0C") << resultText; + }; + EXPECT_DEBUG_DEATH(largeCopy(), "input size match"); +#endif +} + TEST_F(BaseUintTest, base_uint) { static_assert(!std::is_constructible_v>); @@ -198,6 +265,19 @@ TEST_F(BaseUintTest, base_uint) EXPECT_EQ(d, 0); } + { + // There are several ways to create a zero. beast::kZero is tested above. Test some + // others. + BaseUInt96 const z1; + EXPECT_EQ(z1, z) << to_string(z1); + + BaseUInt96 const z2{}; + EXPECT_EQ(z2, z) << to_string(z2); + + BaseUInt96 const z3{0u}; + EXPECT_EQ(z3, z) << to_string(z3); + } + BaseUInt96 n{z}; n++; EXPECT_EQ(n, BaseUInt96(1));