Skip to content

Improve Reserve swap feedback and RPC resilience; upgrade to Node 24 - #645

Draft
L03TJ3 wants to merge 7 commits into
masterfrom
reserve-and-rpc-fixes
Draft

L03TJ3 wants to merge 7 commits into
masterfrom
reserve-and-rpc-fixes

Conversation

@L03TJ3

@L03TJ3 L03TJ3 commented Oct 12, 2026 •

Copy link
Copy Markdown
Collaborator

Description

Aligned with upcoming sdk updates from this PR: GoodDollar/GoodWeb3-Mono#274

Reserve and mobile UI

  • Consume structured Reserve limit errors and the shared good-design error modal.
  • Show alternative-chain availability only when the network/Reserve feature is enabled and a fresh SDK check confirms availability; refresh checks every 15 seconds.
  • Handle both Exception and Fail transaction states for swaps and approvals, closing failed swap confirmations.
  • Remove mobile drag-to-dismiss from the shared modal while retaining its layout and opacity fade.

RPC handling

  • Use bounded fallback read-provider pools for SDK-v2 on Fuse/Celo, preserving SDK-v1’s scalar RPC configuration.
  • Apply RPC exclusions consistently across discovered, configured, cached and fallback endpoints.
  • Add regression coverage for HTTP 429 failover, exhausted providers, exclusions and provider compatibility.

Runtime

  • Upgrade CI/deployment workflows, .nvmrc, engine requirements and setup documentation to Node 24.

Dependencies and validation

  • Requires the related GoodWeb3-Mono Reserve/bridge changes and updated SDK/design packages. Replace temporary local package references before merge.
  • Read-only review completed; tests, lint and builds were not rerun against this final change-set.

Summary by Sourcery

Improve Reserve swap failure feedback and RPC resilience while standardizing the project on Node.js 24.

New Features:

  • Add structured Reserve-limit feedback with a shared error modal, supported alternative-chain guidance, and Celo DEX route links.
  • Add development-only Reserve-limit QA simulation for validating SDK error handling without sending transactions.

Bug Fixes:

  • Handle failed approvals and swaps across both Exception and Fail transaction states, including closing failed confirmations.
  • Prevent stale or unsupported alternative-chain suggestions by requiring enabled features and fresh availability checks.
  • Improve RPC failover for HTTP 429 responses and exhausted provider pools.

Enhancements:

  • Use bounded SDK-v2 read-provider pools for Fuse and Celo while preserving SDK-v1 RPC compatibility.
  • Apply configured RPC exclusions consistently to discovered, cached, configured, and fallback endpoints.
  • Remove mobile drag-to-dismiss behavior from the shared modal while retaining its fade transition.

Build:

  • Upgrade project runtime and package requirements to Node.js 24 with repository-pinned Yarn.
  • Update CI and deployment workflows to current action versions, Node 24, Corepack, immutable installs, and modern runners.

Documentation:

  • Document Node 24 setup and the development-only Reserve-limit QA simulation workflow.

Tests:

  • Add coverage for RPC exclusions, provider compatibility, HTTP 429 failover, exhausted providers, Reserve-limit classification, and QA-mode gating.

Chores:

  • Update SDK and design package references to the local Yalc-based packages required by the paired upstream changes.

L03TJ3 added 2 commits October 5, 2026 18:32
- Integrate the shared Reserve limit modal and verified alternative-chain messaging.
- Handle failed swap/approval states and remove the broken mobile modal drag behavior.
- Add bounded Fuse/Celo read-RPC failover and consistent RPC exclusion filtering.
- Add provider and RPC regression tests.

L03TJ3 commented Oct 20, 2026

Copy link
Copy Markdown
Collaborator Author

@copilot please add the ProtocolUI entry point for the minimal Reserve-limit QA override requested on GoodDollar/GoodWeb3-Mono#274. QA task: GoodDollar/GoodWeb3-Mono#275. Preserve this PR's existing RPC and Reserve changes.

