-
Notifications
You must be signed in to change notification settings - Fork 3
feat(#3): implement tree-based nonce cancellation #55
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
base: feat/permit-tree
Are you sure you want to change the base?
Conversation
Tree-based nonce invalidation with NonceNode structure. Note: Some tests have dependencies on permit-tree structures. Core changes: - src/NonceManager.sol with tree-based invalidation - src/interfaces/INonceManager.sol with new structures - test/TreeCancellation.t.sol and TreeCancellationExtended.t.sol - JavaScript utilities for nonce trees 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
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 introduces tree-based nonce cancellation functionality to the Permit3 system, enabling efficient batch nonce invalidation across multiple chains with UI transparency. The implementation uses a generic TreeNodeLib library for EIP-712 tree reconstruction and adds NonceNode structures parallel to existing PermitNode functionality.
Key Changes:
- Generic TreeNodeLib for tree hash reconstruction shared between PermitNode and NonceNode
- NonceNode/NonceTree structs and multi-chain nonce invalidation support
- Helper utilities for tree construction and proof generation (JavaScript)
- Comprehensive test coverage for tree operations and edge cases
Reviewed Changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/lib/TreeNodeLib.sol | Generic library for EIP-712 tree node reconstruction with parameterized typehash |
| src/NonceManager.sol | Adds tree-based nonce invalidation with MULTICHAIN_INVALIDATE_NONCES_TYPEHASH |
| src/interfaces/INonceManager.sol | Adds NonceNode, NonceTree, NonceSignature structs for tree-based operations |
| test/utils/NonceNodeLibTester.sol | Test wrapper delegating to TreeNodeLib with NONCE_NODE_TYPEHASH |
| test/utils/TreeNodeLibTester.sol | Exposes TreeNodeLib functions for testing |
| test/utils/TestBase.sol | Adds helper functions for NonceNode hashing, proof generation, and tree operations |
| test/TreeNodeLib.t.sol | Comprehensive tests for generic TreeNodeLib with multiple typehashes |
| test/TreeCancellation.t.sol | Integration tests for tree-based nonce cancellation flow |
| test/TreeCancellationExtended.t.sol | Extended edge case tests for tree cancellation scenarios |
| test/NonceManager.t.sol | Updates existing tests to use new NonceTree/NonceSignature structs |
| test/ZeroAddressValidation.t.sol | Updates function signatures to match new struct-based API |
| utils/verify-nonceNode-solidity-match.js | Verification script ensuring JS and Solidity produce matching hashes |
| utils/test-nonceNode.js | Test suite for NonceNode utility functions |
| utils/demo-nonceNode.js | Usage demonstration for NonceNode utilities |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/NonceManager.sol
Outdated
| * @notice Generate EIP-712 hash for NoncesToInvalidate struct | ||
| * @dev Hashes the struct for use as a leaf in tree reconstruction | ||
| * @dev Uses single-nonce optimization for gas efficiency | ||
| * @dev Sorts salts before hashing for consistency |
Copilot
AI
Oct 31, 2025
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.
The documentation claims that salts are sorted before hashing, but the implementation in hashNoncesToInvalidate does not sort the salts array. The salts are hashed in the order they are provided via keccak256(abi.encodePacked(invalidations.salts)). This inconsistency between documentation and implementation could lead to confusion.
src/NonceManager.sol
Outdated
| if (nonceNode.nonces[i].salts.length == 1) { | ||
| nonceInvalidationHashes[i] = nonceNode.nonces[i].salts[0]; | ||
| } else { | ||
| // Multiple nonces - hash as NoncesToInvalidate struct (no sorting, preserve order) |
Copilot
AI
Oct 31, 2025
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.
Comment states 'no sorting, preserve order' but this contradicts the function-level documentation on line 189 which claims salts are sorted. The implementation correctly preserves order (no sorting), but the function-level documentation needs to be corrected.
test/utils/TestBase.sol
Outdated
| _sortBytes32Array(sortedSalts); | ||
|
|
Copilot
AI
Oct 31, 2025
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.
The test helper _hashNonceNode sorts salts within a NoncesToInvalidate struct before hashing (lines 272-276), but the production code in NonceManager.hashNoncesToInvalidate and NonceManager.hashNonceNode does NOT sort salts. This mismatch will cause test hashes to differ from production hashes when NoncesToInvalidate contains multiple nonces in non-sorted order.
| _sortBytes32Array(sortedSalts); |
# Conflicts: # test/utils/TestBase.sol
# Conflicts: # test/utils/TestBase.sol
# Conflicts: # test/ZeroAddressValidation.t.sol
Remove salt sorting from test helper _hashNonceNode to match production NonceManager.hashNoncesToInvalidate behavior. Update documentation to clarify that salt order is preserved (not sorted). This fixes the test-production mismatch identified by GitHub Copilot: - Test was sorting salts before hashing - Production was preserving order - Different inputs could produce different hashes between test and prod Now both test and production preserve salt order for consistent hashing. Addresses: GitHub Copilot review comment on PR #55
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.
Don't think any of these JS files are supposed to be comitted.
| * @param signature EIP-712 signature authorizing the invalidation | ||
| * @param sig Signature data (owner, deadline, signature) | ||
| */ | ||
| function invalidateNonces( |
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.
Same question here. Can't we just treat single-chain cancellation as a sing-node tree?
src/NonceManager.sol
Outdated
| if (tree.currentChainInvalidations.salts.length == 0) { | ||
| revert EmptyArray(); | ||
| } |
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.
You are not checking emptyness in the other invalidateNonces above, and also not checking in the Permit3 corresponding function, so why bother here? Also, hashNoncesToInvalidate expectes empty salts and handles it no problem.
src/NonceManager.sol
Outdated
| * @param nonceNode The nonce node tree structure to hash | ||
| * @return bytes32 The EIP-712 hash of the complete nonce node structure | ||
| */ | ||
| function hashNonceNode( |
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.
internal function but never used?!
Address PR feedback: - Remove unused hashNonceNode internal function - Remove redundant emptiness check (handled by hashNoncesToInvalidate) - Remove unused _sortBytes32Array helper function Changes: - Removed hashNonceNode function (only used tests via _hashNonceNode) - Removed redundant empty array check at line 164-166 - Removed orphaned _sortBytes32Array function after hashNonceNode removal - Simplified code by trusting hashNoncesToInvalidate to handle edge cases
…me7mv5nn2QUpN6XQ refactor: remove unused code and redundant check in NonceManager
Summary
Implement tree-based nonce invalidation system with UI transparency and gas-efficient on-chain verification using TreeNodeLib.
Users sign a complete
NonceNodetree structure showing all nonces being cancelled across chains, while the contract receives a compact proof.Dependencies
feat/tree-lib) - merge that first!✅ Independent of PR #54 - can be reviewed/merged in parallel
Core Changes
New Structures
NonceNode- Tree structure for batch nonce cancellationNonceTree- Compact on-chain structure (proofStructure + proof)NonceSignature- Simplified signature struct (no salt/timestamp needed)Leaf Structure Change
bytes32[]nonces (raw salts)NoncesToInvalidate[](structured with chainId)New Typehashes
INVALIDATE_NONCES_TYPEHASH- Single-chain nonce invalidationMULTICHAIN_INVALIDATE_NONCES_TYPEHASH- Multi-chain tree invalidationNONCE_NODE_TYPEHASH- Internal tree node hashingUpdated Files
src/NonceManager.sol- Tree reconstruction using TreeNodeLibsrc/interfaces/INonceManager.sol- New structs and signaturesTest Coverage
New Tests:
test/TreeCancellation.t.sol(899 lines) - Comprehensive nonce cancellation teststest/TreeCancellationExtended.t.sol(219 lines) - Extended scenariostest/utils/NonceNodeLibTester.sol(79 lines) - Test helperUpdated Tests:
test/NonceManager.t.sol- Updated for new APIJavaScript Utilities
Complete toolkit for nonce tree construction:
utils/demo-nonceNode.js(144 lines) - Interactive demoutils/test-nonceNode.js(247 lines) - JS test suiteutils/verify-nonceNode-solidity-match.js(220 lines) - Cross-validation between JS and SolidityKey Benefits
✅ UI Transparency: Users see all nonces being cancelled in wallet
✅ Batch Operations: Single signature for multiple nonce cancellations
✅ Gas Efficient: O(log n) proof size with linear gas scaling
✅ API Consistency: Matches Permit3 patterns (structs, tree encoding)
API Example
Before (single-chain):
invalidateNonces(owner, deadline, invalidations, proof, signature);After (single-chain):
After (multi-chain):
API Pattern Alignment
The new nonce API mirrors the permit API from PR #54:
PermitTree/NonceTree)Signature/NonceSignature)proofStructure+proofencodingNote
Some tests reference permit-tree structures (expected cross-dependency). These will be resolved naturally when both PRs are merged to main.
🤖 Generated with Claude Code