From c4c95dbe7659ec453fe140f63fb7c9a0d5b4d7ef Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Tue, 12 May 2026 08:26:02 -0400 Subject: [PATCH 01/21] refactor: Replace `featureInvariantsV1_1` with `fixCleanup3_2_0` (#7116) --- include/xrpl/protocol/detail/features.macro | 3 --- src/libxrpl/tx/invariants/InvariantCheck.cpp | 2 +- src/test/app/Invariants_test.cpp | 2 +- 3 files changed, 2 insertions(+), 5 deletions(-) diff --git a/include/xrpl/protocol/detail/features.macro b/include/xrpl/protocol/detail/features.macro index 78b78e132e..2c2cd14e97 100644 --- a/include/xrpl/protocol/detail/features.macro +++ b/include/xrpl/protocol/detail/features.macro @@ -46,9 +46,6 @@ XRPL_FEATURE(Credentials, Supported::Yes, VoteBehavior::DefaultNo XRPL_FEATURE(AMMClawback, Supported::Yes, VoteBehavior::DefaultNo) XRPL_FIX (AMMv1_2, Supported::Yes, VoteBehavior::DefaultNo) XRPL_FEATURE(MPTokensV1, Supported::Yes, VoteBehavior::DefaultNo) -// InvariantsV1_1 will be changes to Supported::yes when all the -// invariants expected to be included under it are complete. -XRPL_FEATURE(InvariantsV1_1, Supported::No, VoteBehavior::DefaultNo) XRPL_FIX (NFTokenPageLinks, Supported::Yes, VoteBehavior::DefaultNo) XRPL_FIX (InnerObjTemplate2, Supported::Yes, VoteBehavior::DefaultNo) XRPL_FIX (EnforceNFTokenTrustline, Supported::Yes, VoteBehavior::DefaultNo) diff --git a/src/libxrpl/tx/invariants/InvariantCheck.cpp b/src/libxrpl/tx/invariants/InvariantCheck.cpp index 4f25a91bdf..e9c32bf14c 100644 --- a/src/libxrpl/tx/invariants/InvariantCheck.cpp +++ b/src/libxrpl/tx/invariants/InvariantCheck.cpp @@ -467,7 +467,7 @@ AccountRootsDeletedClean::finalize( // transaction processing results, however unlikely, only fail if the // feature is enabled. Enabled, or not, though, a fatal-level message will // be logged - [[maybe_unused]] bool const enforce = view.rules().enabled(featureInvariantsV1_1) || + [[maybe_unused]] bool const enforce = view.rules().enabled(fixCleanup3_2_0) || view.rules().enabled(featureSingleAssetVault) || view.rules().enabled(featureLendingProtocol); diff --git a/src/test/app/Invariants_test.cpp b/src/test/app/Invariants_test.cpp index 09b16d7b67..5864d66723 100644 --- a/src/test/app/Invariants_test.cpp +++ b/src/test/app/Invariants_test.cpp @@ -94,7 +94,7 @@ class Invariants_test : public beast::unit_test::Suite static FeatureBitset defaultAmendments() { - return xrpl::test::jtx::testableAmendments() | featureInvariantsV1_1 | fixSecurity3_1_3; + return xrpl::test::jtx::testableAmendments() | fixSecurity3_1_3 | fixCleanup3_2_0; } /** Run a specific test case to put the ledger into a state that will be From aa553924530a62ba9511ff332031aa035a314647 Mon Sep 17 00:00:00 2001 From: Ayaz Salikhov Date: Tue, 12 May 2026 16:54:04 +0100 Subject: [PATCH 02/21] ci: Make `Show test failure summary` work with no build dir (#7124) --- .github/workflows/reusable-build-test-config.yml | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/.github/workflows/reusable-build-test-config.yml b/.github/workflows/reusable-build-test-config.yml index 528934f6de..cc927512ea 100644 --- a/.github/workflows/reusable-build-test-config.yml +++ b/.github/workflows/reusable-build-test-config.yml @@ -283,8 +283,16 @@ jobs: - name: Show test failure summary if: ${{ failure() && !inputs.build_only }} - working-directory: ${{ runner.os == 'Windows' && format('{0}/{1}', env.BUILD_DIR, inputs.build_type) || env.BUILD_DIR }} + env: + WORKING_DIR: ${{ runner.os == 'Windows' && format('{0}\{1}', env.BUILD_DIR, inputs.build_type) || env.BUILD_DIR }} run: | + if [ ! -d "${WORKING_DIR}" ]; then + echo "Working directory '${WORKING_DIR}' does not exist." + exit 0 + fi + + cd "${WORKING_DIR}" + if [ ! -f unittest.log ]; then echo "unittest.log not found; embedded tests may not have run." exit 0 From 6c2266c5c7cc33543b49c52a4c5319177c8893ed Mon Sep 17 00:00:00 2001 From: Ayaz Salikhov Date: Tue, 12 May 2026 20:24:05 +0100 Subject: [PATCH 03/21] refactor: Remove erroneous base_uint ctor from container (#7123) --- include/xrpl/basics/base_uint.h | 27 +++++++++++++++++-- .../ledger/helpers/AccountRootHelpers.cpp | 2 +- src/libxrpl/protocol/AccountID.cpp | 4 +-- src/libxrpl/protocol/Indexes.cpp | 2 +- src/libxrpl/protocol/PublicKey.cpp | 2 +- src/libxrpl/protocol/STIssue.cpp | 4 +-- src/libxrpl/protocol/STPathSet.cpp | 2 +- src/libxrpl/protocol/STVector256.cpp | 2 +- src/libxrpl/protocol/Seed.cpp | 2 +- src/test/app/Invariants_test.cpp | 2 +- src/test/basics/base_uint_test.cpp | 4 +-- src/test/protocol/STParsedJSON_test.cpp | 14 +++++----- .../ledger/detail/LedgerReplayMsgHandler.cpp | 18 +++++++------ src/xrpld/overlay/detail/PeerImp.cpp | 24 ++++++++--------- 14 files changed, 68 insertions(+), 41 deletions(-) diff --git a/include/xrpl/basics/base_uint.h b/include/xrpl/basics/base_uint.h index 3f38d4049b..727768a69a 100644 --- a/include/xrpl/basics/base_uint.h +++ b/include/xrpl/basics/base_uint.h @@ -46,6 +46,11 @@ struct IsContiguousContainer : std::true_type { }; +template +struct AlwaysFalseT : std::bool_constant +{ +}; + } // namespace detail /** Integers of any length that is a multiple of 32-bits @@ -272,10 +277,28 @@ public: std::is_trivially_copyable_v>> explicit BaseUInt(Container const& c) { + // Use AlwaysFalseT so the static_assert condition is dependent + // and only triggers when this constructor template is instantiated. + static_assert( + detail::AlwaysFalseT::value, + "This constructor is not intended to be used and will be soon removed. " + "Use base_uint::fromRaw instead."); + } + + template < + class Container, + class = std::enable_if_t< + detail::IsContiguousContainer::value && + std::is_trivially_copyable_v>> + static BaseUInt + fromRaw(Container const& c) + { + BaseUInt result; XRPL_ASSERT( c.size() * sizeof(typename Container::value_type) == size(), - "xrpl::BaseUInt::BaseUInt(Container auto) : input size match"); - std::memcpy(data_.data(), c.data(), size()); + "xrpl::BaseUInt::fromRaw(Container auto) : input size match"); + std::memcpy(result.data_.data(), c.data(), size()); + return result; } template diff --git a/src/libxrpl/ledger/helpers/AccountRootHelpers.cpp b/src/libxrpl/ledger/helpers/AccountRootHelpers.cpp index 40a41d236a..160629d835 100644 --- a/src/libxrpl/ledger/helpers/AccountRootHelpers.cpp +++ b/src/libxrpl/ledger/helpers/AccountRootHelpers.cpp @@ -152,7 +152,7 @@ pseudoAccountAddress(ReadView const& view, uint256 const& pseudoOwnerKey) RipeshaHasher rsh; auto const hash = sha512Half(i, view.header().parentHash, pseudoOwnerKey); rsh(hash.data(), hash.size()); - AccountID const ret{static_cast(rsh)}; + AccountID const ret = AccountID::fromRaw(static_cast(rsh)); if (!view.read(keylet::account(ret))) return ret; } diff --git a/src/libxrpl/protocol/AccountID.cpp b/src/libxrpl/protocol/AccountID.cpp index 6639835b41..76982560de 100644 --- a/src/libxrpl/protocol/AccountID.cpp +++ b/src/libxrpl/protocol/AccountID.cpp @@ -105,7 +105,7 @@ parseBase58(std::string const& s) auto const result = decodeBase58Token(s, TokenType::AccountID); if (result.size() != AccountID::kBYTES) return std::nullopt; - return AccountID{result}; + return AccountID::fromRaw(result); } //------------------------------------------------------------------------------ @@ -150,7 +150,7 @@ calcAccountID(PublicKey const& pk) RipeshaHasher rsh; rsh(pk.data(), pk.size()); - return AccountID{static_cast(rsh)}; + return AccountID::fromRaw(static_cast(rsh)); } AccountID const& diff --git a/src/libxrpl/protocol/Indexes.cpp b/src/libxrpl/protocol/Indexes.cpp index a8ade0de0f..a74e6cedf4 100644 --- a/src/libxrpl/protocol/Indexes.cpp +++ b/src/libxrpl/protocol/Indexes.cpp @@ -385,7 +385,7 @@ nftpageMin(AccountID const& owner) { std::array buf{}; std::memcpy(buf.data(), owner.data(), owner.size()); - return {ltNFTOKEN_PAGE, uint256{buf}}; + return {ltNFTOKEN_PAGE, uint256::fromRaw(buf)}; } Keylet diff --git a/src/libxrpl/protocol/PublicKey.cpp b/src/libxrpl/protocol/PublicKey.cpp index ad88e60fe7..c38fb781c9 100644 --- a/src/libxrpl/protocol/PublicKey.cpp +++ b/src/libxrpl/protocol/PublicKey.cpp @@ -297,7 +297,7 @@ calcNodeID(PublicKey const& pk) RipeshaHasher h; h(pk.data(), pk.size()); - return NodeID{static_cast(h)}; + return NodeID::fromRaw(static_cast(h)); } } // namespace xrpl diff --git a/src/libxrpl/protocol/STIssue.cpp b/src/libxrpl/protocol/STIssue.cpp index c0019c334f..c9b8109e32 100644 --- a/src/libxrpl/protocol/STIssue.cpp +++ b/src/libxrpl/protocol/STIssue.cpp @@ -28,7 +28,7 @@ STIssue::STIssue(SerialIter& sit, SField const& name) : STBase{name} { auto const currencyOrAccount = sit.get160(); - if (isXRP(static_cast(currencyOrAccount))) + if (isXRP(Currency::fromRaw(currencyOrAccount))) { asset_ = xrpIssue(); } @@ -39,7 +39,7 @@ STIssue::STIssue(SerialIter& sit, SField const& name) : STBase{name} // - 160 bits MPT issuer account // - 160 bits black hole account // - 32 bits sequence - AccountID const account = static_cast(sit.get160()); + AccountID const account = AccountID::fromRaw(sit.get160()); // MPT if (noAccount() == account) { diff --git a/src/libxrpl/protocol/STPathSet.cpp b/src/libxrpl/protocol/STPathSet.cpp index 9c0cc20e96..35d5fd782b 100644 --- a/src/libxrpl/protocol/STPathSet.cpp +++ b/src/libxrpl/protocol/STPathSet.cpp @@ -93,7 +93,7 @@ STPathSet::STPathSet(SerialIter& sit, SField const& name) : STBase(name) XRPL_ASSERT( !(hasCurrency && hasMPT), "xrpl::STPathSet::STPathSet : not has Currency and MPT"); if (hasCurrency) - asset = static_cast(sit.get160()); + asset = Currency::fromRaw(sit.get160()); if (hasMPT) asset = sit.get192(); diff --git a/src/libxrpl/protocol/STVector256.cpp b/src/libxrpl/protocol/STVector256.cpp index 75b833dd7a..7aca309667 100644 --- a/src/libxrpl/protocol/STVector256.cpp +++ b/src/libxrpl/protocol/STVector256.cpp @@ -30,7 +30,7 @@ STVector256::STVector256(SerialIter& sit, SField const& name) : STBase(name) value_.reserve(cnt); for (std::size_t i = 0; i != cnt; ++i) - value_.emplace_back(slice.substr(i * uint256::size(), uint256::size())); + value_.push_back(uint256::fromRaw(slice.substr(i * uint256::size(), uint256::size()))); } STBase* diff --git a/src/libxrpl/protocol/Seed.cpp b/src/libxrpl/protocol/Seed.cpp index f15d4dcff0..42b2e85498 100644 --- a/src/libxrpl/protocol/Seed.cpp +++ b/src/libxrpl/protocol/Seed.cpp @@ -105,7 +105,7 @@ parseGenericSeed(std::string const& str, bool rfc1751) if (RFC1751::getKeyFromEnglish(key, str) == 1) { Blob const blob(key.rbegin(), key.rend()); - return Seed{uint128{blob}}; + return Seed{uint128::fromRaw(blob)}; } } diff --git a/src/test/app/Invariants_test.cpp b/src/test/app/Invariants_test.cpp index 5864d66723..c9f0a04a18 100644 --- a/src/test/app/Invariants_test.cpp +++ b/src/test/app/Invariants_test.cpp @@ -3401,7 +3401,7 @@ class Invariants_test : public beast::unit_test::Suite sleShares->at(sfFlags) = 0; // Setting wrong pseudo account ID - sleShares->at(sfIssuer) = AccountID(uint160(42)); + sleShares->at(sfIssuer) = AccountID(42); sleShares->at(sfOutstandingAmount) = 0; sleShares->at(sfSequence) = sequence; diff --git a/src/test/basics/base_uint_test.cpp b/src/test/basics/base_uint_test.cpp index 8d5b56d480..603536bbe0 100644 --- a/src/test/basics/base_uint_test.cpp +++ b/src/test/basics/base_uint_test.cpp @@ -134,7 +134,7 @@ struct base_uint_test : beast::unit_test::Suite Blob const raw{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12}; BEAST_EXPECT(test96::kBYTES == raw.size()); - test96 u{raw}; + test96 u = test96::fromRaw(raw); uset.insert(u); BEAST_EXPECT(raw.size() == u.size()); BEAST_EXPECT(to_string(u) == "0102030405060708090A0B0C"); @@ -155,7 +155,7 @@ struct base_uint_test : beast::unit_test::Suite // back into another base_uint (w) for comparison with the original Nonhash<96> h{}; hash_append(h, u); - test96 const w{std::vector(h.data.begin(), h.data.end())}; + test96 const w = test96::fromRaw(std::vector(h.data.begin(), h.data.end())); BEAST_EXPECT(w == u); test96 v{~u}; diff --git a/src/test/protocol/STParsedJSON_test.cpp b/src/test/protocol/STParsedJSON_test.cpp index 6b863644a3..bc5c72b4fc 100644 --- a/src/test/protocol/STParsedJSON_test.cpp +++ b/src/test/protocol/STParsedJSON_test.cpp @@ -393,7 +393,7 @@ class STParsedJSON_test : public beast::unit_test::Suite 0xCD, 0xEF}; // NOLINTNEXTLINE(bugprone-unchecked-optional-access) - BEAST_EXPECT(obj.object->getFieldH128(sfEmailHash) == uint128{expected}); + BEAST_EXPECT(obj.object->getFieldH128(sfEmailHash) == uint128::fromRaw(expected)); } // Valid lowercase hex string for UInt128 @@ -488,8 +488,9 @@ class STParsedJSON_test : public beast::unit_test::Suite std::array const expected = {0x01, 0x23, 0x45, 0x67, 0x89, 0xAB, 0xCD, 0xEF, 0x01, 0x23, 0x45, 0x67, 0x89, 0xAB, 0xCD, 0xEF, 0x01, 0x23, 0x45, 0x67}; - // NOLINTNEXTLINE(bugprone-unchecked-optional-access) - BEAST_EXPECT(obj.object->getFieldH160(sfTakerPaysCurrency) == uint160{expected}); + BEAST_EXPECT( + // NOLINTNEXTLINE(bugprone-unchecked-optional-access) + obj.object->getFieldH160(sfTakerPaysCurrency) == uint160::fromRaw(expected)); } // Valid lowercase hex string for UInt160 { @@ -575,8 +576,9 @@ class STParsedJSON_test : public beast::unit_test::Suite std::array const expected = { 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF}; - // NOLINTNEXTLINE(bugprone-unchecked-optional-access) - BEAST_EXPECT(obj.object->getFieldH192(sfMPTokenIssuanceID) == uint192{expected}); + BEAST_EXPECT( + // NOLINTNEXTLINE(bugprone-unchecked-optional-access) + obj.object->getFieldH192(sfMPTokenIssuanceID) == uint192::fromRaw(expected)); } // Valid lowercase hex string for UInt192 @@ -676,7 +678,7 @@ class STParsedJSON_test : public beast::unit_test::Suite 0x67, 0x89, 0xAB, 0xCD, 0xEF, 0x01, 0x23, 0x45, 0x67, 0x89, 0xAB, 0xCD, 0xEF, 0x01, 0x23, 0x45, 0x67, 0x89, 0xAB, 0xCD, 0xEF}; // NOLINTNEXTLINE(bugprone-unchecked-optional-access) - BEAST_EXPECT(obj.object->getFieldH256(sfLedgerHash) == uint256{expected}); + BEAST_EXPECT(obj.object->getFieldH256(sfLedgerHash) == uint256::fromRaw(expected)); } // Valid lowercase hex string for UInt256 { diff --git a/src/xrpld/app/ledger/detail/LedgerReplayMsgHandler.cpp b/src/xrpld/app/ledger/detail/LedgerReplayMsgHandler.cpp index 19dba2e893..07738d99f4 100644 --- a/src/xrpld/app/ledger/detail/LedgerReplayMsgHandler.cpp +++ b/src/xrpld/app/ledger/detail/LedgerReplayMsgHandler.cpp @@ -56,8 +56,8 @@ LedgerReplayMsgHandler::processProofPathRequest( reply.set_ledgerhash(packet.ledgerhash()); reply.set_type(packet.type()); - uint256 const key(packet.key()); - uint256 const ledgerHash(packet.ledgerhash()); + uint256 const key = uint256::fromRaw(packet.key()); + uint256 const ledgerHash = uint256::fromRaw(packet.ledgerhash()); auto ledger = app_.getLedgerMaster().getLedgerByHash(ledgerHash); if (!ledger) { @@ -107,7 +107,8 @@ LedgerReplayMsgHandler::processProofPathResponse( { protocol::TMProofPathResponse const& reply = *msg; if (reply.has_error() || !reply.has_key() || !reply.has_ledgerhash() || !reply.has_type() || - !reply.has_ledgerheader() || reply.path_size() == 0) + !reply.has_ledgerheader() || reply.path_size() == 0 || + reply.ledgerhash().size() != uint256::size() || reply.key().size() != uint256::size()) { JLOG(journal_.debug()) << "Bad message: Error reply"; return false; @@ -121,7 +122,7 @@ LedgerReplayMsgHandler::processProofPathResponse( // deserialize the header auto info = deserializeHeader({reply.ledgerheader().data(), reply.ledgerheader().size()}); - uint256 const replyHash(reply.ledgerhash()); + uint256 const replyHash = uint256::fromRaw(reply.ledgerhash()); if (calculateLedgerHash(info) != replyHash) { JLOG(journal_.debug()) << "Bad message: Hash mismatch"; @@ -129,7 +130,7 @@ LedgerReplayMsgHandler::processProofPathResponse( } info.hash = replyHash; - uint256 const key(reply.key()); + uint256 const key = uint256::fromRaw(reply.key()); if (key != keylet::skip().key) { JLOG(journal_.debug()) << "Bad message: we only support the short skip list for now. " @@ -185,7 +186,7 @@ LedgerReplayMsgHandler::processReplayDeltaRequest( } reply.set_ledgerhash(packet.ledgerhash()); - uint256 const ledgerHash{packet.ledgerhash()}; + uint256 const ledgerHash = uint256::fromRaw(packet.ledgerhash()); auto ledger = app_.getLedgerMaster().getLedgerByHash(ledgerHash); if (!ledger || !ledger->isImmutable()) { @@ -214,14 +215,15 @@ LedgerReplayMsgHandler::processReplayDeltaResponse( std::shared_ptr const& msg) { protocol::TMReplayDeltaResponse const& reply = *msg; - if (reply.has_error() || !reply.has_ledgerheader()) + if (reply.has_error() || !reply.has_ledgerheader() || !reply.has_ledgerhash() || + reply.ledgerhash().size() != uint256::size()) { JLOG(journal_.debug()) << "Bad message: Error reply"; return false; } auto info = deserializeHeader({reply.ledgerheader().data(), reply.ledgerheader().size()}); - uint256 const replyHash(reply.ledgerhash()); + uint256 const replyHash = uint256::fromRaw(reply.ledgerhash()); if (calculateLedgerHash(info) != replyHash) { JLOG(journal_.debug()) << "Bad message: Hash mismatch"; diff --git a/src/xrpld/overlay/detail/PeerImp.cpp b/src/xrpld/overlay/detail/PeerImp.cpp index 3e572a297f..7d1dd27528 100644 --- a/src/xrpld/overlay/detail/PeerImp.cpp +++ b/src/xrpld/overlay/detail/PeerImp.cpp @@ -212,7 +212,7 @@ PeerImp::run() return ret; if (auto const s = base64Decode(value); s.size() == uint256::size()) - return uint256{s}; + return uint256::fromRaw(s); return std::nullopt; }; @@ -1831,7 +1831,7 @@ PeerImp::onMessage(std::shared_ptr const& m) return; } - uint256 const ledgerHash{m->ledgerhash()}; + uint256 const ledgerHash = uint256::fromRaw(m->ledgerhash()); // Otherwise check if received data for a candidate transaction set if (m->type() == protocol::liTS_CANDIDATE) @@ -1893,8 +1893,8 @@ PeerImp::onMessage(std::shared_ptr const& m) return; } - uint256 const proposeHash{set.currenttxhash()}; - uint256 const prevLedger{set.previousledger()}; + uint256 const proposeHash = uint256::fromRaw(set.currenttxhash()); + uint256 const prevLedger = uint256::fromRaw(set.previousledger()); NetClock::time_point const closeTime{NetClock::duration{set.closetime()}}; @@ -2177,7 +2177,7 @@ PeerImp::onMessage(std::shared_ptr const& m) return; } - uint256 const hash{m->hash()}; + uint256 const hash = uint256::fromRaw(m->hash()); if (m->status() == protocol::tsHAVE) { @@ -2617,7 +2617,7 @@ PeerImp::onMessage(std::shared_ptr const& m) auto const& obj = packet.objects(i); if (obj.has_hash() && stringIsUInt256Sized(obj.hash())) { - uint256 const hash{obj.hash()}; + uint256 const hash = uint256::fromRaw(obj.hash()); // VFALCO TODO Move this someplace more sensible so we dont // need to inject the NodeStore interfaces. std::uint32_t const seq{obj.has_ledgerseq() ? obj.ledgerseq() : 0}; @@ -2687,7 +2687,7 @@ PeerImp::onMessage(std::shared_ptr const& m) if (pLDo) { - uint256 const hash{obj.hash()}; + uint256 const hash = uint256::fromRaw(obj.hash()); app_.getLedgerMaster().addFetchPack( hash, std::make_shared(obj.data().begin(), obj.data().end())); @@ -2739,7 +2739,7 @@ PeerImp::handleHaveTransactions(std::shared_ptr co return; } - uint256 hash(m->hashes(i)); + uint256 hash = uint256::fromRaw(m->hashes(i)); auto txn = app_.getMasterTransaction().fetchFromCache(hash); @@ -2873,7 +2873,7 @@ PeerImp::doFetchPack(std::shared_ptr const& packet) fee_.fee = Resource::kFEE_HEAVY_BURDEN_PEER; - uint256 const hash{packet->ledgerhash()}; + uint256 const hash = uint256::fromRaw(packet->ledgerhash()); std::weak_ptr const weak = shared_from_this(); auto elapsed = UptimeClock::now(); @@ -2908,7 +2908,7 @@ PeerImp::doTransactions(std::shared_ptr const& pack return; } - uint256 hash(obj.hash()); + uint256 hash = uint256::fromRaw(obj.hash()); auto txn = app_.getMasterTransaction().fetchFromCache(hash); @@ -3254,7 +3254,7 @@ PeerImp::getLedger(std::shared_ptr const& m) if (m->has_ledgerhash()) { // Attempt to find ledger by hash - uint256 const ledgerHash{m->ledgerhash()}; + uint256 const ledgerHash = uint256::fromRaw(m->ledgerhash()); ledger = app_.getLedgerMaster().getLedgerByHash(ledgerHash); if (!ledger) { @@ -3333,7 +3333,7 @@ PeerImp::getTxSet(std::shared_ptr const& m) const { JLOG(pJournal_.trace()) << "getTxSet: TX set"; - uint256 const txSetHash{m->ledgerhash()}; + uint256 const txSetHash = uint256::fromRaw(m->ledgerhash()); std::shared_ptr shaMap{app_.getInboundTransactions().getSet(txSetHash, false)}; if (!shaMap) { From 8012b5d34f533c559a7f9fababbe38bfedaa6b7d Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Tue, 12 May 2026 15:25:11 -0400 Subject: [PATCH 04/21] fix: Fix touchy "funds are conserved" assertion in LoanPay (#6231) (#6967) Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- src/libxrpl/ledger/helpers/LendingHelpers.cpp | 14 +- .../tx/transactors/lending/LoanPay.cpp | 219 +++++++++++++++--- src/test/app/Loan_test.cpp | 153 +++++++++++- 3 files changed, 345 insertions(+), 41 deletions(-) diff --git a/src/libxrpl/ledger/helpers/LendingHelpers.cpp b/src/libxrpl/ledger/helpers/LendingHelpers.cpp index 89d5ed8e35..a8bee7aecb 100644 --- a/src/libxrpl/ledger/helpers/LendingHelpers.cpp +++ b/src/libxrpl/ledger/helpers/LendingHelpers.cpp @@ -1963,8 +1963,18 @@ loanMakePayment( // ------------------------------------------------------------- // overpayment handling + // + // If the "fixSecurity3_1_3" amendment is enabled, truncate "amount", + // at the loan scale. If the raw value is used, the overpayment + // amount could be meaningless dust. Trying to process such a small + // amount will, at best, waste time when all the result values round + // to zero. At worst, it can cause logical errors with tiny amounts + // of interest that don't add up correctly. + auto const roundedAmount = view.rules().enabled(fixSecurity3_1_3) + ? roundToAsset(asset, amount, loanScale, Number::RoundingMode::TowardsZero) + : amount; if (paymentType == LoanPaymentType::Overpayment && loan->isFlag(lsfLoanOverpayment) && - paymentRemainingProxy > 0 && totalPaid < amount && + paymentRemainingProxy > 0 && totalPaid < roundedAmount && numPayments < kLOAN_MAXIMUM_PAYMENTS_PER_TRANSACTION) { TenthBips32 const overpaymentInterestRate{loan->at(sfOverpaymentInterestRate)}; @@ -1973,7 +1983,7 @@ loanMakePayment( // It shouldn't be possible for the overpayment to be greater than // totalValueOutstanding, because that would have been processed as // another normal payment. But cap it just in case. - Number const overpayment = std::min(amount - totalPaid, *totalValueOutstandingProxy); + Number const overpayment = std::min(roundedAmount - totalPaid, *totalValueOutstandingProxy); detail::ExtendedPaymentComponents const overpaymentComponents = detail::computeOverpaymentComponents( diff --git a/src/libxrpl/tx/transactors/lending/LoanPay.cpp b/src/libxrpl/tx/transactors/lending/LoanPay.cpp index ef93f0dc1b..d7147128cc 100644 --- a/src/libxrpl/tx/transactors/lending/LoanPay.cpp +++ b/src/libxrpl/tx/transactors/lending/LoanPay.cpp @@ -30,6 +30,7 @@ #include #include #include +#include namespace xrpl { @@ -162,6 +163,7 @@ LoanPay::calculateBaseFee(ReadView const& view, STTx const& tx) Number const numPaymentEstimate = static_cast(amount / regularPayment); // Charge one base fee per paymentsPerFeeIncrement payments, rounding up. + // This set round is safe because there's a mode guard just above Number::setround(Number::RoundingMode::Upward); auto const feeIncrements = std::max( std::int64_t(1), @@ -463,9 +465,10 @@ LoanPay::doApply() // Vault object state changes view.update(vaultSle); + Number const assetsAvailableBefore = *assetsAvailableProxy; + Number const assetsTotalBefore = *assetsTotalProxy; #if !NDEBUG { - Number const assetsAvailableBefore = *assetsAvailableProxy; Number const pseudoAccountBalanceBefore = accountHolds( view, vaultPseudoAccount, @@ -489,16 +492,6 @@ LoanPay::doApply() "xrpl::LoanPay::doApply", "assets available must not be greater than assets outstanding"); - if (*assetsAvailableProxy > *assetsTotalProxy) - { - // LCOV_EXCL_START - JLOG(j_.fatal()) << "Vault assets available must not be greater " - "than assets outstanding. Available: " - << *assetsAvailableProxy << ", Total: " << *assetsTotalProxy; - return tecINTERNAL; - // LCOV_EXCL_STOP - } - JLOG(j_.debug()) << "total paid to vault raw: " << totalPaidToVaultRaw << ", total paid to vault rounded: " << totalPaidToVaultRounded << ", total paid to broker: " << totalPaidToBroker @@ -524,12 +517,68 @@ LoanPay::doApply() associateAsset(*vaultSle, asset); // Duplicate some checks after rounding + Number const assetsAvailableAfter = *assetsAvailableProxy; + Number const assetsTotalAfter = *assetsTotalProxy; + XRPL_ASSERT_PARTS( - *assetsAvailableProxy <= *assetsTotalProxy, + assetsAvailableAfter <= assetsTotalAfter, "xrpl::LoanPay::doApply", "assets available must not be greater than assets outstanding"); + if (assetsAvailableAfter == assetsAvailableBefore) + { + // An unchanged assetsAvailable indicates that the amount paid to the + // vault was zero, or rounded to zero. That should be impossible, but I + // can't rule it out for extreme edge cases, so fail gracefully if it + // happens. + // + // LCOV_EXCL_START + JLOG(j_.warn()) << "LoanPay: Vault assets available unchanged after rounding: " // + << "Before: " << assetsAvailableBefore // + << ", After: " << assetsAvailableAfter; + return tecPRECISION_LOSS; + // LCOV_EXCL_STOP + } + if (paymentParts->valueChange != beast::kZERO && assetsTotalAfter == assetsTotalBefore) + { + // Non-zero valueChange with an unchanged assetsTotal indicates that the + // actual value change rounded to zero. That should be impossible, but I + // can't rule it out for extreme edge cases, so fail gracefully if it + // happens. + // + // LCOV_EXCL_START + JLOG(j_.warn()) + << "LoanPay: Vault assets expected change, but unchanged after rounding: " // + << "Before: " << assetsTotalBefore // + << ", After: " << assetsTotalAfter // + << ", ValueChange: " << paymentParts->valueChange; + return tecPRECISION_LOSS; + // LCOV_EXCL_STOP + } + if (paymentParts->valueChange == beast::kZERO && assetsTotalAfter != assetsTotalBefore) + { + // A change in assetsTotal when there was no valueChange indicates that + // something really weird happened. That should be flat out impossible. + // + // LCOV_EXCL_START + JLOG(j_.fatal()) << "LoanPay: Vault assets changed unexpectedly after rounding: " // + << "Before: " << assetsTotalBefore // + << ", After: " << assetsTotalAfter // + << ", ValueChange: " << paymentParts->valueChange; + return tecINTERNAL; + // LCOV_EXCL_STOP + } + if (assetsAvailableAfter > assetsTotalAfter) + { + // Assets available are not allowed to be larger than assets total. + // LCOV_EXCL_START + JLOG(j_.fatal()) << "LoanPay: Vault assets available must not be greater " + "than assets outstanding. Available: " + << assetsAvailableAfter << ", Total: " << assetsTotalAfter; + return tecINTERNAL; + // LCOV_EXCL_STOP + } -#if !NDEBUG + // These three values are used to check that funds are conserved after the transfers auto const accountBalanceBefore = accountHolds( view, account_, @@ -557,7 +606,6 @@ LoanPay::doApply() AuthHandling::IgnoreAuth, j_, SpendableHandling::FullBalance); -#endif if (totalPaidToVaultRounded != beast::kZERO) { @@ -593,19 +641,22 @@ LoanPay::doApply() return ter; #if !NDEBUG - Number const assetsAvailableAfter = *assetsAvailableProxy; - Number const pseudoAccountBalanceAfter = accountHolds( - view, - vaultPseudoAccount, - asset, - FreezeHandling::IgnoreFreeze, - AuthHandling::IgnoreAuth, - j_); - XRPL_ASSERT_PARTS( - assetsAvailableAfter == pseudoAccountBalanceAfter, - "xrpl::LoanPay::doApply", - "vault pseudo balance agrees after"); + { + Number const pseudoAccountBalanceAfter = accountHolds( + view, + vaultPseudoAccount, + asset, + FreezeHandling::IgnoreFreeze, + AuthHandling::IgnoreAuth, + j_); + XRPL_ASSERT_PARTS( + assetsAvailableAfter == pseudoAccountBalanceAfter, + "xrpl::LoanPay::doApply", + "vault pseudo balance agrees after"); + } +#endif + // Check that funds are conserved auto const accountBalanceAfter = accountHolds( view, account_, @@ -633,14 +684,121 @@ LoanPay::doApply() AuthHandling::IgnoreAuth, j_, SpendableHandling::FullBalance); + auto const balanceScale = [&]() { + // Find a reasonable scale to use for the balance comparisons. + // + // First find the minimum and maximum exponent of all the non-zero balances, before and + // after. If min and max are equal, use that value. If they are not, use "max + 1" to reduce + // rounding discrepancies without making the result meaningless. Cap the scale at + // STAmount::kMAX_OFFSET, just in case the numbers are all very large. + std::vector exponents; + exponents.reserve(6); + for (auto const& a : { + accountBalanceBefore, + vaultBalanceBefore, + brokerBalanceBefore, + accountBalanceAfter, + vaultBalanceAfter, + brokerBalanceAfter, + }) + { + // Exclude zeroes + if (a != beast::kZERO) + exponents.push_back(a.exponent()); + } + if (exponents.empty()) + { + UNREACHABLE("xrpl::LoanPay::doApply : all zeroes"); + return 0; + } + auto const [minItr, maxItr] = std::ranges::minmax_element(exponents); + auto const min = *minItr; + auto const max = *maxItr; + JLOG(j_.trace()) << "Min scale: " << min << ", max scale: " << max; + // IOU rounding can be interesting. We want all the balance checks to agree, but don't want + // to round to such an extreme that it becomes meaningless. e.g. Everything rounds to one + // digit. So add 1 to the max (reducing the number of digits after the decimal point by 1) + // if the scales are not already all the same. + return std::min(min == max ? max : max + 1, STAmount::kMAX_OFFSET); + }(); + + // No object changes are made below this point XRPL_ASSERT_PARTS( - accountBalanceBefore + vaultBalanceBefore + brokerBalanceBefore == - accountBalanceAfter + vaultBalanceAfter + brokerBalanceAfter, + Number::getround() == Number::RoundingMode::ToNearest, "xrpl::LoanPay::doApply", - "funds are conserved (with rounding)"); + "Number rounding ToNearest"); + NumberRoundModeGuard const mg(Number::RoundingMode::ToNearest); + + auto const accountBalanceBeforeRounded = roundToScale(accountBalanceBefore, balanceScale); + auto const vaultBalanceBeforeRounded = roundToScale(vaultBalanceBefore, balanceScale); + auto const brokerBalanceBeforeRounded = roundToScale(brokerBalanceBefore, balanceScale); + + auto const totalBalanceBefore = accountBalanceBefore + vaultBalanceBefore + brokerBalanceBefore; + auto const totalBalanceBeforeRounded = roundToScale(totalBalanceBefore, balanceScale); + + JLOG(j_.trace()) << "Before: " // + << "account " << Number(accountBalanceBeforeRounded) << " (" + << Number(accountBalanceBefore) << ")" + << ", vault " << Number(vaultBalanceBeforeRounded) << " (" + << Number(vaultBalanceBefore) << ")" + << ", broker " << Number(brokerBalanceBeforeRounded) << " (" + << Number(brokerBalanceBefore) << ")" + << ", total " << Number(totalBalanceBeforeRounded) << " (" + << Number(totalBalanceBefore) << ")"; + + auto const accountBalanceAfterRounded = roundToScale(accountBalanceAfter, balanceScale); + auto const vaultBalanceAfterRounded = roundToScale(vaultBalanceAfter, balanceScale); + auto const brokerBalanceAfterRounded = roundToScale(brokerBalanceAfter, balanceScale); + + auto const totalBalanceAfter = accountBalanceAfter + vaultBalanceAfter + brokerBalanceAfter; + auto const totalBalanceAfterRounded = roundToScale(totalBalanceAfter, balanceScale); + + JLOG(j_.trace()) << "After: " // + << "account " << Number(accountBalanceAfterRounded) << " (" + << Number(accountBalanceAfter) << ")" + << ", vault " << Number(vaultBalanceAfterRounded) << " (" + << Number(vaultBalanceAfter) << ")" + << ", broker " << Number(brokerBalanceAfterRounded) << " (" + << Number(brokerBalanceAfter) << ")" + << ", total " << Number(totalBalanceAfterRounded) << " (" + << Number(totalBalanceAfter) << ")"; + + auto const accountBalanceChange = accountBalanceAfter - accountBalanceBefore; + auto const vaultBalanceChange = vaultBalanceAfter - vaultBalanceBefore; + auto const brokerBalanceChange = brokerBalanceAfter - brokerBalanceBefore; + + auto const totalBalanceChange = accountBalanceChange + vaultBalanceChange + brokerBalanceChange; + auto const totalBalanceChangeRounded = roundToScale(totalBalanceChange, balanceScale); + + JLOG(j_.trace()) << "Changes: " // + << "account " << to_string(accountBalanceChange) // + << ", vault " << to_string(vaultBalanceChange) // + << ", broker " << to_string(brokerBalanceChange) // + << ", total " << to_string(totalBalanceChangeRounded) << " (" + << Number(totalBalanceChange) << ")"; + + bool const goodRounding = totalBalanceBeforeRounded == totalBalanceAfterRounded || + totalBalanceChangeRounded == beast::kZERO; + if (totalBalanceBeforeRounded != totalBalanceAfterRounded) + { + JLOG((goodRounding ? j_.debug() : j_.warn())) + << "Total rounded balances don't match" + << (totalBalanceChangeRounded == beast::kZERO ? ", but total changes do" : ""); + } + if (totalBalanceChangeRounded != beast::kZERO) + { + JLOG((goodRounding ? j_.debug() : j_.warn())) + << "Total balance changes don't match" + << (totalBalanceBeforeRounded == totalBalanceAfterRounded ? ", but total balances do" + : ""); + } + + // Rounding for IOUs can be weird, so check a few different ways to show + // that funds are conserved. XRPL_ASSERT_PARTS( - accountBalanceAfter >= beast::kZERO, "xrpl::LoanPay::doApply", "positive account balance"); + goodRounding, "xrpl::LoanPay::doApply", "funds are conserved (with rounding)"); + XRPL_ASSERT_PARTS( accountBalanceAfter < accountBalanceBefore || account_ == asset.getIssuer(), "xrpl::LoanPay::doApply", @@ -661,7 +819,6 @@ LoanPay::doApply() vaultBalanceAfter > vaultBalanceBefore || brokerBalanceAfter > brokerBalanceBefore, "xrpl::LoanPay::doApply", "vault and/or broker balance increased"); -#endif return tesSUCCESS; } diff --git a/src/test/app/Loan_test.cpp b/src/test/app/Loan_test.cpp index 67780414c0..70997b1dcd 100644 --- a/src/test/app/Loan_test.cpp +++ b/src/test/app/Loan_test.cpp @@ -370,16 +370,11 @@ protected: env.balance(vaultPseudo, broker.asset).number()); if (ownerCount == 0) { - // Allow some slop for rounding IOUs - - // TODO: This needs to be an exact match once all the - // other rounding issues are worked out. + // The Vault must be perfectly balanced if there + // are no loans outstanding auto const total = vaultSle->at(sfAssetsTotal); auto const available = vaultSle->at(sfAssetsAvailable); - env.test.BEAST_EXPECT( - total == available || - (!broker.asset.integral() && available != 0 && - ((total - available) / available < Number(1, -6)))); + env.test.BEAST_EXPECT(total == available); env.test.BEAST_EXPECT(vaultSle->at(sfLossUnrealized) == 0); } } @@ -7177,6 +7172,144 @@ protected: BEAST_EXPECT(afterSecondCoverAvailable == 0); } + void + testYieldTheftRounding(std::uint32_t flags) + { + testcase("Rounding manipulation does not permit yield theft"); + using namespace jtx; + using namespace loan; + + // 1. Setup Environment + Env env(*this, all_); + Account const issuer{"issuer"}; + Account const lender{"lender"}; + Account const borrower{"borrower"}; + + env.fund(XRP(1000), issuer, lender, borrower); + env.close(); + + // 2. Asset Selection + PrettyAsset const iou = issuer["USD"]; + env(trust(lender, iou(100'000'000))); + env(trust(borrower, iou(100'000'000))); + env(pay(issuer, lender, iou(100'000'000))); + env(pay(issuer, borrower, iou(100'000'000))); + env.close(); + + // 3. Create Vault and Broker with High Debt Limit (100M) + auto const brokerInfo = createVaultAndBroker( + env, + iou, + lender, + { + .vaultDeposit = 5'000'000, + .debtMax = Number{100'000'000}, + .coverDeposit = 500'000, + }); + auto const [currentSeq, vaultKeylet] = [&]() { + auto const brokerSle = env.le(keylet::loanbroker(brokerInfo.brokerID)); + if (!BEAST_EXPECT(brokerSle)) + return std::make_tuple(0u, keylet::unchecked(beast::kZERO)); + auto const currentSeq = brokerSle->at(sfLoanSequence); + auto const vaultKeylet = keylet::vault(brokerSle->at(sfVaultID)); + return std::make_tuple(currentSeq, vaultKeylet); + }(); + + // 4. Loan Parameters (Attack Vector) + Number const principal = 1'000'000; + TenthBips32 const interestRate = TenthBips32{1}; // 0.001% + std::uint32_t const paymentInterval = 86400; + std::uint32_t const paymentTotal = 3650; + + auto const loanSetFee = Fee(env.current()->fees().base * 2); + env(set(borrower, brokerInfo.brokerID, iou(principal).value(), flags), + Sig(sfCounterpartySignature, lender), + loan::kINTEREST_RATE(interestRate), + loan::kPAYMENT_INTERVAL(paymentInterval), + loan::kPAYMENT_TOTAL(paymentTotal), + Fee(loanSetFee)); + env.close(); + + // --- RETRIEVE OBJECTS & SETUP ATTACK --- + + auto borrowerBalance = [&]() { return env.balance(borrower, iou); }; + auto const borrowerScale = static_cast(borrowerBalance()).exponent(); + + auto const loanKeylet = keylet::loan(brokerInfo.brokerID, currentSeq); + auto const maybePeriodicPayment = [&]() -> std::optional { + auto const loanSle = env.le(loanKeylet); + if (!BEAST_EXPECT(loanSle)) + return std::nullopt; + // Construct Payment + return STAmount{iou, loanSle->at(sfPeriodicPayment)}; + }(); + if (!maybePeriodicPayment) + return; + auto const periodicPayment = *maybePeriodicPayment; + auto const roundedPayment = + roundToScale(periodicPayment, borrowerScale, Number::RoundingMode::Upward); + + // ATTACK: Add dust buffer (1e-9) to force 'excess' logic execution + STAmount const paymentBuffer{iou, Number(1, -9)}; + STAmount const attackPayment = periodicPayment + paymentBuffer; + + auto const maybeInitialVaultAssets = [&]() -> std::optional { + auto const vault = env.le(vaultKeylet); + if (!BEAST_EXPECT(vault)) + return std::nullopt; + return vault->at(sfAssetsTotal); + }(); + if (!maybeInitialVaultAssets) + return; + auto const initialVaultAssets = *maybeInitialVaultAssets; + + // 5. Execution Loop + int yieldTheftCount = 0; + auto previousAssetsTotal = initialVaultAssets; + + for (int i = 0; i < 100; ++i) + { + auto const balanceBefore = borrowerBalance(); + env(pay(borrower, loanKeylet.key, attackPayment, flags)); + env.close(); + auto const borrowerDelta = balanceBefore - borrowerBalance(); + BEAST_EXPECT(borrowerDelta.signum() == roundedPayment.signum()); + + auto const loanSle = env.le(loanKeylet); + if (!BEAST_EXPECT(loanSle)) + break; + auto const updatedPayment = STAmount{iou, loanSle->at(sfPeriodicPayment)}; + BEAST_EXPECT( + (roundToScale(updatedPayment, borrowerScale, Number::RoundingMode::Upward) == + roundedPayment)); + BEAST_EXPECT( + (updatedPayment == periodicPayment) || + (flags == tfLoanOverpayment && i >= 2 && updatedPayment < periodicPayment)); + + auto const currentVaultSle = env.le(vaultKeylet); + if (!BEAST_EXPECT(currentVaultSle)) + break; + + auto const currentAssetsTotal = currentVaultSle->at(sfAssetsTotal); + auto const delta = currentAssetsTotal - previousAssetsTotal; + + BEAST_EXPECT( + (delta == beast::kZERO && borrowerDelta <= roundedPayment) || + (delta > beast::kZERO && borrowerDelta > roundedPayment)); + + // If tx succeeded but Assets Total didn't change, interest was + // stolen. + if (delta == beast::kZERO && borrowerDelta > roundedPayment) + { + yieldTheftCount++; + } + + previousAssetsTotal = currentAssetsTotal; + } + + BEAST_EXPECTS(yieldTheftCount == 0, std::to_string(yieldTheftCount)); + } + // Tests that vault withdrawals work correctly when the vault has unrealized // loss from an impaired loan, ensuring the invariant check properly // accounts for the loss. @@ -7497,6 +7630,10 @@ public: testLoanPayLateFullPaymentBypassesPenalties(); testLoanCoverMinimumRoundingExploit(); #endif + for (auto const flags : {0u, tfLoanOverpayment}) + { + testYieldTheftRounding(flags); + } testBugInterestDueDeltaCrash(); testFullLifecycleVaultPnLNearZeroRate(); From 45b1f4dbeb28ddfbc5b383d3ca45f6b824a0fe32 Mon Sep 17 00:00:00 2001 From: rrmanukyan Date: Tue, 12 May 2026 15:27:07 -0400 Subject: [PATCH 05/21] refactor: Fill txJson based on apiVersion (#7109) Co-authored-by: Ayaz Salikhov --- src/xrpld/app/ledger/detail/LedgerToJson.cpp | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/src/xrpld/app/ledger/detail/LedgerToJson.cpp b/src/xrpld/app/ledger/detail/LedgerToJson.cpp index 10a3e5c0bd..7c03e22580 100644 --- a/src/xrpld/app/ledger/detail/LedgerToJson.cpp +++ b/src/xrpld/app/ledger/detail/LedgerToJson.cpp @@ -297,13 +297,24 @@ fillJsonQueue(json::Value& json, LedgerFill const& fill) txJson["last_result"] = transToken(*tx.lastResult); auto&& temp = fillJsonTx(fill, bBinary, bExpanded, tx.txn, nullptr); - if (fill.context->apiVersion > 1) + if (temp.isObject()) { - copyFrom(txJson, temp); + if (fill.context->apiVersion > 1) + { + copyFrom(txJson, temp); + } + else + { + copyFrom(txJson[jss::tx], temp); + } + } + else if (fill.context->apiVersion > 1) + { + txJson[jss::hash] = temp; } else { - copyFrom(txJson[jss::tx], temp); + txJson[jss::tx] = temp; } } } From 448ae8b9df6a73cede0e23e079165397ad852777 Mon Sep 17 00:00:00 2001 From: Sergey Kuznetsov Date: Tue, 12 May 2026 21:13:36 +0100 Subject: [PATCH 06/21] fix: Improve json parsing of currency issuers (#7110) --- src/libxrpl/protocol/Issue.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libxrpl/protocol/Issue.cpp b/src/libxrpl/protocol/Issue.cpp index 33ad3a0835..8de457a5cf 100644 --- a/src/libxrpl/protocol/Issue.cpp +++ b/src/libxrpl/protocol/Issue.cpp @@ -128,7 +128,7 @@ issueFromJson(json::Value const& v) } auto const issuer = parseBase58(issStr.asString()); - if (!issuer) + if (!issuer || *issuer == noAccount() || *issuer == xrpAccount()) { Throw("issueFromJson issuer must be a valid account"); } From 590906dadf153898076b59f991b7a337c6503d0f Mon Sep 17 00:00:00 2001 From: Olek <115580134+oleks-rip@users.noreply.github.com> Date: Tue, 12 May 2026 16:15:17 -0400 Subject: [PATCH 07/21] fix: Use transaction sequence numbers in permissioned domains (#7129) --- .../permissioned_domain/PermissionedDomainSet.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/libxrpl/tx/transactors/permissioned_domain/PermissionedDomainSet.cpp b/src/libxrpl/tx/transactors/permissioned_domain/PermissionedDomainSet.cpp index f794844a57..39b65d0947 100644 --- a/src/libxrpl/tx/transactors/permissioned_domain/PermissionedDomainSet.cpp +++ b/src/libxrpl/tx/transactors/permissioned_domain/PermissionedDomainSet.cpp @@ -110,12 +110,13 @@ PermissionedDomainSet::doApply() if (balance < reserve) return tecINSUFFICIENT_RESERVE; - Keylet const pdKeylet = - keylet::permissionedDomain(account_, ctx_.tx.getFieldU32(sfSequence)); + bool const fix313 = view().rules().enabled(fixSecurity3_1_3); + auto const seq = fix313 ? ctx_.tx.getSeqValue() : ctx_.tx.getFieldU32(sfSequence); + Keylet const pdKeylet = keylet::permissionedDomain(account_, seq); auto slePd = std::make_shared(pdKeylet); slePd->setAccountID(sfOwner, account_); - slePd->setFieldU32(sfSequence, ctx_.tx.getFieldU32(sfSequence)); + slePd->setFieldU32(sfSequence, seq); slePd->peekFieldArray(sfAcceptedCredentials) = std::move(sortedLE); auto const page = view().dirInsert(keylet::ownerDir(account_), pdKeylet, describeOwnerDir(account_)); From 170eb5e588906d8701dee04163229113320344f0 Mon Sep 17 00:00:00 2001 From: Michael Legleux Date: Tue, 12 May 2026 13:46:33 -0700 Subject: [PATCH 08/21] ci: Limit nproc on Linux builds temporarily (#7132) --- .github/workflows/reusable-build-test-config.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/reusable-build-test-config.yml b/.github/workflows/reusable-build-test-config.yml index cc927512ea..4c7c41fd0c 100644 --- a/.github/workflows/reusable-build-test-config.yml +++ b/.github/workflows/reusable-build-test-config.yml @@ -181,7 +181,7 @@ jobs: - name: Build the binary working-directory: ${{ env.BUILD_DIR }} env: - BUILD_NPROC: ${{ steps.nproc.outputs.nproc }} + BUILD_NPROC: ${{ runner.os == 'Linux' && '16' || steps.nproc.outputs.nproc }} BUILD_TYPE: ${{ inputs.build_type }} CMAKE_TARGET: ${{ inputs.cmake_target }} run: | From 6340c986c90b85ec9c2bf1c00de297299978f185 Mon Sep 17 00:00:00 2001 From: Vito Tumas <5780819+Tapanito@users.noreply.github.com> Date: Wed, 13 May 2026 11:42:34 +0200 Subject: [PATCH 09/21] feat: Enable and rename fixSecurity3_1_3 to fixCleanup3_1_3 (#7128) Co-authored-by: Claude Opus 4.6 (1M context) --- include/xrpl/protocol/detail/features.macro | 2 +- .../tx/invariants/PermissionedDEXInvariant.h | 4 +-- .../ledger/helpers/CredentialHelpers.cpp | 2 +- src/libxrpl/ledger/helpers/LendingHelpers.cpp | 4 +-- src/libxrpl/ledger/helpers/MPTokenHelpers.cpp | 4 +-- .../ledger/helpers/PermissionedDEXHelpers.cpp | 6 ++-- src/libxrpl/ledger/helpers/TokenHelpers.cpp | 6 ++-- src/libxrpl/tx/invariants/InvariantCheck.cpp | 6 ++-- .../tx/invariants/LoanBrokerInvariant.cpp | 2 +- .../invariants/PermissionedDEXInvariant.cpp | 6 ++-- .../tx/transactors/lending/LoanManage.cpp | 2 +- .../tx/transactors/lending/LoanPay.cpp | 6 ++-- .../tx/transactors/nft/NFTokenAcceptOffer.cpp | 8 ++--- .../PermissionedDomainSet.cpp | 2 +- .../tx/transactors/vault/VaultClawback.cpp | 4 +-- .../tx/transactors/vault/VaultWithdraw.cpp | 4 +-- src/test/app/Credentials_test.cpp | 6 ++-- src/test/app/Invariants_test.cpp | 15 ++++----- src/test/app/Loan_test.cpp | 12 +++---- src/test/app/MPToken_test.cpp | 8 ++--- src/test/app/NFToken_test.cpp | 32 +++++++++---------- src/test/app/PermissionedDEX_test.cpp | 15 ++++----- src/test/app/TxQ_test.cpp | 23 +++++++------ src/test/app/Vault_test.cpp | 20 ++++++------ 24 files changed, 101 insertions(+), 98 deletions(-) diff --git a/include/xrpl/protocol/detail/features.macro b/include/xrpl/protocol/detail/features.macro index 2c2cd14e97..dc26a5a43b 100644 --- a/include/xrpl/protocol/detail/features.macro +++ b/include/xrpl/protocol/detail/features.macro @@ -17,7 +17,7 @@ XRPL_FIX (Cleanup3_2_0, Supported::No, VoteBehavior::DefaultNo) XRPL_FEATURE(MPTokensV2, Supported::No, VoteBehavior::DefaultNo) -XRPL_FIX (Security3_1_3, Supported::No, VoteBehavior::DefaultNo) +XRPL_FIX (Cleanup3_1_3, Supported::Yes, VoteBehavior::DefaultYes) XRPL_FIX (PermissionedDomainInvariant, Supported::Yes, VoteBehavior::DefaultNo) XRPL_FIX (BatchInnerSigs, Supported::No, VoteBehavior::DefaultNo) XRPL_FEATURE(LendingProtocol, Supported::Yes, VoteBehavior::DefaultNo) diff --git a/include/xrpl/tx/invariants/PermissionedDEXInvariant.h b/include/xrpl/tx/invariants/PermissionedDEXInvariant.h index da187779b2..3784a32840 100644 --- a/include/xrpl/tx/invariants/PermissionedDEXInvariant.h +++ b/include/xrpl/tx/invariants/PermissionedDEXInvariant.h @@ -11,8 +11,8 @@ namespace xrpl { class ValidPermissionedDEX { bool regularOffers_ = false; - bool badHybridsOld_ = false; // pre-fixSecurity3_1_3: missing field/domain or size > 1 - bool badHybrids_ = false; // post-fixSecurity3_1_3: also catches size == 0 (size != 1) + bool badHybridsOld_ = false; // pre-fixCleanup3_1_3: missing field/domain or size > 1 + bool badHybrids_ = false; // post-fixCleanup3_1_3: also catches size == 0 (size != 1) hash_set domains_; public: diff --git a/src/libxrpl/ledger/helpers/CredentialHelpers.cpp b/src/libxrpl/ledger/helpers/CredentialHelpers.cpp index 05e45a404c..cb0cbf6322 100644 --- a/src/libxrpl/ledger/helpers/CredentialHelpers.cpp +++ b/src/libxrpl/ledger/helpers/CredentialHelpers.cpp @@ -61,7 +61,7 @@ removeExpired(ApplyView& view, STVector256 const& arr, beast::Journal const j) JLOG(j.trace()) << "Credentials are expired. Cred: " << sleCred->getText(); // delete expired credentials even if the transaction failed auto const err = deleteSLE(view, sleCred, j); - if (view.rules().enabled(fixSecurity3_1_3) && !isTesSuccess(err)) + if (view.rules().enabled(fixCleanup3_1_3) && !isTesSuccess(err)) return Unexpected(err); foundExpired = true; } diff --git a/src/libxrpl/ledger/helpers/LendingHelpers.cpp b/src/libxrpl/ledger/helpers/LendingHelpers.cpp index a8bee7aecb..3bfa18e3e8 100644 --- a/src/libxrpl/ledger/helpers/LendingHelpers.cpp +++ b/src/libxrpl/ledger/helpers/LendingHelpers.cpp @@ -1964,13 +1964,13 @@ loanMakePayment( // ------------------------------------------------------------- // overpayment handling // - // If the "fixSecurity3_1_3" amendment is enabled, truncate "amount", + // If the "fixCleanup3_1_3" amendment is enabled, truncate "amount", // at the loan scale. If the raw value is used, the overpayment // amount could be meaningless dust. Trying to process such a small // amount will, at best, waste time when all the result values round // to zero. At worst, it can cause logical errors with tiny amounts // of interest that don't add up correctly. - auto const roundedAmount = view.rules().enabled(fixSecurity3_1_3) + auto const roundedAmount = view.rules().enabled(fixCleanup3_1_3) ? roundToAsset(asset, amount, loanScale, Number::RoundingMode::TowardsZero) : amount; if (paymentType == LoanPaymentType::Overpayment && loan->isFlag(lsfLoanOverpayment) && diff --git a/src/libxrpl/ledger/helpers/MPTokenHelpers.cpp b/src/libxrpl/ledger/helpers/MPTokenHelpers.cpp index 75fe61b34b..252921c499 100644 --- a/src/libxrpl/ledger/helpers/MPTokenHelpers.cpp +++ b/src/libxrpl/ledger/helpers/MPTokenHelpers.cpp @@ -169,7 +169,7 @@ authorizeMPToken( auto const mptokenKey = keylet::mptoken(mptIssuanceID, account); auto const sleMpt = view.peek(mptokenKey); if (!sleMpt || (*sleMpt)[sfMPTAmount] != 0 || - (view.rules().enabled(fixSecurity3_1_3) && + (view.rules().enabled(fixCleanup3_1_3) && (*sleMpt)[~sfLockedAmount].valueOr(0) != 0)) return tecINTERNAL; // LCOV_EXCL_LINE @@ -284,7 +284,7 @@ removeEmptyHolding( // accounting out of balance, so fail. Since this should be impossible // anyway, I'm not going to put any effort into it. if (mptoken->at(sfMPTAmount) != 0 || - (view.rules().enabled(fixSecurity3_1_3) && (*mptoken)[~sfLockedAmount].valueOr(0) != 0)) + (view.rules().enabled(fixCleanup3_1_3) && (*mptoken)[~sfLockedAmount].valueOr(0) != 0)) return tecHAS_OBLIGATIONS; return authorizeMPToken( diff --git a/src/libxrpl/ledger/helpers/PermissionedDEXHelpers.cpp b/src/libxrpl/ledger/helpers/PermissionedDEXHelpers.cpp index 7b4194cccf..0151c5b2df 100644 --- a/src/libxrpl/ledger/helpers/PermissionedDEXHelpers.cpp +++ b/src/libxrpl/ledger/helpers/PermissionedDEXHelpers.cpp @@ -60,9 +60,9 @@ offerInDomain( if (sleOffer->getFieldH256(sfDomainID) != domainID) return false; // LCOV_EXCL_LINE - if (view.rules().enabled(fixSecurity3_1_3)) + if (view.rules().enabled(fixCleanup3_1_3)) { - // post-fixSecurity3_1_3: a valid hybrid offer must have + // post-fixCleanup3_1_3: a valid hybrid offer must have // sfAdditionalBooks present with exactly 1 entry if (sleOffer->isFlag(lsfHybrid) && (!sleOffer->isFieldPresent(sfAdditionalBooks) || @@ -75,7 +75,7 @@ offerInDomain( } else { - // pre-fixSecurity3_1_3: a valid hybrid offer must have + // pre-fixCleanup3_1_3: a valid hybrid offer must have // sfAdditionalBooks present (size is not checked) if (sleOffer->isFlag(lsfHybrid) && !sleOffer->isFieldPresent(sfAdditionalBooks)) { diff --git a/src/libxrpl/ledger/helpers/TokenHelpers.cpp b/src/libxrpl/ledger/helpers/TokenHelpers.cpp index 000f459ef6..0894fdba38 100644 --- a/src/libxrpl/ledger/helpers/TokenHelpers.cpp +++ b/src/libxrpl/ledger/helpers/TokenHelpers.cpp @@ -1222,9 +1222,9 @@ directSendNoLimitMultiMPT( std::uint64_t const sendAmount = amount.mpt().value(); - if (view.rules().enabled(fixSecurity3_1_3)) + if (view.rules().enabled(fixCleanup3_1_3)) { - // Post-fixSecurity3_1_3: aggregate MaximumAmount + // Post-fixCleanup3_1_3: aggregate MaximumAmount // check. WARNING: the order of conditions is // critical — each guards the subtraction in the // next against unsigned underflow. Do not reorder. @@ -1242,7 +1242,7 @@ directSendNoLimitMultiMPT( } else { - // Pre-fixSecurity3_1_3: per-iteration MaximumAmount + // Pre-fixCleanup3_1_3: per-iteration MaximumAmount // check. Reads sfOutstandingAmount from a stale // view.read() snapshot — incorrect for multi-destination // sends but retained for ledger replay compatibility. diff --git a/src/libxrpl/tx/invariants/InvariantCheck.cpp b/src/libxrpl/tx/invariants/InvariantCheck.cpp index e9c32bf14c..59887ad18c 100644 --- a/src/libxrpl/tx/invariants/InvariantCheck.cpp +++ b/src/libxrpl/tx/invariants/InvariantCheck.cpp @@ -341,7 +341,7 @@ NoZeroEscrow::visitEntry( bad_ |= true; }; - bool const overwriteFixEnabled = isFeatureEnabled(fixSecurity3_1_3, true); + bool const overwriteFixEnabled = isFeatureEnabled(fixCleanup3_1_3, true); if (after && after->getType() == ltMPTOKEN_ISSUANCE) { @@ -628,7 +628,7 @@ NoXRPTrustLines::visitEntry( std::shared_ptr const&, std::shared_ptr const& after) { - bool const overwriteFixEnabled = isFeatureEnabled(fixSecurity3_1_3, true); + bool const overwriteFixEnabled = isFeatureEnabled(fixCleanup3_1_3, true); if (after && after->getType() == ltRIPPLE_STATE) { @@ -673,7 +673,7 @@ NoDeepFreezeTrustLinesWithoutFreeze::visitEntry( { if (after && after->getType() == ltRIPPLE_STATE) { - bool const overwriteFixEnabled = isFeatureEnabled(fixSecurity3_1_3, true); + bool const overwriteFixEnabled = isFeatureEnabled(fixCleanup3_1_3, true); std::uint32_t const uFlags = after->getFieldU32(sfFlags); bool const lowFreeze = (uFlags & lsfLowFreeze) != 0u; diff --git a/src/libxrpl/tx/invariants/LoanBrokerInvariant.cpp b/src/libxrpl/tx/invariants/LoanBrokerInvariant.cpp index b98d769393..56995ef94c 100644 --- a/src/libxrpl/tx/invariants/LoanBrokerInvariant.cpp +++ b/src/libxrpl/tx/invariants/LoanBrokerInvariant.cpp @@ -197,7 +197,7 @@ ValidLoanBroker::finalize( return false; } - if (view.rules().enabled(fixSecurity3_1_3)) + if (view.rules().enabled(fixCleanup3_1_3)) { // Don't check the balance when LoanBroker is deleted, // sfCoverAvailable is not zeroed diff --git a/src/libxrpl/tx/invariants/PermissionedDEXInvariant.cpp b/src/libxrpl/tx/invariants/PermissionedDEXInvariant.cpp index 6466812743..b480b90a31 100644 --- a/src/libxrpl/tx/invariants/PermissionedDEXInvariant.cpp +++ b/src/libxrpl/tx/invariants/PermissionedDEXInvariant.cpp @@ -41,14 +41,14 @@ ValidPermissionedDEX::visitEntry( regularOffers_ = true; } - // pre-fixSecurity3_1_3: hybrid offer missing domain, missing + // pre-fixCleanup3_1_3: hybrid offer missing domain, missing // sfAdditionalBooks, or sfAdditionalBooks has more than one entry if (after->isFlag(lsfHybrid) && (!after->isFieldPresent(sfDomainID) || !after->isFieldPresent(sfAdditionalBooks) || after->getFieldArray(sfAdditionalBooks).size() > 1)) badHybridsOld_ = true; - // post-fixSecurity3_1_3: same as above but also catches size == 0 + // post-fixCleanup3_1_3: same as above but also catches size == 0 if (after->isFlag(lsfHybrid) && (!after->isFieldPresent(sfDomainID) || !after->isFieldPresent(sfAdditionalBooks) || after->getFieldArray(sfAdditionalBooks).size() != 1)) @@ -70,7 +70,7 @@ ValidPermissionedDEX::finalize( // For each offercreate transaction, check if // permissioned offers are valid - bool const isMalformed = view.rules().enabled(fixSecurity3_1_3) ? badHybrids_ : badHybridsOld_; + bool const isMalformed = view.rules().enabled(fixCleanup3_1_3) ? badHybrids_ : badHybridsOld_; if (txType == ttOFFER_CREATE && isMalformed) { JLOG(j.fatal()) << "Invariant failed: hybrid offer is malformed"; diff --git a/src/libxrpl/tx/transactors/lending/LoanManage.cpp b/src/libxrpl/tx/transactors/lending/LoanManage.cpp index 648f33b980..46c85c9f96 100644 --- a/src/libxrpl/tx/transactors/lending/LoanManage.cpp +++ b/src/libxrpl/tx/transactors/lending/LoanManage.cpp @@ -424,7 +424,7 @@ LoanManage::doApply() // Pre-amendment, associateAsset was only called on the noop (no flags) // path. Post-amendment, we call associateAsset on all successful paths. - if (view.rules().enabled(fixSecurity3_1_3) && isTesSuccess(result)) + if (view.rules().enabled(fixCleanup3_1_3) && isTesSuccess(result)) { associateAsset(*loanSle, vaultAsset); associateAsset(*brokerSle, vaultAsset); diff --git a/src/libxrpl/tx/transactors/lending/LoanPay.cpp b/src/libxrpl/tx/transactors/lending/LoanPay.cpp index d7147128cc..a9668d9f4c 100644 --- a/src/libxrpl/tx/transactors/lending/LoanPay.cpp +++ b/src/libxrpl/tx/transactors/lending/LoanPay.cpp @@ -147,7 +147,7 @@ LoanPay::calculateBaseFee(ReadView const& view, STTx const& tx) std::int64_t constexpr kMAX_FEE_INCREMENTS = kLOAN_MAXIMUM_PAYMENTS_PER_TRANSACTION / kLOAN_PAYMENTS_PER_FEE_INCREMENT; - if (view.rules().enabled(fixSecurity3_1_3) && + if (view.rules().enabled(fixCleanup3_1_3) && amount >= regularPayment * kLOAN_MAXIMUM_PAYMENTS_PER_TRANSACTION) { // The payment handler will never process more than @@ -169,7 +169,7 @@ LoanPay::calculateBaseFee(ReadView const& view, STTx const& tx) std::int64_t(1), static_cast(numPaymentEstimate / kLOAN_PAYMENTS_PER_FEE_INCREMENT)); XRPL_ASSERT( - !view.rules().enabled(fixSecurity3_1_3) || feeIncrements <= kMAX_FEE_INCREMENTS, + !view.rules().enabled(fixCleanup3_1_3) || feeIncrements <= kMAX_FEE_INCREMENTS, "xrpl::LoanPay::calculateBaseFee : number of fee increments is in " "range"); @@ -201,7 +201,7 @@ LoanPay::preclaim(PreclaimContext const& ctx) if (tx.isFlag(tfLoanOverpayment) && !loanSle->isFlag(lsfLoanOverpayment)) { JLOG(ctx.j.warn()) << "Requested overpayment on a loan that doesn't allow it"; - return ctx.view.rules().enabled(fixSecurity3_1_3) ? TER{tecNO_PERMISSION} : temINVALID_FLAG; + return ctx.view.rules().enabled(fixCleanup3_1_3) ? TER{tecNO_PERMISSION} : temINVALID_FLAG; } auto const principalOutstanding = loanSle->at(sfPrincipalOutstanding); diff --git a/src/libxrpl/tx/transactors/nft/NFTokenAcceptOffer.cpp b/src/libxrpl/tx/transactors/nft/NFTokenAcceptOffer.cpp index 2aa2720e46..c46930ba08 100644 --- a/src/libxrpl/tx/transactors/nft/NFTokenAcceptOffer.cpp +++ b/src/libxrpl/tx/transactors/nft/NFTokenAcceptOffer.cpp @@ -68,10 +68,10 @@ NFTokenAcceptOffer::preclaim(PreclaimContext const& ctx) if (hasExpired(ctx.view, (*offerSLE)[~sfExpiration])) { - // Before fixSecurity3_1_3 amendment, expired offers caused tecEXPIRED in preclaim, + // Before fixCleanup3_1_3 amendment, expired offers caused tecEXPIRED in preclaim, // leaving them on ledger forever. After the amendment, we allow expired offers to // reach doApply() where they get deleted and tecEXPIRED is returned. - if (!ctx.view.rules().enabled(fixSecurity3_1_3)) + if (!ctx.view.rules().enabled(fixCleanup3_1_3)) return {nullptr, tecEXPIRED}; // Amendment enabled: return the expired offer to be handled in doApply. } @@ -450,9 +450,9 @@ NFTokenAcceptOffer::doApply() auto bo = loadToken(ctx_.tx[~sfNFTokenBuyOffer]); auto so = loadToken(ctx_.tx[~sfNFTokenSellOffer]); - // With fixSecurity3_1_3 amendment, check for expired offers and delete them, returning + // With fixCleanup3_1_3 amendment, check for expired offers and delete them, returning // tecEXPIRED. This ensures expired offers are properly cleaned up from the ledger. - if (view().rules().enabled(fixSecurity3_1_3)) + if (view().rules().enabled(fixCleanup3_1_3)) { bool foundExpired = false; diff --git a/src/libxrpl/tx/transactors/permissioned_domain/PermissionedDomainSet.cpp b/src/libxrpl/tx/transactors/permissioned_domain/PermissionedDomainSet.cpp index 39b65d0947..3258dda409 100644 --- a/src/libxrpl/tx/transactors/permissioned_domain/PermissionedDomainSet.cpp +++ b/src/libxrpl/tx/transactors/permissioned_domain/PermissionedDomainSet.cpp @@ -110,7 +110,7 @@ PermissionedDomainSet::doApply() if (balance < reserve) return tecINSUFFICIENT_RESERVE; - bool const fix313 = view().rules().enabled(fixSecurity3_1_3); + bool const fix313 = view().rules().enabled(fixCleanup3_1_3); auto const seq = fix313 ? ctx_.tx.getSeqValue() : ctx_.tx.getFieldU32(sfSequence); Keylet const pdKeylet = keylet::permissionedDomain(account_, seq); auto slePd = std::make_shared(pdKeylet); diff --git a/src/libxrpl/tx/transactors/vault/VaultClawback.cpp b/src/libxrpl/tx/transactors/vault/VaultClawback.cpp index b6d92a6083..33f4dbf4fa 100644 --- a/src/libxrpl/tx/transactors/vault/VaultClawback.cpp +++ b/src/libxrpl/tx/transactors/vault/VaultClawback.cpp @@ -243,11 +243,11 @@ VaultClawback::assetsToClawback( auto const mptIssuanceID = *vault->at(sfShareMPTID); MPTIssue const share{mptIssuanceID}; - // Pre-fixSecurity3_1_3: zero-amount clawback returned early without + // Pre-fixCleanup3_1_3: zero-amount clawback returned early without // clamping to assetsAvailable, allowing more assets to be recovered // than available when there was an outstanding loan. Retained for // ledger replay compatibility. - if (!ctx_.view().rules().enabled(fixSecurity3_1_3) && clawbackAmount == beast::kZERO) + if (!ctx_.view().rules().enabled(fixCleanup3_1_3) && clawbackAmount == beast::kZERO) { auto const sharesDestroyed = accountHolds( view(), holder, share, FreezeHandling::IgnoreFreeze, AuthHandling::IgnoreAuth, j_); diff --git a/src/libxrpl/tx/transactors/vault/VaultWithdraw.cpp b/src/libxrpl/tx/transactors/vault/VaultWithdraw.cpp index 94c6f0f6d2..5b4949ccf6 100644 --- a/src/libxrpl/tx/transactors/vault/VaultWithdraw.cpp +++ b/src/libxrpl/tx/transactors/vault/VaultWithdraw.cpp @@ -80,9 +80,9 @@ VaultWithdraw::preclaim(PreclaimContext const& ctx) // LCOV_EXCL_STOP } - if (ctx.view.rules().enabled(fixSecurity3_1_3) && amount.asset() == vaultShare) + if (ctx.view.rules().enabled(fixCleanup3_1_3) && amount.asset() == vaultShare) { - // Post-fixSecurity3_1_3: if the user specified shares, convert + // Post-fixCleanup3_1_3: if the user specified shares, convert // to the equivalent asset amount before checking withdrawal // limits. Pre-amendment the limit check was skipped for // share-denominated withdrawals. diff --git a/src/test/app/Credentials_test.cpp b/src/test/app/Credentials_test.cpp index fced18c387..1d3f3b3ea8 100644 --- a/src/test/app/Credentials_test.cpp +++ b/src/test/app/Credentials_test.cpp @@ -1036,7 +1036,7 @@ struct Credentials_test : public beast::unit_test::Suite void testRemoveExpiredCorruption(FeatureBitset features) { - bool const fixEnabled = features[fixSecurity3_1_3]; + bool const fixEnabled = features[fixCleanup3_1_3]; testcase( "removeExpired ignores deleteSLE failure " + (fixEnabled ? std::string(" after fix") : std::string(" before fix"))); @@ -1164,8 +1164,8 @@ struct Credentials_test : public beast::unit_test::Suite testFlags(all); testRPC(); - testRemoveExpiredCorruption(all - fixSecurity3_1_3); - testRemoveExpiredCorruption(all | fixSecurity3_1_3); + testRemoveExpiredCorruption(all - fixCleanup3_1_3); + testRemoveExpiredCorruption(all | fixCleanup3_1_3); } }; diff --git a/src/test/app/Invariants_test.cpp b/src/test/app/Invariants_test.cpp index c9f0a04a18..b0f1aee03c 100644 --- a/src/test/app/Invariants_test.cpp +++ b/src/test/app/Invariants_test.cpp @@ -94,7 +94,7 @@ class Invariants_test : public beast::unit_test::Suite static FeatureBitset defaultAmendments() { - return xrpl::test::jtx::testableAmendments() | fixSecurity3_1_3 | fixCleanup3_2_0; + return xrpl::test::jtx::testableAmendments() | fixCleanup3_1_3 | fixCleanup3_2_0; } /** Run a specific test case to put the ledger into a state that will be @@ -1801,7 +1801,7 @@ class Invariants_test : public beast::unit_test::Suite using namespace test::jtx; bool const fixPDEnabled = features[fixPermissionedDomainInvariant]; - bool const fixS313Enabled = features[fixSecurity3_1_3]; + bool const fixS313Enabled = features[fixCleanup3_1_3]; testcase << "PermissionedDEX" + std::string(fixPDEnabled ? " fixPD" : "") + std::string(fixS313Enabled ? " fixS313" : ""); @@ -2387,7 +2387,7 @@ class Invariants_test : public beast::unit_test::Suite } // Test: cover available greater than pseudo-account asset balance - // (requires fixSecurity3_1_3) + // (requires fixCleanup3_1_3) doInvariantCheck( {{"Loan Broker cover available is greater than pseudo-account asset balance"}}, [&](Account const&, Account const&, ApplyContext& ac) { @@ -4116,7 +4116,7 @@ class Invariants_test : public beast::unit_test::Suite testInvariantOverwrite(FeatureBitset features) { using namespace test::jtx; - bool const fixEnabled = features[fixSecurity3_1_3]; + bool const fixEnabled = features[fixCleanup3_1_3]; std::initializer_list const failTers = {tecINVARIANT_FAILED, tefINVARIANT_FAILED}; std::initializer_list const passTers = {tesSUCCESS, tesSUCCESS}; @@ -4385,16 +4385,15 @@ public: testPermissionedDEX(defaultAmendments() | fixPermissionedDomainInvariant); testPermissionedDEX(defaultAmendments() - fixPermissionedDomainInvariant); testPermissionedDEX( - (defaultAmendments() | fixPermissionedDomainInvariant) - fixSecurity3_1_3); - testPermissionedDEX( - defaultAmendments() - fixPermissionedDomainInvariant - fixSecurity3_1_3); + (defaultAmendments() | fixPermissionedDomainInvariant) - fixCleanup3_1_3); + testPermissionedDEX(defaultAmendments() - fixPermissionedDomainInvariant - fixCleanup3_1_3); testNoModifiedUnmodifiableFields(); testValidPseudoAccounts(); testValidLoanBroker(); testVault(); testMPT(); testInvariantOverwrite(defaultAmendments()); - testInvariantOverwrite(defaultAmendments() - fixSecurity3_1_3); + testInvariantOverwrite(defaultAmendments() - fixCleanup3_1_3); testVaultComputeCoarsestScale(); } }; diff --git a/src/test/app/Loan_test.cpp b/src/test/app/Loan_test.cpp index 70997b1dcd..ede616bc7c 100644 --- a/src/test/app/Loan_test.cpp +++ b/src/test/app/Loan_test.cpp @@ -2146,7 +2146,7 @@ protected: Ter(tecNO_PERMISSION)); { - env.disableFeature(fixSecurity3_1_3); + env.disableFeature(fixCleanup3_1_3); env(pay(borrower, loanKeylet.key, STAmount{broker.asset, state.periodicPayment * Number{15, -1}}, @@ -2154,7 +2154,7 @@ protected: Fee(XRPAmount{ baseFee * (Number{15, -1} / kLOAN_PAYMENTS_PER_FEE_INCREMENT + 1)}), Ter(temINVALID_FLAG)); - env.enableFeature(fixSecurity3_1_3); + env.enableFeature(fixCleanup3_1_3); } } // Try to send a payment marked as multiple mutually exclusive @@ -4763,7 +4763,7 @@ protected: void testDosLoanPay(FeatureBitset features) { - bool const feeCapped = features[fixSecurity3_1_3]; + bool const feeCapped = features[fixCleanup3_1_3]; // From FIND-005 testcase << "DoS LoanPay: fee calculation " << (feeCapped ? "capped" : "uncapped"); @@ -4780,7 +4780,7 @@ protected: env.fund(XRP(1'000'000), issuer, lender, borrower); env.close(); - BEAST_EXPECT(feeCapped == env.current()->rules().enabled(fixSecurity3_1_3)); + BEAST_EXPECT(feeCapped == env.current()->rules().enabled(fixCleanup3_1_3)); PrettyAsset const iouAsset = issuer[iouCurrency_]; env(trust(lender, iouAsset(100'000'000))); @@ -7664,8 +7664,8 @@ public: testLoanPayDebtDecreaseInvariant(); testWrongMaxDebtBehavior(); testLoanPayComputePeriodicPaymentValidTotalInterestInvariant(); - testDosLoanPay(all | fixSecurity3_1_3); - testDosLoanPay(all - fixSecurity3_1_3); + testDosLoanPay(all | fixCleanup3_1_3); + testDosLoanPay(all - fixCleanup3_1_3); testLoanPayComputePeriodicPaymentValidTotalPrincipalPaidInvariant(); testLoanPayComputePeriodicPaymentValidTotalInterestPaidInvariant(); testLoanNextPaymentDueDateOverflow(); diff --git a/src/test/app/MPToken_test.cpp b/src/test/app/MPToken_test.cpp index 126778c6a0..36895dd1cf 100644 --- a/src/test/app/MPToken_test.cpp +++ b/src/test/app/MPToken_test.cpp @@ -3310,7 +3310,7 @@ class MPToken_test : public beast::unit_test::Suite testMultiSendMaximumAmount(FeatureBitset features) { // Verify that directSendNoLimitMultiMPT correctly enforces MaximumAmount - // when the issuer sends to multiple receivers. Pre-fixSecurity3_1_3, + // when the issuer sends to multiple receivers. Pre-fixCleanup3_1_3, // a stale view.read() snapshot caused per-iteration checks to miss // aggregate overflows. Post-fix, a running total is used instead. testcase("Multi-send MaximumAmount enforcement"); @@ -3412,14 +3412,14 @@ class MPToken_test : public beast::unit_test::Suite // individual send (100 <= 150) even though the aggregate (200) // exceeds MaximumAmount. Preserved for ledger replay. { - // KNOWN BUG (pre-fixSecurity3_1_3): preserved for ledger replay only - env.disableFeature(fixSecurity3_1_3); + // KNOWN BUG (pre-fixCleanup3_1_3): preserved for ledger replay only + env.disableFeature(fixCleanup3_1_3); runTest( R{{alice.id(), 100}, {bob.id(), 100}}, tesSUCCESS, 250, "pre-amendment allows over-send"); - env.enableFeature(fixSecurity3_1_3); + env.enableFeature(fixCleanup3_1_3); } } diff --git a/src/test/app/NFToken_test.cpp b/src/test/app/NFToken_test.cpp index b81976b83a..5be33fc4e5 100644 --- a/src/test/app/NFToken_test.cpp +++ b/src/test/app/NFToken_test.cpp @@ -1095,10 +1095,10 @@ class NFTokenBaseUtil_test : public beast::unit_test::Suite BEAST_EXPECT(ownerCount(env, buyer) == buyerCount); // The buy offer must not have expired. - // NOTE: this is only a preclaim check with the fixSecurity3_1_3 amendment disabled. + // NOTE: this is only a preclaim check with the fixCleanup3_1_3 amendment disabled. env(token::acceptBuyOffer(alice, buyerExpOfferIndex), Ter(tecEXPIRED)); env.close(); - if (features[fixSecurity3_1_3]) + if (features[fixCleanup3_1_3]) { buyerCount--; } @@ -1115,13 +1115,13 @@ class NFTokenBaseUtil_test : public beast::unit_test::Suite BEAST_EXPECT(ownerCount(env, buyer) == buyerCount); // The sell offer must not have expired. - // NOTE: this is only a preclaim check with the fixSecurity3_1_3 amendment disabled. + // NOTE: this is only a preclaim check with the fixCleanup3_1_3 amendment disabled. env(token::acceptSellOffer(buyer, aliceExpOfferIndex), Ter(tecEXPIRED)); env.close(); // Alice's count is decremented by one when the expired offer is // removed. - if (features[fixSecurity3_1_3]) + if (features[fixCleanup3_1_3]) { aliceCount--; } @@ -3101,10 +3101,10 @@ class NFTokenBaseUtil_test : public beast::unit_test::Suite // No one can accept an expired sell offer. env(token::acceptSellOffer(buyer, offer1), Ter(tecEXPIRED)); - // With fixSecurity3_1_3 amendment, the first accept + // With fixCleanup3_1_3 amendment, the first accept // attempt deletes the expired offer. Without the amendment, // the offer remains and we can try to accept it again. - if (features[fixSecurity3_1_3]) + if (features[fixCleanup3_1_3]) { // After amendment: offer was deleted by first accept attempt minterCount--; @@ -3123,7 +3123,7 @@ class NFTokenBaseUtil_test : public beast::unit_test::Suite BEAST_EXPECT(ownerCount(env, minter) == minterCount); BEAST_EXPECT(ownerCount(env, buyer) == buyerCount); - if (!features[fixSecurity3_1_3]) + if (!features[fixCleanup3_1_3]) { // Before amendment: expired offer still exists and needs to be // cancelled @@ -3189,10 +3189,10 @@ class NFTokenBaseUtil_test : public beast::unit_test::Suite // An expired buy offer cannot be accepted. env(token::acceptBuyOffer(minter, offer1), Ter(tecEXPIRED)); - // With fixSecurity3_1_3 amendment, the first accept + // With fixCleanup3_1_3 amendment, the first accept // attempt deletes the expired offer. Without the amendment, // the offer remains and we can try to accept it again. - if (features[fixSecurity3_1_3]) + if (features[fixCleanup3_1_3]) { // After amendment: offer was deleted by first accept attempt buyerCount--; @@ -3211,7 +3211,7 @@ class NFTokenBaseUtil_test : public beast::unit_test::Suite BEAST_EXPECT(ownerCount(env, minter) == minterCount); BEAST_EXPECT(ownerCount(env, buyer) == buyerCount); - if (!features[fixSecurity3_1_3]) + if (!features[fixCleanup3_1_3]) { // Before amendment: expired offer still exists and can be // cancelled @@ -3288,7 +3288,7 @@ class NFTokenBaseUtil_test : public beast::unit_test::Suite env(token::brokerOffers(issuer, buyOffer1, sellOffer1), Ter(tecEXPIRED)); env.close(); - if (features[fixSecurity3_1_3]) + if (features[fixCleanup3_1_3]) { // With amendment: expired offers are deleted minterCount--; @@ -3298,7 +3298,7 @@ class NFTokenBaseUtil_test : public beast::unit_test::Suite BEAST_EXPECT(ownerCount(env, minter) == minterCount); BEAST_EXPECT(ownerCount(env, buyer) == buyerCount); - if (features[fixSecurity3_1_3]) + if (features[fixCleanup3_1_3]) { // The buy offer was deleted, so no need to cancel it // The sell offer still exists, so we can cancel it @@ -3377,7 +3377,7 @@ class NFTokenBaseUtil_test : public beast::unit_test::Suite env.close(); BEAST_EXPECT(ownerCount(env, issuer) == 0); - if (features[fixSecurity3_1_3]) + if (features[fixCleanup3_1_3]) { // After amendment: expired offers were deleted during broker // attempt @@ -3463,7 +3463,7 @@ class NFTokenBaseUtil_test : public beast::unit_test::Suite // The expired offers are still in the ledger. BEAST_EXPECT(ownerCount(env, issuer) == 0); - if (!features[fixSecurity3_1_3]) + if (!features[fixCleanup3_1_3]) { // Before amendment: expired offers still exist in ledger BEAST_EXPECT(ownerCount(env, minter) == 2); @@ -7193,7 +7193,7 @@ public: { testWithFeats( allFeatures_ - fixNFTokenReserve - featureNFTokenMintOffer - featureDynamicNFT - - fixSecurity3_1_3); + fixCleanup3_1_3); } }; @@ -7230,7 +7230,7 @@ class NfTokenWoExpiredOfferRemovalTest : public NFTokenBaseUtil_test void run() override { - testWithFeats(allFeatures_ - fixSecurity3_1_3); + testWithFeats(allFeatures_ - fixCleanup3_1_3); } }; diff --git a/src/test/app/PermissionedDEX_test.cpp b/src/test/app/PermissionedDEX_test.cpp index a0b16aa557..21e6bd8baa 100644 --- a/src/test/app/PermissionedDEX_test.cpp +++ b/src/test/app/PermissionedDEX_test.cpp @@ -1392,13 +1392,12 @@ class PermissionedDEX_test : public beast::unit_test::Suite void testHybridMalformedOffer(FeatureBitset features) { - bool const fixS313Enabled = features[fixSecurity3_1_3]; + bool const fixS313Enabled = features[fixCleanup3_1_3]; testcase << "Hybrid offer with empty AdditionalBooks" - << (fixS313Enabled ? " (fixSecurity3_1_3 enabled)" - : " (fixSecurity3_1_3 disabled)"); + << (fixS313Enabled ? " (fixCleanup3_1_3 enabled)" : " (fixCleanup3_1_3 disabled)"); - // offerInDomain has two code paths gated by fixSecurity3_1_3: + // offerInDomain has two code paths gated by fixCleanup3_1_3: // // pre-fix: only rejects a hybrid offer when sfAdditionalBooks is // entirely absent — an empty array (size 0) passes through. @@ -1425,7 +1424,7 @@ class PermissionedDEX_test : public beast::unit_test::Suite // Directly manipulate the offer SLE in the open ledger so that // sfAdditionalBooks is present but empty (size 0). This is the - // malformed state that fixSecurity3_1_3 is designed to catch. + // malformed state that fixCleanup3_1_3 is designed to catch. auto const offerKey = keylet::offer(bob.id(), bobOfferSeq); env.app().getOpenLedger().modify([&offerKey](OpenView& view, beast::Journal) { auto const sle = view.read(offerKey); @@ -1439,7 +1438,7 @@ class PermissionedDEX_test : public beast::unit_test::Suite if (fixS313Enabled) { - // post-fixSecurity3_1_3: offerInDomain rejects the malformed + // post-fixCleanup3_1_3: offerInDomain rejects the malformed // offer (size == 0), so no valid domain offer is found. env(pay(alice, carol, USD(10)), Path(~USD), @@ -1449,7 +1448,7 @@ class PermissionedDEX_test : public beast::unit_test::Suite } else { - // pre-fixSecurity3_1_3: offerInDomain only checks for a missing + // pre-fixCleanup3_1_3: offerInDomain only checks for a missing // sfAdditionalBooks field; size == 0 passes through, so the // malformed offer is crossed and the payment succeeds. env(pay(alice, carol, USD(10)), Path(~USD), Sendmax(XRP(10)), Domain(domainID)); @@ -1478,7 +1477,7 @@ public: testHybridInvalidOffer(all); testHybridOfferDirectories(all); testHybridMalformedOffer(all); - testHybridMalformedOffer(all - fixSecurity3_1_3); + testHybridMalformedOffer(all - fixCleanup3_1_3); } }; diff --git a/src/test/app/TxQ_test.cpp b/src/test/app/TxQ_test.cpp index cf79fb1d18..c950739078 100644 --- a/src/test/app/TxQ_test.cpp +++ b/src/test/app/TxQ_test.cpp @@ -1023,16 +1023,21 @@ public: checkMetrics(*this, env, 0, std::nullopt, 0, 3); // ledgers in queue is 2 because of makeConfig - auto const initQueueMax = initFee(env, 3, 2, 10, 200, 50); + initFee(env, 3, 2, 10, 200, 50); + // Close an empty ledger to shrink queue from the flag-ledger + // size to 2*3=6, independent of amendment count. + env.close(); + constexpr std::size_t kINIT_QUEUE_MAX = 6; + checkMetrics(*this, env, 0, kINIT_QUEUE_MAX, 0, 3); // Create several accounts while the fee is cheap so they all apply. env.fund(drops(2000), noripple(alice)); env.fund(XRP(500000), noripple(bob, charlie, daria)); - checkMetrics(*this, env, 0, initQueueMax, 4, 3); + checkMetrics(*this, env, 0, kINIT_QUEUE_MAX, 4, 3); // Alice - price starts exploding: held env(noop(alice), Fee(11), queued); - checkMetrics(*this, env, 1, initQueueMax, 4, 3); + checkMetrics(*this, env, 1, kINIT_QUEUE_MAX, 4, 3); auto aliceSeq = env.seq(alice); auto bobSeq = env.seq(bob); @@ -1040,28 +1045,28 @@ public: // Alice - try to queue a second transaction, but leave a gap env(noop(alice), Seq(aliceSeq + 2), Fee(100), Ter(telCAN_NOT_QUEUE)); - checkMetrics(*this, env, 1, initQueueMax, 4, 3); + checkMetrics(*this, env, 1, kINIT_QUEUE_MAX, 4, 3); // Alice - queue a second transaction. Yay! env(noop(alice), Seq(aliceSeq + 1), Fee(13), queued); - checkMetrics(*this, env, 2, initQueueMax, 4, 3); + checkMetrics(*this, env, 2, kINIT_QUEUE_MAX, 4, 3); // Alice - queue a third transaction. Yay. env(noop(alice), Seq(aliceSeq + 2), Fee(17), queued); - checkMetrics(*this, env, 3, initQueueMax, 4, 3); + checkMetrics(*this, env, 3, kINIT_QUEUE_MAX, 4, 3); // Bob - queue a transaction env(noop(bob), queued); - checkMetrics(*this, env, 4, initQueueMax, 4, 3); + checkMetrics(*this, env, 4, kINIT_QUEUE_MAX, 4, 3); // Bob - queue a second transaction env(noop(bob), Seq(bobSeq + 1), Fee(50), queued); - checkMetrics(*this, env, 5, initQueueMax, 4, 3); + checkMetrics(*this, env, 5, kINIT_QUEUE_MAX, 4, 3); // Charlie - queue a transaction, with a higher fee // than default env(noop(charlie), Fee(15), queued); - checkMetrics(*this, env, 6, initQueueMax, 4, 3, 257); + checkMetrics(*this, env, 6, kINIT_QUEUE_MAX, 4, 3, 257); BEAST_EXPECT(env.seq(alice) == aliceSeq); BEAST_EXPECT(env.seq(bob) == bobSeq); diff --git a/src/test/app/Vault_test.cpp b/src/test/app/Vault_test.cpp index f2c42b7bae..2f32cf605a 100644 --- a/src/test/app/Vault_test.cpp +++ b/src/test/app/Vault_test.cpp @@ -4916,7 +4916,7 @@ class Vault_test : public beast::unit_test::Suite using namespace loanBroker; using namespace loan; Env env(*this); - env.enableFeature(fixSecurity3_1_3); + env.enableFeature(fixCleanup3_1_3); auto const setupVault = [&](PrettyAsset const& asset, Account const& owner, @@ -5403,14 +5403,14 @@ class Vault_test : public beast::unit_test::Suite env.close(); testCase(mpt, "MPT", owner, depositor, issuer); - // Test pre-fixSecurity3_1_3 legacy path: zero-amount clawback + // Test pre-fixCleanup3_1_3 legacy path: zero-amount clawback // returns early without clamping to assetsAvailable. { testcase( - "VaultClawback (asset) - IOU pre-fixSecurity3_1_3" + "VaultClawback (asset) - IOU pre-fixCleanup3_1_3" " zero-amount clawback unclamped with outstanding loan"); - env.disableFeature(fixSecurity3_1_3); + env.disableFeature(fixCleanup3_1_3); auto [vault, vaultKeylet] = setupVault(iou, owner, depositor, issuer); @@ -5468,7 +5468,7 @@ class Vault_test : public beast::unit_test::Suite BEAST_EXPECT(sharesAfter == sharesBefore); } - env.enableFeature(fixSecurity3_1_3); + env.enableFeature(fixCleanup3_1_3); } } @@ -5867,7 +5867,7 @@ class Vault_test : public beast::unit_test::Suite { testcase("Vault clawback only recovers unlocked shares"); - Env env{*this, testableAmendments() | fixSecurity3_1_3}; + Env env{*this, testableAmendments() | fixCleanup3_1_3}; auto const baseFee = env.current()->fees().base; Account const owner{"owner"}; Account const depositor{"depositor"}; @@ -5957,9 +5957,9 @@ class Vault_test : public beast::unit_test::Suite auto const allAmendments = testableAmendments() | featureSingleAssetVault; - for (auto const& features : {allAmendments, allAmendments - fixSecurity3_1_3}) + for (auto const& features : {allAmendments, allAmendments - fixCleanup3_1_3}) { - bool const withFix = features[fixSecurity3_1_3]; + bool const withFix = features[fixCleanup3_1_3]; Env env{*this, features}; Account const owner{"owner"}; @@ -6117,7 +6117,7 @@ class Vault_test : public beast::unit_test::Suite env.close(); auto const sleMptAfter = env.le(keylet::mptoken(shareMptID, depositor)); - if (!f[fixSecurity3_1_3]) + if (!f[fixCleanup3_1_3]) { // Without the fix, removeEmptyHolding deletes the MPToken // even though sfLockedAmount > 0, leaving the escrow's locked @@ -6137,7 +6137,7 @@ class Vault_test : public beast::unit_test::Suite } }; - runTest(amendments - fixSecurity3_1_3); + runTest(amendments - fixCleanup3_1_3); runTest(amendments); } From e8bdbaa1e8e75a9d91b45284610d454f07a2108d Mon Sep 17 00:00:00 2001 From: Alex Kremer Date: Wed, 13 May 2026 13:42:05 +0100 Subject: [PATCH 10/21] refactor: Limit JSON array size (#7112) --- include/xrpl/protocol/STParsedJSON.h | 7 ++++ src/libxrpl/protocol/STParsedJSON.cpp | 56 ++++++++++++++++++++------- 2 files changed, 50 insertions(+), 13 deletions(-) diff --git a/include/xrpl/protocol/STParsedJSON.h b/include/xrpl/protocol/STParsedJSON.h index d5b4f33be7..04ffc624fb 100644 --- a/include/xrpl/protocol/STParsedJSON.h +++ b/include/xrpl/protocol/STParsedJSON.h @@ -6,6 +6,13 @@ namespace xrpl { +/** Maximum JSON object nesting depth permitted during parsing. */ +inline constexpr std::size_t kMAX_PARSED_JSON_DEPTH = 64; + +/** Maximum number of elements permitted in any JSON array field during parsing. + Requests exceeding this limit are rejected with an invalidParams error. */ +inline constexpr std::size_t kMAX_PARSED_JSON_ARRAY_SIZE = 512; + /** Holds the serialized result of parsing an input JSON object. This does validation and checking on the provided JSON. */ diff --git a/src/libxrpl/protocol/STParsedJSON.cpp b/src/libxrpl/protocol/STParsedJSON.cpp index 52d7b4d63e..c6d1d0e805 100644 --- a/src/libxrpl/protocol/STParsedJSON.cpp +++ b/src/libxrpl/protocol/STParsedJSON.cpp @@ -136,6 +136,15 @@ arrayExpected(std::string const& object, std::string const& field) RpcInvalidParams, "Field '" + makeName(object, field) + "' must be a JSON array."); } +static inline json::Value +arrayTooBig(std::string const& object, std::string const& field) +{ + return RPC::makeError( + RpcInvalidParams, + "Field '" + makeName(object, field) + "' exceeds allowed JSON array size of " + + std::to_string(kMAX_PARSED_JSON_ARRAY_SIZE) + " elements per field."); +} + static inline json::Value stringExpected(std::string const& object, std::string const& field) { @@ -681,12 +690,18 @@ parseLeaf( break; case STI_VECTOR256: - if (!value.isArrayOrNull()) + if (not value.isArrayOrNull()) { error = arrayExpected(jsonName, fieldName); return ret; } + if (not value.isNull() and value.size() > kMAX_PARSED_JSON_ARRAY_SIZE) + { + error = arrayTooBig(jsonName, fieldName); + return ret; + } + try { STVector256 tail(field); @@ -708,12 +723,18 @@ parseLeaf( break; case STI_PATHSET: - if (!value.isArrayOrNull()) + if (not value.isArrayOrNull()) { error = arrayExpected(jsonName, fieldName); return ret; } + if (not value.isNull() and value.size() > kMAX_PARSED_JSON_ARRAY_SIZE) + { + error = arrayTooBig(jsonName, fieldName); + return ret; + } + try { STPathSet tail(field); @@ -722,7 +743,7 @@ parseLeaf( { STPath p; - if (!value[i].isArrayOrNull()) + if (not value[i].isArrayOrNull()) { std::stringstream ss; ss << fieldName << "[" << i << "]"; @@ -730,6 +751,14 @@ parseLeaf( return ret; } + if (not value[i].isNull() and value[i].size() > kMAX_PARSED_JSON_ARRAY_SIZE) + { + std::stringstream ss; + ss << fieldName << "[" << i << "]"; + error = arrayTooBig(jsonName, ss.str()); + return ret; + } + for (json::UInt j = 0; value[i].isValidIndex(j); ++j) { std::stringstream ss; @@ -946,8 +975,6 @@ parseLeaf( return ret; } -static int const kMAX_DEPTH = 64; - // Forward declaration since parseObject() and parseArray() call each other. static std::optional parseArray( @@ -965,13 +992,13 @@ parseObject( int depth, json::Value& error) { - if (!json.isObjectOrNull()) + if (not json.isObjectOrNull()) { error = notAnObject(jsonName); return std::nullopt; } - if (depth > kMAX_DEPTH) + if (depth > kMAX_PARSED_JSON_DEPTH) { error = tooDeep(jsonName); return std::nullopt; @@ -984,7 +1011,6 @@ parseObject( for (auto const& fieldName : json.getMemberNames()) { json::Value const& value = json[fieldName]; - auto const& field = SField::getField(fieldName); if (field == kSF_INVALID) @@ -1079,18 +1105,24 @@ parseArray( int depth, json::Value& error) { - if (!json.isArrayOrNull()) + if (not json.isArrayOrNull()) { error = notAnArray(jsonName); return std::nullopt; } - if (depth > kMAX_DEPTH) + if (depth > kMAX_PARSED_JSON_DEPTH) { error = tooDeep(jsonName); return std::nullopt; } + if (not json.isNull() and json.size() > kMAX_PARSED_JSON_ARRAY_SIZE) + { + error = arrayTooBig(jsonName, ""); + return std::nullopt; + } + try { STArray tail(inName); @@ -1108,10 +1140,8 @@ parseArray( } // TODO: There doesn't seem to be a nice way to get just the - // first/only key in an object without copying all keys into - // a vector + // first/only key in an object without copying all keys into a vector std::string const memberName(json[i].getMemberNames()[0]); - ; auto const& nameField(SField::getField(memberName)); if (nameField == kSF_INVALID) From 411286c519e32d04492e0bdea3daed38ab646162 Mon Sep 17 00:00:00 2001 From: Valentin Balaschenko <13349202+vlntb@users.noreply.github.com> Date: Wed, 13 May 2026 14:22:00 +0100 Subject: [PATCH 11/21] refactor: Prevent dry-run transactions from being queued (#92) (#7131) --- src/xrpld/app/misc/detail/TxQ.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/xrpld/app/misc/detail/TxQ.cpp b/src/xrpld/app/misc/detail/TxQ.cpp index d98fabecf5..c270ddbe2d 100644 --- a/src/xrpld/app/misc/detail/TxQ.cpp +++ b/src/xrpld/app/misc/detail/TxQ.cpp @@ -744,6 +744,9 @@ TxQ::apply( if (auto directApplied = tryDirectApply(app, view, tx, flags, j)) return *directApplied; + if ((flags & TapDryRun) != 0u) + return {telCAN_NOT_QUEUE, false}; + // If we get past tryDirectApply() without returning then we expect // one of the following to occur: // From 4ad94ae2ff5c3845bb308e93eb49a140f2d0cb23 Mon Sep 17 00:00:00 2001 From: Valentin Balaschenko <13349202+vlntb@users.noreply.github.com> Date: Wed, 13 May 2026 14:53:01 +0100 Subject: [PATCH 12/21] refactor: Use named constant for leaf item size (#39) (#7130) Co-authored-by: Ed Hennis --- include/xrpl/shamap/SHAMapTreeNode.h | 4 +++ src/libxrpl/shamap/SHAMapLeafNode.cpp | 4 +-- src/libxrpl/shamap/SHAMapTreeNode.cpp | 45 ++++++++++++++++++++++++--- 3 files changed, 47 insertions(+), 6 deletions(-) diff --git a/include/xrpl/shamap/SHAMapTreeNode.h b/include/xrpl/shamap/SHAMapTreeNode.h index d50fc65ccb..ac48df8d46 100644 --- a/include/xrpl/shamap/SHAMapTreeNode.h +++ b/include/xrpl/shamap/SHAMapTreeNode.h @@ -7,6 +7,7 @@ #include #include +#include #include #include @@ -20,6 +21,9 @@ static constexpr unsigned char const kWIRE_TYPE_INNER = 2; static constexpr unsigned char const kWIRE_TYPE_COMPRESSED_INNER = 3; static constexpr unsigned char const kWIRE_TYPE_TRANSACTION_WITH_META = 4; +// Lower bound on SHAMap leaf item payload size, in bytes. +inline constexpr std::size_t kMIN_SHA_MAP_ITEM_BYTES = 12; + enum class SHAMapNodeType { TnInner = 1, TnTransactionNm = 2, // transaction, no metadata diff --git a/src/libxrpl/shamap/SHAMapLeafNode.cpp b/src/libxrpl/shamap/SHAMapLeafNode.cpp index ce41a64e56..84c4498698 100644 --- a/src/libxrpl/shamap/SHAMapLeafNode.cpp +++ b/src/libxrpl/shamap/SHAMapLeafNode.cpp @@ -19,7 +19,7 @@ SHAMapLeafNode::SHAMapLeafNode(boost::intrusive_ptr item, std: : SHAMapTreeNode(cowid), item_(std::move(item)) { XRPL_ASSERT( - item_->size() >= 12, + item_->size() >= kMIN_SHA_MAP_ITEM_BYTES, "xrpl::SHAMapLeafNode::SHAMapLeafNode(boost::intrusive_ptr<" "SHAMapItem const>, std::uint32_t) : minimum input size"); } @@ -31,7 +31,7 @@ SHAMapLeafNode::SHAMapLeafNode( : SHAMapTreeNode(cowid, hash), item_(std::move(item)) { XRPL_ASSERT( - item_->size() >= 12, + item_->size() >= kMIN_SHA_MAP_ITEM_BYTES, "xrpl::SHAMapLeafNode::SHAMapLeafNode(boost::intrusive_ptr<" "SHAMapItem const>, std::uint32_t, SHAMapHash const&) : minimum input " "size"); diff --git a/src/libxrpl/shamap/SHAMapTreeNode.cpp b/src/libxrpl/shamap/SHAMapTreeNode.cpp index ac405f3286..66eced2192 100644 --- a/src/libxrpl/shamap/SHAMapTreeNode.cpp +++ b/src/libxrpl/shamap/SHAMapTreeNode.cpp @@ -28,6 +28,13 @@ namespace xrpl { intr_ptr::SharedPtr SHAMapTreeNode::makeTransaction(Slice data, SHAMapHash const& hash, bool hashValid) { + if (data.size() < kMIN_SHA_MAP_ITEM_BYTES) + { + Throw( + "Short TXN node: " + std::to_string(data.size()) + " bytes (minimum " + + std::to_string(kMIN_SHA_MAP_ITEM_BYTES) + " required)"); + } + auto item = makeShamapitem(sha512Half(HashPrefix::TransactionId, data), data); if (hashValid) @@ -44,14 +51,30 @@ SHAMapTreeNode::makeTransactionWithMeta(Slice data, SHAMapHash const& hash, bool uint256 tag; if (s.size() < tag.kBYTES) - Throw("Short TXN+MD node"); + { + Throw( + "Short TXN+MD node: " + std::to_string(s.size()) + " bytes (minimum " + + std::to_string(tag.kBYTES) + " required for tag)"); + } // FIXME: improve this interface so that the above check isn't needed if (!s.getBitString(tag, s.size() - tag.kBYTES)) - Throw("Short TXN+MD node (" + std::to_string(s.size()) + ")"); + { + Throw( + "Short TXN+MD node: failed to read tag at offset " + + std::to_string(s.size() - tag.kBYTES)); + } s.chop(tag.kBYTES); + if (s.size() < kMIN_SHA_MAP_ITEM_BYTES) + { + Throw( + "Short TXN+MD node: " + std::to_string(s.size()) + + " bytes after tag removal (minimum " + std::to_string(kMIN_SHA_MAP_ITEM_BYTES) + + " required)"); + } + auto item = makeShamapitem(tag, s.slice()); if (hashValid) @@ -68,17 +91,31 @@ SHAMapTreeNode::makeAccountState(Slice data, SHAMapHash const& hash, bool hashVa uint256 tag; if (s.size() < tag.kBYTES) - Throw("short AS node"); + { + Throw( + "Short AS node: " + std::to_string(s.size()) + " bytes (minimum " + + std::to_string(tag.kBYTES) + " required for tag)"); + } // FIXME: improve this interface so that the above check isn't needed if (!s.getBitString(tag, s.size() - tag.kBYTES)) - Throw("Short AS node (" + std::to_string(s.size()) + ")"); + { + Throw( + "Short AS node: failed to read tag at offset " + std::to_string(s.size() - tag.kBYTES)); + } s.chop(tag.kBYTES); if (tag.isZero()) Throw("Invalid AS node"); + if (s.size() < kMIN_SHA_MAP_ITEM_BYTES) + { + Throw( + "Short AS node: " + std::to_string(s.size()) + " bytes after tag removal (minimum " + + std::to_string(kMIN_SHA_MAP_ITEM_BYTES) + " required)"); + } + auto item = makeShamapitem(tag, s.slice()); if (hashValid) From c8b42a7f4839b5b8c41df7ab3373fa1b5fc50da8 Mon Sep 17 00:00:00 2001 From: Sergey Kuznetsov Date: Wed, 13 May 2026 15:33:49 +0100 Subject: [PATCH 13/21] refactor: Improve RPC variable naming and handling (#7103) --- .../rpc/handlers/account/AccountObjects.cpp | 80 +++++++++---------- 1 file changed, 38 insertions(+), 42 deletions(-) diff --git a/src/xrpld/rpc/handlers/account/AccountObjects.cpp b/src/xrpld/rpc/handlers/account/AccountObjects.cpp index e17543b339..c544a6ff1a 100644 --- a/src/xrpld/rpc/handlers/account/AccountObjects.cpp +++ b/src/xrpld/rpc/handlers/account/AccountObjects.cpp @@ -59,12 +59,12 @@ getAccountObjects( // iterate NFT pages if the filter says so AND dirIndex == 0 bool iterateNFTPages = (!typeFilter.has_value() || typeMatchesFilter(typeFilter.value(), ltNFTOKEN_PAGE)) && - dirIndex == beast::kZERO; + dirIndex.isZero(); Keylet const firstNFTPage = keylet::nftpageMin(account); // we need to check the marker to see if it is an NFTTokenPage index. - if (iterateNFTPages && entryIndex != beast::kZERO) + if (iterateNFTPages && entryIndex.isNonZero()) { // if it is we will try to iterate the pages up to the limit // and then change over to the owner directory @@ -77,49 +77,44 @@ getAccountObjects( // this is a mutable version of limit, used to seamlessly switch // to iterating directory entries when nftokenpages are exhausted - uint32_t mlimit = limit; + uint32_t limitLeft = limit; // iterate NFTokenPages preferentially if (iterateNFTPages) { Keylet const first = - entryIndex == beast::kZERO ? firstNFTPage : Keylet{ltNFTOKEN_PAGE, entryIndex}; + entryIndex.isZero() ? firstNFTPage : Keylet{ltNFTOKEN_PAGE, entryIndex}; Keylet const last = keylet::nftpageMax(account); - // current key - uint256 ck = ledger.succ(first.key, last.key.next()).value_or(last.key); + auto currentKey = ledger.succ(first.key, last.key.next()).value_or(last.key); - // current page - auto cp = ledger.read(Keylet{ltNFTOKEN_PAGE, ck}); + auto currentPage = ledger.read(Keylet{ltNFTOKEN_PAGE, currentKey}); - while (cp) + while (currentPage) { - jvObjects.append(cp->getJson(JsonOptions::Values::None)); - auto const npm = (*cp)[~sfNextPageMin]; + jvObjects.append(currentPage->getJson(JsonOptions::Values::None)); + auto const npm = (*currentPage)[~sfNextPageMin]; if (npm) { - cp = ledger.read(Keylet(ltNFTOKEN_PAGE, *npm)); + currentPage = ledger.read(Keylet(ltNFTOKEN_PAGE, *npm)); } else { - cp = nullptr; + currentPage = nullptr; } - if (--mlimit == 0) + if (--limitLeft == 0 && currentPage) { - if (cp) - { - jvResult[jss::limit] = limit; - jvResult[jss::marker] = std::string("0,") + to_string(ck); - return true; - } + jvResult[jss::limit] = limit; + jvResult[jss::marker] = std::string("0,") + to_string(currentKey); + return true; } if (!npm) break; - ck = *npm; + currentKey = *npm; } // if execution reaches here then we're about to transition @@ -130,12 +125,12 @@ getAccountObjects( } auto const root = keylet::ownerDir(account); - auto found = false; + auto startEntryFound = false; if (dirIndex.isZero()) { dirIndex = root.key; - found = true; + startEntryFound = true; } auto dir = ledger.read({ltDIR_NODE, dirIndex}); @@ -144,7 +139,7 @@ getAccountObjects( // it's possible the user had nftoken pages but no // directory entries. If there's no nftoken page, we will // give empty array for account_objects. - if (mlimit >= limit) + if (limitLeft >= limit) jvResult[jss::account_objects] = json::ValueType::Array; // non-zero dirIndex validity was checked in the beginning of this @@ -156,33 +151,33 @@ getAccountObjects( return true; } - std::uint32_t i = 0; + std::uint32_t itemsAdded = 0; for (;;) { - auto const& entries = dir->getFieldV256(sfIndexes); - auto iter = entries.begin(); + auto const& dirEntries = dir->getFieldV256(sfIndexes); + auto entryIter = dirEntries.begin(); - if (!found) + if (!startEntryFound) { - iter = std::find(iter, entries.end(), entryIndex); - if (iter == entries.end()) + entryIter = std::find(entryIter, dirEntries.end(), entryIndex); + if (entryIter == dirEntries.end()) return false; - found = true; + startEntryFound = true; } // it's possible that the returned NFTPages exactly filled the // response. Check for that condition. - if (i == mlimit && mlimit < limit) + if (itemsAdded == limitLeft && limitLeft < limit && entryIter != dirEntries.end()) { jvResult[jss::limit] = limit; - jvResult[jss::marker] = to_string(dirIndex) + ',' + to_string(*iter); + jvResult[jss::marker] = to_string(dirIndex) + ',' + to_string(*entryIter); return true; } - for (; iter != entries.end(); ++iter) + for (; entryIter != dirEntries.end(); ++entryIter) { - auto const sleNode = ledger.read(keylet::child(*iter)); + auto const sleNode = ledger.read(keylet::child(*entryIter)); if (!typeFilter.has_value() || typeMatchesFilter(typeFilter.value(), sleNode->getType())) @@ -190,12 +185,12 @@ getAccountObjects( jvObjects.append(sleNode->getJson(JsonOptions::Values::None)); } - if (++i == mlimit) + if (++itemsAdded == limitLeft) { - if (++iter != entries.end()) + if (++entryIter != dirEntries.end()) { jvResult[jss::limit] = limit; - jvResult[jss::marker] = to_string(dirIndex) + ',' + to_string(*iter); + jvResult[jss::marker] = to_string(dirIndex) + ',' + to_string(*entryIter); return true; } @@ -212,13 +207,14 @@ getAccountObjects( if (!dir) return true; - if (i == mlimit) + if (itemsAdded == limitLeft) { - auto const& e = dir->getFieldV256(sfIndexes); - if (!e.empty()) + auto const& currentDirEntries = dir->getFieldV256(sfIndexes); + if (!currentDirEntries.empty()) { jvResult[jss::limit] = limit; - jvResult[jss::marker] = to_string(dirIndex) + ',' + to_string(*e.begin()); + jvResult[jss::marker] = + to_string(dirIndex) + ',' + to_string(*currentDirEntries.begin()); } return true; From 648ec747f2e52a6a53e241c53d1df9e2a0af41f6 Mon Sep 17 00:00:00 2001 From: Ayaz Salikhov Date: Wed, 13 May 2026 16:10:53 +0100 Subject: [PATCH 14/21] feat: Implement nix-based Dockerfile for CI (#7083) --- .github/workflows/build-nix-image.yml | 101 ++++++++++++++++++++++++++ .pre-commit-config.yaml | 6 +- cspell.config.yaml | 1 + docker/nix.Dockerfile | 66 +++++++++++++++++ flake.lock | 26 ++++++- flake.nix | 13 +++- nix/ci-env.nix | 54 ++++++++++++++ nix/devshell.nix | 15 +--- nix/packages.nix | 27 +++++++ nix/utils.nix | 40 +++++----- 10 files changed, 309 insertions(+), 40 deletions(-) create mode 100644 .github/workflows/build-nix-image.yml create mode 100644 docker/nix.Dockerfile create mode 100644 nix/ci-env.nix create mode 100644 nix/packages.nix diff --git a/.github/workflows/build-nix-image.yml b/.github/workflows/build-nix-image.yml new file mode 100644 index 0000000000..6554cf6c08 --- /dev/null +++ b/.github/workflows/build-nix-image.yml @@ -0,0 +1,101 @@ +name: Build Nix Docker image + +on: + push: + branches: + - develop + paths: + - ".github/workflows/build-nix-image.yml" + - "docker/nix.Dockerfile" + - "flake.nix" + - "flake.lock" + - "nix/**" + pull_request: + paths: + - ".github/workflows/build-nix-image.yml" + - "docker/nix.Dockerfile" + - "flake.nix" + - "flake.lock" + - "nix/**" + workflow_dispatch: + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +defaults: + run: + shell: bash + +env: + UBUNTU_VERSION: "20.04" + RHEL_VERSION: "9" + DEBIAN_VERSION: "bookworm" + +jobs: + build: + name: Build and push Nix image (${{ matrix.distro }}) + runs-on: ubuntu-latest + permissions: + contents: read + packages: write + strategy: + matrix: + include: + - distro: nixos + - distro: ubuntu + - distro: rhel + - distro: debian + + steps: + - name: Checkout repository + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + + - name: Determine base image + id: vars + run: | + case "${{ matrix.distro }}" in + nixos) + echo "base_image=nixos/nix:latest" >> $GITHUB_OUTPUT + ;; + ubuntu) + echo "base_image=ubuntu:${UBUNTU_VERSION}" >> $GITHUB_OUTPUT + ;; + rhel) + echo "base_image=registry.access.redhat.com/ubi${RHEL_VERSION}/ubi:latest" >> $GITHUB_OUTPUT + ;; + debian) + echo "base_image=debian:${DEBIAN_VERSION}" >> $GITHUB_OUTPUT + ;; + esac + + - name: Set up Docker Buildx + uses: docker/setup-buildx-action@4d04d5d9486b7bd6fa91e7baf45bbb4f8b9deedd # v4.0.0 + + - name: Login to GitHub Container Registry + if: github.event_name == 'push' + uses: docker/login-action@4907a6ddec9925e35a0a9e82d7399ccc52663121 # v4.1.0 + with: + registry: ghcr.io + username: ${{ github.repository_owner }} + password: ${{ secrets.GITHUB_TOKEN }} + + - name: Docker metadata + id: meta + uses: docker/metadata-action@030e881283bb7a6894de51c315a6bfe6a94e05cf # v6.0.0 + with: + images: ghcr.io/xrplf/ci/nix-${{ matrix.distro }} + tags: | + type=sha,prefix=sha-,format=short + type=raw,value=latest + + - name: Build and push + uses: docker/build-push-action@bcafcacb16a39f128d818304e6c9c0c18556b85f # v7.1.0 + with: + context: . + file: docker/nix.Dockerfile + platforms: linux/amd64 + push: ${{ github.event_name == 'push' }} + tags: ${{ steps.meta.outputs.tags }} + labels: ${{ steps.meta.outputs.labels }} + build-args: BASE_IMAGE=${{ steps.vars.outputs.base_image }} diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 1c0dc94550..1313ad567c 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -70,7 +70,11 @@ repos: rev: a42085ade523f591dca134379a595e7859986445 # frozen: v9.7.0 hooks: - id: cspell # Spell check changed files - exclude: (.config/cspell.config.yaml|^include/xrpl/protocol_autogen/(transactions|ledger_entries)/) + exclude: | + (?x)^( + .config/cspell.config.yaml| + include/xrpl/protocol_autogen/(transactions|ledger_entries)/.* + )$ - id: cspell # Spell check the commit message name: check commit message spelling args: diff --git a/cspell.config.yaml b/cspell.config.yaml index 8b8589a8c9..34482d9d8f 100644 --- a/cspell.config.yaml +++ b/cspell.config.yaml @@ -63,6 +63,7 @@ words: - Bougalis - Britto - Btrfs + - Buildx - canonicality - changespq - checkme diff --git a/docker/nix.Dockerfile b/docker/nix.Dockerfile new file mode 100644 index 0000000000..52faa8b8dc --- /dev/null +++ b/docker/nix.Dockerfile @@ -0,0 +1,66 @@ +ARG BASE_IMAGE=nixos/nix:latest + +# Nix builder +FROM nixos/nix:latest AS builder-source + +RUN mkdir -p ~/.config/nix && \ + echo "experimental-features = nix-command flakes" >> ~/.config/nix/nix.conf + +# Copy our source and setup our working dir. +COPY nix/ci-env.nix /tmp/build/nix/ci-env.nix +COPY nix/packages.nix /tmp/build/nix/packages.nix +COPY nix/utils.nix /tmp/build/nix/utils.nix +COPY flake.nix /tmp/build/ +COPY flake.lock /tmp/build/ +WORKDIR /tmp/build + +FROM builder-source AS builder + +# Build our Nix CI environment (all build tools in a single store path) +RUN nix \ + --option filter-syscalls false \ + build + +# Copy the Nix store closure into a directory. The Nix store closure is the +# entire set of Nix store values that we need for our build. +RUN mkdir /tmp/nix-store-closure && \ + cp -R $(nix-store -qR result/) /tmp/nix-store-closure + +# Final image +FROM ${BASE_IMAGE} + +# bash is not located at /bin/bash in nixos/nix, so we need to create a symlink to it. +RUN if [ -d /nix ]; then \ + ln -s /root/.nix-profile/bin/bash /bin/bash; \ + fi + +# Use Bash as the default shell for RUN commands, using the options +# `set -o errexit -o pipefail`, and as the entrypoint. +SHELL ["/bin/bash", "-e", "-o", "pipefail", "-c"] +ENTRYPOINT ["/bin/bash"] + +# Copy /nix/store and the env symlink tree +COPY --from=builder /tmp/nix-store-closure /nix/store +COPY --from=builder /tmp/build/result /nix/ci-env + +ENV PATH="/nix/ci-env/bin:$PATH" + +RUN < Date: Wed, 13 May 2026 17:03:57 +0100 Subject: [PATCH 15/21] fix: Check network ID in `transactionSignFor` (#7102) --- include/xrpl/basics/Expected.h | 24 ++++++++++++----- src/xrpld/rpc/detail/TransactionSign.cpp | 33 ++++++++++++++++++++++-- 2 files changed, 49 insertions(+), 8 deletions(-) diff --git a/include/xrpl/basics/Expected.h b/include/xrpl/basics/Expected.h index 74a0e76eef..3796151777 100644 --- a/include/xrpl/basics/Expected.h +++ b/include/xrpl/basics/Expected.h @@ -148,17 +148,23 @@ public: } [[nodiscard]] constexpr E const& - error() const + error() const& { return Base::error(); } - constexpr E& - error() + [[nodiscard]] constexpr E& + error() & { return Base::error(); } + [[nodiscard]] constexpr E&& + error() && + { + return std::move(Base::error()); + } + constexpr explicit operator bool() const { @@ -215,17 +221,23 @@ public: } [[nodiscard]] constexpr E const& - error() const + error() const& { return Base::error(); } - constexpr E& - error() + [[nodiscard]] constexpr E& + error() & { return Base::error(); } + [[nodiscard]] constexpr E&& + error() && + { + return std::move(Base::error()); + } + constexpr explicit operator bool() const { diff --git a/src/xrpld/rpc/detail/TransactionSign.cpp b/src/xrpld/rpc/detail/TransactionSign.cpp index f745bd9e1b..0fdc1bfdc7 100644 --- a/src/xrpld/rpc/detail/TransactionSign.cpp +++ b/src/xrpld/rpc/detail/TransactionSign.cpp @@ -14,6 +14,7 @@ #include #include +#include #include #include #include @@ -54,6 +55,7 @@ #include #include +#include #include #include #include @@ -405,6 +407,25 @@ checkTxJsonFields( return ret; } +static Expected +checkNetworkID(json::Value const& txJson, uint32_t appNetworkId) +{ + if (appNetworkId > 1024) + { + if (!txJson.isMember(jss::NetworkID)) + { + return Unexpected( + RPC::makeError(RpcInvalidParams, RPC::missingFieldMessage("tx_json.NetworkID"))); + } + if (!txJson[jss::NetworkID].isIntegral() || txJson[jss::NetworkID].asUInt() != appNetworkId) + { + return Unexpected( + RPC::makeError(RpcInvalidParams, RPC::invalidFieldMessage("tx_json.NetworkID"))); + } + } + return Expected(); +} + //------------------------------------------------------------------------------ // A move-only struct that makes it easy to return either a json::Value or a @@ -1165,8 +1186,16 @@ transactionSignFor( if (!txJson.isObject()) return RPC::objectFieldError(jss::tx_json); - // If the tx_json.SigningPubKey field is missing, - // insert an empty one. + if (auto checkResult = + detail::checkNetworkID(txJson, app.getNetworkIDService().getNetworkID()); + !checkResult) + { + return std::move(checkResult).error(); + } + + // If the tx_json.SigningPubKey field is missing, insert an empty one, + // in order for the `checkMultiSignFields` to not return an error + // for non-multisign transactions. if (!txJson.isMember(sfSigningPubKey.getJsonName())) txJson[sfSigningPubKey.getJsonName()] = ""; } From aa5e4ff89f1871c02c96b7f97ab7fa7d13274281 Mon Sep 17 00:00:00 2001 From: Luc des Trois Maisons Date: Wed, 13 May 2026 12:48:38 -0400 Subject: [PATCH 16/21] refactor: Improve Forwarded header field parsing (#7126) --- src/xrpld/rpc/detail/Role.cpp | 49 +++++++++++++++++++++++------------ 1 file changed, 32 insertions(+), 17 deletions(-) diff --git a/src/xrpld/rpc/detail/Role.cpp b/src/xrpld/rpc/detail/Role.cpp index 998eea15c9..638ef22630 100644 --- a/src/xrpld/rpc/detail/Role.cpp +++ b/src/xrpld/rpc/detail/Role.cpp @@ -18,6 +18,7 @@ #include #include #include +#include #include #include @@ -252,32 +253,46 @@ forwardedFor(http_request_type const& request) // Look for the Forwarded field in the request. if (auto it = request.find(boost::beast::http::field::forwarded); it != request.end()) { - auto asciiTolower = [](char c) -> char { + auto asciiToLower = [](char c) -> char { return ((static_cast(c) - 65U) < 26) ? c + 'a' - 'A' : c; }; - // Look for the first (case insensitive) "for=" - static std::string const kFOR_STR{"for="}; - char const* found = std::search( - it->value().begin(), - it->value().end(), - kFOR_STR.begin(), - kFOR_STR.end(), - [&asciiTolower](char c1, char c2) { return asciiTolower(c1) == asciiTolower(c2); }); + // Look for the first (case insensitive) "for=" at a directive + // boundary (start of value, or preceded by , ; or OWS). + static constexpr std::string_view kFOR_STR{"for="}; + auto const atFieldBoundary = [begin = it->value().begin()](auto p) { + return p == begin || p[-1] == ';' || p[-1] == ',' || p[-1] == ' ' || p[-1] == '\t'; + }; + auto found = it->value().begin(); + while (true) + { + found = std::search( + found, + it->value().end(), + kFOR_STR.begin(), + kFOR_STR.end(), + [&asciiToLower](char c1, char c2) { return asciiToLower(c1) == asciiToLower(c2); }); - if (found == it->value().end()) - return {}; + if (found == it->value().end()) + return {}; - found += kFOR_STR.size(); + if (atFieldBoundary(found)) + break; + + ++found; + } + + std::advance(found, kFOR_STR.size()); // We found a "for=". Scan for the end of the IP address. - std::size_t const pos = [&found, &it]() { - auto const remaining = static_cast(it->value().end() - found); - if (std::size_t const pos = std::string_view(found, remaining).find_first_of(",;"); - pos != std::string_view::npos) + auto const end = it->value().end(); + std::size_t const pos = [&found, &end]() { + std::size_t const pos = + std::string_view(found, std::distance(found, end)).find_first_of(",;"); + if (pos != std::string_view::npos) return pos; - return remaining; + return static_cast(std::distance(found, end)); }(); return extractIpAddrFromField({found, pos}); From 551f3c3b9622e6d8afb49ed1afe44b490bd55d16 Mon Sep 17 00:00:00 2001 From: Sergey Kuznetsov Date: Wed, 13 May 2026 18:48:43 +0100 Subject: [PATCH 17/21] refactor: Move unhex lookup table out of function (#7104) --- include/xrpl/basics/StringUtilities.h | 66 ++++++++++++++++----------- 1 file changed, 39 insertions(+), 27 deletions(-) diff --git a/include/xrpl/basics/StringUtilities.h b/include/xrpl/basics/StringUtilities.h index 13c4f04e0c..f4bdd61f6a 100644 --- a/include/xrpl/basics/StringUtilities.h +++ b/include/xrpl/basics/StringUtilities.h @@ -7,9 +7,11 @@ #include #include +#include #include #include #include +#include namespace xrpl { @@ -26,28 +28,39 @@ namespace xrpl { std::string sqlBlobLiteral(Blob const& blob); +namespace detail { + +template +concept SomeChar = std::same_as, int8_t> || + std::same_as, char> || std::same_as, uint8_t>; + +inline constexpr std::array, 256> const kDIGIT_LOOKUP_TABLE = []() { + std::array, 256> t{}; + + for (int i = 0; i < 10; ++i) + t['0' + i] = i; + + for (int i = 0; i < 6; ++i) + { + t['A' + i] = 10 + i; + t['a' + i] = 10 + i; + } + + return t; +}(); + +inline std::optional +hexCharToInt(SomeChar auto hexChar) +{ + return kDIGIT_LOOKUP_TABLE[static_cast(hexChar)]; +} + +} // namespace detail + template std::optional strUnHex(std::size_t strSize, Iterator begin, Iterator end) { - static constexpr std::array const kDIGIT_LOOKUP_TABLE = []() { - std::array t{}; - - for (auto& x : t) - x = -1; - - for (int i = 0; i < 10; ++i) - t['0' + i] = i; - - for (int i = 0; i < 6; ++i) - { - t['A' + i] = 10 + i; - t['a' + i] = 10 + i; - } - - return t; - }(); - Blob out; out.reserve((strSize + 1) / 2); @@ -56,27 +69,26 @@ strUnHex(std::size_t strSize, Iterator begin, Iterator end) if (strSize & 1) { - int c = kDIGIT_LOOKUP_TABLE[*iter++]; - - if (c < 0) + auto const c = detail::hexCharToInt(*iter++); + if (!c.has_value()) return {}; - out.push_back(c); + out.push_back(static_cast(*c)); } while (iter != end) { - int const cHigh = kDIGIT_LOOKUP_TABLE[*iter++]; + auto const cHigh = detail::hexCharToInt(*iter++); - if (cHigh < 0) + if (!cHigh.has_value()) return {}; - int const cLow = kDIGIT_LOOKUP_TABLE[*iter++]; + auto const cLow = detail::hexCharToInt(*iter++); - if (cLow < 0) + if (!cLow.has_value()) return {}; - out.push_back(static_cast((cHigh << 4) | cLow)); + out.push_back(static_cast((*cHigh << 4) | *cLow)); } return {std::move(out)}; From d4ebd6a1687405445c1604ea30435171d0614fa9 Mon Sep 17 00:00:00 2001 From: Olek <115580134+oleks-rip@users.noreply.github.com> Date: Wed, 13 May 2026 15:22:29 -0400 Subject: [PATCH 18/21] fix: Backport Permissioned Domains fixes (#7016) --- src/test/app/Invariants_test.cpp | 4 ++-- src/test/app/PermissionedDomains_test.cpp | 11 +++++------ 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/src/test/app/Invariants_test.cpp b/src/test/app/Invariants_test.cpp index b0f1aee03c..99de5be302 100644 --- a/src/test/app/Invariants_test.cpp +++ b/src/test/app/Invariants_test.cpp @@ -1291,8 +1291,8 @@ class Invariants_test : public beast::unit_test::Suite if (numCreds != 0u) { - // This array is sorted naturally, but if you willing to change this - // behavior don't forget to use credentials::makeSorted + // This array is sorted naturally, but if you are going to change + // this behavior, don't forget to use credentials::makeSorted STArray credentials(sfAcceptedCredentials, numCreds); for (std::size_t n = 0; n < numCreds; ++n) { diff --git a/src/test/app/PermissionedDomains_test.cpp b/src/test/app/PermissionedDomains_test.cpp index 78ce82b497..008f54ddd8 100644 --- a/src/test/app/PermissionedDomains_test.cpp +++ b/src/test/app/PermissionedDomains_test.cpp @@ -49,14 +49,13 @@ exceptionExpected(Env& env, json::Value const& jv) class PermissionedDomains_test : public beast::unit_test::Suite { - FeatureBitset withoutFeature_{testableAmendments() - featurePermissionedDomains}; FeatureBitset withFeature_{ - testableAmendments() // - | featurePermissionedDomains | featureCredentials}; - + (testableAmendments() // + | featurePermissionedDomains | featureCredentials) - + fixPermissionedDomainInvariant}; FeatureBitset withFix_{ testableAmendments() // - | featurePermissionedDomains | featureCredentials}; + | featurePermissionedDomains | featureCredentials | fixPermissionedDomainInvariant}; // Verify that each tx type can execute if the feature is enabled. void @@ -98,7 +97,7 @@ class PermissionedDomains_test : public beast::unit_test::Suite { testcase("Disabled"); Account const alice("alice"); - Env env(*this, withoutFeature_); + Env env(*this, testableAmendments() - featurePermissionedDomains); env.fund(XRP(1000), alice); pdomain::Credentials const credentials{{alice, "first credential"}}; env(pdomain::setTx(alice, credentials), Ter(temDISABLED)); From 2f65cb561011fadf323b4599f8e732297c487b8c Mon Sep 17 00:00:00 2001 From: Michael Legleux Date: Wed, 13 May 2026 12:34:46 -0700 Subject: [PATCH 19/21] ci: Add Conan retry (#7147) --- conan/global.conf | 2 ++ 1 file changed, 2 insertions(+) diff --git a/conan/global.conf b/conan/global.conf index 37b329a5c5..cc803dc801 100644 --- a/conan/global.conf +++ b/conan/global.conf @@ -3,3 +3,5 @@ core:non_interactive=True core.download:parallel={{ os.cpu_count() }} core.upload:parallel={{ os.cpu_count() }} +tools.files.download:retry=5 +tools.files.download:retry_wait=10 From afbccf971a553625d795502d9dab437997cc3c51 Mon Sep 17 00:00:00 2001 From: Bart Date: Wed, 13 May 2026 16:46:30 -0400 Subject: [PATCH 20/21] chore: Consolidate fix amendments (#7134) Co-authored-by: Bart <11445373+bthomee@users.noreply.github.com> --- include/xrpl/protocol/detail/features.macro | 9 ++- .../PermissionedDomainInvariant.cpp | 2 +- .../PermissionedDomainSet.cpp | 4 +- src/test/app/Invariants_test.cpp | 72 +++++++++---------- src/test/app/PermissionedDEX_test.cpp | 6 +- src/test/app/PermissionedDomains_test.cpp | 7 +- 6 files changed, 45 insertions(+), 55 deletions(-) diff --git a/include/xrpl/protocol/detail/features.macro b/include/xrpl/protocol/detail/features.macro index dc26a5a43b..fd62b74d59 100644 --- a/include/xrpl/protocol/detail/features.macro +++ b/include/xrpl/protocol/detail/features.macro @@ -15,11 +15,10 @@ // Add new amendments to the top of this list. // Keep it sorted in reverse chronological order. -XRPL_FIX (Cleanup3_2_0, Supported::No, VoteBehavior::DefaultNo) -XRPL_FEATURE(MPTokensV2, Supported::No, VoteBehavior::DefaultNo) +XRPL_FIX (Cleanup3_2_0, Supported::No, VoteBehavior::DefaultNo) +XRPL_FEATURE(MPTokensV2, Supported::No, VoteBehavior::DefaultNo) XRPL_FIX (Cleanup3_1_3, Supported::Yes, VoteBehavior::DefaultYes) -XRPL_FIX (PermissionedDomainInvariant, Supported::Yes, VoteBehavior::DefaultNo) -XRPL_FIX (BatchInnerSigs, Supported::No, VoteBehavior::DefaultNo) +XRPL_FIX (BatchInnerSigs, Supported::No, VoteBehavior::DefaultNo) XRPL_FEATURE(LendingProtocol, Supported::Yes, VoteBehavior::DefaultNo) XRPL_FEATURE(PermissionDelegationV1_1, Supported::No, VoteBehavior::DefaultNo) XRPL_FIX (DirectoryLimit, Supported::Yes, VoteBehavior::DefaultNo) @@ -34,7 +33,7 @@ XRPL_FIX (EnforceNFTokenTrustlineV2, Supported::Yes, VoteBehavior::DefaultN XRPL_FIX (AMMv1_3, Supported::Yes, VoteBehavior::DefaultNo) XRPL_FEATURE(PermissionedDEX, Supported::Yes, VoteBehavior::DefaultNo) XRPL_FEATURE(Batch, Supported::No, VoteBehavior::DefaultNo) -XRPL_FEATURE(SingleAssetVault, Supported::Yes, VoteBehavior::DefaultNo) +XRPL_FEATURE(SingleAssetVault, Supported::Yes, VoteBehavior::DefaultNo) XRPL_FIX (PayChanCancelAfter, Supported::Yes, VoteBehavior::DefaultNo) // Check flags in Credential transactions XRPL_FIX (InvalidTxFlags, Supported::Yes, VoteBehavior::DefaultNo) diff --git a/src/libxrpl/tx/invariants/PermissionedDomainInvariant.cpp b/src/libxrpl/tx/invariants/PermissionedDomainInvariant.cpp index 784a2503ca..60cc897f99 100644 --- a/src/libxrpl/tx/invariants/PermissionedDomainInvariant.cpp +++ b/src/libxrpl/tx/invariants/PermissionedDomainInvariant.cpp @@ -102,7 +102,7 @@ ValidPermissionedDomain::finalize( return true; }; - if (view.rules().enabled(fixPermissionedDomainInvariant)) + if (view.rules().enabled(fixCleanup3_1_3)) { // No permissioned domains should be affected if the transaction failed if (!isTesSuccess(result)) diff --git a/src/libxrpl/tx/transactors/permissioned_domain/PermissionedDomainSet.cpp b/src/libxrpl/tx/transactors/permissioned_domain/PermissionedDomainSet.cpp index 3258dda409..83921e8526 100644 --- a/src/libxrpl/tx/transactors/permissioned_domain/PermissionedDomainSet.cpp +++ b/src/libxrpl/tx/transactors/permissioned_domain/PermissionedDomainSet.cpp @@ -110,8 +110,8 @@ PermissionedDomainSet::doApply() if (balance < reserve) return tecINSUFFICIENT_RESERVE; - bool const fix313 = view().rules().enabled(fixCleanup3_1_3); - auto const seq = fix313 ? ctx_.tx.getSeqValue() : ctx_.tx.getFieldU32(sfSequence); + bool const fixEnabled = view().rules().enabled(fixCleanup3_1_3); + auto const seq = fixEnabled ? ctx_.tx.getSeqValue() : ctx_.tx.getFieldU32(sfSequence); Keylet const pdKeylet = keylet::permissionedDomain(account_, seq); auto slePd = std::make_shared(pdKeylet); diff --git a/src/test/app/Invariants_test.cpp b/src/test/app/Invariants_test.cpp index 99de5be302..d9670ed6a3 100644 --- a/src/test/app/Invariants_test.cpp +++ b/src/test/app/Invariants_test.cpp @@ -1314,11 +1314,11 @@ class Invariants_test : public beast::unit_test::Suite { using namespace test::jtx; - bool const fixPDEnabled = features[fixPermissionedDomainInvariant]; + bool const fixEnabled = features[fixCleanup3_1_3]; std::initializer_list const badTers = {tecINVARIANT_FAILED, tecINVARIANT_FAILED}; std::initializer_list const failTers = {tecINVARIANT_FAILED, tefINVARIANT_FAILED}; - testcase << "PermissionedDomain" + std::string(fixPDEnabled ? " fix" : ""); + testcase << "PermissionedDomain" + std::string(fixEnabled ? " fix" : ""); doInvariantCheck( Env(*this, features), @@ -1328,7 +1328,7 @@ class Invariants_test : public beast::unit_test::Suite }, XRPAmount{}, STTx{ttPERMISSIONED_DOMAIN_SET, [](STObject&) {}}, - fixPDEnabled ? failTers : badTers); + fixEnabled ? failTers : badTers); testcase << "PermissionedDomain 2"; @@ -1341,7 +1341,7 @@ class Invariants_test : public beast::unit_test::Suite }, XRPAmount{}, STTx{ttPERMISSIONED_DOMAIN_SET, [](STObject&) {}}, - fixPDEnabled ? failTers : badTers); + fixEnabled ? failTers : badTers); testcase << "PermissionedDomain 3"; doInvariantCheck( @@ -1365,7 +1365,7 @@ class Invariants_test : public beast::unit_test::Suite }, XRPAmount{}, STTx{ttPERMISSIONED_DOMAIN_SET, [](STObject&) {}}, - fixPDEnabled ? failTers : badTers); + fixEnabled ? failTers : badTers); testcase << "PermissionedDomain 4"; doInvariantCheck( @@ -1388,7 +1388,7 @@ class Invariants_test : public beast::unit_test::Suite }, XRPAmount{}, STTx{ttPERMISSIONED_DOMAIN_SET, [](STObject&) {}}, - fixPDEnabled ? failTers : badTers); + fixEnabled ? failTers : badTers); testcase << "PermissionedDomain Set 1"; doInvariantCheck( @@ -1409,7 +1409,7 @@ class Invariants_test : public beast::unit_test::Suite }, XRPAmount{}, STTx{ttPERMISSIONED_DOMAIN_SET, [](STObject&) {}}, - fixPDEnabled ? failTers : badTers); + fixEnabled ? failTers : badTers); testcase << "PermissionedDomain Set 2"; doInvariantCheck( @@ -1440,7 +1440,7 @@ class Invariants_test : public beast::unit_test::Suite }, XRPAmount{}, STTx{ttPERMISSIONED_DOMAIN_SET, [](STObject&) {}}, - fixPDEnabled ? failTers : badTers); + fixEnabled ? failTers : badTers); testcase << "PermissionedDomain Set 3"; doInvariantCheck( @@ -1470,7 +1470,7 @@ class Invariants_test : public beast::unit_test::Suite }, XRPAmount{}, STTx{ttPERMISSIONED_DOMAIN_SET, [](STObject&) {}}, - fixPDEnabled ? failTers : badTers); + fixEnabled ? failTers : badTers); testcase << "PermissionedDomain Set 4"; doInvariantCheck( @@ -1498,7 +1498,7 @@ class Invariants_test : public beast::unit_test::Suite }, XRPAmount{}, STTx{ttPERMISSIONED_DOMAIN_SET, [](STObject&) {}}, - fixPDEnabled ? failTers : badTers); + fixEnabled ? failTers : badTers); std::initializer_list const goodTers = {tesSUCCESS, tesSUCCESS}; @@ -1516,7 +1516,7 @@ class Invariants_test : public beast::unit_test::Suite testcase << "PermissionedDomain set 2 domains "; doInvariantCheck( Env(*this, features), - fixPDEnabled ? badMoreThan1 : emptyV, + fixEnabled ? badMoreThan1 : emptyV, [](Account const& a1, Account const& a2, ApplyContext& ac) { createPermissionedDomain(ac, a1, a2); createPermissionedDomain(ac, a1, a2, 2, 11); @@ -1524,7 +1524,7 @@ class Invariants_test : public beast::unit_test::Suite }, XRPAmount{}, STTx{ttPERMISSIONED_DOMAIN_SET, [](STObject&) {}}, - fixPDEnabled ? failTers : goodTers); + fixEnabled ? failTers : goodTers); } { @@ -1545,7 +1545,7 @@ class Invariants_test : public beast::unit_test::Suite std::move(env1), a1, a2, - fixPDEnabled ? badMoreThan1 : emptyV, + fixEnabled ? badMoreThan1 : emptyV, [&pd1, &pd2](Account const&, Account const&, ApplyContext& ac) { auto sle1 = ac.view().peek({ltPERMISSIONED_DOMAIN, pd1}); auto sle2 = ac.view().peek({ltPERMISSIONED_DOMAIN, pd2}); @@ -1555,18 +1555,18 @@ class Invariants_test : public beast::unit_test::Suite }, XRPAmount{}, STTx{ttPERMISSIONED_DOMAIN_DELETE, [](STObject&) {}}, - fixPDEnabled ? failTers : goodTers); + fixEnabled ? failTers : goodTers); } { testcase << "PermissionedDomain set 0 domains "; doInvariantCheck( Env(*this, features), - fixPDEnabled ? badNoDomains : emptyV, + fixEnabled ? badNoDomains : emptyV, [](Account const&, Account const&, ApplyContext&) { return true; }, XRPAmount{}, STTx{ttPERMISSIONED_DOMAIN_SET, [](STObject&) {}}, - fixPDEnabled ? badTers : goodTers); + fixEnabled ? badTers : goodTers); } { @@ -1587,11 +1587,11 @@ class Invariants_test : public beast::unit_test::Suite Env(*this, features), a1, a2, - fixPDEnabled ? badNoDomains : emptyV, + fixEnabled ? badNoDomains : emptyV, [](Account const&, Account const&, ApplyContext&) { return true; }, XRPAmount{}, STTx{ttPERMISSIONED_DOMAIN_DELETE, [](STObject&) {}}, - fixPDEnabled ? badTers : goodTers); + fixEnabled ? badTers : goodTers); } { @@ -1611,7 +1611,7 @@ class Invariants_test : public beast::unit_test::Suite std::move(env1), a1, a2, - fixPDEnabled ? badDeleted : emptyV, + fixEnabled ? badDeleted : emptyV, [&pd1](Account const&, Account const&, ApplyContext& ac) { auto sle1 = ac.view().peek({ltPERMISSIONED_DOMAIN, pd1}); ac.view().erase(sle1); @@ -1619,28 +1619,28 @@ class Invariants_test : public beast::unit_test::Suite }, XRPAmount{}, STTx{ttPERMISSIONED_DOMAIN_SET, [](STObject&) {}}, - fixPDEnabled ? failTers : goodTers); + fixEnabled ? failTers : goodTers); } { testcase << "PermissionedDomain del, create domain "; doInvariantCheck( Env(*this, features), - fixPDEnabled ? badNotDeleted : emptyV, + fixEnabled ? badNotDeleted : emptyV, [](Account const& a1, Account const& a2, ApplyContext& ac) { createPermissionedDomain(ac, a1, a2); return true; }, XRPAmount{}, STTx{ttPERMISSIONED_DOMAIN_DELETE, [](STObject&) {}}, - fixPDEnabled ? failTers : goodTers); + fixEnabled ? failTers : goodTers); } { testcase << "PermissionedDomain invalid tx"; doInvariantCheck( - fixPDEnabled ? badTx : emptyV, + fixEnabled ? badTx : emptyV, [&](Account const& a1, Account const& a2, ApplyContext& ac) { createPermissionedDomain(ac, a1, a2); return true; @@ -1800,11 +1800,9 @@ class Invariants_test : public beast::unit_test::Suite { using namespace test::jtx; - bool const fixPDEnabled = features[fixPermissionedDomainInvariant]; - bool const fixS313Enabled = features[fixCleanup3_1_3]; + bool const fixEnabled = features[fixCleanup3_1_3]; - testcase << "PermissionedDEX" + std::string(fixPDEnabled ? " fixPD" : "") + - std::string(fixS313Enabled ? " fixS313" : ""); + testcase << "PermissionedDEX" + std::string(fixEnabled ? " fix" : ""); doInvariantCheck( Env(*this, features), @@ -1908,8 +1906,8 @@ class Invariants_test : public beast::unit_test::Suite std::move(env1), a1, a2, - fixS313Enabled ? std::vector{{"hybrid offer is malformed"}} - : std::vector{}, + fixEnabled ? std::vector{{"hybrid offer is malformed"}} + : std::vector{}, [&pd1](Account const& a1, Account const& a2, ApplyContext& ac) { Keylet const offerKey = keylet::offer(a2.id(), 10); auto sleOffer = std::make_shared(offerKey); @@ -1926,9 +1924,8 @@ class Invariants_test : public beast::unit_test::Suite }, XRPAmount{}, STTx{ttOFFER_CREATE, [&](STObject&) {}}, - fixS313Enabled - ? std::initializer_list{tecINVARIANT_FAILED, tecINVARIANT_FAILED} - : std::initializer_list{tesSUCCESS, tesSUCCESS}); + fixEnabled ? std::initializer_list{tecINVARIANT_FAILED, tecINVARIANT_FAILED} + : std::initializer_list{tesSUCCESS, tesSUCCESS}); } // hybrid offer missing sfAdditionalBooks @@ -4380,13 +4377,10 @@ public: testNoZeroEscrow(); testValidNewAccountRoot(); testNFTokenPageInvariants(); - testPermissionedDomainInvariants(defaultAmendments() | fixPermissionedDomainInvariant); - testPermissionedDomainInvariants(defaultAmendments() - fixPermissionedDomainInvariant); - testPermissionedDEX(defaultAmendments() | fixPermissionedDomainInvariant); - testPermissionedDEX(defaultAmendments() - fixPermissionedDomainInvariant); - testPermissionedDEX( - (defaultAmendments() | fixPermissionedDomainInvariant) - fixCleanup3_1_3); - testPermissionedDEX(defaultAmendments() - fixPermissionedDomainInvariant - fixCleanup3_1_3); + testPermissionedDomainInvariants(defaultAmendments() | fixCleanup3_1_3); + testPermissionedDomainInvariants(defaultAmendments() - fixCleanup3_1_3); + testPermissionedDEX(defaultAmendments() | fixCleanup3_1_3); + testPermissionedDEX(defaultAmendments() - fixCleanup3_1_3); testNoModifiedUnmodifiableFields(); testValidPseudoAccounts(); testValidLoanBroker(); diff --git a/src/test/app/PermissionedDEX_test.cpp b/src/test/app/PermissionedDEX_test.cpp index 21e6bd8baa..45b4a58901 100644 --- a/src/test/app/PermissionedDEX_test.cpp +++ b/src/test/app/PermissionedDEX_test.cpp @@ -1392,10 +1392,10 @@ class PermissionedDEX_test : public beast::unit_test::Suite void testHybridMalformedOffer(FeatureBitset features) { - bool const fixS313Enabled = features[fixCleanup3_1_3]; + bool const fixEnabled = features[fixCleanup3_1_3]; testcase << "Hybrid offer with empty AdditionalBooks" - << (fixS313Enabled ? " (fixCleanup3_1_3 enabled)" : " (fixCleanup3_1_3 disabled)"); + << (fixEnabled ? " (fixCleanup3_1_3 enabled)" : " (fixCleanup3_1_3 disabled)"); // offerInDomain has two code paths gated by fixCleanup3_1_3: // @@ -1436,7 +1436,7 @@ class PermissionedDEX_test : public beast::unit_test::Suite return true; }); - if (fixS313Enabled) + if (fixEnabled) { // post-fixCleanup3_1_3: offerInDomain rejects the malformed // offer (size == 0), so no valid domain offer is found. diff --git a/src/test/app/PermissionedDomains_test.cpp b/src/test/app/PermissionedDomains_test.cpp index 008f54ddd8..0593b06eaa 100644 --- a/src/test/app/PermissionedDomains_test.cpp +++ b/src/test/app/PermissionedDomains_test.cpp @@ -50,12 +50,9 @@ exceptionExpected(Env& env, json::Value const& jv) class PermissionedDomains_test : public beast::unit_test::Suite { FeatureBitset withFeature_{ - (testableAmendments() // - | featurePermissionedDomains | featureCredentials) - - fixPermissionedDomainInvariant}; + (testableAmendments() | featurePermissionedDomains | featureCredentials) - fixCleanup3_1_3}; FeatureBitset withFix_{ - testableAmendments() // - | featurePermissionedDomains | featureCredentials | fixPermissionedDomainInvariant}; + testableAmendments() | featurePermissionedDomains | featureCredentials | fixCleanup3_1_3}; // Verify that each tx type can execute if the feature is enabled. void From cce4cfef1052107b29ada988bdb9b4b7fd1a2541 Mon Sep 17 00:00:00 2001 From: Jingchen Date: Thu, 14 May 2026 18:07:08 +0100 Subject: [PATCH 21/21] feat: Add verify_endpoints to help local peer network development (#7268) Co-authored-by: xrplf-ai-reviewer[bot] <266832837+xrplf-ai-reviewer[bot]@users.noreply.github.com> --- cfg/xrpld-example.cfg | 11 +++++ cspell.config.yaml | 1 + src/libxrpl/beast/net/IPAddressV4.cpp | 40 +++++++++++++++- src/libxrpl/beast/net/IPAddressV6.cpp | 48 ++++++++++++++++--- src/test/peerfinder/PeerFinder_test.cpp | 2 +- src/xrpld/app/main/Application.cpp | 2 +- src/xrpld/overlay/Overlay.h | 1 + src/xrpld/overlay/detail/OverlayImpl.cpp | 15 ++++-- src/xrpld/overlay/make_Overlay.h | 2 +- src/xrpld/peerfinder/PeerfinderManager.h | 10 +++- src/xrpld/peerfinder/detail/Logic.h | 4 +- .../peerfinder/detail/PeerfinderConfig.cpp | 15 ++---- 12 files changed, 124 insertions(+), 27 deletions(-) diff --git a/cfg/xrpld-example.cfg b/cfg/xrpld-example.cfg index 45269903b2..5de61bb6d1 100644 --- a/cfg/xrpld-example.cfg +++ b/cfg/xrpld-example.cfg @@ -527,6 +527,17 @@ # # The current default (which is subject to change) is 300 seconds. # +# verify_endpoints = <0 | 1> +# +# If set to 0, the server will skip validation of endpoint +# addresses received in TMEndpoints peer protocol messages, +# allowing addresses that are not publicly routable or have a +# port of 0. The default is 1 (verification enabled). +# +# WARNING: Disabling this option is a security risk and should +# only be used for local testing and debugging. Do not disable +# on mainnet. +# # # [transaction_queue] EXPERIMENTAL # diff --git a/cspell.config.yaml b/cspell.config.yaml index 34482d9d8f..bc56ef5d79 100644 --- a/cspell.config.yaml +++ b/cspell.config.yaml @@ -326,3 +326,4 @@ words: - xrplf - xxhash - xxhasher + - CGNAT diff --git a/src/libxrpl/beast/net/IPAddressV4.cpp b/src/libxrpl/beast/net/IPAddressV4.cpp index 48554d0ec3..e087da92eb 100644 --- a/src/libxrpl/beast/net/IPAddressV4.cpp +++ b/src/libxrpl/beast/net/IPAddressV4.cpp @@ -14,7 +14,45 @@ isPrivate(AddressV4 const& addr) bool isPublic(AddressV4 const& addr) { - return !isPrivate(addr) && !addr.is_multicast(); + if (isPrivate(addr)) + return false; + if (addr.is_multicast()) + return false; + + auto const ip = addr.to_uint(); + + // 0.0.0.0/8 "This network" + if ((ip & 0xff000000) == 0x00000000) + return false; + // 100.64.0.0/10 Shared Address Space (CGNAT) - RFC 6598 + if ((ip & 0xffc00000) == 0x64400000) + return false; + // 169.254.0.0/16 Link-local + if ((ip & 0xffff0000) == 0xa9fe0000) + return false; + // 192.0.0.0/24 IETF Protocol Assignments - RFC 6890 + if ((ip & 0xffffff00) == 0xc0000000) + return false; + // 192.0.2.0/24 TEST-NET-1 (documentation) - RFC 5737 + if ((ip & 0xffffff00) == 0xc0000200) + return false; + // 192.88.99.0/24 6to4 Relay Anycast (deprecated) - RFC 7526 + if ((ip & 0xffffff00) == 0xc0586300) + return false; + // 198.18.0.0/15 Benchmarking - RFC 2544 + if ((ip & 0xfffe0000) == 0xc6120000) + return false; + // 198.51.100.0/24 TEST-NET-2 (documentation) - RFC 5737 + if ((ip & 0xffffff00) == 0xc6336400) + return false; + // 203.0.113.0/24 TEST-NET-3 (documentation) - RFC 5737 + if ((ip & 0xffffff00) == 0xcb007100) + return false; + // 240.0.0.0/4 Reserved for future use - RFC 1112 + if ((ip & 0xf0000000) == 0xf0000000) + return false; + + return true; } char diff --git a/src/libxrpl/beast/net/IPAddressV6.cpp b/src/libxrpl/beast/net/IPAddressV6.cpp index fad11dddc0..c75ccaf1cc 100644 --- a/src/libxrpl/beast/net/IPAddressV6.cpp +++ b/src/libxrpl/beast/net/IPAddressV6.cpp @@ -9,17 +9,53 @@ namespace beast::IP { bool isPrivate(AddressV6 const& addr) { - return ( - ((addr.to_bytes()[0] & 0xfd) != 0) || // TODO fc00::/8 too ? - (addr.is_v4_mapped() && - isPrivate(boost::asio::ip::make_address_v4(boost::asio::ip::v4_mapped, addr)))); + // fc00::/7 - Unique Local Address (ULA), covers fc00:: and fd00:: + if ((addr.to_bytes()[0] & 0xfe) == 0xfc) + return true; + if (addr.is_v4_mapped()) + return isPrivate(boost::asio::ip::make_address_v4(boost::asio::ip::v4_mapped, addr)); + return false; } bool isPublic(AddressV6 const& addr) { - // TODO is this correct? - return !isPrivate(addr) && !addr.is_multicast(); + if (addr.is_loopback()) + return false; + if (addr.is_v4_mapped()) + return isPublic(boost::asio::ip::make_address_v4(boost::asio::ip::v4_mapped, addr)); + if (isPrivate(addr)) + return false; + if (addr.is_multicast()) + return false; + if (addr.is_unspecified()) + return false; + + auto const b = addr.to_bytes(); + + // fe80::/10 - Link-local + if (b[0] == 0xfe && (b[1] & 0xc0) == 0x80) + return false; + // 100::/64 - Discard prefix (RFC 6666) + if (b[0] == 0x01 && b[1] == 0x00 && b[2] == 0 && b[3] == 0 && b[4] == 0 && b[5] == 0 && + b[6] == 0 && b[7] == 0) + return false; + // 2001:db8::/32 - Documentation (RFC 3849) + if (b[0] == 0x20 && b[1] == 0x01 && b[2] == 0x0d && b[3] == 0xb8) + return false; + // 2001::/32 - IETF Protocol Assignments / Teredo (RFC 4380) + if (b[0] == 0x20 && b[1] == 0x01 && b[2] == 0x00 && b[3] == 0x00) + return false; + // 2001:20::/28 - ORCHIDv2 (RFC 7343) + // 28-bit prefix: 0x2001002 => b[0]=0x20, b[1]=0x01, b[2]=0x00, + // top nibble of b[3]=0x2 + if (b[0] == 0x20 && b[1] == 0x01 && b[2] == 0x00 && (b[3] & 0xf0) == 0x20) + return false; + // 2002::/16 - 6to4 (RFC 3056, deprecated by RFC 7526) + if (b[0] == 0x20 && b[1] == 0x02) + return false; + + return true; } } // namespace beast::IP diff --git a/src/test/peerfinder/PeerFinder_test.cpp b/src/test/peerfinder/PeerFinder_test.cpp index 17ce6835ef..2e4ab001b4 100644 --- a/src/test/peerfinder/PeerFinder_test.cpp +++ b/src/test/peerfinder/PeerFinder_test.cpp @@ -424,7 +424,7 @@ public: (c.PEERS_MAX == max && c.PEERS_IN_MAX == 0 && c.PEERS_OUT_MAX == 0) || (c.PEERS_IN_MAX == *maxIn && c.PEERS_OUT_MAX == *maxOut)); - Config const config = Config::makeConfig(c, port, false, 0); + Config const config = Config::makeConfig(c, port, false, 0, true); Counts counts; counts.onConfig(config); diff --git a/src/xrpld/app/main/Application.cpp b/src/xrpld/app/main/Application.cpp index f7679d14d8..20a81f6768 100644 --- a/src/xrpld/app/main/Application.cpp +++ b/src/xrpld/app/main/Application.cpp @@ -1357,7 +1357,7 @@ ApplicationImp::setup(boost::program_options::variables_map const& cmdline) // if (!config_.standalone()) overlay_ = makeOverlay( *this, - setupOverlay(*config_), + setupOverlay(*config_, journal_), *serverHandler_, *resourceManager_, *resolver_, diff --git a/src/xrpld/overlay/Overlay.h b/src/xrpld/overlay/Overlay.h index 80430293d8..ef97ea7f24 100644 --- a/src/xrpld/overlay/Overlay.h +++ b/src/xrpld/overlay/Overlay.h @@ -47,6 +47,7 @@ public: std::uint32_t crawlOptions = 0; std::optional networkID; bool vlEnabled = true; + bool verifyEndpoints = true; }; using PeerSequence = std::vector>; diff --git a/src/xrpld/overlay/detail/OverlayImpl.cpp b/src/xrpld/overlay/detail/OverlayImpl.cpp index bfed850e5c..770df6e43a 100644 --- a/src/xrpld/overlay/detail/OverlayImpl.cpp +++ b/src/xrpld/overlay/detail/OverlayImpl.cpp @@ -508,7 +508,8 @@ OverlayImpl::start() app_.config(), serverHandler_.setup().overlay.port(), app_.getValidationPublicKey().has_value(), - setup_.ipLimit); + setup_.ipLimit, + setup_.verifyEndpoints); peerFinder_->setConfig(config); peerFinder_->start(); @@ -1510,7 +1511,7 @@ OverlayImpl::deleteIdlePeers() //------------------------------------------------------------------------------ Overlay::Setup -setupOverlay(BasicConfig const& config) +setupOverlay(BasicConfig const& config, beast::Journal j) { Overlay::Setup setup; @@ -1528,9 +1529,17 @@ setupOverlay(BasicConfig const& config) { boost::system::error_code ec; setup.publicIp = boost::asio::ip::make_address(ip, ec); - if (ec || beast::IP::isPrivate(setup.publicIp)) + if (ec || !beast::IP::isPublic(setup.publicIp)) Throw("Configured public IP is invalid"); } + + set(setup.verifyEndpoints, true, "verify_endpoints", section); + if (!setup.verifyEndpoints) + { + JLOG(j.warn()) << "Endpoint verification is disabled. This is a " + "security risk and should only be used for " + "testing."; + } } { diff --git a/src/xrpld/overlay/make_Overlay.h b/src/xrpld/overlay/make_Overlay.h index 94d1110f4b..acd477deec 100644 --- a/src/xrpld/overlay/make_Overlay.h +++ b/src/xrpld/overlay/make_Overlay.h @@ -10,7 +10,7 @@ namespace xrpl { Overlay::Setup -setupOverlay(BasicConfig const& config); +setupOverlay(BasicConfig const& config, beast::Journal j); /** Creates the implementation of Overlay. */ std::unique_ptr diff --git a/src/xrpld/peerfinder/PeerfinderManager.h b/src/xrpld/peerfinder/PeerfinderManager.h index 5b235a4db1..083abf92c6 100644 --- a/src/xrpld/peerfinder/PeerfinderManager.h +++ b/src/xrpld/peerfinder/PeerfinderManager.h @@ -59,6 +59,9 @@ struct Config /** Limit how many incoming connections we allow per IP */ int ipLimit{0}; + /** `true` if we want to verify endpoints in TMEndpoints messages */ + bool verifyEndpoints = true; + //-------------------------------------------------------------------------- /** Create a configuration with default values. */ @@ -81,6 +84,8 @@ struct Config * @param port server's listening port * @param validationPublicKey true if validation public key is not empty * @param ipLimit limit of incoming connections per IP + * @param verifyEndpoints `true` if we want to verify endpoints in + * TMEndpoints messages * @return PeerFinder::Config */ static Config @@ -88,10 +93,11 @@ struct Config xrpl::Config const& config, std::uint16_t port, bool validationPublicKey, - int ipLimit); + int ipLimit, + bool verifyEndpoints); friend bool - operator==(Config const& lhs, Config const& rhs); + operator==(Config const& lhs, Config const& rhs) = default; }; //------------------------------------------------------------------------------ diff --git a/src/xrpld/peerfinder/detail/Logic.h b/src/xrpld/peerfinder/detail/Logic.h index fce5bb4afa..4d0cc9cf29 100644 --- a/src/xrpld/peerfinder/detail/Logic.h +++ b/src/xrpld/peerfinder/detail/Logic.h @@ -696,7 +696,7 @@ public: } // Discard invalid addresses - if (!isValidAddress(ep.address)) + if (config_.verifyEndpoints && !isValidAddress(ep.address)) { JLOG(journal.debug()) << beast::Leftw(18) << "Endpoints drop " << ep.address << " as invalid"; @@ -1088,6 +1088,8 @@ public: { if (isUnspecified(address)) return false; + if (isLoopback(address)) + return false; if (!isPublic(address)) return false; if (address.port() == 0) diff --git a/src/xrpld/peerfinder/detail/PeerfinderConfig.cpp b/src/xrpld/peerfinder/detail/PeerfinderConfig.cpp index 83b78883db..a8323f6239 100644 --- a/src/xrpld/peerfinder/detail/PeerfinderConfig.cpp +++ b/src/xrpld/peerfinder/detail/PeerfinderConfig.cpp @@ -15,16 +15,6 @@ Config::Config() : outPeers(calcOutPeers()) { } -bool -operator==(Config const& lhs, Config const& rhs) -{ - return lhs.autoConnect == rhs.autoConnect && lhs.peerPrivate == rhs.peerPrivate && - lhs.wantIncoming == rhs.wantIncoming && lhs.inPeers == rhs.inPeers && - lhs.maxPeers == rhs.maxPeers && lhs.outPeers == rhs.outPeers && - lhs.features == rhs.features && lhs.ipLimit == rhs.ipLimit && - lhs.listeningPort == rhs.listeningPort; -} - std::size_t Config::calcOutPeers() const { @@ -61,6 +51,7 @@ Config::onWrite(beast::PropertyStream::Map& map) const map["port"] = listeningPort; map["features"] = features; map["ip_limit"] = ipLimit; + map["verify_endpoints"] = verifyEndpoints; } Config @@ -68,7 +59,8 @@ Config::makeConfig( xrpl::Config const& cfg, std::uint16_t port, bool validationPublicKey, - int ipLimit) + int ipLimit, + bool verifyEndpoints) { PeerFinder::Config config; @@ -121,6 +113,7 @@ Config::makeConfig( config.listeningPort = port; config.features = ""; config.ipLimit = ipLimit; + config.verifyEndpoints = verifyEndpoints; // Enforce business rules config.applyTuning();