Read only the requested inclusion height range from the DB in the wallet box endpoints

[Ergologica]

Closes #1870.

What the issue asks

Consider slicing by height in /boxes/unspent. Add heights range in unspentBoxes arguments in WalletRegistry, and so extract only boxes in given range from DB, like done for /wallet/transactions.

@pragmaxim asked on the issue: by inclusionHeight or spendingHeight? — by inclusionHeight:

• the route parameters are already named minInclusionHeight / maxInclusionHeight;
• the in-memory filter these endpoints apply today (WalletApiOperations.boxConfirmationHeightFilter) reads trackedBox.inclusionHeightOpt;
• the index that already exists in WalletRegistry (InclusionHeightScanBoxPrefix) is keyed by inclusion height;
• for unspent boxes a spending height does not exist by definition.

The problem

minInclusionHeight / maxInclusionHeight are accepted and documented, but they are applied in memory, after the boxes have been read, and getWalletBoxes reads the unspent index with a limit:

val confirmed = state.registry.walletUnspentBoxes(state.maxInputsToUse * BoxSelector.ScanDepthFactor)

That index (UnspentIndexPrefix) is keyed by box id, not by height, so the limit (100 x 300 = 30000 by default) takes an arbitrary subset. For a wallet holding more unspent boxes than that, a query for a height window can return an arbitrary subset of it, or nothing at all, with no indication that anything was dropped — and offset/limit paging on top is unstable for the same reason. For unspentOnly = false there is no limit, but the whole box space is read and then thrown away.

Meanwhile the DB-level primitive already exists and is used only by the scan endpoints: WalletRegistry.unspentBoxesByInclusionHeight, backed by the inclusion-height index and already exercised by /scan/unspentBoxes. This PR wires the wallet path to it.

The change

• WalletRegistry: two thin helpers over the existing height index for the payments scan — walletUnspentBoxesByInclusionHeight and walletBoxesByInclusionHeight.
• GetWalletBoxes, ErgoWalletReader.walletBoxes, ErgoWalletService.getWalletBoxes: carry minHeight / maxHeight, defaulting to 0 / -1 so every existing caller is unaffected.
• ErgoWalletServiceImpl.getWalletBoxes: when a height range is requested, read it from the inclusion-height index; otherwise keep reading the unspent index exactly as before.

val heightRangeRequested = minHeight > 0 || maxHeight >= 0

The conditional is deliberate: reading the height index unconditionally would drop the maxInputsToUse * ScanDepthFactor cap for the common unbounded query and could load the whole box space of a large wallet. This way the new path is never worse than the current one.

-1 as "unbounded" is the convention already used by /scan/unspentBoxes, and it works directly against the key encoding (putInt(-1) is 0xFFFFFFFF, the largest 4-byte prefix). There is now a test pinning that.

The in-memory boxConfirmationHeightFilter in the route is kept: it also handles minConfirmations / maxConfirmations, and it preserves the current treatment of off-chain boxes, which have no inclusion height. So this PR changes which boxes are read from the database, not which boxes are returned — except that boxes previously lost to the limit are now actually returned.

/wallet/boxes gets the same treatment as /wallet/boxes/unspent: it is the same function, and the endpoint takes the same two parameters.

Tests

• WalletRegistrySpec: new should get wallet boxes by inclusion height — spent/unspent split, inclusive bounds on both ends, empty window, lower bound only, and -1 as upper bound returning exactly what the unspent index returns.
• ErgoWalletServiceSpec: new it should read wallet boxes within an inclusion height range only — two disjoint height windows over distinct boxes, checking neither leaks into the other, that spent boxes stay excluded when unspentOnly is set, that an empty window yields nothing, and that maxHeight = -1 keeps the previous behaviour.
• Stubs: the wallet actor stub now applies the height range, so the route specs cover the parameters being threaded all the way to the actor message.

sbt "testOnly org.ergoplatform.http.routes.* org.ergoplatform.nodeView.wallet.*" — 210 tests, all green.

[Ergologica]

Flagging something a reviewer should not have to find out the hard way: #2284 already exists for #1870, opened in December, and I did not see it before opening this. Here these two do overlap — same approach, same call chain.

