Commit Graph

439 Commits

Author SHA1 Message Date
Bart
1cbcded326 fix: Refuse to walk an invalid SHAMap in getMissingNodes
A walk that reaches a position only a leaf may occupy marks the map Invalid and abandons the
descent. Reaching that position is otherwise fatal: SHAMapNodeID::getChildNodeID() throws
std::logic_error past kLeafDepth, and there is no try/catch around getMissingNodes() in
InboundLedger::trigger(), around job.doJob() in JobQueue, or in Workers::Worker::run(), so the
exception leaves the thread function and reaches std::terminate(). It is reachable without any node
passing through addKnownNode(): InboundLedgers::gotStaleData() stores any parseable node from an
unsolicited liAS_NODE reply into the fetch pack keyed by its own hash, with no relatedness check, and
getFetchPack() re-verifies only that hash, so such a node canonicalizes into the tree and the walk
descends onto it. Steering which hash a node acquires needs it to have no trusted validations -
starting up, or an empty or misconfigured UNL - with the attacker holding its peer slots, which makes
this a conditioned remote denial of service rather than a single-packet one.

As in addKnownNode(), the depth test precedes the full-below cache lookup, since that cache is keyed
by node hash and shared across maps and a hash does not cover depth, so a hit would carry the whole
branch past the guard. Four further tests of isValid() bound what the walk does once the verdict
lands: it short-circuits on entry rather than re-deriving a verdict already reached; it breaks out of
the descent but falls through to the deferred-read drain, since posted reads hold a reference to the
MissingNodes block on this frame; it discards whatever was collected, which belongs to a tree that
cannot exist; and it re-tests before clearSynching(), since another thread's addKnownNode() can write
the verdict after the loop's own test. Callers therefore have to re-check isValid() before reading an
empty result as nothing left to fetch, which getMissingNodes()'s docstring states. Three of those
four guards are defensive and no test drives them; only the entry short-circuit and the verdict itself
are pinned.

DeepChain gains withDecoys(): the same chain, but with a second and unresolvable child at every
level, so a backed map's descendAsync() posts a real asynchronous read at every level. That is what
leaves reads in flight when a walk reaches kLeafDepth, which is what the sanitizer case below needs.

Seven cases cover this. Five drive the walk through a ChainFilter, which stands in for a fetch pack by
serving nodes by hash and never structurally: the walk reaches the verdict itself on an unbacked map;
it does so on a backed map with the offending node already marked full below; it drains the reads a
decoy child at every level leaves in flight, which only a sanitizer can see; it refuses an
already-invalid map; and it leaves a walk that stops one level short alone, reporting the genuinely
missing child. The sixth pins that addRootNode() cannot clear the synching flag on an invalid map,
since that call site needs a leaf root and so a zero root hash. The seventh races a walk against
setImmutable() under ThreadSanitizer, and asserts only what trySetState() offers: the verdict stands,
whatever the interleaving. It is skipped at run time rather than compiled out, so every build parses
it.
2026-08-23 16:31:06 -04:00
Bart
f588200c05 fix: Refuse to make an invalid SHAMap or Ledger immutable
An immutable map is treated as persistable, so SHAMap::setImmutable() returns [[nodiscard]] bool and
refuses a map that has been proven impossible. Every state change goes through trySetState(),
whose compare-exchange refuses to leave Invalid however it interleaves with another thread's, so
setSynching() and clearSynching() cannot launder an abandoned map back into a state that passes
isValid() either. clearSynching() reports its refusal and carries on, since peer data produces that
verdict and an abort there would be one a peer could ask for, while setSynching() keeps an UNREACHABLE
and says why it is out of reach: it only ever runs on a map that has just been constructed. The
snapshot constructor carries Invalid over rather than promoting it, since a snapshot shares the
source's root, and reads the source's state once into a local so a concurrent walk cannot have it
report two different things.

Ledger::setImmutable() and Ledger::setAccepted() do the same one level up. Both return [[nodiscard]]
bool, and setImmutable() asks mapsValid() before writing anything, so a refusal leaves the header
exactly as it was rather than relabelled on its way to failing. Past that check it settles both maps
through setMapsImmutable(), which is deliberately not short-circuited: each map becomes Immutable or
stays Invalid on its own, and neither is left mid-sync because the other refused. immutable_ is set
last, so isImmutable() never reports a ledger whose maps are not both immutable. The guard is
best-effort by nature, which the comments say: setInvalid() outranks Immutable, so a walk that reaches
the verdict after both maps are settled narrows the window rather than closing it.

