Skip to content

[GoodBounty] Complete external-wallet login in GoodWallet #76

Open

Description

@L03TJ3

Summary

Finish PR #72: Reown existing-wallet login, rather than rebuild it.

Already implemented: a “Connect Wallet” login option opening Reown, EVM-only accounts, wallet-submitted transactions, session restoration, and Options → Logout. External account changes/disconnection end the session and return to login with a reconnect message. These flows need verification, not a new design.

Remaining work: prevent old account data/permissions surviving account changes, coordinate network switching, handle replaced transactions, and correctly reject unsupported dApp connections.

Baseline: feat-connect-wallet-reown. (the target branch for the PR)
Contributor bounty: Mythic

Contributor task

  • Request assignment with an ETA. Run the branch locally and reproduce the issues below.
  • Use Node 24 and pinned Yarn 4.17.0. Configure a test environment with a valid NEXT_PUBLIC_WALLET_CONNECT_PROJECT_ID and NEXT_PUBLIC_LOGIN_EXISTING_WALLET_ENABLED=true.
  • Fix the four areas below, add regression tests, and provide a reviewable PR with evidence.
  • Preserve social login and the existing reconnect-on-account-change behaviour. No account linking, new account switcher, dashboard redesign or non-EVM support is required.
  • Keep the production flag off; merging, deployment and production enablement are outside this bounty.

Scope checks

1. Keep each account’s permissions and data separate

Problem: connections from other dApps to GoodWallet can survive social/private-key logout and later use a different active account. Transaction requests also overwrite a supplied sender instead of rejecting a mismatch. History is not cleared or keyed by account; its ten-minute refresh delay can retain the previous account’s activity. Balances and swap clients have reset paths, but unfinished requests need protection against writing stale results.

Done when: logout/account changes invalidate the old signing account and incompatible dApp connections. Requests for account A cannot be silently signed as B. New sessions show only their own data, including when earlier requests finish late.

Verify: connect a dApp as A, logout, then login as B within ten minutes. Check activity/balances and reject requests still addressed to A. Repeat with delayed balance/history/swap initialization.

Sources: session permissions, sender validation, history store.

2. Avoid competing network switches

Problem: parallel claims can switch the same external wallet between networks while another transaction is being prepared.

Done when: network switching and signing/submission are coordinated across claims and other wallet operations. Each action uses its intended account/network; rejection leaves the interface usable.

Verify: start Celo and XDC claims together; delay or reject one switch. Check transaction destinations and that neither action remains stuck. Cover overlapping signing/swap operations with deterministic tests. Review finding.

3. Finish transaction tracking

Problem: tracking only the original transaction hash can leave Send/Claim pending after the user speeds up or cancels in their wallet.

Done when: success, revert, replacement and cancellation have accurate outcomes. Temporary RPC uncertainty preserves the submitted hash and does not trigger a duplicate send. Preserve receipt and Divvi tracking.

Verify: test mined success, revert, speed-up, cancellation and delayed RPC visibility; check final status, hash and activity entries. Review finding.

4. Complete connection feedback and compatibility checks

Problem: unsupported required chains produce a local error but leave the requesting dApp waiting. Proposal compatibility must also be checked against the methods available for the active wallet/session. External disconnection also uses the “account changed” message.

Done when: unsupported required chains or methods receive a proper rejection; unsupported optional chains or methods can be omitted. Advertise and approve only methods actually supported by the active signer/session. Disconnect/account-change messages describe what happened, and Connect Wallet remains usable for retry.

Verify: pair an EVM dApp and one requiring an unsupported chain. Test supported methods, unsupported required methods, and a mix of supported and unsupported optional methods. Confirm the requesting dApp receives a rejection for unsupported requirements rather than waiting, approved namespaces contain only supported methods, and advertised methods can be executed. Include the existing external-wallet restrictions on raw transaction signing/sending and EIP-5792. Test connect/cancel/retry, reload/restore, Options → Logout, external disconnect and account change. Check feature-flag-off login fallback. Review finding.

