Repository navigation
fix(bridging-widget): correct chain-aware bridge flow and history - #266
edehvictor wants to merge 9 commits into
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Submission preparation can outlive account changes, and failed history refreshes can prevent subsequent polling results from updating pending entries.
3 open issues
What changed in this PR
Updates the bridging widget from #251 to address #257’s chain-aware amounts, wallet selection, quote freshness, and history requirements.
Changes:
- Uses chain-specific G$ decimals and displays buffered provider fees.
- Separates source selection from the wallet network and refreshes quotes before submission.
- Revises history polling and expands Storybook fixtures, flow checks, and screenshot capture.
| File | Description |
|---|---|
| tests/widgets/bridging-widget/states.spec.ts | Adds decimal and interactive-flow checks; improves screenshots. |
| packages/bridging-widget/tests/historyTransactions.test.mjs | Tests history pairing and status-failure handling. |
| packages/bridging-widget/src/widgetRuntimeContract.ts | Separates wallet and selected-source state. |
| packages/bridging-widget/src/components/shared.tsx | Lists supported chains in protocol labels. |
| packages/bridging-widget/src/components/ReviewScreen.tsx | Displays chain-aware amounts and charged fees. |
| packages/bridging-widget/src/components/HistoryTab.tsx | Formats history amounts and preserves entries on refresh errors. |
| packages/bridging-widget/src/components/FeeBreakdown.tsx | Uses destination decimals and buffered provider fees. |
| packages/bridging-widget/src/components/BridgingWidgetView.tsx | Keeps bridge UI visible and adds wallet controls. |
| packages/bridging-widget/src/components/BridgeTab.tsx | Adds source selection and clearer connection states. |
| packages/bridging-widget/src/components/BridgeHistoryDetailModal.tsx | Formats history-detail amounts by source chain. |
| packages/bridging-widget/src/components/AmountEntry.tsx | Updates balance formatting and disconnected display. |
| packages/bridging-widget/src/BridgingWidget.tsx | Supplies preview wallet identity overrides. |
| packages/bridging-widget/src/adapter/useBridgeSubmission.ts | Loads fresh quotes and guards submission callbacks. |
| packages/bridging-widget/src/adapter/useBridgeQuote.ts | Parses source decimals and hides stale quotes. |
| packages/bridging-widget/src/adapter/useBridgeHistory.ts | Separates pending-status polling from history scans. |
| packages/bridging-widget/src/adapter/useBridgeConfig.ts | Hides config from previous routes or accounts. |
| packages/bridging-widget/src/adapter/historyTransactions.ts | Extracts history normalization and retains unresolved entries. |
| packages/bridging-widget/src/adapter/chains.ts | Adds chain-decimal and native-fee helpers. |
| packages/bridging-widget/src/adapter.ts | Coordinates source selection, resets, and fresh-quote submission. |
| examples/storybook/src/stories/helpers/bridgingWidgetStories.tsx | Updates fixtures and adds decimal and interactive scenarios. |
| examples/storybook/src/stories/bridging-widget/BridgingWidgetQA.stories.tsx | Exposes the new QA stories. |
Review effort: Balanced
| setSelectedHistoryTransaction(null) | ||
| submission.reset() | ||
| }, [isReady]) | ||
| if (submission.step === 'idle' || submission.step === 'failed') submission.reset() |
There was a problem hiding this comment.
Thanks, fixed in f53387c. Account or source changes now invalidate in-flight submission preparation, and the captured operation/account is checked before doBridge(). Once submission has started, the transaction progress is preserved. Added regressions for account changes during the switch, source changes during the fresh quote, and account changes after submission begins.
| active = false | ||
| clearInterval(intervalId) | ||
| } | ||
| }, [sdk, address, state.transactions]) |
There was a problem hiding this comment.
Thanks, fixed in f53387c. Pending-status polling now follows each history scan's request identity, so a failed refresh cannot leave later status updates discarded. Added a regression that fails a refresh while a transfer is pending and then verifies that delivery still updates.
| test('BridgingWidget completes the interactive route-to-submit flow', async ({ page }) => { | ||
| await gotoStory(page, 'interactive-submit') |
There was a problem hiding this comment.
Agreed. I kept the presentation test and added a real-adapter Storybook fixture plus Playwright flow in f53387c, mocking only the wallet/SDK boundaries. It asserts confirmation switches to the selected source, obtains a fresh quote, and passes that quote to doBridge(). The focused bridge suite passed 27/27 locally without retries; the committed recording is scripted, not a real signed transfer.
|
@edehvictor A review can only be done when the following conditions are satisfied:
|
|
@pheobeayo, thank you. I'm still working on it. When I'm done. I'll let you know |
|
@edehvictor @pheobeayo lets try to keep hygiene around the stages of how we work on bounties.
Only when the initial checks are met, confirmed by bounty-lead, should turn this into a ready for review PR and the group of reviewers can be assigned. |
|
@edehvictor per request, below the related design references. you can see in other widgets how its used and applied. Main bridge screen:
Notes:
Confirm transaction screen:
Notes:
Transaction confirmed:
Notes:
|
|
Update on draft PR #266: pushed f53387c (the three Copilot fixes and regression coverage) and 23e7f7f (Review actions reachable on short mobile screens). I replied on each Copilot thread with the specific fix and test. Local verification: bridging-widget lint passed; focused Playwright suite passed 27/27, one worker, no retries. Evidence: scripted real-adapter happy-path recording, result screenshot, and 390 x 504 Review-footer screenshot. The mobile drawer was also manually checked for scrolling. The recording uses mocked wallet/SDK boundaries; it is not a signed live transfer. Live wallet fees/delivery and a reference-versus-result design sign-off remain open. Full-workspace build/lint are not certified because of unrelated existing errors, detailed in the updated PR description. Current CI has a waiting test authorization and a skipped Playwright job. I'm keeping this PR draft, as requested, and am not requesting review. |
|
@pheobeayo @L03TJ3 I compared Lewis's three references with the current result while keeping #266 draft. The #257 correctness flow is verified locally (27/27 focused Playwright tests; bridging-widget lint passes), but I do not want to claim visual parity yet.
Could you confirm whether these remaining design/feature differences belong in this #257 follow-up, the parent #251 feature work, or a separate issue? I'll keep #266 draft and avoid broad UI changes until you confirm scope. The scripted happy-path video mocks wallet/SDK boundaries; no funded transfer was made. |
|
@pheobeayo, kindly review |
|
@edehvictor Thanks for the thorough update and for closing out the Copilot threads. Before this can go to a review:
|
|
@edehvictor Thanks — 42f6243 fixes the history decimals correctly. Formatting at 18 decimals matches the SDK’s normalizedAmount, and the fixture and test updates look good. I ran the unit tests and lint locally on the new head; both pass.
|
|
@pheobeayo Thanks for the review. I’ve added the three reference/result image pairs directly to the PR description and kept the remaining visual differences explicit. The latest local bridge Playwright run passed 27/27 with one worker and no retries, but the external-fork Playwright check has not run in CI. Could you authorize or trigger that workflow (or point me to whoever can)? I also fixed the optional route-limit fixture inconsistency in 4873552: both Storybook fixtures now use the SDK’s normalized 18-decimal limits, while token balances remain chain-specific. The commit is pushed; changed-file lint and the focused bridge Playwright suite passed locally (27/27, one worker, no retries) after this fix. For the requested real-wallet recording through Review → Confirm → wallet signing, the current widget routes are configured for mainnet. Could you confirm a supported testnet setup, or provide a small test allowance for G$ and source-chain gas so I can record the flow safely with a separate test account? I will review and sign the wallet request myself. The existing scripted video uses mocked wallet/SDK boundaries and is not a live transfer. I’ll keep the PR draft until CI and the manual evidence are ready, then ask for your go-ahead before marking it ready for review. |
|
@edehvictor Thanks — the limit fixtures are now correctly normalized, and the Ethereum RPC fix with its explanatory comment makes sense. I ran the unit tests (5/5) and bridging-widget lint on 4873552; both pass. One question on 8d8f242: removing the revert rule fixes the misleading message for read-call reverts, but it also drops it for actual doBridge reverts, which now show only “Bridge transaction failed.” Was that intended, or should the revert hint stay for the submission path only? @L03TJ3 is this good enough for review now? |
|
@pheobeayo Thanks for catching that. Dropping the hint for an actual |
|
@edehvictor we requested a happy-path video alongside automated playwright tests screenshots. The request is: test it yourself. manually. make a video. but all of this will end up being used by real users. so we need a critical review by the developer to make sure things work. not based only on tests. |
|
Hi @L03TJ3, |
|
@edehvictor
|
@L03TJ3 I completed the requested **developer-run manual wallet test**, separate from the earlier Playwright recording. Here is the [Loom recording](https://www.loom.com/share/94776e052d0548f0b68891098fc6b154): I used a funded MetaMask test account for a 50 G$ Celo -> Ethereum bridge via Axelar, went through Review and Confirm, and observed the source submission.
The recording captured the browser tab only, so it does not show the separate MetaMask confirmation popup, and it ends while delivery is still in progress. I don't want to imply otherwise. The public records confirm the actual signed transfer and later delivery: Celo source transaction succeeded, Axelar reports executed, and the Ethereum destination transaction succeeded with 49.95 G$ delivered to the test account. I updated the PR description to separate this live evidence from the mocked fixture screenshots. Could you confirm whether this evidence meets the manual-test requirement despite the popup not appearing in the tab recording? The PR remains draft. Upstream Playwright CI is still skipped on the latest head, so maintainer authorization/a green CI run is also outstanding. |










Summary
Follow-up to #251 for issue #257. This draft PR makes the bridge flow chain-aware and addresses review feedback in the current branch.
Issue #257 checklist
The checked items below describe implementation and local QA. The separate funded manual check is documented below.
Verification
882534awith one worker and no retries.Manual live-wallet check
Reference versus result
These compare the lead's designs with the current
882534aUI captured in deterministic Storybook/Playwright fixtures. Routes, amounts, fees, and delivery states differ between reference and result. Fixture fees are mocked; these images do not establish live fees or pixel parity.Mobile access checks: Review actions and progress tracking and background action at 390 x 504.
Verification status
pnpm buildandpnpm lintremain uncertified in this sandbox: its pnpm 11/package-linking restriction blocks the repository's pnpm 9.15 entrypoints. Direct UI build reports an unrelatedTS2322inpackages/ui/src/components-test/Select.tsx; direct workspace lint reports an unrelated unused governance import. Bridging-widget lint passes.playwright-demowas skipped andauthorize-testsis waiting for maintainer authorization. Local Playwright success does not substitute for green CI.