From 9a17ce13839ff0fb4f0312b146a6bd46962923bb Mon Sep 17 00:00:00 2001 From: Alex Kroeger Date: Mon, 26 Apr 2021 14:32:35 -0700 Subject: [PATCH] add registerAllowedSigner to NativeOrdersFeature (#195) * add registerAllowedSigner to NativeOrdersFeature * fix PR reference in changelog * prettier * add cancel pair orders functions for signers * refactored cancelPairOrders logic, changed naming of signerRegistry to orderSigner registry everywhere, cleaned up tests * updated changelog for protocol-utils, made batchCancelPairOrders functions more efficient * clean up function documentation * added additional tests for batchCancelPairsWithSigner functions, added additional checks for events being emitted correctly * fix typos in function documentation * fix typo in function documentation * update docs * update comments on cancel functions, small tweaks to docs --- contracts/zero-ex/CHANGELOG.json | 9 + .../src/errors/LibNativeOrdersRichErrors.sol | 15 + .../src/features/NativeOrdersFeature.sol | 6 + .../interfaces/INativeOrdersEvents.sol | 10 + .../interfaces/INativeOrdersFeature.sol | 97 +++- .../NativeOrdersCancellation.sol | 279 ++++++++--- .../native_orders/NativeOrdersInfo.sol | 30 +- .../native_orders/NativeOrdersSettlement.sol | 21 +- .../src/storage/LibNativeOrdersStorage.sol | 3 + ...tOrderSignerRegistryWithContractWallet.sol | 55 ++ contracts/zero-ex/package.json | 2 +- contracts/zero-ex/test/artifacts.ts | 2 + .../features/native_orders_feature_test.ts | 472 +++++++++++++++++- contracts/zero-ex/test/wrappers.ts | 1 + contracts/zero-ex/tsconfig.json | 1 + docs/basics/events.rst | 19 +- docs/basics/functions.rst | 266 +++++++--- docs/basics/orders.rst | 2 +- packages/protocol-utils/CHANGELOG.json | 9 + packages/protocol-utils/src/orders.ts | 5 +- .../src/revert-errors/native_orders.ts | 10 + 21 files changed, 1180 insertions(+), 134 deletions(-) create mode 100644 contracts/zero-ex/contracts/test/TestOrderSignerRegistryWithContractWallet.sol diff --git a/contracts/zero-ex/CHANGELOG.json b/contracts/zero-ex/CHANGELOG.json index 596748e4c0..3ee3746959 100644 --- a/contracts/zero-ex/CHANGELOG.json +++ b/contracts/zero-ex/CHANGELOG.json @@ -1,4 +1,13 @@ [ + { + "version": "0.22.0", + "changes": [ + { + "note": "Add order signer registry to NativeOrdersFeature", + "pr": 195 + } + ] + }, { "timestamp": 1618259868, "version": "0.21.1", diff --git a/contracts/zero-ex/contracts/src/errors/LibNativeOrdersRichErrors.sol b/contracts/zero-ex/contracts/src/errors/LibNativeOrdersRichErrors.sol index e339caac44..a12ab14281 100644 --- a/contracts/zero-ex/contracts/src/errors/LibNativeOrdersRichErrors.sol +++ b/contracts/zero-ex/contracts/src/errors/LibNativeOrdersRichErrors.sol @@ -88,6 +88,21 @@ library LibNativeOrdersRichErrors { ); } + function InvalidSignerError( + address maker, + address signer + ) + internal + pure + returns (bytes memory) + { + return abi.encodeWithSelector( + bytes4(keccak256("InvalidSignerError(address,address)")), + maker, + signer + ); + } + function OrderNotFillableBySenderError( bytes32 orderHash, address sender, diff --git a/contracts/zero-ex/contracts/src/features/NativeOrdersFeature.sol b/contracts/zero-ex/contracts/src/features/NativeOrdersFeature.sol index 93684b4dac..100e3ebe20 100644 --- a/contracts/zero-ex/contracts/src/features/NativeOrdersFeature.sol +++ b/contracts/zero-ex/contracts/src/features/NativeOrdersFeature.sol @@ -74,9 +74,13 @@ contract NativeOrdersFeature is _registerFeatureFunction(this.batchCancelLimitOrders.selector); _registerFeatureFunction(this.batchCancelRfqOrders.selector); _registerFeatureFunction(this.cancelPairLimitOrders.selector); + _registerFeatureFunction(this.cancelPairLimitOrdersWithSigner.selector); _registerFeatureFunction(this.batchCancelPairLimitOrders.selector); + _registerFeatureFunction(this.batchCancelPairLimitOrdersWithSigner.selector); _registerFeatureFunction(this.cancelPairRfqOrders.selector); + _registerFeatureFunction(this.cancelPairRfqOrdersWithSigner.selector); _registerFeatureFunction(this.batchCancelPairRfqOrders.selector); + _registerFeatureFunction(this.batchCancelPairRfqOrdersWithSigner.selector); _registerFeatureFunction(this.getLimitOrderInfo.selector); _registerFeatureFunction(this.getRfqOrderInfo.selector); _registerFeatureFunction(this.getLimitOrderHash.selector); @@ -87,6 +91,8 @@ contract NativeOrdersFeature is _registerFeatureFunction(this.getRfqOrderRelevantState.selector); _registerFeatureFunction(this.batchGetLimitOrderRelevantStates.selector); _registerFeatureFunction(this.batchGetRfqOrderRelevantStates.selector); + _registerFeatureFunction(this.registerAllowedOrderSigner.selector); + _registerFeatureFunction(this.isValidOrderSigner.selector); return LibMigrate.MIGRATE_SUCCESS; } } diff --git a/contracts/zero-ex/contracts/src/features/interfaces/INativeOrdersEvents.sol b/contracts/zero-ex/contracts/src/features/interfaces/INativeOrdersEvents.sol index d055f1705c..ff7c7810b6 100644 --- a/contracts/zero-ex/contracts/src/features/interfaces/INativeOrdersEvents.sol +++ b/contracts/zero-ex/contracts/src/features/interfaces/INativeOrdersEvents.sol @@ -113,4 +113,14 @@ interface INativeOrdersEvents { address[] addrs, bool allowed ); + + /// @dev Emitted when new order signers are registered + /// @param maker The maker address that is registering a designated signer. + /// @param signer The address that will sign on behalf of maker. + /// @param allowed Indicates whether the address should be allowed. + event OrderSignerRegistered( + address maker, + address signer, + bool allowed + ); } diff --git a/contracts/zero-ex/contracts/src/features/interfaces/INativeOrdersFeature.sol b/contracts/zero-ex/contracts/src/features/interfaces/INativeOrdersFeature.sol index 9ddb5306bd..b85130b0a8 100644 --- a/contracts/zero-ex/contracts/src/features/interfaces/INativeOrdersFeature.sol +++ b/contracts/zero-ex/contracts/src/features/interfaces/INativeOrdersFeature.sol @@ -137,13 +137,13 @@ interface INativeOrdersFeature is external returns (uint128 takerTokenFilledAmount, uint128 makerTokenFilledAmount); - /// @dev Cancel a single limit order. The caller must be the maker. + /// @dev Cancel a single limit order. The caller must be the maker or a valid order signer. /// Silently succeeds if the order has already been cancelled. /// @param order The limit order. function cancelLimitOrder(LibNativeOrder.LimitOrder calldata order) external; - /// @dev Cancel a single RFQ order. The caller must be the maker. + /// @dev Cancel a single RFQ order. The caller must be the maker or a valid order signer. /// Silently succeeds if the order has already been cancelled. /// @param order The RFQ order. function cancelRfqOrder(LibNativeOrder.RfqOrder calldata order) @@ -156,13 +156,13 @@ interface INativeOrdersFeature is function registerAllowedRfqOrigins(address[] memory origins, bool allowed) external; - /// @dev Cancel multiple limit orders. The caller must be the maker. + /// @dev Cancel multiple limit orders. The caller must be the maker or a valid order signer. /// Silently succeeds if the order has already been cancelled. /// @param orders The limit orders. function batchCancelLimitOrders(LibNativeOrder.LimitOrder[] calldata orders) external; - /// @dev Cancel multiple RFQ orders. The caller must be the maker. + /// @dev Cancel multiple RFQ orders. The caller must be the maker or a valid order signer. /// Silently succeeds if the order has already been cancelled. /// @param orders The RFQ orders. function batchCancelRfqOrders(LibNativeOrder.RfqOrder[] calldata orders) @@ -183,7 +183,23 @@ interface INativeOrdersFeature is external; /// @dev Cancel all limit orders for a given maker and pair with a salt less - /// than the value provided. The caller must be the maker. Subsequent + /// than the value provided. The caller must be a signer registered to the maker. + /// Subsequent calls to this function with the same maker and pair require the + /// new salt to be >= the old salt. + /// @param maker The maker for which to cancel. + /// @param makerToken The maker token. + /// @param takerToken The taker token. + /// @param minValidSalt The new minimum valid salt. + function cancelPairLimitOrdersWithSigner( + address maker, + IERC20TokenV06 makerToken, + IERC20TokenV06 takerToken, + uint256 minValidSalt + ) + external; + + /// @dev Cancel all limit orders for a given maker and pairs with salts less + /// than the values provided. The caller must be the maker. Subsequent /// calls to this function with the same caller and pair require the /// new salt to be >= the old salt. /// @param makerTokens The maker tokens. @@ -196,6 +212,22 @@ interface INativeOrdersFeature is ) external; + /// @dev Cancel all limit orders for a given maker and pairs with salts less + /// than the values provided. The caller must be a signer registered to the maker. + /// Subsequent calls to this function with the same maker and pair require the + /// new salt to be >= the old salt. + /// @param maker The maker for which to cancel. + /// @param makerTokens The maker tokens. + /// @param takerTokens The taker tokens. + /// @param minValidSalts The new minimum valid salts. + function batchCancelPairLimitOrdersWithSigner( + address maker, + IERC20TokenV06[] memory makerTokens, + IERC20TokenV06[] memory takerTokens, + uint256[] memory minValidSalts + ) + external; + /// @dev Cancel all RFQ orders for a given maker and pair with a salt less /// than the value provided. The caller must be the maker. Subsequent /// calls to this function with the same caller and pair require the @@ -211,7 +243,23 @@ interface INativeOrdersFeature is external; /// @dev Cancel all RFQ orders for a given maker and pair with a salt less - /// than the value provided. The caller must be the maker. Subsequent + /// than the value provided. The caller must be a signer registered to the maker. + /// Subsequent calls to this function with the same maker and pair require the + /// new salt to be >= the old salt. + /// @param maker The maker for which to cancel. + /// @param makerToken The maker token. + /// @param takerToken The taker token. + /// @param minValidSalt The new minimum valid salt. + function cancelPairRfqOrdersWithSigner( + address maker, + IERC20TokenV06 makerToken, + IERC20TokenV06 takerToken, + uint256 minValidSalt + ) + external; + + /// @dev Cancel all RFQ orders for a given maker and pairs with salts less + /// than the values provided. The caller must be the maker. Subsequent /// calls to this function with the same caller and pair require the /// new salt to be >= the old salt. /// @param makerTokens The maker tokens. @@ -224,6 +272,22 @@ interface INativeOrdersFeature is ) external; + /// @dev Cancel all RFQ orders for a given maker and pairs with salts less + /// than the values provided. The caller must be a signer registered to the maker. + /// Subsequent calls to this function with the same maker and pair require the + /// new salt to be >= the old salt. + /// @param maker The maker for which to cancel. + /// @param makerTokens The maker tokens. + /// @param takerTokens The taker tokens. + /// @param minValidSalts The new minimum valid salts. + function batchCancelPairRfqOrdersWithSigner( + address maker, + IERC20TokenV06[] memory makerTokens, + IERC20TokenV06[] memory takerTokens, + uint256[] memory minValidSalts + ) + external; + /// @dev Get the order info for a limit order. /// @param order The limit order. /// @return orderInfo Info about the order. @@ -345,4 +409,25 @@ interface INativeOrdersFeature is uint128[] memory actualFillableTakerTokenAmounts, bool[] memory isSignatureValids ); + + /// @dev Register a signer who can sign on behalf of msg.sender + /// This allows one to sign on behalf of a contract that calls this function + /// @param signer The address from which you plan to generate signatures + /// @param allowed True to register, false to unregister. + function registerAllowedOrderSigner( + address signer, + bool allowed + ) + external; + + /// @dev checks if a given address is registered to sign on behalf of a maker address + /// @param maker The maker address encoded in an order (can be a contract) + /// @param signer The address that is providing a signature + function isValidOrderSigner( + address maker, + address signer + ) + external + view + returns (bool isAllowed); } diff --git a/contracts/zero-ex/contracts/src/features/native_orders/NativeOrdersCancellation.sol b/contracts/zero-ex/contracts/src/features/native_orders/NativeOrdersCancellation.sol index b90f0a9c91..b3b336952a 100644 --- a/contracts/zero-ex/contracts/src/features/native_orders/NativeOrdersCancellation.sol +++ b/contracts/zero-ex/contracts/src/features/native_orders/NativeOrdersCancellation.sol @@ -48,14 +48,14 @@ abstract contract NativeOrdersCancellation is // solhint-disable no-empty-blocks } - /// @dev Cancel a single limit order. The caller must be the maker. + /// @dev Cancel a single limit order. The caller must be the maker or a valid order signer. /// Silently succeeds if the order has already been cancelled. /// @param order The limit order. function cancelLimitOrder(LibNativeOrder.LimitOrder memory order) public { bytes32 orderHash = getLimitOrderHash(order); - if (msg.sender != order.maker) { + if (msg.sender != order.maker && !isValidOrderSigner(order.maker, msg.sender)) { LibNativeOrdersRichErrors.OnlyOrderMakerAllowed( orderHash, msg.sender, @@ -65,14 +65,14 @@ abstract contract NativeOrdersCancellation is _cancelOrderHash(orderHash, order.maker); } - /// @dev Cancel a single RFQ order. The caller must be the maker. + /// @dev Cancel a single RFQ order. The caller must be the maker or a valid order signer. /// Silently succeeds if the order has already been cancelled. /// @param order The RFQ order. function cancelRfqOrder(LibNativeOrder.RfqOrder memory order) public { bytes32 orderHash = getRfqOrderHash(order); - if (msg.sender != order.maker) { + if (msg.sender != order.maker && !isValidOrderSigner(order.maker, msg.sender)) { LibNativeOrdersRichErrors.OnlyOrderMakerAllowed( orderHash, msg.sender, @@ -82,7 +82,7 @@ abstract contract NativeOrdersCancellation is _cancelOrderHash(orderHash, order.maker); } - /// @dev Cancel multiple limit orders. The caller must be the maker. + /// @dev Cancel multiple limit orders. The caller must be the maker or a valid order signer. /// Silently succeeds if the order has already been cancelled. /// @param orders The limit orders. function batchCancelLimitOrders(LibNativeOrder.LimitOrder[] memory orders) @@ -93,7 +93,7 @@ abstract contract NativeOrdersCancellation is } } - /// @dev Cancel multiple RFQ orders. The caller must be the maker. + /// @dev Cancel multiple RFQ orders. The caller must be the maker or a valid order signer. /// Silently succeeds if the order has already been cancelled. /// @param orders The RFQ orders. function batchCancelRfqOrders(LibNativeOrder.RfqOrder[] memory orders) @@ -118,33 +118,34 @@ abstract contract NativeOrdersCancellation is ) public { - LibNativeOrdersStorage.Storage storage stor = - LibNativeOrdersStorage.getStorage(); + _cancelPairLimitOrders(msg.sender, makerToken, takerToken, minValidSalt); + } - uint256 oldMinValidSalt = - stor.limitOrdersMakerToMakerTokenToTakerTokenToMinValidOrderSalt - [msg.sender] - [address(makerToken)] - [address(takerToken)]; - - // New min salt must >= the old one. - if (oldMinValidSalt > minValidSalt) { - LibNativeOrdersRichErrors. - CancelSaltTooLowError(minValidSalt, oldMinValidSalt) - .rrevert(); + /// @dev Cancel all limit orders for a given maker and pair with a salt less + /// than the value provided. The caller must be a signer registered to the maker. + /// Subsequent calls to this function with the same caller and pair require the + /// new salt to be >= the old salt. + /// @param maker the maker for whom the msg.sender is the signer. + /// @param makerToken The maker token. + /// @param takerToken The taker token. + /// @param minValidSalt The new minimum valid salt. + function cancelPairLimitOrdersWithSigner( + address maker, + IERC20TokenV06 makerToken, + IERC20TokenV06 takerToken, + uint256 minValidSalt + ) + public + { + // verify that the signer is authorized for the maker + if (!isValidOrderSigner(maker, msg.sender)) { + LibNativeOrdersRichErrors.InvalidSignerError( + maker, + msg.sender + ).rrevert(); } - stor.limitOrdersMakerToMakerTokenToTakerTokenToMinValidOrderSalt - [msg.sender] - [address(makerToken)] - [address(takerToken)] = minValidSalt; - - emit PairCancelledLimitOrders( - msg.sender, - address(makerToken), - address(takerToken), - minValidSalt - ); + _cancelPairLimitOrders(maker, makerToken, takerToken, minValidSalt); } /// @dev Cancel all limit orders for a given maker and pair with a salt less @@ -168,7 +169,47 @@ abstract contract NativeOrdersCancellation is ); for (uint256 i = 0; i < makerTokens.length; ++i) { - cancelPairLimitOrders( + _cancelPairLimitOrders( + msg.sender, + makerTokens[i], + takerTokens[i], + minValidSalts[i] + ); + } + } + + /// @dev Cancel all limit orders for a given maker and pair with a salt less + /// than the value provided. The caller must be a signer registered to the maker. + /// Subsequent calls to this function with the same caller and pair require the + /// new salt to be >= the old salt. + /// @param maker the maker for whom the msg.sender is the signer. + /// @param makerTokens The maker tokens. + /// @param takerTokens The taker tokens. + /// @param minValidSalts The new minimum valid salts. + function batchCancelPairLimitOrdersWithSigner( + address maker, + IERC20TokenV06[] memory makerTokens, + IERC20TokenV06[] memory takerTokens, + uint256[] memory minValidSalts + ) + public + { + require( + makerTokens.length == takerTokens.length && + makerTokens.length == minValidSalts.length, + "NativeOrdersFeature/MISMATCHED_PAIR_ORDERS_ARRAY_LENGTHS" + ); + + if (!isValidOrderSigner(maker, msg.sender)) { + LibNativeOrdersRichErrors.InvalidSignerError( + maker, + msg.sender + ).rrevert(); + } + + for (uint256 i = 0; i < makerTokens.length; ++i) { + _cancelPairLimitOrders( + maker, makerTokens[i], takerTokens[i], minValidSalts[i] @@ -190,33 +231,33 @@ abstract contract NativeOrdersCancellation is ) public { - LibNativeOrdersStorage.Storage storage stor = - LibNativeOrdersStorage.getStorage(); + _cancelPairRfqOrders(msg.sender, makerToken, takerToken, minValidSalt); + } - uint256 oldMinValidSalt = - stor.rfqOrdersMakerToMakerTokenToTakerTokenToMinValidOrderSalt - [msg.sender] - [address(makerToken)] - [address(takerToken)]; - - // New min salt must >= the old one. - if (oldMinValidSalt > minValidSalt) { - LibNativeOrdersRichErrors. - CancelSaltTooLowError(minValidSalt, oldMinValidSalt) - .rrevert(); + /// @dev Cancel all RFQ orders for a given maker and pair with a salt less + /// than the value provided. The caller must be a signer registered to the maker. + /// Subsequent calls to this function with the same caller and pair require the + /// new salt to be >= the old salt. + /// @param maker the maker for whom the msg.sender is the signer. + /// @param makerToken The maker token. + /// @param takerToken The taker token. + /// @param minValidSalt The new minimum valid salt. + function cancelPairRfqOrdersWithSigner( + address maker, + IERC20TokenV06 makerToken, + IERC20TokenV06 takerToken, + uint256 minValidSalt + ) + public + { + if (!isValidOrderSigner(maker, msg.sender)) { + LibNativeOrdersRichErrors.InvalidSignerError( + maker, + msg.sender + ).rrevert(); } - stor.rfqOrdersMakerToMakerTokenToTakerTokenToMinValidOrderSalt - [msg.sender] - [address(makerToken)] - [address(takerToken)] = minValidSalt; - - emit PairCancelledRfqOrders( - msg.sender, - address(makerToken), - address(takerToken), - minValidSalt - ); + _cancelPairRfqOrders(maker, makerToken, takerToken, minValidSalt); } /// @dev Cancel all RFQ orders for a given maker and pair with a salt less @@ -240,7 +281,47 @@ abstract contract NativeOrdersCancellation is ); for (uint256 i = 0; i < makerTokens.length; ++i) { - cancelPairRfqOrders( + _cancelPairRfqOrders( + msg.sender, + makerTokens[i], + takerTokens[i], + minValidSalts[i] + ); + } + } + + /// @dev Cancel all RFQ orders for a given maker and pairs with salts less + /// than the values provided. The caller must be a signer registered to the maker. + /// Subsequent calls to this function with the same caller and pair require the + /// new salt to be >= the old salt. + /// @param maker the maker for whom the msg.sender is the signer. + /// @param makerTokens The maker tokens. + /// @param takerTokens The taker tokens. + /// @param minValidSalts The new minimum valid salts. + function batchCancelPairRfqOrdersWithSigner( + address maker, + IERC20TokenV06[] memory makerTokens, + IERC20TokenV06[] memory takerTokens, + uint256[] memory minValidSalts + ) + public + { + require( + makerTokens.length == takerTokens.length && + makerTokens.length == minValidSalts.length, + "NativeOrdersFeature/MISMATCHED_PAIR_ORDERS_ARRAY_LENGTHS" + ); + + if (!isValidOrderSigner(maker, msg.sender)) { + LibNativeOrdersRichErrors.InvalidSignerError( + maker, + msg.sender + ).rrevert(); + } + + for (uint256 i = 0; i < makerTokens.length; ++i) { + _cancelPairRfqOrders( + maker, makerTokens[i], takerTokens[i], minValidSalts[i] @@ -262,4 +343,90 @@ abstract contract NativeOrdersCancellation is emit OrderCancelled(orderHash, maker); } + + /// @dev Cancel all RFQ orders for a given maker and pair with a salt less + /// than the value provided. + /// @param maker The target maker address + /// @param makerToken The maker token. + /// @param takerToken The taker token. + /// @param minValidSalt The new minimum valid salt. + function _cancelPairRfqOrders( + address maker, + IERC20TokenV06 makerToken, + IERC20TokenV06 takerToken, + uint256 minValidSalt + ) + private + { + LibNativeOrdersStorage.Storage storage stor = + LibNativeOrdersStorage.getStorage(); + + uint256 oldMinValidSalt = + stor.rfqOrdersMakerToMakerTokenToTakerTokenToMinValidOrderSalt + [maker] + [address(makerToken)] + [address(takerToken)]; + + // New min salt must >= the old one. + if (oldMinValidSalt > minValidSalt) { + LibNativeOrdersRichErrors. + CancelSaltTooLowError(minValidSalt, oldMinValidSalt) + .rrevert(); + } + + stor.rfqOrdersMakerToMakerTokenToTakerTokenToMinValidOrderSalt + [maker] + [address(makerToken)] + [address(takerToken)] = minValidSalt; + + emit PairCancelledRfqOrders( + maker, + address(makerToken), + address(takerToken), + minValidSalt + ); + } + + /// @dev Cancel all limit orders for a given maker and pair with a salt less + /// than the value provided. + /// @param maker The target maker address + /// @param makerToken The maker token. + /// @param takerToken The taker token. + /// @param minValidSalt The new minimum valid salt. + function _cancelPairLimitOrders( + address maker, + IERC20TokenV06 makerToken, + IERC20TokenV06 takerToken, + uint256 minValidSalt + ) + private + { + LibNativeOrdersStorage.Storage storage stor = + LibNativeOrdersStorage.getStorage(); + + uint256 oldMinValidSalt = + stor.limitOrdersMakerToMakerTokenToTakerTokenToMinValidOrderSalt + [maker] + [address(makerToken)] + [address(takerToken)]; + + // New min salt must >= the old one. + if (oldMinValidSalt > minValidSalt) { + LibNativeOrdersRichErrors. + CancelSaltTooLowError(minValidSalt, oldMinValidSalt) + .rrevert(); + } + + stor.limitOrdersMakerToMakerTokenToTakerTokenToMinValidOrderSalt + [maker] + [address(makerToken)] + [address(takerToken)] = minValidSalt; + + emit PairCancelledLimitOrders( + maker, + address(makerToken), + address(takerToken), + minValidSalt + ); + } } diff --git a/contracts/zero-ex/contracts/src/features/native_orders/NativeOrdersInfo.sol b/contracts/zero-ex/contracts/src/features/native_orders/NativeOrdersInfo.sol index 47d6fde3ba..16ea926c89 100644 --- a/contracts/zero-ex/contracts/src/features/native_orders/NativeOrdersInfo.sol +++ b/contracts/zero-ex/contracts/src/features/native_orders/NativeOrdersInfo.sol @@ -168,8 +168,10 @@ abstract contract NativeOrdersInfo is orderInfo: orderInfo }) ); - isSignatureValid = order.maker == - LibSignature.getSignerOfHash(orderInfo.orderHash, signature); + address signerOfHash = LibSignature.getSignerOfHash(orderInfo.orderHash, signature); + isSignatureValid = + (order.maker == signerOfHash) || + isValidOrderSigner(order.maker, signerOfHash); } /// @dev Get order info, fillable amount, and signature validity for an RFQ order. @@ -202,8 +204,10 @@ abstract contract NativeOrdersInfo is orderInfo: orderInfo }) ); - isSignatureValid = order.maker == - LibSignature.getSignerOfHash(orderInfo.orderHash, signature); + address signerOfHash = LibSignature.getSignerOfHash(orderInfo.orderHash, signature); + isSignatureValid = + (order.maker == signerOfHash) || + isValidOrderSigner(order.maker, signerOfHash); } /// @dev Batch version of `getLimitOrderRelevantState()`, without reverting. @@ -389,4 +393,22 @@ abstract contract NativeOrdersInfo is uint256(params.orderTakerAmount) ).safeDowncastToUint128(); } + + /// @dev checks if a given address is registered to sign on behalf of a maker address + /// @param maker The maker address encoded in an order (can be a contract) + /// @param signer The address that is providing a signature + function isValidOrderSigner( + address maker, + address signer + ) + public + view + returns (bool isValid) + { + // returns false if it the mapping doesn't exist + return LibNativeOrdersStorage.getStorage() + .orderSignerRegistry + [maker] + [signer]; + } } diff --git a/contracts/zero-ex/contracts/src/features/native_orders/NativeOrdersSettlement.sol b/contracts/zero-ex/contracts/src/features/native_orders/NativeOrdersSettlement.sol index 7c030e1840..ea7e832a44 100644 --- a/contracts/zero-ex/contracts/src/features/native_orders/NativeOrdersSettlement.sol +++ b/contracts/zero-ex/contracts/src/features/native_orders/NativeOrdersSettlement.sol @@ -370,7 +370,7 @@ abstract contract NativeOrdersSettlement is orderInfo.orderHash, params.signature ); - if (signer != params.order.maker) { + if (signer != params.order.maker && !isValidOrderSigner(params.order.maker, signer)) { LibNativeOrdersRichErrors.OrderNotSignedByMakerError( orderInfo.orderHash, signer, @@ -478,7 +478,7 @@ abstract contract NativeOrdersSettlement is // Signature must be valid for the order. { address signer = LibSignature.getSignerOfHash(orderInfo.orderHash, signature); - if (signer != order.maker) { + if (signer != order.maker && !isValidOrderSigner(order.maker, signer)) { LibNativeOrdersRichErrors.OrderNotSignedByMakerError( orderInfo.orderHash, signer, @@ -565,4 +565,21 @@ abstract contract NativeOrdersSettlement is makerTokenFilledAmount ); } + + /// @dev register a signer who can sign on behalf of msg.sender + /// @param signer The address from which you plan to generate signatures + /// @param allowed True to register, false to unregister. + function registerAllowedOrderSigner( + address signer, + bool allowed + ) + external + { + LibNativeOrdersStorage.Storage storage stor = + LibNativeOrdersStorage.getStorage(); + + stor.orderSignerRegistry[msg.sender][signer] = allowed; + + emit OrderSignerRegistered(msg.sender, signer, allowed); + } } diff --git a/contracts/zero-ex/contracts/src/storage/LibNativeOrdersStorage.sol b/contracts/zero-ex/contracts/src/storage/LibNativeOrdersStorage.sol index a4adf413c1..ceb899281e 100644 --- a/contracts/zero-ex/contracts/src/storage/LibNativeOrdersStorage.sol +++ b/contracts/zero-ex/contracts/src/storage/LibNativeOrdersStorage.sol @@ -43,6 +43,9 @@ library LibNativeOrdersStorage { // For a given order origin, which tx.origin addresses are allowed to // fill the order. mapping(address => mapping(address => bool)) originRegistry; + // For a given maker address, which addresses are allowed to + // sign on its behalf. + mapping(address => mapping(address => bool)) orderSignerRegistry; } /// @dev Get the storage bucket for this contract. diff --git a/contracts/zero-ex/contracts/test/TestOrderSignerRegistryWithContractWallet.sol b/contracts/zero-ex/contracts/test/TestOrderSignerRegistryWithContractWallet.sol new file mode 100644 index 0000000000..6bd23aff34 --- /dev/null +++ b/contracts/zero-ex/contracts/test/TestOrderSignerRegistryWithContractWallet.sol @@ -0,0 +1,55 @@ +// SPDX-License-Identifier: Apache-2.0 +/* + + Copyright 2021 ZeroEx Intl. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + +*/ + +pragma solidity ^0.6.5; +pragma experimental ABIEncoderV2; + +import "@0x/contracts-utils/contracts/src/v06/OwnableV06.sol"; +import "@0x/contracts-erc20/contracts/src/v06/IERC20TokenV06.sol"; +import "../src/IZeroEx.sol"; + +contract TestOrderSignerRegistryWithContractWallet is OwnableV06 { + + IZeroEx immutable zeroex; + + constructor(IZeroEx _zeroex) public { + zeroex = _zeroex; + } + + function registerAllowedOrderSigner( + address signer, + bool allowed + ) + external + onlyOwner + { + zeroex.registerAllowedOrderSigner(signer, allowed); + } + + function approveERC20( + IERC20TokenV06 token, + address spender, + uint256 value + ) + external + onlyOwner + { + token.approve(spender, value); + } +} diff --git a/contracts/zero-ex/package.json b/contracts/zero-ex/package.json index f3ca125b90..5246ec3e48 100644 --- a/contracts/zero-ex/package.json +++ b/contracts/zero-ex/package.json @@ -43,7 +43,7 @@ "config": { "publicInterfaceContracts": "IZeroEx,ZeroEx,FullMigration,InitialMigration,IFlashWallet,IERC20Transformer,IOwnableFeature,ISimpleFunctionRegistryFeature,ITransformERC20Feature,FillQuoteTransformer,PayTakerTransformer,PositiveSlippageFeeTransformer,WethTransformer,OwnableFeature,SimpleFunctionRegistryFeature,TransformERC20Feature,AffiliateFeeTransformer,MetaTransactionsFeature,LogMetadataTransformer,BridgeAdapter,LiquidityProviderFeature,ILiquidityProviderFeature,NativeOrdersFeature,INativeOrdersFeature,FeeCollectorController,FeeCollector,CurveLiquidityProvider,BatchFillNativeOrdersFeature,IBatchFillNativeOrdersFeature,MultiplexFeature,IMultiplexFeature", "abis:comment": "This list is auto-generated by contracts-gen. Don't edit manually.", - "abis": "./test/generated-artifacts/@(AffiliateFeeTransformer|BatchFillNativeOrdersFeature|BootstrapFeature|BridgeAdapter|BridgeProtocols|CurveLiquidityProvider|FeeCollector|FeeCollectorController|FillQuoteTransformer|FixinCommon|FixinEIP712|FixinProtocolFees|FixinReentrancyGuard|FixinTokenSpender|FlashWallet|FullMigration|IBatchFillNativeOrdersFeature|IBootstrapFeature|IBridgeAdapter|IERC20Bridge|IERC20Transformer|IFeature|IFlashWallet|ILiquidityProvider|ILiquidityProviderFeature|ILiquidityProviderSandbox|IMetaTransactionsFeature|IMooniswapPool|IMultiplexFeature|INativeOrdersEvents|INativeOrdersFeature|IOwnableFeature|IPancakeSwapFeature|ISimpleFunctionRegistryFeature|IStaking|ITestSimpleFunctionRegistryFeature|ITokenSpenderFeature|ITransformERC20Feature|IUniswapFeature|IUniswapV2Pair|IZeroEx|InitialMigration|LibBootstrap|LibCommonRichErrors|LibERC20Transformer|LibFeeCollector|LibLiquidityProviderRichErrors|LibMetaTransactionsRichErrors|LibMetaTransactionsStorage|LibMigrate|LibNativeOrder|LibNativeOrdersRichErrors|LibNativeOrdersStorage|LibOwnableRichErrors|LibOwnableStorage|LibProxyRichErrors|LibProxyStorage|LibReentrancyGuardStorage|LibSignature|LibSignatureRichErrors|LibSimpleFunctionRegistryRichErrors|LibSimpleFunctionRegistryStorage|LibStorage|LibTransformERC20RichErrors|LibTransformERC20Storage|LibWalletRichErrors|LiquidityProviderFeature|LiquidityProviderSandbox|LogMetadataTransformer|MetaTransactionsFeature|MixinBalancer|MixinBancor|MixinCoFiX|MixinCryptoCom|MixinCurve|MixinDodo|MixinDodoV2|MixinKyber|MixinMStable|MixinMakerPSM|MixinMooniswap|MixinNerve|MixinOasis|MixinShell|MixinUniswap|MixinUniswapV2|MixinZeroExBridge|MooniswapLiquidityProvider|MultiplexFeature|NativeOrdersCancellation|NativeOrdersFeature|NativeOrdersInfo|NativeOrdersProtocolFees|NativeOrdersSettlement|OwnableFeature|PancakeSwapFeature|PayTakerTransformer|PermissionlessTransformerDeployer|PositiveSlippageFeeTransformer|SimpleFunctionRegistryFeature|TestBridge|TestCallTarget|TestCurve|TestDelegateCaller|TestFeeCollectorController|TestFillQuoteTransformerBridge|TestFillQuoteTransformerExchange|TestFillQuoteTransformerHost|TestFixinProtocolFees|TestFixinTokenSpender|TestFullMigration|TestInitialMigration|TestLibNativeOrder|TestLibSignature|TestLiquidityProvider|TestMetaTransactionsNativeOrdersFeature|TestMetaTransactionsTransformERC20Feature|TestMigrator|TestMintTokenERC20Transformer|TestMintableERC20Token|TestMooniswap|TestNativeOrdersFeature|TestPermissionlessTransformerDeployerSuicidal|TestPermissionlessTransformerDeployerTransformer|TestRfqOriginRegistration|TestSimpleFunctionRegistryFeatureImpl1|TestSimpleFunctionRegistryFeatureImpl2|TestStaking|TestTokenSpenderERC20Token|TestTransformERC20|TestTransformerBase|TestTransformerDeployerTransformer|TestTransformerHost|TestWeth|TestWethTransformerHost|TestZeroExFeature|TransformERC20Feature|Transformer|TransformerDeployer|UniswapFeature|WethTransformer|ZeroEx|ZeroExOptimized).json" + "abis": "./test/generated-artifacts/@(AffiliateFeeTransformer|BatchFillNativeOrdersFeature|BootstrapFeature|BridgeAdapter|BridgeProtocols|CurveLiquidityProvider|FeeCollector|FeeCollectorController|FillQuoteTransformer|FixinCommon|FixinEIP712|FixinProtocolFees|FixinReentrancyGuard|FixinTokenSpender|FlashWallet|FullMigration|IBatchFillNativeOrdersFeature|IBootstrapFeature|IBridgeAdapter|IERC20Bridge|IERC20Transformer|IFeature|IFlashWallet|ILiquidityProvider|ILiquidityProviderFeature|ILiquidityProviderSandbox|IMetaTransactionsFeature|IMooniswapPool|IMultiplexFeature|INativeOrdersEvents|INativeOrdersFeature|IOwnableFeature|IPancakeSwapFeature|ISimpleFunctionRegistryFeature|IStaking|ITestSimpleFunctionRegistryFeature|ITokenSpenderFeature|ITransformERC20Feature|IUniswapFeature|IUniswapV2Pair|IZeroEx|InitialMigration|LibBootstrap|LibCommonRichErrors|LibERC20Transformer|LibFeeCollector|LibLiquidityProviderRichErrors|LibMetaTransactionsRichErrors|LibMetaTransactionsStorage|LibMigrate|LibNativeOrder|LibNativeOrdersRichErrors|LibNativeOrdersStorage|LibOwnableRichErrors|LibOwnableStorage|LibProxyRichErrors|LibProxyStorage|LibReentrancyGuardStorage|LibSignature|LibSignatureRichErrors|LibSimpleFunctionRegistryRichErrors|LibSimpleFunctionRegistryStorage|LibStorage|LibTransformERC20RichErrors|LibTransformERC20Storage|LibWalletRichErrors|LiquidityProviderFeature|LiquidityProviderSandbox|LogMetadataTransformer|MetaTransactionsFeature|MixinBalancer|MixinBancor|MixinCoFiX|MixinCryptoCom|MixinCurve|MixinDodo|MixinDodoV2|MixinKyber|MixinMStable|MixinMakerPSM|MixinMooniswap|MixinNerve|MixinOasis|MixinShell|MixinUniswap|MixinUniswapV2|MixinZeroExBridge|MooniswapLiquidityProvider|MultiplexFeature|NativeOrdersCancellation|NativeOrdersFeature|NativeOrdersInfo|NativeOrdersProtocolFees|NativeOrdersSettlement|OwnableFeature|PancakeSwapFeature|PayTakerTransformer|PermissionlessTransformerDeployer|PositiveSlippageFeeTransformer|SimpleFunctionRegistryFeature|TestBridge|TestCallTarget|TestCurve|TestDelegateCaller|TestFeeCollectorController|TestFillQuoteTransformerBridge|TestFillQuoteTransformerExchange|TestFillQuoteTransformerHost|TestFixinProtocolFees|TestFixinTokenSpender|TestFullMigration|TestInitialMigration|TestLibNativeOrder|TestLibSignature|TestLiquidityProvider|TestMetaTransactionsNativeOrdersFeature|TestMetaTransactionsTransformERC20Feature|TestMigrator|TestMintTokenERC20Transformer|TestMintableERC20Token|TestMooniswap|TestNativeOrdersFeature|TestOrderSignerRegistryWithContractWallet|TestPermissionlessTransformerDeployerSuicidal|TestPermissionlessTransformerDeployerTransformer|TestRfqOriginRegistration|TestSimpleFunctionRegistryFeatureImpl1|TestSimpleFunctionRegistryFeatureImpl2|TestStaking|TestTokenSpenderERC20Token|TestTransformERC20|TestTransformerBase|TestTransformerDeployerTransformer|TestTransformerHost|TestWeth|TestWethTransformerHost|TestZeroExFeature|TransformERC20Feature|Transformer|TransformerDeployer|UniswapFeature|WethTransformer|ZeroEx|ZeroExOptimized).json" }, "repository": { "type": "git", diff --git a/contracts/zero-ex/test/artifacts.ts b/contracts/zero-ex/test/artifacts.ts index c3571e729b..a703fecefa 100644 --- a/contracts/zero-ex/test/artifacts.ts +++ b/contracts/zero-ex/test/artifacts.ts @@ -127,6 +127,7 @@ import * as TestMintableERC20Token from '../test/generated-artifacts/TestMintabl import * as TestMintTokenERC20Transformer from '../test/generated-artifacts/TestMintTokenERC20Transformer.json'; import * as TestMooniswap from '../test/generated-artifacts/TestMooniswap.json'; import * as TestNativeOrdersFeature from '../test/generated-artifacts/TestNativeOrdersFeature.json'; +import * as TestOrderSignerRegistryWithContractWallet from '../test/generated-artifacts/TestOrderSignerRegistryWithContractWallet.json'; import * as TestPermissionlessTransformerDeployerSuicidal from '../test/generated-artifacts/TestPermissionlessTransformerDeployerSuicidal.json'; import * as TestPermissionlessTransformerDeployerTransformer from '../test/generated-artifacts/TestPermissionlessTransformerDeployerTransformer.json'; import * as TestRfqOriginRegistration from '../test/generated-artifacts/TestRfqOriginRegistration.json'; @@ -278,6 +279,7 @@ export const artifacts = { TestMintableERC20Token: TestMintableERC20Token as ContractArtifact, TestMooniswap: TestMooniswap as ContractArtifact, TestNativeOrdersFeature: TestNativeOrdersFeature as ContractArtifact, + TestOrderSignerRegistryWithContractWallet: TestOrderSignerRegistryWithContractWallet as ContractArtifact, TestPermissionlessTransformerDeployerSuicidal: TestPermissionlessTransformerDeployerSuicidal as ContractArtifact, TestPermissionlessTransformerDeployerTransformer: TestPermissionlessTransformerDeployerTransformer as ContractArtifact, TestRfqOriginRegistration: TestRfqOriginRegistration as ContractArtifact, diff --git a/contracts/zero-ex/test/features/native_orders_feature_test.ts b/contracts/zero-ex/test/features/native_orders_feature_test.ts index 393019950b..10cbfe4041 100644 --- a/contracts/zero-ex/test/features/native_orders_feature_test.ts +++ b/contracts/zero-ex/test/features/native_orders_feature_test.ts @@ -7,7 +7,15 @@ import { randomAddress, verifyEventsFromLogs, } from '@0x/contracts-test-utils'; -import { LimitOrder, LimitOrderFields, OrderStatus, RevertErrors, RfqOrder, RfqOrderFields } from '@0x/protocol-utils'; +import { + LimitOrder, + LimitOrderFields, + OrderStatus, + RevertErrors, + RfqOrder, + RfqOrderFields, + SignatureType, +} from '@0x/protocol-utils'; import { AnyRevertError, BigNumber } from '@0x/utils'; import { TransactionReceiptWithDecodedLogs } from 'ethereum-types'; @@ -25,7 +33,11 @@ import { getRandomRfqOrder, NativeOrdersTestEnvironment, } from '../utils/orders'; -import { TestMintableERC20TokenContract, TestRfqOriginRegistrationContract } from '../wrappers'; +import { + TestMintableERC20TokenContract, + TestOrderSignerRegistryWithContractWalletContract, + TestRfqOriginRegistrationContract, +} from '../wrappers'; blockchainTests.resets('NativeOrdersFeature', env => { const { NULL_ADDRESS, MAX_UINT256, NULL_BYTES32, ZERO_AMOUNT } = constants; @@ -36,17 +48,28 @@ blockchainTests.resets('NativeOrdersFeature', env => { let taker: string; let notMaker: string; let notTaker: string; + let contractWalletOwner: string; + let contractWalletSigner: string; let zeroEx: IZeroExContract; let verifyingContract: string; let makerToken: TestMintableERC20TokenContract; let takerToken: TestMintableERC20TokenContract; let wethToken: TestMintableERC20TokenContract; let testRfqOriginRegistration: TestRfqOriginRegistrationContract; + let contractWallet: TestOrderSignerRegistryWithContractWalletContract; let testUtils: NativeOrdersTestEnvironment; before(async () => { let owner; - [owner, maker, taker, notMaker, notTaker] = await env.getAccountAddressesAsync(); + [ + owner, + maker, + taker, + notMaker, + notTaker, + contractWalletOwner, + contractWalletSigner, + ] = await env.getAccountAddressesAsync(); [makerToken, takerToken, wethToken] = await Promise.all( [...new Array(3)].map(async () => TestMintableERC20TokenContract.deployFrom0xArtifactAsync( @@ -82,6 +105,21 @@ blockchainTests.resets('NativeOrdersFeature', env => { env.txDefaults, artifacts, ); + // contract wallet for signer delegation + contractWallet = await TestOrderSignerRegistryWithContractWalletContract.deployFrom0xArtifactAsync( + artifacts.TestOrderSignerRegistryWithContractWallet, + env.provider, + { + from: contractWalletOwner, + }, + artifacts, + zeroEx.address, + ); + + await contractWallet + .approveERC20(makerToken.address, zeroEx.address, MAX_UINT256) + .awaitTransactionSuccessAsync({ from: contractWalletOwner }); + testUtils = new NativeOrdersTestEnvironment( maker, taker, @@ -1569,4 +1607,432 @@ blockchainTests.resets('NativeOrdersFeature', env => { } }); }); + + describe('registerAllowedSigner()', () => { + it('fires appropriate events', async () => { + const receiptAllow = await contractWallet + .registerAllowedOrderSigner(contractWalletSigner, true) + .awaitTransactionSuccessAsync({ from: contractWalletOwner }); + + verifyEventsFromLogs( + receiptAllow.logs, + [ + { + maker: contractWallet.address, + signer: contractWalletSigner, + allowed: true, + }, + ], + IZeroExEvents.OrderSignerRegistered, + ); + + // then disallow signer + const receiptDisallow = await contractWallet + .registerAllowedOrderSigner(contractWalletSigner, false) + .awaitTransactionSuccessAsync({ from: contractWalletOwner }); + + verifyEventsFromLogs( + receiptDisallow.logs, + [ + { + maker: contractWallet.address, + signer: contractWalletSigner, + allowed: false, + }, + ], + IZeroExEvents.OrderSignerRegistered, + ); + }); + + it('allows for fills on orders signed by a approved signer', async () => { + const order = getTestRfqOrder({ maker: contractWallet.address }); + const sig = await order.getSignatureWithProviderAsync( + env.provider, + SignatureType.EthSign, + contractWalletSigner, + ); + + // covers taker + await testUtils.prepareBalancesForOrdersAsync([order]); + // need to provide contract wallet with a balance + await makerToken.mint(contractWallet.address, order.makerAmount).awaitTransactionSuccessAsync(); + + await contractWallet + .registerAllowedOrderSigner(contractWalletSigner, true) + .awaitTransactionSuccessAsync({ from: contractWalletOwner }); + + await zeroEx.fillRfqOrder(order, sig, order.takerAmount).awaitTransactionSuccessAsync({ from: taker }); + + const info = await zeroEx.getRfqOrderInfo(order).callAsync(); + assertOrderInfoEquals(info, { + status: OrderStatus.Filled, + orderHash: order.getHash(), + takerTokenFilledAmount: order.takerAmount, + }); + }); + + it('disallows fills if the signer is revoked', async () => { + const order = getTestRfqOrder({ maker: contractWallet.address }); + const sig = await order.getSignatureWithProviderAsync( + env.provider, + SignatureType.EthSign, + contractWalletSigner, + ); + + // covers taker + await testUtils.prepareBalancesForOrdersAsync([order]); + // need to provide contract wallet with a balance + await makerToken.mint(contractWallet.address, order.makerAmount).awaitTransactionSuccessAsync(); + + // first allow signer + await contractWallet + .registerAllowedOrderSigner(contractWalletSigner, true) + .awaitTransactionSuccessAsync({ from: contractWalletOwner }); + + // then disallow signer + await contractWallet + .registerAllowedOrderSigner(contractWalletSigner, false) + .awaitTransactionSuccessAsync({ from: contractWalletOwner }); + + const tx = zeroEx.fillRfqOrder(order, sig, order.takerAmount).awaitTransactionSuccessAsync({ from: taker }); + return expect(tx).to.revertWith( + new RevertErrors.NativeOrders.OrderNotSignedByMakerError( + order.getHash(), + contractWalletSigner, + order.maker, + ), + ); + }); + + it(`doesn't allow fills with an unapproved signer`, async () => { + const order = getTestRfqOrder({ maker: contractWallet.address }); + const sig = await order.getSignatureWithProviderAsync(env.provider, SignatureType.EthSign, maker); + + // covers taker + await testUtils.prepareBalancesForOrdersAsync([order]); + // need to provide contract wallet with a balance + await makerToken.mint(contractWallet.address, order.makerAmount).awaitTransactionSuccessAsync(); + + const tx = zeroEx.fillRfqOrder(order, sig, order.takerAmount).awaitTransactionSuccessAsync({ from: taker }); + return expect(tx).to.revertWith( + new RevertErrors.NativeOrders.OrderNotSignedByMakerError(order.getHash(), maker, order.maker), + ); + }); + + it(`allows an approved signer to cancel an RFQ order`, async () => { + const order = getTestRfqOrder({ maker: contractWallet.address }); + + await contractWallet + .registerAllowedOrderSigner(contractWalletSigner, true) + .awaitTransactionSuccessAsync({ from: contractWalletOwner }); + + const receipt = await zeroEx + .cancelRfqOrder(order) + .awaitTransactionSuccessAsync({ from: contractWalletSigner }); + + verifyEventsFromLogs( + receipt.logs, + [{ maker: contractWallet.address, orderHash: order.getHash() }], + IZeroExEvents.OrderCancelled, + ); + + const info = await zeroEx.getRfqOrderInfo(order).callAsync(); + assertOrderInfoEquals(info, { + status: OrderStatus.Cancelled, + orderHash: order.getHash(), + takerTokenFilledAmount: new BigNumber(0), + }); + }); + + it(`allows an approved signer to cancel a limit order`, async () => { + const order = getTestLimitOrder({ maker: contractWallet.address }); + + await contractWallet + .registerAllowedOrderSigner(contractWalletSigner, true) + .awaitTransactionSuccessAsync({ from: contractWalletOwner }); + + const receipt = await zeroEx + .cancelLimitOrder(order) + .awaitTransactionSuccessAsync({ from: contractWalletSigner }); + + verifyEventsFromLogs( + receipt.logs, + [{ maker: contractWallet.address, orderHash: order.getHash() }], + IZeroExEvents.OrderCancelled, + ); + + const info = await zeroEx.getLimitOrderInfo(order).callAsync(); + assertOrderInfoEquals(info, { + status: OrderStatus.Cancelled, + orderHash: order.getHash(), + takerTokenFilledAmount: new BigNumber(0), + }); + }); + + it(`doesn't allow an unapproved signer to cancel an RFQ order`, async () => { + const order = getTestRfqOrder({ maker: contractWallet.address }); + + const tx = zeroEx.cancelRfqOrder(order).awaitTransactionSuccessAsync({ from: maker }); + + return expect(tx).to.revertWith( + new RevertErrors.NativeOrders.OnlyOrderMakerAllowed(order.getHash(), maker, order.maker), + ); + }); + + it(`doesn't allow an unapproved signer to cancel a limit order`, async () => { + const order = getTestLimitOrder({ maker: contractWallet.address }); + + const tx = zeroEx.cancelLimitOrder(order).awaitTransactionSuccessAsync({ from: maker }); + + return expect(tx).to.revertWith( + new RevertErrors.NativeOrders.OnlyOrderMakerAllowed(order.getHash(), maker, order.maker), + ); + }); + + it(`allows a signer to cancel pair RFQ orders`, async () => { + const order = getTestRfqOrder({ maker: contractWallet.address, salt: new BigNumber(1) }); + + await contractWallet + .registerAllowedOrderSigner(contractWalletSigner, true) + .awaitTransactionSuccessAsync({ from: contractWalletOwner }); + + // Cancel salts <= the order's + const minValidSalt = order.salt.plus(1); + + const receipt = await zeroEx + .cancelPairRfqOrdersWithSigner( + contractWallet.address, + makerToken.address, + takerToken.address, + minValidSalt, + ) + .awaitTransactionSuccessAsync({ from: contractWalletSigner }); + verifyEventsFromLogs( + receipt.logs, + [ + { + maker: contractWallet.address, + makerToken: makerToken.address, + takerToken: takerToken.address, + minValidSalt, + }, + ], + IZeroExEvents.PairCancelledRfqOrders, + ); + + const info = await zeroEx.getRfqOrderInfo(order).callAsync(); + + assertOrderInfoEquals(info, { + status: OrderStatus.Cancelled, + orderHash: order.getHash(), + takerTokenFilledAmount: new BigNumber(0), + }); + }); + + it(`doesn't allow an unapproved signer to cancel pair RFQ orders`, async () => { + const minValidSalt = new BigNumber(2); + + const tx = zeroEx + .cancelPairRfqOrdersWithSigner( + contractWallet.address, + makerToken.address, + takerToken.address, + minValidSalt, + ) + .awaitTransactionSuccessAsync({ from: maker }); + + return expect(tx).to.revertWith( + new RevertErrors.NativeOrders.InvalidSignerError(contractWallet.address, maker), + ); + }); + + it(`allows a signer to cancel pair limit orders`, async () => { + const order = getTestLimitOrder({ maker: contractWallet.address, salt: new BigNumber(1) }); + + await contractWallet + .registerAllowedOrderSigner(contractWalletSigner, true) + .awaitTransactionSuccessAsync({ from: contractWalletOwner }); + + // Cancel salts <= the order's + const minValidSalt = order.salt.plus(1); + + const receipt = await zeroEx + .cancelPairLimitOrdersWithSigner( + contractWallet.address, + makerToken.address, + takerToken.address, + minValidSalt, + ) + .awaitTransactionSuccessAsync({ from: contractWalletSigner }); + verifyEventsFromLogs( + receipt.logs, + [ + { + maker: contractWallet.address, + makerToken: makerToken.address, + takerToken: takerToken.address, + minValidSalt, + }, + ], + IZeroExEvents.PairCancelledLimitOrders, + ); + + const info = await zeroEx.getLimitOrderInfo(order).callAsync(); + + assertOrderInfoEquals(info, { + status: OrderStatus.Cancelled, + orderHash: order.getHash(), + takerTokenFilledAmount: new BigNumber(0), + }); + }); + + it(`doesn't allow an unapproved signer to cancel pair limit orders`, async () => { + const minValidSalt = new BigNumber(2); + + const tx = zeroEx + .cancelPairLimitOrdersWithSigner( + contractWallet.address, + makerToken.address, + takerToken.address, + minValidSalt, + ) + .awaitTransactionSuccessAsync({ from: maker }); + + return expect(tx).to.revertWith( + new RevertErrors.NativeOrders.InvalidSignerError(contractWallet.address, maker), + ); + }); + + it(`allows a signer to cancel multiple RFQ order pairs`, async () => { + const orders = [ + getTestRfqOrder({ maker: contractWallet.address, salt: new BigNumber(1) }), + // Flip the tokens for the other order. + getTestRfqOrder({ + makerToken: takerToken.address, + takerToken: makerToken.address, + maker: contractWallet.address, + salt: new BigNumber(1), + }), + ]; + + await contractWallet + .registerAllowedOrderSigner(contractWalletSigner, true) + .awaitTransactionSuccessAsync({ from: contractWalletOwner }); + + const minValidSalt = new BigNumber(2); + const receipt = await zeroEx + .batchCancelPairRfqOrdersWithSigner( + contractWallet.address, + [makerToken.address, takerToken.address], + [takerToken.address, makerToken.address], + [minValidSalt, minValidSalt], + ) + .awaitTransactionSuccessAsync({ from: contractWalletSigner }); + verifyEventsFromLogs( + receipt.logs, + [ + { + maker: contractWallet.address, + makerToken: makerToken.address, + takerToken: takerToken.address, + minValidSalt, + }, + { + maker: contractWallet.address, + makerToken: takerToken.address, + takerToken: makerToken.address, + minValidSalt, + }, + ], + IZeroExEvents.PairCancelledRfqOrders, + ); + const statuses = (await Promise.all(orders.map(o => zeroEx.getRfqOrderInfo(o).callAsync()))).map( + oi => oi.status, + ); + expect(statuses).to.deep.eq([OrderStatus.Cancelled, OrderStatus.Cancelled]); + }); + + it(`doesn't allow an unapproved signer to batch cancel pair rfq orders`, async () => { + const minValidSalt = new BigNumber(2); + + const tx = zeroEx + .batchCancelPairRfqOrdersWithSigner( + contractWallet.address, + [makerToken.address, takerToken.address], + [takerToken.address, makerToken.address], + [minValidSalt, minValidSalt], + ) + .awaitTransactionSuccessAsync({ from: maker }); + + return expect(tx).to.revertWith( + new RevertErrors.NativeOrders.InvalidSignerError(contractWallet.address, maker), + ); + }); + + it(`allows a signer to cancel multiple limit order pairs`, async () => { + const orders = [ + getTestLimitOrder({ maker: contractWallet.address, salt: new BigNumber(1) }), + // Flip the tokens for the other order. + getTestLimitOrder({ + makerToken: takerToken.address, + takerToken: makerToken.address, + maker: contractWallet.address, + salt: new BigNumber(1), + }), + ]; + + await contractWallet + .registerAllowedOrderSigner(contractWalletSigner, true) + .awaitTransactionSuccessAsync({ from: contractWalletOwner }); + + const minValidSalt = new BigNumber(2); + const receipt = await zeroEx + .batchCancelPairLimitOrdersWithSigner( + contractWallet.address, + [makerToken.address, takerToken.address], + [takerToken.address, makerToken.address], + [minValidSalt, minValidSalt], + ) + .awaitTransactionSuccessAsync({ from: contractWalletSigner }); + verifyEventsFromLogs( + receipt.logs, + [ + { + maker: contractWallet.address, + makerToken: makerToken.address, + takerToken: takerToken.address, + minValidSalt, + }, + { + maker: contractWallet.address, + makerToken: takerToken.address, + takerToken: makerToken.address, + minValidSalt, + }, + ], + IZeroExEvents.PairCancelledLimitOrders, + ); + const statuses = (await Promise.all(orders.map(o => zeroEx.getLimitOrderInfo(o).callAsync()))).map( + oi => oi.status, + ); + expect(statuses).to.deep.eq([OrderStatus.Cancelled, OrderStatus.Cancelled]); + }); + + it(`doesn't allow an unapproved signer to batch cancel pair limit orders`, async () => { + const minValidSalt = new BigNumber(2); + + const tx = zeroEx + .batchCancelPairLimitOrdersWithSigner( + contractWallet.address, + [makerToken.address, takerToken.address], + [takerToken.address, makerToken.address], + [minValidSalt, minValidSalt], + ) + .awaitTransactionSuccessAsync({ from: maker }); + + return expect(tx).to.revertWith( + new RevertErrors.NativeOrders.InvalidSignerError(contractWallet.address, maker), + ); + }); + }); }); diff --git a/contracts/zero-ex/test/wrappers.ts b/contracts/zero-ex/test/wrappers.ts index a63b610343..8f24a81317 100644 --- a/contracts/zero-ex/test/wrappers.ts +++ b/contracts/zero-ex/test/wrappers.ts @@ -125,6 +125,7 @@ export * from '../test/generated-wrappers/test_mint_token_erc20_transformer'; export * from '../test/generated-wrappers/test_mintable_erc20_token'; export * from '../test/generated-wrappers/test_mooniswap'; export * from '../test/generated-wrappers/test_native_orders_feature'; +export * from '../test/generated-wrappers/test_order_signer_registry_with_contract_wallet'; export * from '../test/generated-wrappers/test_permissionless_transformer_deployer_suicidal'; export * from '../test/generated-wrappers/test_permissionless_transformer_deployer_transformer'; export * from '../test/generated-wrappers/test_rfq_origin_registration'; diff --git a/contracts/zero-ex/tsconfig.json b/contracts/zero-ex/tsconfig.json index 3671fbdca4..6f03092d4a 100644 --- a/contracts/zero-ex/tsconfig.json +++ b/contracts/zero-ex/tsconfig.json @@ -156,6 +156,7 @@ "test/generated-artifacts/TestMintableERC20Token.json", "test/generated-artifacts/TestMooniswap.json", "test/generated-artifacts/TestNativeOrdersFeature.json", + "test/generated-artifacts/TestOrderSignerRegistryWithContractWallet.json", "test/generated-artifacts/TestPermissionlessTransformerDeployerSuicidal.json", "test/generated-artifacts/TestPermissionlessTransformerDeployerTransformer.json", "test/generated-artifacts/TestRfqOriginRegistration.json", diff --git a/docs/basics/events.rst b/docs/basics/events.rst index 1852857805..15e017dbae 100644 --- a/docs/basics/events.rst +++ b/docs/basics/events.rst @@ -53,6 +53,8 @@ illustrates how events are emitted when trading through the Exchange Proxy. +-------------------------------+-----------------------------------------------------------------------------------------------------------------------------------------------------------+---------------------+ | `RfqOrderOriginsAllowed`_ | Emitted when a tx.origin is added/removed for RFQ, via `registerAllowedRfqOrigins <./functions.html#registerallowedrfqorigins>`_ | ExchangeProxy | +-------------------------------+-----------------------------------------------------------------------------------------------------------------------------------------------------------+---------------------+ +| `OrderSignerRegistered`_ | Emitted when an order signer is added/removed for a maker, via `registerAllowedOrderSigner <./functions.html#registerallowedordersigner>`_ | ExchangeProxy | ++-------------------------------+-----------------------------------------------------------------------------------------------------------------------------------------------------------+---------------------+ | `TransformedERC20`_ | Emitted when an `ERC20 Transformation <../advanced/erc20_transformations.html>`_ completes. | ExchangeProxy | +-------------------------------+-----------------------------------------------------------------------------------------------------------------------------------------------------------+---------------------+ | `TransformerDeployerUpdated`_ | Emitted when the Transformer Deployer is upgraded. | ExchangeProxy | @@ -227,7 +229,7 @@ PairCancelledLimitOrders ); PairCancelledRfqOrders ------------------------- +---------------------- .. code-block:: solidity @@ -311,6 +313,21 @@ RfqOrderOriginsAllowed bool allowed ); +OrderSignerRegistered +------------------------- + +.. code-block:: solidity + + /// @dev Emitted when new order signers are registered + /// @param maker The maker address that is registering a designated signer. + /// @param signer The address that will sign on behalf of maker. + /// @param allowed Indicates whether the address should be allowed. + event OrderSignerRegistered( + address maker, + address signer, + bool allowed + ); + TransformedERC20 ---------------- diff --git a/docs/basics/functions.rst b/docs/basics/functions.rst index 52c61f6576..f5866100f1 100644 --- a/docs/basics/functions.rst +++ b/docs/basics/functions.rst @@ -4,54 +4,69 @@ Basic Functionality Below is a catalog of basic Exchange functionality. For more advanced usage, like meta-transactions and dex aggregation, see the Advanced section. -+---------------------------------+--------------------------------------------------------------------------+ -| **Limit Orders** | **Overview** | -+---------------------------------+--------------------------------------------------------------------------+ -| `fillLimitOrder`_ | Fills a Limit Order up to the amount requested. | -+---------------------------------+--------------------------------------------------------------------------+ -| `fillOrKillLimitOrder`_ | Fills exactly the amount requested or reverts. | -+---------------------------------+--------------------------------------------------------------------------+ -| `cancelLimitOrder`_ | Cancels an order so that it can no longer be filled. | -+---------------------------------+--------------------------------------------------------------------------+ -| `batchCancelLimitOrders`_ | A batch call to `cancelLimitOrder`. | -+---------------------------------+--------------------------------------------------------------------------+ -| `cancelPairLimitOrders`_ | Cancels Limit orders in a specific market pair. | -| | Ex: Cancel all Limit Orders selling WETH for USDC. | -+---------------------------------+--------------------------------------------------------------------------+ -| `batchCancelPairLimitOrders`_ | A batch call to `cancelPairLimitOrders`. | -+---------------------------------+--------------------------------------------------------------------------+ -| `getLimitOrderInfo`_ | Returns the state of a given order. | -+---------------------------------+--------------------------------------------------------------------------+ -| `getLimitOrderHash`_ | Returns the EIP-712 hash for an order. | -+---------------------------------+--------------------------------------------------------------------------+ -| **RFQ Orders** | **Overview** | -+---------------------------------+--------------------------------------------------------------------------+ -| `fillRfqOrder`_ | These are analogous to the above LimitOrder functions. | -+---------------------------------+ | -| `fillOrKillRfqOrder`_ | | -+---------------------------------+ | -| `cancelRfqOrder`_ | | -+---------------------------------+ | -| `batchCancelRfqOrders`_ | | -+---------------------------------+ | -| `cancelPairRfqOrders`_ | | -+---------------------------------+ | -| `batchCancelPairRfqOrders`_ | | -+---------------------------------+ | -| `getRfqOrderInfo`_ | | -+---------------------------------+ | -| `getRfqOrderHash`_ | | -+---------------------------------+--------------------------------------------------------------------------+ -| `registerAllowedRfqOrigins`_ | Register tx.origin addresses that are allowed to fill an RFQ order. | -+---------------------------------+--------------------------------------------------------------------------+ -| **Protocol Fees** | **Overview** | -+---------------------------------+--------------------------------------------------------------------------+ -| `getProtocolFeeMultiplier`_ | Takers of limit orders pay a protocol fee of `Multiplier * tx.gasprice`. | -| | This returns the `Multiplier`. | -+---------------------------------+--------------------------------------------------------------------------+ -| `transferProtocolFeesForPools`_ | Transfers protocol fees from escrow to the 0x Staking System. | -| | This should be called near the end of each epoch. | -+---------------------------------+--------------------------------------------------------------------------+ ++-----------------------------------------+--------------------------------------------------------------------------+ +| **Limit Orders** | **Overview** | ++-----------------------------------------+--------------------------------------------------------------------------+ +| `fillLimitOrder`_ | Fills a Limit Order up to the amount requested. | ++-----------------------------------------+--------------------------------------------------------------------------+ +| `fillOrKillLimitOrder`_ | Fills exactly the amount requested or reverts. | ++-----------------------------------------+--------------------------------------------------------------------------+ +| `cancelLimitOrder`_ | Cancels an order so that it can no longer be filled. | ++-----------------------------------------+--------------------------------------------------------------------------+ +| `batchCancelLimitOrders`_ | A batch call to `cancelLimitOrder`. | ++-----------------------------------------+--------------------------------------------------------------------------+ +| `cancelPairLimitOrders`_ | Cancels Limit orders in a specific market pair. | +| | Ex: Cancel all Limit Orders selling WETH for USDC. | ++-----------------------------------------+--------------------------------------------------------------------------+ +| `cancelPairLimitOrdersWithSigner`_ | Same functionality to ``cancelPairLimitOrders`` but called by a | +| | registered order signer instead of the maker itself. | ++-----------------------------------------+--------------------------------------------------------------------------+ +| `batchCancelPairLimitOrders`_ | A batch call to `cancelPairLimitOrders`. | ++-----------------------------------------+--------------------------------------------------------------------------+ +| `batchCancelPairLimitOrdersWithSigner`_ | Same functionality to ``cancelPairLimitOrders`` but called by a | +| | registered order signer instead of the maker itself. | ++-----------------------------------------+--------------------------------------------------------------------------+ +| `getLimitOrderInfo`_ | Returns the state of a given order. | ++-----------------------------------------+--------------------------------------------------------------------------+ +| `getLimitOrderHash`_ | Returns the EIP-712 hash for an order. | ++-----------------------------------------+--------------------------------------------------------------------------+ +| **RFQ Orders** | **Overview** | ++-----------------------------------------+--------------------------------------------------------------------------+ +| `fillRfqOrder`_ | These are analogous to the above LimitOrder functions. | ++-----------------------------------------+ | +| `fillOrKillRfqOrder`_ | | ++-----------------------------------------+ | +| `cancelRfqOrder`_ | | ++-----------------------------------------+ | +| `batchCancelRfqOrders`_ | | ++-----------------------------------------+ | +| `cancelPairRfqOrders`_ | | ++-----------------------------------------+ | +| `cancelPairRfqOrdersWithSigner`_ | | ++-----------------------------------------+ | +| `batchCancelPairRfqOrders`_ | | ++-----------------------------------------+ | +| `batchCancelPairRfqOrdersWithSigner`_ | | ++-----------------------------------------+ | +| `getRfqOrderInfo`_ | | ++-----------------------------------------+ | +| `getRfqOrderHash`_ | | ++-----------------------------------------+--------------------------------------------------------------------------+ +| `registerAllowedRfqOrigins`_ | Register tx.origin addresses that are allowed to fill an RFQ order. | ++-----------------------------------------+--------------------------------------------------------------------------+ +| `registerAllowedOrderSigner`_ | Register addresses that can sign orders on behalf of ``msg.sender``. | ++-----------------------------------------+--------------------------------------------------------------------------+ +| `isValidOrderSigner`_ | Returns whether a given address is allowed to sign orders for a given | +| | maker address. | ++-----------------------------------------+--------------------------------------------------------------------------+ +| **Protocol Fees** | **Overview** | ++-----------------------------------------+--------------------------------------------------------------------------+ +| `getProtocolFeeMultiplier`_ | Takers of limit orders pay a protocol fee of `Multiplier * tx.gasprice`. | +| | This returns the `Multiplier`. | ++-----------------------------------------+--------------------------------------------------------------------------+ +| `transferProtocolFeesForPools`_ | Transfers protocol fees from escrow to the 0x Staking System. | +| | This should be called near the end of each epoch. | ++-----------------------------------------+--------------------------------------------------------------------------+ Limit Orders @@ -131,7 +146,7 @@ This function cancels a single limit order created by the caller: ) external; -This function emits an `OrderCancelled <../basics/events.html#ordercancelled>`_ event if the cancellation is successful. The call will revert if ``msg.sender != order.maker``. +This function emits an `OrderCancelled <../basics/events.html#ordercancelled>`_ event if the cancellation is successful. The call will revert if ``msg.sender != order.maker`` or ``!isValidOrderSigner(maker, msg.sender)``. batchCancelLimitOrders ---------------------- @@ -146,7 +161,7 @@ This function cancels multiple limit orders created by the caller: ) external; -This function emits an `OrderCancelled <../basics/events.html#ordercancelled>`_ event for each order it cancels. The call will revert if ``msg.sender != order.maker`` for any of the orders. +This function emits an `OrderCancelled <../basics/events.html#ordercancelled>`_ event for each order it cancels. The call will revert if ``msg.sender != order.maker`` or ``!isValidOrderSigner(maker, msg.sender)`` for any of the orders. cancelPairLimitOrders --------------------- @@ -162,10 +177,32 @@ This function cancels all limit orders created by the caller with with a maker a ) external; -This function emits a `PairCancelledLimitOrders <../basics/events.html#paircancelledlimitorders>`_ event, or reverts in one of the following scenarios: +This function emits a `PairCancelledLimitOrders <../basics/events.html#paircancelledlimitorders>`_ event, or reverts if the ``salt`` parameter is ≤ to a previous ``salt``. -- ``msg.sender != order.maker`` -- The ``salt`` parameter is ≤ to a previous ``salt``. +cancelPairLimitOrdersWithSigner +------------------------------- + +Same functionality as ``cancelPairLimitOrders`` but ``msg.sender`` is a registered order signer instead of the maker itself. + +.. code-block:: solidity + + /// @dev Cancel all limit orders for a given maker and pair with a salt less + /// than the value provided. The caller must be a signer registered to the maker. + /// Subsequent calls to this function with the same maker and pair require the + /// new salt to be >= the old salt. + /// @param maker The maker for which to cancel. + /// @param makerToken The maker token. + /// @param takerToken The taker token. + /// @param minValidSalt The new minimum valid salt. + function cancelPairLimitOrdersWithSigner( + address maker, + IERC20TokenV06 makerToken, + IERC20TokenV06 takerToken, + uint256 minValidSalt + ) + external; + +Reverts if ``!isValidOrderSigner(maker, msg.sender)``. batchCancelPairLimitOrders -------------------------- @@ -183,6 +220,31 @@ This function performs multiple ``cancelPairLimitOrders()`` at once. Each respec This function emits a `PairCancelledLimitOrders <../basics/events.html#paircancelledlimitorders>`_ event for each market pair it cancels. It reverts if any of the individual cancellations revert. +batchCancelPairLimitOrdersWithSigner +------------------------------------ + +Same functionality as ``batchCancelPairLimitOrders`` but ``msg.sender`` is a registered order signer instead of the maker itself. + +.. code-block:: solidity + + /// @dev Cancel all limit orders for a given maker and pairs with salts less + /// than the values provided. The caller must be a signer registered to the maker. + /// Subsequent calls to this function with the same maker and pair require the + /// new salt to be >= the old salt. + /// @param maker The maker for which to cancel. + /// @param makerTokens The maker tokens. + /// @param takerTokens The taker tokens. + /// @param minValidSalts The new minimum valid salts. + function batchCancelPairLimitOrdersWithSigner( + address maker, + IERC20TokenV06[] memory makerTokens, + IERC20TokenV06[] memory takerTokens, + uint256[] memory minValidSalts + ) + external; + +Reverts if ``!isValidOrderSigner(maker, msg.sender)``. + getLimitOrderInfo ----------------- @@ -369,7 +431,7 @@ Similar to limit orders, RFQ orders can be cancelled on-chain through a variety ) external; -This function emits an `OrderCancelled <../basics/events.html#ordercancelled>`_ event if the cancellation is successful. The call will revert if ``msg.sender != order.maker``. +This function emits an `OrderCancelled <../basics/events.html#ordercancelled>`_ event if the cancellation is successful. The call will revert if ``msg.sender != order.maker`` or ``!isValidOrderSigner(maker, msg.sender)``. batchCancelRfqOrders -------------------- @@ -384,7 +446,7 @@ This function cancels multiple RFQ orders created by the caller: ) external; -This function emits an `OrderCancelled <../basics/events.html#ordercancelled>`_ event for each order it cancels. The call will revert if ``msg.sender != order.maker`` for any of the orders. +This function emits an `OrderCancelled <../basics/events.html#ordercancelled>`_ event for each order it cancels. The call will revert if ``msg.sender != order.maker`` or ``!isValidOrderSigner(maker, msg.sender)`` for any orders for any of the orders. cancelPairRfqOrders ------------------- @@ -400,10 +462,32 @@ This function cancels all RFQ orders created by the caller with with a maker and ) external; -This function emits a `PairCancelledRfqOrders <../basics/events.html#paircancelledrfqorders>`_ event, or reverts in one of the following scenarios: +This function emits a `PairCancelledRfqOrders <../basics/events.html#paircancelledrfqorders>`_ event, or reverts if the ``salt`` parameter is ≤ to a previous ``salt``. -- ``msg.sender != order.maker`` -- The ``salt`` parameter is ≤ to a previous ``salt``. +cancelPairRfqOrdersWithSigner +----------------------------- + +Same functionality as ``cancelPairRfqOrders`` but ``msg.sender`` is a registered order signer instead of the maker itself. + +.. code-block:: solidity + + /// @dev Cancel all RFQ orders for a given maker and pair with a salt less + /// than the value provided. The caller must be a signer registered to the maker. + /// Subsequent calls to this function with the same maker and pair require the + /// new salt to be >= the old salt. + /// @param maker The maker for which to cancel. + /// @param makerToken The maker token. + /// @param takerToken The taker token. + /// @param minValidSalt The new minimum valid salt. + function cancelPairRfqOrdersWithSigner( + address maker, + IERC20TokenV06 makerToken, + IERC20TokenV06 takerToken, + uint256 minValidSalt + ) + external; + +Reverts if ``!isValidOrderSigner(maker, msg.sender)``. batchCancelPairRfqOrders ------------------------ @@ -421,6 +505,31 @@ batchCancelPairRfqOrders This function emits a `PairCancelledRfqOrders <../basics/events.html#paircancelledrfqorders>`_ event for each market pair it cancels. It reverts if any of the individual cancellations revert. +batchCancelPairRfqOrdersWithSigner +---------------------------------- + +Same functionality as ``batchCancelPairRfqOrders`` but ``msg.sender`` is a registered order signer instead of the maker itself. + +.. code-block:: solidity + + /// @dev Cancel all RFQ orders for a given maker and pairs with salts less + /// than the values provided. The caller must be a signer registered to the maker. + /// Subsequent calls to this function with the same maker and pair require the + /// new salt to be >= the old salt. + /// @param maker The maker for which to cancel. + /// @param makerTokens The maker tokens. + /// @param takerTokens The taker tokens. + /// @param minValidSalts The new minimum valid salts. + function batchCancelPairRfqOrdersWithSigner( + address maker, + IERC20TokenV06[] memory makerTokens, + IERC20TokenV06[] memory takerTokens, + uint256[] memory minValidSalts + ) + external; + +Reverts if ``!isValidOrderSigner(maker, msg.sender)``. + getRfqOrderInfo --------------- @@ -549,6 +658,47 @@ Looking at the 2nd use case, a maker can register valid tx origins using this fu This function emits a `RfqOrderOriginsAllowed <../basics/events.html#rfqorderoriginsallowed>`_ event. +registerAllowedOrderSigner +-------------------------- + +Calls to fill functions require a signature provided by the maker. In cases where the signer can't be the maker itself (e.g. a contract wallet), the maker can delegate signing to another address. + +To register a new delegated order signer, the maker can call ``registerAllowedOrderSigner`` with ``allowed == true``. + +To revoke permission to a signer, the maker can call ``registerAllowedOrderSigner`` with ``allowed == false``. + +.. code-block:: solidity + + /// @dev Register a signer who can sign on behalf of msg.sender + /// This allows one to sign on behalf of a contract that calls this function + /// @param signer The address from which you plan to generate signatures + /// @param allowed True to register, false to unregister. + function registerAllowedOrderSigner( + address signer, + bool allowed + ) + external; + +This function emits an `OrderSignerRegistered <../basics/events.html#ordersignerregistered>`_ event. + +isValidOrderSigner +------------------ + +Returns whether the ``signer`` is allowed to sign orders on behalf of the ``maker``. + +.. code-block:: solidity + + /// @dev checks if a given address is registered to sign on behalf of a maker address + /// @param maker The maker address encoded in an order (can be a contract) + /// @param signer The address that is providing a signature + function isValidOrderSigner( + address maker, + address signer + ) + external + view + returns (bool isAllowed); + Protocol Fees ============= diff --git a/docs/basics/orders.rst b/docs/basics/orders.rst index bbd6807e4e..eab521b384 100644 --- a/docs/basics/orders.rst +++ b/docs/basics/orders.rst @@ -92,7 +92,7 @@ The ``RFQOrder`` struct has the following fields: How To Sign ============== -Both Limit & RFQ orders must be signed by the `maker`. This signature is needed to fill an order, see `Basic Functionality <./functions.html>`_. +Both Limit & RFQ orders must be signed by the `maker` or a registered order signer (`registerAllowedOrderSigner <./functions.html#registerallowedrfqorigins>`_). This signature is needed to fill an order, see `Basic Functionality <./functions.html>`_. The protocol accepts signatures defined by the following struct: diff --git a/packages/protocol-utils/CHANGELOG.json b/packages/protocol-utils/CHANGELOG.json index 6951ef42b5..2a5be75d2c 100644 --- a/packages/protocol-utils/CHANGELOG.json +++ b/packages/protocol-utils/CHANGELOG.json @@ -1,4 +1,13 @@ [ + { + "version": "1.5.0", + "changes": [ + { + "note": "Add support for orderSignerRegistry in getSignatureWithProviderAsync()", + "pr": 195 + } + ] + }, { "timestamp": 1618259868, "version": "1.4.1", diff --git a/packages/protocol-utils/src/orders.ts b/packages/protocol-utils/src/orders.ts index 040a40279b..171402230e 100644 --- a/packages/protocol-utils/src/orders.ts +++ b/packages/protocol-utils/src/orders.ts @@ -114,12 +114,13 @@ export abstract class OrderBase { public async getSignatureWithProviderAsync( provider: SupportedProvider, type: SignatureType = SignatureType.EthSign, + signer: string = this.maker, ): Promise { switch (type) { case SignatureType.EIP712: - return eip712SignTypedDataWithProviderAsync(this.getEIP712TypedData(), this.maker, provider); + return eip712SignTypedDataWithProviderAsync(this.getEIP712TypedData(), signer, provider); case SignatureType.EthSign: - return ethSignHashWithProviderAsync(this.getHash(), this.maker, provider); + return ethSignHashWithProviderAsync(this.getHash(), signer, provider); default: throw new Error(`Cannot sign with signature type: ${type}`); } diff --git a/packages/protocol-utils/src/revert-errors/native_orders.ts b/packages/protocol-utils/src/revert-errors/native_orders.ts index 56d8d41ff4..c7c8de1485 100644 --- a/packages/protocol-utils/src/revert-errors/native_orders.ts +++ b/packages/protocol-utils/src/revert-errors/native_orders.ts @@ -49,6 +49,15 @@ export class OrderNotSignedByMakerError extends RevertError { } } +export class InvalidSignerError extends RevertError { + constructor(maker?: string, signer?: string) { + super('InvalidSignerError', 'InvalidSignerError(address maker, address signer)', { + maker, + signer, + }); + } +} + export class OrderNotFillableBySenderError extends RevertError { constructor(orderHash?: string, sender?: string, orderSender?: string) { super( @@ -135,6 +144,7 @@ const types = [ FillOrKillFailedError, OnlyOrderMakerAllowed, BatchFillIncompleteError, + InvalidSignerError, ]; // Register the types we've defined.