GoodID’s empty-name acceptance and mobile-wallet handoff are compatibility checks, not confirmed defects. Use the PR’s existing human QA guide now; retest affected flows after fixes. Cover English/Danish and desktop/mobile. Record blocked checks honestly. Existing private-key screenshots do not prove external-wallet compatibility.

Required commands

yarn install --immutable
yarn biome:verify:all
yarn build
yarn tsc --noEmit
yarn test
yarn test:e2e

For interactive testing: yarn dev. It runs drizzle-kit push: use an isolated test database. Playwright starts its own server on port 3000.

Add runnable external-wallet/provider mocks for failure cases. Separately record actual-wallet checks using approved test funds, never production balances. Explain failed/unrun commands; mocks are not live-wallet evidence.

Handoff comment

Contributor: @handle
PR / final commit:
Fixed: list each scope area and linked review finding
Verified: commands, scenarios and results; distinguish mocks from actual wallets
Evidence: test paths, screenshots/recording, CI and wallet/browser versions
Remaining: unresolved issues or blocked checks, reason and next step

Activity

  1. L03TJ3 commented on Oct 23, 2026

    @L03TJ3
    ContributorAuthor

    @sirpy assigned you and please give an update when the local follow-ups are pushed

  2. moved this from To be organized to Ready for Assignment in GoodBountieson Nov 3, 2026
  3. changed the title [-][DRAFT] Complete external-wallet login in GoodWallet[/-] [+][GoodBounty] Complete external-wallet login in GoodWallet[/+] on Nov 3, 2026
  4. Ryjen1 commented on Nov 3, 2026

    @Ryjen1
    Collaborator

    Hello team, I’d like to take on this bounty

    I’ve reviewed the issue and understand the goal is to complete the existing work on PR #72 (feat-connect-wallet-reown branch), not rebuild external-wallet login from scratch.

    Plan:

    1. Run the feat-connect-wallet-reown branch locally (Node 24 + Yarn 4.17.0), set up the Reown test environment, and reproduce the open issues.
    2. Address the remaining scope areas:
      • Account data & permission isolation
      • Coordinated network switching / serialization of wallet network switches + tx submission
      • Replacement / cancelled transaction tracking
      • dApp connection compatibility (reject unsupported-chain proposals, disconnect incompatible sessions on account change, preserve requested sender)
    3. Add regression tests and submit a clean, reviewable PR with verification commands, test evidence, and any blocked checks clearly reported.
    4. Preserve existing Google / Facebook / SMS (Web3Auth) login flows.

    ETA: 7–10 working days from assignment, with an initial reproduction / status update within 24 hours.

    Please assign this issue to me if it’s still available. Thanks!

  5. Godbrand0 commented on Nov 3, 2026

    @Godbrand0

    Hello, I would first run the feat-connect-wallet-reown branch locally using Node 24 and Yarn 4.17.0, configure the Reown test environment, and reproduce the reported issues.

    I’ll then investigate session isolation, network-switch coordination, transaction replacement tracking, and dApp connection compatibility before implementing fixes and regression tests. I’ll preserve the existing login flows and provide a reviewable PR with verification evidence.

    ETA: 20 hours.

    Please assign me.

  6. HushLuxe commented on Nov 3, 2026

    @HushLuxe

    Hello, I would like to request assignment for this bounty.

    I have read the issue and understand that the task is to complete PR #72 from the feat-connect-wallet-reown branch, rather than rebuild the external-wallet login. I will first run the branch locally using Node 24 and Yarn 4.17.0, set up the specified test environment, and reproduce the issues across the four scope areas: account data and permission isolation, coordinated network switching, replacement-transaction tracking, and dApp compatibility feedback.

    I will add regression tests and submit a reviewable PR with the required verification commands, test evidence, and any blocked checks clearly reported.

    ETA: 10 working days from assignment, with an initial reproduction update within 24 hours.

    Please assign this issue to me if it is still available. Thank you.

  7. edehvictor commented on Nov 3, 2026

    @edehvictor

    Claim: Contributor Role
    Contributor: @edehvictor
    ETA: 7-10 working days from assignment, with an initial reproduction update within 24 hours

    I’ve reviewed #76 and the existing work in PR #72. I recently worked on GoodWidget’s bridge correctness issue #257 in PR #266, covering account and source-chain changes, fresh quotes, wallet-switch rejection and Retry, and transaction progress. The bridge suite passed 38/38 locally, and I documented a separate funded wallet test rather than presenting mocks as live evidence. That experience is relevant here, though I’ll reproduce GoodWallet’s Reown-specific issues before changing them.

    Plan

    • Run feat-connect-wallet-reown locally with Node 24, Yarn 4.17.0, the required feature flags, and an isolated test database.
    • Audit PR Add Reown existing-wallet login alongside Web3Auth #72 against issue [GoodBounty] Complete external-wallet login in GoodWallet #76 and reproduce the reported failures.
    • Address account/session isolation, competing network switches, transaction replacements, and unsupported dApp connection requests.
    • Add regression tests, run the issue’s required Yarn commands, and distinguish mocked tests from actual-wallet evidence.
    • Post a handoff with results and any blocked checks. I will keep the production flag off.
      Initial Assessment
      PR Add Reown existing-wallet login alongside Web3Auth #72 already implements the external-wallet login path, so I would build on it rather than redesign it. The remaining work is in account safety, transaction outcomes, and connection compatibility. I saw the earlier assignment note; please confirm this bounty is available before assigning it to me.

    Kindly assign me @L03TJ3

  8. nomsoscript commented on Nov 3, 2026

    @nomsoscript

    Hi @L03TJ3 , would like to take on #76 if it’s still available.

    I understand the task is to complete PR #72, not rebuild the external-wallet login. My approach would be to reproduce the reported issues, then address account/session isolation, network-switch coordination, transaction replacement tracking, and dApp compatibility.

    I’ll preserve existing login flows, add regression tests, keep the production flag off, and clearly distinguish mocked tests from actual-wallet verification.

    Please let me know whether this bounty is open to assignment.

    ETA: 7-10 from assignment.

  9. robertocarlous commented on Nov 3, 2026

    @robertocarlous

    Hi @L03TJ3, I’d like to be assigned to this issue.

    Approach: I’ll finish PR #72 on top of feat-connect-wallet-reown rather than rebuild it. I’ll start by auditing the baseline against the four scope areas and posting my findings, so you can see what is verified and what is broken before I change anything.

    Account isolation: On logout or account change, invalidate the old signing account and any incompatible dApp sessions. Reject a request when the sender doesn’t match the active account instead of overwriting it. Guard history and balance fetches so a late response from a previous account can’t update the UI.
    Network switching: Serialize switch, sign and submit operations across claims and other wallet actions. A rejected switch leaves the UI usable.
    Transaction tracking: Handle success, revert, speed-up, cancel and replace. A temporary RPC failure won’t trigger a resend. The receipt and Divvi tracking stay intact.
    Connection feedback: Explicitly reject unsupported required chains or methods. Omit unsupported optional ones. Approve only supported methods. Make disconnect messages accurate, and keep Connect Wallet usable for retry.

    I’ll keep social login and the reconnect-on-account-change behavior unchanged, leave the production flag off, and stay out of the excluded scope. Each area will get regression tests, and any mocks will be labeled as mocks. The PR will be reviewable and will pass the required commands (biome, build, tsc, test, e2e). I’ll finish with a handoff comment listing fixes, verification, evidence and anything blocked.

    ETA: 3-5 working days for a reviewable PR. I’ll post a progress update after the first audit.

    Thanks for considering it.

  10. Abidoyesimze commented on Nov 3, 2026

    @Abidoyesimze

    Hi @L03TJ3 , I'd like to take this on. Please assign it to me.

    ETA: 3-4 working days (with QA evidence).

    About me: I'm a TypeScript/Node engineer working on wallet and payments infrastructure (CowryPay: transaction signing, on-chain transfers on Celo, payout flows), so session handling, nonce/replacement tracking and wallet edge cases are familiar ground.

    Plan, on top of feat-connect-wallet-reown:

    1. Account isolation: tear down dApp sessions and signer references on logout/account change; reject (rather than overwrite) a request whose from doesn't match the active account; key or clear history by account; guard balance/history/swap writes with an account/session token so late responses for A are dropped after switching to B.
    2. Network switching: serialize switch → sign → submit through a single wallet-operation queue/lock, so parallel Celo/XDC claims can't switch the chain under each other; make sure a rejected switch releases the lock and leaves the UI usable.
    3. Transaction tracking: track replacements (speed-up/cancel) rather than only the original hash; map mined/reverted/replaced/cancelled to the right final status; keep the submitted hash and avoid resends during temporary RPC misses; keep receipt and Divvi tracking intact.
    4. Connection compatibility: reject proposals with unsupported required chains/methods, drop unsupported optional ones, and approve only methods the active signer supports (including the existing raw-signing and EIP-5792 restrictions); separate disconnect vs account-change messaging and keep Connect Wallet retryable.

    I'll add provider/wallet mocks and regression tests for each area, run the full required command set, and follow the existing QA guide (EN/DA, desktop/mobile). I'll keep mock results clearly separate from real-wallet checks done with test funds, and record any blocked checks honestly in the handoff comment.

    The production flag stays off; social login and reconnect-on-account-change will be preserved.

  11. Yormee-103 commented on Nov 3, 2026

    @Yormee-103

    Hi @L03TJ3, I'd like to request assignment for this bounty.

    I've reviewed issue #76 and understand that the goal is to complete the existing Reown external-wallet login implementation in PR #72, rather than rebuild the authentication flow.

    My approach would be to:

    1. Set up the "feat-connect-wallet-reown" branch using Node 24 and Yarn 4.17.0, configure the required test environment and reproduce the reported issues.
    2. Investigate account/session isolation, ensuring old permissions and asynchronous requests cannot affect a newly authenticated account.
    3. Coordinate wallet network switching and transaction operations to prevent competing requests from interfering with each other.
    4. Implement accurate transaction replacement and cancellation tracking while preserving receipt and Divvi tracking.
    5. Fix dApp compatibility checks, ensuring unsupported required chains and methods receive proper rejections.

    I'll add regression tests for the affected areas, preserve existing social login functionality, and keep the production feature flag disabled.

    I'll also follow the existing QA guide and distinguish mocked test results from actual-wallet verification.

    ETA: 7–10 working days from assignment, with an initial reproduction and progress update within 24 hours.

    I'll follow the repository's contributing guidelines and provide a reviewable PR with verification results, evidence and any outstanding limitations.

    Please assign me if the bounty is still available. Thank you!

  12. moved this from Ready for Assignment to In Progress in GoodBountieson Nov 3, 2026
  13. L03TJ3 commented on Nov 3, 2026

    @L03TJ3
    ContributorAuthor

    Okay, you are assigned! @Ryjen1
    once you start please reach out on telegram so I can share potential needed environment variables to be able to run the wallet locally

  14. Ryjen1 commented on Nov 3, 2026

    @Ryjen1
    Collaborator

    Thank you, @L03TJ3 - assignment acknowledged. I’ll reach out on Telegram once I'm about to start.

  15. Abuchtela commented on Nov 5, 2026

    @Abuchtela

    Claim: Contributor Role
    Contributor: @Abuchtela
    ETA: update within 24/48 hours

    I’ve reviewed #76 and the existing work in PR #72. The bridge suite passed 38/38 locally, and I documented a separate funded wallet test rather than presenting mocks as live evidence. That experience is relevant here, though I’ll reproduce GoodWallet’s Reown-specific issues before changing them.

    Plan

    Run feat-connect-wallet-reown locally with Node 24, Yarn 4.17.0, the required feature flags, and an isolated test database.
    Audit PR Add Reown existing-wallet login alongside Web3Auth #72 against issue [GoodBounty] Complete external-wallet login in GoodWallet #76 and reproduce the reported failures.
    Address account/session isolation, competing network switches, transaction replacements, and unsupported dApp connection requests.
    Add regression tests, run the issue’s required Yarn commands, and distinguish mocked tests from actual-wallet evidence.
    Post a handoff with results and any blocked checks. I will keep the production flag off.
    Initial Assessment
    PR Add Reown existing-wallet login alongside Web3Auth #72 already implements the external-wallet login path, so I would build on it rather than redesign it. The remaining work is in account safety, transaction outcomes, and connection compatibility.
    Kindly assign me @L03TJ3

  16. Ryjen1 commented on Nov 5, 2026

    @Ryjen1
    Collaborator

    Contributor: @Ryjen1
    PR / final commit: [https://github.com//pull/78] — final commit e652cf4

    Fixed:

    Scope 1 — Keep each account's permissions and data separate

    • Activity history cache is now keyed by active account and purged on account change/logout. A session token set at login gates every async refresh; callbacks that arrive after the token changes are discarded, so a slow history or balance load from account A cannot populate account B's view. transactionHistoryStore, amountsStore. (b1e0300, acc1672)
    • WalletWrapper validates the dApp-supplied from address against the connected account using a case-insensitive helper (isSameAddress) so that EIP-55 casing variants are accepted while genuinely different addresses are rejected. (b1e0300, 46a9753)
    • All logout paths — social, private-key, external disconnect, and account change — now call disconnectDappSessions() before clearing state, so no dApp connection can survive a session boundary. storage.ts. (4e0f486)
    • Incompatible restored WalletConnect sessions (wrong account or unsupported namespace) are pruned inside initializeWalletConnect before the signer is marked usable. walletConnectStore.ts. (141d918, 60926a9)
      Review finding addressed: Add Reown existing-wallet login alongside Web3Auth #72 (comment), Add Reown existing-wallet login alongside Web3Auth #72 (comment)

    Scope 2 — Avoid competing network switches

    • A chainOpLock mutex serializes ensureChain, submitTransaction, signTypedData, signMessage, and getReownEip1193Provider. Concurrent claims on different networks queue instead of racing. The lock is always released in a finally block so a failed or rejected switch does not leave the interface stuck. reown.ts. (4e0f486, 89db1b3)
    • watchReownAccount is split into onAccountChanged and onDisconnected callbacks so external account changes and disconnections produce distinct messages and behaviours. (4e0f486)
      Review finding addressed: Add Reown existing-wallet login alongside Web3Auth #72 (comment)

    Scope 3 — Finish transaction tracking

    • External-wallet submissions now use provider.waitForTransaction with replaceableTransaction(hash, startBlock), enabling ethers to follow speed-ups and cancellations through the mempool. utils.ts. (ab4e1df)
    • startBlock is captured by getBlockNumber before submitTransaction is called, so a fast replacement cannot be mined before the tracking window opens. (cd22844)
    • TRANSACTION_REPLACED handling branches on err.reason:
      • "repriced" (speed-up): replacement receipt is used; Divvi referral tracking proceeds on the replacement hash — original intent is preserved.
      • "cancelled" / "replaced" (different intent): error is rethrown; the caller's error path surfaces the failure, submitReferral is not reached, and no false-success analytics fire. (e652cf4)
    • Transient RPC getTransaction failures (null result) are retried up to 40 times at 1.5 s intervals before giving up. (ab4e1df)
    • Divvi referral tracking is preserved for mined and repriced outcomes; skipped for cancelled and replaced. (ab4e1df, e652cf4)
      Review finding addressed: Add Reown existing-wallet login alongside Web3Auth #72 (comment)

    Scope 4 — Complete connection feedback and compatibility checks

    • RequestHandler now inspects required namespaces in every session proposal. If any required chain is not in the supported set, rejectSession is called with WalletConnect error code UNSUPPORTED_CHAINS so the requesting dApp receives an immediate rejection rather than waiting forever. (141d918)
    • Supported namespaces advertised to approved dApps contain only methods the active signer can execute; unsupported optional chains/methods are omitted silently. (141d918)
    • Incompatible restored sessions are disconnected on initializeWalletConnect, preventing cross-account rebinding. (141d918, 60926a9)
    • External disconnect and account-change events produce distinct user-facing messages. (4e0f486)
      Review finding addressed: Add Reown existing-wallet login alongside Web3Auth #72 (comment)

    Verified:
    All required commands run on Node 24, Yarn 4.17.0, macOS, branch fix/external-wallet-login-hardening at e652cf4:

    • yarn install --immutable ✅ pass
    • yarn biome check ✅ pass — 363 files, 0 errors
    • yarn tsc --noEmit ✅ pass — 0 type errors
    • yarn build ✅ pass — Turbopack ~34 s, 11/11 static pages generated
    • yarn test ✅ pass — 19 test files, 151 tests
    • yarn test:e2e ✅ pass — 10/10 Playwright scenarios

    All failure cases (cancelled transaction, replaced transaction, late async balance response, concurrent network switch, unsupported proposal rejection, case-insensitive sender validation) are covered by deterministic unit tests using provider/wallet mocks.
    Interactive QA was performed with yarn dev against a testnet environment (NEXT_PUBLIC_GOODDOLLAR_CONTRACTS=development) using MetaMask on Brave. Verified: external login, automatic logout on external account switch, account data isolation, hard refresh session restore, and logout. NEXT_PUBLIC_LOGIN_EXISTING_WALLET_ENABLED remains false for production.

    Evidence:
    Test files: transactionHistoryStore.test.ts, amountsStore.test.ts, WalletWrapper.test.ts, reown.test.ts, utils.test.ts, RequestHandler.test.ts, walletConnectStore.test.ts.
    Build: Turbopack, 11/11 static routes, clean.
    E2E: Playwright 10/10 on desktop + mobile, EN + DA.

    Remaining:

    • N1-R3: disconnectDappSessions in storage.ts is fire-and-forget (pre-existing pattern).
    • N2-R3: chainOpLock has no timeout for a hung wallet popup.
    • N3-R2: LiFi swap-sequence operations in lifiStore are not serialized through chainOpLock.
  17. cyzcz commented on Nov 7, 2026

    @cyzcz

    PR: #80 (#80)
    Contributor: @cyzcz
    Commit: 2f19955 (targeting feat-connect-wallet-reown)

    Fixed

    • Post-approval WalletConnect session re-verification (src/sections/WalletConnect/requests/RequestHandler.ts): Re-queries walletConnectInstance.getActiveSessions()[topic] after openWalletConnectDialog resolves so a session revoked or disconnected while the approval modal was open cannot proceed to sign.
    • Multi-step swap session-change guard (src/sections/Swap/components/RouteSlider/RouteSlider.tsx): Added an activityHistoryStore.getCurrentAccountToken() !== initialAccountToken check inside updateTransactionRequestHook so a mid-route logout/account switch aborts before signing subsequent steps with another account's signer.
    • Includes full coverage of Scope 1–4 (account/history/balance isolation, serialized chainOpLock network switching, reverted/repriced replacement transaction handling, and UNSUPPORTED_CHAINS / UNSUPPORTED_METHODS proposal rejection).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions