From a5b39d784c4439ce5963d9330acd9b34c47616ee Mon Sep 17 00:00:00 2001 From: Jem <0x0xjem@gmail.com> Date: Wed, 10 Jan 2024 16:44:42 +0400 Subject: [PATCH] Validation for hooks and allowlist --- src/bases/Auctioneer.sol | 24 +++++++++++--- test/AuctionHouse/auction.t.sol | 44 ++++++++++++++++++++++++-- test/modules/Auction/MockAllowlist.sol | 20 +++++++++++- 3 files changed, 80 insertions(+), 8 deletions(-) diff --git a/src/bases/Auctioneer.sol b/src/bases/Auctioneer.sol index f02b1b69..16bf873b 100644 --- a/src/bases/Auctioneer.sol +++ b/src/bases/Auctioneer.sol @@ -93,8 +93,9 @@ abstract contract Auctioneer is WithModules { /// - Validation for the auction parameters fails /// - The module for the optional specified derivative type is not installed /// - Validation for the optional specified derivative type fails - /// - Validation for the optional allowlist fails - /// - The module for the optional specified condenser type is not installed + /// - Registration for the optional allowlist fails + /// - The optional specified hooks contract is not a contract + /// - The condenser module is not installed or is sunset /// /// @param routing_ Routing information for the auction lot /// @param params_ Auction parameters for the auction lot @@ -150,7 +151,6 @@ abstract contract Auctioneer is WithModules { routing.owner = msg.sender; routing.baseToken = routing_.baseToken; routing.quoteToken = routing_.quoteToken; - routing.hooks = routing_.hooks; // Derivative if (fromKeycode(routing_.derivativeType) != bytes5("")) { @@ -197,13 +197,27 @@ abstract contract Auctioneer is WithModules { // If allowlist is being used, validate the allowlist data and register the auction on the allowlist if (address(routing_.allowlist) != address(0)) { - // TODO validation - // TODO registration with allowlist + // Check that it is a contract + // It is assumed that the user will do validation of the allowlist + if (address(routing_.allowlist).code.length == 0) revert InvalidParams(); + + // Register with the allowlist + routing_.allowlist.register(lotId, routing_.allowlistParams); // Store allowlist information routing.allowlist = routing_.allowlist; } + // If hooks are being used, validate the hooks data + if (address(routing_.hooks) != address(0)) { + // Check that it is a contract + // It is assumed that the user will do validation of the hooks + if (address(routing_.hooks).code.length == 0) revert InvalidParams(); + + // Store hooks information + routing.hooks = routing_.hooks; + } + emit AuctionCreated(lotId, address(routing.baseToken), address(routing.quoteToken)); } diff --git a/test/AuctionHouse/auction.t.sol b/test/AuctionHouse/auction.t.sol index f0f85664..44519bbc 100644 --- a/test/AuctionHouse/auction.t.sol +++ b/test/AuctionHouse/auction.t.sol @@ -401,8 +401,9 @@ contract AuctionTest is Test { // Won't revert } - // [ ] allowlist - // [ ] reverts when allowlist validation fails + // [X] allowlist + // [X] reverts when the allowlist address is not a contract + // [X] reverts when allowlist validation fails // [X] sets the allowlist on the auction lot function test_success_allowlistIsSet() external whenAuctionModuleIsInstalled { @@ -416,11 +417,50 @@ contract AuctionTest is Test { (,,,,, IAllowlist lotAllowlist,,,) = auctionHouse.lotRouting(lotId); assertEq(address(lotAllowlist), address(mockAllowlist), "allowlist mismatch"); + + // Check that it has been registered with the allowlist + uint256[] memory registeredIds = mockAllowlist.getRegisteredIds(); + assertEq(registeredIds.length, 1, "registered ids length mismatch"); + assertEq(registeredIds[0], lotId, "registered id mismatch"); + } + + function testReverts_whenAllowlistIsNotContract() external whenAuctionModuleIsInstalled { + // Update routing params + routingParams.allowlist = IAllowlist(address(0x10)); + + // Expect revert + bytes memory err = abi.encodeWithSelector(Auctioneer.InvalidParams.selector); + vm.expectRevert(err); + + auctionHouse.auction(routingParams, auctionParams); + } + + function testReverts_whenAllowlistValidationFails() external whenAuctionModuleIsInstalled { + // Update routing params + routingParams.allowlist = mockAllowlist; + + // Expect revert + mockAllowlist.setRegisterReverts(true); + vm.expectRevert("MockAllowlist: register reverted"); + + auctionHouse.auction(routingParams, auctionParams); } // [X] hooks + // [X] reverts when the hooks address is not a contract // [X] sets the hooks on the auction lot + function testReverts_whenHooksIsNotContract() external whenAuctionModuleIsInstalled { + // Update routing params + routingParams.hooks = IHooks(address(0x10)); + + // Expect revert + bytes memory err = abi.encodeWithSelector(Auctioneer.InvalidParams.selector); + vm.expectRevert(err); + + auctionHouse.auction(routingParams, auctionParams); + } + function test_success_hooksIsSet() external whenAuctionModuleIsInstalled { // Update routing params routingParams.hooks = mockHook; diff --git a/test/modules/Auction/MockAllowlist.sol b/test/modules/Auction/MockAllowlist.sol index 85143b92..356db4db 100644 --- a/test/modules/Auction/MockAllowlist.sol +++ b/test/modules/Auction/MockAllowlist.sol @@ -4,6 +4,10 @@ pragma solidity 0.8.19; import {IAllowlist} from "src/bases/Auctioneer.sol"; contract MockAllowlist is IAllowlist { + bool registerReverts = false; + + uint256[] public registeredIds; + function isAllowed( address user_, bytes calldata proof_ @@ -17,5 +21,19 @@ contract MockAllowlist is IAllowlist { function register(bytes calldata params_) external override {} - function register(uint256 id_, bytes calldata params_) external override {} + function register(uint256 id_, bytes calldata params_) external override { + if (registerReverts) { + revert("MockAllowlist: register reverted"); + } + + registeredIds.push(id_); + } + + function setRegisterReverts(bool registerReverts_) external { + registerReverts = registerReverts_; + } + + function getRegisteredIds() external view returns (uint256[] memory) { + return registeredIds; + } }