fix: Make NodePathStack fail closed when assertions are compiled out

[bthomee]

Part 7/8 of a stack. Base: part 6 (bthomee/shamap-stack-06-unify-upper-lower-bound).

The stack asserted its preconditions and then went ahead regardless. Asserts
expand to assert, so in a release build every one of those was a no-op in
front of the operation it was guarding: reading or popping an empty
std::stack is undefined, pushChild's out-of-range branch check was missing
entirely, and getChildNodeID throws std::logic_error at leaf depth. None
of these conditions are reachable through any of SHAMap's public entry
points today, but the failure modes if they ever did happen would be
disproportionate: an out-of-range branch would silently corrupt a node ID
instead of failing loudly, and a logic_error reaching an unguarded call
chain would abort the process, since nothing in this codebase catches it.

Pushes now return false instead of throwing or silently corrupting the ID, and
reads degrade to a null node rather than undefined behavior. [[nodiscard]]
makes an unchecked push a compile error. Each new guard is marked
UNREACHABLE rather than left implicitly untested, since no test fixture in
this suite can reach these paths without building a deliberately corrupt map.
pushRoot gets the same conversion as every sibling method; it was the one
push still asserting instead of returning false.

walkTowardsKey's two modes (with and without a caller-supplied stack) must
fail at the same node and leave the stack in a state every caller already
knows how to handle; on failure the stack is now cleared via a restored
clear(), and a restored pushCurrent lambda keeps the loop-entry and
post-loop push-and-clear logic from being duplicated. It also stops deriving
each node ID twice: a caller-supplied stack now reads the ID pushNode just
computed off stack->top().second, instead of a redundant local copy that
additionally went stale once the loop exited. belowHelper and
peekNextItem finally get the fallback top()'s own docstring promises: both
read stack.top() right after an assert-only emptiness check, with no
fallback for release builds, so both now return early on an empty stack
instead of dereferencing a null SHAMapTreeNodePtr.

pushChild's hard guard also only checked the parent's depth against
kLeafDepth, one level too permissive for an inner child: a parent at 63
passed the check, then pushed an inner child at 64 with only a debug-only
assert catching it, the exact gap this commit exists to close. Tightened to
require depth + 1 below kLeafDepth for an inner child, with
walkTowardsKey's no-stack path given the identical tightening so a
malformed map fails at the same node in both modes.

[Copilot]

Pull request overview

This PR hardens SHAMap traversal/path-stack logic so that release builds (where XRPL_ASSERT compiles out) fail closed instead of risking UB (e.g., std::stack::top()/pop() on empty) or inconsistent failure behavior between stack vs no-stack traversal modes.

Changes:

• Convert NodePathStack push operations (pushRoot/pushChild/pushNode) to [[nodiscard]] bool and add explicit guards for invalid pushes (empty parent, null node, out-of-range branch, overly-deep inner child).
• Update SHAMap traversal/iteration helpers to check push outcomes / empty-stack conditions and return nullptr or throw SHAMapMissingNode rather than continuing under invalid preconditions.
• Tighten walkTowardsKey depth handling for the no-stack mode to mirror pushChild’s “inner child depth” constraint and keep both modes aligned.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
src/libxrpl/shamap/SHAMap.cpp	Updates traversal/iteration code paths to handle push failures and empty stacks safely in release builds, and aligns depth-failure behavior between stack/no-stack modes.
include/xrpl/shamap/SHAMap.h	Hardens NodePathStack to avoid UB when assertions are compiled out by adding runtime guards and returning status from push operations.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.