Skip to content

Wallet must not spend boxes tracked by external scans only (#1905) - #2451

Open
Ergologica wants to merge 1 commit into
ergoplatform:masterfrom
Ergologica:fix/wallet-external-scan-boxes-1905
Open

Wallet must not spend boxes tracked by external scans only (#1905)#2451
Ergologica wants to merge 1 commit into
ergoplatform:masterfrom
Ergologica:fix/wallet-external-scan-boxes-1905

Conversation

@Ergologica

Copy link
Copy Markdown

Closes #1905

The problem

As reported in #1905, /wallet/payment/send could try to spend boxes belonging to a custom application scan not shared with the wallet, while /wallet/boxes/unspent (with default parameters) does not even return such boxes.

Cause: ErgoWalletState.getBoxesToSpend assembles spendable boxes as registry.walletUnspentBoxes(...) ++ offChainRegistry.offChainBoxes. The confirmed part is correctly limited to the wallet's payments scan, but OffChainRegistry.offChainBoxes holds the off-chain boxes of all scans (it also serves the scan APIs), including boxes tracked only by external application scans. So any unconfirmed box matching a custom scan became spendable by wallet transactions. The same leak affected /wallet/boxes/unspent?considerUnconfirmed=true.

The fix

  • New OffChainRegistry.walletOffChainBoxes: off-chain boxes tracked by the wallet's PaymentsScanId (which includes boxes of scans shared with the wallet, and excludes external-scan-only boxes).
  • ErgoWalletState.getBoxesToSpend and ErgoWalletService.getWalletBoxes (both unspentOnly branches) now use it.
  • Scan-related APIs (getScanUnspentBoxes etc.) keep using the unfiltered offChainBoxes, so external scans still see their unconfirmed boxes.

Testing

New property in ErgoWalletServiceSpec with three off-chain boxes — wallet-only, external-scan-only, and shared (wallet + external scan): getBoxesToSpend and getWalletBoxes(considerUnconfirmed = true) must return the wallet and shared boxes but not the external-scan-only one, while getScanUnspentBoxes for the external scan still returns its two boxes.

The whole ErgoWalletServiceSpec suite passes locally.

@Ergologica

Copy link
Copy Markdown
Author

On the red CI — Run it node tests is red on master as well.

Here it fails on Deep rollback handling. The latest CI run on master (Merge pull request #2415, CI #3602) fails the same job on should Utxo state nodes synchronisation (5 blocks) — a different test in the same node-synchronisation integration suite. 12 passed, 1 failed, in both.

Different test each time, in a suite this branch does not touch: the wallet change here is in ErgoWalletService / WalletRegistry and their unit specs. Rerunning the job should be enough to turn it green, but I cannot trigger one from a fork.

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.

/wallet/payment/send is trying to spend custom scan boxes

1 participant