Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Packing overrides and filler #221

Closed
wants to merge 8 commits into from
Closed
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1 +1 @@
188845
187839
2 changes: 1 addition & 1 deletion .forge-snapshots/Base-V2DutchOrder-ExecuteBatch.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
210669
208628
Original file line number Diff line number Diff line change
@@ -1 +1 @@
221076
218396
Original file line number Diff line number Diff line change
@@ -1 +1 @@
275385
272068
Original file line number Diff line number Diff line change
@@ -1 +1 @@
204195
202154
2 changes: 1 addition & 1 deletion .forge-snapshots/Base-V2DutchOrder-ExecuteSingle.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
155052
154046
Original file line number Diff line number Diff line change
@@ -1 +1 @@
140619
139613
Original file line number Diff line number Diff line change
@@ -1 +1 @@
164362
163357
96 changes: 96 additions & 0 deletions src/lib/CosignerDataLib.sol
Original file line number Diff line number Diff line change
@@ -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;
Comment on lines +5 to +8
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

// Encoded exclusiveFiller, inputOverride, and outputOverrides, where needed
// First byte: fff0 0000 where f is a flag signalling inclusion of the 3 variables
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and the last 5 bits are the length of outputs array?

// 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) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh yeah good point i'll try that

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);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can just do empty return right?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also in what case would we ever have 0 length?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

// 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);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we do a nicer custom err here?

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;
}
}
}
}
14 changes: 1 addition & 13 deletions src/lib/V2DutchOrderLib.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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;
// 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;
}
import {CosignerData} from "./CosignerDataLib.sol";

struct V2DutchOrder {
// generic order information
Expand Down
42 changes: 26 additions & 16 deletions src/reactors/V2DutchOrderReactor.sol
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ 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} from "../lib/CosignerDataLib.sol";
import {V2DutchOrderLib, V2DutchOrder, CosignerData, DutchOutput, DutchInput} from "../lib/V2DutchOrderLib.sol";
import {SignedOrder, ResolvedOrder} from "../base/ReactorStructs.sol";

Expand All @@ -23,6 +24,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();
Expand Down Expand Up @@ -53,8 +55,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,
Expand All @@ -63,7 +68,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
Expand All @@ -78,25 +83,30 @@ 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) {
revert InvalidOutputOverride();
}
for (uint256 i = 0; i < order.outputs.length; i++) {
DutchOutput memory output = order.outputs[i];
uint256 outputOverride = order.cosignerData.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;
}
}
}
Expand Down
Loading
Loading