-
Notifications
You must be signed in to change notification settings - Fork 3
On chain allocator #10
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
Conversation
b4a3aac to
d99dc22
Compare
marktoda
left a comment
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.
first pass thoughts
| // Decode the orderData | ||
| (Order calldata orderData, uint32 deposit) = _decodeOrderData(order_.orderData); | ||
|
|
||
| uint160 caller = uint160(deposit * uint160(msg.sender)); // for a deposit, the nonce will be scoped to the caller + user |
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.
unclear to me why we do this multiplication - could use better explanation I think
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.
could this be internalized into _toNonceId?
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.
should deposit be depositId instead? just deposit makes me think its an amount
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 will create a better documentation for it!
deposit is actually a boolean in the Order struct, so it can always be either 0 or 1. I just use it as a uint32, because it is stored in the same calldata slot as the expires in the OrderDataOnChain:
struct OrderDataOnChain {
Order order; // The remaining BatchCompact and Mandate data
uint256 expires; // COMPACT - The time at which the claim expires and the user is able to withdraw their funds.
}
struct OrderDataGasless {
Order order; // The remaining BatchCompact and Mandate data
bool deposit; // Weather the order includes a deposit of the relevant tokens. This allows to skip a sponsor confirmation
}Thats why it is returned as "additionalData" from the _decodeOrderData function, since it is different depending on if the open or openFor function is called. Which is nice in this case, because it allows us to use it for calculations.
Basically, if you allocateAndRegister for someone else (so deposit == 1), you will then receive an individual nonce, completely independent of the sponsors incrementing nonces. This is to prevent griefing. Imagine a user creates a signature for a specific nonce, and a relayer will do the allocation for him. If allocateAndRegister was using the same incrementing nonces of the user, someone could essentially "burn" a valid signature by incrementing the nonce via a deposit (allocateAndRegister) of some worthless token on the users behalf.
If I would always scope the nonce to the caller though, the users signature would always be scoped to a specific relayer and worthless to any other relayer (except if they happen to currently be at the exact same nonce).
Thats why I multiply by deposit. If it is doing a allocateAndRegister call (deposit == 1), the callers signature will become part of the nonce scope (msg.sender * 1). If the caller is doing an allocateFor call (deposit == 0), their address will get wiped to address zero and the nonce will be in the scope of the user. This way, the system fully support incrementing nonces which is way cheaper (updating a storage slot instead of writing to a clean one each allocation) and easier, because the contract just assigns an incoming allocation a nonce, which means a user does not have to know do any nonce management beforehand.
| revert InvalidNonce(order_.nonce, nonces[nonceIdentifier] + 1); | ||
| } | ||
|
|
||
| bytes32 qualification = bytes32(uint256(orderData.qualification) * deposit); // delete qualification if not a deposit |
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 could be more clear I think - ie why? maybe like "branchless: if not deposit => 0, else qualification * deposit" or something
9c83938 to
09ca4db
Compare
added support for IOnChainAllocator
Pull Request
Description
Adding the OnChainAllocator and the ERC7683Allocator.
OnChainAllocator
ERC7683Allocation
Not all contracts are fully tested yet.