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

HCS running hash should concatenate message hash & not message #36

Closed
2 tasks done
tinker-michaelj opened this issue May 12, 2020 · 0 comments
Closed
2 tasks done
Assignees
Labels
Bug An error that causes the feature to behave differently than what was expected based on design. Impact Potentially impacts SDK, clients, docs and/or users. P1 High priority issue, which must be completed in the milestone otherwise the release is at risk. Release Note Issues which need a release note created.

Comments

@tinker-michaelj
Copy link
Collaborator

Summary of the defect
The new topicRunningHash of a topic receiving a ConsensusSubmitMessage should be the SHA-384 digest of, in order:

  1. The previous topicRunningHash of the topic (48 bytes)
  2. The topicRunningHashVersion (8 bytes)
  3. The topic's shard (8 bytes)
  4. The topic's realm (8 bytes)
  5. The topic's number (8 bytes)
  6. The number of seconds since the epoch before the ConsensusSubmitMessage reached consensus (8 bytes)
  7. The number of nanoseconds since 6. before the ConsensusSubmitMessage reached consensus (4 bytes)
  8. The topicSequenceNumber (8 bytes)
  9. The output of the SHA-384 digest of the message bytes from the ConsensusSubmitMessage (48 bytes)
    But in fact, we have not versioned the topic running hash; and are updating the digest with the bytes of the submitted message, not their SHA-384 hash.

Suggested resolution

  • Add a topicRunningHashVersion to the protobuf TransactionReceipt type.
  • Correct the hash calculation.

Impact
Mirror nodes should store for each message, the version number of the record for it. That way, when checking the running hash, they know how to calculate it for both old messages and new messages.

@tinker-michaelj tinker-michaelj added Bug An error that causes the feature to behave differently than what was expected based on design. Impact Potentially impacts SDK, clients, docs and/or users. P1 High priority issue, which must be completed in the milestone otherwise the release is at risk. Release Note Issues which need a release note created. labels May 12, 2020
ljianghedera pushed a commit that referenced this issue Aug 1, 2020
Added a config file for myself, regression.sh improvement
tinker-michaelj pushed a commit that referenced this issue Sep 18, 2020
Documentation: Update the rest of the Protobufs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug An error that causes the feature to behave differently than what was expected based on design. Impact Potentially impacts SDK, clients, docs and/or users. P1 High priority issue, which must be completed in the milestone otherwise the release is at risk. Release Note Issues which need a release note created.
Projects
None yet
Development

No branches or pull requests

3 participants