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

[fix] #1801: fix schema generation for HashOf, SignatureOf #1804

Merged
merged 1 commit into from
Jan 12, 2022

Conversation

mversic
Copy link
Contributor

@mversic mversic commented Jan 11, 2022

…schemas are missing

Signed-off-by: Marin Veršić marin.versic101@gmail.com

Description of the Change

  • Fixes schema serialization of HashOf, SignatureOf
  • adds test to ensure all schemas are in place

Issue

Closes #1801

Benefits

Possible Drawbacks

Usage Examples or Tests [optional]

Alternate Designs [optional]

IntoSchema macro can be improved to account for PhantomData fields. Maybe in the future

@github-actions github-actions bot added the iroha2-dev The re-implementation of a BFT hyperledger in RUST label Jan 11, 2022
@codecov
Copy link

codecov bot commented Jan 11, 2022

Codecov Report

Merging #1804 (3dbcb76) into iroha2-dev (84d01ac) will increase coverage by 1.53%.
The diff coverage is 82.00%.

Impacted file tree graph

@@              Coverage Diff               @@
##           iroha2-dev    #1804      +/-   ##
==============================================
+ Coverage       77.22%   78.75%   +1.53%     
==============================================
  Files             137      137              
  Lines           21145    21166      +21     
==============================================
+ Hits            16329    16670     +341     
+ Misses           4816     4496     -320     
Impacted Files Coverage Δ
data_model/src/expression.rs 70.58% <ø> (+6.41%) ⬆️
data_model/src/isi.rs 86.13% <ø> (+1.54%) ⬆️
schema/src/lib.rs 76.04% <70.58%> (+35.61%) ⬆️
schema/bin/src/main.rs 80.88% <76.36%> (+79.51%) ⬆️
crypto/src/hash.rs 97.87% <100.00%> (+5.18%) ⬆️
crypto/src/signature.rs 84.04% <100.00%> (+2.00%) ⬆️
actor/src/actor_id.rs 90.00% <0.00%> (-5.00%) ⬇️
p2p/src/peer.rs 82.03% <0.00%> (-1.11%) ⬇️
client/src/http_client.rs 94.68% <0.00%> (-1.07%) ⬇️
actor/src/lib.rs 89.07% <0.00%> (-0.69%) ⬇️
... and 17 more

Continue to review full report at Codecov.

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

@@ -84,13 +84,6 @@ pub struct RemoveKeyValueBox {
pub key: EvaluatesTo<Name>,
}

/// Sized structure for all possible Sets.
#[derive(Debug, Clone, Serialize, Deserialize, Encode, Decode, PartialEq, Eq, IntoSchema)]
pub struct SetBox {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

never used?

Copy link
Contributor

Choose a reason for hiding this comment

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

Two uses total. Here and in Schema. I would ask the library developers if they use it. If they don't I'd get rid of it.

Copy link
Contributor

@appetrosyan appetrosyan left a comment

Choose a reason for hiding this comment

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

Not sure about SetBox, and why coverage dropped 5%. Otherwise LGTM.

@@ -84,13 +84,6 @@ pub struct RemoveKeyValueBox {
pub key: EvaluatesTo<Name>,
}

/// Sized structure for all possible Sets.
#[derive(Debug, Clone, Serialize, Deserialize, Encode, Decode, PartialEq, Eq, IntoSchema)]
pub struct SetBox {
Copy link
Contributor

Choose a reason for hiding this comment

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

Two uses total. Here and in Schema. I would ask the library developers if they use it. If they don't I'd get rid of it.

…schemas are missing

Signed-off-by: Marin Veršić <marin.versic101@gmail.com>
@mversic mversic merged commit 79d0df6 into hyperledger:iroha2-dev Jan 12, 2022
@mversic mversic deleted the missing_schemas branch April 16, 2024 09:37
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants