Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix deriving on bad paths #3257

Merged
merged 7 commits into from
Mar 10, 2023
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
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export const Finish = ({ isAddingAccount }: { isAddingAccount?: boolean }) => {
}
setLoading(false);
})();
}, [onboardingData, isAddingAccount]);
}, [background, isAddingAccount, onboardingData, maybeCreateUser]);

return !loading ? (
<SetupComplete
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ export const HardwareDefaultWallet = ({
}[blockchain];
setLedgerWallet(ledgerWallet);
})();
}, [blockchain]);
}, [blockchain, transport]);

// This is effectively the same as UI_RPC_METHOD_FIND_WALLET_DESCRIPTOR
useEffect(() => {
Expand Down Expand Up @@ -92,6 +92,7 @@ export const HardwareDefaultWallet = ({
const publicKey = publicKeys[0];
const derivationPath = recoveryPaths[0];
onNext({
blockchain,
derivationPath,
publicKey,
});
Expand All @@ -100,7 +101,7 @@ export const HardwareDefaultWallet = ({
setAccountIndex(accountIndex + 1);
}
})();
}, [ledgerWallet, accountIndex]);
}, [accountIndex, background, blockchain, ledgerWallet, onError, onNext]);

return <Loading />;
};
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ export const HardwareDeriveWallet = ({
}[blockchain];
setLedgerWallet(ledgerWallet);
})();
}, [blockchain]);
}, [blockchain, transport]);

useEffect(() => {
(async () => {
Expand Down Expand Up @@ -71,11 +71,12 @@ export const HardwareDeriveWallet = ({
// path if unusable

onNext({
blockchain,
derivationPath,
publicKey,
});
})();
}, [ledgerWallet]);
}, [background, blockchain, ledgerWallet, onError, onNext]);

return <Loading />;
};
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,9 @@ export function useHardwareOnboardSteps({
// Flow for onboarding a hardware wallet.
//
const steps = [
<ConnectHardwareWelcome onNext={nextStep} />,
<ConnectHardwareWelcome key="ConnectHardwareWelcome" onNext={nextStep} />,
<ConnectHardwareSearching
key="ConnectHardwareSearching"
blockchain={blockchain}
onNext={(transport) => {
setTransport(transport);
Expand Down Expand Up @@ -141,6 +142,7 @@ export function useHardwareOnboardSteps({
? [
// Sign the found wallet descriptor for API submit
<HardwareSign
key="HardwareSign"
blockchain={blockchain}
walletDescriptor={walletDescriptor}
message={
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,13 +58,13 @@ export const HardwareSearchWallet = ({
}
}
if (bs58.encode(ledgerAddress) === publicKey) {
onNext({ derivationPath, publicKey });
onNext({ blockchain, derivationPath, publicKey });
return;
}
}
setError(true);
})();
}, [publicKey]);
}, [blockchain, publicKey, onError, onNext, transport]);

if (!error) {
return <Loading />;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ export const MnemonicSearch = ({
const index = publicKeys.findIndex((p: string) => p === publicKey);
if (index !== -1) {
walletDescriptors.push({
blockchain,
derivationPath: recoveryPaths[index],
publicKey,
});
Expand All @@ -59,7 +60,7 @@ export const MnemonicSearch = ({
setError(true);
}
})();
}, [serverPublicKeys, mnemonic]);
}, [background, serverPublicKeys, mnemonic, onNext]);

if (!error) {
return <Loading />;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,8 @@ import type {
SignedWalletDescriptor,
WalletDescriptor,
} from "@coral-xyz/common";
import {
getCreateMessage,
UI_RPC_METHOD_KEYRING_STORE_KEEP_ALIVE,
} from "@coral-xyz/common";
import {
useBackgroundClient,
useOnboarding,
useSignMessageForWallet,
} from "@coral-xyz/recoil";
import { getCreateMessage } from "@coral-xyz/common";
import { useOnboarding, useSignMessageForWallet } from "@coral-xyz/recoil";

import { useSteps } from "../../../hooks/useSteps";
import { CreatePassword } from "../../common/Account/CreatePassword";
Expand Down Expand Up @@ -59,14 +52,15 @@ export const OnboardAccount = ({
signedWalletDescriptors,
selectedBlockchains,
} = onboardingData;

const signMessageForWallet = useSignMessageForWallet(mnemonic);

useEffect(() => {
// Reset blockchain keyrings on certain changes that invalidate the addresses
setOnboardingData({
signedWalletDescriptors: [],
});
}, [action, keyringType, mnemonic]);
}, [action, keyringType, mnemonic, setOnboardingData]);

const steps = [
<InviteCodeForm
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export function ConnectHardware({
UI_RPC_METHOD_BLOCKCHAIN_KEYRINGS_ADD;
await background.request({
method,
params: [blockchain, signedWalletDescriptor],
params: [signedWalletDescriptor],
});
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,15 +66,15 @@ export function CreateMenu({ blockchain }: { blockchain: Blockchain }) {
);
}
})();
}, [blockchain]);
}, [background, blockchain]);

useEffect(() => {
const prevTitle = nav.title;
nav.setOptions({ headerTitle: "" });
return () => {
nav.setOptions({ headerTitle: prevTitle });
};
}, [nav.setOptions]);
}, [nav]);

useEffect(() => {
(async () => {
Expand All @@ -84,7 +84,7 @@ export function CreateMenu({ blockchain }: { blockchain: Blockchain }) {
});
setKeyringExists(blockchainKeyrings.includes(blockchain));
})();
}, [blockchain]);
}, [background, blockchain]);

