Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@

- added: `EdgeCurrencyEngineCallbacks.onSyncStatusChanged` callback.
- added: `EdgeCurrencyInfo.syncDisplayPrecision` option.
- added: `EdgeCurrencyWallet.split` method.
- added: `EdgeCurrencyWallet.syncStatus` property and matching `EdgeSyncStatus` type.
- deprecated: `EdgeAccount.splitWalletInfo` method.
- deprecated: `EdgeCurrencyEngineCallbacks.onAddressesChecked`. Use `onSyncStatusChanged` instead.
- deprecated: `EdgeCurrencyWallet.syncRatio`. Use `syncStatus` instead.

Expand Down
46 changes: 37 additions & 9 deletions src/core/account/account-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import {
EdgeWalletInfoFull,
EdgeWalletStates
} from '../../types/types'
import { makeEdgeResult } from '../../util/edgeResult'
import { base58 } from '../../util/encoding'
import { getPublicWalletInfo } from '../currency/wallet/currency-wallet-pixie'
import {
Expand Down Expand Up @@ -502,7 +503,42 @@ export function makeAccountApi(ai: ApiInput, accountId: string): EdgeAccount {
walletId: string,
newWalletType: string
): Promise<string> {
return await splitWalletInfo(ai, accountId, walletId, newWalletType)
const { allWalletInfosFull } = accountState()
const walletInfo = allWalletInfosFull.find(
walletInfo => walletInfo.id === walletId
)
if (walletInfo == null) throw new Error(`Invalid wallet id ${walletId}`)
const existingWallet =
ai.props.output?.currency?.wallets[walletInfo.id]?.walletApi

// The following check has not been needed since about 2021,
// when the currency plugins became responsible for listing
// their own splittable types, but keep it for safety:
if (
walletInfo.type === 'wallet:bitcoin' &&
walletInfo.keys.format === 'bip49' &&
newWalletType === 'wallet:bitcoincash'
) {
throw new Error(
'Cannot split segwit-format Bitcoin wallets to Bitcoin Cash'
)
}

const [result] = await splitWalletInfo(
ai,
accountId,
walletInfo,
[
{
walletType: newWalletType,
name: existingWallet?.name ?? undefined,
fiatCurrencyCode: existingWallet?.fiatCurrencyCode
}
],
true
)
if (result.ok) return result.result.id
throw result.error
Copy link

Choose a reason for hiding this comment

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

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)

Fix in Cursor Fix in Web

},

async listSplittableWalletTypes(walletId: string): Promise<string[]> {
Expand Down Expand Up @@ -828,11 +864,3 @@ function getRawPrivateKey(
}
return info
}

