Skip to content

Commit 5c6baab

Browse files
Claudius-Maginificentlklimekclaude
authored
test(platform-wallet): QA-V6→V26 fixes + parallelism contract (#3609)
Co-authored-by: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent ce94389 commit 5c6baab

52 files changed

Lines changed: 4731 additions & 698 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

packages/rs-platform-wallet/Cargo.toml

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,8 +90,12 @@ tokio-util = { version = "0.7", features = ["rt"] }
9090
# and `framework/context_provider.rs` and is currently disabled
9191
# (see harness.rs) — re-enable when SPV cold-start is stable
9292
# (Task #15).
93-
rs-sdk-trusted-context-provider = { path = "../rs-sdk-trusted-context-provider" }
94-
93+
rs-sdk-trusted-context-provider = { path = "../rs-sdk-trusted-context-provider", features = ["dpns-contract"] }
94+
# In-memory test runs (NoPlatformPersistence) need finalized txs retained in RAM.
95+
# Re-declaring here enables the feature for the test target only; production
96+
# builds pay no memory overhead. Per upstream rust-dashcore maintainer guidance.
97+
key-wallet = { workspace = true, features = ["keep-finalized-transactions"] }
98+
key-wallet-manager = { workspace = true, features = ["keep-finalized-transactions"] }
9599

96100
[features]
97101
default = ["bls", "eddsa"]

packages/rs-platform-wallet/src/error.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,20 @@ pub enum PlatformWalletError {
7474
#[error("Address operation failed: {0}")]
7575
AddressOperation(String),
7676

77+
#[error(
78+
"gap-limit exceeded: requested {requested} fresh unused addresses but only \
79+
{available} are derivable past the current gap-limit boundary \
80+
(highest_used={highest_used:?}, highest_generated={highest_generated:?}, \
81+
gap_limit={gap_limit})"
82+
)]
83+
GapLimitExceeded {
84+
requested: usize,
85+
available: u32,
86+
highest_used: Option<u32>,
87+
highest_generated: Option<u32>,
88+
gap_limit: u32,
89+
},
90+
7791
#[error("{}", format_no_selectable_inputs(funded_outputs, *sub_min_count, *sub_min_aggregate, *min_input_amount))]
7892
NoSelectableInputs {
7993
/// Funded addresses dropped by the input-equals-output filter.

packages/rs-platform-wallet/src/wallet/core/broadcast.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,16 @@ impl<B: TransactionBroadcaster + ?Sized> CoreWallet<B> {
194194
"Wallet not found in wallet manager".to_string(),
195195
)
196196
})?;
197+
tracing::debug!(
198+
target: "platform_wallet::core::broadcast",
199+
txid = %tx.txid(),
200+
account_type = ?account_type,
201+
account_index,
202+
inputs = tx.input.len(),
203+
outputs = tx.output.len(),
204+
"post-broadcast: dispatching check_core_transaction(Mempool) — \
205+
must mark consumed UTXOs spent on the matching account collection"
206+
);
197207
info.check_core_transaction(&tx, TransactionContext::Mempool, wallet, true, true)
198208
.await;
199209
}

packages/rs-platform-wallet/tests/e2e.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
//! harness; `cases/` hosts `#[tokio_shared_rt::test(shared)]` entries.
66
77
#![allow(dead_code, unused_imports)]
8+
#![allow(clippy::result_large_err)]
89

910
// `tests/e2e.rs` is the integration-test crate root; explicit
1011
// `#[path]` keeps the on-disk layout grouped under `tests/e2e/`.

packages/rs-platform-wallet/tests/e2e/README.md

Lines changed: 32 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -164,16 +164,42 @@ Tracing output (SPV sync events, balance polls, sweep results) is written to std
164164

165165
---
166166

167-
## Multi-process safety
167+
## Parallelism
168168

169-
Multiple `cargo test` invocations running concurrently — for example, parallel CI jobs
170-
on different branches — must not share the same bank wallet or working directory, or
171-
they will conflict on nonces.
169+
The harness supports running cases in parallel within a single `cargo test`
170+
invocation (`--test-threads=N`, N > 1) AND across multiple concurrent invocations
171+
on the same machine.
172172