All fourteen call sites branch on the result, and the rule that a ledger built or loaded locally
cannot have an invalid map is stated once, in Ledger::setImmutable()'s docstring, with each such site
pointing there. The tiers differ by what the caller can do: the two genesis paths and buildLedgerImpl()
call logicError(), the last of those explaining why it takes the harsher tier on the consensus hot
path; loadLedgerFromFile(), getLastFullLedger() and finishLoadByIndexOrHash() return instead, and the
last of those clears the pointer, since nothing gates usability on the full flag and a caller that
took the ledger would abort further on. InboundLedger and TransactionAcquire recover, since for them a
refusal is an outcome a peer can produce: each withdraws complete_ alongside the failure, so a guard
that reads that flag before failed_ cannot go on treating the result as delivered.

Six cases cover it. Five are gtest: an invalid map refuses repeatedly and is not synching either; a
refusal leaves both header map hashes and the ledger hash untouched; an invalid transaction map and an
invalid state map each block the enclosing ledger, covering both operands of the test in
setImmutable(); and a snapshot of an invalid map is invalid and unpersistable in both flavors. The
boost case drives InboundLedger::done() with a map invalidated after the have-flags were set, and
checks the acquisition reports neither complete nor delivered and remembers the hash as a failure.
2026-08-23 16:04:32 -04:00
Bart
f45cce80e6 fix: Report a map-invalidating node as invalid data
SHAMap::addKnownNode() reports invalid() for the two kinds of node it does not hook into the map: an
inner node arriving at kLeafDepth, a depth only a leaf may occupy, and a node whose ID does not match
the position the descent stopped at. That is the verdict callers already handle as bad data, so
neither counts as forward progress. Each emits one warning naming the node and where the descent
stopped, matching the sibling branches beside them, and carries a SOMETIMES() hint for the fuzzer.
The depth rule is spelled once, as a file-local isLeafDepth() that hasLeafNode() reads too. The
verdict on a map-invalidating node belongs to the root hash that was asked for rather than to this
copy of the tree, since every node from the root down hash-verified to get there: no peer can satisfy
such a hash, retrying is futile, and it cannot arise by accident. A charge for it is a deterrent
rather than a control even so, which the comment says, because the same node can reach a map through a
fetch pack or an unsolicited object reply and neither passes through here.

The depth test precedes the full-below cache lookup on the way down. That cache is keyed by node hash
and shared by every map of a family, and a hash covers a node's children but not its depth, so the
same subtree hash can be cached as complete at one depth and reached at kLeafDepth here. Testing the
depth first is what keeps the verdict independent of whatever an unrelated map cached, which is the
determinism the acquisition paths need from it. Skipping the shortcut at the deepest level only
forgoes an optimization, and the depth it guards cannot occur in a real tree.

Three tests drive the map through DeepChain's fill() and addOffendingNode(), so a case names the
position no valid tree can occupy without spelling out the descent. They cover the map-invalidating
node; the three ways a node cannot be hooked anywhere while leaving the map sound; and the same
offending node with a full-below entry already in place, so the depth test is what has to reach the
verdict. A file-local tallyIs() reads each verdict as counts, which leaves get()'s wording pinned in
one place rather than at every site with a verdict to check.
2026-08-23 16:04:26 -04:00
Bart
adb772ef2f fix: Make the SHAMap sync-path state atomic
Background ledger acquisition reads and writes SHAMap::state_, SHAMap::full_, SHAMap::ledgerSeq_ and
SHAMapInnerNode::fullBelowGen_ concurrently with the thread driving it, so all four are std::atomic.
state_ is read through state() and written through setInvalid() and the existing setters, and
SHAMapState carries an explicit std::uint8_t underlying type. finishFetch() withdraws full_ with an
exchange behind a relaxed load, so exactly one of the reader threads that miss reports the gap, and a
map that is already not full stays off the exclusive-write path: full_ shares a cache line with
state_ and ledgerSeq_, and a walk posts up to 512 reads per pass. ledgerSeq_ is read through
ledgerSeq() and relaxed both ways, since it only serves as a lookup hint for a nodestore keyed by
hash.