const createNewWithPhrase = async () => {
// Mnemonic based keyring. This is the simple case because we don't
Expand Down Expand Up @@ -117,7 +117,7 @@ export function CreateMenu({ blockchain }: { blockchain: Blockchain }) {
});
await background.request({
method: UI_RPC_METHOD_BLOCKCHAIN_KEYRINGS_ADD,
params: [blockchain, { ...walletDescriptor, signature }],
params: [{ ...walletDescriptor, signature }],
});
newPublicKey = walletDescriptor.publicKey;
// Keyring now exists, toggle to other options
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ export function ImportMnemonic({
return () => {
nav.setOptions({ headerTitle: prevTitle });
};
}, [theme]);
}, [nav, theme]);

// TODO replace the left nav button to go to the previous step if step > 0

Expand All @@ -74,13 +74,13 @@ export function ImportMnemonic({
// import the path
pk = await background.request({
method: UI_RPC_METHOD_KEYRING_IMPORT_WALLET,
params: [blockchain, signedWalletDescriptor],
params: [signedWalletDescriptor],
});
} else {
// Blockchain keyring doesn't exist, init
pk = await background.request({
method: UI_RPC_METHOD_BLOCKCHAIN_KEYRINGS_ADD,
params: [blockchain, signedWalletDescriptor],
params: [signedWalletDescriptor],
});
}
} else {
Expand All @@ -106,6 +106,7 @@ export function ImportMnemonic({
...(inputMnemonic
? [
<MnemonicInput
key="MnemonicInput"
buttonLabel="Next"
onNext={(mnemonic) => {
setMnemonic(mnemonic);
Expand All @@ -115,6 +116,7 @@ export function ImportMnemonic({
// Must prompt for a name if using an input mnemonic, because we can't
// easily generate one
<InputName
key="InputName"
onNext={(name) => {
setName(name);
nextStep();
Expand All @@ -123,7 +125,8 @@ export function ImportMnemonic({
]
: []),
<ImportWallets
blockchain={blockchain!}
key="ImportWallets"
blockchain={blockchain}
mnemonic={mnemonic}
recovery={publicKey}
allowMultiple={false}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ export function ImportWallets({
// If the query failed assume all are valid
}
})();
}, [walletDescriptors]);
}, [background, blockchain, walletDescriptors]);

//
// Load a list of accounts and their associated balances
Expand Down Expand Up @@ -239,6 +239,7 @@ export function ImportWallets({
.then(async (publicKeys: string[]) => {
setWalletDescriptors(
derivationPaths.map((derivationPath, i) => ({
blockchain,
publicKey: publicKeys[i],
derivationPath,
}))
Expand Down Expand Up @@ -388,9 +389,10 @@ export function ImportWallets({
if (currentIndex === -1) {
// Not selected, add it
const walletDescriptor = {
blockchain,
derivationPath,
publicKey,
} as WalletDescriptor;
};
// Adding the account
if (allowMultiple) {
newCheckedWalletDescriptors.push(walletDescriptor);
Expand Down Expand Up @@ -448,8 +450,8 @@ export function ImportWallets({
select
disabled={ledgerLocked}
>
{derivationPathOptions.map((o, index) => (
<MenuItem value={o.label} key={index}>
{derivationPathOptions.map((o) => (
<MenuItem value={o.label} key={o.label}>
{o.label}
</MenuItem>
))}
Expand Down
16 changes: 6 additions & 10 deletions packages/background/src/backend/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1180,13 +1180,10 @@ export class Backend {
return SUCCESS_RESPONSE;
}

async ledgerImport(
blockchain: Blockchain,
signedWalletDescriptor: SignedWalletDescriptor
) {
async ledgerImport(signedWalletDescriptor: SignedWalletDescriptor) {
const { signature, ...walletDescriptor } = signedWalletDescriptor;
const { publicKey } = walletDescriptor;
await this.keyringStore.ledgerImport(blockchain, walletDescriptor);
const { blockchain, publicKey } = walletDescriptor;
await this.keyringStore.ledgerImport(walletDescriptor);
try {
await this.userAccountPublicKeyCreate(blockchain, publicKey, signature);
} catch (error) {
Expand Down Expand Up @@ -1259,7 +1256,7 @@ export class Backend {
publicKey: publicKeys[index],
derivationPath: recoveryPaths[index],
};
await this.blockchainKeyringsAdd(blockchain, {
await this.blockchainKeyringsAdd({
...walletDescriptor,
signature: "",
});
Expand Down Expand Up @@ -1549,6 +1546,7 @@ export class Backend {
const publicKey = publicKeys[0];
const derivationPath = recoveryPaths[0];
return {
blockchain,
derivationPath,
publicKey,
};
Expand Down Expand Up @@ -1681,15 +1679,13 @@ export class Backend {
* Add a new blockchain keyring to the keyring store (i.e. initialize it).
*/
async blockchainKeyringsAdd(
blockchain: Blockchain,
signedWalletDescriptor: SignedWalletDescriptor
): Promise<string> {
await this.keyringStore.blockchainKeyringAdd(
blockchain,
signedWalletDescriptor as WalletDescriptor
);

const { signature, publicKey } = signedWalletDescriptor;
const { blockchain, signature, publicKey } = signedWalletDescriptor;

// Add the new public key to the API
try {
Expand Down
Loading