Conversation
mgretzke
force-pushed
the
permit2-routing
branch
from
552f52e to
1ed1b77
Compare
0age
left a comment
0age
left a comment
There was a problem hiding this comment.
all looking good, left a few small comments
looks like we do still need to work out the precise methodology for composing onchain + offchain allocations in one shot, but this should be a pretty good starting point for that!
| AL.verifyNonce(nonce, AL.OFF_CHAIN_NONCE, sponsor); | ||
|
|
||
| // Check the allocator data for a valid signature by an authorized signer | ||
| bytes32 digest = _deriveDigest(claimHash, _COMPACT_DOMAIN_SEPARATOR); |
There was a problem hiding this comment.
note that if the domain separator on The Compact changes (e.g. from a chain fork) this will not work anymore
| /// @dev The actual nonce will be a combination of the next free nonce and the user address. | ||
| mapping(address user => uint96 nonce) public nonces; | ||
| /// @dev The actual nonce will be a combination of the on chain nonce command, the users address and the free nonce. | ||
| mapping(address user => uint88 nonce) public nonces; |
There was a problem hiding this comment.
i'd avoid "user" as terminology; better to refer to the actor in question (here it's the sponsor i believe)
| revert InvalidAmount(commitments[i].amount); | ||
| } | ||
|
|
||
| _storeAllocation( |
There was a problem hiding this comment.
since allocations are scoped to specific claim hashes anyway, I wonder if it'd be possible to store a single hash of all the respective commitments rather than one at a time
There was a problem hiding this comment.
this is more of a musing than a suggestion at this stage, i think we could explore that optimization down the line but just food for thought for now
| nonce := or(shl(96, sponsor), add(nonce96, 1)) | ||
| sstore(nonceSlot, add(nonce96, 1)) | ||
| let nonce88 := sload(nonceSlot) | ||
| nonce := or(onChainNonceCommand, or(shl(88, sponsor), add(nonce88, 1))) |
There was a problem hiding this comment.
minor thing but we could do the add right when we do the sload and save an operation over needing to do it twice
| if (newSigner_ == address(0) || msg.sender != newSigner_) { | ||
| revert InvalidSigner(); | ||
| /// @inheritdoc IHybridAllocator | ||
| function proposeOwnerReplacement(address newOwner_) external onlyOwner { |
There was a problem hiding this comment.
i actually think we should consider allowing address(0) here so you can "cancel" an ownership transfer; erroneously putting in address(0) is not that big of a deal since nothing will happen
2 participants
Pull Request
Description
This PR adds Permit2-based allocation support to both OnChainAllocator and HybridAllocator, enabling users to deposit, register, and allocate tokens in a single transaction using Permit2 signatures. It also introduces a new nonce command structure to differentiate between on-chain, off-chain, and Permit2 allocations.
New Features
Nonce Structure Refactoring