173-
The framework handles this at two levels:
173+
### In-process (`--test-threads=N`)
174+
175+
All tests share one `E2eContext` (singleton via `tokio::sync::OnceCell`), one bank
176+
wallet, one SPV runtime, and one workdir slot. Per-test isolation comes from:
177+
178+
- **Fresh per-test wallets** — every `setup()` mints a fresh OS-random 64-byte seed,
179+
so two parallel tests have disjoint wallet ids, addresses, identities, and nonces.
180+
- **Serialised bank funding**`bank.fund_address` and `bank.send_core_to` lock a
181+
process-global `FUNDING_MUTEX` so concurrent callers don't race UTXO selection or
182+
nonce assignment. Tests waiting on `wait_for_balance` do NOT hold the mutex —
183+
bank serialisation only covers the actual broadcast critical section.
184+
- **Compile-time `Send + Sync`**`E2eContext` and `SetupGuard` are statically
185+
asserted thread-safe (`framework/mod.rs`). A future field addition that breaks
186+
thread-safety fails to compile.
187+
188+
Two cases need a note under parallel execution:
189+
190+
- **PA-008c** observes the process-global `FUNDING_MUTEX_HISTORY` ring buffer to
191+
prove the mutex serialises. Asserts a lower bound on entry count (`>= 3`) and
192+
the pairwise non-overlap property — both hold regardless of sibling traffic.
193+
- **PA-010** is `#[ignore]`'d pending a per-test bank instance API; bank is
194+
process-shared by design.
195+
196+
### Cross-process (concurrent `cargo test` invocations)
197+
198+
Multiple `cargo test` invocations on the same machine — for example, parallel CI
199+
jobs or developer worktrees — must NOT share the same bank wallet or workdir slot.
174200

175201
**Workdir slots** — each process tries to acquire an exclusive `flock` on the base
176-
working directory. If that lock is already held it tries up to 10 numbered slot
202+
working directory. If that lock is already held it walks up to 10 numbered slot
177203
directories (`<workdir>-1`, `<workdir>-2`, ...). A slot holds the SPV block cache,
178204
the SDK config, and the test-wallet registry independently from every other slot.
179205

packages/rs-platform-wallet/tests/e2e/TEST_SPEC.md

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2311,4 +2311,111 @@ Each question's answer changes the spec; numbered for reference.
23112311

23122312
---
23132313

