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

[cryptography] Introudce Array #447

Merged
merged 54 commits into from
Feb 6, 2025

Conversation

najeal
Copy link
Contributor

@najeal najeal commented Feb 3, 2025

Closes: #411

Unify representation of fixed-length bytes and detect malformed bytes on parse.

Future Work: #457

@najeal najeal marked this pull request as ready for review February 3, 2025 23:55
@najeal
Copy link
Contributor Author

najeal commented Feb 3, 2025

When running the cargo clippy command, you should get these errors:

  • error: very complex type used. Consider factoring parts into type definitions
  • usage of contains_key followed by insert on a BTreeMap

Not sure how we want to handle the first error.
The second error appeared after the modification of a file but we were already doing the insert this way.

@patrick-ogrady patrick-ogrady changed the title Scheme traits mirroring [cryptography] Introudce Octets Feb 5, 2025
@patrick-ogrady
Copy link
Contributor

Just need to fix the signing tests now!

utils/src/lib.rs Outdated
@@ -97,6 +97,26 @@ pub fn modulo(bytes: &[u8], n: u64) -> u64 {
result
}

pub trait Serializable {
Copy link
Contributor

Choose a reason for hiding this comment

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

If we don't implement this as part of the Octets trait, we can implement it for other types (like u64)

@patrick-ogrady patrick-ogrady changed the title [cryptography] Introudce Octets [cryptography] Introudce FormattedArray Feb 6, 2025
@patrick-ogrady patrick-ogrady changed the title [cryptography] Introudce FormattedArray [cryptography] Introudce Array Feb 6, 2025
@@ -25,15 +19,13 @@ fn benchmark_signature_verification(c: &mut Criterion) {
|| {
let mut signer = Bls12381::new(&mut thread_rng());
let signature = signer.sign(Some(namespace), &msg);
let public = group::Public::deserialize(signer.public_key().as_ref()).unwrap();
Copy link
Contributor

Choose a reason for hiding this comment

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

We would've seen a nice benchmark gain if this was previously using Scheme (as it should've been).

@patrick-ogrady patrick-ogrady merged commit 3a65f7e into commonwarexyz:main Feb 6, 2025
6 of 7 checks passed
Copy link

codecov bot commented Feb 6, 2025

Codecov Report

Attention: Patch coverage is 88.02488% with 154 lines in your changes missing coverage. Please review.

Project coverage is 94.15%. Comparing base (cee6569) to head (14a2f75).
Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
cryptography/src/bls12381/scheme.rs 83.44% 50 Missing ⚠️
cryptography/src/ed25519/scheme.rs 79.87% 33 Missing ⚠️
cryptography/src/secp256r1/scheme.rs 83.87% 30 Missing ⚠️
consensus/src/simplex/actors/voter/actor.rs 78.08% 16 Missing ⚠️
cryptography/src/lib.rs 84.21% 6 Missing ⚠️
cryptography/src/bls12381/primitives/group.rs 0.00% 5 Missing ⚠️
consensus/src/simplex/verifier.rs 85.71% 3 Missing ⚠️
p2p/src/authenticated/config.rs 33.33% 2 Missing ⚠️
p2p/src/simulated/network.rs 91.66% 2 Missing ⚠️
consensus/src/simplex/actors/resolver/actor.rs 80.00% 1 Missing ⚠️
... and 6 more
@@            Coverage Diff             @@
##             main     #447      +/-   ##
==========================================
- Coverage   94.43%   94.15%   -0.28%     
==========================================
  Files          93       94       +1     
  Lines       26611    27403     +792     
==========================================
+ Hits        25129    25802     +673     
- Misses       1482     1601     +119     
Files with missing lines Coverage Δ
consensus/src/simplex/actors/voter/ingress.rs 100.00% <ø> (ø)
consensus/src/simplex/config.rs 68.75% <ø> (ø)
consensus/src/simplex/encoder.rs 100.00% <100.00%> (ø)
consensus/src/simplex/engine.rs 94.38% <100.00%> (+0.40%) ⬆️
consensus/src/simplex/metrics.rs 100.00% <100.00%> (ø)
consensus/src/simplex/mocks/application.rs 91.18% <100.00%> (ø)
consensus/src/simplex/mocks/conflicter.rs 90.78% <100.00%> (+0.78%) ⬆️
consensus/src/simplex/mocks/nuller.rs 88.09% <100.00%> (+0.91%) ⬆️
consensus/src/simplex/mocks/relay.rs 88.88% <100.00%> (ø)
consensus/src/simplex/mocks/supervisor.rs 95.41% <100.00%> (ø)
... and 58 more

... and 6 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cee6569...14a2f75. Read the comment docs.

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.

[cryptography] Mirror Hasher::Digest changes for Scheme::{PublicKey, PrivateKey, Signature}
2 participants