Migrate DuskEVM bridge to current contracts and SDK - #947
Conversation
9917946 to
05cd966
Compare
56e4441 to
c041393
Compare
There was a problem hiding this comment.
Pull request overview
This PR migrates the web-wallet’s DuskEVM bridge UI and wallet-store flows off the legacy bridge contract/data-driver surface and onto the current contracts, including OP-style withdrawal status/prove/finalize handling and explicit byte-level encoding.
Changes:
- Replace legacy pending-withdrawals/finalization-period UI with an OP-style withdrawal status + prove/finalize flow backed by Portal/DGF data-drivers.
- Implement explicit bridge payload encoders for L1 deposits (
depositETHToWithValue) and L2→L1 withdrawal recipientextraData(0x00 || raw Dusk BLS pubkey). - Update wallet-store bridge transaction submission paths, add unit tests, and document new env/config requirements.
Reviewed changes
Copilot reviewed 22 out of 28 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/routes/(app)/dashboard/bridge/transactions/+page.svelte | Drops server-provided pending withdrawals; renders the new self-contained transactions UI. |
| src/routes/(app)/dashboard/bridge/+page.svelte | Removes pendingWithdrawals prop wiring to the bridge container. |
| src/routes/(app)/dashboard/bridge/+layout.js | Removes legacy pending-withdrawals load call; returns empty layout data. |
| src/lib/web3/abi/bridgeABI.json | Adds bridgeETHTo ABI entry alongside bridgeETH. |
| src/lib/stores/walletStore.js | Reworks deposit call to new payload builder; adds generic contract function executor + prove/finalize helpers. |
| src/lib/stores/stores.d.ts | Updates wallet/network store service typings for new bridge/finalization APIs. |
| src/lib/stores/networkStore.js | Adds getLatestBlockTimestamp for L1 proof maturity checks. |
| src/lib/stores/tests/walletStore.spec.js | Adds deposit payload submission coverage and env-missing rejection test. |
| src/lib/containers/BridgeContract/BridgeContract.svelte | Removes pending withdrawals prop; forwards unshieldedAccount to bridge UI. |
| src/lib/constants/messages.js | Updates withdrawal pending message copy. |
| src/lib/components/EvmTransactions/EvmTransactions.svelte | Replaces pending-withdrawals list with withdrawal status/prove/finalize UI. |
| src/lib/components/Bridge/Bridge.svelte | Removes legacy WASM extraData encoding; uses explicit recipient encoding and links into withdrawal status UI. |
| src/lib/components/tests/EvmTransactions.spec.js | Adds UI coverage for new withdrawal status/prove flow. |
| src/lib/bridge/withdrawals.js | Implements withdrawal receipt parsing, proof discovery via DGF, portal preflight readiness, and prove/finalize submission. |
| src/lib/bridge/pendingWithdrawals.js | Removes legacy pending-withdrawals helper (deleted). |
| src/lib/bridge/formatBlocksAsTime.js | Removes legacy block→time helper (deleted). |
| src/lib/bridge/encoding.js | Adds BLS pubkey raw-byte encoding and bridge recipient helpers. |
| src/lib/bridge/deposit.js | Adds deposit payload encoding matching Rust rkyv driver encoding. |
| src/lib/bridge/bridge.d.ts | Removes legacy pending withdrawal types (deleted). |
| src/lib/bridge/tests/withdrawals.spec.js | Adds regression tests for receipt parsing, config validation, and readiness/finalization gating. |
| src/lib/bridge/tests/encoding.spec.js | Adds byte-level tests for recipient + deposit payload encoding. |
| README.md | Documents new bridge env vars, data-driver requirements, and scope distinctions (bridge vs generic envelopes). |
| package.json | Adds @noble/curves dependency for BLS key conversion. |
| package-lock.json | Locks @noble/curves dependency. |
| CHANGELOG.md | Notes bridge migration in changelog. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
c041393 to
6eb52eb
Compare
|
I revalidated this PR against current Blocking: withdrawal recipients are encoded in the removed legacy format. The bundled Portal and DGF data-driver schemas do match the current contract signatures (41/41 and 35/35 methods respectively), and all four Copilot threads are resolved/outdated. Fresh validation passed: Recommended path:
|
HDauven
changed the title
|
The blocking recipient-format mismatch is addressed in
Local validation passes: 43 focused tests, full tests, format, lint, typecheck, and production build. I also corrected the PR description so it no longer presents the superseded legacy withdrawal evidence as valid. |
|
Local withdrawal E2E follow-up:
Validated the wallet test suite for this module (15/15), formatting, ESLint, Svelte typecheck, the Portal readiness test, and the current-Rusk query-height regression. The actual sender-bearing L1 finalization also succeeded in the original local E2E. |
|
Fresh Rusk 1.7.1 withdrawal E2E is complete and the full evidence is now recorded in the PR description. Key results:
Commit |
|
Implemented the OP-style withdrawal activity UX in
Validation is green: lint, Svelte typecheck (0/0), 126 test files / 599 tests, production build, and authenticated Chromium at 1440px and 390px with no console errors or horizontal overflow. Blockscout documents the |
|
Refactored the withdrawal activity implementation in Structural changes:
This improves review boundaries rather than raw diff size: the added component/module boilerplate raises the PR to 5,230 additions across 39 files. Validation remains green: 20 focused tests, all 126 test files / 599 tests, formatting, lint, Svelte typecheck (0 errors/warnings), production build, and authenticated Chromium at 1440px and 390px with no console errors or horizontal overflow. |
|
Added resumable DuskDS -> DuskEVM deposit lifecycle tracking in The wallet now waits through DuskDS finality and L2 relay, preserves pending state across navigation/reload and transient RPC failures, shows incoming DUSK, distinguishes L1/L2 failures, and exposes deposit activity alongside withdrawals. Validated with full |
| return await unprovenWithdrawalStatus(event); | ||
| } | ||
|
|
||
| const proofSubmitter = await readProofSubmitterAt( |
There was a problem hiding this comment.
WARNING: Readiness/preflight is evaluated for the last-listed proof submitter, but finalization binds to the wallet (msg.sender) — a mismatch can present a doomed "Finalize" action.
Context: loadWithdrawalStatus picks proofSubmitters[count-1] and provenWithdrawalStatus preflights checkWithdrawal(hash, thatSubmitter), but the finalize path (finalizeWithdrawal → walletStore.finalizeDuskEvmWithdrawal) submits the finalizeWithdrawalTransaction contract call with only the withdrawal and no submitter arg — so the Portal finalizes against msg.sender, the wallet's own proof, not the last submitter's. In OP anyone may prove a withdrawal.
Impact: If a third party proved the withdrawal and the wallet never did, the last submitter is that third party; their mature proof yields ready_to_finalize, the Finalize button (gated only on stage, no ownership check) enables, and submit reverts as not proven by msg.sender. Any hash is selectable via activity or manual lookup, so this is not limited to the wallet's own withdrawals, and the pre-submit re-check keys on the same last submitter so it does not catch it. The normal flow — prove then finalize your own withdrawal — is unaffected, which is why tests pass.
Severity-rationale: Reachable only when the wallet is not itself a valid mature submitter (e.g. a third party proved it), and it rests on OP msg.sender finalize semantics — if the Dusk portal finalizes against any mature-proof holder, the mismatch is harmless. A maintainer judgment call, not a blocker.
Fix: Preflight readiness against the account that will actually finalize (the wallet's own submitter identity), or document and enforce the single-submitter assumption; add a multi-submitter test.
| const VITE_EVM_BRIDGE_GAME_SEARCH_DEPTH = BigInt( | ||
| import.meta.env.VITE_EVM_BRIDGE_GAME_SEARCH_DEPTH ?? 64 | ||
| ); |
There was a problem hiding this comment.
WARNING: ?? only guards null/undefined, so an empty-string env value becomes BigInt("") = 0n and a non-numeric value throws at module load.
Context: Unset (undefined) defaults to 64, but VITE_EVM_BRIDGE_GAME_SEARCH_DEPTH= (a common .env state) yields 0n, and at depth 0n scanGamePages's while (remaining > 0n) never iterates.
Impact: No dispute-game proof is ever found — every withdrawal is stuck reporting "waiting for output" and prove is impossible, with nothing pointing at config as the cause. A non-numeric value instead throws during module evaluation, breaking the whole bridge-transactions route.
Fix: Parse defensively as depositLifecycle does for its threshold — coerce with Number, require finite-and-positive, fall back to 64 otherwise — rather than BigInt(env ?? 64).
| ### Changed | ||
|
|
||
| - Migrate DuskEVM bridge flows to the current contracts and the published | ||
| `@dusk/evm-sdk` beta [#947] |
There was a problem hiding this comment.
NOTE: CHANGELOG entry uses reference-style link [#947] with no matching link definition, so it renders as literal text; every other entry links a tracking issue, not the PR.
Context: Only [#928], [#932], [#939] definitions exist at the file bottom. This PR closes #946 and #948.
Fix: Reference the issues ([#946], [#948]) and add their link definitions in the block at the bottom, matching the existing format.
| export function encodeDepositETHToWithValueArgs({ | ||
| to, | ||
| amountLux, | ||
| minGasLimit, | ||
| extraData = new Uint8Array(), | ||
| }) { | ||
| const amount = BigInt(amountLux); | ||
| const extra = normalizeBytes(extraData, "extraData"); | ||
| const gas = BigInt(minGasLimit); | ||
| const paddedExtraLength = Math.ceil(extra.length / 8) * 8; | ||
| const rootOffset = paddedExtraLength; | ||
| const out = new Uint8Array(rootOffset + 40); | ||
|
|
||
| assertUnsignedInteger(amount, UINT64_MAX, "amountLux"); | ||
| assertUnsignedInteger(gas, UINT32_MAX, "minGasLimit"); | ||
|
|
||
| out.set(extra, 0); | ||
| writeDepositArchiveRoot(out, rootOffset, { | ||
| amount, | ||
| extraLength: extra.length, | ||
| gas, | ||
| toBytes: evmAddressToBytes(to), | ||
| }); | ||
| return out; | ||
| } |
There was a problem hiding this comment.
NOTE: Deposit calldata is a hand-reproduced copy of the Rust rkyv::to_bytes layout — a cross-language contract enforced only by hard-coded golden literals in a JS test.
Context: The recipient path moved behind the SDK's canonical encoder, but deposits still hand-code the archived struct. The cross-language fixture the PR touts covers the 196-byte recipient, not this encoder — its vectors in encoding.spec.js are JS literals never checked against Rust, so a field reorder or rkyv version bump silently breaks deposits with no compile/test signal.
Fix: Move deposit encoding behind the SDK too (matching the withdrawal direction), or add a checked-in cross-language fixture generated from the Rust driver.
| if (typeof value === "string") { | ||
| return value.startsWith("0x") ? BigInt(value) : BigInt(value); | ||
| } |
There was a problem hiding this comment.
NOTE: Dead ternary — both branches are identical BigInt(value); a non-0x hex string would throw, suggesting the hex branch was intended but never implemented.
Fix: Drop the ternary (return BigInt(value)), or handle the non-0x hex case explicitly if that was the intent.
| target="_blank" | ||
| /> | ||
| <AppAnchorButton | ||
| href={withdrawalStatusHref(rememberWithdrawalTxHash(hash))} |
There was a problem hiding this comment.
NOTE: A side-effecting call (localStorage write + rememberWithdrawalTransaction store mutation) runs inside a template href expression, re-evaluated on every render of the success fragment.
Fix: Remember the withdrawal in the success handler (or a reactive/onMount effect) and pass only the resolved hash to withdrawalStatusHref.
| const l1Block = await client.request({ | ||
| method: "eth_getBlockByNumber", | ||
| params: [`0x${blockHeight.toString(16)}`, true], | ||
| }); | ||
|
|
||
| if (l1Block === null) return null; | ||
|
|
||
| const l1Transaction = l1Block.transactions?.[transactionIndex]; | ||
| const resolved = | ||
| typeof l1Transaction === "string" ? l1Transaction : l1Transaction?.hash; | ||
|
|
||
| const hash = normalizedTxHash(resolved); | ||
| if (!hash) { | ||
| throw new Error( | ||
| "The DuskEVM adapter returned no matching transaction for the finalized DuskDS deposit." | ||
| ); | ||
| } | ||
|
|
||
| updateDepositTransaction(remembered.transactionHash, { | ||
| l1TransactionHash: hash, | ||
| }); | ||
| return hash; |
There was a problem hiding this comment.
NOTE: Resolving the canonical L1 deposit hash assumes the Dusk block's transaction index aligns 1:1 with the adapter's L1 block at the same height — an unenforced cross-service invariant.
Context: If the adapter ever prepends system transactions or reorders, transactionIndex points at the wrong tx. Partly self-correcting — a wrong hash makes observeDepositStatus fail into trackingError rather than mislead — and gated by the documented adapter dependency (#152), but the alignment contract lives only in prose.
Fix: Cross-check the resolved L1 transaction against the Dusk tx id (or an adapter-provided mapping) instead of trusting positional index alignment.
| const tracker = (async () => { | ||
| while (true) { | ||
| const item = await refreshDepositTransaction(hash); | ||
|
|
||
| if (!item || isDepositTerminal(item)) return item; | ||
|
|
||
| await delay( | ||
| document.visibilityState === "hidden" | ||
| ? BACKGROUND_POLL_INTERVAL_MS | ||
| : ACTIVE_POLL_INTERVAL_MS | ||
| ); | ||
| } | ||
| })().finally(() => activeTrackers.delete(hash)); |
There was a problem hiding this comment.
NOTE: The per-deposit tracker polls in an unbounded while (true) with no max-attempts or backoff; a deposit that never reaches a terminal stage (Dusk tx never finalizes, or L1 hash never resolves) polls every 5–30s for the life of the page. On error, refreshDepositTransaction returns a non-terminal item with trackingError, so the loop keeps polling rather than stopping.
Fix: Cap attempts or back off after prolonged non-progress, surfacing a stalled state rather than polling forever.
moCello
left a comment
moCello
left a comment
There was a problem hiding this comment.
COMMENT — Core migration is sound, all prior threads resolved, and #948's readiness gating holds for the normal flow (prove/finalize gate on Portal status, not explorer state; finalize re-checks before submit). Two inline warnings set the token and neither hard-blocks: the game-search-depth env parse (empty string silently disables all proof discovery; non-numeric breaks the route at module load) and the readiness/finalize submitter mismatch (a third-party-proven withdrawal can surface a doomed Finalize).
Not posted inline: the activity-lifecycle UX (deposit/withdrawal Blockscout feeds and their lifecycle components) could ship as its own PR separate from the core contract migration — smaller reviews land faster. Advisory, not a gate; the contract-migration core is the mergeable spine.
41fe260 to
7fceb99
Compare
Summary
depositETHToWithValue@dusk/evm-sdk@0.1.0-beta.3release for protocol-owned DuskEVM bridge behavior0x02 || 0x01 || 0x00 || raw Dusk BLS public keybridgeETHTo(address,uint32,bytes)surfaceMessagePassedparsing/hash verification, withdrawal hashing, L1 proof/finalization serialization, generated method metadata, and protocol constants to the SDKCloses #946.
Closes #948.
SDK dependency
The wallet consumes the JSR release through its npm compatibility endpoint:
Release: https://jsr.io/@dusk/evm-sdk@0.1.0-beta.3
The dependency is pinned exactly while the SDK remains beta. The previous direct
@noble/curvesdependency and handwritten L2 bridge ABI have been removed.Validation
npm testnpm run formatnpm run lintnpm run typechecknpm run buildTypecheck/build still print the existing SvelteKit
jsconfigwarning. Tests still print existing jsdom CSS parse warnings, and the build prints existing third-party Rolldown annotation/chunk warnings; all commands exit successfully. The existing transitiveundiciaudit advisory is also present onmainand is not introduced by this PR.Protocol evidence
A fresh end-to-end withdrawal run completed against Rusk 1.7.1 (
a9cdb5a76), current DuskEVM contracts (e0752a7), and the adapter (9a673af):0x3612baede6a85572c223e2884e46bb79d0da04d5a9ecafcf92c14457f2199a54, block3180xf8b21d130d20880d7b5a51557dac3fc8d99ac4e49c56f5047d4f61d08383652a90fea53616703983b39605619799049c3b2637d5ef45e3436082d1c963a52472ac13c83124054a5ed27b8264778940bb8a010eb43b0b6a7b7d4dbb1d30ef9c6eand06d398365aa7c95505022422043c064350c15f9f76db4e06442bf20601f2fb69checkWithdrawal(withdrawalHash, proofSubmitter)returnedready_to_finalizethrough the wallet's Portal data driverfee62a03d21631ef397f470da77b4b4aadc7e47b473b74305a5a922c4d8d270c, included at block751without an execution error0x533d8034c7ba773b6f87e29100ec4acdb0d5a53ccc694bf3579e6484c43279d7, with the four canonical bridge, messenger, and Portal eventsfinalized0.1 DUSK; the observed account delta was exactly+0.097716912 DUSKafter0.002283088 DUSKfinalization gasThe L2 withdrawal was initiated with the published SDK and Viem. Proof discovery, readiness checking, proof submission, and finalization ran through the actual web-wallet browser modules. The local rehearsal had no challenger/resolver service, so the referenced fault game was resolved explicitly after its configured clock expired. Local Chromium disabled cross-origin enforcement because the standalone local Rusk endpoint does not emit browser CORS headers.
Neither previous Rusk 1.6 failure appeared: there was no sender-dependent root query and no false
proof maturity delay not metresult. Rusk 1.7.1 or later is therefore documented as a runtime requirement for this flow.Withdrawal activity UX
bridgeETHTo(address,uint32,bytes)calls to the configured L2 bridge before enriching them with Blockscout Optimism withdrawal statusLatest validation after this UX update:
npm run lint,npm run typecheck(0 errors/warnings), fullnpm test(126 files, 599 tests),npm run build, and authenticated Chromium checks at 1440px and 390px with no console errors or horizontal overflow. The mnemonic suite produced its existing probabilistic false failure once because a randomly substituted final BIP-39 word happened to retain a valid checksum; the isolated rerun and two subsequent full runs passed.Deposit lifecycle UX
Latest validation after the deposit update: full
npm run checks(128 files, 604 tests),npm run build, and isolated Chromium checks at 1440px and 390px with no console errors or horizontal overflow. A real local deposit was resolved from DuskDS transaction0xedd48f60f23d7cb7fdb84851628b10f0ecc4a7d03d8d99e5247894b2bcd77643to DuskEVM transaction0x719503f03d4d9ac964396ec4fcd641c89eeec57eccd2b090740cdee8a62cfb69and the wallet reachedAvailable.Native Dusk transaction IDs
The Dusk-native submission API returns an unprefixed Dusk transaction ID, while OP deposit tracking is keyed by the adapter-translated Ethereum transaction hash. The wallet now preserves those as separate identities: the native hash remains the DuskDS transaction reference, and the canonical adapter hash is resolved before invoking the SDK observer.
Adapter dependency: https://github.com/dusk-network/dusk-duskevm-adapter/pull/152 must be merged and deployed with this wallet update. Until the adapter has completed the containing block, the deposit remains in the submitted state without surfacing a false error.
A live local regression reproduced the reported transaction and proved the fixed path: Dusk transaction
11cef6c5f6990853caf1580d011ce44bb8ab4a54382b12fc78632c8215ab150bcompleted successfully, resolved to canonical L1 transaction0xfbc8c23983a18d1deef6ae97ddc3c73a662af63a04d25d6fd45a3869ea8cf45f, derived a successful L2 deposit receipt in block 5, and credited exactly 50 DUSK to the recipient. Validation after the fix: fullnpm test(129 files, 607 tests),npm run lint, andnpm run typecheck(0 errors/warnings).