From 6a9cd636754aa95857af0bb46df0b329a0f78299 Mon Sep 17 00:00:00 2001 From: Hammad Ghazi Date: Wed, 11 Oct 2023 17:34:22 +0500 Subject: [PATCH 01/47] implemented order specific sanity checks --- src/AdvancedOrderEngine.sol | 26 ++++++++++++++++++++++++++ src/AdvancedOrderEngineErrors.sol | 3 +++ 2 files changed, 29 insertions(+) diff --git a/src/AdvancedOrderEngine.sol b/src/AdvancedOrderEngine.sol index 33dcf3e..bc433a7 100644 --- a/src/AdvancedOrderEngine.sol +++ b/src/AdvancedOrderEngine.sol @@ -46,6 +46,32 @@ contract AdvancedOrderEngine { revert ZeroFacilitatorTargetAddress(); // TBD: use generic error message i.e. ZeroAddress() } + for (uint256 i; i < orders.length; ) { + OrderEngine.Order calldata order = orders[i]; + + bytes32 orderHash; + + if (block.timestamp > order.validTill) { + revert OrderExpired(orderHash); + } + + if (order.buyTokenAmount == 0 || order.sellTokenAmount == 0) { + revert ZeroTokenAmounts(); + } + + if ( + order.maker == address(0) || + order.buyToken == address(0) || + order.sellToken == address(0) + ) { + revert ZeroAddress(); + } + + unchecked { + ++i; + } + } + // Loop start // Perform order specific sanity checks // Verify signatjures diff --git a/src/AdvancedOrderEngineErrors.sol b/src/AdvancedOrderEngineErrors.sol index 8270062..c02a532 100644 --- a/src/AdvancedOrderEngineErrors.sol +++ b/src/AdvancedOrderEngineErrors.sol @@ -4,3 +4,6 @@ pragma solidity 0.8.21; error ArraysLengthMismatch(uint256 ordersArrLen, uint256 clearingPricesArrLen); error ZeroFacilitatorTargetAddress(); error EmptyOrdersArray(); +error OrderExpired(bytes32 orderHash); +error ZeroTokenAmounts(); +error ZeroAddress(); From 305fe990833eff6ae802f984e661a55ff40f03c5 Mon Sep 17 00:00:00 2001 From: Hammad Ghazi Date: Sun, 15 Oct 2023 17:26:23 +0500 Subject: [PATCH 02/47] fix: order schema Removed 'signature' member from 'Order' struct as it's essentially a message that users will sign. Having signature in the message makes the contract susceptible to signature malleability attacks. --- src/libraries/OrderEngine.sol | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libraries/OrderEngine.sol b/src/libraries/OrderEngine.sol index 124e541..eed7fd7 100644 --- a/src/libraries/OrderEngine.sol +++ b/src/libraries/OrderEngine.sol @@ -20,6 +20,5 @@ library OrderEngine { bytes predicates; bytes preInteraction; bytes postInteraction; - bytes signature; } } From 74b3fe427768059eebd49f572ac76448eb5c6ad1 Mon Sep 17 00:00:00 2001 From: Hammad Ghazi Date: Sun, 15 Oct 2023 17:32:29 +0500 Subject: [PATCH 03/47] feat: Import EIP712 and EIP1271 contracts for signature verification and define constructor --- src/AdvancedOrderEngine.sol | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/AdvancedOrderEngine.sol b/src/AdvancedOrderEngine.sol index bc433a7..f9b1705 100644 --- a/src/AdvancedOrderEngine.sol +++ b/src/AdvancedOrderEngine.sol @@ -1,12 +1,19 @@ // SPDX-License-Identifier: MIT pragma solidity 0.8.21; +import {EIP712} from "openzeppelin/utils/cryptography/EIP712.sol"; +import {IERC1271} from "openzeppelin/interfaces/IERC1271.sol"; import {OrderEngine} from "./libraries/OrderEngine.sol"; import "./AdvancedOrderEngineErrors.sol"; -contract AdvancedOrderEngine { +contract AdvancedOrderEngine is EIP712 { using OrderEngine for OrderEngine.Order; + constructor( + string memory name, + string memory version + ) EIP712(name, version) {} + /** * @notice Fills multiple orders by processing the specified orders and clearing prices. * From f7620c672582bb6e76d0433b6d9201196944c04c Mon Sep 17 00:00:00 2001 From: Hammad Ghazi Date: Sun, 15 Oct 2023 17:39:14 +0500 Subject: [PATCH 04/47] feat: implement order_type_hash for signature verification --- src/libraries/OrderEngine.sol | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/libraries/OrderEngine.sol b/src/libraries/OrderEngine.sol index eed7fd7..c1f9fa4 100644 --- a/src/libraries/OrderEngine.sol +++ b/src/libraries/OrderEngine.sol @@ -21,4 +21,25 @@ library OrderEngine { bytes preInteraction; bytes postInteraction; } + + bytes32 public constant ORDER_TYPE_HASH = + keccak256( + "Order(" + "uint256 nonce," + "uint256 validTill," + "uint256 sellTokenAmount," + "uint256 buyTokenAmount," + "uint256 feeAmounts," + "address maker," + "address taker," + "address recipient," + "address sellToken," + "address buyToken," + "bool isPartiallyFillable," + "bytes32 extraData," + "bytes predicates," + "bytes preInteraction," + "bytes postInteraction" + ")" + ); } From f5146924a654624d96b3e0f8bc41f0a9cdbfb8d2 Mon Sep 17 00:00:00 2001 From: Hammad Ghazi Date: Sun, 15 Oct 2023 17:41:34 +0500 Subject: [PATCH 05/47] feat: implement and use order hash fns for signature verification --- src/AdvancedOrderEngine.sol | 3 ++- src/libraries/OrderEngine.sol | 25 +++++++++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/src/AdvancedOrderEngine.sol b/src/AdvancedOrderEngine.sol index f9b1705..9afd705 100644 --- a/src/AdvancedOrderEngine.sol +++ b/src/AdvancedOrderEngine.sol @@ -56,7 +56,8 @@ contract AdvancedOrderEngine is EIP712 { for (uint256 i; i < orders.length; ) { OrderEngine.Order calldata order = orders[i]; - bytes32 orderHash; + bytes32 orderHash = order.hash(); + bytes32 orderMessageHash = _hashTypedDataV4(orderHash); if (block.timestamp > order.validTill) { revert OrderExpired(orderHash); diff --git a/src/libraries/OrderEngine.sol b/src/libraries/OrderEngine.sol index c1f9fa4..19df372 100644 --- a/src/libraries/OrderEngine.sol +++ b/src/libraries/OrderEngine.sol @@ -42,4 +42,29 @@ library OrderEngine { "bytes postInteraction" ")" ); + + function hash(Order calldata order) public pure returns (bytes32) { + return ( + keccak256( + abi.encode( + ORDER_TYPE_HASH, + order.nonce, + order.validTill, + order.sellTokenAmount, + order.buyTokenAmount, + order.feeAmounts, + order.maker, + order.taker, + order.recipient, + order.sellToken, + order.buyToken, + order.isPartiallyFillable, + order.extraData, + keccak256(order.predicates), + keccak256(order.preInteraction), + keccak256(order.postInteraction) + ) + ) + ); + } } From 9650e28ac251d30e64f9be4ca4cb7a05a5ee2833 Mon Sep 17 00:00:00 2001 From: Hammad Ghazi Date: Sun, 15 Oct 2023 18:09:53 +0500 Subject: [PATCH 06/47] fix: add signatures param in fillOrder fn --- src/AdvancedOrderEngine.sol | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/AdvancedOrderEngine.sol b/src/AdvancedOrderEngine.sol index 9afd705..09f1938 100644 --- a/src/AdvancedOrderEngine.sol +++ b/src/AdvancedOrderEngine.sol @@ -25,6 +25,7 @@ contract AdvancedOrderEngine is EIP712 { function fillOrders( OrderEngine.Order[] calldata orders, uint256[] calldata clearingPrices, + bytes[] calldata signatures, bytes calldata facilitatorInteractionCalldata, address facilitatorInteractionTargetContract ) external { @@ -55,6 +56,7 @@ contract AdvancedOrderEngine is EIP712 { for (uint256 i; i < orders.length; ) { OrderEngine.Order calldata order = orders[i]; + bytes calldata signature = signatures[i]; bytes32 orderHash = order.hash(); bytes32 orderMessageHash = _hashTypedDataV4(orderHash); From c77178c4946b38cd5995553cd932d78a02afc729 Mon Sep 17 00:00:00 2001 From: Hammad Ghazi Date: Sun, 15 Oct 2023 18:30:01 +0500 Subject: [PATCH 07/47] feat: implement signature verification core logic --- src/AdvancedOrderEngine.sol | 19 ++++++++++++++++++- src/AdvancedOrderEngineErrors.sol | 1 + src/libraries/OrderEngine.sol | 4 ++++ 3 files changed, 23 insertions(+), 1 deletion(-) diff --git a/src/AdvancedOrderEngine.sol b/src/AdvancedOrderEngine.sol index 09f1938..0b55d69 100644 --- a/src/AdvancedOrderEngine.sol +++ b/src/AdvancedOrderEngine.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT pragma solidity 0.8.21; -import {EIP712} from "openzeppelin/utils/cryptography/EIP712.sol"; +import {EIP712, ECDSA} from "openzeppelin/utils/cryptography/EIP712.sol"; import {IERC1271} from "openzeppelin/interfaces/IERC1271.sol"; import {OrderEngine} from "./libraries/OrderEngine.sol"; import "./AdvancedOrderEngineErrors.sol"; @@ -77,6 +77,23 @@ contract AdvancedOrderEngine is EIP712 { revert ZeroAddress(); } + // TBD: debatable, can take signing scheme in order schema or can verify like 1inch + if (order.isContract()) { + if ( + !(IERC1271(order.maker).isValidSignature( + orderMessageHash, + signature + ) == IERC1271.isValidSignature.selector) + ) { + revert BadSignature(); + } + } else { + address signer = ECDSA.recover(orderMessageHash, signature); + if (signer != order.maker) { + revert BadSignature(); + } + } + unchecked { ++i; } diff --git a/src/AdvancedOrderEngineErrors.sol b/src/AdvancedOrderEngineErrors.sol index c02a532..c15c174 100644 --- a/src/AdvancedOrderEngineErrors.sol +++ b/src/AdvancedOrderEngineErrors.sol @@ -7,3 +7,4 @@ error EmptyOrdersArray(); error OrderExpired(bytes32 orderHash); error ZeroTokenAmounts(); error ZeroAddress(); +error BadSignature(); diff --git a/src/libraries/OrderEngine.sol b/src/libraries/OrderEngine.sol index 19df372..e1fb5e5 100644 --- a/src/libraries/OrderEngine.sol +++ b/src/libraries/OrderEngine.sol @@ -67,4 +67,8 @@ library OrderEngine { ) ); } + + function isContract(Order calldata order) public view returns (bool) { + return order.maker.code.length > 0; + } } From e2bf7f61b620789e22c598e4da3bce1a04b9f23d Mon Sep 17 00:00:00 2001 From: Hammad Ghazi Date: Sun, 15 Oct 2023 18:31:07 +0500 Subject: [PATCH 08/47] fix: stack too deep by setting via_ir to true --- foundry.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/foundry.toml b/foundry.toml index ad7e0d0..30f53b6 100644 --- a/foundry.toml +++ b/foundry.toml @@ -6,6 +6,7 @@ libs = ['lib'] auto_detect_solc = false solc = '0.8.21' +via_ir = true optimizer = true optimizer_runs = 10_000 From de85df4afb710ab9a78115db3bee87368a4e66c5 Mon Sep 17 00:00:00 2001 From: Hammad Ghazi Date: Sun, 15 Oct 2023 21:52:13 +0500 Subject: [PATCH 09/47] feat: implement fn to decode calldata --- src/AdvancedOrderEngineErrors.sol | 1 + src/libraries/Decoder.sol | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+) create mode 100644 src/libraries/Decoder.sol diff --git a/src/AdvancedOrderEngineErrors.sol b/src/AdvancedOrderEngineErrors.sol index c15c174..54c94ab 100644 --- a/src/AdvancedOrderEngineErrors.sol +++ b/src/AdvancedOrderEngineErrors.sol @@ -8,3 +8,4 @@ error OrderExpired(bytes32 orderHash); error ZeroTokenAmounts(); error ZeroAddress(); error BadSignature(); +error IncorrectDataLength(); diff --git a/src/libraries/Decoder.sol b/src/libraries/Decoder.sol new file mode 100644 index 0000000..7feb06f --- /dev/null +++ b/src/libraries/Decoder.sol @@ -0,0 +1,19 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.21; + +import "../AdvancedOrderEngineErrors.sol"; + +library Decoder { + function decodeTargetAndCalldata( + bytes calldata data + ) internal pure returns (address target, bytes calldata args) { + if (data.length < 20) revert IncorrectDataLength(); + // no memory ops inside so this insertion is automatically memory safe + assembly { + // solhint-disable-line no-inline-assembly + target := shr(96, calldataload(data.offset)) + args.offset := add(data.offset, 20) + args.length := sub(data.length, 20) + } + } +} From 8009baa15b21b1b819b5afdca3eff28fd889b03f Mon Sep 17 00:00:00 2001 From: Hammad Ghazi Date: Sun, 15 Oct 2023 22:03:41 +0500 Subject: [PATCH 10/47] define pre-interaction hook interface --- .../IPreInteractionNotificationReceiver.sol | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 src/interfaces/IPreInteractionNotificationReceiver.sol diff --git a/src/interfaces/IPreInteractionNotificationReceiver.sol b/src/interfaces/IPreInteractionNotificationReceiver.sol new file mode 100644 index 0000000..41f3b61 --- /dev/null +++ b/src/interfaces/IPreInteractionNotificationReceiver.sol @@ -0,0 +1,22 @@ +// SPDX-License-Identifier: MIT + +pragma solidity 0.8.21; + +/** + * @notice Interface for maker pre-interaction hook, it is invoked before funds are transferred from the 'maker' to the 'vault'. + */ +interface IPreInteractionNotificationReceiver { + /** + * @notice Callback method that gets called before any funds transfers + * @param orderHash Hash of the order being processed + * @param maker Maker address + * @param clearingPrice Actual amount maker will receive + * @param interactionData Interaction calldata + */ + function fillOrderPreInteraction( + bytes32 orderHash, + address maker, + uint256 clearingPrice, + bytes memory interactionData + ) external; +} From a396f4cca7ae9bc81901bf29bf4e35bb81a9d59c Mon Sep 17 00:00:00 2001 From: Hammad Ghazi Date: Sun, 15 Oct 2023 22:05:17 +0500 Subject: [PATCH 11/47] feat: import and attach decoder lib for hooks --- src/AdvancedOrderEngine.sol | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/AdvancedOrderEngine.sol b/src/AdvancedOrderEngine.sol index 0b55d69..29d079a 100644 --- a/src/AdvancedOrderEngine.sol +++ b/src/AdvancedOrderEngine.sol @@ -4,10 +4,14 @@ pragma solidity 0.8.21; import {EIP712, ECDSA} from "openzeppelin/utils/cryptography/EIP712.sol"; import {IERC1271} from "openzeppelin/interfaces/IERC1271.sol"; import {OrderEngine} from "./libraries/OrderEngine.sol"; +import {IPreInteractionNotificationReceiver} from "./interfaces/IPreInteractionNotificationReceiver.sol"; +import {Decoder} from "./libraries/Decoder.sol"; + import "./AdvancedOrderEngineErrors.sol"; contract AdvancedOrderEngine is EIP712 { using OrderEngine for OrderEngine.Order; + using Decoder for bytes; constructor( string memory name, From 9aaac8a4505b21889f1a137ae1264ab3b2207fbd Mon Sep 17 00:00:00 2001 From: Hammad Ghazi Date: Sun, 15 Oct 2023 22:06:03 +0500 Subject: [PATCH 12/47] feat: implement core logic of pre-interaction hook --- src/AdvancedOrderEngine.sol | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/AdvancedOrderEngine.sol b/src/AdvancedOrderEngine.sol index 29d079a..df8222b 100644 --- a/src/AdvancedOrderEngine.sol +++ b/src/AdvancedOrderEngine.sol @@ -98,6 +98,21 @@ contract AdvancedOrderEngine is EIP712 { } } + if (order.preInteraction.length >= 20) { + // proceed only if interaction length is enough to store address + ( + address interactionTarget, + bytes calldata interactionData + ) = order.preInteraction.decodeTargetAndCalldata(); + IPreInteractionNotificationReceiver(interactionTarget) + .fillOrderPreInteraction( + orderHash, + order.maker, + clearingPrices[i], + interactionData + ); + } + unchecked { ++i; } From f7620690568274fb393626b288d0568a701d93c7 Mon Sep 17 00:00:00 2001 From: Hammad Ghazi Date: Sun, 15 Oct 2023 22:21:20 +0500 Subject: [PATCH 13/47] fix: modify stubs as comments --- src/AdvancedOrderEngine.sol | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/src/AdvancedOrderEngine.sol b/src/AdvancedOrderEngine.sol index df8222b..d718306 100644 --- a/src/AdvancedOrderEngine.sol +++ b/src/AdvancedOrderEngine.sol @@ -33,6 +33,8 @@ contract AdvancedOrderEngine is EIP712 { bytes calldata facilitatorInteractionCalldata, address facilitatorInteractionTargetContract ) external { + // STUB: ONLY OPERATOR // + // TBD: max array length check needed? Considering fn will be restricted to operators only /** @@ -98,6 +100,8 @@ contract AdvancedOrderEngine is EIP712 { } } + // STUB: VERIFY PREDICATES // + if (order.preInteraction.length >= 20) { // proceed only if interaction length is enough to store address ( @@ -113,23 +117,18 @@ contract AdvancedOrderEngine is EIP712 { ); } + // STUB: TRANSER FUNDS FROM MAKER TO VAULT // + unchecked { ++i; } } - // Loop start - // Perform order specific sanity checks - // Verify signatjures - // Verify predicates - // Call pre-interaction hook - // Transfer funds from maker to vault - // Loop end - // Call facilitator interaction - // Loop start - // Ensure facilitator is respecting maker price - // Transfer funds from vault to maker - // Call post-interaction hook - // Emit event (decide where to emit event, as its considered as an effect so maybe do it somewhere in the start) + // STUB: CALL FACILITATOR INTERACTION // + // STUB: START ANOTHER LOOP // + // STUB: ENSURE FACILITATOR IS RESPECTING MAKER PRICE // + // STUB: TRANSFER FUNDS FROM VAULT TO MAKER // + // STUB: CALL POST-INTERACTION HOOK // + // STUB: EMIT EVENT (decide where to emit event, as its considered as an effect so maybe do it somewhere in the start) // } } From 410a1205c0eb98d46ad5bf24cf10782bf47fab7d Mon Sep 17 00:00:00 2001 From: Hammad Ghazi Date: Sun, 15 Oct 2023 22:24:53 +0500 Subject: [PATCH 14/47] fix: add another stub as a comment --- src/AdvancedOrderEngine.sol | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/AdvancedOrderEngine.sol b/src/AdvancedOrderEngine.sol index d718306..87fc4ab 100644 --- a/src/AdvancedOrderEngine.sol +++ b/src/AdvancedOrderEngine.sol @@ -83,6 +83,8 @@ contract AdvancedOrderEngine is EIP712 { revert ZeroAddress(); } + // STUB: PARTIAL FILL FEATURE // + // TBD: debatable, can take signing scheme in order schema or can verify like 1inch if (order.isContract()) { if ( From 2f7baa3e8028cfe556586f5c35d44d1bc7966593 Mon Sep 17 00:00:00 2001 From: Hammad Ghazi Date: Mon, 16 Oct 2023 16:44:02 +0500 Subject: [PATCH 15/47] feat: implement vault contract implemented vault contract to facilitate asset transfers between the maker and the vault. --- src/AdvancedOrderEngine.sol | 1 - src/Vault.sol | 28 ++++++++++++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) create mode 100644 src/Vault.sol diff --git a/src/AdvancedOrderEngine.sol b/src/AdvancedOrderEngine.sol index 87fc4ab..ced10a5 100644 --- a/src/AdvancedOrderEngine.sol +++ b/src/AdvancedOrderEngine.sol @@ -6,7 +6,6 @@ import {IERC1271} from "openzeppelin/interfaces/IERC1271.sol"; import {OrderEngine} from "./libraries/OrderEngine.sol"; import {IPreInteractionNotificationReceiver} from "./interfaces/IPreInteractionNotificationReceiver.sol"; import {Decoder} from "./libraries/Decoder.sol"; - import "./AdvancedOrderEngineErrors.sol"; contract AdvancedOrderEngine is EIP712 { diff --git a/src/Vault.sol b/src/Vault.sol new file mode 100644 index 0000000..a0cc65e --- /dev/null +++ b/src/Vault.sol @@ -0,0 +1,28 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.21; + +import {SafeERC20} from "openzeppelin/token/ERC20/utils/SafeERC20.sol"; +import {IERC20} from "openzeppelin/token/ERC20/IERC20.sol"; + +abstract contract Vault { + using SafeERC20 for IERC20; + + function _receiveAsset( + address asset, + uint256 amount, + address maker + ) internal { + IERC20 token = _asIERC20(asset); + + token.safeTransferFrom(maker, address(this), amount); + } + + function _sendAsset(address asset, uint256 amount, address maker) internal { + IERC20 token = _asIERC20(asset); + token.safeTransfer(maker, amount); + } + + function _asIERC20(address asset) internal pure returns (IERC20) { + return IERC20(asset); + } +} From a4193576df3be0b85078cc9ad2616228965d851a Mon Sep 17 00:00:00 2001 From: Hammad Ghazi Date: Mon, 16 Oct 2023 16:52:21 +0500 Subject: [PATCH 16/47] feat: integrate vault contract for maker-to-vault asset transfer integrated vault contract for asset transfers from the maker's address to the vault contract in AdvancedOrderEngine contract. Imported vault contract and invoked its _receiveAsset function for funds transfer --- src/AdvancedOrderEngine.sol | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/AdvancedOrderEngine.sol b/src/AdvancedOrderEngine.sol index ced10a5..1806731 100644 --- a/src/AdvancedOrderEngine.sol +++ b/src/AdvancedOrderEngine.sol @@ -7,8 +7,9 @@ import {OrderEngine} from "./libraries/OrderEngine.sol"; import {IPreInteractionNotificationReceiver} from "./interfaces/IPreInteractionNotificationReceiver.sol"; import {Decoder} from "./libraries/Decoder.sol"; import "./AdvancedOrderEngineErrors.sol"; +import {Vault} from "./Vault.sol"; -contract AdvancedOrderEngine is EIP712 { +contract AdvancedOrderEngine is Vault, EIP712 { using OrderEngine for OrderEngine.Order; using Decoder for bytes; @@ -118,7 +119,8 @@ contract AdvancedOrderEngine is EIP712 { ); } - // STUB: TRANSER FUNDS FROM MAKER TO VAULT // + // TODO: reorder params type + _receiveAsset(order.sellToken, order.sellTokenAmount, order.maker); unchecked { ++i; From 99211c4104357039c13e5110ed8a8d3ab7a16ce9 Mon Sep 17 00:00:00 2001 From: Hammad Ghazi Date: Mon, 16 Oct 2023 18:08:11 +0500 Subject: [PATCH 17/47] feat: implement logic for vault-to-maker asset transfer --- src/AdvancedOrderEngine.sol | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/src/AdvancedOrderEngine.sol b/src/AdvancedOrderEngine.sol index 1806731..1778caf 100644 --- a/src/AdvancedOrderEngine.sol +++ b/src/AdvancedOrderEngine.sol @@ -128,10 +128,22 @@ contract AdvancedOrderEngine is Vault, EIP712 { } // STUB: CALL FACILITATOR INTERACTION // - // STUB: START ANOTHER LOOP // - // STUB: ENSURE FACILITATOR IS RESPECTING MAKER PRICE // - // STUB: TRANSFER FUNDS FROM VAULT TO MAKER // - // STUB: CALL POST-INTERACTION HOOK // + + // TODO: Need optimization + for (uint256 i; i < orders.length; ) { + // STUB: ENSURE FACILITATOR IS RESPECTING MAKER PRICE // + + OrderEngine.Order calldata order = orders[i]; + + // TODO: reorder params type + _sendAsset(order.buyToken, order.buyTokenAmount, order.maker); + + // STUB: CALL POST-INTERACTION HOOK // + + unchecked { + ++i; + } + } // STUB: EMIT EVENT (decide where to emit event, as its considered as an effect so maybe do it somewhere in the start) // } } From aa21e29737c26810f167384fc9e3ba32af468e57 Mon Sep 17 00:00:00 2001 From: Hammad Ghazi Date: Mon, 16 Oct 2023 18:25:06 +0500 Subject: [PATCH 18/47] feat: define post-interaction hook interface --- src/AdvancedOrderEngine.sol | 2 -- .../IPostInteractionNotificationReceiver.sol | 22 +++++++++++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) create mode 100644 src/interfaces/IPostInteractionNotificationReceiver.sol diff --git a/src/AdvancedOrderEngine.sol b/src/AdvancedOrderEngine.sol index 1778caf..d4fcb64 100644 --- a/src/AdvancedOrderEngine.sol +++ b/src/AdvancedOrderEngine.sol @@ -138,8 +138,6 @@ contract AdvancedOrderEngine is Vault, EIP712 { // TODO: reorder params type _sendAsset(order.buyToken, order.buyTokenAmount, order.maker); - // STUB: CALL POST-INTERACTION HOOK // - unchecked { ++i; } diff --git a/src/interfaces/IPostInteractionNotificationReceiver.sol b/src/interfaces/IPostInteractionNotificationReceiver.sol new file mode 100644 index 0000000..137e159 --- /dev/null +++ b/src/interfaces/IPostInteractionNotificationReceiver.sol @@ -0,0 +1,22 @@ +// SPDX-License-Identifier: MIT + +pragma solidity 0.8.21; + +/** + * @notice Interface for maker post-interaction hook, it is invoked after funds are transferred from the 'vault' to the 'maker'. + */ +interface IPostInteractionNotificationReceiver { + /** + * @notice Callback method that gets called after all funds transfers + * @param orderHash Hash of the order being processed + * @param maker Maker address + * @param clearingPrice Actual amount maker will receive + * @param interactionData Interaction calldata + */ + function fillOrderPostInteraction( + bytes32 orderHash, + address maker, + uint256 clearingPrice, + bytes memory interactionData + ) external; +} From 1646350c8243ac66eeedbd5f6f375c592eaf9d56 Mon Sep 17 00:00:00 2001 From: Hammad Ghazi Date: Mon, 16 Oct 2023 18:29:36 +0500 Subject: [PATCH 19/47] feat: implement core logic of post-interaction hook --- src/AdvancedOrderEngine.sol | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/AdvancedOrderEngine.sol b/src/AdvancedOrderEngine.sol index d4fcb64..d06e841 100644 --- a/src/AdvancedOrderEngine.sol +++ b/src/AdvancedOrderEngine.sol @@ -5,6 +5,8 @@ import {EIP712, ECDSA} from "openzeppelin/utils/cryptography/EIP712.sol"; import {IERC1271} from "openzeppelin/interfaces/IERC1271.sol"; import {OrderEngine} from "./libraries/OrderEngine.sol"; import {IPreInteractionNotificationReceiver} from "./interfaces/IPreInteractionNotificationReceiver.sol"; +import {IPostInteractionNotificationReceiver} from "./interfaces/IPostInteractionNotificationReceiver.sol"; + import {Decoder} from "./libraries/Decoder.sol"; import "./AdvancedOrderEngineErrors.sol"; import {Vault} from "./Vault.sol"; @@ -135,9 +137,27 @@ contract AdvancedOrderEngine is Vault, EIP712 { OrderEngine.Order calldata order = orders[i]; + bytes32 orderHash = order.hash(); + bytes32 orderMessageHash = _hashTypedDataV4(orderHash); + // TODO: reorder params type _sendAsset(order.buyToken, order.buyTokenAmount, order.maker); + if (order.postInteraction.length >= 20) { + // proceed only if interaction length is enough to store address + ( + address interactionTarget, + bytes calldata interactionData + ) = order.postInteraction.decodeTargetAndCalldata(); + IPostInteractionNotificationReceiver(interactionTarget) + .fillOrderPostInteraction( + orderMessageHash, + order.maker, + clearingPrices[i], + interactionData + ); + } + unchecked { ++i; } From c8ede00189f9915cf4694fb6a8747b66788cf209 Mon Sep 17 00:00:00 2001 From: Hammad Ghazi Date: Mon, 16 Oct 2023 18:58:47 +0500 Subject: [PATCH 20/47] feat: add validation for maker requested amount Added a validation check to ensure that maker receives an amount equal to or more than requested --- src/AdvancedOrderEngine.sol | 10 ++++++++-- src/AdvancedOrderEngineErrors.sol | 1 + 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/AdvancedOrderEngine.sol b/src/AdvancedOrderEngine.sol index d06e841..35ecb0f 100644 --- a/src/AdvancedOrderEngine.sol +++ b/src/AdvancedOrderEngine.sol @@ -133,13 +133,19 @@ contract AdvancedOrderEngine is Vault, EIP712 { // TODO: Need optimization for (uint256 i; i < orders.length; ) { - // STUB: ENSURE FACILITATOR IS RESPECTING MAKER PRICE // - OrderEngine.Order calldata order = orders[i]; bytes32 orderHash = order.hash(); bytes32 orderMessageHash = _hashTypedDataV4(orderHash); + // STUB: ENSURE FACILITATOR IS RESPECTING MAKER PRICE // + if (order.buyTokenAmount > clearingPrices[i]) { + revert LimitPriceNotRespected( + order.buyTokenAmount, + clearingPrices[i] + ); + } + // TODO: reorder params type _sendAsset(order.buyToken, order.buyTokenAmount, order.maker); diff --git a/src/AdvancedOrderEngineErrors.sol b/src/AdvancedOrderEngineErrors.sol index 54c94ab..76f9429 100644 --- a/src/AdvancedOrderEngineErrors.sol +++ b/src/AdvancedOrderEngineErrors.sol @@ -9,3 +9,4 @@ error ZeroTokenAmounts(); error ZeroAddress(); error BadSignature(); error IncorrectDataLength(); +error LimitPriceNotRespected(uint256 desiredAmount, uint256 offeredAmount); From a9af474390f2c27affd951aaa7b75752578ecfb6 Mon Sep 17 00:00:00 2001 From: Hammad Ghazi Date: Mon, 16 Oct 2023 19:01:18 +0500 Subject: [PATCH 21/47] fix: corrected transfer from offered amount instead of requested amount in vault-to-maker asset transfer --- src/AdvancedOrderEngine.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/AdvancedOrderEngine.sol b/src/AdvancedOrderEngine.sol index 35ecb0f..db82646 100644 --- a/src/AdvancedOrderEngine.sol +++ b/src/AdvancedOrderEngine.sol @@ -147,7 +147,7 @@ contract AdvancedOrderEngine is Vault, EIP712 { } // TODO: reorder params type - _sendAsset(order.buyToken, order.buyTokenAmount, order.maker); + _sendAsset(order.buyToken, clearingPrices[i], order.maker); if (order.postInteraction.length >= 20) { // proceed only if interaction length is enough to store address From 8821760c0d0e078d36f43b5c93189a689c66d0eb Mon Sep 17 00:00:00 2001 From: Hammad Ghazi Date: Mon, 16 Oct 2023 19:02:46 +0500 Subject: [PATCH 22/47] refacator: renamed 'clearingPrices' fillOrder fn param to 'offeredAmounts' --- src/AdvancedOrderEngine.sol | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/AdvancedOrderEngine.sol b/src/AdvancedOrderEngine.sol index db82646..9db3bf8 100644 --- a/src/AdvancedOrderEngine.sol +++ b/src/AdvancedOrderEngine.sol @@ -24,13 +24,13 @@ contract AdvancedOrderEngine is Vault, EIP712 { * @notice Fills multiple orders by processing the specified orders and clearing prices. * * @param orders An array of order structs representing the orders to be filled. - * @param clearingPrices An array of clearing prices that the facilitator is offering to the makers. + * @param offeredAmounts An array of clearing prices that the facilitator is offering to the makers. * @param facilitatorInteractionCalldata The calldata for the facilitator's interaction. * @param facilitatorInteractionTargetContract The address of the facilitator's target contract. */ function fillOrders( OrderEngine.Order[] calldata orders, - uint256[] calldata clearingPrices, + uint256[] calldata offeredAmounts, bytes[] calldata signatures, bytes calldata facilitatorInteractionCalldata, address facilitatorInteractionTargetContract @@ -40,8 +40,8 @@ contract AdvancedOrderEngine is Vault, EIP712 { // TBD: max array length check needed? Considering fn will be restricted to operators only /** - TBD: no need to check for clearingPrices length to be equal to 0 as if that's the case, txn will revert in subsequent check - but should we check for clearingPrices length to be equal to zero explicitly for better error reporting? + TBD: no need to check for offeredAmounts length to be equal to 0 as if that's the case, txn will revert in subsequent check + but should we check for offeredAmounts length to be equal to zero explicitly for better error reporting? Also consider generic error message */ // Revert if the orders array length is zero. @@ -50,8 +50,8 @@ contract AdvancedOrderEngine is Vault, EIP712 { } // Revert if the length of the orders array does not match the clearing prices array. - if (orders.length != clearingPrices.length) { - revert ArraysLengthMismatch(orders.length, clearingPrices.length); + if (orders.length != offeredAmounts.length) { + revert ArraysLengthMismatch(orders.length, offeredAmounts.length); } // Revert if the facilitator has provided calldata for its interaction but has provided null target contract address. @@ -116,7 +116,7 @@ contract AdvancedOrderEngine is Vault, EIP712 { .fillOrderPreInteraction( orderHash, order.maker, - clearingPrices[i], + offeredAmounts[i], interactionData ); } @@ -139,15 +139,15 @@ contract AdvancedOrderEngine is Vault, EIP712 { bytes32 orderMessageHash = _hashTypedDataV4(orderHash); // STUB: ENSURE FACILITATOR IS RESPECTING MAKER PRICE // - if (order.buyTokenAmount > clearingPrices[i]) { + if (order.buyTokenAmount > offeredAmounts[i]) { revert LimitPriceNotRespected( order.buyTokenAmount, - clearingPrices[i] + offeredAmounts[i] ); } // TODO: reorder params type - _sendAsset(order.buyToken, clearingPrices[i], order.maker); + _sendAsset(order.buyToken, offeredAmounts[i], order.maker); if (order.postInteraction.length >= 20) { // proceed only if interaction length is enough to store address @@ -159,7 +159,7 @@ contract AdvancedOrderEngine is Vault, EIP712 { .fillOrderPostInteraction( orderMessageHash, order.maker, - clearingPrices[i], + offeredAmounts[i], interactionData ); } From 44d6b9f392e77faa532158e1c3f46c4fa18f47c3 Mon Sep 17 00:00:00 2001 From: Hammad Ghazi Date: Mon, 16 Oct 2023 19:05:56 +0500 Subject: [PATCH 23/47] fix: passing 'orderMessageHash' instead of 'orderHash' in pre-interaction hook --- src/AdvancedOrderEngine.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/AdvancedOrderEngine.sol b/src/AdvancedOrderEngine.sol index 9db3bf8..4ce750a 100644 --- a/src/AdvancedOrderEngine.sol +++ b/src/AdvancedOrderEngine.sol @@ -114,7 +114,7 @@ contract AdvancedOrderEngine is Vault, EIP712 { ) = order.preInteraction.decodeTargetAndCalldata(); IPreInteractionNotificationReceiver(interactionTarget) .fillOrderPreInteraction( - orderHash, + orderMessageHash, order.maker, offeredAmounts[i], interactionData From edee642f3c3f44dca4a4b9be414d2d8368cac34f Mon Sep 17 00:00:00 2001 From: Hammad Ghazi Date: Mon, 16 Oct 2023 19:16:49 +0500 Subject: [PATCH 24/47] feat: log OrderFill event --- src/AdvancedOrderEngine.sol | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/src/AdvancedOrderEngine.sol b/src/AdvancedOrderEngine.sol index 4ce750a..44c9e52 100644 --- a/src/AdvancedOrderEngine.sol +++ b/src/AdvancedOrderEngine.sol @@ -15,6 +15,13 @@ contract AdvancedOrderEngine is Vault, EIP712 { using OrderEngine for OrderEngine.Order; using Decoder for bytes; + event OrderFill( + address operator, + address maker, + bytes32 orderHash, + uint256 offeredAmount + ); + constructor( string memory name, string memory version @@ -164,10 +171,18 @@ contract AdvancedOrderEngine is Vault, EIP712 { ); } + // TODO: decide where to emit event, as its considered as an effect so maybe do it somewhere in the start; what params to log; + // events spam? ; consider emitting just one event + emit OrderFill( + msg.sender, + order.maker, + orderMessageHash, + offeredAmounts[i] + ); + unchecked { ++i; } } - // STUB: EMIT EVENT (decide where to emit event, as its considered as an effect so maybe do it somewhere in the start) // } } From 5479a535214a8f4b6228d00ab6f81989aead4c87 Mon Sep 17 00:00:00 2001 From: Hammad Ghazi Date: Mon, 16 Oct 2023 20:16:47 +0500 Subject: [PATCH 25/47] define facilitator interaction hook interface --- .../IInteractionNotificationReceiver.sol | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) create mode 100644 src/interfaces/IInteractionNotificationReceiver.sol diff --git a/src/interfaces/IInteractionNotificationReceiver.sol b/src/interfaces/IInteractionNotificationReceiver.sol new file mode 100644 index 0000000..f1c4695 --- /dev/null +++ b/src/interfaces/IInteractionNotificationReceiver.sol @@ -0,0 +1,24 @@ +// SPDX-License-Identifier: MIT + +pragma solidity 0.8.21; + +/** + * @notice Interface for facilitator interaction hook, it is invoked after funds are transferred from the 'maker' to the 'vault'. + */ +interface IInteractionNotificationReceiver { + /** + * @notice Callback method that gets called after funds transfer from the 'maker' to the 'vault + * @param operator Address of the caller who executed orders on behalf of the facilitator. + * @param maker Address of the oorder maker + * @param orderSellAmount Amount of the asset the maker is willing to sell. + * @param amountOffered Amount of the asset the facilitator is willing to pay in exchange for the sell amount. + * @param interactionData Interaction calldata + */ + function fillOrderInteraction( + address operator, + address maker, + uint256 orderSellAmount, + uint256 amountOffered, + bytes memory interactionData + ) external; +} From 7ea9136295e751e6984e2a1b6fa7141af3f1c0b9 Mon Sep 17 00:00:00 2001 From: Hammad Ghazi Date: Mon, 16 Oct 2023 20:35:19 +0500 Subject: [PATCH 26/47] refactor: modify 'fillOrders' fn param removed the 'facilitatorInteractionTargetContract' var from the 'fillOrders' fn param. Facilitator is now expected to provide the target contract address within the 'facilitatorInteractionCallData' param. Also removed the facilitator target contract address check --- src/AdvancedOrderEngine.sol | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/src/AdvancedOrderEngine.sol b/src/AdvancedOrderEngine.sol index 44c9e52..7b8ae74 100644 --- a/src/AdvancedOrderEngine.sol +++ b/src/AdvancedOrderEngine.sol @@ -33,14 +33,12 @@ contract AdvancedOrderEngine is Vault, EIP712 { * @param orders An array of order structs representing the orders to be filled. * @param offeredAmounts An array of clearing prices that the facilitator is offering to the makers. * @param facilitatorInteractionCalldata The calldata for the facilitator's interaction. - * @param facilitatorInteractionTargetContract The address of the facilitator's target contract. */ function fillOrders( OrderEngine.Order[] calldata orders, uint256[] calldata offeredAmounts, bytes[] calldata signatures, - bytes calldata facilitatorInteractionCalldata, - address facilitatorInteractionTargetContract + bytes calldata facilitatorInteractionCalldata ) external { // STUB: ONLY OPERATOR // @@ -61,14 +59,6 @@ contract AdvancedOrderEngine is Vault, EIP712 { revert ArraysLengthMismatch(orders.length, offeredAmounts.length); } - // Revert if the facilitator has provided calldata for its interaction but has provided null target contract address. - if ( - facilitatorInteractionCalldata.length != 0 && - facilitatorInteractionTargetContract == address(0) - ) { - revert ZeroFacilitatorTargetAddress(); // TBD: use generic error message i.e. ZeroAddress() - } - for (uint256 i; i < orders.length; ) { OrderEngine.Order calldata order = orders[i]; bytes calldata signature = signatures[i]; From 583ae7951bda35f972634a860ec7e3353ecfad05 Mon Sep 17 00:00:00 2001 From: Hammad Ghazi Date: Mon, 16 Oct 2023 20:46:19 +0500 Subject: [PATCH 27/47] refactor: renamed 'facilitatorInteractionCalldata' var of fillOrders fn param to 'facilitatorInteraction' --- src/AdvancedOrderEngine.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/AdvancedOrderEngine.sol b/src/AdvancedOrderEngine.sol index 7b8ae74..8c5282c 100644 --- a/src/AdvancedOrderEngine.sol +++ b/src/AdvancedOrderEngine.sol @@ -32,13 +32,13 @@ contract AdvancedOrderEngine is Vault, EIP712 { * * @param orders An array of order structs representing the orders to be filled. * @param offeredAmounts An array of clearing prices that the facilitator is offering to the makers. - * @param facilitatorInteractionCalldata The calldata for the facilitator's interaction. + * @param facilitatorInteraction The calldata for the facilitator's interaction. */ function fillOrders( OrderEngine.Order[] calldata orders, uint256[] calldata offeredAmounts, bytes[] calldata signatures, - bytes calldata facilitatorInteractionCalldata + bytes calldata facilitatorInteraction ) external { // STUB: ONLY OPERATOR // From 301081699323dec10996c57af5b38994e0a51108 Mon Sep 17 00:00:00 2001 From: Hammad Ghazi Date: Mon, 16 Oct 2023 20:53:48 +0500 Subject: [PATCH 28/47] refactor: modified facilitator interaction hook interface --- src/interfaces/IInteractionNotificationReceiver.sol | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/interfaces/IInteractionNotificationReceiver.sol b/src/interfaces/IInteractionNotificationReceiver.sol index f1c4695..83471c3 100644 --- a/src/interfaces/IInteractionNotificationReceiver.sol +++ b/src/interfaces/IInteractionNotificationReceiver.sol @@ -2,6 +2,8 @@ pragma solidity 0.8.21; +import {OrderEngine} from "../libraries/OrderEngine.sol"; + /** * @notice Interface for facilitator interaction hook, it is invoked after funds are transferred from the 'maker' to the 'vault'. */ @@ -9,16 +11,14 @@ interface IInteractionNotificationReceiver { /** * @notice Callback method that gets called after funds transfer from the 'maker' to the 'vault * @param operator Address of the caller who executed orders on behalf of the facilitator. - * @param maker Address of the oorder maker - * @param orderSellAmount Amount of the asset the maker is willing to sell. - * @param amountOffered Amount of the asset the facilitator is willing to pay in exchange for the sell amount. + * @param orders Address of the oorder maker + * @param offeredAmounts Amounts of the asset the facilitator is offering to the makers. * @param interactionData Interaction calldata */ function fillOrderInteraction( address operator, - address maker, - uint256 orderSellAmount, - uint256 amountOffered, + OrderEngine.Order[] calldata orders, + uint256[] calldata offeredAmounts, bytes memory interactionData ) external; } From 9ed479a84d10dad3bdd3a13e139b0cf02b4eca6f Mon Sep 17 00:00:00 2001 From: Hammad Ghazi Date: Mon, 16 Oct 2023 20:56:49 +0500 Subject: [PATCH 29/47] feat: implement core logic of facilitator interaction hook --- src/AdvancedOrderEngine.sol | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/src/AdvancedOrderEngine.sol b/src/AdvancedOrderEngine.sol index 8c5282c..fe9c344 100644 --- a/src/AdvancedOrderEngine.sol +++ b/src/AdvancedOrderEngine.sol @@ -7,6 +7,8 @@ import {OrderEngine} from "./libraries/OrderEngine.sol"; import {IPreInteractionNotificationReceiver} from "./interfaces/IPreInteractionNotificationReceiver.sol"; import {IPostInteractionNotificationReceiver} from "./interfaces/IPostInteractionNotificationReceiver.sol"; +import {IInteractionNotificationReceiver} from "./interfaces/IInteractionNotificationReceiver.sol"; + import {Decoder} from "./libraries/Decoder.sol"; import "./AdvancedOrderEngineErrors.sol"; import {Vault} from "./Vault.sol"; @@ -126,7 +128,20 @@ contract AdvancedOrderEngine is Vault, EIP712 { } } - // STUB: CALL FACILITATOR INTERACTION // + if (facilitatorInteraction.length >= 20) { + // proceed only if interaction length is enough to store address + ( + address interactionTarget, + bytes calldata interactionData + ) = facilitatorInteraction.decodeTargetAndCalldata(); + IInteractionNotificationReceiver(interactionTarget) + .fillOrderInteraction( + msg.sender, + orders, + offeredAmounts, + interactionData + ); + } // TODO: Need optimization for (uint256 i; i < orders.length; ) { From 52a9beabe945a4f855fa9e9b0199dea92d906204 Mon Sep 17 00:00:00 2001 From: Hammad Ghazi Date: Mon, 16 Oct 2023 21:01:22 +0500 Subject: [PATCH 30/47] docs: modified natspec comments of facilitator interaction hook interface --- src/interfaces/IInteractionNotificationReceiver.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/interfaces/IInteractionNotificationReceiver.sol b/src/interfaces/IInteractionNotificationReceiver.sol index 83471c3..b9b3ce0 100644 --- a/src/interfaces/IInteractionNotificationReceiver.sol +++ b/src/interfaces/IInteractionNotificationReceiver.sol @@ -9,9 +9,9 @@ import {OrderEngine} from "../libraries/OrderEngine.sol"; */ interface IInteractionNotificationReceiver { /** - * @notice Callback method that gets called after funds transfer from the 'maker' to the 'vault + * @notice Callback method that gets called after funds transfer from the 'maker' to the 'vault'. * @param operator Address of the caller who executed orders on behalf of the facilitator. - * @param orders Address of the oorder maker + * @param orders Orders the facilitator is willing to fill. * @param offeredAmounts Amounts of the asset the facilitator is offering to the makers. * @param interactionData Interaction calldata */ From 5d1d7e326e59f3715df43cb26c10d5c67f5ffb0a Mon Sep 17 00:00:00 2001 From: Hammad Ghazi Date: Tue, 17 Oct 2023 00:16:15 +0500 Subject: [PATCH 31/47] refactor: removed unnecessary comment --- src/AdvancedOrderEngine.sol | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/AdvancedOrderEngine.sol b/src/AdvancedOrderEngine.sol index fe9c344..d455676 100644 --- a/src/AdvancedOrderEngine.sol +++ b/src/AdvancedOrderEngine.sol @@ -17,6 +17,10 @@ contract AdvancedOrderEngine is Vault, EIP712 { using OrderEngine for OrderEngine.Order; using Decoder for bytes; + // @notice Stores unfilled amounts for each order. + // TBD: public or private? + mapping(bytes32 => uint256) public remainingFillableAmount; + event OrderFill( address operator, address maker, @@ -150,7 +154,6 @@ contract AdvancedOrderEngine is Vault, EIP712 { bytes32 orderHash = order.hash(); bytes32 orderMessageHash = _hashTypedDataV4(orderHash); - // STUB: ENSURE FACILITATOR IS RESPECTING MAKER PRICE // if (order.buyTokenAmount > offeredAmounts[i]) { revert LimitPriceNotRespected( order.buyTokenAmount, From a09b9b18b9863c2c033c0504e8d31d93793f813d Mon Sep 17 00:00:00 2001 From: Hammad Ghazi Date: Tue, 17 Oct 2023 01:06:07 +0500 Subject: [PATCH 32/47] feat: implement only operator modifier --- src/AdvancedOrderEngine.sol | 11 ++++++++--- src/AdvancedOrderEngineErrors.sol | 1 + 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/AdvancedOrderEngine.sol b/src/AdvancedOrderEngine.sol index d455676..a76f3f9 100644 --- a/src/AdvancedOrderEngine.sol +++ b/src/AdvancedOrderEngine.sol @@ -17,9 +17,7 @@ contract AdvancedOrderEngine is Vault, EIP712 { using OrderEngine for OrderEngine.Order; using Decoder for bytes; - // @notice Stores unfilled amounts for each order. - // TBD: public or private? - mapping(bytes32 => uint256) public remainingFillableAmount; + mapping(address => bool) public isOperator; event OrderFill( address operator, @@ -33,6 +31,13 @@ contract AdvancedOrderEngine is Vault, EIP712 { string memory version ) EIP712(name, version) {} + modifier onlyOperator() { + if (!isOperator[msg.sender]) { + revert NotAnOperator(msg.sender); + } + _; + } + /** * @notice Fills multiple orders by processing the specified orders and clearing prices. * diff --git a/src/AdvancedOrderEngineErrors.sol b/src/AdvancedOrderEngineErrors.sol index 76f9429..50f1a02 100644 --- a/src/AdvancedOrderEngineErrors.sol +++ b/src/AdvancedOrderEngineErrors.sol @@ -10,3 +10,4 @@ error ZeroAddress(); error BadSignature(); error IncorrectDataLength(); error LimitPriceNotRespected(uint256 desiredAmount, uint256 offeredAmount); +error NotAnOperator(address caller); From dd87423fbbbff573042a7fa5938968ae46d9ea78 Mon Sep 17 00:00:00 2001 From: Hammad Ghazi Date: Tue, 17 Oct 2023 01:20:23 +0500 Subject: [PATCH 33/47] feat: implement manage operator role fn --- src/AdvancedOrderEngine.sol | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/src/AdvancedOrderEngine.sol b/src/AdvancedOrderEngine.sol index a76f3f9..b66cc06 100644 --- a/src/AdvancedOrderEngine.sol +++ b/src/AdvancedOrderEngine.sol @@ -12,8 +12,9 @@ import {IInteractionNotificationReceiver} from "./interfaces/IInteractionNotific import {Decoder} from "./libraries/Decoder.sol"; import "./AdvancedOrderEngineErrors.sol"; import {Vault} from "./Vault.sol"; +import {Ownable2Step} from "openzeppelin/access/Ownable2Step.sol"; -contract AdvancedOrderEngine is Vault, EIP712 { +contract AdvancedOrderEngine is Vault, Ownable2Step, EIP712 { using OrderEngine for OrderEngine.Order; using Decoder for bytes; @@ -25,6 +26,7 @@ contract AdvancedOrderEngine is Vault, EIP712 { bytes32 orderHash, uint256 offeredAmount ); + event OperatorAccessModified(address indexed authorized, bool access); constructor( string memory name, @@ -38,6 +40,21 @@ contract AdvancedOrderEngine is Vault, EIP712 { _; } + function manageOperatorPrivilege( + address _address, + bool _access + ) external onlyOwner { + if (_address == address(0)) { + revert ZeroAddress(); + } + + // TBD: should we not allow if owner is trying to set same access? (con: additional gas) + // Overwrites the access previously granted. + isOperator[_address] = _access; + + emit OperatorAccessModified(_address, _access); + } + /** * @notice Fills multiple orders by processing the specified orders and clearing prices. * @@ -50,7 +67,7 @@ contract AdvancedOrderEngine is Vault, EIP712 { uint256[] calldata offeredAmounts, bytes[] calldata signatures, bytes calldata facilitatorInteraction - ) external { + ) external onlyOperator { // STUB: ONLY OPERATOR // // TBD: max array length check needed? Considering fn will be restricted to operators only From 5c9f6b547f40f5e830a4b9c51d3483318c73fc19 Mon Sep 17 00:00:00 2001 From: Hammad Ghazi Date: Tue, 17 Oct 2023 01:39:06 +0500 Subject: [PATCH 34/47] fix: transfer funds to recipient from vault instead of maker --- src/AdvancedOrderEngine.sol | 7 ++++--- src/libraries/OrderEngine.sol | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/AdvancedOrderEngine.sol b/src/AdvancedOrderEngine.sol index b66cc06..d00a590 100644 --- a/src/AdvancedOrderEngine.sol +++ b/src/AdvancedOrderEngine.sol @@ -68,7 +68,7 @@ contract AdvancedOrderEngine is Vault, Ownable2Step, EIP712 { bytes[] calldata signatures, bytes calldata facilitatorInteraction ) external onlyOperator { - // STUB: ONLY OPERATOR // + // TBD: Private orders? // TBD: max array length check needed? Considering fn will be restricted to operators only @@ -105,7 +105,8 @@ contract AdvancedOrderEngine is Vault, Ownable2Step, EIP712 { if ( order.maker == address(0) || order.buyToken == address(0) || - order.sellToken == address(0) + order.sellToken == address(0) || + order.recipient == address(0) ) { revert ZeroAddress(); } @@ -184,7 +185,7 @@ contract AdvancedOrderEngine is Vault, Ownable2Step, EIP712 { } // TODO: reorder params type - _sendAsset(order.buyToken, offeredAmounts[i], order.maker); + _sendAsset(order.buyToken, offeredAmounts[i], order.recipient); if (order.postInteraction.length >= 20) { // proceed only if interaction length is enough to store address diff --git a/src/libraries/OrderEngine.sol b/src/libraries/OrderEngine.sol index e1fb5e5..a5b9beb 100644 --- a/src/libraries/OrderEngine.sol +++ b/src/libraries/OrderEngine.sol @@ -12,7 +12,7 @@ library OrderEngine { uint256 feeAmounts; address maker; address taker; // null on public orders - address recipient; // TODO: maybe use null to represent msg.sender? + address recipient; // TBD: use null to represent maker? Right now expecting explicit address set address sellToken; address buyToken; bool isPartiallyFillable; From d2a300a4622be87f38a7945e56430ed9292a2ff0 Mon Sep 17 00:00:00 2001 From: Hammad Ghazi Date: Tue, 17 Oct 2023 01:42:15 +0500 Subject: [PATCH 35/47] docs: added some TBD comments --- src/AdvancedOrderEngine.sol | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/AdvancedOrderEngine.sol b/src/AdvancedOrderEngine.sol index d00a590..91e121e 100644 --- a/src/AdvancedOrderEngine.sol +++ b/src/AdvancedOrderEngine.sol @@ -15,6 +15,11 @@ import {Vault} from "./Vault.sol"; import {Ownable2Step} from "openzeppelin/access/Ownable2Step.sol"; contract AdvancedOrderEngine is Vault, Ownable2Step, EIP712 { + // TBD: consider making extraData a separate param + // TBD: consider making interfaces generic + // TBD: consider changing data type to IERC20 of buy and sell token + // TBD: consider allowing facilitator to tell offeredAmounts in its interaction + using OrderEngine for OrderEngine.Order; using Decoder for bytes; From efe5a3b61a0fd25dfff135e957a3ca21f1b308da Mon Sep 17 00:00:00 2001 From: Gul Hameed <90828739+gul-hameed@users.noreply.github.com> Date: Thu, 19 Oct 2023 16:00:50 +0500 Subject: [PATCH 36/47] Create slither.yml --- .github/workflows/slither.yml | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 .github/workflows/slither.yml diff --git a/.github/workflows/slither.yml b/.github/workflows/slither.yml new file mode 100644 index 0000000..59dded9 --- /dev/null +++ b/.github/workflows/slither.yml @@ -0,0 +1,12 @@ +name: Slither Analysiss +on: + push: + branches: + - 'feat/operator-restrictions' +jobs: + analyze: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + - uses: crytic/slither-action@v0.3.0 + From 48d19f217687165a9af8a629ae2f038962a09cd8 Mon Sep 17 00:00:00 2001 From: Gul Hameed <90828739+gul-hameed@users.noreply.github.com> Date: Thu, 19 Oct 2023 17:03:12 +0500 Subject: [PATCH 37/47] Update slither.yml --- .github/workflows/slither.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/slither.yml b/.github/workflows/slither.yml index 59dded9..8f7eaa3 100644 --- a/.github/workflows/slither.yml +++ b/.github/workflows/slither.yml @@ -9,4 +9,5 @@ jobs: steps: - uses: actions/checkout@v3 - uses: crytic/slither-action@v0.3.0 + run: slither . --print function-summary From caa07f0f6ecc05a75454d05bc947c92aa00ad7dc Mon Sep 17 00:00:00 2001 From: Gul Hameed <90828739+gul-hameed@users.noreply.github.com> Date: Thu, 19 Oct 2023 17:04:58 +0500 Subject: [PATCH 38/47] Update slither.yml --- .github/workflows/slither.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/slither.yml b/.github/workflows/slither.yml index 8f7eaa3..f5b683c 100644 --- a/.github/workflows/slither.yml +++ b/.github/workflows/slither.yml @@ -9,5 +9,5 @@ jobs: steps: - uses: actions/checkout@v3 - uses: crytic/slither-action@v0.3.0 - run: slither . --print function-summary + - run: slither . --print function-summary From 564a5e4fec7bea820afd7455d7f36a09ca576e70 Mon Sep 17 00:00:00 2001 From: Gul Hameed <90828739+gul-hameed@users.noreply.github.com> Date: Thu, 19 Oct 2023 17:13:26 +0500 Subject: [PATCH 39/47] Update slither.yml --- .github/workflows/slither.yml | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/.github/workflows/slither.yml b/.github/workflows/slither.yml index f5b683c..6eba84d 100644 --- a/.github/workflows/slither.yml +++ b/.github/workflows/slither.yml @@ -9,5 +9,10 @@ jobs: steps: - uses: actions/checkout@v3 - uses: crytic/slither-action@v0.3.0 - - run: slither . --print function-summary + - uses: actions/checkout@v2.3.4 + - uses: GuillaumeFalourd/command-output-file-action@v1.1 + with: + command_line: ls -lha + output_file_name: output.txt + display_file_content: YES # this is also the default value if not informed From 20e8888d82a355beddab39ea79e289ff8a45d93c Mon Sep 17 00:00:00 2001 From: Gul Hameed <90828739+gul-hameed@users.noreply.github.com> Date: Thu, 19 Oct 2023 17:20:14 +0500 Subject: [PATCH 40/47] Update slither.yml --- .github/workflows/slither.yml | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/.github/workflows/slither.yml b/.github/workflows/slither.yml index 6eba84d..5339ba7 100644 --- a/.github/workflows/slither.yml +++ b/.github/workflows/slither.yml @@ -9,10 +9,5 @@ jobs: steps: - uses: actions/checkout@v3 - uses: crytic/slither-action@v0.3.0 - - uses: actions/checkout@v2.3.4 - - uses: GuillaumeFalourd/command-output-file-action@v1.1 - with: - command_line: ls -lha - output_file_name: output.txt - display_file_content: YES # this is also the default value if not informed + From 6a6b68f35255ad76182348d507e6a1180a1d8de2 Mon Sep 17 00:00:00 2001 From: Gul Hameed <90828739+gul-hameed@users.noreply.github.com> Date: Thu, 19 Oct 2023 17:30:55 +0500 Subject: [PATCH 41/47] Update slither.yml --- .github/workflows/slither.yml | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/.github/workflows/slither.yml b/.github/workflows/slither.yml index 5339ba7..a0bb593 100644 --- a/.github/workflows/slither.yml +++ b/.github/workflows/slither.yml @@ -1,13 +1,26 @@ -name: Slither Analysiss +name: Slither Analysis + on: push: branches: - 'feat/operator-restrictions' + jobs: analyze: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3 - - uses: crytic/slither-action@v0.3.0 - - + - name: Checkout repository + uses: actions/checkout@v3 + + - name: Run Slither + uses: crytic/slither-action@v0.3.0 + with: + args: --json > slither-result.json + + - name: Commit and push Slither results + run: | + git config --local user.email "action@github.com" + git config --local user.name "GitHub Action" + git add slither-result.json + git commit -m "Add Slither analysis results" -a || echo "No changes to commit" + git push From caf8f71f6f7f8004286219d4532b69ac23b61d1a Mon Sep 17 00:00:00 2001 From: Gul Hameed <90828739+gul-hameed@users.noreply.github.com> Date: Thu, 19 Oct 2023 17:37:27 +0500 Subject: [PATCH 42/47] Update slither.yml --- .github/workflows/slither.yml | 33 +++++++++++++++++++++++---------- 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/.github/workflows/slither.yml b/.github/workflows/slither.yml index a0bb593..c53f53c 100644 --- a/.github/workflows/slither.yml +++ b/.github/workflows/slither.yml @@ -1,4 +1,4 @@ -name: Slither Analysis +name: Slither Analysis & Workflow Modification on: push: @@ -6,21 +6,34 @@ on: - 'feat/operator-restrictions' jobs: - analyze: + analyze-and-modify: runs-on: ubuntu-latest steps: - name: Checkout repository uses: actions/checkout@v3 + + - name: Run Slither and save output + run: | + slither . --json slither-result.json - - name: Run Slither - uses: crytic/slither-action@v0.3.0 - with: - args: --json > slither-result.json - - - name: Commit and push Slither results + # --- MODIFY WORKFLOWS --- + - name: Check and modify deprecated GitHub actions + run: | + workflowList=$(ls ./.github/workflows/*) + for workflow in $workflowList + do + if grep -q -e ::save-state -e ::set-output $workflow; then + sed -i 's|"::set-output name=\([^"]*\)::\([^"]*\)"|"\1=\2" >> $GITHUB_OUTPUT|g' $workflow + sed -i 's|"::save-state name=\([^"]*\)::\([^"]*\)"|"\1=\2" >> $GITHUB_STATE|g' $workflow + fi + done + + # --- COMMIT CHANGES --- + - name: Commit changes run: | git config --local user.email "action@github.com" git config --local user.name "GitHub Action" - git add slither-result.json - git commit -m "Add Slither analysis results" -a || echo "No changes to commit" + + git add . + git commit -m "Update Slither results and modify workflows" || echo "No changes to commit" git push From 6af3d103c5f0431367d3e02a85527ae104192f17 Mon Sep 17 00:00:00 2001 From: Gul Hameed <90828739+gul-hameed@users.noreply.github.com> Date: Thu, 19 Oct 2023 17:49:49 +0500 Subject: [PATCH 43/47] Update slither.yml --- .github/workflows/slither.yml | 36 ++++------------------------------- 1 file changed, 4 insertions(+), 32 deletions(-) diff --git a/.github/workflows/slither.yml b/.github/workflows/slither.yml index c53f53c..2530a99 100644 --- a/.github/workflows/slither.yml +++ b/.github/workflows/slither.yml @@ -1,39 +1,11 @@ -name: Slither Analysis & Workflow Modification - +name: Slither Analysiss on: push: branches: - 'feat/operator-restrictions' - jobs: - analyze-and-modify: + analyze: runs-on: ubuntu-latest steps: - - name: Checkout repository - uses: actions/checkout@v3 - - - name: Run Slither and save output - run: | - slither . --json slither-result.json - - # --- MODIFY WORKFLOWS --- - - name: Check and modify deprecated GitHub actions - run: | - workflowList=$(ls ./.github/workflows/*) - for workflow in $workflowList - do - if grep -q -e ::save-state -e ::set-output $workflow; then - sed -i 's|"::set-output name=\([^"]*\)::\([^"]*\)"|"\1=\2" >> $GITHUB_OUTPUT|g' $workflow - sed -i 's|"::save-state name=\([^"]*\)::\([^"]*\)"|"\1=\2" >> $GITHUB_STATE|g' $workflow - fi - done - - # --- COMMIT CHANGES --- - - name: Commit changes - run: | - git config --local user.email "action@github.com" - git config --local user.name "GitHub Action" - - git add . - git commit -m "Update Slither results and modify workflows" || echo "No changes to commit" - git push + - uses: actions/checkout@v3 + - uses: crytic/slither-action@v0.3.0 From 31880f4ded23119696b5e3e0d72cf0bf379ed390 Mon Sep 17 00:00:00 2001 From: Gul Hameed <90828739+gul-hameed@users.noreply.github.com> Date: Wed, 25 Oct 2023 19:54:49 +0500 Subject: [PATCH 44/47] Update slither.yml --- .github/workflows/slither.yml | 44 +++++++++++++++++++++++++++++------ 1 file changed, 37 insertions(+), 7 deletions(-) diff --git a/.github/workflows/slither.yml b/.github/workflows/slither.yml index 2530a99..77a07aa 100644 --- a/.github/workflows/slither.yml +++ b/.github/workflows/slither.yml @@ -1,11 +1,41 @@ -name: Slither Analysiss -on: - push: - branches: - - 'feat/operator-restrictions' +name: Slither Analysis + +on: [push, pull_request] + jobs: analyze: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3 - - uses: crytic/slither-action@v0.3.0 + - name: Checkout repository + uses: actions/checkout@v3 + + - name: Run Slither + uses: crytic/slither-action@v0.3.0 + id: slither + with: + target: 'src/' + node-version: 16 + fail-on: none + slither-args: --checklist --show-ignored-findings --markdown-root ${{ github.server_url }}/${{ github.repository }}/blob/${{ github.sha }}/ + + - name: Save Slither output to file + run: echo "${{ steps.slither.outputs.stdout }}" > slither-output.txt + + - name: Configure Git + run: | + git config user.name "GitHub Actions Bot" + git config user.email "actions@github.com" + + - name: Commit and push changes + run: | + git add slither-output.txt + git commit -m "Update Slither analysis output" + git push + + # Upload Slither Report as an Artifact + - name: Upload Slither Report + if: always() # Ensure the artifact is uploaded even if previous steps fail + uses: actions/upload-artifact@v3 + with: + name: slither-report + path: slither-output.txt From ccaaaf04a40d051451ad5f05f7283075898c92df Mon Sep 17 00:00:00 2001 From: Gul Hameed <90828739+gul-hameed@users.noreply.github.com> Date: Wed, 25 Oct 2023 19:57:56 +0500 Subject: [PATCH 45/47] Update slither.yml --- .github/workflows/slither.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/slither.yml b/.github/workflows/slither.yml index 77a07aa..51d23d2 100644 --- a/.github/workflows/slither.yml +++ b/.github/workflows/slither.yml @@ -14,7 +14,7 @@ jobs: id: slither with: target: 'src/' - node-version: 16 + node-version: 20 fail-on: none slither-args: --checklist --show-ignored-findings --markdown-root ${{ github.server_url }}/${{ github.repository }}/blob/${{ github.sha }}/ From 4508197d6ddfc5e78069a9475f8851161455242e Mon Sep 17 00:00:00 2001 From: Gul Hameed <90828739+gul-hameed@users.noreply.github.com> Date: Wed, 25 Oct 2023 19:58:44 +0500 Subject: [PATCH 46/47] Create slither-output.txt --- slither-output.txt | 1 + 1 file changed, 1 insertion(+) create mode 100644 slither-output.txt diff --git a/slither-output.txt b/slither-output.txt new file mode 100644 index 0000000..8b13789 --- /dev/null +++ b/slither-output.txt @@ -0,0 +1 @@ + From c4921e16e46d37e59fe41a44330f52b8be92bfa7 Mon Sep 17 00:00:00 2001 From: Gul Hameed <90828739+gul-hameed@users.noreply.github.com> Date: Wed, 25 Oct 2023 20:08:44 +0500 Subject: [PATCH 47/47] Update slither.yml --- .github/workflows/slither.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/slither.yml b/.github/workflows/slither.yml index 51d23d2..1d2a8c2 100644 --- a/.github/workflows/slither.yml +++ b/.github/workflows/slither.yml @@ -13,8 +13,8 @@ jobs: uses: crytic/slither-action@v0.3.0 id: slither with: - target: 'src/' - node-version: 20 + target: 'src' + node-version: 16 fail-on: none slither-args: --checklist --show-ignored-findings --markdown-root ${{ github.server_url }}/${{ github.repository }}/blob/${{ github.sha }}/