Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a new "Simple native orders" system that replaces the existing ETH orders system with a more streamlined approach using factory pattern and cloned contracts. The key change is shifting from a centralized ETH order management system to individual native order contracts deployed via a factory.
- Replaces ETHOrders contract with NativeOrderFactory and NativeOrderImpl contracts
- Implements a factory pattern using cloned contracts for individual orders
- Simplifies order creation, cancellation, and signature validation processes
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| test/helpers/utils.js | Adds helper function to extract event arguments from transaction receipts |
| test/LimitOrderProtocol.js | Updates tests to use new native order system instead of ETH orders |
| deploy/deploy-NativeOrderFactory.js | Adds deployment script for the new NativeOrderFactory contract |
| contracts/extensions/NativeOrderImpl.sol | New implementation contract for individual native orders with ERC1271 signature validation |
| contracts/extensions/NativeOrderFactory.sol | New factory contract for creating deterministic clones of native orders |
| contracts/extensions/ETHOrders.sol | Removes the old ETH orders implementation |
| contracts/OrderLib.sol | Adds memory-based order hashing function for in-memory order structs |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
k06a
force-pushed
the
feature/simple-native-order
branch
from
15a573e to
dd75891
Compare
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| function isValidSignature(bytes32 hash, bytes calldata /* signature */) external view returns(bytes4) { | ||
| uint80 orderHashTail = purpose.orderHashTail; | ||
| uint256 expiration = purpose.expiration; | ||
| if (uint80(uint256(hash)) != orderHashTail) { |
There was a problem hiding this comment.
Comparing only the lower 80 bits of the order hash is insufficient for security. Hash collisions become much more likely with truncated hashes, potentially allowing malicious actors to create orders with matching tails. Use the full hash for comparison.
There was a problem hiding this comment.
But 80 bits considered as strong enough in crypto
1 participant
No description provided.