2314+
## 7. Known Issues
2315+
2316+
Tracked production bugs and harness gaps that affect test outcomes. Tests are
2317+
`#[ignore]`d in these cases — but **`#[ignore]` does NOT mean "never runs"**:
2318+
2319+
- `cargo test` (default): ignored tests are **skipped**.
2320+
- `cargo test -- --ignored`: runs **only** ignored tests. PA-004b, PA-009, and PA-010 execute under this flag and fail by design. Any failure mode other than the one documented per-entry below is a regression.
2321+
2322+
Do not modify production code in this section — these are documentation entries only.
2323+
2324+
### V27-007 — `PlatformAddressWallet::transfer` ledger pollution (production bug)
2325+
2326+
**Status**: tracked, fix deferred. Tests `pa_004b_sweep_below_dust_gate_no_broadcast`
2327+
and `pa_009_cleanup_gate_tracks_platform_version_min_input_amount` are `#[ignore]`'d
2328+
with reason `"FAILING — production bug in PlatformAddressWallet::transfer pollutes local ledger with non-owned addresses. See TEST_SPEC.md (V27-007) and TODO comment below."` — they run under `cargo test -- --ignored` and fail by design until the production fix lands.
2329+
2330+
**Expected failure mode** (PA-004b and PA-009): the `assert_eq!(addr_1_residual, TARGET_RESIDUAL, ...)` assertion panics because `total_credits()` returns the bank's full balance (~40.8 tDASH) instead of the wallet's actual residual (`TARGET_RESIDUAL = 1_000`). Any failure at a different assertion or with a different value is a regression.
2331+
2332+
**PA-010 — harness gap** (`pa_010_bank_starvation_typed_error`): this test is also `#[ignore]`'d (`"BLOCKED — needs harness refactor: per-test bank instance (Bank::with_test_balance) OR injectable balance override on the singleton, plus a typed BankError::Underfunded variant. See spec status."`) and fails under `cargo test -- --ignored` by design — it always panics with:
2333+
2334+
```
2335+
PA-010 is BLOCKED on a harness refactor. The bank is a process-shared singleton (E2eContext.bank, OnceCell-backed); building a `with_test_balance(5_000_000)` underfunded instance for ONE test conflicts with that lifecycle. The current under-funded fail mode is also a generic AddressOperation error, not a typed BankError::Underfunded. See TEST_SPEC.md → PA-010 → **Status**.
2336+
```
2337+
2338+
This is a harness gap (not a production bug); fix path is tracked in the harness roadmap (Wave 4 / `Bank::with_test_balance` constructor). Any panic message other than the one above, or a failure that propagates past the `panic!` call, is a regression.
2339+
2340+
**Bug**: `PlatformAddressWallet::transfer` at
2341+
`packages/rs-platform-wallet/src/wallet/platform_addresses/transfer.rs:160` calls
2342+
`account.set_address_credit_balance(p2pkh, funds.balance, key_source.as_ref())`
2343+
for every address in the transition (inputs ∪ outputs), with no ownership check.
2344+
When a wallet transfers to an externally-owned address (e.g., bank's primary
2345+
receive address), the externally-owned post-balance gets staged into the source
2346+
wallet's local `address_balances` ledger.
2347+
2348+
**Symptom**: `wallet.total_credits()` after a transfer-to-external returns the
2349+
external address's balance summed in. PA-004b/PA-009 see the bank's full
2350+
~40.8 tDASH on what should be a dust-residual wallet → assertions panic.
2351+
2352+
**Same unguarded primitive** also exists at:
2353+
- `packages/rs-platform-wallet/src/wallet/platform_addresses/withdrawal.rs:141`
2354+
- `packages/rs-platform-wallet/src/wallet/platform_addresses/fund_from_asset_lock.rs:129`
2355+
2356+
Currently safe by caller behavior (those iterate only-owned addresses), but
2357+
identical shape; defense-in-depth fix should apply there too.
2358+
2359+
**Severity**:
2360+
- **Tests**: HIGH — every `total_credits()` post-transfer-to-external is a false read.
2361+
- **SDK consumers**: HIGH — anyone following `transfer → read total_credits` sees
2362+
inflated balances and could make wrong spend decisions.
2363+
- **Production sweep path**: MEDIUM-LOW — sweep would build inputs against the
2364+
external address, but the source wallet can't sign for it; Drive rejects the
2365+
transition; error swallowed → no on-chain leak.
2366+
2367+
**Fix sketch** (~6 LOC, do not apply in this PR):
2368+
Filter the loop in `transfer.rs:145-160` so `set_address_credit_balance` is
2369+
called only for addresses the source account owns:
2370+
2371+
```rust
2372+
for (addr, maybe_info) in address_infos.iter() {
2373+
let PlatformAddress::P2pkh(hash) = addr else { continue };
2374+
let p2pkh = PlatformP2PKHAddress::new(*hash);
2375+
// Skip addresses the source account doesn't own; address_infos covers
2376+
// inputs ∪ outputs and outputs we don't own must not pollute the local
2377+
// credit ledger.
2378+
if !account.address_balances.contains_key(&p2pkh)
2379+
&& account.addresses.address_info_by_p2pkh(&p2pkh).is_none()
2380+
{
2381+
continue;
2382+
}
2383+
// ... existing set_address_credit_balance + changeset push
2384+
}
2385+
```
2386+
2387+
Defense-in-depth: apply same filter at `withdrawal.rs:141` and
2388+
`fund_from_asset_lock.rs:129`. Optionally make `set_address_credit_balance`
2389+
itself reject addresses not in the pool (wider change in `key-wallet`).
2390+
2391+
**Confirmation audit**:
2392+
- Search for any aggregate that sums `total_credits()` across multiple wallets in the manager (production code, dashboards, telemetry) — would double-count.
2393+
- Run e2e suite with the fix in place, verify PA-004b/PA-009 pass.
2394+
- Add debug assertion in `set_address_credit_balance` that the address is in the pool — every callsite that violates would surface.
2395+
2396+
**Investigated**: Bilby read-only audit, 2026-05-08, agent ID `a2d81349f872a0c6a`.
2397+
2398+
---
2399+
2400+
### V28-303 — PA-003 partial fix: deficit closed, contention timeout remains
2401+
2402+
**Status**: partial. PA-003 (`pa_003_fee_scaling`) is NOT `#[ignore]`'d — it runs in the default `cargo test` cohort. However, it is not reliably green under concurrency.
2403+
2404+
**What V28-303 did**: bumped `FUNDING_CREDITS` from 400M to 500M and `FUNDING_FLOOR` from 350M to 450M (`cases/pa_003_fee_scaling.rs`). This closed the "available 240,524,980 credits, required 250,000,000" deficit that caused a deterministic failure on the 5-output transfer leg: with 400M pre-fund, `addr_src` retained only ~200M after the 1-out transfer and five marker transfers, giving ~235M of reachable candidate balance against a 250M requirement. With 500M pre-fund, `addr_src` retains ≥300M post-setup and the auto-selector has comfortable headroom.
2405+
2406+
**What V28-303 did NOT fix**: at `threads=8` (standard CI concurrency), the `wait_for_balance` call on funding confirmation hits the 60s deadline before the balance settles. Current observed failure mode:
2407+
2408+
```
2409+
wait_for_balance timed out after 60s — addr_src balance never reached FUNDING_FLOOR (450_000_000)
2410+
```
2411+
2412+
This is a contention symptom: eight concurrent tests competing for DAPI bandwidth and bank-wallet nonce slots delay the funding broadcast confirmation beyond the per-step `STEP_TIMEOUT = Duration::from_secs(60)`.
2413+
2414+
**Claiming "V28-303 fixes PA-003" or "PA-003 first time passing" is wrong.** V28-303 narrows the failure surface (one deterministic failure mode removed) but does not green-light PA-003 in standard CI.
2415+
2416+
**Real fix path**: QA-V28-403 — raise `STEP_TIMEOUT` per step (or use a dynamic deadline tied to observed DAPI latency under load). Until that lands, PA-003 may pass in low-concurrency or low-load runs and fail under the standard 8-thread CI tier.
2417+
2418+
---
2419+
2420+
23142421
<sub>Catalogued by Marvin (QA), with the resigned competence of someone who has read every line of this code twice. Edge-case expansion by Trillian, who knows that the difference between "tested" and "tested at the boundary" is the difference between "ships" and "ships back".</sub>

