Skip to content

test(platform-wallet): CR-004 — legacy BIP32 UTXO update after spend (FAILING-by-design)#3613

Merged
lklimek merged 6 commits into
feat/rs-platform-wallet-e2efrom
test/cr-004-legacy-bip32-utxo
May 8, 2026
Merged

test(platform-wallet): CR-004 — legacy BIP32 UTXO update after spend (FAILING-by-design)#3613
lklimek merged 6 commits into
feat/rs-platform-wallet-e2efrom
test/cr-004-legacy-bip32-utxo

Conversation

@Claudius-Maginificent
Copy link
Copy Markdown
Collaborator

Issue being fixed or feature implemented

Reproduces dashpay/dash-evo-tool#845 — after spending from a legacy BIP-32 account, the wallet's UTXO state is not updated, leaving phantom spendable UTXOs that cause subsequent sends to appear to succeed while silently using already-spent inputs.

TEST_SPEC.md entry already merged via #3610. This PR brings only the test.

What was done?

  • Added packages/rs-platform-wallet/tests/e2e/cases/cr_004_legacy_bip32_utxo_update_after_spend.rs — the end-to-end regression test for dash-evo-tool#845.
  • Registered cr_004 in cases/mod.rs.
  • Added parse_truthy helper to framework::config (prerequisite for the env-gate; introduced on the source branch in an out-of-scope commit, brought over as a minimal helper).
  • Test is gated on PLATFORM_WALLET_E2E_RUN_FAILING_BY_DESIGN=1 — without that var, the test body early-returns as a passing no-op, keeping standard --ignored runs clean.
  • FAILING-by-design until upstream dash-evo-tool#845 is resolved.

Note: the cherry-picked commit also adds a TODO(CR-004) comment and tracing::debug! breadcrumb to broadcast.rs — no behavior change, companion annotation only.

How Has This Been Tested?

  • cargo build --tests --no-default-features — PASS
  • cargo build --tests — PASS
  • cargo clippy --tests --all-features -- -D warnings — clean
  • cargo fmt --all -- --check — clean
  • cargo test --test e2e cr_004 -- --list confirms the test is discoverable
  • NOT run against testnet (FAILING-by-design by construction)

Breaking Changes

None.

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have added "!" to the title and described breaking changes in the corresponding section if my code contains any
  • I have made corresponding changes to the documentation if needed (TEST_SPEC already merged via test(platform-wallet): CR-004 spec — legacy BIP32 account UTXO update (dash-evo-tool#845) #3610)

🤖 Co-authored by Claudius the Magnificent AI Agent

lklimek and others added 3 commits May 7, 2026 14:15
…nd (dash-evo-tool#845)

Pin the post-broadcast UTXO-mutation contract on standard_bip32_accounts
end-to-end (TEST_SPEC.md → CR-004) and annotate the production call
site so a regression in the routing layer lands at a named breadcrumb,
not silently in dash-evo-tool's issue tracker.

Test (FAILING-by-design until SPV runtime gates clear and the harness
exposes a stable BIP32 receive-address derivation point):
- Funds the legacy BIP32 account 0 with two distinct UTXOs via the
  existing bank.send_core_to helper (no harness refactor — derives the
  BIP32 receive address inline by mirroring the BIP-44 helper shape
  on standard_bip32_accounts).
- Asserts no cross-account contamination: BIP-44 stays empty, BIP-32
  ends up at exactly 2 spendable UTXOs before the broadcast.
- Sweeps the legacy account via send_to_addresses(BIP32Account, 0, ...).
- POST-pin: standard_bip32_accounts[0].spendable_utxos == empty AND a
  follow-up send fails with TransactionBuild naming an empty input set
  (NOT Ok(...) — the bug surface from dash-evo-tool#845 is exactly the
  Ok-with-phantom-UTXOs case).

Production breadcrumb (no behavior change):
- broadcast.rs:185 (the post-broadcast check_core_transaction(Mempool)
  hook) gains a TODO(CR-004) docstring naming the BIP32-routing
  contract and a tracing::debug! that surfaces the affected
  account_type + tx outpoint counts. The downstream routing in
  key_wallet::TransactionRouter::get_relevant_account_types currently
  DOES include StandardBIP32 for TransactionType::Standard at the
  pinned revision; the breadcrumb makes a future drop-out visible at
  a single grep-able call site.

Verification:
- cargo check -p platform-wallet --tests           ok
- cargo clippy -p platform-wallet --tests
       --all-features -- -D warnings               clean
- cargo fmt --all -- --check                       clean
- cargo test -p platform-wallet --lib              149 passed

Refs: dashpay/dash-evo-tool#845, TEST_SPEC.md → CR-004

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…GN env-var guard for CR-004 (QA-V18-003)

`#[ignore]` is bypassed by `cargo test -- --ignored`, which runs
every ignored case — so CR-004's FAILING-by-design body still
panicked and polluted the standard ignored-cohort run. Add a
runtime opt-in guard at the top of the test body: unless
`PLATFORM_WALLET_E2E_RUN_FAILING_BY_DESIGN` is truthy
(`1`/`true`/`yes`/`on`, case-insensitive) the test early-returns
as a passing no-op with an `eprintln!` breadcrumb. Operators who
want to exercise the pinned regression set the var explicitly.

Also declare the new env-var key in `framework::config::vars` next
to `DISABLE_SPV` so the cohort of operator-facing knobs stays in
one place; the existing `parse_truthy` helper handles the truthy
parse so no new logic is needed.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… env-gate

`fafa5b4fa9` references `crate::framework::config::parse_truthy` which
was introduced in `d1d81a3294` (DISABLE_SPV feature) on the source
branch but is absent from `feat/rs-platform-wallet-e2e` at the base
commit. Add just the helper function here so the env-gate compiles
without pulling in the full DISABLE_SPV harness changes.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 7, 2026

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: c8811e1b-e7e9-41be-b01a-e59e6f116757

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch test/cr-004-legacy-bip32-utxo

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

lklimek and others added 3 commits May 8, 2026 15:06
…th (#3613 review)

Trim the 12-line TODO(CR-004 / dash-evo-tool#845) commentary on
`send_to_addresses` post-broadcast hook to a 3-line marker pointing
at the e2e test name. Drop the 9-line `tracing::debug!` call that
fired on every standard-tx broadcast — it logged an invariant the
CR-004 test asserts via state inspection, not via log scraping, and
hot-path logging of static invariants violates the project logging
policy (skill: coding-best-practices § Logging Levels).

Addresses F-001 (MEDIUM) and F-002 (MEDIUM) in audit report.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…3613 review)

Trim the 33-line module docstring to a 10-line present-state summary;
the call-site walkthrough and "Why FAILING-by-design and not WIRED"
framing belong in the PR description / git history, not in source.
Collapse multi-paragraph assertion messages to single-line essence
naming the contract violated. Drop dangling references to "module
docstring point 3" now that the numbered list is gone.

Addresses F-003, F-004, F-005 (LOW) in audit report.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…o test/cr-004-legacy-bip32-utxo

# Conflicts:
#	packages/rs-platform-wallet/tests/e2e/framework/config.rs
@lklimek lklimek marked this pull request as ready for review May 8, 2026 13:47
@lklimek lklimek requested a review from QuantumExplorer as a code owner May 8, 2026 13:47
@lklimek lklimek merged commit e7adc2f into feat/rs-platform-wallet-e2e May 8, 2026
0 of 3 checks passed
@lklimek lklimek deleted the test/cr-004-legacy-bip32-utxo branch May 8, 2026 13:47
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.

2 participants