What is the same: threading minHeight/maxHeight from WalletApiRoute through GetWalletBoxes → ErgoWalletService.getWalletBoxes → a new WalletRegistry.walletUnspentBoxesByInclusionHeight. Same idea, arrived at independently, and theirs was first.

Where this one goes further:

• Tests. Fix issue #1870: Add height range filtering to /boxes/unspent endpoint #2284 adds none. This adds coverage in WalletRegistrySpec and ErgoWalletServiceSpec, including the case that bit me while writing it: TrackedBox.copy does not change box.id, so a naive test reuses the same box ids across both height windows and passes for the wrong reason.
• The spent path too. walletBoxesByInclusionHeight, so /wallet/boxes gets the same treatment as /wallet/boxes/unspent, not just the unspent one.
• A guard on the default path, so a request with no range asked for keeps the current behaviour exactly rather than silently going through the range branch.

And the reason I think this is worth more than an API convenience: UnspentIndexPrefix is keyed by box id, not by height. So the existing code reading maxInputsToUse * ScanDepthFactor (30000) entries is not reading the newest 30000 boxes — it is reading an arbitrary 30000 in key order, and silently returning a subset once a wallet exceeds that. Filtering by height after the read cannot fix that; the range has to reach the index. There is a correctness bug under the performance one, and it is worth a maintainer looking at that specifically whichever PR ends up being taken.

I am not asking for #2284 to be closed, and the choice between them is not mine. @glasgowm148 for the registry side — reservation is ErgoDevs/Ergo-Bounties#57, and their earlier submission #35 was closed.

[Ergologica]

One more thing a reviewer will hit before they read a line of this: #1870 already has a merged PR that says closes #1870. I would rather explain that than have it close this PR.

#1872, "i1870 filter /scan/unspent boxes by inclusion height efficiently", merged 2022-11-03. Its author flagged it himself in the body:

@kushti I need more time for this one, the KV store where the keys are composed of multiple "navigation" byte sequences are very hard to reason about.

It did the /scan/unspent half and stopped. Four years on the issue is still open, and this is what is still there on master:

// ScanApiRoute.scala:71 - the scan endpoint, done by #1872
withWallet(_.scanUnspentBoxes(scanId, considerUnconfirmed, minHeight, maxHeight)

// WalletApiRoute.scala:307 - the endpoint #1870 actually names, untouched
wallet.walletBoxes(unspentOnly = true, considerUnconfirmed)
  .map { boxes => boxes.filter(boxConfirmationHeightFilter(_, minConfNum, maxConfNum, minHeight, maxHeight)) }

The scan route gets a database range. /wallet/boxes/unspent reads first and filters in memory afterwards. That asymmetry is the remaining half of #1870, and it is what this PR closes — walletUnspentBoxesByInclusionHeight and walletBoxesByInclusionHeight are just unspentBoxesByInclusionHeight(Constants.PaymentsScanId, …), reusing the index #1872 built rather than adding a new one.

Why the in-memory filter is not merely slower

I said above that UnspentIndexPrefix is keyed by box id. Confirming it at the key-encoding level, since it is the load-bearing claim:

// WalletRegistry.scala:587 - how an unspent-index key is built
composeKeyWithId(prefix, scanId, trackedBox.box.id)

// :542-546 - the range walletUnspentBoxes scans
composeKey(UnspentIndexPrefix, scanId, 0) .. composeKey(UnspentIndexPrefix, scanId, -1)

// :555-558 - the height index, which exists and the wallet path does not use
composeKey(InclusionHeightScanBoxPrefix, scanId, height)

So walletUnspentBoxes(maxInputsToUse * ScanDepthFactor) = 30 000 entries in box-id order. Box ids are hashes, so that is an arbitrary 30 000, not the newest 30 000. Once a wallet holds more than that, the endpoint silently returns a subset — and no height filter applied afterwards can recover the rows that were never read. The range has to reach the index, which is what this changes.

That is a correctness bug sitting under the performance one, and worth a maintainer's eye independently of which PR you take for #1870.