Skip to content

Apply wallet confirmation filters within valid height ranges - #2536

Open
a-shannon wants to merge 6 commits into
ergoplatform:masterfrom
a-shannon:fix/wallet-confirmation-window
Open

a-shannon wants to merge 6 commits into
ergoplatform:masterfrom
a-shannon:fix/wallet-confirmation-window

Conversation

@a-shannon

@a-shannon a-shannon commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Wallet transaction and scan queries can return transactions below the requested minimum confirmation count because the upper height bound omits the current chain height. The fix derives inclusive bounds with Long arithmetic and skips empty or inverted database ranges.

At height 100, a request for 5 to 10 confirmations now selects heights 90 through 95. Requests beyond the available history return an empty result. The existing confirmation convention, scan selection, descending order and unconfirmed-transaction height gate are preserved.

The current head is 8c91586dbd0d5172b0b3da0e56af471177fe992e. It merges the exact shared test-support commit 5eedfdc91d7a515f66822643e730b4ca9b3197df from #2535, including the complete, non-mining shared seed before rollback miners are isolated.

Independent composition review completed without findings. Local validation was performed in separate runs:

  • All 32 pure integration-support tests passed, and integration compilation passed.
  • The node-fixture run completed 70 tests: 2 funded-generator, 20 candidate-generator, 19 transaction and 29 mempool tests. The subsequent full node-view-holder suite was interrupted by a native LevelDB C++ crash on Windows, so that suite did not complete.
  • Fresh runs passed all five wallet-confirmation properties and all four changed node-view-holder script-failure cases, each with exit code 0. The wallet properties exercise the production actor query and a real registry, covering exact identities, inclusive endpoints, empty windows, unfiltered reads, unconfirmed gating and integer bounds.

Linux CI for the exact current head passed all eight required checks. This revision is ready for maintainer review. The separate passing local runs do not establish that the full native Windows suite is healthy.

The previous CI run at e3489d0cf passed seven of eight checks; integration passed 12 of 13 cases and timed out in DeepRollBackSpec. That is historical evidence, and the shared seed update does not establish the cause of that timeout.

This query correction is independent of the registry-write changes in #2514 and #2516.

Keep synchronization scenarios with their correction in ergoplatform#2511. Reuse the spendable sorting fixture already present in ergoplatform#2480.
Move the exact UtxoStateNodesSyncSpec correction from ergoplatform#2511 into the shared test-support prerequisite. Preserve topology, height and the deadline while observing eventual agreement of selected headers; retained sibling IDs are not selected-chain disagreement.
@a-shannon
a-shannon marked this pull request as ready for review September 10, 2026 05:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant