Skip to content

fix: preserve imported token migrations#5989

Open
samholmes wants to merge 2 commits intodevelopfrom
sam/migrate-fix-zano
Open

fix: preserve imported token migrations#5989
samholmes wants to merge 2 commits intodevelopfrom
sam/migrate-fix-zano

Conversation

@samholmes
Copy link
Copy Markdown
Contributor

@samholmes samholmes commented Mar 30, 2026

Imported token migrations were losing their parent wallet context between wallet creation and the migration flow, which made token migrations disappear or crash before users could review them.

Track the created wallet ids during import completion and carry the parent wallet metadata into the migration payload so imported tokens reach the migration scenes as first-class assets.

CHANGELOG

Does this branch warrant an entry to the CHANGELOG?

  • Yes
  • No

Dependencies

none

Requirements

If you have made any visual changes to the GUI. Make sure you have:

  • Tested on iOS device
  • Tested on Android device
  • Tested on small-screen device (iPod Touch)
  • Tested on large-screen device (tablet)

Note

High Risk
Touches wallet migration and private-key sweep transaction construction, including fee calculation and token enable/disable behavior, so regressions could impact fund transfers and user balances. Adds a new multi-asset makeMaxSpend path that needs plugin compatibility and careful error handling.

Overview
Fixes imported-wallet migration so selected tokens stay attached to their newly-created parent wallet by building the migration payload from only successfully created wallets and propagating the correct createWalletId/wallet metadata into token items.

Updates migration and sweep fee/transfer flows to optionally use a plugin-provided multi-asset otherMethods.makeMaxSpend, marking token fees as "Included" when bundled into a single transaction, and adjusts sweep completion logic to handle the single multi-asset transaction case.

Adds the string_included locale string and updates ESLint overrides to stop listing some scenes that are no longer exempted.

Reviewed by Cursor Bugbot for commit b4ec9dc. Bugbot is set up for automated code reviews on this repo. Configure here.


Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Autofix Details

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: PluginId-keyed map loses wallets sharing same plugin
    • Changed createdWalletIdsByPluginId to key by unique item.key instead of pluginId, preventing wallet ID overwrites when multiple wallets share the same pluginId.

Create PR

Or push these changes by commenting:

@cursor push 14928f7cff
Preview (14928f7cff)
diff --git a/src/components/scenes/CreateWalletCompletionScene.tsx b/src/components/scenes/CreateWalletCompletionScene.tsx
--- a/src/components/scenes/CreateWalletCompletionScene.tsx
+++ b/src/components/scenes/CreateWalletCompletionScene.tsx
@@ -56,8 +56,9 @@
   const defaultIsoFiat = useSelector(state => state.ui.settings.defaultIsoFiat)
 
   const [done, setDone] = React.useState(false)
-  const [createdWalletIdsByPluginId, setCreatedWalletIdsByPluginId] =
-    React.useState<Record<string, string>>({})
+  const [createdWalletIdsByKey, setCreatedWalletIdsByKey] = React.useState<
+    Record<string, string>
+  >({})
 
   const { newWalletItems, newTokenItems } = React.useMemo(
     () => splitCreateWalletItems(createWalletList),
@@ -157,13 +158,12 @@
       }
 
       // Save the created wallets
-      const newCreatedWalletIdsByPluginId: Record<string, string> = {}
+      const newCreatedWalletIdsByKey: Record<string, string> = {}
       walletResults.forEach((result, index) => {
         if (!result.ok) return
-        newCreatedWalletIdsByPluginId[newWalletItems[index].pluginId] =
-          result.result.id
+        newCreatedWalletIdsByKey[newWalletItems[index].key] = result.result.id
       })
-      setCreatedWalletIdsByPluginId(newCreatedWalletIdsByPluginId)
+      setCreatedWalletIdsByKey(newCreatedWalletIdsByKey)
 
       setDone(true)
       return () => {}
@@ -264,7 +264,8 @@
 
       return {
         ...createWallet,
-        createWalletId: createdWalletIdsByPluginId[pluginId] ?? '',
+        createWalletId:
+          createdWalletIdsByKey[parentWalletItem?.key ?? key] ?? '',
         walletType: parentWalletItem?.walletType ?? createWallet.walletType,
         displayName:
           parentWalletItem == null

This Bugbot Autofix run was free. To enable autofix for future PRs, go to the Cursor dashboard.

@samholmes samholmes force-pushed the sam/migrate-fix-zano branch from e9c9336 to 11a010a Compare March 30, 2026 23:56
Motivation: keep token migrations bundled with their parent wallet so
multi-asset chains like Zano do not split into separate wallet bundles.
This also enables the makeMaxSpend migration path to run as a single
routine for a wallet bundle while preserving the per-asset fallback.
Enable the sweep private key scenes to use a single multi-asset spend when
memory wallets expose makeMaxSpend, matching migrate wallet behavior.

This keeps token selection and post-sweep token enabling consistent when a
single transaction represents multiple assets, while preserving the legacy
per-asset fallback for wallets without the method.
@samholmes samholmes force-pushed the sam/migrate-fix-zano branch from 11a010a to b4ec9dc Compare April 4, 2026 01:03
Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: Double successCount increment in makeMaxSpend path
    • Removed the redundant successCount++ at line 278 to make both makeMaxSpend and legacy paths increment successCount consistently only once per successful bundle.

Create PR

Or push these changes by commenting:

@cursor push a855b7e9e0
Preview (a855b7e9e0)
diff --git a/src/components/scenes/MigrateWalletCalculateFeeScene.tsx b/src/components/scenes/MigrateWalletCalculateFeeScene.tsx
--- a/src/components/scenes/MigrateWalletCalculateFeeScene.tsx
+++ b/src/components/scenes/MigrateWalletCalculateFeeScene.tsx
@@ -274,8 +274,6 @@
                     item.tokenId == null ? txFee : 'included'
                   )
                 }
-
-                successCount++
               } catch (e: any) {
                 for (const key of bundlesFeeTotals.keys()) {
                   const insufficientFundsError =

This Bugbot Autofix run was free. To enable autofix for future PRs, go to the Cursor dashboard.

Reviewed by Cursor Bugbot for commit b4ec9dc. Configure here.

)
}

successCount++
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Double successCount increment in makeMaxSpend path

Low Severity

In the makeMaxSpend branch, successCount is incremented at line 278 inside the spend routine on success, and then incremented again at line 361 after the routine completes (because bundlesFeeTotals contains non-Error values like fee strings and 'included', so the success check passes). The legacy (else) branch only increments at line 361. This double counting is a logic error, though current impact is minimal since the only consumer checks successCount > 0.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit b4ec9dc. Configure here.

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