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: consistent message hashes #72

Merged
merged 1 commit into from
Jan 10, 2025

Conversation

skosito
Copy link
Contributor

@skosito skosito commented Jan 5, 2025

https://github.com/zeta-chain/protocol-private/issues/181

I don't think there is danger of hash colisions, since this is hashed and signed by tss in zetaclient. However, there is good point to have hashes organized a bit better, and with expected field size.

Changes:

  • add ZETACHAIN at begining of hash (i think this is optional, but i guess nice to have)
  • order of arguments like instruction identifier, chain id, nonce is same in every hash
  • for instruction names, i guess there are a lot of strategies to have them occupy consistent space and not use variable length strings, i used enum, but open for discussion

@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 0% with 10 lines in your changes missing coverage. Please review.

Project coverage is 2.85%. Comparing base (cf38502) to head (fe5e506).

Files with missing lines Patch % Lines
programs/gateway/src/lib.rs 0.00% 10 Missing ⚠️
Additional details and impacted files
@@                       Coverage Diff                        @@
##           non-security-related-findings     #72      +/-   ##
================================================================
- Coverage                           2.86%   2.85%   -0.02%     
================================================================
  Files                                  1       1              
  Lines                                453     456       +3     
================================================================
  Hits                                  13      13              
- Misses                               440     443       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@lumtis lumtis left a comment

Choose a reason for hiding this comment

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

LGTM

@lumtis lumtis merged commit 554afcb into non-security-related-findings Jan 10, 2025
14 checks passed
@lumtis lumtis deleted the consistent-message-hashes branch January 10, 2025 09:49
skosito added a commit that referenced this pull request Jan 13, 2025
skosito added a commit that referenced this pull request Jan 13, 2025
skosito added a commit that referenced this pull request Jan 13, 2025
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