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

Sync with new block message proto #84

Merged

Conversation

stanislavlyalin
Copy link
Contributor

This PR is an update of this library in accordance with the changes in the proto files in Block metadata refactoring (part 2) PR. Fixed broken tests.

@zsluedem zsluedem changed the base branch from master to 0.4.0-alpha May 12, 2022 15:14
rchain/crypto.py Outdated
block.sender,
StringValue(value=block.sigAlgorithm).SerializeToString(),
Int32Value(value=block.seqNum).SerializeToString(),
StringValue(value=block.shardId).SerializeToString(),
block.extraBytes])
StringValue(value=block.shardId).SerializeToString()])

Choose a reason for hiding this comment

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

Block hash must include all fields except signature and block hash.
https://github.com/rchain/rchain/pull/3688/files#diff-4a2241d942011ebc9d96deb51072793f5fc957fc3da883deab43e721aec7e34cR86-R95

Serialize can be called on the whole BlockMessageProto object with these two fields deleted. If it's mutable copy can be created.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@stanislavlyalin stanislavlyalin force-pushed the sync-with-new-block-message-proto branch from 6e213de to 90ea9bf Compare May 13, 2022 07:36
Copy link

@tgrospic tgrospic left a comment

Choose a reason for hiding this comment

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

LGTM

@zsluedem zsluedem merged commit 978cf38 into rchain:0.4.0-alpha May 13, 2022
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.

3 participants