Paired API contract requested from the SDK: optional qa?: { forceTradeLimits?: boolean } on Web3Provider / its context, default off.

  • Wire REACT_APP_QA_FORCE_TRADE_LIMITS=true through src/hooks/useWeb3.tsx to that SDK option. This app uses Vite with the existing REACT_APP_ convention. Gate on development build explicitly; never activate in a production build, and do not confuse build mode with the production/mainnet contract deployment env.
  • In src/pages/gd/Swap/SwapCore/mentoReserve.tsx, provide a visibly labelled QA simulation path through existing swap confirmation to the SDK send/error classifier and existing limit modal. An empty wallet must be able to use this without funding or approval. Only the labelled QA path may bypass balance/allowance gating; do not fake displayed balances or approval success. Keep the valid quote/account/network prerequisites explicit.
  • Disable/block real Reserve approval and submission while this simulation is active. Fail closed if the installed SDK lacks or does not activate the QA capability: an old/inactive SDK must never turn a UI gate bypass into a live send. Coordinate a small effective-mode/capability signal with Mono#274 if needed.
  • The SDK must inject the limit error before signer access. Do not open a hardcoded modal in place of exercising the existing error path. Verify confirmation closes, correct Celo/XDC message and links appear (Celo Uniswap/Ubeswap; XDC XSwap), and close/reopen works without stale errors.
  • Add focused tests for empty-wallet reachability, safe SDK wiring/no real approval or send, off/production behaviour, and classifier-to-modal behaviour. Verify the modal at a mobile viewport such as 375x812 for text/link/close-button overflow. This is layout verification, not mobile transaction QA. Report actual test/check commands and any checks not run.
  • Provide exact enable/disable/restart instructions and prerequisites for the QA tester. Use the existing yalc setup from Chore: QA release #275; package publication is not a QA prerequisite. No tester code patches or general mock harness.

Keep scope limited to forced Reserve-limit handling; do not claim this verifies real trade limits, mined receipts or delegated-wallet transaction execution.

Co-authored-by: L03TJ3 <6606028+L03TJ3@users.noreply.github.com>
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Oct 20, 2026 •

Copy link
Copy Markdown

Deploying goodprotocolui with  Cloudflare Pages  Cloudflare Pages

Latest commit: ac50050
Status:🚫  Build failed.

View logs

Co-authored-by: L03TJ3 <6606028+L03TJ3@users.noreply.github.com>

L03TJ3 commented Oct 20, 2026

Copy link
Copy Markdown
Collaborator Author

@copilot focused review follow-up on eccc37b: the documented .env.local setup currently does not match the configuration reads.

src/hooks/useWeb3.tsx:249 and src/pages/gd/Swap/SwapCore/mentoReserve.tsx:72 read process.env.REACT_APP_QA_FORCE_TRADE_LIMITS. But vite.config.ts:41 calls loadEnv(mode, process.cwd()) with its default VITE_ prefix, while dotenv.config() loads .env, not .env.local. The existing envPrefix: 'REACT_APP_' exposes these variables through import.meta.env, not that manually defined process.env object.

Please use import.meta.env.REACT_APP_QA_FORCE_TRADE_LIMITS consistently (or explicitly load the proper prefix), keeping the development guard. Verify the exact documented .env.local + restart workflow actually activates the paired SDK context and labelled empty-wallet simulation, and that false/absent/production cannot activate it.

The added helper tests are useful, but do not yet render the confirmation or modal. Please add/perform the requested integration verification: empty wallet -> labelled confirmation -> actual SDK limit classification -> confirmation closes -> Celo/XDC modal and correct DEX links -> close/reopen. Ensure no signer/send/approval calls; check SDK capability absent/inactive fails closed. Also report the 375x812 layout check (or explicitly mark it not run). Keep the accepted yalc setup; no npm release gate. Report exact test commands/results and any environment blockers rather than claiming helper tests establish these UI outcomes.

Copilot AI commented Oct 20, 2026

Copy link
Copy Markdown
Contributor

@copilot please add the ProtocolUI entry point for the minimal Reserve-limit QA override requested on GoodDollar/GoodWeb3-Mono#274. QA task: GoodDollar/GoodWeb3-Mono#275. Pre...

Implemented in 9c2e54a (helper move in eccc37b): dev-only SDK wiring, effective-capability/modal fail-closed gating, a labelled confirmation simulation, real-action guards, focused tests, and yalc setup instructions. QA stays unavailable until Mono #274 implements the qa.forceTradeLimits effective signal and injects the classified error before signer access. Standalone helper typecheck/assertions and secret scanning passed. Jest, lint, build, and the 375×812 layout check were not run: this checkout has no install state/yalc packages and runs Node 22 instead of the required Node 24.

Co-authored-by: L03TJ3 <6606028+L03TJ3@users.noreply.github.com>

Copilot AI commented Oct 20, 2026

Copy link
Copy Markdown
Contributor

@copilot focused review follow-up on eccc37b: the documented .env.local setup currently does not match the configuration reads.

