-
Notifications
You must be signed in to change notification settings - Fork 45
feat: support bytes codec in ProtocolKey
#2557
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
Conversation
Test Results 3 files ± 0 88 suites ±0 13m 55s ⏱️ +34s Results for commit 873cdb8. ± Comparison against base commit 90439c9. This pull request removes 5 and adds 13 tests. Note that renamed tests count towards both.
♻️ This comment has been updated with latest results. |
96b5396
to
dce8f68
Compare
b79692a
to
c49bdec
Compare
c49bdec
to
e9ebeb2
Compare
41c907d
to
919b73d
Compare
e9ebeb2
to
0745bb3
Compare
0745bb3
to
09ad81c
Compare
09ad81c
to
ac4adcc
Compare
ac4adcc
to
4a54f95
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements bytes codec support for ProtocolKey along with broad enhancements to binary encoding/decoding, error handling improvements, and code refactoring across cryptographic utilities and related modules. Key changes include:
- Introduction and integration of TryToBytes/TryFromBytes traits for binary serialization across multiple cryptographic types.
- Enhanced error context and safer slicing for deserialization in functions like from_bytes.
- Refactoring of codec modules (bech32, binary, json_hex) and updates to Debug implementations and test helpers.
Reviewed Changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
mithril-common/src/test_utils/fake_keys.rs | Added trait bounds to assert conversions in test helpers. |
mithril-common/src/messages/message_parts/signer.rs | Adjusted Debug trait implementations for Signer and SignerWithStake types. |
mithril-common/src/messages/certificate.rs | Propagated error context in genesis signature encoding. |
mithril-common/src/entities/certificate.rs | Updated ProtocolKey usage with unwrap in byte encoding conversion. |
mithril-common/src/crypto_helper/types/protocol_key.rs | Extended trait bounds to support binary codec implementations. |
mithril-common/src/crypto_helper/merkle_tree.rs & merkle_map.rs | Introduced to_bytes and from_bytes functions for proof serialization. |
mithril-common/src/crypto_helper/cardano/key_certification.rs | Modified slicing for safety when reading initializer bytes. |
mithril-common/src/crypto_helper/cardano/codec.rs | Revised CBOR serialization/deserialization with hex encoding. |
Cargo.toml | Added bincode dependency with serde support. |
mithril-aggregator/src/database/record/certificate.rs | Updated conversion of genesis signature to bytes hex using unwrap. |
mithril-aggregator/src/message_adapters/from_register_signature.rs | Added golden tests to verify consistency between json hex and bytes hex encodings. |
Comments suppressed due to low confidence (1)
mithril-common/src/messages/message_parts/signer.rs:276
- [nitpick] Verify that the Debug trait implementations for 'SignerMessagePart' and 'SignerWithStakeMessagePart' are correctly assigned to avoid naming inconsistencies that could lead to confusing debug output.
impl Debug for SignerMessagePart {
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
And add support for failure of the conversion.
Using 'bincode' crate.
Using 'bincode' crate.
To make sure that both json hex and bytes hex encoding are supported.
To make sure that both json hex and bytes hex encoding are supported.
To make sure that both json hex and bytes hex encoding are supported.
* mithril-aggregator from `0.7.62` to `0.7.63` * mithril-common from `0.5.39` to `0.5.40`
4a54f95
to
873cdb8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Content
This PR includes the support for bytes codec in
ProtocolKey
.Serialization and Deserialization Enhancements:
TryToBytes
andTryFromBytes
traits to standardize binary serialization and deserialization across multiple cryptographic structures. (mithril-common/src/crypto_helper/codec/binary.rs
, mithril-common/src/crypto_helper/codec/binary.rsR1-R261)to_cbor_bytes
,to_cbor_hex
,from_cbor_bytes
, andfrom_cbor_hex
for CBOR encoding and decoding in theSerDeShelleyFileFormat
trait. (mithril-common/src/crypto_helper/cardano/codec.rs
, [1] [2]encode_bech32
for Bech32 encoding with test coverage. (mithril-common/src/crypto_helper/codec/bech32.rs
, mithril-common/src/crypto_helper/codec/bech32.rsR1-R29)Modularization:
bech32
,binary
,json_hex
) for improved modularity and maintainability. (mithril-common/src/crypto_helper/codec/mod.rs
, mithril-common/src/crypto_helper/codec/mod.rsR1-R7)mithril-common/src/crypto_helper/codec.rs
tomithril-common/src/crypto_helper/codec/json_hex.rs
and removed Bech32-related code from this file. (mithril-common/src/crypto_helper/codec/json_hex.rs
, [1] [2] [3]Cryptographic Structures Updates:
get()
to avoid panics. (mithril-common/src/crypto_helper/cardano/key_certification.rs
, mithril-common/src/crypto_helper/cardano/key_certification.rsL233-R239)mithril-common/src/crypto_helper/merkle_map.rs
, [1] [2];mithril-common/src/crypto_helper/merkle_tree.rs
, [3]Dependency Updates:
bincode
dependency withserde
feature for efficient binary serialization. (mithril-common/Cargo.toml
, mithril-common/Cargo.tomlR49)Test Coverage:
mithril-aggregator/src/message_adapters/from_register_signature.rs
, mithril-aggregator/src/message_adapters/from_register_signature.rsR47-R91)mithril-common/src/crypto_helper/codec/bech32.rs
, mithril-common/src/crypto_helper/codec/bech32.rsR1-R29)mithril-common/src/crypto_helper/merkle_map.rs
, mithril-common/src/crypto_helper/merkle_map.rsR840-R864)Pre-submit checklist
Issue(s)
Closes #2536