added support for IOnChainAllocator #11
Conversation
| address compactContract, | ||
| uint256 nonce, | ||
| address recipient, | ||
| uint256[2][] calldata idsAndAmounts, |
There was a problem hiding this comment.
one thing that's a little funky is that there doesn't seem to be a way to prepare an allocation for an unknown amount (e.g. fee on transfer balance post-deposit). That said, I think it's still workable in most cases through other means before you get to the preparation stage
There was a problem hiding this comment.
It is! You can leave the amounts empty in my implementation, it will only use the amounts it retrieved directly via its balance checks. The only check I do is in the OnChainAllocator to make sure the amounts do not exceed 224.max because that is not supported. The only I did idsAndAmounts is because it is the exact same format as batchDepositAndRegisterFor (plus you can do small checks like the unit224.max check)
| error InvalidPreparation(); | ||
| error InvalidRegistration(address recipient, bytes32 claimHash, bytes32 typehash); | ||
|
|
||
| function prepareAllocation( |
There was a problem hiding this comment.
in this step, shouldn't we also ensure that the compact isn't already registered (so that you can't prepare and execute multiple times?)
There was a problem hiding this comment.
I thought about this as well, but you can't register it multiple times because the nonce increments each execution
| } | ||
| } | ||
|
|
||
| function executeAllocation( |
There was a problem hiding this comment.
tstore should get cleared so you can't execute multiple times
There was a problem hiding this comment.
See comment before:
As far as I can tell, there is no way to reexecute because of the incrementing nonce and it's nice to save a couple 100 gas. But please let me know if you still see a way to do the execution again!
0age
left a comment
0age
left a comment
There was a problem hiding this comment.
left a few notes (as single comments) that we should look into but mainly looking good
Preparations to add support for the IOnChainAllocator interface Uniswap/the-compact#154