Add a new EdgeCurrencyWallet.split method#701
Conversation
d0b80ce to
931f4a8
Compare
src/core/login/splitting.ts
Outdated
| await makeEdgeResult( | ||
| finishWalletSplitting( | ||
| ai, | ||
| splitInfos.get(splitInfo.walletType)?.id ?? '', |
There was a problem hiding this comment.
The splitInfos.get(splitInfo.walletType)?.id ?? '' fallback passes an empty string to finishWalletSplitting if the map lookup fails. While this should never happen after the validation loop, consider throwing an explicit error:
const walletId = splitInfos.get(splitInfo.walletType)?.id
if (walletId == null) throw new Error(`Missing wallet info for ${splitInfo.walletType}`)Or maybe just a comment
931f4a8 to
70cc264
Compare
70cc264 to
7bcfbd0
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| true | ||
| ) | ||
| if (result.ok) return result.result.id | ||
| throw result.error |
There was a problem hiding this comment.
Deprecated method now throws on wallet load failure
Medium Severity
The deprecated splitWalletInfo now throws if waitForCurrencyWallet fails, whereas previously the error was caught and the wallet ID was still returned. In the old code, the entire wallet-load-and-copy-metadata block was wrapped in a single try/catch that called ai.props.onError and then returned newWalletInfo.id. Now, finishWalletSplitting lets waitForCurrencyWallet errors propagate into makeEdgeResult, producing { ok: false, error }, which the account-api destructures as throw result.error. Since the keys are already saved on the server at that point, the split succeeded at the data level but the caller gets an exception and cannot retry (due to the rejectDupes duplicate check).
Additional Locations (1)
| } | ||
| if (needsProtection) return newWalletInfo.id | ||
| throw new Error('This wallet has already been split') | ||
| if (hasChanges) await changeWalletStates(ai, accountId, newStates) |
There was a problem hiding this comment.
Redundant archived/deleted check in restore loop
Low Severity
The if (existingWalletInfo.archived || existingWalletInfo.deleted) check at line 177 is always true because items are only added to toRestore when that exact condition holds (line 156). This makes the hasChanges flag and its guard also always true when toRestore.length > 0, adding unnecessary indirection that may mislead future readers into thinking some toRestore items might not need state changes.


CHANGELOG
Does this branch warrant an entry to the CHANGELOG?
Dependencies
noneDescription
noneNote
Medium Risk
Touches wallet-splitting and key/login-kit application paths, which can affect wallet creation/restoration and BCH replay-protection behavior. The surface area is moderate and covered by updated unit tests, but failures could impact wallet availability if edge cases aren’t handled.
Overview
Adds wallet-scoped splitting via
EdgeCurrencyWallet.split, allowing callers to request one or more split wallets and receive per-walletEdgeResultsuccess/failure objects.Refactors
splitWalletInfoto accept the sourcewalletInfo, validate requested wallet types/plugins, optionally apply BCH→BSV replay protection, restore previously archived/deleted split wallets, and best-effort apply requestedname/fiatCurrencyCodeon new wallets.EdgeAccount.splitWalletInforemains but is now deprecated and implemented as a thin wrapper around the new split logic, and a sharedmakeEdgeResulthelper is moved tosrc/util/edgeResult.ts.Updates types/tests and fake plugins to support the new API and multi-plugin split scenarios (including improved duplicate-split error messaging and a new
tulipcointest plugin).Written by Cursor Bugbot for commit 7bcfbd0. This will update automatically on new commits. Configure here.