src/hooks/useWeb3.tsx:249 and `src/pages/gd/Swap/SwapCore/mentoRese...

Fixed in 6d539fe: both flag reads now use import.meta.env.REACT_APP_QA_FORCE_TRADE_LIMITS; the development-only guard remains, and false/absent cases are covered. The exact flag/capability gates passed standalone typecheck/assertions and the documented .env.local key aligns with Vite’s REACT_APP_ env prefix. Rendered confirmation-to-modal/SDK integration and 375×812 checks remain unverified: this checkout lacks Node 24, install state, yalc packages, and the QA-capable SDK. Focused Jest could not start because Yarn reports no install state.

L03TJ3 commented Oct 20, 2026

Copy link
Copy Markdown
Collaborator Author

@copilot a concrete build blocker from paired local validation, confirmed in current head 6d539fe: src/hooks/useWeb3.tsx opens <QaAwareWeb3Provider> at line 264 but closes </Web3Provider> at line 276. Please correct the closing tag to match the opening component.

The paired local typecheck and Vite build both catch this JSX error. The 18 focused UI tests pass but do not compile this provider, so they did not catch it. Add a focused compile/typecheck safeguard covering the actual provider wiring, or otherwise ensure that file is included in the validation you run; report the exact check and result. Keep this correction small and preserve the accepted yalc workflow and existing changes.

For clarity, the separate clean-install/lockfile and yalc-package setup issues are not functional QA results. They do not explain away this source-level JSX error. The QA issue remains pending paired validation; do not claim the app is ready based only on helper tests.

Co-authored-by: L03TJ3 <6606028+L03TJ3@users.noreply.github.com>

Copilot AI commented Oct 20, 2026

Copy link
Copy Markdown
Contributor

@copilot a concrete build blocker from paired local validation, confirmed in current head 6d539fe: src/hooks/useWeb3.tsx opens <QaAwareWeb3Provider> at line 264 but closes </Web3Provider> at l...

Fixed in af93662: the provider now closes with </QaAwareWeb3Provider>. Added src/hooks/useWeb3.test.ts, which transpiles the actual provider source and checks tag pairing and QA prop wiring. The focused test command (yarn test --runInBand src/hooks/useWeb3.test.ts) could not start because there is no Yarn install state. Parsing the provider with tsc --ignoreConfig --noEmit --jsx react-jsx --target ESNext --module ESNext --moduleResolution bundler --skipLibCheck --pretty false src/hooks/useWeb3.tsx produced no JSX/syntax diagnostics; remaining errors are unresolved project dependencies/types. I could not inspect Actions logs because the GitHub Actions tools are unavailable in this session.

L03TJ3 commented Oct 20, 2026

Copy link
Copy Markdown
Collaborator Author

@copilot one small simplification following the maintainer's clarified lifecycle: QA uses the updated Mono SDK through yalc; after validation the SDKs will be published and ProtocolUI upgraded before merge. Compatibility with older SDK types is not required for this change.

Current af93662 fixes the JSX mismatch but retains QaAwareWeb3Provider, a type-cast alias of the existing provider. Please remove that alias and use the existing Web3Provider directly with its new SDK-typed qa={{ forceTradeLimits: ... }} prop, with matching JSX tags. Update the new provider test accordingly. Keep the development/default-off and no-signing safeguards, and the straightforward boolean pass-through; no additional provider abstraction or old-SDK compatibility workaround is needed.

Validate against the paired updated SDK via the documented yalc workflow. An unreleased registry SDK or CI checkout lacking yalc packages is expected at this stage and is not itself a QA blocker; report actual source errors separately. No broader refactor.

Co-authored-by: L03TJ3 <6606028+L03TJ3@users.noreply.github.com>

Copilot AI commented Oct 20, 2026

Copy link
Copy Markdown
Contributor

@copilot one small simplification following the maintainer's clarified lifecycle: QA uses the updated Mono SDK through yalc; after validation the SDKs will be published and ProtocolUI upgraded before ...

Implemented in ac50050: removed the type-cast alias and now pass qa={{ forceTradeLimits: forceTradeLimitsQa }} directly to the SDK-typed <Web3Provider>, with matching closing tag. Updated the provider source test to assert direct tag and prop wiring. tsc parsing found no JSX/syntax diagnostics; the Jest test and paired SDK typecheck could not run because this checkout has no node_modules/yalc package and only Node 22. git diff --check and secret scanning passed.

This branch had an error being deployed

1 failed (outdated) and 1 active (outdated) deployments
preview — 4191c80f Deployed Oct 12, 2026 by L03TJ3 via preview #1071
internal — 4191c80f Deployed Oct 12, 2026 by L03TJ3 via authorize #1071
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants