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

adds ledger option for creating signing commitment #5398

Merged

Conversation

hughy
Copy link
Contributor

@hughy hughy commented Sep 19, 2024

Summary

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

Testing Plan

manual testing:

  • created ledger multisig account using dkg
  • ran miner to fund default account, sent transaction to ledger multisig
  • created unsigned transaction from ledger multisig
  • created signing commitment using ledger multisig

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.

[ ] Yes

Breaking Change

Is this a breaking change? If yes, add notes below on why this is breaking and label it with breaking-change-rpc or breaking-change-sdk.

[ ] Yes

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
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
@hughy hughy merged commit a487c5d into feat/ledger-dkg-cli Sep 19, 2024
8 checks passed
@hughy hughy deleted the feat/hughy/ifl-2943/ledger-signing-commitment branch September 19, 2024 23:39
patnir pushed a commit that referenced this pull request Sep 24, 2024
* 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
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 <97762857+mat-if@users.noreply.github.com>

---------

Co-authored-by: mat-if <97762857+mat-if@users.noreply.github.com>

* 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 <97762857+mat-if@users.noreply.github.com>

* 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 <hugh@iflabs.network>
Co-authored-by: Hugh Cunningham <57735705+hughy@users.noreply.github.com>
Co-authored-by: mat-if <97762857+mat-if@users.noreply.github.com>
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 <97762857+mat-if@users.noreply.github.com>

---------

Co-authored-by: mat-if <97762857+mat-if@users.noreply.github.com>

* 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 <97762857+mat-if@users.noreply.github.com>

* 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 <hugh@iflabs.network>
Co-authored-by: Hugh Cunningham <57735705+hughy@users.noreply.github.com>
Co-authored-by: mat-if <97762857+mat-if@users.noreply.github.com>

* 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 <patnir@users.noreply.github.com>

---------

Co-authored-by: Rahul Patni <patnir@users.noreply.github.com>

* 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 <5459049+rohanjadvani@users.noreply.github.com>
Co-authored-by: mat-if <97762857+mat-if@users.noreply.github.com>
Co-authored-by: Jason Spafford <nullprogrammer@gmail.com>
Co-authored-by: Rahul Patni <patnir@users.noreply.github.com>
Co-authored-by: Derek Guenther <derek@iflabs.network>
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