add uniswapx v2 initial structs #213
Conversation
This commit fixes a couple typos. Fixes: N-03
Gas limit for native transfer can cause some reasonable smart contract wallets that perform operations in their receive function to be unable to receive native output from uniswapX. compatibilty preferred here Resolves M-02
Fixes: N-02
Fixes: L-05
Fixes: L-04
Previously there was ambiguous undefined behavior for if startTime and endTime were the same - This commit removes that ambiguity by enforcing startTime !== endTime
document magic numbers in orderquoter Fixes: L-08
* Remove unused return names * Update OrderQuoter.sol
| // The tokens that the swapper will provide when settling the order | ||
| uint256 inputOverride; | ||
| // The tokens that must be received to satisfy the order | ||
| uint256[] outputOverrides; |
There was a problem hiding this comment.
do we need inputOverride and outputOverrides? if a filler is overriding, theyre going to do so at exactly the bps that are required to override right (because why would they override by more).
There was a problem hiding this comment.
inputOverride is for exactOutput trades, outputOverrides are for exactInput
if a filler is overriding, theyre going to do so at exactly the bps that are required to override right (because why would they override by more).
note its not up to the filler to override in this case, these are strictly enforced on top of the inner order based on the secondary auction. True that fillers wouldn't do more than required, but that's ok in this case
src/reactors/V2DutchOrderReactor.sol
Outdated
| sig: signedOrder.sig, | ||
| hash: order.hash() | ||
| }); | ||
| resolvedOrder.handleOverride(order.exclusiveFiller, order.decayStartTime, order.exclusivityOverrideBps); |
There was a problem hiding this comment.
maybe im missing something but i feel like the override logic is weird…
- first in validateOrder it validates the overrides exist and arent bigger/smaller than the original amount but not their %s
- then it goes through again and overrides the actual amounts of the order with the override values that were provided, without checking that they were big enough
- then it calls
handleOverridewhich increases all of the values of the order with one increased by x%
There was a problem hiding this comment.
i feel like you definitely dont need both updateOverrides and handleOverrides
There was a problem hiding this comment.
ohh ok theres 2 types of overrides - the first 2 are the newly introduced cosigner overrides which define the result of the secondary auction. Agreed we can remove the validation loop
the third bullet is the filler override which we already had in xv1, it basically is if some other public market filler that didn't compete in the auction has a significantly better price they can fill it at some fixed rate above the resolved price. I think we might want to remove that for v2 but keeping in anyways
There was a problem hiding this comment.
overloaded the terminology :D
There was a problem hiding this comment.
removing the public override for now
This commit flattens the V2 struct to maintain the reactor interface requirement that OrderInfo should be first struct so integrations like quoters can reliably fetch the reactor address and other order info fields Also saves a decent amt of gas from less encoding/decoding
|
|
||
| (bytes32 r, bytes32 s) = abi.decode(order.cosignature, (bytes32, bytes32)); | ||
| uint8 v = uint8(order.cosignature[64]); | ||
| // cosigner signs over (orderHash || cosignerData) |
There was a problem hiding this comment.
this means orderHash 'concat' cosignerData right? at first I thought it's logical or
Although I guess EIP-712 spec used ‖ the same way too
There was a problem hiding this comment.
oh yea its concat
There was a problem hiding this comment.
i was taught at university that this was the symbol for concat... but now that im googling i think that ive just been living a lie 😆
Add Initial UniswapXV2 reactor implementation