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.

Sign up for GitHub

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

Jump to bottom

[refactor] #4339: refactor iroha_crypto, update API #4341

Merged
merged 10 commits into from
Mar 13, 2024
Merged

[refactor] #4339: refactor iroha_crypto, update API #4341

merged 10 commits into from
Mar 13, 2024

Conversation

0x009922
Copy link
Contributor

@0x009922 0x009922 commented Mar 4, 2024

Description

See linked issue.

Linked issue

Closes #4339

@0x009922 0x009922 added iroha2-dev The re-implementation of a BFT hyperledger in RUST api-changes Changes in the API for client libraries Refactor Improvement to overall code quality crypto labels Mar 4, 2024
@0x009922 0x009922 self-assigned this Mar 4, 2024
@coveralls
Copy link

coveralls commented Mar 4, 2024 •
edited
Loading

Pull Request Test Coverage Report for Build 8257299862

Details

  • 215 of 244 (88.11%) changed or added relevant lines in 24 files are covered.
  • 4097 unchanged lines in 75 files lost coverage.
  • Overall coverage decreased (-0.2%) to 56.638%
Changes Missing Coverage Covered Lines Changed/Added Lines %
cli/src/samples.rs 0 1 0.0%
tools/kagami/src/genesis.rs 0 1 0.0%
core/test_network/src/lib.rs 0 2 0.0%
tools/kagami/src/crypto.rs 0 6 0.0%
crypto/src/lib.rs 88 107 82.24%
Files with Coverage Reduction New Missed Lines %
primitives/src/conststr.rs 1 91.14%
ffi/derive/src/convert.rs 1 84.45%
primitives/src/lib.rs 1 0.0%
core/src/sumeragi/network_topology.rs 1 98.78%
data_model/derive/src/has_origin.rs 3 95.16%
config/src/kura.rs 3 80.0%
primitives/src/addr.rs 3 63.82%
schema/derive/src/lib.rs 5 83.25%
config/src/logger.rs 5 72.73%
tools/kagami/src/main.rs 5 0.0%
Totals Coverage Status
Change from base Build 7884695009: -0.2%
Covered Lines: 22134
Relevant Lines: 39080
💛 - Coveralls

mversic
mversic reviewed Mar 5, 2024
View reviewed changes
crypto/src/signature/mod.rs Outdated Show resolved Hide resolved
@0x009922 0x009922 mentioned this pull request Mar 5, 2024
Merged
mversic
mversic reviewed Mar 5, 2024
View reviewed changes
crypto/src/lib.rs Outdated Show resolved Hide resolved
@0x009922
Copy link
Contributor Author

0x009922 commented Mar 5, 2024

Okay, so what's next:

  • rename to from_bytes anyway
  • change serialization to uppercase everywhere
  • create an issue with a bug of KeyGenConfiguration::from_private_key()

@0x009922
Copy link
Contributor Author

0x009922 commented Mar 5, 2024

I've updated KeyGenConfiguration using type-state-pattern, but it fails in checks and with ffi features enabled.

Will address it next. Probably, will try to simplify the builder yet remain it type-safe.

mversic
mversic previously approved these changes Mar 6, 2024
View reviewed changes
DCNick3
DCNick3 previously approved these changes Mar 6, 2024
View reviewed changes
@mversic mversic self-requested a review March 6, 2024 07:57
mversic
mversic reviewed Mar 6, 2024
View reviewed changes
crypto/src/lib.rs Outdated Show resolved Hide resolved
mversic
mversic reviewed Mar 6, 2024
View reviewed changes
crypto/src/lib.rs Outdated Show resolved Hide resolved
@0x009922 0x009922 dismissed stale reviews from DCNick3 and mversic via e79f617 March 6, 2024 09:31
@0x009922
Copy link
Contributor Author

0x009922 commented Mar 6, 2024

Updated how keys generation is configured. With some refactoring, managed to avoid unnecessary validation and panics in runtime thanks to types.

Renamed KeyGenConfiguration and KeyGenOption to KeyPairGenConfig and KeyPairGenOption respectively.

