From 4cb3286c9425a72e58c44c29d17b31261b1dd94e Mon Sep 17 00:00:00 2001 From: TAMARA LIPOWSKI Date: Wed, 22 Jan 2025 12:21:13 -0500 Subject: [PATCH 1/3] feat: Set swap executors and verifiers - Moved the deployment method into a test template for organization - Created skeletons of dispatcher contracts - Added all possible test cases for thoroughness --- .../src/CallbackVerificationDispatcher.sol | 17 +++ foundry/src/SwapExecutionDispatcher.sol | 17 +++ foundry/src/TychoRouter.sol | 62 +++++++- foundry/test/Constants.sol | 10 ++ foundry/test/TestTemplate.sol | 33 +++++ foundry/test/TychoRouter.t.sol | 133 +++++++++++++++++- 6 files changed, 262 insertions(+), 10 deletions(-) create mode 100644 foundry/src/CallbackVerificationDispatcher.sol create mode 100644 foundry/src/SwapExecutionDispatcher.sol create mode 100644 foundry/test/Constants.sol create mode 100644 foundry/test/TestTemplate.sol diff --git a/foundry/src/CallbackVerificationDispatcher.sol b/foundry/src/CallbackVerificationDispatcher.sol new file mode 100644 index 0000000..644a20c --- /dev/null +++ b/foundry/src/CallbackVerificationDispatcher.sol @@ -0,0 +1,17 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.0; + +/** + * @title Dispatch callback verification to external contracts + * @author PropellerHeads Devs + * @dev Provides the ability to delegate callback verification to external + * contracts. This allows dynamically adding new supported protocols + * without needing to upgrade any contracts. External contracts will + * be called using delegatecall so they can share state with the main + * contract if needed. + * + * Note Verifier contracts need to implement the ICallbackVerifier interface + */ +contract CallbackVerificationDispatcher { + mapping(address => bool) public callbackVerifiers; +} diff --git a/foundry/src/SwapExecutionDispatcher.sol b/foundry/src/SwapExecutionDispatcher.sol new file mode 100644 index 0000000..3173104 --- /dev/null +++ b/foundry/src/SwapExecutionDispatcher.sol @@ -0,0 +1,17 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.28; + +/** + * @title SwapExecutionDispatcher - Dispatch swap execution to external contracts + * @author PropellerHeads Devs + * @dev Provides the ability to delegate execution of swaps to external + * contracts. This allows dynamically adding new supported protocols + * without needing to upgrade any contracts. External contracts will + * be called using delegatecall so they can share state with the main + * contract if needed. + * + * Note Executor contracts need to implement the ISwapExecutor interface + */ +contract SwapExecutionDispatcher { + mapping(address => bool) public swapExecutors; +} diff --git a/foundry/src/TychoRouter.sol b/foundry/src/TychoRouter.sol index 59c0c7a..62a2b0c 100644 --- a/foundry/src/TychoRouter.sol +++ b/foundry/src/TychoRouter.sol @@ -2,14 +2,22 @@ pragma solidity ^0.8.28; import "@openzeppelin/contracts/access/AccessControl.sol"; -import "@permit2/src/interfaces/IAllowanceTransfer.sol"; import "@openzeppelin/contracts/token/ERC20/IERC20.sol"; import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; +import "@permit2/src/interfaces/IAllowanceTransfer.sol"; +import "./SwapExecutionDispatcher.sol"; +import "./CallbackVerificationDispatcher.sol"; error TychoRouter__WithdrawalFailed(); error TychoRouter__InvalidReceiver(); +error TychoRouter__NonContractExecutor(); +error TychoRouter__NonContractVerifier(); -contract TychoRouter is AccessControl { +contract TychoRouter is + AccessControl, + SwapExecutionDispatcher, + CallbackVerificationDispatcher +{ IAllowanceTransfer public immutable permit2; using SafeERC20 for IERC20; @@ -68,7 +76,55 @@ contract TychoRouter is AccessControl { external onlyRole(DEFAULT_ADMIN_ROLE) { - // TODO + for (uint256 i = 0; i < accounts.length; i++) { + _grantRole(role, accounts[i]); + } + } + + /** + * @dev Entrypoint to add or replace an approved swap executor contract address + * @param target address of the swap method contract + */ + function setSwapExecutor(address target) + external + onlyRole(EXECUTOR_SETTER_ROLE) + { + if (target.code.length == 0) revert TychoRouter__NonContractExecutor(); + swapExecutors[target] = true; + } + + /** + * @dev Entrypoint to remove an approved swap executor contract address + * @param target address of the swap method contract + */ + function removeSwapExecutor(address target) + external + onlyRole(EXECUTOR_SETTER_ROLE) + { + delete swapExecutors[target]; + } + + /** + * @dev Entrypoint to add or replace an approved swap executor contract address + * @param target address of the swap method contract + */ + function setCallbackVerifier(address target) + external + onlyRole(EXECUTOR_SETTER_ROLE) + { + if (target.code.length == 0) revert TychoRouter__NonContractVerifier(); + callbackVerifiers[target] = true; + } + + /** + * @dev Entrypoint to remove an approved swap executor contract address + * @param target address of the swap method contract + */ + function removeCallbackVerifier(address target) + external + onlyRole(EXECUTOR_SETTER_ROLE) + { + delete callbackVerifiers[target]; } /** diff --git a/foundry/test/Constants.sol b/foundry/test/Constants.sol new file mode 100644 index 0000000..43e09af --- /dev/null +++ b/foundry/test/Constants.sol @@ -0,0 +1,10 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.13; + +contract Constants { + address ADMIN = address(12395012351212343412541234); //admin=us + address BOB = address(123); //bob=someone!=us + + // dummy contracts + address DUMMY = address(0x1234); +} diff --git a/foundry/test/TestTemplate.sol b/foundry/test/TestTemplate.sol new file mode 100644 index 0000000..7927ec4 --- /dev/null +++ b/foundry/test/TestTemplate.sol @@ -0,0 +1,33 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.13; + +import "forge-std/Test.sol"; +import "@src/TychoRouter.sol"; +import "./Constants.sol"; + +contract TychoRouterTestTemplate is Test, Constants { + TychoRouter tychoRouter; + address tychoRouterAddress; + address executorSetter; + + function deployTychoRouter() internal { + vm.startPrank(ADMIN); + + address permit2Address = + address(0x000000000022D473030F116dDEE9F6B43aC78BA3); + tychoRouter = new TychoRouter(permit2Address); + tychoRouterAddress = address(tychoRouter); + tychoRouter.grantRole(keccak256("EXECUTOR_SETTER_ROLE"), BOB); + executorSetter = BOB; + + vm.stopPrank(); + } + + /** + * @dev Deploys a dummy contract with non-empty bytecode + */ + function deployDummyContract() internal { + bytes memory minimalBytecode = hex"01"; // Single-byte bytecode + vm.etch(DUMMY, minimalBytecode); // Deploy minimal bytecode + } +} diff --git a/foundry/test/TychoRouter.t.sol b/foundry/test/TychoRouter.t.sol index 3f28459..79bb26e 100644 --- a/foundry/test/TychoRouter.t.sol +++ b/foundry/test/TychoRouter.t.sol @@ -1,19 +1,138 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity ^0.8.28; -import {Test, console} from "forge-std/Test.sol"; import {TychoRouter} from "@src/TychoRouter.sol"; +import "./TestTemplate.sol"; -contract TychoRouterTest is Test { - TychoRouter public tychoRouter; +contract TychoRouterTest is TychoRouterTestTemplate { + bytes32 public constant EXECUTOR_SETTER_ROLE = + 0x6a1dd52dcad5bd732e45b6af4e7344fa284e2d7d4b23b5b09cb55d36b0685c87; + bytes32 public constant FEE_SETTER_ROLE = + 0xe6ad9a47fbda1dc18de1eb5eeb7d935e5e81b4748f3cfc61e233e64f88182060; + bytes32 public constant PAUSER_ROLE = + 0x65d7a28e3265b37a6474929f336521b332c1681b933f6cb9f3376673440d862a; + bytes32 public constant FUND_RESCUER_ROLE = + 0x912e45d663a6f4cc1d0491d8f046e06c616f40352565ea1cdb86a0e1aaefa41b; function setupTychoRouter() public { - address permit2Address = - address(0x000000000022D473030F116dDEE9F6B43aC78BA3); - tychoRouter = new TychoRouter(permit2Address); + deployTychoRouter(); } - function testSetupTychoRouter() public { + function testSetValidExecutor() public { setupTychoRouter(); + deployDummyContract(); + + vm.startPrank(executorSetter); + tychoRouter.setSwapExecutor(DUMMY); + vm.stopPrank(); + + assert(tychoRouter.swapExecutors(DUMMY) == true); + } + + function testRemoveExecutor() public { + setupTychoRouter(); + deployDummyContract(); + + vm.startPrank(executorSetter); + tychoRouter.setSwapExecutor(DUMMY); + tychoRouter.removeSwapExecutor(DUMMY); + vm.stopPrank(); + assert(tychoRouter.swapExecutors(DUMMY) == false); + } + + function testRemoveUnSetExecutor() public { + setupTychoRouter(); + deployDummyContract(); + + vm.startPrank(executorSetter); + tychoRouter.removeSwapExecutor(BOB); + vm.stopPrank(); + assert(tychoRouter.swapExecutors(BOB) == false); + } + + function testRemoveExecutorMissingSetterRole() public { + setupTychoRouter(); + deployDummyContract(); + vm.expectRevert(); + tychoRouter.removeSwapExecutor(BOB); + } + + function testSetExecutorMissingSetterRole() public { + setupTychoRouter(); + deployDummyContract(); + + vm.expectRevert(); + tychoRouter.setSwapExecutor(DUMMY); + } + + function testSetExecutorNonContract() public { + setupTychoRouter(); + deployDummyContract(); + + vm.startPrank(executorSetter); + vm.expectRevert( + abi.encodeWithSelector(TychoRouter__NonContractExecutor.selector) + ); + tychoRouter.setSwapExecutor(BOB); + vm.stopPrank(); + } + + function testSetValidVerifier() public { + setupTychoRouter(); + deployDummyContract(); + + vm.startPrank(executorSetter); + tychoRouter.setCallbackVerifier(DUMMY); + vm.stopPrank(); + + assert(tychoRouter.callbackVerifiers(DUMMY) == true); + } + + function testRemoveVerifier() public { + setupTychoRouter(); + deployDummyContract(); + + vm.startPrank(executorSetter); + tychoRouter.setCallbackVerifier(DUMMY); + tychoRouter.removeCallbackVerifier(DUMMY); + vm.stopPrank(); + assert(tychoRouter.callbackVerifiers(DUMMY) == false); + } + + function testRemoveUnSetVerifier() public { + setupTychoRouter(); + deployDummyContract(); + + vm.startPrank(executorSetter); + tychoRouter.removeCallbackVerifier(BOB); + vm.stopPrank(); + assert(tychoRouter.callbackVerifiers(BOB) == false); + } + + function testRemoveVerifierMissingSetterRole() public { + setupTychoRouter(); + deployDummyContract(); + vm.expectRevert(); + tychoRouter.removeCallbackVerifier(BOB); + } + + function testSetVerifierMissingSetterRole() public { + setupTychoRouter(); + deployDummyContract(); + + vm.expectRevert(); + tychoRouter.setCallbackVerifier(DUMMY); + } + + function testSetVerifierNonContract() public { + setupTychoRouter(); + deployDummyContract(); + + vm.startPrank(executorSetter); + vm.expectRevert( + abi.encodeWithSelector(TychoRouter__NonContractVerifier.selector) + ); + tychoRouter.setCallbackVerifier(BOB); + vm.stopPrank(); } } From 59950a7575d2a388cfc040ff8da63d98de544ac0 Mon Sep 17 00:00:00 2001 From: TAMARA LIPOWSKI Date: Wed, 22 Jan 2025 12:49:01 -0500 Subject: [PATCH 2/3] feat: Emit events when setting executors/verifiers --- foundry/src/TychoRouter.sol | 4 ++++ foundry/test/TychoRouter.t.sol | 11 +++++++++++ 2 files changed, 15 insertions(+) diff --git a/foundry/src/TychoRouter.sol b/foundry/src/TychoRouter.sol index 62a2b0c..73bca57 100644 --- a/foundry/src/TychoRouter.sol +++ b/foundry/src/TychoRouter.sol @@ -35,6 +35,8 @@ contract TychoRouter is event Withdrawal( address indexed token, uint256 amount, address indexed receiver ); + event ExecutorSet(address indexed executor); + event CallbackVerifierSet(address indexed callbackVerifier); constructor(address _permit2) { permit2 = IAllowanceTransfer(_permit2); @@ -91,6 +93,7 @@ contract TychoRouter is { if (target.code.length == 0) revert TychoRouter__NonContractExecutor(); swapExecutors[target] = true; + emit ExecutorSet(target); } /** @@ -114,6 +117,7 @@ contract TychoRouter is { if (target.code.length == 0) revert TychoRouter__NonContractVerifier(); callbackVerifiers[target] = true; + emit CallbackVerifierSet(target); } /** diff --git a/foundry/test/TychoRouter.t.sol b/foundry/test/TychoRouter.t.sol index 79bb26e..d261c60 100644 --- a/foundry/test/TychoRouter.t.sol +++ b/foundry/test/TychoRouter.t.sol @@ -14,6 +14,9 @@ contract TychoRouterTest is TychoRouterTestTemplate { bytes32 public constant FUND_RESCUER_ROLE = 0x912e45d663a6f4cc1d0491d8f046e06c616f40352565ea1cdb86a0e1aaefa41b; + event ExecutorSet(address indexed executor); + event CallbackVerifierSet(address indexed callbackVerifier); + function setupTychoRouter() public { deployTychoRouter(); } @@ -23,6 +26,10 @@ contract TychoRouterTest is TychoRouterTestTemplate { deployDummyContract(); vm.startPrank(executorSetter); + vm.expectEmit(); + // Define the event we expect to be emitted at the next step + emit ExecutorSet(DUMMY); + tychoRouter.setSwapExecutor(DUMMY); vm.stopPrank(); @@ -82,6 +89,10 @@ contract TychoRouterTest is TychoRouterTestTemplate { deployDummyContract(); vm.startPrank(executorSetter); + vm.expectEmit(); + // Define the event we expect to be emitted at the next step + emit CallbackVerifierSet(DUMMY); + tychoRouter.setCallbackVerifier(DUMMY); vm.stopPrank(); From 34243e9016894a7e1f0cd9207987786cc30b4f69 Mon Sep 17 00:00:00 2001 From: TAMARA LIPOWSKI Date: Wed, 22 Jan 2025 14:21:49 -0500 Subject: [PATCH 3/3] docs: remove mention of delegatecall from verifier doc - Because we will more likely use a regular call (safer and delegatecall is unnecessary) --- foundry/src/CallbackVerificationDispatcher.sol | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/foundry/src/CallbackVerificationDispatcher.sol b/foundry/src/CallbackVerificationDispatcher.sol index 644a20c..89e05d1 100644 --- a/foundry/src/CallbackVerificationDispatcher.sol +++ b/foundry/src/CallbackVerificationDispatcher.sol @@ -4,11 +4,9 @@ pragma solidity ^0.8.0; /** * @title Dispatch callback verification to external contracts * @author PropellerHeads Devs - * @dev Provides the ability to delegate callback verification to external - * contracts. This allows dynamically adding new supported protocols - * without needing to upgrade any contracts. External contracts will - * be called using delegatecall so they can share state with the main - * contract if needed. + * @dev Provides the ability call external contracts to perform callback + * verification. This allows dynamically adding new supported protocols + * without needing to upgrade any contracts. * * Note Verifier contracts need to implement the ICallbackVerifier interface */