-
Notifications
You must be signed in to change notification settings - Fork 12
Nonce as uint256
#126
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
Nonce as uint256
#126
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR changes the nonce field type from u64 to Uint256 (a 256-bit unsigned integer) in the UserOperation and PackedUserOperation structures to support larger nonce values as required by the ERC-4337 standard.
Key changes:
- Introduces a new
Uint256wrapper type aroundprimitive_types::U256with serialization support - Updates all
UserOperationandPackedUserOperationstructs to useUint256for nonce fields - Adjusts language bindings (WASM and Python) to handle the larger integer type through string representations
Reviewed Changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| nucypher-core/src/prim_types.rs | New module defining the Uint256 wrapper type with conversion and serialization methods |
| nucypher-core/src/lib.rs | Exports the new Uint256 type from the public API |
| nucypher-core/src/signature_request.rs | Updates UserOperation and PackedUserOperation structs to use Uint256 for nonce, changes JSON serialization to use decimal strings |
| nucypher-core/Cargo.toml | Adds primitive-types dependency with serde support |
| nucypher-core-wasm/src/lib.rs | Updates WASM bindings to accept/return nonce as string |
| nucypher-core-wasm/tests/wasm.rs | Updates tests to use string representation for nonce values |
| nucypher-core-python/src/lib.rs | Updates Python bindings to accept PyLong and return BigUint for nonce |
| nucypher-core-python/Cargo.toml | Adds num-bigint dependency and enables pyo3 num-bigint feature |
| Cargo.lock | Updates dependency tree with new crates |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #126 +/- ##
==========================================
+ Coverage 63.51% 63.96% +0.44%
==========================================
Files 20 21 +1
Lines 4498 4554 +56
==========================================
+ Hits 2857 2913 +56
Misses 1641 1641 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…256 values specified in the spec.
d81e4fe to
82a8687
Compare
| } | ||
|
|
||
| /// Converts the `Uint256` to a big-endian byte array. | ||
| pub fn to_be_bytes(&self) -> [u8; 32] { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you consider to change this to to_big_endian_bytes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have no issue changing it at all.
For context, this is a common naming pattern in core Rust:
- https://doc.rust-lang.org/std/primitive.u32.html#method.to_be_bytes
- https://doc.rust-lang.org/std/primitive.u64.html#method.to_be_bytes
- https://doc.rust-lang.org/std/primitive.u128.html#method.to_be_bytes
and also followed by some of our existing code:
- https://github.com/nucypher/rust-umbral/blob/master/umbral-pre/src/keys.rs#L154
- https://github.com/nucypher/rust-umbral/blob/master/umbral-pre/src/keys.rs#L259
If this doesn’t change anything from your perspective, just say the word and I’ll update it.
Switch order so that Added is before Fixed.
Type of PR:
Required reviews:
What this does:
Issues fixed/closed:
Why it's needed:
Notes for reviewers: