audit(16): Round decays in favor of swapper #305
Conversation
| @@ -702,6 +703,54 @@ contract V3DutchOrderTest is PermitSignature, DeployPermit2, BaseReactorTest { | |||
| assertEq(resolvedOrder.input.amount, 0); | |||
| } | |||
|
|
|||
| // 1000 - (100 * (1659087340-1659029740) / (65535)) = 912.1 | |||
| // This is the input, which should round down to favor the swapper: 912 | |||
| function testV3ResolveInputEndBlockAfterNow() public { | |||
There was a problem hiding this comment.
nit: no test for output positive / negative slope?
There was a problem hiding this comment.
I left this comment in the audit. LMK what you think.
Good to flag this. I think the difference this will make in practice will be quite small and wouldn't justify the additional complexity. The decay's will likely be < 50 bps across ~32 blocks. This means we'd be at most .99% off due to rounding (.495 bps).
It's also not clear that being too high or too low on the curve is actually beneficial for the user since we don't know what the real price is (and it's changing in real time). Imagine the actual price is 3. In one world we round up to 3 and get executed in block N. In another world we round down to 2 and hit it at block N+1 (at 4 instead).
* Pass in decay function for input/output * forge fmt * Remove MockNonlinearDutchDecayLibContract
To favor the swapper, decay outputs should be rounded up and decay inputs should be rounded down.