Packing overrides and filler #221
Conversation
| // The time at which price becomes static | ||
| uint256 decayEndTime; | ||
| // Encoded exclusiveFiller, inputOverride, and outputOverrides, where needed | ||
| // First byte: fff0 0000 where f is a flag signalling inclusion of the 3 variables |
There was a problem hiding this comment.
and the last 5 bits are the length of outputs array?
| bytes32 constant OUTPUTS_LENGTH_FLAG_MASK = 0x1F00000000000000000000000000000000000000000000000000000000000000; | ||
|
|
||
| // "True" first bit of byte 1 signals that there is an exclusive filler | ||
| function hasExclusiveFiller(bytes memory extraData) internal pure returns (bool flag) { |
There was a problem hiding this comment.
would it be even cheaper to just take a bytes1 flags or bytes32 flags here? so we only have to do the mload once? or is that canceled out by the masking and expansion?
There was a problem hiding this comment.
oh yeah good point i'll try that
| pure | ||
| returns (address filler, uint256 inputOverride, uint256[] memory outputOverrides) | ||
| { | ||
| if (extraData.length == 0) return (filler, inputOverride, outputOverrides); |
There was a problem hiding this comment.
can just do empty return right?
There was a problem hiding this comment.
also in what case would we ever have 0 length?
There was a problem hiding this comment.
length==0 when there is no exclusive filler, no inputOverrides, and no outputoverrides... so i guess an open order where someone fills the exact amount in the order itself
|
|
||
| if (hasExclusiveFiller(extraData)) { | ||
| // + 20 bytes for address, - 32 bytes for the length offset | ||
| require(extraData.length >= bytesOffset - 12); |
There was a problem hiding this comment.
can we do a nicer custom err here?
| // The time at which the DutchOutputs start decaying | ||
| uint256 decayStartTime; | ||
| // The time at which price becomes static | ||
| uint256 decayEndTime; |
There was a problem hiding this comment.
any benefit to including these in extraData as well and having them be only uint32s? that may also allow slightly cleaner reading as you could just have parseInputOverrides(bytes encodedCosignerData) -> CosignerData or something
| uint256 inputOverride, | ||
| uint256 outputOverride, | ||
| uint256 outputOverrideLength |
There was a problem hiding this comment.
I think you can take arrays in the fuzzer if you want to test different amounts
|
skipping for now |
Very open to renaming things in this PR
extrabytes is used to encode any or all of exclusiveFiller, inputOverride, and outputOverrides.
If none of these 3 fields are needed for a tranasction, extraBytes is empty
In the case where at least 1 of these values is included:
First byte:
hasExclusiveFillerhasInputOverridehasOutputOverridesoutputOverrides.lengthAfter this first byte, the relevant fields are appended in the order
exclusiveFiller,inputOverride,outputOverrides.exclusiveFilleris 20 bytes,inputOverrideis 32 bytes, andoutputOverridesis(32 * outputOverrides.length)bytes.An
exactInputtrade with an exclusive filler (assuming the filler overrides the outputs), should save:--> saving 75 x 0 bytes
This isn't huge gas savings on mainnet (though is still decent as you can see) but is much higher savings on L2s