packages/rs-platform-wallet/tests/e2e/cases/cr_003_asset_lock_funded_registration.rs

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ use platform_wallet::wallet::identity::types::funding::IdentityFundingMethod;
4141

4242
use crate::framework::prelude::*;
4343
use crate::framework::signer::{derive_identity_key, SeedBackedIdentitySigner};
44-
use crate::framework::wait::wait_for_identity_balance;
44+
use crate::framework::wait::{wait_for_identity_balance, wait_for_identity_visible_to_platform};
4545

4646
/// DIP-9 identity index used for the asset-lock registration. Slot 0
4747
/// is canonical for "first identity on this wallet" — same convention
@@ -217,9 +217,22 @@ async fn cr_003_asset_lock_funded_registration() {
217217
asset-lock output value (fees are subtracted, not added)."
218218
);
219219

220-
// Step 5: round-trip the identity via the SDK to assert the
221-
// returned shape matches the on-chain shape — same MASTER key id,
222-
// same balance, same revision = 0 baseline.
220+
// Step 5: wait for the identity to be visible across enough DAPI
221+
// replicas before the round-trip fetch. The asset-lock-funded path
222+
// has different proof convergence than the address-funded path —
223+
// `wait_for_identity_balance` above confirms credits landed, but
224+
// a subsequent `Identity::fetch` on a still-lagging replica returns
225+
// `Ok(None)`. Two consecutive successes bias toward distinct nodes
226+
// having replicated the identity (QA-911).
227+
wait_for_identity_visible_to_platform(
228+
s.test_wallet.platform_wallet().sdk(),
229+
identity_id,
230+
IDENTITY_VISIBILITY_TIMEOUT,
231+
2,
232+
)
233+
.await
234+
.expect("identity propagation gate cleared before round-trip fetch (QA-911)");
235+
223236
let fetched = Identity::fetch(s.test_wallet.platform_wallet().sdk(), identity_id)
224237
.await
225238
.expect("Identity::fetch round-trip after registration")

packages/rs-platform-wallet/tests/e2e/cases/dpns_001_register_name.rs

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -24,19 +24,33 @@ use rand::RngCore;
2424
use crate::framework::prelude::*;
2525
use crate::framework::wait::wait_for_dpns_name_visible;
2626

27-
/// Bank → funding-address gross. Sized to cover the registration
28-
/// transition (`REGISTRATION_FUNDING`) plus the chain-time
29-
/// `IdentityCreateFromAddresses` dynamic fee paid from the address
30-
/// residual (~96M observed at ID-001 calibration), with comfortable
31-
/// headroom for DPNS-register-side fees that come out of the
32-
/// identity's credit balance afterwards.
33-
const FUNDING_CREDITS: u64 = 200_000_000;
34-
3527
/// Pre-fee credits committed to the new identity by
3628
/// `IdentityCreateFromAddresses`. The identity arrives on chain with
3729
/// exactly this balance — DPNS register fees draw against it.
3830
const REGISTRATION_FUNDING: u64 = 130_000_000;
3931

