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

deserialized endorsements fail signature verification in doctest #3381

Closed
Ben-PH opened this issue Jan 6, 2023 · 3 comments
Closed

deserialized endorsements fail signature verification in doctest #3381

Ben-PH opened this issue Jan 6, 2023 · 3 comments
Labels
bug Something isn't working documentation Improvements or additions to documentation tests Issues related to testing the node

Comments

@Ben-PH
Copy link
Contributor

Ben-PH commented Jan 6, 2023

Describe the bug
If one verifies the signature of the endorsements following the deserialization loop found in

src/export_active_block.rs - export_active_block::ExportActiveBlockDeserializer::deserialize (line 157)

the verifications fail.

To Reproduce

apply this diff to commit 65fcea9 then run cargo test -F testing -p massa_consensus_exports --doc

index f0e5bae6b..b20b3c4c3 100644
--- a/massa-models/src/block.rs
+++ b/massa-models/src/block.rs
@@ -771,6 +771,9 @@ impl Deserializer<BlockHeader> for BlockHeaderDeserializer {
             ),
         )
         .parse(rest)?;
+        for end in endorsements.iter() {
+            end.verify_signature().unwrap();
+        }
 
         Ok((
             rest,

Expected behavior

passing tests

Additional context

Discovered as part of work being done in #3364

@Ben-PH Ben-PH added bug Something isn't working documentation Improvements or additions to documentation models tests Issues related to testing the node labels Jan 6, 2023
@AurelienFT
Copy link
Member

This is maybe similar to #3378 the creation of the endorsement is maybe wrong and doesn't old the right data to be a valid endorsement.

@Ben-PH
Copy link
Contributor Author

Ben-PH commented Jan 12, 2023

This can better reveal the crux of the issue:

checkout deser-bug-demo branch
git checkout HEAD~1
cd into massa-consensus-export
cargo test --doc

see failure

git checkout deser-bug-demo
cargo test--doc

passes.

the commit that allows a pass is here

@AurelienFT
Copy link
Member

Done in #3364

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working documentation Improvements or additions to documentation tests Issues related to testing the node
Projects
None yet
Development

No branches or pull requests

2 participants