From 2c2c45da6b60b4673a45e8da17c3ad0e50d26480 Mon Sep 17 00:00:00 2001 From: Ayaz Salikhov Date: Tue, 12 May 2026 20:24:05 +0100 Subject: [PATCH] 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) {