Ledger::setFull() sets each map's ledger sequence before its full flag. A release store publishes
only what is sequenced before it, and the finishFetch() thread that wins the exchange on the flag
reads the sequence after that, so this is the order that makes the sequence visible to the once-only
gap report.

Static assertions pin the three SHAMap members lock-free, and pin fullBelowGen_'s size and alignment
to those of a plain std::uint32_t, so SHAMapInnerNode's packed layout stays byte-identical and
isFullBelow() takes no mutex once per node of every walk. Its accessors are relaxed, since a
generation is only ever compared for equality and the children it vouches for are published through
the node's own child lock. Three tests cover this: sixteen unresolvable branches posted at a backed
map with four nodestore reader threads, so finishFetch() runs concurrently for one map and the single
gap report is observable; that Ledger::setFull() publishes the sequence that report names; and that
every node the sync path hands to a filter carries it.
2026-08-23 16:04:19 -04:00
Bart
ad603b2531 test: Read a SHAMapAddNode verdict as counts
SHAMapAddNode gains getBad() and getDuplicate() beside getGood(), so a verdict can be read as
counts. Every accessor, mutator and factory is documented, and reset(), get() and operator+= move
after the static factories, so the counters and the ways to read or combine them stay grouped. get()
is a log format, and src/tests/libxrpl/shamap/SHAMapAddNode.cpp is the one place that depends on its
wording: it pins that format, the counts the three accessors report, and the way one tally
accumulates into another.
2026-08-23 16:00:23 -04:00
Vito Tumas
d1dc7a6ccf refactor: Extract invariant invocation into free checkInvariants runner (#7404)
Co-authored-by: Cursor <cursoragent@cursor.com>
2026-08-19 14:10:11 +00:00
Timur Yalymov
368ff1afce fix: Exempt loan default from asset freeze (#7932)
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Vito Tumas <5780819+Tapanito@users.noreply.github.com>
Co-authored-by: Ayaz Salikhov <mathbunnyru@users.noreply.github.com>
2026-08-19 13:43:40 +00:00
Vito Tumas
3adf2d40b5 fix: Reject VaultWithdraw fixed-share amounts that round to zero (#7950) 2026-08-19 13:09:38 +00:00
Bart
ca39bff3c8 refactor: Add SHAMapNodeID::isPrefixOf (#7939)
Co-authored-by: Bart <11445373+bthomee@users.noreply.github.com>
2026-08-18 12:35:32 +00:00
Copilot
820ca5b332 refactor: Convert boost::beast::string_view to std::string_view (#6306)
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: mvadari <8029314+mvadari@users.noreply.github.com>
Co-authored-by: Mayukha Vadari <mvadari@ripple.com>
Co-authored-by: Ayaz Salikhov <mathbunnyru@users.noreply.github.com>
Co-authored-by: xrplf-ai-reviewer[bot] <266832837+xrplf-ai-reviewer[bot]@users.noreply.github.com>
Co-authored-by: Mayukha Vadari <mvadari@gmail.com>
Co-authored-by: Timur Yalymov <36795566+tyalymov@users.noreply.github.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Vito Tumas <5780819+Tapanito@users.noreply.github.com>
2026-08-17 23:19:56 +00:00
Gregory Tsipenyuk
1b226c8b2e perf: Optimize MPT freeze checks to reduce redundant state reads (#7411)
Co-authored-by: Chenna Keshava B S <21219765+ckeshava@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
2026-08-17 21:15:16 +00:00
Gregory Tsipenyuk
ca6121c5b3 feat: Enforce MPT CanTransfer on AMM LPTokens transfers (#7418) 2026-08-17 20:58:46 +00:00
Bart
5337d028a2 refactor: Use unsigned int for branch-related operations (#7938)
Co-authored-by: Bart <11445373+bthomee@users.noreply.github.com>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-17 10:07:14 +00:00
Bart
2adffaef72 refactor: Remove support for protocol version 2.1 (#7432)
Co-authored-by: Bart <11445373+bthomee@users.noreply.github.com>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-14 15:36:47 +00:00
Mayukha Vadari
d34aa37b3c refactor: Use std::format instead of boost::format where it fits (#7996)
Co-authored-by: Timur Yalymov <36795566+tyalymov@users.noreply.github.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Vito Tumas <5780819+Tapanito@users.noreply.github.com>
Co-authored-by: Ayaz Salikhov <mathbunnyru@users.noreply.github.com>
2026-08-14 13:49:08 +00:00
Jingchen
8e9b1791c5 feat: Add a new closed ended vault to extend SAV (#7921)
Co-authored-by: Vito Tumas <5780819+Tapanito@users.noreply.github.com>
2026-08-12 17:07:43 +00:00
Copilot
153b7839a7 refactor: Replace boost::filesystem with std::filesystem across the codebase (#7012)
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: mvadari <8029314+mvadari@users.noreply.github.com>
Co-authored-by: Mayukha Vadari <mvadari@ripple.com>
Co-authored-by: Mayukha Vadari <mvadari@gmail.com>
Co-authored-by: Ayaz Salikhov <mathbunnyru@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: mathbunnyru <12270691+mathbunnyru@users.noreply.github.com>
2026-08-12 13:40:39 +00:00
Gregory Tsipenyuk
26cc683ec1 fix: Assorted MPT/DEX fixes (#7299)
Co-authored-by: Valentin Balaschenko <13349202+vlntb@users.noreply.github.com>
2026-08-11 18:15:51 +00:00
Mayukha Vadari
6ca2fb84d4 refactor: Replace Boost trim and to_lower with libxrpl helpers (#7995) 2026-08-11 18:15:35 +00:00
klemenfn
a3147740f2 build: Fix GCC 14 compilation (#7981)
Co-authored-by: Ayaz Salikhov <mathbunnyru@users.noreply.github.com>
2026-08-11 13:24:56 +00:00
Alex Kremer
0a572833ea chore: Gtest migration followups second pass (#7888) 2026-08-11 12:38:40 +00:00
Kassaking7
60291c3ed6 fix: Allow OverrideFreeze to bypass individual/deep freeze on AMM trust lines (#6959) 2026-08-10 21:34:28 +00:00
yinyiqian1
b19c3c64f2 fix: Add zero keylet check in credential (#7971) 2026-08-10 17:47:16 +00:00
Mayukha Vadari
a0e1e578a0 refactor: Remove operator!= overloads that C++20 synthesizes (#7994) 2026-08-10 17:23:02 +00:00
Mayukha Vadari
4f8819565a fix: Assorted cleanup fixes (#7988) 2026-08-10 17:18:22 +00:00
Mayukha Vadari
6580b200db refactor: Replace boost::lexical_cast with existing alternatives (#7991) 2026-08-10 17:10:18 +00:00
Gregory Tsipenyuk
94bccb3a5a fix: Fix MPT/DEX Audit/Attackathon reports (Phase 2) (#7537)
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Sergey Kuznetsov <skuznetsov@ripple.com>
Co-authored-by: Ayaz Salikhov <mathbunnyru@users.noreply.github.com>
Co-authored-by: Andrzej Budzanowski <andrzej.budzanowski@neti-soft.com>
Co-authored-by: Marek Foss <marek.foss@neti-soft.com>
Co-authored-by: Alex Kremer <akremer@ripple.com>
Co-authored-by: Vito Tumas <5780819+Tapanito@users.noreply.github.com>
Co-authored-by: Bart <bthomee@users.noreply.github.com>
Co-authored-by: Bart <11445373+bthomee@users.noreply.github.com>
Co-authored-by: Mayukha Vadari <mvadari@ripple.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
2026-08-07 21:53:54 +00:00
Mayukha Vadari
0fb92c3194 refactor: Use SeqProxy instead of uint32 for all sequence-based keylets (#7890)
Co-authored-by: xrplf-ai-reviewer[bot] <266832837+xrplf-ai-reviewer[bot]@users.noreply.github.com>
2026-08-07 21:29:11 +00:00
Ayaz Salikhov
9859e5ceda Merge remote-tracking branch 'upstream/release/3.3.x' into mathbunnyru/merge-3.3.0-to-develop
* upstream/release/3.3.x: (41 commits)
  chore: Bump version to 3.3.0
  chore: Bump version to 3.3.0-rc7
  fix: Increase manifest protocol message size cap and fix manifests relay
  fix: Cap untrusted manifests per message and drop oversized ones
  chore: Bump version to 3.2.1
  chore: Bump version to 3.2.1-rc1
  fix: Cap untrusted manifests per message and drop oversized ones
  fix: Reject oversized validator manifest before decoding
  fix: Reduce untrusted manifest cache cap to 100
  fix: Bound untrusted manifest cache
  chore: Bump version to 3.3.0-rc6
  feat: Package validator-keys inside rippled
  chore: Bump version to 3.3.0-rc5
  fix: Switch SponsorshipSet to use a delta for sfFeeAmount
  fix: Re-revert "fix: Set request size limits and differential pricing for get-object-by-hash calls"
  chore: Bump version to 3.3.0-rc4
  fix: Revert "fix: Set request size limits and differential pricing for get-object-by-hash calls"
  chore: Bump version to 3.3.0-rc3
  fix: Reduce untrusted manifest cache cap to 100
  fix: Revert "fix: Reject oversized SHAMap nodes in gotStaleData and fetch-pack path"
  ...
2026-08-07 16:00:25 +01:00
Ayaz Salikhov
cb425647a4 ci: Generate protocol_autogen only once in CI (#7918) 2026-08-06 13:25:28 +00:00
Pratik Mankawde
54cfdda00b fix: Increase manifest protocol message size cap and fix manifests relay
Signed-off-by: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com>
2026-08-04 17:08:43 -04:00
Vito Tumas
c3ee602002 test: Split Loan_test.cpp into topical suites (#7864)
Co-authored-by: Ayaz Salikhov <mathbunnyru@users.noreply.github.com>
2026-08-04 15:43:59 +00:00
Alex Kremer
06488c1318 chore: Rename CamelCase namespaces to snake_case (#7933)
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
2026-08-04 13:46:55 +00:00
Luc des Trois Maisons
b8451ffa32 fix: Add missing value_type to JSON iterators (#7907) 2026-08-03 21:17:23 +00:00
Pratik Mankawde
8461ded0d8 fix: Cap untrusted manifests per message and drop oversized ones 2026-08-03 12:00:11 -04:00
Bart
21cd615407 perf: Replace node ID by depth in TMLedgerNode (#6353)
Co-authored-by: Bart <11445373+bthomee@users.noreply.github.com>
2026-07-30 15:02:05 +00:00
Vito Tumas
8a5eded4f1 feat: Implement LoanBroker cash-basis accounting (#7817) 2026-07-30 11:55:39 +00:00
Alex Kremer
6ddad54985 chore: Move lexical cast tests to gtest (#7873) 2026-07-29 22:54:46 +00:00
Mayukha Vadari
24b6dad287 fix: Switch SponsorshipSet to use a delta for sfFeeAmount 2026-07-29 14:24:55 -04:00
Alex Kremer
86832edc70 chore: Move semantic version tests to gtest (#7872)
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
2026-07-27 18:48:53 +00:00
Alex Kremer
6c9c7f0555 chore: Trivial gtest migrations (#7865) 2026-07-27 16:34:53 +00:00
Andrzej Budzanowski
29120dfcbd test: Migrate nodestore tests from Beast to GTest (#7292)
Co-authored-by: Marek Foss <marek.foss@neti-soft.com>
Co-authored-by: Alex Kremer <akremer@ripple.com>
2026-07-27 13:00:14 +00:00
Kassaking7
9afa1cf4d1 fix: Update PermissionedDEX invariant domain tracking for valid offer replacement (#7387)
Co-authored-by: Bart <bthomee@users.noreply.github.com>
2026-07-23 21:40:21 +00:00
Marek Foss
4c0180b3db test: Migrate csf and xrpld-consensus Beast non-JTx tests to GTest (#7046)
Co-authored-by: Alex Kremer <akremer@ripple.com>
2026-07-23 21:38:21 +00:00
Marek Foss
4acccfeda8 test: Modularize Peerfinder component and migrate Peerfinder tests from Beast to GTest and GMock (#7054)
Co-authored-by: Alex Kremer <akremer@ripple.com>
2026-07-23 21:00:06 +00:00
Pratik Mankawde
c50edf507c fix: Reduce untrusted manifest cache cap to 100 2026-07-23 16:27:45 -04:00
Mayukha Vadari
38c54c3f36 feat: Add fixCleanup3_4_0 amendment (no functionality yet) (#7854) 2026-07-23 18:50:59 +00:00
Shawn Xie
6b3eaf091b fix: Change ConfidentialMPTConvert to no delegate 2026-07-17 16:06:17 -04:00
Pratik Mankawde
68a765d929 fix: Bound untrusted manifest cache 2026-07-17 14:07:57 -04:00
yinyiqian1
033dca2f0e feat: Make DynamicMPT opt-in-immutable 2026-07-17 14:02:35 -04:00