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

docs: fix message type hash in snip12 guide #1273 #1274

Merged

Conversation

trishtzy
Copy link
Contributor

@trishtzy trishtzy commented Dec 28, 2024

Fixes #1273

This PR updates the SNIP-12 documentation guide with a corrected message type hash value. Here are the key changes:

  1. Typed Message Update:

    • Changed the typed message in the selector from
      • "expiry\":\"u64\" to "expiry\":\"u128\" and,
      • "u256"("low":"felt","high":"felt") to "u256"("low":"u128","high":"u128")
    • Added a clarifying comment explaining that since SNIP-12 doesn't support u64, u128 is used for the expiry field instead.
  2. Hash Value Update:

    • The message type hash constant was updated throughout the document from: 0x120ae1bdaf7c1e48349da94bb8dad27351ca115d6605ce345aee02d68d99ec1 to: 0x28bf13f11bba405c77ce010d2781c5903cbed100f01f72fcff1664f98343eb6

PR Checklist

  • Tests
  • Documentation
  • Added entry to CHANGELOG.md
  • Tried the feature on a public network

CHANGELOG.md Outdated Show resolved Hide resolved
@trishtzy trishtzy marked this pull request as ready for review December 28, 2024 04:27
@trishtzy trishtzy force-pushed the fix/snip12-message-type-hash-#1273 branch from c6d0a8b to a901bb9 Compare December 28, 2024 05:09
);
----

which is the same as:

[,cairo]
----
let message_type_hash = 0x120ae1bdaf7c1e48349da94bb8dad27351ca115d6605ce345aee02d68d99ec1;
let message_type_hash = 0xa2a7036c1f406af7c47722b209f23bd2f2d6ac21423c8c73bd92cf28409ee2;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
let message_type_hash = 0xa2a7036c1f406af7c47722b209f23bd2f2d6ac21423c8c73bd92cf28409ee2;
let message_type_hash = 0x1bd8318a34d524539867e6c110cb97d385c745575910b6c2afec623432ee55;

This is the hash I'm getting^

Can you confirm?

use core::to_byte_array::FormatAsByteArray;

let res = selector!("\"Message\"(\"recipient\":\"ContractAddress\",\"amount\":\"u256\",\"nonce\":\"felt\",\"expiry\":\"u128\")\"u256\"(\"low\":\"felt\",\"high\":\"felt\")");
println!("res: {}", res); // 49196983552140144393093620210836098680955957904181459224647480219440180821

let hex = res.format_as_byte_array(16);
println!("hex: 0x{}", hex); // 0x1bd8318a34d524539867e6c110cb97d385c745575910b6c2afec623432ee55

Copy link
Contributor Author

@trishtzy trishtzy Dec 31, 2024

Choose a reason for hiding this comment

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

Sorry! I was using timestamp for expiry and forgot to re-generate the hash.

The expected hash should be 0x28bf13f11bba405c77ce010d2781c5903cbed100f01f72fcff1664f98343eb6, where u256 is broken down to u128 low, high values instead of felt (ref).

(Updated PR description too)

use core::to_byte_array::FormatAsByteArray;

let res = selector!("\"Message\"(\"recipient\":\"ContractAddress\",\"amount\":\"u256\",\"nonce\":\"felt\",\"expiry\":\"u128\")\"u256\"(\"low\":\"u128\",\"high\":\"u128\")");
println!("res: {}", res); // 1151882460384444625458237696336527865031165261341111302013491262776159780534

let hex = res.format_as_byte_array(16);
println!("hex: 0x{}", hex); // 0x28bf13f11bba405c77ce010d2781c5903cbed100f01f72fcff1664f98343eb6

Copy link
Collaborator

Choose a reason for hiding this comment

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

Good catch!

@trishtzy trishtzy force-pushed the fix/snip12-message-type-hash-#1273 branch from a901bb9 to 0e5ad17 Compare December 31, 2024 02:50
Copy link
Collaborator

@andrew-fleming andrew-fleming left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks, @trishtzy!

@andrew-fleming andrew-fleming merged commit 6d623a2 into OpenZeppelin:main Jan 2, 2025
6 checks passed
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.

SNIP12: Incorrect use of u64 in docs example
2 participants