test(rs-platform-wallet/e2e): ID-007 — pin contract that identity-auth addresses are unmonitored#3586
Draft
Claudius-Maginificent wants to merge 34 commits intotest/platform-wallet-e2efrom
Draft
Conversation
…ddresses unmonitored contract pin ID-007 pins the current contract that DIP-9 identity-auth addresses (`m/9'/coinType'/5'/0..3'/identity_index'/key_index'`) are NOT in `PlatformWalletInfo::monitored_addresses()` at the pinned `key-wallet` revision (`fe2476611f`). `WalletAccountCreationOptions::Default` does not create `BlockchainIdentities*` accounts, so a Core (Layer-1) send to one of those addresses is invisible to the SPV bloom filter and never increases the wallet's Core balance. Tracks the scenario from closed PR `dashpay/rust-dashcore#554` (the parked attempt to ship `BlockchainIdentities*` AccountType variants and flip Default to monitor those addresses) and DET follow-up issue `dash-evo-tool#692`. The PR was closed without merge or supersede pointer; investigation confirmed the scenario is silently unhandled when consumed by `rs-platform-wallet`. Status BLOCKED — full test body lands alongside this entry but is gated behind `#[ignore]` until: (1) Task #15 (SPV runtime), (2) Core-funded bank wallet helper (CR-003 prerequisite), (3) the `Bank::send_core_to` stub gets wired to a real Layer-1 broadcast. Defensive-pin precedent: same shape as Found-003 / Found-004 — pin a known-incomplete contract as an asserted invariant so silent drift becomes loud breakage. When upstream `key-wallet` ships any shape of `BlockchainIdentities` support and the wallet opts in, the assertions flip in that same PR. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Claudius the Magnificent <noreply@anthropic.com>
…-auth addresses are unmonitored Implements the full scenario from TEST_SPEC.md § ID-007. The test is `#[ignore]`-gated (does not run in `cargo test --lib` or default `cargo test --tests`), so CI stays green; running it under `--ignored` will fail today on the `unimplemented!()` stub of `BankWallet::send_core_to` until Task #15 + the Core-funded bank helper land. Scenario: 1. Register one identity at slot 0 via `setup_with_n_identities`. 2. Derive the P2PKH `dashcore::Address` for `(identity_index = 0, key_index = 0)` via `derive_ecdsa_identity_auth_keypair_from_master`, plus `(identity_index = 1, key_index = 0)` for the in-test negative variant (registration status is irrelevant — the derivation is pure). 3. Snapshot `monitored_addresses()` before any Core send and assert neither address is in the set. 4. Attempt a 100_000-duff Layer-1 send via `bank().send_core_to(..)` (currently `unimplemented!()`). 5. Snapshot `monitored_addresses()` again and assert the contract still excludes both addresses. 6. `wait_for_core_balance` for 30 s expecting timeout — the SPV bloom filter must not carry these addresses, so the balance never moves. 7. Assert no UTXO matching `(value = 100_000, address = auth_addr_zero)` exists in the wallet's UTXO set. Framework additions: - `framework::wait::wait_for_core_balance(test_wallet, expected_min, timeout)` — Layer-1-balance parallel of `wait_for_balance`. Polls `PlatformWallet::state().balance().spendable()` every backstop interval. Re-exported via `framework::prelude`. - `BankWallet::send_core_to(target, duffs) -> unimplemented!()` — CR-003 prerequisite stub. The bank today holds Platform credits via DIP-17 platform-payment accounts, not Core duffs on a DIP-9 / BIP-44 receive account; wire it through when Task #15 exposes a Core-funded account. The BLS subfeature negative variant is left as a `TODO(ID-007)` comment in the test body — `derive_*_bls_identity_auth_keypair_from_master` doesn't exist in the upstream `key-wallet` API yet. Defensive-pin precedent: Found-003 / Found-004. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Claudius the Magnificent <noreply@anthropic.com>
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
… presets (#3587) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The e2e test framework was constructing a wallet manager without starting the SPV runtime, leaving identity-auth address monitoring (and other Layer-1 dependent assertions) unable to verify their contracts. ID-007 specifically requires SPV to be alive for the monitored_addresses() snapshot to be meaningful. Re-enables `SpvRuntime::start(ClientConfig::testnet())` during `setup()`, with the existing 180s mn-list-sync deadline (which the helper internally raises to the 600s cold-cache floor). SDK keeps `TrustedHttpContextProvider` — proof verification doesn't need the SPV-backed quorum lookup yet; future tests that do can swap to `SpvContextProvider::new(spv_runtime)` via `sdk.set_context_provider` (it's `ArcSwap`-backed, safe to call post-construction). Verified with ID-007 on testnet: SPV mn-list synced from cold cache in ~90s, and the test correctly proceeded through bank L1 funding (`balance reached target observed=130000000`) before the unrelated identity-registration headroom panic in `setup_with_n_identities` (see follow-up: REGISTRATION_HEADROOM=100M < required ~110.86M). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> 🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent
…urrent dynamic fee The setup_with_n_identities helper was sizing each funding address as `funding_per + 100M`, which on the current testnet falls short of the dynamic IdentityCreateFromAddresses fee. Marvin's diagnosis pegs that fee at ~110.86M credits — a ~96M baseline (validate_fees_of_event_v0 PaidFromAddressInputs) plus ~14.85M for the slot-2 TRANSFER key's storage cost. Bumping the headroom to 150M leaves a ~39M buffer for protocol-version drift while staying well clear of bank-budget concerns. Unblocks every helper-using test that was previously panicking at line 75 of id_007 (and friends) on a residual-too-small registration failure. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-Authored-By: Claudius the Magnificent <noreply@anthropic.com>
…e_to Replaces the BankWallet::send_core_to unimplemented! stub with a real implementation that builds, signs, and broadcasts a Layer-1 Core transaction from the bank's BIP-44 account 0 via CoreWallet::send_to_addresses (which delegates to the SpvBroadcaster already wired during framework init). Key details: - Serialises in-process on the existing FUNDING_MUTEX so concurrent Core/Platform funding flows can't race UTXO selection or change- address derivation. - Pre-flight balance check returns FrameworkError::Bank with an operator-actionable "top up at <addr>" pointer when the bank's confirmed Core balance is below `duffs + 10_000` (a generous fee reserve floor that only gates the pre-check; the wallet's coin selector picks the actual fee). - Surfaces two new helpers — `core_balance_confirmed()` and `primary_core_receive_address()` — used by the harness pre-flight log and by ID-007's diagnostic surface. - Harness now logs the bank's Core balance + primary Core receive address once at framework init under the `platform_wallet::e2e::bank` target, so operators can see at a glance whether the bank is Core-funded and where to send testnet duffs if it isn't. Most tests don't need duffs, so a zero balance is not a hard failure — only CR-/ID-007-class cases trip the under-funded path. This unblocks ID-007 at the framework level. End-to-end runs still require the bank's Core address to be funded on testnet (the address prints during init); once funded, the test runs through and pins the current contract that DIP-9 identity-auth addresses are NOT in monitored_addresses(). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-Authored-By: Claudius the Magnificent <noreply@anthropic.com>
Framework prerequisites for ID-007 are now resolved: SPV runtime is live (Task #15) and BankWallet::send_core_to is implemented (CR-003). The test runs end-to-end up to the point where it tries to send Core duffs from the bank — gated only on operator pre-funding the bank's BIP-44 account 0 receive address with at least `CORE_SEND_DUFFS + ~fee` testnet duffs. Updates: - TEST_SPEC.md ID-007 entry: status flipped from BLOCKED to FRAMEWORK-READY with the new operator-funding gate documented. - Test #[ignore] reason: same flip — points operators at the framework init log line where the Core receive address prints. - Module-level doc comment + step 4 inline comment: no longer claims send_core_to is unimplemented; describes the current contract pinning rationale for the negative wait_for_core_balance window. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-Authored-By: Claudius the Magnificent <noreply@anthropic.com>
…t bank Core log
Two extensions to the CR-003 BankWallet::send_core_to landing:
1. Factor the actual Core-broadcast body into a free function
`core_send(wallet, target, duffs)` in `framework/bank.rs`. The bank's
send_core_to is now a thin wrapper that adds the FUNDING_MUTEX guard,
the under-funded pre-check (with the bank's own receive address in
the error), and the broadcast-info log; the free function is what
the upcoming test-wallet Core sweep in cleanup.rs reuses so we have
one Core-broadcast surface across the framework.
2. Promote the bank's Core (Layer-1) status log at framework init to a
prominent, copy-pasteable line:
═══ BANK CORE ADDRESS (fund here for CR-* / ID-007 tests) ═══
Most tests don't need duffs, so a zero balance is not fatal — but
when a CR-/ID-007-class case runs, the operator now has a single
visually-distinct line in the test output naming the address to
top up. Field names switched to `bank_core_addr` / `bank_core_balance`
for parity with the under-funded error message.
CORE_TX_FEE_RESERVE is hoisted from a per-fn const to a `pub const`
on the bank module so the cleanup-side sweep can share the same fee
reserve floor.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claudius the Magnificent <noreply@anthropic.com>
Extends `cleanup::teardown_one` and `cleanup::sweep_orphans` with a real Core (Layer-1) sweep — counterpart to the existing platform- credit / identity-credit sweeps. Recovers Core duffs on every test wallet's BIP-44 account 0 back to the bank's primary BIP-44 receive address. Mechanics: - `sweep_core_addresses` (no longer a no-op stub) reads the wallet's lock-free confirmed Core balance, gates on a `CORE_SWEEP_DUST_FLOOR` of 100_000 duffs (so we never burn most of a balance to fee), and delegates the actual broadcast to `bank::core_send` — the same free function the bank's send_core_to wraps. Failures are logged at WARN and propagated; the orphan-recovery loop catches them and retains the registry entry for next-run retry. - `TestWallet` gains two thin helpers — `core_balance_confirmed()` and `sweep_core_to(target, amount)` — that mirror the bank's surface for individual tests that want to broadcast a Core send without going through teardown. Most tests don't need them; they exist for completeness and for cases where the test body itself needs to trigger the sweep path explicitly. For ID-007 specifically, the test wallet's Core balance stays at 0 under the negative contract (auth_addr is not in the bloom filter), so the sweep is a no-op there — `core sweep: balance at or below dust floor; nothing to sweep`. That's correct behaviour, not a bug. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-Authored-By: Claudius the Magnificent <noreply@anthropic.com>
…helper Lands `setup_with_core_funded_test_wallet(duffs)` next to `setup_with_n_identities` for cases that need an asset-lock-buildable balance on the test wallet's own Core (Layer-1) side. Composes the existing `setup` → `bank.send_core_to` → `wait_for_core_balance` chain into a single guard so CR-003 (and any future Core-funded case) doesn't re-derive the boilerplate. Surfaces `FrameworkError::Bank` verbatim from `BankWallet::send_core_to` so the operator-actionable "top up at <addr>" message reaches the test log unchanged on bank under-funding. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…entity registration
Wires the canonical asset-lock-funded `IdentityCreate` path:
`setup_with_core_funded_test_wallet(200M duffs)` →
`IdentityWallet::register_identity_with_funding_external_signer`
with `IdentityFundingMethod::FundWithWallet { amount_duffs = 100M }`.
The wallet internally drives `AssetLockManager::create_funded_asset_lock_proof`
(build → broadcast → wait IS / fall back to ChainLock) and submits the
`IdentityCreate` transition against the resolved proof.
Asserts:
- on-chain identity is independently fetchable with balance ≥ half lock
amount (deterministic, fee-tolerant lower bound),
- slot-0 key on the fetched identity is MASTER+AUTHENTICATION (the
protocol's signer-key contract for `IdentityCreate`),
- every tracked asset lock landed in `InstantSendLocked` /
`ChainLocked` final state (no `Built` / `Broadcast` orphans).
Tagged `#[ignore]` for testnet env vars + bank Core funding. Mirrors
DET's `test_tc004_create_registration_asset_lock`.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…n present Flips CR-003 from `BLOCKED — needs harness refactor` to `STUB — full test body implemented, #[ignore]-tagged behind testnet env vars + bank Core funding`. Documents the framework prerequisites that landed (SPV runtime, `BankWallet::send_core_to`, `setup_with_core_funded_test_wallet`) and the exact funding floor the operator needs on the bank's Core address before a non-`--ignored` run can clear: `TEST_WALLET_CORE_FUNDING + CORE_TX_FEE_RESERVE` ≈ 2.0001 DASH testnet. Updates the wallet-feature-exercised pointer to the unified `register_identity_with_funding_external_signer` flow. Annotates the Quick Index row to match. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…n API Both `create_wallet_from_mnemonic` and `create_wallet_from_seed_bytes` now take a `birth_height_override: Option<u32>` controlling the SPV compact-filter scan window for the new wallet: - `None` keeps the prior behaviour (seed birth height to SPV's current confirmed header tip — fine for fresh wallets that only need to see funding from now on). - `Some(0)` requests a full historical scan from genesis, required when an address may have received funds before registration. - `Some(h)` pins the scan to a specific block height. The override flows through `register_wallet` into both the in-memory `ManagedWalletInfo` checkpoint and the persisted `WalletMetadataEntry` so the SPV scan window is consistent across restarts. Previously those two carried independent values (in-memory hardcoded to 0, persisted seeded from SPV tip), which was incoherent. FFI bindings and the basic_usage example pass `None` to preserve existing semantics. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…storical L1 funding is visible The bank is a long-lived testnet address that may receive Layer-1 funding before any test run starts. Pinning `birth_height` to the current SPV tip (the previous default) made the compact-filter scan window `[H_now, ∞)`, hiding any UTXO confirmed before init — exactly the QA-001 / QA-002 / QA-003 finding documented in `/tmp/bank-core-balance-diagnosis.md`. The user's confirmed 4 DASH at `yXyzNWRRASxYzWwskmqNmb5xFjGc94bn5F` was being reported as zero for this reason. Pass `Some(0)` to `create_wallet_from_mnemonic` so SPV scans from genesis. Other test callers (`TestWallet::create`, post-sweep re-derivations, cleanup sweeps, the `spv_sync` integration test) still pass `None` — fresh test wallets don't need historical scan. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
QA-003 LOW: the bank pre-flight log already showed `bank_core_addr` and `bank_core_balance`, but not `birth_height` — leaving operators unable to tell "wallet starts above your funding tx" (filter scan window past the funding block) from "your tx hasn't confirmed yet" (legitimate zero balance) when seeing `bank_core_balance=0`. Add `birth_height` to both the info and warn variants of the BANK CORE ADDRESS log line, plus a separate WARN when the balance is zero and birth_height > 0 explaining that any funding tx confirmed below the birth_height is invisible to SPV until re-broadcast. The bank itself now passes `Some(0)`, so the warn is defence-in-depth for the case where someone changes that behaviour without updating the operator-facing diagnostic. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…3591) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…word (#3590) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
#3589) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Helper polls BankWallet::core_balance_confirmed until it reaches the configured floor or the timeout elapses, and emits an info-level progress line every 30s including the SPV compact-filter scan height vs the chain tip — operator can tell "scan still walking" from "scan at tip, balance genuinely zero". Adds Config::bank_core_gate_duffs (env: PLATFORM_WALLET_E2E_BANK_CORE_GATE, default 0 = skip). CR-* / ID-007 cases raise this floor to gate harness init on the bank's pre-funded UTXOs being visible to SPV — Marvin's QA-001: a cold-cache run on testnet samples core_balance ~52s in while SPV is still ~15min from completing the genesis-to-tip filter walk and a CR-003 / ID-007 send_core_to fails on a false-zero balance. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…er scan completion Calls wait_for_bank_funded between BankWallet::load and the bank Core address banner so the banner reflects the post-scan balance instead of a false-zero mid-scan (Marvin's QA-001). Default gate is 0 — most tests don't need duffs and the wait is wasted; CR-* / ID-007 operators raise it via PLATFORM_WALLET_E2E_BANK_CORE_GATE and accept the cold-cache ~15min wait for the first run (subsequent runs reuse the on-disk SPV cache and clear in seconds). Gate failure is demoted to a warn rather than a hard abort so unrelated tests still run; tests that need bank Core funding panic at send_core_to with the operator-actionable "top up at <addr>" message. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… + CR-003 Adds an "Operator notes" line on each entry recording the ~15min cold- cache compact-filter scan, the PLATFORM_WALLET_E2E_BANK_CORE_GATE env var the operator sets to gate harness init on the post-scan balance, and the RUST_LOG target that surfaces scan-progress lines (Marvin's QA-002). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…wait_for_mn_list_synced
Previously wait_for_mn_list_synced only polled the mn-list snapshot and
burned the full 600s cold-cache floor when dash-spv had already given
up internally — most commonly when the QRInfo retry loop hard-caps at
3 attempts with "Required rotated chain lock sig at h - 0 not present".
Two complementary signals now short-circuit the wait:
1. Event-driven: register a single-purpose PlatformEventHandler on the
SpvRuntime's event manager that forwards SyncEvent::ManagerError
(scoped to ManagerIdentifier::Masternode) into an mpsc channel.
The wait loop selects on this channel ahead of the poll tick, so
the engine signal surfaces in O(ms) with an operator-actionable
error message ("wipe spv-data/, or wait 10-20 min for the next
testnet ChainLock cycle").
2. Heuristic backstop: if the engine ever stops emitting the error
(e.g. silent retry loop), the wait still bails after 120s of no
forward progress on the mn-list snapshot.
A thin pass-through accessor SpvRuntime::event_manager() is added so
the framework can subscribe without touching dash-spv internals.
Effect: a known-stalled run that used to wait 600s now bails in well
under 120s — the event path typically in ~1s after dash-spv emits
ManagerError.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add a Known Issues / Operator Notes subsection (1.3) covering the dash-spv QRInfo retry-cap stall that wait_for_mn_list_synced now surfaces eagerly, with operator workaround steps (wait for next testnet ChainLock cycle, or wipe spv-data/). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Pulls in any newer rs-dashcore/dash-spv revisions that may have landed since the branch was forked. Specifically interested in any fix to dash-spv's QRInfo retry path — the testnet rotated-quorum-cycle race currently blocks ID-007/CR-003 testnet runs. 🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent
ID-007 verified PASS on testnet at HEAD 32ee2cd after the dash-spv mn-list QRInfo retry-race window (cycle boundary 1471104, prior runs 1471135) cleared. Mn-list synced in 8.5s warm cache, bank Core funding gate cleared at 1_600_000_000 duffs (gate 110_000), identity EwATqMdBoCrDQoEBTwcammqAcGcKihzxGrW1qaLoDAJW registered, total wall-clock 130s. CR-003 not flipped: re-run failed at cr_003_asset_lock_funded_registration.rs:99 with "PRE-pin violated: setup_with_core_funded_test_wallet returned with confirmed Core balance 0 < TEST_WALLET_CORE_FUNDING 200000000". The helper observed mempool funds (target=200000000 reached at +2.0s) but the case's PRE-pin requires confirmed balance — meaningful contract mismatch, deferred for separate investigation. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…rmed balance Marvin QA-001 HIGH: setup_with_core_funded_test_wallet was returning ~2s after broadcast on mempool-only visibility, then CR-003's PRE-pin panicked because confirmed_core_balance was still 0. The waiter polled state().await.balance().spendable() (mempool-inclusive) while every caller — both the helper's own docstring and CR-003's asset-lock builder — needs *confirmed* UTXOs to reference. Switch wait_for_core_balance to poll TestWallet::core_balance_confirmed (the same lock-free atomic accessor the PRE-pin checks against), drop the stale state().await chain, and rewrite the doc comment to make the confirmed-only contract explicit. ID-007's pre_balance is updated in lockstep so the pin compares against the same metric the waiter reads — the timeout still fires because auth_addr_zero isn't in monitored_addresses(), independent of confirmation depth. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…D-007 heading Marvin QA-002 LOW: TEST_SPEC heading still read "(BLOCKED on Task #15)" even though Task #15 has landed and ID-007 just passed end-to-end on testnet. The Status field on the same entry already records the PASS at HEAD 32ee2cd — the parenthetical is the only remaining stale marker. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…, fail until upstream fixes BlockchainIdentities ID-007 previously pinned the broken contract — green-while-buggy. That inverts the meaning of the test suite: a green run could mean either "feature works" or "feature still broken in the same way as before", and there's no way to tell at a glance. Flip the polarity. The test now asserts the CORRECT behavior: - identity-auth addresses ARE in `monitored_addresses()` before and after the Layer-1 send, - the wallet's confirmed Core balance INCREASES after the inbound UTXO confirms, - the wallet's UTXO set CONTAINS the new entry. All three assertions currently FAIL because rust-dashcore's `WalletAccountCreationOptions::Default` excludes `BlockchainIdentities*` `AccountType` variants at the pinned `key-wallet` revision (closed PR `dashpay/rust-dashcore#554` attempted this; closed without merge). The negative-axis variant (`identity_index = 1`, unregistered slot) carries the same correct-behavior assertions — registration status is irrelevant to monitoring since the derivation is pure. Renames the test file and function from `..._not_monitored` to `..._monitored` to match the inverted intent. Bumps `wait_for_core_balance` timeout to 5 minutes (testnet block time ~2.5 min plus SPV bloom-filter propagation headroom) since the assertion is now "balance reaches target", not "wait times out". The `#[ignore]` reason now spells out "FAILS by design until upstream lands BlockchainIdentities* support". DET parallel: `dash-evo-tool#692`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…-design ID-007's status flipped from "Pass" (which pinned the broken contract as-is) to "FAILING — by design until upstream lands `BlockchainIdentities*` support". The Quick index entry, the per-entry Status, the Assertions block, the Variants section, the Rationale and the Notes are all rewritten to reflect: the test asserts the CORRECT contract; green = feature works, red = feature broken; contrast with `Found-003` / `Found-004` (defensive pins of broken behavior, kept where the bug is the contract). No `red` / `green` legend exists in TEST_SPEC.md to update — status values are free-form English (Pass / IMPLEMENTED / BLOCKED / STUB / FAILING). Quick index has no Status column. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Identity balances are denominated in credits (`dpp::fee::Credits`); the asset-lock amount is in duffs. The previous POST-pin compared the two without conversion, so a successful registration that landed ~99.9M duffs (≈ 99.9G credits, fees subtracted) tripped the upper bound `observed <= ASSET_LOCK_AMOUNT (100M)` by 1000×. Convert via `dpp::balances::credits::CREDITS_PER_DUFF` (= 1000) so both sides of every comparison are in credits, and update the panic message to call out the unit explicitly. Marvin's QA-001 (HIGH). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…satisfied target Marvin's QA-002 (LOW) flagged that the helper returned in 2.002s for CR-003 — well below testnet block time. Investigation against the pinned `key-wallet` rev (`fe2476611fcf72d6f36f1154a39a2f9af3b6a248`) confirmed `WalletCoreBalance::confirmed` counts mature UTXOs that are EITHER in a block OR InstantSend-locked (per upstream rustdoc on `balance.rs:18`). There is no separate accessor that excludes IS-locked UTXOs at this revision; tightening the helper to require strictly block-confirmed semantics would require an upstream API change. Document the actual semantics honestly in the rustdoc (the previous "deliberately NOT counted" claim about IS-locked UTXOs was wrong), and add a `path` field to the success-log line distinguishing `pre_funded_workdir_cache` (target met on first poll → likely a pre-existing UTXO, not freshly arriving funds) from `confirmed_or_is_locked` (at least one poll observed below-target before the threshold was reached). This lets future post-mortems on suspiciously fast returns tell the two paths apart at a glance without inferring it from elapsed timestamps. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ified Asset-lock-funded identity registration runs end-to-end against testnet: asset-lock built, IS-lock observed, identity registered on-chain, balance decrement asserted in duffs (post units fix). Test gated on PLATFORM_WALLET_E2E_BANK_CORE_GATE. Verified at HEAD fa55e64. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
3 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.
Summary
Adds spec entry and a full test body for ID-007 — pins the contract that identity-auth addresses (DIP-9 subfeature 0..3 —
m/9'/coinType'/5'/0..3'/identity_index'/key_index') are NOT inPlatformWalletInfo::monitored_addresses()at the pinnedkey-walletrevision (fe2476611f). The implementation is complete; only the runtime gating (Task #15 + Core-funded bank helper) prevents it from running today.Tracks closed PR
dashpay/rust-dashcore#554(the parked attempt to addBlockchainIdentities*AccountType variants and flipWalletAccountCreationOptions::Defaultto monitor those addresses), and DET follow-up issuedash-evo-tool#692.Why now
The PR was closed without merge or supersede pointer. Investigation confirmed: when the scenario from #554 is consumed by
rs-platform-wallet, it is silently unhandled.derive_ecdsa_identity_auth_keypair_from_masteris a pure function that bypasses the wallet'sAccountCollectionentirely;monitored_addresses()walksaccounts.all_accounts(); identity-auth addresses are silently excluded from the SPV bloom filter. Any future recovery / sweep / DET-style flow that sends Dash to a user's own identity-auth address loses funds (relative to the wallet's view) until manual bootstrap. ID-007 makes that exclusion an asserted contract.Status: BLOCKED P2 — full implementation, runtime-gated
The test body is complete and asserts the current contract end-to-end. It is
#[ignore]-gated socargo test --liband defaultcargo test --testsstay green; it will fail loudly under--ignoreduntil:CR-001/CR-002/CR-003too).BankWallet::send_core_to(..)is currently stubbed withunimplemented!(); wire it through when CR-003's Core-funded account lands.wait_for_core_balance(wallet, expected_min, timeout)inframework/wait.rs— Layer-1-balance parallel ofwait_for_balance.TODO(ID-007)comment in the test body.When the prerequisites land, the assertions stay pinned to the current behaviour ("not monitored, balance unchanged"); when upstream
key-walletthen ships any shape ofBlockchainIdentitiessupport and the wallet opts in, all three assertion clusters flip in that same PR.Framework helpers
wait_for_core_balanceframework/wait.rsPlatformWallet::state().balance().spendable(). Re-exported viaframework::prelude.Bank::send_core_toframework/bank.rsunimplemented!()with a docstring linking to CR-003 / ID-007. Wire when Task #15 lands.Test plan
cases/mod.rsand gated with#[ignore]identity_index = 1, unregistered slot)TODO(ID-007)(helper doesn't exist upstream)cargo fmt --all -- --checkgreencargo check --tests --all-features -p platform-walletgreencargo clippy --tests --all-features -p platform-wallet -- -D warningsgreencargo test --lib -p platform-walletgreen (141 passed, 0 failed)Bank::send_core_tostub with real Core-funded broadcast — deferred to follow-up PR once Task fix(sdk): hash is not a function #15 + CR-003's Core-funded bank landDefensive-pin precedent
Same shape as
Found-003/Found-004— pins a known-incomplete contract as an asserted invariant so silent drift becomes loud breakage.🤖 Co-authored by Claudius the Magnificent AI Agent