32+
/// Headroom carried on the funding address residual so the chain-time
33+
/// `IdentityCreateFromAddresses` dynamic fee (~110.86M observed on
34+
/// testnet — `validate_fees_of_event_v0 PaidFromAddressInputs`
35+
/// baseline plus the slot-2 TRANSFER key's storage cost) clears with
36+
/// buffer for protocol-version drift. Mirrors the
37+
/// `setup_with_n_identities` `REGISTRATION_HEADROOM` constant in
38+
/// `framework/mod.rs` — the residual must absorb the dynamic fee
39+
/// after registration consumes `REGISTRATION_FUNDING`, otherwise the
40+
/// chain returns
41+
/// `AddressesNotEnoughFundsError(required=110_862_220)` (QA-701-B).
42+
const REGISTRATION_HEADROOM: u64 = 150_000_000;
43+
44+
/// Bank → funding-address gross. Funds the registration transition
45+
/// (`REGISTRATION_FUNDING`) plus the dynamic-fee residual headroom
46+
/// (`REGISTRATION_HEADROOM`). Earlier sizings (~200M) left only ~70M
47+
/// after the registration consumed `REGISTRATION_FUNDING`, which fell
48+
/// short of the ~110.86M dynamic fee — DPNS-001 then panicked with
49+
/// "Insufficient combined address balances: total available is less
50+
/// than required 110862220". Reuses the same arithmetic as
51+
/// `setup_with_n_identities`'s funding policy.
52+
const FUNDING_CREDITS: u64 = REGISTRATION_FUNDING + REGISTRATION_HEADROOM;
53+
4054
/// Floor `wait_for_balance` keys on before registration runs. Under
4155
/// Option C (DeductFromInput) the address receives exactly
4256
/// `FUNDING_CREDITS`, so the floor equals the funded amount.

packages/rs-platform-wallet/tests/e2e/cases/id_001_register_identity_from_addresses.rs

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -20,18 +20,18 @@ use crate::framework::prelude::*;
2020

2121
/// Funds the bank submits to the funding address. Option C
2222
/// (DeductFromInput) delivers exactly this amount to the address.
23-
/// Sized so that after the 50M registration, the residual (130M)
23+
/// Sized so that after the 50M registration, the residual (160M)
2424
/// covers the chain-time IdentityCreateFromAddresses dynamic fee
25-
/// (~110.86M, from validate_fees_of_event_v0 PaidFromAddressInputs;
26-
/// grew from ~96M after the slot-2 TRANSFER key was added in
27-
/// `173b2e15ce`, +~550 bytes × 27_000 credits/byte ≈ +14.85M) with
28-
/// ~19M buffer.
29-
const FUNDING_CREDITS: u64 = 180_000_000;
25+
/// (~125.71M, from validate_fees_of_event_v0 PaidFromAddressInputs;
26+
/// grew from ~110.86M after QA-800 added the CRITICAL key in slot 4,
27+
/// +~550 bytes × 27_000 credits/byte ≈ +14.85M) with ~30M buffer for
28+
/// the teardown sweep fee.
29+
const FUNDING_CREDITS: u64 = 210_000_000;
3030

3131
/// Floor the wait_for_balance keys on before registration runs.
3232
/// Under Option C the address receives exactly FUNDING_CREDITS, so
3333
/// the floor equals the funded amount.
34-
const FUNDING_FLOOR: u64 = 180_000_000;
34+
const FUNDING_FLOOR: u64 = 210_000_000;
3535

3636
/// Credits committed to the new identity in the registration
3737
/// transition. The address loses this exact amount minus the bank's
@@ -104,8 +104,8 @@ async fn id_001_register_identity_from_addresses() {
104104
);
105105
assert_eq!(
106106
on_chain.public_keys().len(),
107-
3,
108-
"registered identity must carry exactly three keys (MASTER + HIGH + TRANSFER)"
107+
4,
108+
"registered identity must carry exactly four keys (MASTER + HIGH + TRANSFER + CRITICAL)"
109109
);
110110
assert!(
111111
on_chain.balance() >= IDENTITY_BALANCE_FLOOR,
@@ -125,7 +125,7 @@ async fn id_001_register_identity_from_addresses() {
125125

126126
// Address residual: register_from_addresses consumed
127127
// REGISTRATION_FUNDING from the address AND the chain-time
128-
// dynamic fee (~96M observed). After both, residual <
128+
// dynamic fee (~125.71M observed). After both, residual <
129129
// FUNDING_CREDITS - REGISTRATION_FUNDING (the headroom).
130130
s.test_wallet
131131
.sync_balances()

0 commit comments

Comments
 (0)