@0x009922 0x009922 requested a review from DCNick3 March 6, 2024 09:37
mversic
mversic reviewed Mar 6, 2024
View reviewed changes
crypto/src/lib.rs Outdated Show resolved Hide resolved
@0x009922 0x009922 requested a review from mversic March 7, 2024 09:33
mversic
mversic reviewed Mar 7, 2024
View reviewed changes
crypto/src/lib.rs Outdated Show resolved Hide resolved
mversic
mversic reviewed Mar 7, 2024
View reviewed changes
crypto/src/lib.rs Outdated Show resolved Hide resolved
0x009922
0x009922 commented Mar 7, 2024
View reviewed changes
crypto/src/lib.rs Outdated Show resolved Hide resolved
0x009922 added a commit to 0x009922/iroha-javascript that referenced this pull request Mar 8, 2024
@0x009922
[refactor]: update upstream to hyperledger-iroha/iroha#4341
c40c79c 
Signed-off-by: Dmitry Balashov <43530070+0x009922@users.noreply.github.com>
@0x009922 0x009922 mentioned this pull request Mar 11, 2024
Open
mversic
mversic previously approved these changes Mar 11, 2024
View reviewed changes
@0x009922
[refactor] hyperledger-iroha#4339: refactor iroha_crypto, update API
599102a 
Signed-off-by: Dmitry Balashov <43530070+0x009922@users.noreply.github.com>
@0x009922
[chore]: clean
2d74a74 
Signed-off-by: Dmitry Balashov <43530070+0x009922@users.noreply.github.com>
@0x009922
[refactor]: use from_bytes/to_bytes
b990384 
Signed-off-by: Dmitry Balashov <43530070+0x009922@users.noreply.github.com>
@0x009922
[refactor]: use try_from and into serde container attrs
c3f8ff4 
Signed-off-by: Dmitry Balashov <43530070+0x009922@users.noreply.github.com>
@0x009922
[refactor]: serialize keys' payloads in uppercase hex
25d64f3 
There is also `iroha_swarm`, which still uses lowercase

Signed-off-by: Dmitry Balashov <43530070+0x009922@users.noreply.github.com>
@0x009922
[fix]: also use uppercase for Signature's payload
1708b01 
Signed-off-by: Dmitry Balashov <43530070+0x009922@users.noreply.github.com>
@0x009922
[refactor]: stronger types in KeyGenOption, renames and fixes
14de07b 
Signed-off-by: Dmitry Balashov <43530070+0x009922@users.noreply.github.com>
@0x009922
[refactor]: revert rename
03fb5a8 
Signed-off-by: Dmitry Balashov <43530070+0x009922@users.noreply.github.com>
@0x009922
[refactor]: snip KeyGenConfig entirely
e466a6a 
- Rename `KeyPair::generate` to `KeyPair::random`
- Add `KeyPair::random_with_algorithm`
- Add `KeyPair::from_seed`
- Add `impl From<PrivateKey> for KeyPair`

Also:

- Rename `KeyPair::new` to `KeyPair::from_raw_parts`
  for consistency

Signed-off-by: Dmitry Balashov <43530070+0x009922@users.noreply.github.com>
@0x009922
[refactor]: restruct KeyPair methods
0d7c78a 
- revert `from_raw_parts` to `new`
- rename `into_raw_parts` to `into_parts`
- remove `impl From<KeyPair> to (K, K)`

Signed-off-by: Dmitry Balashov <43530070+0x009922@users.noreply.github.com>
@0x009922 0x009922 requested a review from mversic March 13, 2024 00:07
@0x009922 0x009922 merged commit 7014f53 into hyperledger-iroha:iroha2-dev Mar 13, 2024
13 checks passed
@0x009922 0x009922 deleted the 4339-crypto-refactor branch March 13, 2024 09:30
0x009922 added a commit to hyperledger-iroha/iroha-javascript that referenced this pull request Mar 14, 2024
@0x009922
Rewrite Crypto WASM using the original iroha_crypto crate (#188)
787a198 
* [refactor]: update WASM API, wip

`cargo check --target wasm32-unknown-unknown` fails:
`getrandom` crate features are invalid

Signed-off-by: Dmitry Balashov <43530070+0x009922@users.noreply.github.com>

* [feat]: complete refactoring (upstream is WIP)

Signed-off-by: Dmitry Balashov <43530070+0x009922@users.noreply.github.com>

* [refactor]: update upstream to hyperledger-iroha/iroha#4341

Signed-off-by: Dmitry Balashov <43530070+0x009922@users.noreply.github.com>

* [refactor]: update crypto, finally

Signed-off-by: Dmitry Balashov <43530070+0x009922@users.noreply.github.com>

---------

Signed-off-by: Dmitry Balashov <43530070+0x009922@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mversic mversic mversic approved these changes

@DCNick3 DCNick3 DCNick3 approved these changes

@s8sato s8sato Awaiting requested review from s8sato

@Arjentix Arjentix Awaiting requested review from Arjentix

@SamHSmith SamHSmith Awaiting requested review from SamHSmith

@outoftardis outoftardis Awaiting requested review from outoftardis

@Erigara Erigara Awaiting requested review from Erigara

@VAmuzing VAmuzing Awaiting requested review from VAmuzing

@Asem-Abdelhady Asem-Abdelhady Awaiting requested review from Asem-Abdelhady

Assignees

@DCNick3 DCNick3

@0x009922 0x009922

@mversic mversic

Labels
api-changes Changes in the API for client libraries crypto iroha2-dev The re-implementation of a BFT hyperledger in RUST Refactor Improvement to overall code quality
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@0x009922 @coveralls @DCNick3 @mversic