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

[refactor] #4045: Use concrete key types #4181

Merged
merged 8 commits into from
Feb 6, 2024

Conversation

Arjentix
Copy link
Contributor

@Arjentix Arjentix commented Dec 28, 2023

Description

  • PublicKey and PrivateKey are now enums with explicit key types. No invalid state is possible
  • Fixed some tests
  • Fixed now useless errors signatures of depended code
  • amcl_wrapper replaced with w3f-bls
  • iroha_crypto is now compatible with no-std and WASM
  • Fixed some clippy lints

Linked issue

Benefits

  • More robust solution because now we are sure that stored keys are valid
  • No more errors are possible when creating keys or signing
  • Test are more real now

Checklist

  • I've read CONTRIBUTING.md
  • I've used the standard signed-off commit format (or will squash just before merging)
  • All applicable CI checks pass (or I promised to make them pass later)
  • (optional) I've written unit tests for the code changes
  • I replied to all comments after code review, marking all implemented changes with thumbs up

@Arjentix Arjentix added iroha2-dev The re-implementation of a BFT hyperledger in RUST Refactor Improvement to overall code quality Security This issue asks for improved security labels Dec 28, 2023
@Arjentix Arjentix self-assigned this Dec 28, 2023
core/src/sumeragi/view_change.rs Outdated Show resolved Hide resolved
crypto/src/kex/mod.rs Outdated Show resolved Hide resolved
p2p/src/peer.rs Outdated Show resolved Hide resolved
data_model/src/query/mod.rs Outdated Show resolved Hide resolved
data_model/src/peer.rs Outdated Show resolved Hide resolved
crypto/src/lib.rs Outdated Show resolved Hide resolved
crypto/src/lib.rs Outdated Show resolved Hide resolved
crypto/src/lib.rs Outdated Show resolved Hide resolved
crypto/src/lib.rs Outdated Show resolved Hide resolved
crypto/src/lib.rs Outdated Show resolved Hide resolved
@mversic mversic self-assigned this Jan 3, 2024
@mversic
Copy link
Contributor

mversic commented Jan 3, 2024

Currently it's impossible to build Executor or any wasm, because used crypto libraries are feature-gated, meaning that even PublicKey and PrivateKey cannot be represented as is in WASM.

Solutions I see:

* Let WASM have all crypto code and remove feature-gate. Unused code will likely be optimized by Rust, but I'm not sure.

* Have another `PublicKey` and `PrivateKey` for WASM types which will be compatible with types from Host in terms of encoding and serialization

For any of this solutions I would like to hear other opiniont, especially from @mversic .

I would try to get PublicKey/PrivateKey work in no-std. If that is not possible, then we can try using std in wasm. The last thing is making FFI calls

genesis/src/lib.rs Outdated Show resolved Hide resolved
@DCNick3 DCNick3 self-assigned this Jan 9, 2024
Copy link
Contributor

@DCNick3 DCNick3 left a comment

Choose a reason for hiding this comment

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

Very nice to see all those .unwrap()'s gone!

cli/src/lib.rs Show resolved Hide resolved
client/tests/integration/offline_peers.rs Outdated Show resolved Hide resolved
client_cli/src/main.rs Outdated Show resolved Hide resolved
config/src/path.rs Outdated Show resolved Hide resolved
data_model/src/query/mod.rs Outdated Show resolved Hide resolved
crypto/src/signature/secp256k1.rs Outdated Show resolved Hide resolved
crypto/src/signature/secp256k1.rs Outdated Show resolved Hide resolved
data_model/src/predicate.rs Outdated Show resolved Hide resolved
data_model/src/predicate.rs Outdated Show resolved Hide resolved
genesis/src/lib.rs Outdated Show resolved Hide resolved
crypto/src/lib.rs Outdated Show resolved Hide resolved
data_model/src/peer.rs Outdated Show resolved Hide resolved
@Arjentix Arjentix force-pushed the typesafe_keys branch 2 times, most recently from 1bafba1 to 0152e33 Compare January 23, 2024 21:47
@Arjentix Arjentix marked this pull request as ready for review January 23, 2024 21:48
crypto/Cargo.toml Outdated Show resolved Hide resolved
genesis/src/lib.rs Outdated Show resolved Hide resolved
data_model/src/block.rs Outdated Show resolved Hide resolved
Signed-off-by: Daniil Polyakov <arjentix@gmail.com>
Signed-off-by: Daniil Polyakov <arjentix@gmail.com>
Signed-off-by: Daniil Polyakov <arjentix@gmail.com>
Signed-off-by: Daniil Polyakov <arjentix@gmail.com>
Signed-off-by: Daniil Polyakov <arjentix@gmail.com>
Signed-off-by: Daniil Polyakov <arjentix@gmail.com>
Signed-off-by: Daniil Polyakov <arjentix@gmail.com>
Signed-off-by: Daniil Polyakov <arjentix@gmail.com>
@mversic mversic merged commit e5ad44a into hyperledger:iroha2-dev Feb 6, 2024
11 of 12 checks passed
@Arjentix Arjentix deleted the typesafe_keys branch February 8, 2024 11:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
iroha2-dev The re-implementation of a BFT hyperledger in RUST Refactor Improvement to overall code quality Security This issue asks for improved security
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants