updates ironfish to use latest ironfish-frost changes, etc. #5369
Conversation
deserialize_round2_combined_public_package takes a serialized 'CombinedPublicPackage' from dkg round2 and returns an object containing an array of round2 public packages with all fields available as strings
throws FrostLibErrors using new_with_source to give error messages updates decryption test for new decryption error message
we've updated encryption/decryption in the ironfish-frost crate and changed the structure of encrypted data older account exports cannot be decrypted with the current 'decrypt' method and must use 'decrypt_legacy' instead defines 'decrypt_legacy_data' on ParticipantSecret and updates account decryption to try decrypting with that method if the first decryption attempt fails
hughy
force-pushed
the
hughy/ironfish-frost-integration
branch
from
3946a3f
to
9abbc9a
Compare
hughy
force-pushed
the
hughy/ironfish-frost-integration
branch
from
9abbc9a
to
0e10875
Compare
ironfish-rust-nodejs/src/multisig.rs
Outdated
| @@ -78,7 +83,7 @@ pub fn create_signing_commitment( | |||
| &signers, | |||
| ); | |||
|
|
|||
| let bytes = signing_commitment.serialize(); | |||
| let bytes = signing_commitment.unwrap().serialize(); | |||
There was a problem hiding this comment.
Should we bubble up the error instead of unwrapping here?
ironfish-rust-nodejs/src/multisig.rs
Outdated
| @@ -415,6 +432,7 @@ pub fn dkg_round2( | |||
| &round1_public_packages, | |||
| thread_rng(), | |||
| ) | |||
| .map_err(|e| IronfishError::new_with_source(IronfishErrorKind::FrostLibError, e)) | |||
There was a problem hiding this comment.
I think we should look into adding fmt::Display for the errors returned from the frost lib in std mode, then I think we can skip spamming these map_err everywhere. Doesn't need to happen right now, just thinking out loud.
There was a problem hiding this comment.
I think we do have fmt::Display in std mode 🤔
I'll see if we can easily get rid of these
ironfish-rust/Cargo.toml
Outdated
| @@ -43,7 +43,7 @@ chacha20poly1305 = "0.10.1" | |||
| crypto_box = { version = "0.9", features = ["std"] } | |||
| ff = "0.12.0" | |||
| group = "0.12.0" | |||
| ironfish-frost = { git = "https://github.com/iron-fish/ironfish-frost.git", branch = "main" } | |||
| ironfish-frost = { git = "https://github.com/iron-fish/ironfish-frost.git", branch = "main", features = ["std", "signing"] } | |||
There was a problem hiding this comment.
Do we want to use the default features here, so we don't need to define custom features? The difference I guess is that ironfish-frost has dkg, but we aren't enabling it here https://github.com/iron-fish/ironfish-frost/blob/main/Cargo.toml#L27
There was a problem hiding this comment.
Yeah I think we can use the default features here. We also support dkg in the CLI, and I can't remember what's behind the dkg feature in ironfish-frost
| let authorizing_key = Option::from(SubgroupPoint::from_bytes(&authorizing_key.serialize())) | ||
| .expect("failied to derive authorizing key"); | ||
| let mut bytes: [u8; 32] = [0; 32]; | ||
| bytes.copy_from_slice(&authorizing_key.serialize().unwrap()); |
There was a problem hiding this comment.
We should re-factor this to return a result instead of using an unwrap and expect on the next line
| @@ -33,7 +35,7 @@ pub fn split_spender_key( | |||
| split_secret(spender_key, identities, min_signers, thread_rng())?; | |||
|
|
|||
| assert_eq!( | |||
| public_key_package.verifying_key().serialize(), | |||
| public_key_package.verifying_key().serialize().unwrap(), | |||
| @@ -229,7 +229,9 @@ impl UnsignedTransaction { | |||
|
|
|||
| let serialized_signature = authorizing_group_signature.serialize(); | |||
|
|
|||
| let transaction = self.add_signature(serialized_signature)?; | |||
| let mut bytes = [0; 64]; | |||
| bytes.copy_from_slice(&serialized_signature.unwrap()); | |||
Summary
integrates latest updates to
ironfish-frostto support Ledger: iron-fish/ironfish-frost#81upgrades napi to accommodate dependency conflicts
adds deserialization helper functions to support round3_min
fixes test failures from encryption and error message changes
Testing Plan
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.