Skip to content

Conversation

@tholonious
Copy link
Contributor

No description provided.

@tholonious
Copy link
Contributor Author

Current dependencies on/for this PR:

This comment was auto-generated by Graphite.

@tholonious tholonious requested review from AeonSw4n and lazynina July 5, 2023 21:38
@tholonious tholonious marked this pull request as ready for review July 5, 2023 21:38
@tholonious tholonious requested a review from a team as a code owner July 5, 2023 21:38
Copy link
Contributor

@AeonSw4n AeonSw4n left a comment

Choose a reason for hiding this comment

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

Looks good! Got one comment regarding the handling of preSignature flag. In addition, I think we should increase our test coverage around MsgDeSoHeader, in particular testing the signing of the header hash.

// The preSignature flag is unused during byte encoding in
// in header versions 0 and 1. We set it to true to ensure that
// it's forward compatible for versions 2 and beyond.
headerBytes, err := msg.ToBytes(true)
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah yes so my initial comment re preSignature flag is applicable. We should add tests for signing the Hash to make sure everything is bulletproof.

Copy link
Contributor Author

@tholonious tholonious Jul 6, 2023

Choose a reason for hiding this comment

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

Done! I added more testing coverage here: 854d3e9 and 5ddecbf

@tholonious tholonious merged commit cb1d99f into feature/pos-messaging-schema Jul 6, 2023
@tholonious tholonious deleted the sa/add-block-propser-keys-and-signature-to-block-header branch July 6, 2023 14:36
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.

4 participants