test(wallet): add cross-wallet top-up isolation e2e coverage - #956
Draft
Claudius-Maginificent wants to merge 1 commit into
Draft
test(wallet): add cross-wallet top-up isolation e2e coverage#956Claudius-Maginificent wants to merge 1 commit into
Claudius-Maginificent wants to merge 1 commit into
Conversation
Covers the behaviour fixed by #954 (`resolve_top_up_route` in `src/backend_task/identity/top_up_identity.rs`) end-to-end against Dash testnet. Offline tests in `src/context/wallet_lifecycle/tests.rs` already cover routing and the corruption regression at the unit level; this closes the live-network gap — a real asset lock, broadcast, IS proof, `TopUpIdentity` acceptance on Platform, and the paying wallet's persister state afterward. Four assertions: 1. A foreign top-up lands on Platform, confirmed independently via `RefreshIdentity` rather than the task's own return value. 2. The payer wallet is left uncorrupted — no orphaned `identity_keys` rows in the live persister, and both wallets survive a real `ensure_wallets_registered` reload (the "Saved wallet data appears damaged" failure mode). 3. Resuming the now-spent unbound asset lock via `UseAssetLock` is refused, so the lock lingering in a non-`Consumed` state cannot become a double-credit. 4. Same-wallet (owned-identity) top-up still works. The test file was written and live-verified during #954's development (104s testnet run, all four assertions passed) but was dropped by that PR's squash merge; this restores it. Marked `#[ignore]` per `tests/backend-e2e/README.md` — it needs network access and a funded testnet wallet, and is not run by CI. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WcX9SNDB26o5TVoUhUQkEN
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TL;DR: Adds a live-testnet end-to-end test proving cross-wallet identity top-up no longer corrupts the paying wallet — regression coverage for a fix (#954) that merged without its test.
User story
As a developer maintaining Dash Evo Tool's wallet top-up flow, I want an end-to-end regression test for cross-wallet top-up isolation, so a future change can't silently reintroduce the corruption #954 fixed.
Scenario
Base flow
One wallet (the payer) funds another wallet's identity top-up using an asset lock.
Actual behavior
#954 fixed cross-wallet top-up corrupting the payer wallet's own state, and was developed with a matching end-to-end test — but the PR landed as a squash merge that didn't include the test file, so the fix shipped with no regression coverage on
v1.0-dev.Expected behavior
A backend-e2e test (
cross_wallet_topup_e2e) exercises the real flow against Dash testnet and asserts: the payer wallet's own identity/UTXO state stays intact, both wallets reload cleanly afterward, resuming a spent unbound asset-lock is correctly refused, and the existing owned-identity top-up path still works.Detailed discussion
What was done
Recovered
tests/backend-e2e/cross_wallet_topup.rsand itstests/backend-e2e/main.rsmodule registration from the branch that originally accompanied #954's fix, rebased cleanly onto currentv1.0-dev, and cleaned up references that had gone stale in the rebase (a commit hash squashed away by the merge, an outdated file path, internal tracking IDs replaced with plain-language rationale).Testing
Run live against Dash testnet earlier in development (~104s, all 4 assertions passed). This PR's rebased version compiles clean (
cargo test --test backend-e2e --all-features --no-run, exit 0). The test is#[ignore]-gated and network-dependent per this repo's backend-e2e conventions, so it's not part of CI — seetests/backend-e2e/README.mdfor how to run it live before merge if a reviewer wants to re-confirm.Breaking changes
None (test-only addition).
Checklist
--no-runcheck)tests/backend-e2e/README.mdconventionscargo fmtcleanPrior work
Fixed by #954 — this PR only adds the test coverage that PR's squash merge dropped.
Attribution
🤖 Co-authored by Claudius the Magnificent AI Agent