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

Use concrete signature types #715

Merged
merged 55 commits into from
Jul 14, 2023
Merged

Conversation

DaughterOfMars
Copy link

Description of change

This PR updates crypto and stronghold, and replaces byte arrays with the concrete crypto types.

Links to any relevant issues

Type of change

Choose a type of change, and delete any options that are not relevant.

  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

@semenov-vladyslav
Copy link

It looks like a simple fn from_u32_hardened could have solved the issue with repeated pattern .into_iter().map(Segment::harden).collect().

But the right solution seems to be to introduce a Bip44 type as all the bip32_chains in the code are in fact bip44_chains. BIP-44 has strict rules for hardening. We can apply the correct hardening rules (depending on the key type) when converting it into iterator.

@semenov-vladyslav
Copy link

bip44 PR. You can use slip10::Bip44 as the type of your chains. You can use [..].into() or Bip44::from([..]) to create Bip44 chains without the need to explicitly harden segments. You can use let sk: ed25519::SecretKey = bip44_chain.derive(&master_key).secret_key() to derive signing key, segments will be hardened automatically.

Copy link
Contributor

@kwek20 kwek20 left a comment

Choose a reason for hiding this comment

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

I dont like this

.into_iter()
.map(Segment::harden)
.collect(),

Maybe we can add a helper method to Segment or something?

edit: ah i see vlad thought the same-ish :D

bindings/core/src/method_handler/utils.rs Show resolved Hide resolved
bindings/core/src/method_handler/wallet.rs Outdated Show resolved Hide resolved
sdk/src/client/api/types.rs Outdated Show resolved Hide resolved
sdk/src/client/secret/ledger_nano.rs Outdated Show resolved Hide resolved
sdk/src/types/block/signature/mod.rs Show resolved Hide resolved
bindings/core/Cargo.toml Outdated Show resolved Hide resolved
sdk/src/client/utils.rs Outdated Show resolved Hide resolved
sdk/src/client/utils.rs Outdated Show resolved Hide resolved
Thoralf-M
Thoralf-M previously approved these changes Jul 14, 2023
Thoralf-M
Thoralf-M previously approved these changes Jul 14, 2023
Alex6323
Alex6323 previously approved these changes Jul 14, 2023
@DaughterOfMars DaughterOfMars dismissed stale reviews from Alex6323 and Thoralf-M via cbe7e8d July 14, 2023 12:38
Co-authored-by: Thibault Martinez <thibault@iota.org>
@thibault-martinez thibault-martinez merged commit 1c2941d into develop Jul 14, 2023
22 checks passed
@thibault-martinez thibault-martinez deleted the feat/ed25519-signature branch July 14, 2023 13:32
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.

Ed25519Signature could use stronghold/crypto types
6 participants