refactor: Unify upperBound and lowerBound into boundHelper

[bthomee]

Part 6/8 of a stack. Base: part 5 (bthomee/shamap-stack-05-derive-traversal-node-ids).

The two functions were near duplicates: walk to the key, then look for the
nearest leaf on one side. Only the scan direction, the comparison deciding a
leaf qualifies, and whether to take the first or last leaf below the subtree
differed, exactly the distinction BelowDirection already draws for
belowHelper, so the pair collapse into one parameterised walk. Also drops
the stale // TODO: what to return here? above lowerBound's return end():
no predecessor is the correct answer for the smallest key, and the tests pin
it.

Existing coverage only exercised boundHelper's inner-node branch, every test
map had at least three items, so the root was always an inner node and the
leaf branch at the top of the function was never reached with a real answer to
give. Adds coverage for a single-item map, where the leaf branch alone decides
the outcome, and an empty map, where the scan must find nothing on every
branch before falling through to end().

Fixes the single-item test's own comment, which claimed root_ becomes a
leaf, when in fact root_ stays the inner node it was constructed with for
any map built via addItem; only a single-item map synced from a peer
(addRootNode) ever replaces root_ with a leaf directly.

[Copilot]

Pull request overview

Refactors SHAMap bound traversal logic by consolidating the duplicated upperBound/lowerBound implementations into a single parameterized helper, and expands traversal test coverage to include empty and single-item maps.

Changes:

• Introduces SHAMap::boundHelper(id, direction) to unify predecessor/successor search behavior.
• Re-implements upperBound and lowerBound as thin wrappers over boundHelper.
• Adds traversal tests covering empty-map and single-item-map bound behavior.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
src/libxrpl/shamap/SHAMap.cpp	Collapses duplicated bound-walk logic into boundHelper and delegates upperBound/lowerBound to it.
include/xrpl/shamap/SHAMap.h	Declares the new private boundHelper used by bound queries.
src/tests/libxrpl/shamap/SHAMap.cpp	Adds empty/single-item bound tests to cover previously unexercised traversal branches.

Suppressed comments (1)

src/tests/libxrpl/shamap/SHAMap.cpp:451

• This comment claims boundHelper’s leaf branch decides the outcome “before root_'s own inner-node scan would ever run”, but for probes equal to the single key the leaf comparison fails and boundHelper pops the leaf then scans the root (which then falls through to end()). This matters for understanding why upperBound(key)/lowerBound(key) return end().

    // it was constructed with, with the single leaf one level below it. So here the stack holds
    // that inner root plus the leaf, and boundHelper's leaf branch, examined first, decides the
    // outcome before root_'s own inner-node scan would ever run.

---

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