fix(test-suite): stabilize platform e2e tests#4214
Open
shumkov wants to merge 17 commits into
Open
Conversation
Wait for restored Core nodes to converge before mining, restore finite historical header streams, and run E2Es for direct inputs with cache-safe cold starts. Test would have caught these regressions in CI: ✖ before fix, ✔ after.
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe pull request expands E2E reliability coverage across wallet initialization, SPV validation, proof verification, document handling, epoch queries, Dashmate startup, trusted-context refresh, Swift CI keychain management, and workflow orchestration. It also adds a detailed reliability specification and regression coverage. ChangesE2E reliability changes
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant Wallet
participant BlockHeadersProvider
participant Account
Caller->>Wallet: request account
Wallet->>BlockHeadersProvider: initialize chain
BlockHeadersProvider-->>Wallet: initialization complete
Wallet->>Account: create account
Account-->>Caller: return account
sequenceDiagram
participant Changes
participant E2EFilter as filter-e2e
participant Builds
participant Tests
Changes->>E2EFilter: evaluate changed paths
E2EFilter-->>Changes: publish e2e-tests-changed
Changes->>Builds: enable required builds
Builds-->>Tests: provide successful build results
Changes->>Tests: enable E2E jobs
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
shumkov
changed the title
Collaborator
|
⛔ Blockers found — Sonnet deferred (commit 259d3ba) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
packages/dashmate/src/listr/tasks/startGroupNodesTaskFactory.js (1)
39-39: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider passing
coreRpcClientsvia Listrctxinstead of a closure variable.
coreRpcClientsis populated in one task step and consumed by the next relying on their being adjacent, sequential Listr steps. This file already usesctxfor cross-step state (e.g.ctx.skipBuildServices), which is the more idiomatic and reorder-safe pattern here.♻️ Suggested refactor
- let coreRpcClients = []; - const minerConfig = configGroup.find((config) => ( config.get('core.miner.enable') )); @@ { title: 'Wait for Core peers to be connected', enabled: isLocalMinerEnabled, - task: async () => { - coreRpcClients = await Promise.all(configGroup.map(async (config) => ( + task: async (ctx) => { + ctx.coreRpcClients = await Promise.all(configGroup.map(async (config) => ( createRpcClient({ port: config.get('core.rpc.port'), user: 'dashmate', pass: config.get('core.rpc.users.dashmate.password'), host: await getConnectionHost(config, 'core', 'core.rpc.host'), }) ))); const tasks = configGroup.map((config, index) => ({ title: `Checking ${config.getName()} peers`, - task: () => waitForCorePeersConnected(coreRpcClients[index]), + task: () => waitForCorePeersConnected(ctx.coreRpcClients[index]), })); return new Listr(tasks, { concurrent: true }); }, }, { title: 'Wait for Core nodes to have the same height', enabled: isLocalMinerEnabled, - task: () => waitForNodesToHaveTheSameHeight( - coreRpcClients, + task: (ctx) => waitForNodesToHaveTheSameHeight( + ctx.coreRpcClients, WAIT_FOR_NODES_TIMEOUT, ), },Also applies to: 72-98
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/dashmate/src/listr/tasks/startGroupNodesTaskFactory.js` at line 39, Move coreRpcClients from the closure-scoped variable into the Listr context, initializing and updating the corresponding ctx property in the producer task and reading it in the consumer task. Follow the existing ctx.skipBuildServices pattern so the tasks no longer depend on closure state or adjacency.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/actions/local-network/action.yaml:
- Line 37: The shared local-network cache key is incomplete because it omits
image/source inputs that determine the generated genesis volumes. Update the
restore key at .github/actions/local-network/action.yaml lines 37-37 to include
the relevant image/source build inputs or immutable image digests, and apply the
identical completed key contract at .github/workflows/tests-dashmate.yml lines
93-93; keep both restore sites consistent.
In @.github/workflows/tests.yml:
- Around line 462-464: Add an explicit least-privilege permissions block for the
affected workflow job, beginning with contents: read and including only
permissions verified as required by its Docker, AWS, secrets, and E2E steps.
Keep the existing trigger condition unchanged, and avoid relying on
repository-default GITHUB_TOKEN permissions.
In `@packages/dashmate/test/unit/listr/tasks/startGroupNodesTaskFactory.spec.js`:
- Around line 104-190: Update the Sinon-Chai assertions in the affected tests to
use property syntax: remove trailing parentheses from
`.to.not.have.been.called()` and `.to.have.been.calledOnce()`. Preserve existing
assertion targets and argument-bearing assertions such as
`.calledOnceWithExactly(...)`.
---
Nitpick comments:
In `@packages/dashmate/src/listr/tasks/startGroupNodesTaskFactory.js`:
- Line 39: Move coreRpcClients from the closure-scoped variable into the Listr
context, initializing and updating the corresponding ctx property in the
producer task and reading it in the consumer task. Follow the existing
ctx.skipBuildServices pattern so the tasks no longer depend on closure state or
adjacency.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: d95fadbe-cb92-4532-8eb9-a0fdfbcc61a6
📒 Files selected for processing (9)
.github/actions/local-network/action.yaml.github/workflows/tests-dashmate.yml.github/workflows/tests.ymldocs/E2E_TESTS_GREEN_SPEC.mdpackages/dashmate/src/createDIContainer.jspackages/dashmate/src/listr/tasks/startGroupNodesTaskFactory.jspackages/dashmate/test/unit/listr/tasks/startGroupNodesTaskFactory.spec.jspackages/js-dapi-client/lib/BlockHeadersProvider/createBlockHeadersProviderFromOptions.jspackages/js-dapi-client/test/unit/BlockHeadersProvider/createBlockHeadersProviderFromOptions.spec.js
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## v4.2-dev #4214 +/- ##
==========================================
Coverage 87.51% 87.52%
==========================================
Files 2666 2667 +1
Lines 337942 338070 +128
==========================================
+ Hits 295765 295892 +127
- Misses 42177 42178 +1
🚀 New features to boost your workflow:
|
Await one wallet-shared block-header provider initialization so empty stores authenticate the network genesis before height-1 historical sync. Preserve custom transports and serialize concurrent getAccount calls by account index. Remove the disproven default-provider callback change; the browser E2E uses the already-correct configured-provider path. Test would have caught this in CI: ✖ before fix, empty stores skipped initialization, concurrent getAccount returned duplicate index-0 accounts, and custom transports threw; ✔ after fix, focused tests, wallet unit/integration suites, and 412 Chrome tests pass.
Treat grpc-web's exact already-closed exception as idempotent cleanup while continuing to propagate other cancellation failures. Test would have caught this in CI: ✖ before fix, ✔ after.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@packages/js-dapi-client/test/unit/BlockHeadersProvider/BlockHeadersReader.spec.js`:
- Line 140: Update the test description in the rejected-headers stream case so
it starts with “should” and moves the “[data]” marker to the end, preserving the
existing test meaning.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 4bd4bcd9-5a4f-46aa-9fdf-6db763a9e04a
📒 Files selected for processing (8)
docs/E2E_TESTS_GREEN_SPEC.mdpackages/js-dapi-client/lib/BlockHeadersProvider/BlockHeadersReader.jspackages/js-dapi-client/test/integration/BlockHeadersProvider/BlockHeadersProvider.spec.jspackages/js-dapi-client/test/unit/BlockHeadersProvider/BlockHeadersReader.spec.jspackages/wallet-lib/src/types/Wallet/Wallet.jspackages/wallet-lib/src/types/Wallet/methods/createAccount.jspackages/wallet-lib/src/types/Wallet/methods/createAccount.spec.jspackages/wallet-lib/src/types/Wallet/methods/getAccount.js
Dash Core regtest keeps the preceding target while dash-spv was applying Dark Gravity Wave after 24 headers. Test would have caught this in CI: ✖ before fix: a valid fast-mined header was rejected and a changed-target first header was accepted. ✔ after fix: the fast chain is accepted and target changes are rejected from genesis.
Restore cached wallet headers at their first authenticated height, use affected-state proof waits and explicit epoch selectors, and sanitize schema-declared binary document properties before transition construction. Test would have caught these in CI: ✖ before fix, ✔ after.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/E2E_TESTS_GREEN_SPEC.md`:
- Around line 244-245: Qualify the wallet genesis-initialization requirement in
the documented goal to apply only to DAPI-backed online wallets, and explicitly
state that custom online transports without DAPI internals intentionally skip
provider initialization. Keep the existing historical-header behavior and align
the wording with the custom-transport exception documented elsewhere.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: e1f9b614-4e57-4dad-b176-9cda091bfdea
📒 Files selected for processing (12)
docs/E2E_TESTS_GREEN_SPEC.mdpackages/bench-suite/lib/client/createPlatformProofVerifier.jspackages/dash-spv/lib/consensus.jspackages/dash-spv/test/index.jspackages/js-dapi-client/test/unit/BlockHeadersProvider/BlockHeadersReader.spec.jspackages/js-dash-sdk/src/SDK/Client/Platform/IPlatformProofVerifier.tspackages/platform-test-suite/lib/test/createPlatformProofVerifier.jspackages/platform-test-suite/test/unit/createPlatformProofVerifier.spec.jspackages/rs-sdk/src/platform/transition/put_document.rspackages/wallet-lib/src/types/Wallet/methods/createAccount.jspackages/wallet-lib/src/types/Wallet/methods/createAccount.spec.jspackages/wasm-sdk/tests/functional/epochs-blocks.spec.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/wallet-lib/src/types/Wallet/methods/createAccount.spec.js
- packages/wallet-lib/src/types/Wallet/methods/createAccount.js
- packages/js-dapi-client/test/unit/BlockHeadersProvider/BlockHeadersReader.spec.js
Proof-wait E2Es in workflow 30036234705 reproduced unknown-contract verification failures for custom document batches, while Swift resolver tests reproduced SecItemAdd -61 on the persistent runner keychain. Test evidence: ✖ the prior workflow failed both paths; ✔ focused contract extraction/cache regressions, shell syntax, formatting, and forced-failure keychain cleanup checks pass locally. Full Rust and network E2E confirmation follows in CI because the configured local sccache fails at the rustc -vV probe.
Limit the SPV genesis-initialization goal to DAPI-backed wallets and retain the documented custom-transport exception. Tests omitted: documentation-only clarification with no behavior change.
Repair a dangling user-domain default from validated per-user keychains before provisioning the ephemeral test keychain. Keep partial preference mutations contained, preserve unrelated Security errors, and run the regression from the reusable Swift workflow. Test would have caught this in CI: ✖ before fix: exit-44 fixture returned 44 before provisioning; ✔ after fix: Bash 3.2 state-machine regression passes recovery and all injected mutation boundaries.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/wasm-sdk/src/state_transitions/broadcast.rs (1)
21-42: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winParallelize the per-contract refresh calls.
prepare_state_transition_contextcallsrefresh_contract(contract_id)sequentially, while eachDataContract::fetch(...).ok_or_else(...)call bypasses the cache and must complete proof verification before the next contract is fetched. Collect the independent futures for the distinct batch contract IDs andtry_join_all/join_allthem instead of awaiting one by one.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/wasm-sdk/src/state_transitions/broadcast.rs` around lines 21 - 42, The per-contract refreshes in WasmSdk::prepare_state_transition_context currently run sequentially. Collect futures for each distinct ID from referenced_contract_ids and await them concurrently with try_join_all (propagating refresh_contract errors), preserving the existing successful empty-set behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/wasm-sdk/src/state_transitions/broadcast.rs`:
- Around line 21-42: The per-contract refreshes in
WasmSdk::prepare_state_transition_context currently run sequentially. Collect
futures for each distinct ID from referenced_contract_ids and await them
concurrently with try_join_all (propagating refresh_contract errors), preserving
the existing successful empty-set behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: dbd35af5-8701-4beb-b1d9-bcda6826c380
📒 Files selected for processing (7)
.github/workflows/swift-sdk-build.yml.github/workflows/tests.ymldocs/E2E_TESTS_GREEN_SPEC.mdpackages/swift-sdk/run_tests.shpackages/swift-sdk/tests/run-tests-keychain-regression.shpackages/wasm-sdk/src/sdk.rspackages/wasm-sdk/src/state_transitions/broadcast.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/workflows/tests.yml
- docs/E2E_TESTS_GREEN_SPEC.md
The reusable Platform test-suite job could exhaust its 15-minute cap during a healthy cold helper-image pull, cancelling the main suite before readiness and a browser shard while tests were passing. Keep the timeout bounded but raise it to 30 minutes so bootstrap variance does not consume the execution budget. Test would have caught this in CI: ✖ before fix: the timeout policy assertion reported 15m below the cold-pull-safe 30m floor; ✔ after fix: the assertion and workflow parsers pass at 30m.
Refresh current and previous trusted quorum caches at the shared proof-preparation boundary. Browser requests bypass HTTP caches and have a bounded timeout; partial failures retain every successful exact-hash update while proof verification remains fail-closed. TDD proof: ✖ before, browser credit transfer failed after quorum rotation with an InvalidQuorum cache miss in run 30048343835; ✔ after, deterministic provider and WASM preparation regressions cover absent-to-present refresh, independent failures, cache preservation, semantic failures, and exact-hash rejection (CI is the execution authority because the local sccache rustc probe is blocked).
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/rs-sdk-trusted-context-provider/src/provider.rs (1)
318-344: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winConsider fetching current/previous quorums concurrently.
refresh_quorum_cachesawaits both fetches sequentially even though they're independent. Since this refresh runs on the proof-verification hot path (perprepare_state_transition_contextin broadcast.rs), running them concurrently halves the round-trip latency.Please confirm `tokio::join!` is available/appropriate for this crate's wasm32 target (it doesn't spawn tasks, only interleaves polling, but worth double-checking against this crate's tokio feature flags for wasm32 builds).♻️ Proposed fix
- let current_error = self.fetch_current_quorums().await.err(); - let previous_error = self.fetch_previous_quorums().await.err(); + let (current_result, previous_result) = + tokio::join!(self.fetch_current_quorums(), self.fetch_previous_quorums()); + let current_error = current_result.err(); + let previous_error = previous_result.err();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/rs-sdk-trusted-context-provider/src/provider.rs` around lines 318 - 344, Update refresh_quorum_caches to initiate fetch_current_quorums and fetch_previous_quorums concurrently using an appropriate non-spawning combinator such as tokio::join!, preserving the existing independent-error handling and result messages. Verify the crate’s Tokio features and wasm32 compatibility before using it.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/rs-sdk-trusted-context-provider/src/provider.rs`:
- Around line 42-44: Generalize the WASM timeout helper used by quorum_request
into an http_request method that applies the per-request timeout consistently.
Update both existing quorum_request call sites and fetch_masternode_addresses to
use http_request instead of calling self.client.get directly, preserving the
existing request behavior while ensuring masternode discovery cannot hang
indefinitely on wasm32.
---
Nitpick comments:
In `@packages/rs-sdk-trusted-context-provider/src/provider.rs`:
- Around line 318-344: Update refresh_quorum_caches to initiate
fetch_current_quorums and fetch_previous_quorums concurrently using an
appropriate non-spawning combinator such as tokio::join!, preserving the
existing independent-error handling and result messages. Verify the crate’s
Tokio features and wasm32 compatibility before using it.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: bc4b7b5e-83cd-43b7-80fb-d767d35b5eb2
📒 Files selected for processing (5)
.github/workflows/tests-test-suite.ymldocs/E2E_TESTS_GREEN_SPEC.mdpackages/rs-sdk-trusted-context-provider/src/provider.rspackages/wasm-sdk/src/context_provider.rspackages/wasm-sdk/src/state_transitions/broadcast.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/wasm-sdk/src/state_transitions/broadcast.rs
- docs/E2E_TESTS_GREEN_SPEC.md
Red evidence: PR run 30052329512 observed a stale purchased-document owner and the restored-wallet test exhausted its 950-second timeout. Green evidence: focused syntax and ESLint checks pass; the required full functional and main E2E proof will come from this commit's PR run.
Test would have caught this in CI: ✖ unsafe remote checkpoint and stale header baseline before fix, ✔ genesis-rooted resume and durable header normalization after.
Treat missing user search-list preferences as recoverable only on the dedicated Swift runner, preserve validated baselines, and verify cleanup before removing the job keychain. Test would have caught this in CI: ✖ before fix: the exit-44 search-list fixture stopped before provisioning; ✔ after fix: the Bash 3.2 lifecycle suite passes recovery, cleanup containment, and next-run orphan isolation.
The Dashmate local-network E2E spent 11m13s pulling healthy helper images and was then cancelled by the reusable job's 15-minute cap. Match the sibling platform test-suite workflow's bounded 30-minute budget without changing any test-level deadline. Test would have caught this in CI: ✖ before fix: the timeout policy assertion reported the Dashmate 15m budget below the cold-pull-safe 30m floor; ✔ after fix: both reusable E2E workflows parse and satisfy the 30m policy.
Use the job keychain path for the CLI smoke read and delete, then scope the macOS resolver integration tests to the user preference domain with guaranteed restoration and serialized execution. Test would have caught this in CI: ✖ before fix: stale implicit search exited 44 before the Swift body (expected sentinel 23) ✔ after fix: explicit read/delete reached the Swift body and preserved sentinel 23
Warm the WASM functional process once against the real trusted-context readiness condition, preserve concrete retry diagnostics, and keep the cold-start workflow within its established outer budget. Poll wallet transaction indexes before asserting exact membership so balance propagation cannot race the state under test. Test would have caught this in CI: ✖ the old retry skipped the final partial sleep and the main/package-functional jobs failed; ✔ the focused regression, 398 Node tests, and 398 Karma tests pass before the full E2E rerun.
thepastaclaw
suggested changes
Jul 24, 2026
thepastaclaw
left a comment
thepastaclaw
left a comment
Collaborator
There was a problem hiding this comment.
Preliminary review — Codex only
The proof-verification integration generally authenticates GroveDB state and Tenderdash roots, but the affected-state fallback weakens the state-transition confirmation boundary. Because a verified snapshot does not prove execution and the caller relies on an unauthenticated DAPI error field, affected-state transition families can be reported as successful without having executed.
Validated blockers were found in the Codex precheck. Sonnet is deferred until a fresh Codex revalidation clears the blocker gate.
Review provenance
- Codex reviewers:
gpt-5.6-sol— general (completed),gpt-5.6-sol— security-auditor (completed),gpt-5.6-sol— ffi-engineer (completed) - Verifier:
gpt-5.6-sol— verifier - Sonnet: not run (deferred by blocker gate)
🔴 1 blocking
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `packages/platform-test-suite/lib/test/createPlatformProofVerifier.js`:
- [BLOCKING] packages/platform-test-suite/lib/test/createPlatformProofVerifier.js:169-171: Affected-state snapshots cannot confirm transition execution
`waitForAffectedState` accepts `AffectedState` outcomes that authenticate only a height-pinned snapshot of keys affected by the transition; the Rust and WASM APIs explicitly state that these outcomes are not evidence that the specific transition executed. This verifier nevertheless implements `verifyStateTransitionResult`, whose caller in `broadcastStateTransition.ts` returns success after checking the original DAPI response's unauthenticated `error` field and merely requiring proof and metadata to be present. A malicious or faulty selected DAPI node can therefore suppress a consensus error or drop an identity top-up, transfer, withdrawal, address-funds movement, shield, or no-history token transition, then return a valid proof of unchanged affected state from a quorum-authenticated root. The application will report the transition as successful even though no execution was proved. Keep this confirmation API strict, or redesign the caller to expose affected-state results as non-confirming snapshots instead of successful transition confirmations. Apply the same correction to `packages/bench-suite/lib/client/createPlatformProofVerifier.js:165-167`.
Comment on lines
+169
to
+171
| // Verify either the execution result or the affected-state snapshot, | ||
| // depending on the proof shape supported by the transition family. | ||
| await sdk.stateTransitions.waitForAffectedState(stateTransition); |
Collaborator
There was a problem hiding this comment.
🔴 Blocking: Affected-state snapshots cannot confirm transition execution
waitForAffectedState accepts AffectedState outcomes that authenticate only a height-pinned snapshot of keys affected by the transition; the Rust and WASM APIs explicitly state that these outcomes are not evidence that the specific transition executed. This verifier nevertheless implements verifyStateTransitionResult, whose caller in broadcastStateTransition.ts returns success after checking the original DAPI response's unauthenticated error field and merely requiring proof and metadata to be present. A malicious or faulty selected DAPI node can therefore suppress a consensus error or drop an identity top-up, transfer, withdrawal, address-funds movement, shield, or no-history token transition, then return a valid proof of unchanged affected state from a quorum-authenticated root. The application will report the transition as successful even though no execution was proved. Keep this confirmation API strict, or redesign the caller to expose affected-state results as non-confirming snapshots instead of successful transition confirmations. Apply the same correction to packages/bench-suite/lib/client/createPlatformProofVerifier.js:165-167.
Suggested change
| // Verify either the execution result or the affected-state snapshot, | |
| // depending on the proof shape supported by the transition family. | |
| await sdk.stateTransitions.waitForAffectedState(stateTransition); | |
| await sdk.stateTransitions.waitForResponse(stateTransition); |
source: ['codex']
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.
Issue being fixed or feature implemented
Platform E2Es could fail for two independent startup/lifecycle reasons:
restored Core nodes could begin mining before sharing a common tip, and a new
browser wallet could process height-1 historical headers before its SPV chain
had an authenticated genesis root. When historical-header validation rejects a
batch, repeated cleanup of the same gRPC-web stream can then surface
Client already closed - cannot .close()instead of the primary SPV error.Once exposed, that primary error showed that SPV applied Dark Gravity Wave to
fast-mined regtest headers even though Dash Core disables regtest retargeting.
Relevant package and harness changes also did not trigger the E2E fleet unless
they changed the Platform version.
Related: #4210
What was done?
and block hash before the local miner starts. A mismatch or the established
five-minute deadline fails startup instead of mining onto a divergent tip.
headers or the configured network genesis before account synchronization.
Initialization is awaited once per wallet; concurrent
getAccountcallsshare account creation by index, while offline and custom transports without
DAPI client internals keep their prior behavior.
exception as idempotent. Other cancellation failures still propagate, while
the original SPV rejection now reaches the caller and CI output.
Dash Core's no-retarget rule. Canonical-target, proof-of-work, timestamp, and
changed-target checks remain enforced; other networks keep the DGW path.
consumed environment templates trigger the full fleet. Fixture cache keys
include their deterministic construction inputs, and cold cache misses always
execute the uncached Dashmate test.
The initial callback-swap hypothesis was removed after the first PR run showed
that browser E2Es use the already-correct configured-provider path. The trigger
deliberately follows direct E2E consumers rather than the entire transitive
monorepo dependency graph; broad Platform source changes continue to use the
existing version-change gate.
How Has This Been Tested?
initializeChainWith([], -1)and concurrent account creation did not shareprovider initialization; both regressions pass after the awaited,
wallet-shared initialization.
getAccount({ index: 0 })returned two distinct accounts and a conforming custom transport threw on
transport.client; the focused suite went from 5 passing / 2 failing to9 passing.
cancellation succeeds and the second throws grpc-web's exact already-closed
exception. Without the fix the cleanup exception escapes; with it the reader
performs both cleanup attempts, drains the tracked stream, and emits the
original validation error.
30029414181: before the consensus fix, a validfast-mined regtest header was rejected after the 24-header DGW window and a
changed-target first header was accepted. After the fix, the same regressions
accept Core's no-retarget chain and reject the target change from genesis.
412 completed, 4 skipped.
4 skipped. The genesis-to-height-1 provider integration regression passes.
ordering, timeout forwarding, failure propagation, and miner blocking.
fixture attempted to write the sandbox-blocked
~/.dashmatepath; the changedfocused suite completed independently.
warnings remain. Edited YAML parses, deterministic trigger/cache assertions
pass,
yarn constraintspasses, andgit diff --checkis clean.PR run
30025175452proved the Core convergence gate: all three Dashmate E2Espassed, including the cached local-network job. Both browser shards then
reproduced the repeated-cleanup exception, while the Node platform and
functional suites timed out during wallet startup without exposing the
underlying validation error. Diagnostic run
30029414181on561da403d8preserved that error as
SPV: Header 2e53e5ed...d8afbd55 is invalidduring theheight-1 request for 1393 regtest headers. Signed head
3bfa5d7dfbpins andfixes the confirmed no-retarget mismatch; its full E2E run is in progress.
Breaking Changes
None.
Checklist:
For repository code-owners and collaborators only
Post-Deploy Monitoring & Validation
an infrastructure flake requires one.
every Dashmate, platform-suite, browser, and functional E2E job; no dependent
job is unexpectedly skipped; all launched jobs finish green.
Client already closed,SPV:,Empty SPV chain,unauthenticated remote checkpoint,mismatched block hashes,Core tips do not match, andcache-hit.first historical batch is rejected, convergence reaches the five-minute
deadline, a cold-cache Dashmate job runs no Mocha command, or an expected E2E
job is skipped/cancelled.
gh run view; revert theresponsible startup/workflow change if it broadly blocks valid local-network
startup, otherwise correct the pinned root cause and rerun the full fleet
before merge.
Summary by CodeRabbit