prompts for new name and retries import in round3 #5407
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
extracts the import/name prompt loop logic from the 'wallet:import' command into an 'importAccount' util uses the 'importAccount' util to import the account at the end of round3 so that import is retried in the case of name collisions
hughy
force-pushed
the
feat/hughy/ifl-2957/round3-import-loop
branch
from
5f7b068
to
5553a92
Compare
mat-if
approved these changes
Sep 20, 2024
patnir
pushed a commit
that referenced
this pull request
Sep 24, 2024
extracts the import/name prompt loop logic from the 'wallet:import' command into an 'importAccount' util uses the 'importAccount' util to import the account at the end of round3 so that import is retried in the case of name collisions
patnir
added a commit
that referenced
this pull request
Sep 24, 2024
* updates zondax/ledger-ironfish dependency to dev branch * updates zondax/ledger-ironfish-js to local dependency * updates zondax/ledger-js to dev branch on iron-fish fork * updates ledger.ts for upgrade to @zondax/ledger-js (#5386) * updates ledger.ts for upgrade to @zondax/ledger-js removes response handling for error codes, which are no longer included in responses implements tryInstruction to handle errors from any app instructions adds methods to help discriminate between KeyResponse types and checks type of KeyResponse for each type of key adds method to recognize Ledger response errors defines classes for common recoverable Ledger errors: locked device and app not open. the error code for the app not being open is identified in the docs as a 'technical error', but this is the error code we get when the app isn't open * adds ledger flag to participant creation (#5387) * adds ledger flag to participant creation adds dkgGetIDentity method to ledger util module adds '--ledger' flag to 'wallet:multisig:participant:create' command uses 'wallet/multisig/importParticipant' RPC to add the participant identity to the walletDb * implements ledger dkg round1 (#5389) * implements ledger dkg round1 adds dkgRound1 method to Ledger util fills in logic of performRound1WithLedger reads identity from node and adds it to list of signer identities if it's missing * implements ledger dkg round2 (#5390) * implements ledger dkg round2 adds dkgRound2 method to Ledger util implements performRound2WithLedger in round2 CLI command * implements ledger dkg round3 (#5391) * implements ledger dkg round3 implements dkgRound3 method on Ledger util class implements dkgRetrieveKeys on Ledger util class to retrieve all shared multisig keys implements dkgGetPublicPackage on Ledger util class to retrieve public key package updates round3 command to run dkg round3, retrieve keys, get public key package, and import account with ledger flag * includes participant's own gsk share in round3 input * renames createParticipantWithLedger to getIdentityFromLedger * fixes Ledger approval output in round3 (#5397) ask to approve round3 on Ledger, but don't ask for approval/confirmation when retrieving keys or public package * adds ledger option for creating signing commitment (#5398) * adds ledger option for creating signing commitment implements dkgGetCommitments on Ledger util class adds '--ledger' flag to 'wallet:multisig:commitment:create' adds multisig account selector to 'commitment:create', fetches the identity for the selected account adds 'hash' method to UnsignedTransaction primitive * adds ledger flag for creating signature shares (#5399) implements dkgSign on Ledger util class to obtain a signature share from the device adds '--ledger' flag to 'wallet:multisig:signature:create' command adds selector to command to choose multisig account if one is not provided adds 'publicKeyRandomness' method to UnsignedTransaction primitive to extract the randomness from the transaction (needed for signing) adds 'serialize' napi method to NativeSignatureShare to support constructing the signature share from its raw parts and then printing it * improves error message from importParticipant RPC (#5402) includes name of conflicting identity in error message if the request tries to import an identity that already exists in the database * implements ledger multisig backup command (#5400) * implements ledger multisig backup command adds a CLI command, 'wallet:multisig:ledger:backup', to create an encrypted backup of multisig keys from the ironfish dkg ledger app users can restore the keys to their ledger app if they reinstall the app on their device or overwrite the multisig keys in the app * adds cli command to restore ledger multisig backup (#5401) * adds cli command to restore ledger multisig backup if the ironfish dkg ledger app is uninstalled, or if it is used to create a second set of keys, then the multisig keys on the device may be lost the 'wallet:multisig:ledger:backup' command creates an encrypted key backup that can be restored to the device the 'wallet:multisig:ledger:restore' command restores the backed up keys to the ledger * Update ironfish-cli/src/commands/wallet/multisig/ledger/restore.ts Co-authored-by: mat-if <[email protected]> --------- Co-authored-by: mat-if <[email protected]> * updates command description adds additional log output asking the user to save the backup and explaining how to restore the keys * fixes lint --------- Co-authored-by: mat-if <[email protected]> * feat: DKG Create Command (#5403) * adds ledger backup to the end of round3 (#5406) creates an encrypted backup and prints it to the terminal at the end of round3 of dkg * prompts for new name and retries import in round3 (#5407) extracts the import/name prompt loop logic from the 'wallet:import' command into an 'importAccount' util uses the 'importAccount' util to import the account at the end of round3 so that import is retried in the case of name collisions * adds cli command to import multisig account from ledger (#5409) reads identity, shared multisig keys, and public key package from Ledger device imports multisig account to wallet and prompts for new name if the provided name is taken removes hardcoded account version number from other ledger imports (these imports aren't from serialized accounts, so the schema version should always be the current version) * adds clear signing to ledger signing flows (#5413) uses 'reviewTransaction' method from '@zondax/ledger-ironfish' to force user to review decrypted transaction outputs on the ledger device updates our '@zondax/ledger-ironfish' dependency to use the latest release, which now includes dkg methods note: for interactive signing we may only need to review once before creating the signing commitments * always display asset id with notes in transaction details when approving a transaction using a ledger device users are shown the asset id of each output note on the ledger device the transaction details they see in the cli will not include the asset id if the asset is verified (the symbol will be displayed instead). this makes it more difficult to verify that the transaction you're seeing on the ledger is correct based on what the cli is showing you if the asset has a verified symbol, also displays the asset id for the note on the line below the amount * Revert "always display asset id with notes in transaction details" This reverts commit e7f3e46. * always display asset id with notes in transaction details (#5416) when approving a transaction using a ledger device users are shown the asset id of each output note on the ledger device the transaction details they see in the cli will not include the asset id if the asset is verified (the symbol will be displayed instead). this makes it more difficult to verify that the transaction you're seeing on the ledger is correct based on what the cli is showing you if the asset has a verified symbol, also displays the asset id for the note on the line below the amount * Adding ledger support for dkg:create (#5410) Also added the ability to reuse an existing identity for the DKG process. The user can specify the account name separately from the participant name. * multisig:sign command (#5411) * fix typo --------- Co-authored-by: Hugh Cunningham <[email protected]> Co-authored-by: Hugh Cunningham <[email protected]> Co-authored-by: mat-if <[email protected]>
mat-if
added a commit
that referenced
this pull request
Sep 27, 2024
* feat(cli): Unhide encryption CLI commands (#5396) * feat(cli,ironfish): Add descriptions to primitives index (#5412) * feat(cli,ironfish): Improve export for AccountImport (#5414) * Add the ability to use a ledger device with the mint command (#5417) * exports makeFakeWitness test util from ironfish sdk (#5415) * exports makeFakeWitness test util from ironfish sdk allows external developers to test transactions without a blockchain * exports makeFakeWitness as part of testUtilities export creates the same effect as a namespace * Enable transfering in a mint in the CLI (#5418) * use the testnet api url for asset verification when on testnet network (#5384) * Revert "exports makeFakeWitness test util from ironfish sdk (#5415)" (#5419) This reverts commit dcaabd8. * Feat/ledger dkg cli (#5421) * updates zondax/ledger-ironfish dependency to dev branch * updates zondax/ledger-ironfish-js to local dependency * updates zondax/ledger-js to dev branch on iron-fish fork * updates ledger.ts for upgrade to @zondax/ledger-js (#5386) * updates ledger.ts for upgrade to @zondax/ledger-js removes response handling for error codes, which are no longer included in responses implements tryInstruction to handle errors from any app instructions adds methods to help discriminate between KeyResponse types and checks type of KeyResponse for each type of key adds method to recognize Ledger response errors defines classes for common recoverable Ledger errors: locked device and app not open. the error code for the app not being open is identified in the docs as a 'technical error', but this is the error code we get when the app isn't open * adds ledger flag to participant creation (#5387) * adds ledger flag to participant creation adds dkgGetIDentity method to ledger util module adds '--ledger' flag to 'wallet:multisig:participant:create' command uses 'wallet/multisig/importParticipant' RPC to add the participant identity to the walletDb * implements ledger dkg round1 (#5389) * implements ledger dkg round1 adds dkgRound1 method to Ledger util fills in logic of performRound1WithLedger reads identity from node and adds it to list of signer identities if it's missing * implements ledger dkg round2 (#5390) * implements ledger dkg round2 adds dkgRound2 method to Ledger util implements performRound2WithLedger in round2 CLI command * implements ledger dkg round3 (#5391) * implements ledger dkg round3 implements dkgRound3 method on Ledger util class implements dkgRetrieveKeys on Ledger util class to retrieve all shared multisig keys implements dkgGetPublicPackage on Ledger util class to retrieve public key package updates round3 command to run dkg round3, retrieve keys, get public key package, and import account with ledger flag * includes participant's own gsk share in round3 input * renames createParticipantWithLedger to getIdentityFromLedger * fixes Ledger approval output in round3 (#5397) ask to approve round3 on Ledger, but don't ask for approval/confirmation when retrieving keys or public package * adds ledger option for creating signing commitment (#5398) * adds ledger option for creating signing commitment implements dkgGetCommitments on Ledger util class adds '--ledger' flag to 'wallet:multisig:commitment:create' adds multisig account selector to 'commitment:create', fetches the identity for the selected account adds 'hash' method to UnsignedTransaction primitive * adds ledger flag for creating signature shares (#5399) implements dkgSign on Ledger util class to obtain a signature share from the device adds '--ledger' flag to 'wallet:multisig:signature:create' command adds selector to command to choose multisig account if one is not provided adds 'publicKeyRandomness' method to UnsignedTransaction primitive to extract the randomness from the transaction (needed for signing) adds 'serialize' napi method to NativeSignatureShare to support constructing the signature share from its raw parts and then printing it * improves error message from importParticipant RPC (#5402) includes name of conflicting identity in error message if the request tries to import an identity that already exists in the database * implements ledger multisig backup command (#5400) * implements ledger multisig backup command adds a CLI command, 'wallet:multisig:ledger:backup', to create an encrypted backup of multisig keys from the ironfish dkg ledger app users can restore the keys to their ledger app if they reinstall the app on their device or overwrite the multisig keys in the app * adds cli command to restore ledger multisig backup (#5401) * adds cli command to restore ledger multisig backup if the ironfish dkg ledger app is uninstalled, or if it is used to create a second set of keys, then the multisig keys on the device may be lost the 'wallet:multisig:ledger:backup' command creates an encrypted key backup that can be restored to the device the 'wallet:multisig:ledger:restore' command restores the backed up keys to the ledger * Update ironfish-cli/src/commands/wallet/multisig/ledger/restore.ts Co-authored-by: mat-if <[email protected]> --------- Co-authored-by: mat-if <[email protected]> * updates command description adds additional log output asking the user to save the backup and explaining how to restore the keys * fixes lint --------- Co-authored-by: mat-if <[email protected]> * feat: DKG Create Command (#5403) * adds ledger backup to the end of round3 (#5406) creates an encrypted backup and prints it to the terminal at the end of round3 of dkg * prompts for new name and retries import in round3 (#5407) extracts the import/name prompt loop logic from the 'wallet:import' command into an 'importAccount' util uses the 'importAccount' util to import the account at the end of round3 so that import is retried in the case of name collisions * adds cli command to import multisig account from ledger (#5409) reads identity, shared multisig keys, and public key package from Ledger device imports multisig account to wallet and prompts for new name if the provided name is taken removes hardcoded account version number from other ledger imports (these imports aren't from serialized accounts, so the schema version should always be the current version) * adds clear signing to ledger signing flows (#5413) uses 'reviewTransaction' method from '@zondax/ledger-ironfish' to force user to review decrypted transaction outputs on the ledger device updates our '@zondax/ledger-ironfish' dependency to use the latest release, which now includes dkg methods note: for interactive signing we may only need to review once before creating the signing commitments * always display asset id with notes in transaction details when approving a transaction using a ledger device users are shown the asset id of each output note on the ledger device the transaction details they see in the cli will not include the asset id if the asset is verified (the symbol will be displayed instead). this makes it more difficult to verify that the transaction you're seeing on the ledger is correct based on what the cli is showing you if the asset has a verified symbol, also displays the asset id for the note on the line below the amount * Revert "always display asset id with notes in transaction details" This reverts commit e7f3e46. * always display asset id with notes in transaction details (#5416) when approving a transaction using a ledger device users are shown the asset id of each output note on the ledger device the transaction details they see in the cli will not include the asset id if the asset is verified (the symbol will be displayed instead). this makes it more difficult to verify that the transaction you're seeing on the ledger is correct based on what the cli is showing you if the asset has a verified symbol, also displays the asset id for the note on the line below the amount * Adding ledger support for dkg:create (#5410) Also added the ability to reuse an existing identity for the DKG process. The user can specify the account name separately from the participant name. * multisig:sign command (#5411) * fix typo --------- Co-authored-by: Hugh Cunningham <[email protected]> Co-authored-by: Hugh Cunningham <[email protected]> Co-authored-by: mat-if <[email protected]> * Allows `--metadata ''` when minting an asset (#5424) * adds exportable devUtils module (#5423) * separates Ledger DKG logic from single signer (#5425) * separates Ledger DKG logic from single signer defines a separate class, LedgerDkg, for interacting with the Ledger Ironfish DKG app updates all DKG and multisig commands to use LedgerDkg adds aliased dependencies @zondax/ledger-js-dkg and @zondax/ledger-ironfish-dkg to use the versions of these packages that we need for the DKG app restores the previous version of the Ledger class for use with the single signer Ledger app uses v0.1.2 of @zondax/ledger-ironfish for compatibility with the single signer app * Update ironfish-cli/src/utils/ledger.ts Co-authored-by: Rahul Patni <[email protected]> --------- Co-authored-by: Rahul Patni <[email protected]> * Error for too many participants (#5426) At the moment, ledger dkg only supports up to 4 participants. This commit adds an error message when the user tries to create a dkg with more than 4 participants. * Ask for confirmation while retrying ledger connect (#5428) * fix resolution of @zondax/ledger-js package (#5427) * fix resolution of @zondax/ledger-js package we have two versions of @zondax/ledger-ironfish in our dependencies that require different versions of @zondax/ledger-js we need to make sure that each version's dependencies resolve correctly in order for both the DKG and single signer apps to work * removes resolutions block from package.json * Review ledger transaction before commitment (#5429) This fits with the latest changes in the ledger app/ sdk. * Expect to participant in signing (#5431) Previously, this command was written in a way where the user performing this command didn't have to be a participant. This changes that to make sure that the user performing this command is a participant in the multisig wallet. This is because this command is catering to the most common use case. * Fix logical error on catching duplicates (#5432) * optionally sets createdAt when importing multisig accounts (#5430) * optionally sets createdAt when importing multisig accounts adds '--createdAt' flag to 'wallet:multisig:dkg:create', 'wallet:multisig:dkg:round3', and 'wallet:multisig:ledger:import' updates round3 RPC to accept optional 'accountCreatedAt' parameter allows user to set the 'createdAt' sequence when creating a multisig account or importing one from a ledger device. this allows users to start using their accounts more quickly without waiting for the account to scan the entire chain * defaults dkg createdAt to head of node's chain keeps the createdAt flag in place in case user wants to set it on an account when their node isn't synced, but defaults to the chain head * fixes rendering of tx assets if node not synced (#5436) when testing multisig signing we found that rendering unsigned transaction details failed with the error 'No asset found with identifier' when the transaction contained a custom asset, but the wallet had not scanned a transaction that included that asset users will likely use multisig signing offline, so they may not have access to synced asset data updates '_renderTransactionDetails' to use 'getAssetVerificationByIds', which handles errors from missing assets * Use Chainport fallback token list API (#5437) * Rename interactive step Identity to Participant Identity (#5434) * Rename newAccount flag in dkg create command to name (#5435) * chainport config update with mainnet fields (#5422) * feat: Add MAINNET CLI support for Chainport * update error message * Fix minor typos (show -> shown) (#5433) * signing app error display improvement (#5439) * Temporarily assign different URL for mainnet (#5441) * explicityly asking to broadcast ledger transaction (#5440) * Detects whether ironfish dkg app is open (#5442) * Detects whether ironfish dkg app is open Calling app.getVersion errors when the app is locked and when the app is not open. This is sufficient enough for us to detect whether the app is open or not. If it isn't open, we just end the process for the dkg commands. * commetn explaining why we call this and do nothing with the response * refresh Ledger connection before each instruction (#5444) some errors cause the CLI to disconnect from the device. this can make it impossible to continue running commands until a new connection is made handles 'disconnect' events on the app transport by closing the connection refreshes the connection before each instruction: checks if the app is undefined and establishes a new connection refactors 'tryInstruction' to take a function instead of a promise * displays own identity at start of signing process (#5445) users must enter the identities of all signers (excluding their own) before creating a signing commitment displays the user's own identity and asks the user to share it with other signers before prompting for the identities of other signers * ensures that min signers entered is <= total participants (#5446) the minimum number of signers cannot be greater than the number of total participants throw an error when the user enters an invalid number instead of failing during round1 * changes 'backup' from flag to arg in 'restore' (#5447) updates 'wallet:multisig:ledger:restore' to accept the encrypted keys backup as an arg instead of as a flag this makes the command consistent with our cli style guide * uses actionable error message for common Ledger errors (#5448) we've seen several errors occur commonly when the Ledger app is unavailable either because the device is locked or the app isn't open catches errors with these codes and displays an error message telling the user that the error may be due to a locked device or closed app * wallet:multisig:account:participants displays your identity separately (#5450) * inputNumberPrompt and usage (#5443) * Adds prompt specific to numbers The problem with parse int is that even if you pass it something like "1234123asdf" it will return a success and return 1234123. This is not what we want. We want to make sure that the user has entered a valid number. We also want to retry the prompt if the user enters an invalid number or a decimal when an integer is expected. * adding min signers check back: lint fix * bumps versions for v2.7.0 (#5452) --------- Co-authored-by: Rohan Jadvani <[email protected]> Co-authored-by: mat-if <[email protected]> Co-authored-by: Jason Spafford <[email protected]> Co-authored-by: Rahul Patni <[email protected]> Co-authored-by: Derek Guenther <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
extracts the import/name prompt loop logic from the 'wallet:import' command into an 'importAccount' util
uses the 'importAccount' util to import the account at the end of round3 so that import is retried in the case of name collisions
Testing Plan
manual testing with
wallet:import:manual testing with
wallet:multisig:dkg:* --ledger:Documentation
Does this change require any updates to the Iron Fish Docs (ex. the RPC API
Reference)? If yes, link a
related documentation pull request for the website.
Breaking Change
Is this a breaking change? If yes, add notes below on why this is breaking and label it with
breaking-change-rpcorbreaking-change-sdk.