async function makeEdgeResult<T>(promise: Promise<T>): Promise<EdgeResult<T>> {
try {
return { ok: true, result: await promise }
} catch (error) {
return { ok: false, error }
}
}
15 changes: 15 additions & 0 deletions src/core/currency/wallet/currency-wallet-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,12 @@ import {
EdgeParsedUri,
EdgePaymentProtocolInfo,
EdgeReceiveAddress,
EdgeResult,
EdgeSaveTxMetadataOptions,
EdgeSignMessageOptions,
EdgeSpendInfo,
EdgeSpendTarget,
EdgeSplitCurrencyWallet,
EdgeStakingStatus,
EdgeStreamTransactionOptions,
EdgeSyncStatus,
Expand All @@ -41,6 +43,7 @@ import {
EdgeWalletInfo
} from '../../../types/types'
import { makeMetaTokens } from '../../account/custom-tokens'
import { splitWalletInfo } from '../../login/splitting'
import { toApiInput } from '../../root-pixie'
import { makeStorageWalletApi } from '../../storage/storage-api'
import { getCurrencyMultiplier } from '../currency-selectors'
Expand Down Expand Up @@ -731,6 +734,18 @@ export function makeCurrencyWalletApi(
emit(out, 'transactionsRemoved', undefined)
},

async split(
splitWallets: EdgeSplitCurrencyWallet[]
): Promise<Array<EdgeResult<EdgeCurrencyWallet>>> {
return await splitWalletInfo(
ai,
accountId,
walletInfo,
splitWallets,
false
)
},

// URI handling:
async encodeUri(options: EdgeEncodeUri): Promise<string> {
return await tools.encodeUri(
Expand Down
169 changes: 114 additions & 55 deletions src/core/login/splitting.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,15 @@ import { base64 } from 'rfc4648'

import {
EdgeCurrencyWallet,
EdgeResult,
EdgeSpendInfo,
EdgeSplitCurrencyWallet,
EdgeWalletInfo,
EdgeWalletInfoFull,
EdgeWalletStates
} from '../../types/types'
import { hmacSha256 } from '../../util/crypto/hashes'
import { makeEdgeResult } from '../../util/edgeResult'
import { utf8 } from '../../util/encoding'
import { changeWalletStates } from '../account/account-files'
import { waitForCurrencyWallet } from '../currency/currency-selectors'
Expand Down Expand Up @@ -102,80 +106,135 @@ export function makeSplitWalletInfo(
export async function splitWalletInfo(
ai: ApiInput,
accountId: string,
walletId: string,
newWalletType: string
): Promise<string> {
walletInfo: EdgeWalletInfoFull,
splitWallets: EdgeSplitCurrencyWallet[],
rejectDupes: boolean
): Promise<Array<EdgeResult<EdgeCurrencyWallet>>> {
const accountState = ai.props.state.accounts[accountId]
const { allWalletInfosFull, sessionKey } = accountState

// Find the wallet we are going to split:
const walletInfo = allWalletInfosFull.find(
walletInfo => walletInfo.id === walletId
)
if (walletInfo == null) throw new Error(`Invalid wallet id ${walletId}`)

// Handle BCH / BTC+segwit special case:
if (
newWalletType === 'wallet:bitcoincash' &&
walletInfo.type === 'wallet:bitcoin' &&
walletInfo.keys.format === 'bip49'
) {
throw new Error(
'Cannot split segwit-format Bitcoin wallets to Bitcoin Cash'
)
// Validate the wallet types:
const plugins = ai.props.state.plugins.currency
const splitInfos = new Map<string, EdgeWalletInfo>()
for (const item of splitWallets) {
const { walletType } = item
const pluginId = maybeFindCurrencyPluginId(plugins, item.walletType)
if (pluginId == null) {
throw new Error(`Cannot find plugin for wallet type "${walletType}"`)
}
if (splitInfos.has(walletType)) {
throw new Error(`Duplicate wallet type "${walletType}"`)
}
splitInfos.set(walletType, makeSplitWalletInfo(walletInfo, walletType))
}

// Handle BitcoinABC/SV replay protection:
// Do we need BitcoinABC/SV replay protection?
const needsProtection =
newWalletType === 'wallet:bitcoinsv' &&
walletInfo.type === 'wallet:bitcoincash'
walletInfo.type === 'wallet:bitcoincash' &&
// We can re-protect a wallet by doing a repeated split,
// so don't check if the wallet already exists:
splitInfos.has('wallet:bitcoinsv')
if (needsProtection) {
const oldWallet = ai.props.output.currency.wallets[walletId].walletApi
if (oldWallet == null) throw new Error('Missing Wallet')
await protectBchWallet(oldWallet)
const existingWallet =
ai.props.output?.currency?.wallets[walletInfo.id]?.walletApi
if (existingWallet == null) {
throw new Error(`Cannot find wallet ${walletInfo.id}`)
}
await protectBchWallet(existingWallet)
}

// See if the wallet has already been split:
const newWalletInfo = makeSplitWalletInfo(walletInfo, newWalletType)
const existingWalletInfo = allWalletInfosFull.find(
walletInfo => walletInfo.id === newWalletInfo.id
)
if (existingWalletInfo != null) {
if (existingWalletInfo.archived || existingWalletInfo.deleted) {
// Simply undelete the existing wallet:
const walletInfos: EdgeWalletStates = {}
walletInfos[newWalletInfo.id] = {
archived: false,
deleted: false,
migratedFromWalletId: existingWalletInfo.migratedFromWalletId
// Sort the wallet infos into two categories:
const toRestore: EdgeWalletInfoFull[] = []
const toCreate: EdgeWalletInfo[] = []
for (const newWalletInfo of splitInfos.values()) {
const existingWalletInfo = allWalletInfosFull.find(
info => info.id === newWalletInfo.id
)
if (existingWalletInfo == null) {
toCreate.push(newWalletInfo)
} else {
if (existingWalletInfo.archived || existingWalletInfo.deleted) {
toRestore.push(existingWalletInfo)
} else if (rejectDupes) {
if (
// It's OK to re-split if we are adding protection:
walletInfo.type !== 'wallet:bitcoincash' ||
newWalletInfo.type !== 'wallet:bitcoinsv'
) {
throw new Error(
`This wallet has already been split (${newWalletInfo.type})`
)
}
}
}
}

// Restore anything that has simply been deleted:
if (toRestore.length > 0) {
const newStates: EdgeWalletStates = {}
let hasChanges = false
for (const existingWalletInfo of toRestore) {
if (existingWalletInfo.archived || existingWalletInfo.deleted) {
hasChanges = true
newStates[existingWalletInfo.id] = {
archived: false,
deleted: false,
migratedFromWalletId: existingWalletInfo.migratedFromWalletId
}
}
await changeWalletStates(ai, accountId, walletInfos)
return newWalletInfo.id
}
if (needsProtection) return newWalletInfo.id
throw new Error('This wallet has already been split')
if (hasChanges) await changeWalletStates(ai, accountId, newStates)
Copy link

Choose a reason for hiding this comment

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

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.

Fix in Cursor Fix in Web

}

// Add the keys to the login:
const kit = makeKeysKit(ai, sessionKey, [newWalletInfo], true)
await applyKit(ai, sessionKey, kit)
if (toCreate.length > 0) {
const kit = makeKeysKit(ai, sessionKey, toCreate, true)
await applyKit(ai, sessionKey, kit)
}

// Wait for the new wallets to load:
const out = await Promise.all(
splitWallets.map(async splitInfo => {
const walletInfo = splitInfos.get(splitInfo.walletType)
if (walletInfo == null) {
throw new Error(`Missing wallet info for ${splitInfo.walletType}`)
}
return await makeEdgeResult(
finishWalletSplitting(
ai,
walletInfo.id,
toCreate.find(info => info.type === splitInfo.walletType) != null
? splitInfo
: undefined
)
)
})
)

return out
}

async function finishWalletSplitting(
ai: ApiInput,
walletId: string,
item?: EdgeSplitCurrencyWallet
): Promise<EdgeCurrencyWallet> {
const wallet = await waitForCurrencyWallet(ai, walletId)

// Try to copy metadata on a best-effort basis.
// In the future we should clone the repo instead:
try {
const wallet = await waitForCurrencyWallet(ai, newWalletInfo.id)
const oldWallet = ai.props.output.currency.wallets[walletId].walletApi
if (oldWallet != null) {
if (oldWallet.name != null) await wallet.renameWallet(oldWallet.name)
if (oldWallet.fiatCurrencyCode != null) {
await wallet.setFiatCurrencyCode(oldWallet.fiatCurrencyCode)
}
}
} catch (error: unknown) {
ai.props.onError(error)
if (item?.name != null) {
await wallet
.renameWallet(item.name)
.catch((error: unknown) => ai.props.onError(error))
}
if (item?.fiatCurrencyCode != null) {
await wallet
.setFiatCurrencyCode(item.fiatCurrencyCode)
.catch((error: unknown) => ai.props.onError(error))
}

return newWalletInfo.id
return wallet
}

async function protectBchWallet(wallet: EdgeCurrencyWallet): Promise<void> {
Expand Down
19 changes: 15 additions & 4 deletions src/types/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1408,6 +1408,9 @@ export interface EdgeCurrencyWallet {
// Wallet management:
readonly dumpData: () => Promise<EdgeDataDump>
readonly resyncBlockchain: () => Promise<void>
readonly split: (
splitWallets: EdgeSplitCurrencyWallet[]
) => Promise<Array<EdgeResult<EdgeCurrencyWallet>>>

// URI handling:
readonly encodeUri: (obj: EdgeEncodeUri) => Promise<string>
Expand Down Expand Up @@ -1664,6 +1667,12 @@ export type EdgeCreateCurrencyWallet = EdgeCreateCurrencyWalletOptions & {
walletType: string
}

export interface EdgeSplitCurrencyWallet {
fiatCurrencyCode?: string
name?: string
walletType: string
}

export interface EdgeCurrencyConfig {
readonly watch: Subscriber<EdgeCurrencyConfig>

Expand Down Expand Up @@ -1867,10 +1876,6 @@ export interface EdgeAccount {
readonly getWalletInfo: (id: string) => EdgeWalletInfoFull | undefined
readonly listWalletIds: () => string[]
readonly listSplittableWalletTypes: (walletId: string) => Promise<string[]>
readonly splitWalletInfo: (
walletId: string,
newWalletType: string
) => Promise<string>

// Key access:
readonly getDisplayPrivateKey: (walletId: string) => Promise<string>
Expand Down Expand Up @@ -1917,6 +1922,12 @@ export interface EdgeAccount {
request: EdgeSwapRequest,
opts?: EdgeSwapRequestOptions
) => Promise<EdgeSwapQuote[]>

/** @deprecated Use `EdgeCurrencyWallet.split` instead */
readonly splitWalletInfo: (
walletId: string,
newWalletType: string
) => Promise<string>
}

// ---------------------------------------------------------------------
Expand Down
11 changes: 11 additions & 0 deletions src/util/edgeResult.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import { EdgeResult } from '../types/types'

export async function makeEdgeResult<T>(
promise: Promise<T>
): Promise<EdgeResult<T>> {
try {
return { ok: true, result: await promise }
} catch (error) {
return { ok: false, error }
}
}
4 changes: 2 additions & 2 deletions test/core/account/account.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
import { expectRejection } from '../../expect-rejection'
import { fakeUser } from '../../fake/fake-user'

const plugins = { fakecoin: true }
const plugins = { fakecoin: true, tulipcoin: true }
const contextOptions = { apiKey: '', appId: '', plugins }
const quiet = { onLog() {} }

Expand Down Expand Up @@ -311,7 +311,7 @@ describe('account', function () {
// Splitting back should not work:
await expectRejection(
account.splitWalletInfo(tulipWallet.id, 'wallet:fakecoin'),
'Error: This wallet has already been split'
'Error: This wallet has already been split (wallet:fakecoin)'
)
})

Expand Down
Loading
Loading