From 24cde907c77f1c82feca375a7b812ec04e83c30e Mon Sep 17 00:00:00 2001 From: hensha256 Date: Thu, 1 Feb 2024 16:17:47 -0300 Subject: [PATCH 1/7] Initial draft --- src/lib/V2DutchOrderLib.sol | 93 +++++++++++++++++++++++-- src/reactors/V2DutchOrderReactor.sol | 32 ++++++--- test/reactors/V2DutchOrderReactor.t.sol | 72 +++++++++---------- 3 files changed, 143 insertions(+), 54 deletions(-) diff --git a/src/lib/V2DutchOrderLib.sol b/src/lib/V2DutchOrderLib.sol index 59f88d8c..881493df 100644 --- a/src/lib/V2DutchOrderLib.sol +++ b/src/lib/V2DutchOrderLib.sol @@ -10,12 +10,12 @@ struct CosignerData { uint256 decayStartTime; // The time at which price becomes static uint256 decayEndTime; - // The address who has exclusive rights to the order until decayStartTime - address exclusiveFiller; - // 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; + // Encoded exclusiveFiller, inputOverride, and outputOverrides, where needed + // First byte: fff0 0000 where f is a flag signalling inclusion of the 3 variables + // exclusiveFiller: The address who has exclusive rights to the order until decayStartTime + // inputOverride: The number of tokens that the swapper will provide when settling the order + // outputOverrides: The tokens that must be received to satisfy the order + bytes extraData; } struct V2DutchOrder { @@ -80,3 +80,84 @@ library V2DutchOrderLib { ); } } + +library CosignerExtraDataLib { + bytes32 constant EXCL_FILLER_FLAG_MASK = 0x8000000000000000000000000000000000000000000000000000000000000000; + bytes32 constant INPUT_OVERRIDE_FLAG_MASK = 0x4000000000000000000000000000000000000000000000000000000000000000; + bytes32 constant OUTPUT_OVERRIDE_FLAG_MASK = 0x2000000000000000000000000000000000000000000000000000000000000000; + + // "True" first bit of byte 1 signals that there is an exclusive filler + function hasExclusiveFiller(bytes memory extraData) internal pure returns (bool flag) { + if (extraData.length == 0) return false; + assembly { + flag := and(mload(extraData), EXCL_FILLER_FLAG_MASK) + } + } + + // "True" second bit of byte 1 signals that there is an input override + function hasInputOverride(bytes memory extraData) internal pure returns (bool flag) { + if (extraData.length == 0) return false; + assembly { + flag := and(mload(extraData), INPUT_OVERRIDE_FLAG_MASK) + } + } + + // "True" third bit of byte 1 signals that there is an output override + function hasOutputOverrides(bytes memory extraData) internal pure returns (bool flag) { + if (extraData.length == 0) return false; + assembly { + flag := and(mload(extraData), OUTPUT_OVERRIDE_FLAG_MASK) + } + } + + function decodeExtraParameters(bytes memory extraData) + internal + pure + returns (address filler, uint256 inputOverride, uint256[] memory outputOverrides) + { + if (extraData.length == 0) return (filler, inputOverride, outputOverrides); + // The first byte (index 0) only contains flags of whether each field is included in the bytes + // So we can start from index 1 to start decoding each field + uint256 bytesOffset = 1; + + if (hasExclusiveFiller(extraData)) { + // an address is 20 bytes long + require(extraData.length >= bytesOffset + 20); + assembly { + // it loads a full 32 bytes, shift right 96 bits so only the address remains + filler := shr(mload(add(extraData, bytesOffset)), 96) + } + bytesOffset += 20; + } + + if (hasInputOverride(extraData)) { + // a uint256 is 32 bytes long, so we just load the next 32 bytes + require(extraData.length >= bytesOffset + 32); + assembly { + inputOverride := mload(add(extraData, bytesOffset)) + } + bytesOffset += 32; + } + + if (hasOutputOverrides(extraData)) { + require(extraData.length >= bytesOffset + 32); + uint256 length; + assembly { + length := mload(add(extraData, bytesOffset)) + } + bytesOffset += 32; + + // each element of the array is 32 bytes + require(extraData.length == bytesOffset + length * 32); + outputOverrides = new uint256[](length); + + uint256 outputOverride; + for (uint256 i = 0; i < length; i++) { + assembly { + outputOverride := mload(add(extraData, add(bytesOffset, mul(i, 32)))) + } + outputOverrides[i] = outputOverride; + } + } + } +} diff --git a/src/reactors/V2DutchOrderReactor.sol b/src/reactors/V2DutchOrderReactor.sol index a23bd7c3..64a156f4 100644 --- a/src/reactors/V2DutchOrderReactor.sol +++ b/src/reactors/V2DutchOrderReactor.sol @@ -6,7 +6,14 @@ import {IPermit2} from "permit2/src/interfaces/IPermit2.sol"; import {Permit2Lib} from "../lib/Permit2Lib.sol"; import {ExclusivityLib} from "../lib/ExclusivityLib.sol"; import {DutchDecayLib} from "../lib/DutchDecayLib.sol"; -import {V2DutchOrderLib, V2DutchOrder, CosignerData, DutchOutput, DutchInput} from "../lib/V2DutchOrderLib.sol"; +import { + CosignerExtraDataLib, + V2DutchOrderLib, + V2DutchOrder, + CosignerData, + DutchOutput, + DutchInput +} from "../lib/V2DutchOrderLib.sol"; import {SignedOrder, ResolvedOrder} from "../base/ReactorStructs.sol"; /// @notice Reactor for v2 dutch orders @@ -23,6 +30,7 @@ contract V2DutchOrderReactor is BaseReactor { using V2DutchOrderLib for V2DutchOrder; using DutchDecayLib for DutchOutput[]; using DutchDecayLib for DutchInput; + using CosignerExtraDataLib for bytes; /// @notice thrown when an order's deadline is before its end time error DeadlineBeforeEndTime(); @@ -53,8 +61,11 @@ contract V2DutchOrderReactor is BaseReactor { // hash the order _before_ overriding amounts, as this is the hash the user would have signed bytes32 orderHash = order.hash(); + (address exclusiveFiller, uint256 inputOverride, uint256[] memory outputOverrides) = + order.cosignerData.extraData.decodeExtraParameters(); + _validateOrder(orderHash, order); - _updateWithOverrides(order); + _updateWithOverrides(order, inputOverride, outputOverrides); resolvedOrder = ResolvedOrder({ info: order.info, @@ -63,7 +74,7 @@ contract V2DutchOrderReactor is BaseReactor { sig: signedOrder.sig, hash: orderHash }); - ExclusivityLib.handleStrictExclusivity(order.cosignerData.exclusiveFiller, order.cosignerData.decayStartTime); + ExclusivityLib.handleStrictExclusivity(exclusiveFiller, order.cosignerData.decayStartTime); } /// @inheritdoc BaseReactor @@ -78,20 +89,23 @@ contract V2DutchOrderReactor is BaseReactor { ); } - function _updateWithOverrides(V2DutchOrder memory order) internal pure { - if (order.cosignerData.inputOverride != 0) { - if (order.cosignerData.inputOverride > order.input.startAmount) { + function _updateWithOverrides(V2DutchOrder memory order, uint256 inputOverride, uint256[] memory outputOverrides) + internal + pure + { + if (inputOverride != 0) { + if (inputOverride > order.input.startAmount) { revert InvalidInputOverride(); } - order.input.startAmount = order.cosignerData.inputOverride; + order.input.startAmount = inputOverride; } - if (order.cosignerData.outputOverrides.length != order.outputs.length) { + if (outputOverrides.length != order.outputs.length) { revert InvalidOutputOverride(); } for (uint256 i = 0; i < order.outputs.length; i++) { DutchOutput memory output = order.outputs[i]; - uint256 outputOverride = order.cosignerData.outputOverrides[i]; + uint256 outputOverride = outputOverrides[i]; if (outputOverride != 0) { if (outputOverride < output.startAmount) { revert InvalidOutputOverride(); diff --git a/test/reactors/V2DutchOrderReactor.t.sol b/test/reactors/V2DutchOrderReactor.t.sol index bb61717d..2cce80cb 100644 --- a/test/reactors/V2DutchOrderReactor.t.sol +++ b/test/reactors/V2DutchOrderReactor.t.sol @@ -33,6 +33,8 @@ contract V2DutchOrderTest is PermitSignature, DeployPermit2, BaseDutchOrderReact uint256 constant cosignerPrivateKey = 0x99999999; + uint256[] internal NO_OUTPUT_OVERRIDES = new uint256[](0); + function name() public pure override returns (string memory) { return "V2DutchOrder"; } @@ -59,17 +61,10 @@ contract V2DutchOrderTest is PermitSignature, DeployPermit2, BaseDutchOrderReact }); } - uint256[] memory outputOverrides = new uint256[](request.outputs.length); - for (uint256 i = 0; i < request.outputs.length; i++) { - outputOverrides[i] = 0; - } - CosignerData memory cosignerData = CosignerData({ decayStartTime: block.timestamp, decayEndTime: request.info.deadline, - exclusiveFiller: address(0), - inputOverride: 0, - outputOverrides: outputOverrides + extraData: encodeExtraCosignerData(address(0), 0, NO_OUTPUT_OVERRIDES) }); V2DutchOrder memory order = V2DutchOrder({ @@ -93,16 +88,10 @@ contract V2DutchOrderTest is PermitSignature, DeployPermit2, BaseDutchOrderReact override returns (SignedOrder memory signedOrder, bytes32 orderHash) { - uint256[] memory outputOverrides = new uint256[](request.outputs.length); - for (uint256 i = 0; i < request.outputs.length; i++) { - outputOverrides[i] = 0; - } CosignerData memory cosignerData = CosignerData({ decayStartTime: request.decayStartTime, decayEndTime: request.decayEndTime, - exclusiveFiller: address(0), - inputOverride: 0, - outputOverrides: outputOverrides + extraData: encodeExtraCosignerData(address(0), 0, NO_OUTPUT_OVERRIDES) }); V2DutchOrder memory order = V2DutchOrder({ @@ -124,9 +113,7 @@ contract V2DutchOrderTest is PermitSignature, DeployPermit2, BaseDutchOrderReact CosignerData memory cosignerData = CosignerData({ decayStartTime: block.timestamp, decayEndTime: block.timestamp + 100, - exclusiveFiller: address(0), - inputOverride: 1 ether, - outputOverrides: ArrayBuilder.fill(1, 1 ether) + extraData: encodeExtraCosignerData(address(0), 1 ether, ArrayBuilder.fill(1, 1 ether)) }); V2DutchOrder memory order = V2DutchOrder({ @@ -148,10 +135,7 @@ contract V2DutchOrderTest is PermitSignature, DeployPermit2, BaseDutchOrderReact CosignerData memory cosignerData = CosignerData({ decayStartTime: block.timestamp, decayEndTime: block.timestamp + 100, - exclusiveFiller: address(0), - // override is more input tokens than expected - inputOverride: 0.9 ether, - outputOverrides: ArrayBuilder.fill(1, 1.1 ether) + extraData: encodeExtraCosignerData(address(0), 0.9 ether, ArrayBuilder.fill(1, 1.1 ether)) }); V2DutchOrder memory order = V2DutchOrder({ info: OrderInfoBuilder.init(address(reactor)).withSwapper(swapper), @@ -172,10 +156,7 @@ contract V2DutchOrderTest is PermitSignature, DeployPermit2, BaseDutchOrderReact CosignerData memory cosignerData = CosignerData({ decayStartTime: block.timestamp, decayEndTime: block.timestamp + 100, - exclusiveFiller: address(0), - // override is more input tokens than expected - inputOverride: 1 ether, - outputOverrides: ArrayBuilder.fill(1, 0.9 ether) + extraData: encodeExtraCosignerData(address(0), 1 ether, ArrayBuilder.fill(1, 0.9 ether)) }); V2DutchOrder memory order = V2DutchOrder({ info: OrderInfoBuilder.init(address(reactor)).withSwapper(swapper), @@ -196,10 +177,7 @@ contract V2DutchOrderTest is PermitSignature, DeployPermit2, BaseDutchOrderReact CosignerData memory cosignerData = CosignerData({ decayStartTime: block.timestamp, decayEndTime: block.timestamp + 100, - exclusiveFiller: address(0), - // override is more input tokens than expected - inputOverride: 1 ether, - outputOverrides: ArrayBuilder.fill(2, 1.1 ether) + extraData: encodeExtraCosignerData(address(0), 1 ether, ArrayBuilder.fill(2, 1.1 ether)) }); V2DutchOrder memory order = V2DutchOrder({ info: OrderInfoBuilder.init(address(reactor)).withSwapper(swapper), @@ -225,9 +203,7 @@ contract V2DutchOrderTest is PermitSignature, DeployPermit2, BaseDutchOrderReact CosignerData memory cosignerData = CosignerData({ decayStartTime: block.timestamp, decayEndTime: block.timestamp + 100, - exclusiveFiller: address(0), - inputOverride: overriddenInputAmount, - outputOverrides: ArrayBuilder.fill(1, 0) + extraData: encodeExtraCosignerData(address(0), overriddenInputAmount, NO_OUTPUT_OVERRIDES) }); V2DutchOrder memory order = V2DutchOrder({ @@ -256,9 +232,7 @@ contract V2DutchOrderTest is PermitSignature, DeployPermit2, BaseDutchOrderReact CosignerData memory cosignerData = CosignerData({ decayStartTime: block.timestamp, decayEndTime: block.timestamp + 100, - exclusiveFiller: address(0), - inputOverride: 0, - outputOverrides: ArrayBuilder.fill(1, overriddenOutputAmount) + extraData: encodeExtraCosignerData(address(0), 0, ArrayBuilder.fill(1, overriddenOutputAmount)) }); V2DutchOrder memory order = V2DutchOrder({ @@ -286,9 +260,7 @@ contract V2DutchOrderTest is PermitSignature, DeployPermit2, BaseDutchOrderReact CosignerData memory cosignerData = CosignerData({ decayStartTime: block.timestamp, decayEndTime: block.timestamp + 100, - exclusiveFiller: address(1), - inputOverride: 0, - outputOverrides: ArrayBuilder.fill(1, 0) + extraData: encodeExtraCosignerData(address(1), 0, NO_OUTPUT_OVERRIDES) }); V2DutchOrder memory order = V2DutchOrder({ @@ -319,4 +291,26 @@ contract V2DutchOrderTest is PermitSignature, DeployPermit2, BaseDutchOrderReact result[i] = SignedOrder(abi.encode(orders[i]), sig); } } + + function encodeExtraCosignerData(address exclusiveFiller, uint256 inputOverride, uint256[] memory outputOverrides) + private + pure + returns (bytes memory extraData) + { + bool hasExclusiveFiller = (exclusiveFiller != address(0)); + bool hasInputOverride = (inputOverride != 0); + bool hasOutputOverrides = (outputOverrides.length == 0); + + bytes1 firstByte = 0x00; + if (hasExclusiveFiller) firstByte |= 0x80; + if (hasInputOverride) firstByte |= 0x40; + if (hasOutputOverrides) firstByte |= 0x20; + + if (firstByte == 0x00) return ""; + + extraData = abi.encodePacked(firstByte); + if (hasExclusiveFiller) bytes.concat(extraData, abi.encodePacked(exclusiveFiller)); + if (hasInputOverride) bytes.concat(extraData, abi.encodePacked(inputOverride)); + if (hasOutputOverrides) bytes.concat(extraData, abi.encode(outputOverrides)); + } } From d63c7d4212c5e640802de2116354b54a959ceaab Mon Sep 17 00:00:00 2001 From: hensha256 Date: Thu, 1 Feb 2024 20:17:48 -0300 Subject: [PATCH 2/7] Bug fixes --- ...V2DutchOrder-BaseExecuteSingleWithFee.snap | 2 +- .../Base-V2DutchOrder-ExecuteBatch.snap | 2 +- ...utchOrder-ExecuteBatchMultipleOutputs.snap | 2 +- ...teBatchMultipleOutputsDifferentTokens.snap | 2 +- ...V2DutchOrder-ExecuteBatchNativeOutput.snap | 2 +- .../Base-V2DutchOrder-ExecuteSingle.snap | 2 +- ...2DutchOrder-ExecuteSingleNativeOutput.snap | 2 +- ...-V2DutchOrder-ExecuteSingleValidation.snap | 2 +- src/lib/V2DutchOrderLib.sol | 15 ++++++----- src/reactors/V2DutchOrderReactor.sol | 22 ++++++++------- test/reactors/V2DutchOrderReactor.t.sol | 27 ++++++++++++++++--- 11 files changed, 51 insertions(+), 29 deletions(-) diff --git a/.forge-snapshots/Base-V2DutchOrder-BaseExecuteSingleWithFee.snap b/.forge-snapshots/Base-V2DutchOrder-BaseExecuteSingleWithFee.snap index 8ee2724e..f95509d3 100644 --- a/.forge-snapshots/Base-V2DutchOrder-BaseExecuteSingleWithFee.snap +++ b/.forge-snapshots/Base-V2DutchOrder-BaseExecuteSingleWithFee.snap @@ -1 +1 @@ -188845 \ No newline at end of file +187839 \ No newline at end of file diff --git a/.forge-snapshots/Base-V2DutchOrder-ExecuteBatch.snap b/.forge-snapshots/Base-V2DutchOrder-ExecuteBatch.snap index 114edd4c..f4bb3159 100644 --- a/.forge-snapshots/Base-V2DutchOrder-ExecuteBatch.snap +++ b/.forge-snapshots/Base-V2DutchOrder-ExecuteBatch.snap @@ -1 +1 @@ -210669 \ No newline at end of file +208628 \ No newline at end of file diff --git a/.forge-snapshots/Base-V2DutchOrder-ExecuteBatchMultipleOutputs.snap b/.forge-snapshots/Base-V2DutchOrder-ExecuteBatchMultipleOutputs.snap index ed5bc4b8..f2b19a69 100644 --- a/.forge-snapshots/Base-V2DutchOrder-ExecuteBatchMultipleOutputs.snap +++ b/.forge-snapshots/Base-V2DutchOrder-ExecuteBatchMultipleOutputs.snap @@ -1 +1 @@ -221076 \ No newline at end of file +218396 \ No newline at end of file diff --git a/.forge-snapshots/Base-V2DutchOrder-ExecuteBatchMultipleOutputsDifferentTokens.snap b/.forge-snapshots/Base-V2DutchOrder-ExecuteBatchMultipleOutputsDifferentTokens.snap index 5de710b2..1a5b8119 100644 --- a/.forge-snapshots/Base-V2DutchOrder-ExecuteBatchMultipleOutputsDifferentTokens.snap +++ b/.forge-snapshots/Base-V2DutchOrder-ExecuteBatchMultipleOutputsDifferentTokens.snap @@ -1 +1 @@ -275385 \ No newline at end of file +272068 \ No newline at end of file diff --git a/.forge-snapshots/Base-V2DutchOrder-ExecuteBatchNativeOutput.snap b/.forge-snapshots/Base-V2DutchOrder-ExecuteBatchNativeOutput.snap index 8325a204..33dccbe5 100644 --- a/.forge-snapshots/Base-V2DutchOrder-ExecuteBatchNativeOutput.snap +++ b/.forge-snapshots/Base-V2DutchOrder-ExecuteBatchNativeOutput.snap @@ -1 +1 @@ -204195 \ No newline at end of file +202154 \ No newline at end of file diff --git a/.forge-snapshots/Base-V2DutchOrder-ExecuteSingle.snap b/.forge-snapshots/Base-V2DutchOrder-ExecuteSingle.snap index 71d55745..4c4bb29b 100644 --- a/.forge-snapshots/Base-V2DutchOrder-ExecuteSingle.snap +++ b/.forge-snapshots/Base-V2DutchOrder-ExecuteSingle.snap @@ -1 +1 @@ -155052 \ No newline at end of file +154046 \ No newline at end of file diff --git a/.forge-snapshots/Base-V2DutchOrder-ExecuteSingleNativeOutput.snap b/.forge-snapshots/Base-V2DutchOrder-ExecuteSingleNativeOutput.snap index 40e24830..1679097e 100644 --- a/.forge-snapshots/Base-V2DutchOrder-ExecuteSingleNativeOutput.snap +++ b/.forge-snapshots/Base-V2DutchOrder-ExecuteSingleNativeOutput.snap @@ -1 +1 @@ -140619 \ No newline at end of file +139613 \ No newline at end of file diff --git a/.forge-snapshots/Base-V2DutchOrder-ExecuteSingleValidation.snap b/.forge-snapshots/Base-V2DutchOrder-ExecuteSingleValidation.snap index fa9e1ada..a8b66dc3 100644 --- a/.forge-snapshots/Base-V2DutchOrder-ExecuteSingleValidation.snap +++ b/.forge-snapshots/Base-V2DutchOrder-ExecuteSingleValidation.snap @@ -1 +1 @@ -164362 \ No newline at end of file +163357 \ No newline at end of file diff --git a/src/lib/V2DutchOrderLib.sol b/src/lib/V2DutchOrderLib.sol index 881493df..0933e968 100644 --- a/src/lib/V2DutchOrderLib.sol +++ b/src/lib/V2DutchOrderLib.sol @@ -90,7 +90,7 @@ library CosignerExtraDataLib { function hasExclusiveFiller(bytes memory extraData) internal pure returns (bool flag) { if (extraData.length == 0) return false; assembly { - flag := and(mload(extraData), EXCL_FILLER_FLAG_MASK) + flag := and(mload(add(extraData, 32)), EXCL_FILLER_FLAG_MASK) } } @@ -98,7 +98,7 @@ library CosignerExtraDataLib { function hasInputOverride(bytes memory extraData) internal pure returns (bool flag) { if (extraData.length == 0) return false; assembly { - flag := and(mload(extraData), INPUT_OVERRIDE_FLAG_MASK) + flag := and(mload(add(extraData, 32)), INPUT_OVERRIDE_FLAG_MASK) } } @@ -106,7 +106,7 @@ library CosignerExtraDataLib { function hasOutputOverrides(bytes memory extraData) internal pure returns (bool flag) { if (extraData.length == 0) return false; assembly { - flag := and(mload(extraData), OUTPUT_OVERRIDE_FLAG_MASK) + flag := and(mload(add(extraData, 32)), OUTPUT_OVERRIDE_FLAG_MASK) } } @@ -116,16 +116,17 @@ library CosignerExtraDataLib { returns (address filler, uint256 inputOverride, uint256[] memory outputOverrides) { if (extraData.length == 0) return (filler, inputOverride, outputOverrides); + // The first 32 bytes are length // The first byte (index 0) only contains flags of whether each field is included in the bytes - // So we can start from index 1 to start decoding each field - uint256 bytesOffset = 1; + // So we can start from index 1 (after 32) to start decoding each field + uint256 bytesOffset = 33; if (hasExclusiveFiller(extraData)) { // an address is 20 bytes long require(extraData.length >= bytesOffset + 20); assembly { // it loads a full 32 bytes, shift right 96 bits so only the address remains - filler := shr(mload(add(extraData, bytesOffset)), 96) + filler := shr(96, mload(add(extraData, bytesOffset))) } bytesOffset += 20; } @@ -148,7 +149,7 @@ library CosignerExtraDataLib { bytesOffset += 32; // each element of the array is 32 bytes - require(extraData.length == bytesOffset + length * 32); + require(extraData.length == bytesOffset + (length - 1) * 32); outputOverrides = new uint256[](length); uint256 outputOverride; diff --git a/src/reactors/V2DutchOrderReactor.sol b/src/reactors/V2DutchOrderReactor.sol index 64a156f4..64591ee9 100644 --- a/src/reactors/V2DutchOrderReactor.sol +++ b/src/reactors/V2DutchOrderReactor.sol @@ -100,17 +100,19 @@ contract V2DutchOrderReactor is BaseReactor { order.input.startAmount = inputOverride; } - if (outputOverrides.length != order.outputs.length) { - revert InvalidOutputOverride(); - } - for (uint256 i = 0; i < order.outputs.length; i++) { - DutchOutput memory output = order.outputs[i]; - uint256 outputOverride = outputOverrides[i]; - if (outputOverride != 0) { - if (outputOverride < output.startAmount) { - revert InvalidOutputOverride(); + if (outputOverrides.length != 0) { + if (outputOverrides.length != order.outputs.length) { + revert InvalidOutputOverride(); + } + for (uint256 i = 0; i < order.outputs.length; i++) { + DutchOutput memory output = order.outputs[i]; + uint256 outputOverride = outputOverrides[i]; + if (outputOverride != 0) { + if (outputOverride < output.startAmount) { + revert InvalidOutputOverride(); + } + output.startAmount = outputOverride; } - output.startAmount = outputOverride; } } } diff --git a/test/reactors/V2DutchOrderReactor.t.sol b/test/reactors/V2DutchOrderReactor.t.sol index 2cce80cb..1b4c1c2d 100644 --- a/test/reactors/V2DutchOrderReactor.t.sol +++ b/test/reactors/V2DutchOrderReactor.t.sol @@ -6,6 +6,7 @@ import {DeployPermit2} from "../util/DeployPermit2.sol"; import { V2DutchOrder, V2DutchOrderLib, + CosignerExtraDataLib, CosignerData, V2DutchOrderReactor, ResolvedOrder, @@ -30,6 +31,7 @@ import {DutchOrder, BaseDutchOrderReactorTest} from "./BaseDutchOrderReactor.t.s contract V2DutchOrderTest is PermitSignature, DeployPermit2, BaseDutchOrderReactorTest { using OrderInfoBuilder for OrderInfo; using V2DutchOrderLib for V2DutchOrder; + using CosignerExtraDataLib for bytes; uint256 constant cosignerPrivateKey = 0x99999999; @@ -252,6 +254,20 @@ contract V2DutchOrderTest is PermitSignature, DeployPermit2, BaseDutchOrderReact assertEq(tokenIn.balanceOf(address(fillContract)), inputAmount); } + function testEncoding() public { + bytes memory encodedBytes = encodeExtraCosignerData(address(1111111111111111), 22222222222222, ArrayBuilder.fill(3, 5)); + assertTrue(encodedBytes.hasExclusiveFiller()); + assertTrue(encodedBytes.hasInputOverride()); + assertTrue(encodedBytes.hasOutputOverrides()); + (address filler, uint256 input, uint256[] memory output) = encodedBytes.decodeExtraParameters(); + assertEq(filler, address(1111111111111111)); + assertEq(input, 22222222222222); + assertEq(output.length, 3); + assertEq(output[0], 5); + assertEq(output[1], 5); + assertEq(output[2], 5); + } + function testExclusivity() public { uint256 inputAmount = 1 ether; tokenIn.mint(swapper, inputAmount); @@ -299,7 +315,7 @@ contract V2DutchOrderTest is PermitSignature, DeployPermit2, BaseDutchOrderReact { bool hasExclusiveFiller = (exclusiveFiller != address(0)); bool hasInputOverride = (inputOverride != 0); - bool hasOutputOverrides = (outputOverrides.length == 0); + bool hasOutputOverrides = (outputOverrides.length != 0); bytes1 firstByte = 0x00; if (hasExclusiveFiller) firstByte |= 0x80; @@ -309,8 +325,11 @@ contract V2DutchOrderTest is PermitSignature, DeployPermit2, BaseDutchOrderReact if (firstByte == 0x00) return ""; extraData = abi.encodePacked(firstByte); - if (hasExclusiveFiller) bytes.concat(extraData, abi.encodePacked(exclusiveFiller)); - if (hasInputOverride) bytes.concat(extraData, abi.encodePacked(inputOverride)); - if (hasOutputOverrides) bytes.concat(extraData, abi.encode(outputOverrides)); + if (hasExclusiveFiller) extraData = bytes.concat(extraData, abi.encodePacked(exclusiveFiller)); + if (hasInputOverride) extraData = bytes.concat(extraData, abi.encodePacked(inputOverride)); + if (hasOutputOverrides) { + extraData = bytes.concat(extraData, abi.encodePacked(outputOverrides.length)); + extraData = bytes.concat(extraData, abi.encodePacked(outputOverrides)); + } } } From 1a475e204517ed46629cd8da78808506a1e96d9c Mon Sep 17 00:00:00 2001 From: hensha256 Date: Thu, 1 Feb 2024 20:37:40 -0300 Subject: [PATCH 3/7] All tests passing --- src/lib/V2DutchOrderLib.sol | 10 +++++----- test/reactors/V2DutchOrderReactor.t.sol | 3 ++- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/lib/V2DutchOrderLib.sol b/src/lib/V2DutchOrderLib.sol index 0933e968..be1f0fe7 100644 --- a/src/lib/V2DutchOrderLib.sol +++ b/src/lib/V2DutchOrderLib.sol @@ -122,8 +122,8 @@ library CosignerExtraDataLib { uint256 bytesOffset = 33; if (hasExclusiveFiller(extraData)) { - // an address is 20 bytes long - require(extraData.length >= bytesOffset + 20); + // + 20 bytes for address, - 32 bytes for the length offset + require(extraData.length >= bytesOffset - 12); assembly { // it loads a full 32 bytes, shift right 96 bits so only the address remains filler := shr(96, mload(add(extraData, bytesOffset))) @@ -132,8 +132,8 @@ library CosignerExtraDataLib { } if (hasInputOverride(extraData)) { - // a uint256 is 32 bytes long, so we just load the next 32 bytes - require(extraData.length >= bytesOffset + 32); + // + 32 bytes for uint256, - 32 bytes for the length offset + require(extraData.length >= bytesOffset); assembly { inputOverride := mload(add(extraData, bytesOffset)) } @@ -148,7 +148,7 @@ library CosignerExtraDataLib { } bytesOffset += 32; - // each element of the array is 32 bytes + // each element of the array is 32 bytes, - 32 bytes for the length offset require(extraData.length == bytesOffset + (length - 1) * 32); outputOverrides = new uint256[](length); diff --git a/test/reactors/V2DutchOrderReactor.t.sol b/test/reactors/V2DutchOrderReactor.t.sol index 1b4c1c2d..15d5482c 100644 --- a/test/reactors/V2DutchOrderReactor.t.sol +++ b/test/reactors/V2DutchOrderReactor.t.sol @@ -255,7 +255,8 @@ contract V2DutchOrderTest is PermitSignature, DeployPermit2, BaseDutchOrderReact } function testEncoding() public { - bytes memory encodedBytes = encodeExtraCosignerData(address(1111111111111111), 22222222222222, ArrayBuilder.fill(3, 5)); + bytes memory encodedBytes = + encodeExtraCosignerData(address(1111111111111111), 22222222222222, ArrayBuilder.fill(3, 5)); assertTrue(encodedBytes.hasExclusiveFiller()); assertTrue(encodedBytes.hasInputOverride()); assertTrue(encodedBytes.hasOutputOverrides()); From e63b15d64654b63bfaca5b20bae209c691bc26b7 Mon Sep 17 00:00:00 2001 From: hensha256 Date: Thu, 1 Feb 2024 21:00:13 -0300 Subject: [PATCH 4/7] Move length into header byte --- src/lib/V2DutchOrderLib.sol | 5 ++--- test/reactors/V2DutchOrderReactor.t.sol | 9 +++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/lib/V2DutchOrderLib.sol b/src/lib/V2DutchOrderLib.sol index be1f0fe7..32748b06 100644 --- a/src/lib/V2DutchOrderLib.sol +++ b/src/lib/V2DutchOrderLib.sol @@ -85,6 +85,7 @@ library CosignerExtraDataLib { bytes32 constant EXCL_FILLER_FLAG_MASK = 0x8000000000000000000000000000000000000000000000000000000000000000; bytes32 constant INPUT_OVERRIDE_FLAG_MASK = 0x4000000000000000000000000000000000000000000000000000000000000000; bytes32 constant OUTPUT_OVERRIDE_FLAG_MASK = 0x2000000000000000000000000000000000000000000000000000000000000000; + 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) { @@ -141,12 +142,10 @@ library CosignerExtraDataLib { } if (hasOutputOverrides(extraData)) { - require(extraData.length >= bytesOffset + 32); uint256 length; assembly { - length := mload(add(extraData, bytesOffset)) + length := shr(248, and(mload(add(extraData, 32)), OUTPUTS_LENGTH_FLAG_MASK)) } - bytesOffset += 32; // each element of the array is 32 bytes, - 32 bytes for the length offset require(extraData.length == bytesOffset + (length - 1) * 32); diff --git a/test/reactors/V2DutchOrderReactor.t.sol b/test/reactors/V2DutchOrderReactor.t.sol index 15d5482c..1b9c1941 100644 --- a/test/reactors/V2DutchOrderReactor.t.sol +++ b/test/reactors/V2DutchOrderReactor.t.sol @@ -322,15 +322,16 @@ contract V2DutchOrderTest is PermitSignature, DeployPermit2, BaseDutchOrderReact if (hasExclusiveFiller) firstByte |= 0x80; if (hasInputOverride) firstByte |= 0x40; if (hasOutputOverrides) firstByte |= 0x20; + if (hasOutputOverrides) { + require(outputOverrides.length < 32); + firstByte |= bytes1(uint8(outputOverrides.length)); + } if (firstByte == 0x00) return ""; extraData = abi.encodePacked(firstByte); if (hasExclusiveFiller) extraData = bytes.concat(extraData, abi.encodePacked(exclusiveFiller)); if (hasInputOverride) extraData = bytes.concat(extraData, abi.encodePacked(inputOverride)); - if (hasOutputOverrides) { - extraData = bytes.concat(extraData, abi.encodePacked(outputOverrides.length)); - extraData = bytes.concat(extraData, abi.encodePacked(outputOverrides)); - } + if (hasOutputOverrides) extraData = bytes.concat(extraData, abi.encodePacked(outputOverrides)); } } From 2d2760c5a8b0e708a2d78de59b75568324958873 Mon Sep 17 00:00:00 2001 From: hensha256 Date: Thu, 1 Feb 2024 21:26:22 -0300 Subject: [PATCH 5/7] Remove helper test --- test/reactors/V2DutchOrderReactor.t.sol | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/test/reactors/V2DutchOrderReactor.t.sol b/test/reactors/V2DutchOrderReactor.t.sol index 1b9c1941..b04ba527 100644 --- a/test/reactors/V2DutchOrderReactor.t.sol +++ b/test/reactors/V2DutchOrderReactor.t.sol @@ -254,21 +254,6 @@ contract V2DutchOrderTest is PermitSignature, DeployPermit2, BaseDutchOrderReact assertEq(tokenIn.balanceOf(address(fillContract)), inputAmount); } - function testEncoding() public { - bytes memory encodedBytes = - encodeExtraCosignerData(address(1111111111111111), 22222222222222, ArrayBuilder.fill(3, 5)); - assertTrue(encodedBytes.hasExclusiveFiller()); - assertTrue(encodedBytes.hasInputOverride()); - assertTrue(encodedBytes.hasOutputOverrides()); - (address filler, uint256 input, uint256[] memory output) = encodedBytes.decodeExtraParameters(); - assertEq(filler, address(1111111111111111)); - assertEq(input, 22222222222222); - assertEq(output.length, 3); - assertEq(output[0], 5); - assertEq(output[1], 5); - assertEq(output[2], 5); - } - function testExclusivity() public { uint256 inputAmount = 1 ether; tokenIn.mint(swapper, inputAmount); From cf4e70be594c399ad5221b1e7ef3c09afa7ba48e Mon Sep 17 00:00:00 2001 From: hensha256 Date: Fri, 2 Feb 2024 11:52:33 -0300 Subject: [PATCH 6/7] Move new lib into different file --- src/lib/CosignerDataLib.sol | 96 +++++++++++++++++++++++++ src/lib/V2DutchOrderLib.sol | 95 +----------------------- src/reactors/V2DutchOrderReactor.sol | 10 +-- test/reactors/V2DutchOrderReactor.t.sol | 2 +- 4 files changed, 100 insertions(+), 103 deletions(-) create mode 100644 src/lib/CosignerDataLib.sol diff --git a/src/lib/CosignerDataLib.sol b/src/lib/CosignerDataLib.sol new file mode 100644 index 00000000..3b54b995 --- /dev/null +++ b/src/lib/CosignerDataLib.sol @@ -0,0 +1,96 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +pragma solidity ^0.8.0; + +struct CosignerData { + // The time at which the DutchOutputs start decaying + uint256 decayStartTime; + // 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 + // exclusiveFiller: The address who has exclusive rights to the order until decayStartTime + // inputOverride: The number of tokens that the swapper will provide when settling the order + // outputOverrides: The tokens that must be received to satisfy the order + bytes extraData; +} + +library CosignerExtraDataLib { + bytes32 constant EXCL_FILLER_FLAG_MASK = 0x8000000000000000000000000000000000000000000000000000000000000000; + bytes32 constant INPUT_OVERRIDE_FLAG_MASK = 0x4000000000000000000000000000000000000000000000000000000000000000; + bytes32 constant OUTPUT_OVERRIDE_FLAG_MASK = 0x2000000000000000000000000000000000000000000000000000000000000000; + 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) { + if (extraData.length == 0) return false; + assembly { + flag := and(mload(add(extraData, 32)), EXCL_FILLER_FLAG_MASK) + } + } + + // "True" second bit of byte 1 signals that there is an input override + function hasInputOverride(bytes memory extraData) internal pure returns (bool flag) { + if (extraData.length == 0) return false; + assembly { + flag := and(mload(add(extraData, 32)), INPUT_OVERRIDE_FLAG_MASK) + } + } + + // "True" third bit of byte 1 signals that there is an output override + function hasOutputOverrides(bytes memory extraData) internal pure returns (bool flag) { + if (extraData.length == 0) return false; + assembly { + flag := and(mload(add(extraData, 32)), OUTPUT_OVERRIDE_FLAG_MASK) + } + } + + function decodeExtraParameters(bytes memory extraData) + internal + pure + returns (address filler, uint256 inputOverride, uint256[] memory outputOverrides) + { + if (extraData.length == 0) return (filler, inputOverride, outputOverrides); + // The first 32 bytes are length + // The first byte (index 0) only contains flags of whether each field is included in the bytes + // So we can start from index 1 (after 32) to start decoding each field + uint256 bytesOffset = 33; + + if (hasExclusiveFiller(extraData)) { + // + 20 bytes for address, - 32 bytes for the length offset + require(extraData.length >= bytesOffset - 12); + assembly { + // it loads a full 32 bytes, shift right 96 bits so only the address remains + filler := shr(96, mload(add(extraData, bytesOffset))) + } + bytesOffset += 20; + } + + if (hasInputOverride(extraData)) { + // + 32 bytes for uint256, - 32 bytes for the length offset + require(extraData.length >= bytesOffset); + assembly { + inputOverride := mload(add(extraData, bytesOffset)) + } + bytesOffset += 32; + } + + if (hasOutputOverrides(extraData)) { + uint256 length; + assembly { + length := shr(248, and(mload(add(extraData, 32)), OUTPUTS_LENGTH_FLAG_MASK)) + } + + // each element of the array is 32 bytes, - 32 bytes for the length offset + require(extraData.length == bytesOffset + (length - 1) * 32); + outputOverrides = new uint256[](length); + + uint256 outputOverride; + for (uint256 i = 0; i < length; i++) { + assembly { + outputOverride := mload(add(extraData, add(bytesOffset, mul(i, 32)))) + } + outputOverrides[i] = outputOverride; + } + } + } +} diff --git a/src/lib/V2DutchOrderLib.sol b/src/lib/V2DutchOrderLib.sol index 32748b06..2112565b 100644 --- a/src/lib/V2DutchOrderLib.sol +++ b/src/lib/V2DutchOrderLib.sol @@ -4,19 +4,7 @@ pragma solidity ^0.8.0; import {OrderInfo} from "../base/ReactorStructs.sol"; import {DutchOutput, DutchInput, DutchOrderLib} from "./DutchOrderLib.sol"; import {OrderInfoLib} from "./OrderInfoLib.sol"; - -struct CosignerData { - // The time at which the DutchOutputs start decaying - uint256 decayStartTime; - // 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 - // exclusiveFiller: The address who has exclusive rights to the order until decayStartTime - // inputOverride: The number of tokens that the swapper will provide when settling the order - // outputOverrides: The tokens that must be received to satisfy the order - bytes extraData; -} +import {CosignerData} from "./CosignerDataLib.sol"; struct V2DutchOrder { // generic order information @@ -80,84 +68,3 @@ library V2DutchOrderLib { ); } } - -library CosignerExtraDataLib { - bytes32 constant EXCL_FILLER_FLAG_MASK = 0x8000000000000000000000000000000000000000000000000000000000000000; - bytes32 constant INPUT_OVERRIDE_FLAG_MASK = 0x4000000000000000000000000000000000000000000000000000000000000000; - bytes32 constant OUTPUT_OVERRIDE_FLAG_MASK = 0x2000000000000000000000000000000000000000000000000000000000000000; - 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) { - if (extraData.length == 0) return false; - assembly { - flag := and(mload(add(extraData, 32)), EXCL_FILLER_FLAG_MASK) - } - } - - // "True" second bit of byte 1 signals that there is an input override - function hasInputOverride(bytes memory extraData) internal pure returns (bool flag) { - if (extraData.length == 0) return false; - assembly { - flag := and(mload(add(extraData, 32)), INPUT_OVERRIDE_FLAG_MASK) - } - } - - // "True" third bit of byte 1 signals that there is an output override - function hasOutputOverrides(bytes memory extraData) internal pure returns (bool flag) { - if (extraData.length == 0) return false; - assembly { - flag := and(mload(add(extraData, 32)), OUTPUT_OVERRIDE_FLAG_MASK) - } - } - - function decodeExtraParameters(bytes memory extraData) - internal - pure - returns (address filler, uint256 inputOverride, uint256[] memory outputOverrides) - { - if (extraData.length == 0) return (filler, inputOverride, outputOverrides); - // The first 32 bytes are length - // The first byte (index 0) only contains flags of whether each field is included in the bytes - // So we can start from index 1 (after 32) to start decoding each field - uint256 bytesOffset = 33; - - if (hasExclusiveFiller(extraData)) { - // + 20 bytes for address, - 32 bytes for the length offset - require(extraData.length >= bytesOffset - 12); - assembly { - // it loads a full 32 bytes, shift right 96 bits so only the address remains - filler := shr(96, mload(add(extraData, bytesOffset))) - } - bytesOffset += 20; - } - - if (hasInputOverride(extraData)) { - // + 32 bytes for uint256, - 32 bytes for the length offset - require(extraData.length >= bytesOffset); - assembly { - inputOverride := mload(add(extraData, bytesOffset)) - } - bytesOffset += 32; - } - - if (hasOutputOverrides(extraData)) { - uint256 length; - assembly { - length := shr(248, and(mload(add(extraData, 32)), OUTPUTS_LENGTH_FLAG_MASK)) - } - - // each element of the array is 32 bytes, - 32 bytes for the length offset - require(extraData.length == bytesOffset + (length - 1) * 32); - outputOverrides = new uint256[](length); - - uint256 outputOverride; - for (uint256 i = 0; i < length; i++) { - assembly { - outputOverride := mload(add(extraData, add(bytesOffset, mul(i, 32)))) - } - outputOverrides[i] = outputOverride; - } - } - } -} diff --git a/src/reactors/V2DutchOrderReactor.sol b/src/reactors/V2DutchOrderReactor.sol index 64591ee9..8e3158fe 100644 --- a/src/reactors/V2DutchOrderReactor.sol +++ b/src/reactors/V2DutchOrderReactor.sol @@ -6,14 +6,8 @@ import {IPermit2} from "permit2/src/interfaces/IPermit2.sol"; import {Permit2Lib} from "../lib/Permit2Lib.sol"; import {ExclusivityLib} from "../lib/ExclusivityLib.sol"; import {DutchDecayLib} from "../lib/DutchDecayLib.sol"; -import { - CosignerExtraDataLib, - V2DutchOrderLib, - V2DutchOrder, - CosignerData, - DutchOutput, - DutchInput -} from "../lib/V2DutchOrderLib.sol"; +import {CosignerExtraDataLib} from "../lib/CosignerDataLib.sol"; +import {V2DutchOrderLib, V2DutchOrder, CosignerData, DutchOutput, DutchInput} from "../lib/V2DutchOrderLib.sol"; import {SignedOrder, ResolvedOrder} from "../base/ReactorStructs.sol"; /// @notice Reactor for v2 dutch orders diff --git a/test/reactors/V2DutchOrderReactor.t.sol b/test/reactors/V2DutchOrderReactor.t.sol index b04ba527..1927bff7 100644 --- a/test/reactors/V2DutchOrderReactor.t.sol +++ b/test/reactors/V2DutchOrderReactor.t.sol @@ -6,7 +6,6 @@ import {DeployPermit2} from "../util/DeployPermit2.sol"; import { V2DutchOrder, V2DutchOrderLib, - CosignerExtraDataLib, CosignerData, V2DutchOrderReactor, ResolvedOrder, @@ -14,6 +13,7 @@ import { DutchInput, BaseReactor } from "../../src/reactors/V2DutchOrderReactor.sol"; +import {CosignerExtraDataLib} from "../../src/lib/CosignerDataLib.sol"; import {OrderInfo, InputToken, SignedOrder, OutputToken} from "../../src/base/ReactorStructs.sol"; import {ExclusivityLib} from "../../src/lib/ExclusivityLib.sol"; import {DutchDecayLib} from "../../src/lib/DutchDecayLib.sol"; From 26ead0d373d7bf1b660174a22e08e33c75b6a775 Mon Sep 17 00:00:00 2001 From: hensha256 Date: Fri, 2 Feb 2024 12:08:05 -0300 Subject: [PATCH 7/7] Fuzz test for decoding function --- test/reactors/V2DutchOrderReactor.t.sol | 27 +++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/test/reactors/V2DutchOrderReactor.t.sol b/test/reactors/V2DutchOrderReactor.t.sol index 1927bff7..49ede6b8 100644 --- a/test/reactors/V2DutchOrderReactor.t.sol +++ b/test/reactors/V2DutchOrderReactor.t.sol @@ -280,6 +280,33 @@ contract V2DutchOrderTest is PermitSignature, DeployPermit2, BaseDutchOrderReact fillContract.execute(signedOrder); } + function testDecodeExtraParametersFuzz( + address filler, + uint256 inputOverride, + uint256 outputOverride, + uint256 outputOverrideLength + ) public { + vm.assume(outputOverrideLength < 32); + + bytes memory extraData = + encodeExtraCosignerData(filler, inputOverride, ArrayBuilder.fill(outputOverrideLength, outputOverride)); + + assertEq(filler != address(0), extraData.hasExclusiveFiller()); + assertEq(inputOverride != 0, extraData.hasInputOverride()); + assertEq(outputOverrideLength != 0, extraData.hasOutputOverrides()); + + (address decodedFiller, uint256 decodedInputOverride, uint256[] memory decodedOutputOverrides) = + extraData.decodeExtraParameters(); + + assertEq(decodedFiller, filler); + assertEq(decodedInputOverride, inputOverride); + assertEq(decodedOutputOverrides.length, outputOverrideLength); + + for (uint256 i = 0; i < outputOverrideLength; i++) { + assertEq(decodedOutputOverrides[i], outputOverride); + } + } + function cosignOrder(bytes32 orderHash, CosignerData memory cosignerData) private pure returns (bytes memory sig) { bytes32 msgHash = keccak256(abi.encodePacked(orderHash, abi.encode(cosignerData))); (uint8 v, bytes32 r, bytes32 s) = vm.sign(cosignerPrivateKey, msgHash);