From f621f3036464919fbfa87967da61e40ff40b757c Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Thu, 17 Apr 2025 13:22:52 +0100 Subject: [PATCH 01/22] fix: remove gas airdrop logic --- contracts/bridge/SynapseBridge.sol | 7 ------- 1 file changed, 7 deletions(-) diff --git a/contracts/bridge/SynapseBridge.sol b/contracts/bridge/SynapseBridge.sol index e5a896b4b..997b132ac 100644 --- a/contracts/bridge/SynapseBridge.sol +++ b/contracts/bridge/SynapseBridge.sol @@ -244,9 +244,6 @@ contract SynapseBridge is Initializable, AccessControlUpgradeable, ReentrancyGua emit TokenMint(to, token, amount.sub(fee), fee, kappa); token.mint(address(this), amount); IERC20(token).safeTransfer(to, amount.sub(fee)); - if (chainGasAmount != 0 && address(this).balance > chainGasAmount) { - to.call.value(chainGasAmount)(""); - } } /** @@ -353,10 +350,6 @@ contract SynapseBridge is Initializable, AccessControlUpgradeable, ReentrancyGua require(!kappaMap[kappa], "Kappa is already present"); kappaMap[kappa] = true; fees[address(token)] = fees[address(token)].add(fee); - // Transfer gas airdrop - if (chainGasAmount != 0 && address(this).balance > chainGasAmount) { - to.call.value(chainGasAmount)(""); - } // first check to make sure more will be given than min amount required uint256 expectedOutput = ISwap(pool).calculateSwap(tokenIndexFrom, tokenIndexTo, amount.sub(fee)); From 3b7545d1f5f512ce656a829e691f27c40331b2dc Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Thu, 17 Apr 2025 13:26:51 +0100 Subject: [PATCH 02/22] feat: allow governance to withdraw unspent gas airdops --- contracts/bridge/SynapseBridge.sol | 36 +++++++++++++++++++++--------- 1 file changed, 26 insertions(+), 10 deletions(-) diff --git a/contracts/bridge/SynapseBridge.sol b/contracts/bridge/SynapseBridge.sol index 997b132ac..8e06d2ff0 100644 --- a/contracts/bridge/SynapseBridge.sol +++ b/contracts/bridge/SynapseBridge.sol @@ -48,6 +48,12 @@ contract SynapseBridge is Initializable, AccessControlUpgradeable, ReentrancyGua chainGasAmount = amount; } + function withdrawChainGas() external { + require(hasRole(GOVERNANCE_ROLE, msg.sender), "Not governance"); + (bool success, ) = msg.sender.call{value: address(this).balance}(""); + require(success, "ETH_TRANSFER_FAILED"); + } + function setWethAddress(address payable _wethAddress) external { require(hasRole(DEFAULT_ADMIN_ROLE, msg.sender), "Not admin"); WETH_ADDRESS = _wethAddress; @@ -161,7 +167,8 @@ contract SynapseBridge is Initializable, AccessControlUpgradeable, ReentrancyGua * @param chainId which chain to bridge assets onto * @param token ERC20 compatible token to deposit into the bridge * @param amount Amount in native token decimals to transfer cross-chain pre-fees - **/ + * + */ function deposit( address to, uint256 chainId, @@ -178,7 +185,8 @@ contract SynapseBridge is Initializable, AccessControlUpgradeable, ReentrancyGua * @param chainId which underlying chain to bridge assets onto * @param token ERC20 compatible token to deposit into the bridge * @param amount Amount in native token decimals to transfer cross-chain pre-fees - **/ + * + */ function redeem( address to, uint256 chainId, @@ -196,7 +204,8 @@ contract SynapseBridge is Initializable, AccessControlUpgradeable, ReentrancyGua * @param amount Amount in native token decimals to withdraw * @param fee Amount in native token decimals to save to the contract as fees * @param kappa kappa - **/ + * + */ function withdraw( address to, IERC20 token, @@ -228,7 +237,8 @@ contract SynapseBridge is Initializable, AccessControlUpgradeable, ReentrancyGua * @param amount Amount in native token decimals to transfer cross-chain post-fees * @param fee Amount in native token decimals to save to the contract as fees * @param kappa kappa - **/ + * + */ function mint( address payable to, IERC20Mintable token, @@ -256,7 +266,8 @@ contract SynapseBridge is Initializable, AccessControlUpgradeable, ReentrancyGua * @param tokenIndexTo the token the user wants to swap to * @param minDy the min amount the user would like to receive, or revert to only minting the SynERC20 token crosschain. * @param deadline latest timestamp to accept this transaction - **/ + * + */ function depositAndSwap( address to, uint256 chainId, @@ -281,7 +292,8 @@ contract SynapseBridge is Initializable, AccessControlUpgradeable, ReentrancyGua * @param tokenIndexTo the token the user wants to swap to * @param minDy the min amount the user would like to receive, or revert to only minting the SynERC20 token crosschain. * @param deadline latest timestamp to accept this transaction - **/ + * + */ function redeemAndSwap( address to, uint256 chainId, @@ -305,7 +317,8 @@ contract SynapseBridge is Initializable, AccessControlUpgradeable, ReentrancyGua * @param swapTokenIndex Specifies which of the underlying LP assets the nodes should attempt to redeem for * @param swapMinAmount Specifies the minimum amount of the underlying asset needed for the nodes to execute the redeem/swap * @param swapDeadline Specificies the deadline that the nodes are allowed to try to redeem/swap the LP token - **/ + * + */ function redeemAndRemove( address to, uint256 chainId, @@ -332,7 +345,8 @@ contract SynapseBridge is Initializable, AccessControlUpgradeable, ReentrancyGua * @param minDy Minumum amount (in final asset decimals) that must be swapped for, otherwise the user will receive the SynERC20. * @param deadline Epoch time of the deadline that the swap is allowed to be executed. * @param kappa kappa - **/ + * + */ function mintAndSwap( address payable to, IERC20Mintable token, @@ -437,7 +451,8 @@ contract SynapseBridge is Initializable, AccessControlUpgradeable, ReentrancyGua * @param swapMinAmount Specifies the minimum amount of the underlying asset needed for the nodes to execute the redeem/swap * @param swapDeadline Specificies the deadline that the nodes are allowed to try to redeem/swap the LP token * @param kappa kappa - **/ + * + */ function withdrawAndRemove( address to, IERC20 token, @@ -513,7 +528,8 @@ contract SynapseBridge is Initializable, AccessControlUpgradeable, ReentrancyGua * @param chainId which underlying chain to bridge assets onto * @param token ERC20 compatible token to deposit into the bridge * @param amount Amount in native token decimals to transfer cross-chain pre-fees - **/ + * + */ function redeemV2( bytes32 to, uint256 chainId, From 2cde3ea81824caa06fdd51d6a4a698292c276f29 Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Thu, 17 Apr 2025 14:00:18 +0100 Subject: [PATCH 03/22] feat: add legacy flag --- contracts/bridge/SynapseBridge.sol | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/contracts/bridge/SynapseBridge.sol b/contracts/bridge/SynapseBridge.sol index 8e06d2ff0..964958264 100644 --- a/contracts/bridge/SynapseBridge.sol +++ b/contracts/bridge/SynapseBridge.sol @@ -34,6 +34,12 @@ contract SynapseBridge is Initializable, AccessControlUpgradeable, ReentrancyGua address payable public WETH_ADDRESS; mapping(bytes32 => bool) private kappaMap; + bool public isLegacyBridgeDisabled; + + modifier legacyBridgeEnabled() { + require(!isLegacyBridgeDisabled, "Legacy bridge is disabled"); + _; + } receive() external payable {} @@ -54,6 +60,11 @@ contract SynapseBridge is Initializable, AccessControlUpgradeable, ReentrancyGua require(success, "ETH_TRANSFER_FAILED"); } + function setLegacyBridgeDisabled(bool _isLegacyBridgeDisabled) external { + require(hasRole(GOVERNANCE_ROLE, msg.sender), "Not governance"); + isLegacyBridgeDisabled = _isLegacyBridgeDisabled; + } + function setWethAddress(address payable _wethAddress) external { require(hasRole(DEFAULT_ADMIN_ROLE, msg.sender), "Not admin"); WETH_ADDRESS = _wethAddress; @@ -174,7 +185,7 @@ contract SynapseBridge is Initializable, AccessControlUpgradeable, ReentrancyGua uint256 chainId, IERC20 token, uint256 amount - ) external nonReentrant whenNotPaused { + ) external nonReentrant whenNotPaused legacyBridgeEnabled { emit TokenDeposit(to, chainId, token, amount); token.safeTransferFrom(msg.sender, address(this), amount); } @@ -192,7 +203,7 @@ contract SynapseBridge is Initializable, AccessControlUpgradeable, ReentrancyGua uint256 chainId, ERC20Burnable token, uint256 amount - ) external nonReentrant whenNotPaused { + ) external nonReentrant whenNotPaused legacyBridgeEnabled { emit TokenRedeem(to, chainId, token, amount); token.burnFrom(msg.sender, amount); } @@ -277,7 +288,7 @@ contract SynapseBridge is Initializable, AccessControlUpgradeable, ReentrancyGua uint8 tokenIndexTo, uint256 minDy, uint256 deadline - ) external nonReentrant whenNotPaused { + ) external nonReentrant whenNotPaused legacyBridgeEnabled { emit TokenDepositAndSwap(to, chainId, token, amount, tokenIndexFrom, tokenIndexTo, minDy, deadline); token.safeTransferFrom(msg.sender, address(this), amount); } @@ -303,7 +314,7 @@ contract SynapseBridge is Initializable, AccessControlUpgradeable, ReentrancyGua uint8 tokenIndexTo, uint256 minDy, uint256 deadline - ) external nonReentrant whenNotPaused { + ) external nonReentrant whenNotPaused legacyBridgeEnabled { emit TokenRedeemAndSwap(to, chainId, token, amount, tokenIndexFrom, tokenIndexTo, minDy, deadline); token.burnFrom(msg.sender, amount); } @@ -327,7 +338,7 @@ contract SynapseBridge is Initializable, AccessControlUpgradeable, ReentrancyGua uint8 swapTokenIndex, uint256 swapMinAmount, uint256 swapDeadline - ) external nonReentrant whenNotPaused { + ) external nonReentrant whenNotPaused legacyBridgeEnabled { emit TokenRedeemAndRemove(to, chainId, token, amount, swapTokenIndex, swapMinAmount, swapDeadline); token.burnFrom(msg.sender, amount); } @@ -535,7 +546,7 @@ contract SynapseBridge is Initializable, AccessControlUpgradeable, ReentrancyGua uint256 chainId, ERC20Burnable token, uint256 amount - ) external nonReentrant whenNotPaused { + ) external nonReentrant whenNotPaused legacyBridgeEnabled { emit TokenRedeemV2(to, chainId, token, amount); token.burnFrom(msg.sender, amount); } From 20b104d1b5576ccd6f1879be5e2486dcdc075b0a Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Thu, 17 Apr 2025 14:00:45 +0100 Subject: [PATCH 04/22] test: coverage for legacy flagf --- test/bridge/legacy/SynapseBridge.t.sol | 226 +++++++++++++++++++++++++ 1 file changed, 226 insertions(+) create mode 100644 test/bridge/legacy/SynapseBridge.t.sol diff --git a/test/bridge/legacy/SynapseBridge.t.sol b/test/bridge/legacy/SynapseBridge.t.sol new file mode 100644 index 000000000..cb87057e1 --- /dev/null +++ b/test/bridge/legacy/SynapseBridge.t.sol @@ -0,0 +1,226 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.6.12; +pragma experimental ABIEncoderV2; + +import {SynapseBridge, IERC20, ERC20Burnable} from "../../../contracts/bridge/SynapseBridge.sol"; +import {SynapseERC20} from "../../../contracts/bridge/SynapseERC20.sol"; + +import {Test} from "forge-std/Test.sol"; + +// solhint-disable func-name-mixedcase +contract SynapseBridgeLegacyTest is Test { + SynapseBridge internal bridge; + SynapseERC20 internal token; + + address internal user = makeAddr("User"); + + event TokenDeposit(address indexed to, uint256 chainId, address token, uint256 amount); + event TokenRedeem(address indexed to, uint256 chainId, address token, uint256 amount); + event TokenDepositAndSwap( + address indexed to, + uint256 chainId, + address token, + uint256 amount, + uint8 tokenIndexFrom, + uint8 tokenIndexTo, + uint256 minDy, + uint256 deadline + ); + event TokenRedeemAndSwap( + address indexed to, + uint256 chainId, + address token, + uint256 amount, + uint8 tokenIndexFrom, + uint8 tokenIndexTo, + uint256 minDy, + uint256 deadline + ); + event TokenRedeemAndRemove( + address indexed to, + uint256 chainId, + address token, + uint256 amount, + uint8 swapTokenIndex, + uint256 swapMinAmount, + uint256 swapDeadline + ); + + event TokenRedeemV2(bytes32 indexed to, uint256 chainId, address token, uint256 amount); + + function setUp() public { + bridge = new SynapseBridge(); + bridge.initialize(); + bridge.grantRole(bridge.GOVERNANCE_ROLE(), address(this)); + + token = new SynapseERC20(); + token.initialize("Test", "TST", 18, address(this)); + token.grantRole(token.MINTER_ROLE(), address(this)); + + token.mint(user, 1 ether); + vm.prank(user); + token.approve(address(bridge), type(uint256).max); + } + + function test_deposit() public { + vm.expectEmit(address(bridge)); + emit TokenDeposit({to: address(1), chainId: 2, token: address(token), amount: 3}); + vm.prank(user); + bridge.deposit({to: address(1), chainId: 2, token: IERC20(address(token)), amount: 3}); + } + + function test_deposit_revert_disabled() public { + bridge.setLegacyBridgeDisabled(true); + vm.expectRevert("Legacy bridge is disabled"); + vm.prank(user); + bridge.deposit({to: address(1), chainId: 2, token: IERC20(address(token)), amount: 3}); + } + + function test_depositAndSwap() public { + vm.expectEmit(address(bridge)); + emit TokenDepositAndSwap({ + to: address(1), + chainId: 2, + token: address(token), + amount: 3, + tokenIndexFrom: 4, + tokenIndexTo: 5, + minDy: 6, + deadline: 7 + }); + vm.prank(user); + bridge.depositAndSwap({ + to: address(1), + chainId: 2, + token: IERC20(address(token)), + amount: 3, + tokenIndexFrom: 4, + tokenIndexTo: 5, + minDy: 6, + deadline: 7 + }); + } + + function test_depositAndSwap_revert_disabled() public { + bridge.setLegacyBridgeDisabled(true); + vm.expectRevert("Legacy bridge is disabled"); + vm.prank(user); + bridge.depositAndSwap({ + to: address(1), + chainId: 2, + token: IERC20(address(token)), + amount: 3, + tokenIndexFrom: 4, + tokenIndexTo: 5, + minDy: 6, + deadline: 7 + }); + } + + function test_redeem() public { + vm.expectEmit(address(bridge)); + emit TokenRedeem({to: address(1), chainId: 2, token: address(token), amount: 3}); + vm.prank(user); + bridge.redeem({to: address(1), chainId: 2, token: ERC20Burnable(address(token)), amount: 3}); + } + + function test_redeem_revert_disabled() public { + bridge.setLegacyBridgeDisabled(true); + vm.expectRevert("Legacy bridge is disabled"); + vm.prank(user); + bridge.redeem({to: address(1), chainId: 2, token: ERC20Burnable(address(token)), amount: 3}); + } + + function test_redeemAndSwap() public { + vm.expectEmit(address(bridge)); + emit TokenRedeemAndSwap({ + to: address(1), + chainId: 2, + token: address(token), + amount: 3, + tokenIndexFrom: 4, + tokenIndexTo: 5, + minDy: 6, + deadline: 7 + }); + vm.prank(user); + bridge.redeemAndSwap({ + to: address(1), + chainId: 2, + token: ERC20Burnable(address(token)), + amount: 3, + tokenIndexFrom: 4, + tokenIndexTo: 5, + minDy: 6, + deadline: 7 + }); + } + + function test_redeemAndSwap_revert_disabled() public { + bridge.setLegacyBridgeDisabled(true); + vm.expectRevert("Legacy bridge is disabled"); + vm.prank(user); + bridge.redeemAndSwap({ + to: address(1), + chainId: 2, + token: ERC20Burnable(address(token)), + amount: 3, + tokenIndexFrom: 4, + tokenIndexTo: 5, + minDy: 6, + deadline: 7 + }); + } + + function test_redeemAndRemove() public { + vm.expectEmit(address(bridge)); + emit TokenRedeemAndRemove({ + to: address(1), + chainId: 2, + token: address(token), + amount: 3, + swapTokenIndex: 4, + swapMinAmount: 5, + swapDeadline: 6 + }); + vm.prank(user); + bridge.redeemAndRemove({ + to: address(1), + chainId: 2, + token: ERC20Burnable(address(token)), + amount: 3, + swapTokenIndex: 4, + swapMinAmount: 5, + swapDeadline: 6 + }); + } + + function test_redeemAndRemove_revert_disabled() public { + bridge.setLegacyBridgeDisabled(true); + vm.expectRevert("Legacy bridge is disabled"); + vm.prank(user); + bridge.redeemAndRemove({ + to: address(1), + chainId: 2, + token: ERC20Burnable(address(token)), + amount: 3, + swapTokenIndex: 4, + swapMinAmount: 5, + swapDeadline: 6 + }); + } + + function test_redeemV2() public { + vm.expectEmit(address(bridge)); + emit TokenRedeemV2({to: bytes32(uint256(1)), chainId: 2, token: address(token), amount: 3}); + vm.prank(user); + bridge.redeemV2({to: bytes32(uint256(1)), chainId: 2, token: ERC20Burnable(address(token)), amount: 3}); + } + + function test_redeemV2_revert_disabled() public { + bridge.setLegacyBridgeDisabled(true); + vm.expectRevert("Legacy bridge is disabled"); + vm.prank(user); + bridge.redeemV2({to: bytes32(uint256(1)), chainId: 2, token: ERC20Burnable(address(token)), amount: 3}); + } +} From 285d915cbe38dcb775b583b1124a9d8825cbdac2 Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Thu, 17 Apr 2025 14:06:23 +0100 Subject: [PATCH 05/22] test: coverage for new gov actions --- test/bridge/legacy/SynapseBridge.t.sol | 54 ++++++++++++++++++++++---- 1 file changed, 47 insertions(+), 7 deletions(-) diff --git a/test/bridge/legacy/SynapseBridge.t.sol b/test/bridge/legacy/SynapseBridge.t.sol index cb87057e1..7da585f82 100644 --- a/test/bridge/legacy/SynapseBridge.t.sol +++ b/test/bridge/legacy/SynapseBridge.t.sol @@ -13,6 +13,7 @@ contract SynapseBridgeLegacyTest is Test { SynapseERC20 internal token; address internal user = makeAddr("User"); + address internal governance = makeAddr("Governance"); event TokenDeposit(address indexed to, uint256 chainId, address token, uint256 amount); event TokenRedeem(address indexed to, uint256 chainId, address token, uint256 amount); @@ -51,7 +52,7 @@ contract SynapseBridgeLegacyTest is Test { function setUp() public { bridge = new SynapseBridge(); bridge.initialize(); - bridge.grantRole(bridge.GOVERNANCE_ROLE(), address(this)); + bridge.grantRole(bridge.GOVERNANCE_ROLE(), governance); token = new SynapseERC20(); token.initialize("Test", "TST", 18, address(this)); @@ -62,6 +63,45 @@ contract SynapseBridgeLegacyTest is Test { token.approve(address(bridge), type(uint256).max); } + function disableLegacyBridge() public { + vm.prank(governance); + bridge.setLegacyBridgeDisabled(true); + } + + function test_disableLegacyBridge() public { + disableLegacyBridge(); + assertTrue(bridge.isLegacyBridgeDisabled()); + } + + function test_enableLegacyBridge() public { + disableLegacyBridge(); + vm.prank(governance); + bridge.setLegacyBridgeDisabled(false); + assertFalse(bridge.isLegacyBridgeDisabled()); + } + + function test_setLegacyBridgeDisabled_revert_notGovernance(address caller) public { + vm.assume(caller != governance); + vm.prank(caller); + vm.expectRevert("Not governance"); + bridge.setLegacyBridgeDisabled(true); + } + + function test_withdrawChainGas() public { + deal(address(bridge), 123456); + vm.prank(governance); + bridge.withdrawChainGas(); + assertEq(governance.balance, 123456); + } + + function test_withdrawChainGas_revert_notGovernance(address caller) public { + deal(address(bridge), 123456); + vm.assume(caller != governance); + vm.prank(caller); + vm.expectRevert("Not governance"); + bridge.withdrawChainGas(); + } + function test_deposit() public { vm.expectEmit(address(bridge)); emit TokenDeposit({to: address(1), chainId: 2, token: address(token), amount: 3}); @@ -70,7 +110,7 @@ contract SynapseBridgeLegacyTest is Test { } function test_deposit_revert_disabled() public { - bridge.setLegacyBridgeDisabled(true); + disableLegacyBridge(); vm.expectRevert("Legacy bridge is disabled"); vm.prank(user); bridge.deposit({to: address(1), chainId: 2, token: IERC20(address(token)), amount: 3}); @@ -102,7 +142,7 @@ contract SynapseBridgeLegacyTest is Test { } function test_depositAndSwap_revert_disabled() public { - bridge.setLegacyBridgeDisabled(true); + disableLegacyBridge(); vm.expectRevert("Legacy bridge is disabled"); vm.prank(user); bridge.depositAndSwap({ @@ -125,7 +165,7 @@ contract SynapseBridgeLegacyTest is Test { } function test_redeem_revert_disabled() public { - bridge.setLegacyBridgeDisabled(true); + disableLegacyBridge(); vm.expectRevert("Legacy bridge is disabled"); vm.prank(user); bridge.redeem({to: address(1), chainId: 2, token: ERC20Burnable(address(token)), amount: 3}); @@ -157,7 +197,7 @@ contract SynapseBridgeLegacyTest is Test { } function test_redeemAndSwap_revert_disabled() public { - bridge.setLegacyBridgeDisabled(true); + disableLegacyBridge(); vm.expectRevert("Legacy bridge is disabled"); vm.prank(user); bridge.redeemAndSwap({ @@ -196,7 +236,7 @@ contract SynapseBridgeLegacyTest is Test { } function test_redeemAndRemove_revert_disabled() public { - bridge.setLegacyBridgeDisabled(true); + disableLegacyBridge(); vm.expectRevert("Legacy bridge is disabled"); vm.prank(user); bridge.redeemAndRemove({ @@ -218,7 +258,7 @@ contract SynapseBridgeLegacyTest is Test { } function test_redeemV2_revert_disabled() public { - bridge.setLegacyBridgeDisabled(true); + disableLegacyBridge(); vm.expectRevert("Legacy bridge is disabled"); vm.prank(user); bridge.redeemV2({to: bytes32(uint256(1)), chainId: 2, token: ERC20Burnable(address(token)), amount: 3}); From f7785719629399c21aed2e3c08367c37c254b486 Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Thu, 17 Apr 2025 14:08:27 +0100 Subject: [PATCH 06/22] test: reenabling --- test/bridge/legacy/SynapseBridge.t.sol | 44 ++++++++++++++++++++++++-- 1 file changed, 42 insertions(+), 2 deletions(-) diff --git a/test/bridge/legacy/SynapseBridge.t.sol b/test/bridge/legacy/SynapseBridge.t.sol index 7da585f82..e8b0c99c6 100644 --- a/test/bridge/legacy/SynapseBridge.t.sol +++ b/test/bridge/legacy/SynapseBridge.t.sol @@ -68,6 +68,11 @@ contract SynapseBridgeLegacyTest is Test { bridge.setLegacyBridgeDisabled(true); } + function enableLegacyBridge() public { + vm.prank(governance); + bridge.setLegacyBridgeDisabled(false); + } + function test_disableLegacyBridge() public { disableLegacyBridge(); assertTrue(bridge.isLegacyBridgeDisabled()); @@ -75,8 +80,7 @@ contract SynapseBridgeLegacyTest is Test { function test_enableLegacyBridge() public { disableLegacyBridge(); - vm.prank(governance); - bridge.setLegacyBridgeDisabled(false); + enableLegacyBridge(); assertFalse(bridge.isLegacyBridgeDisabled()); } @@ -109,6 +113,12 @@ contract SynapseBridgeLegacyTest is Test { bridge.deposit({to: address(1), chainId: 2, token: IERC20(address(token)), amount: 3}); } + function test_deposit_reenabled() public { + disableLegacyBridge(); + enableLegacyBridge(); + test_deposit(); + } + function test_deposit_revert_disabled() public { disableLegacyBridge(); vm.expectRevert("Legacy bridge is disabled"); @@ -141,6 +151,12 @@ contract SynapseBridgeLegacyTest is Test { }); } + function test_depositAndSwap_reenabled() public { + disableLegacyBridge(); + enableLegacyBridge(); + test_depositAndSwap(); + } + function test_depositAndSwap_revert_disabled() public { disableLegacyBridge(); vm.expectRevert("Legacy bridge is disabled"); @@ -164,6 +180,12 @@ contract SynapseBridgeLegacyTest is Test { bridge.redeem({to: address(1), chainId: 2, token: ERC20Burnable(address(token)), amount: 3}); } + function test_redeem_reenabled() public { + disableLegacyBridge(); + enableLegacyBridge(); + test_redeem(); + } + function test_redeem_revert_disabled() public { disableLegacyBridge(); vm.expectRevert("Legacy bridge is disabled"); @@ -196,6 +218,12 @@ contract SynapseBridgeLegacyTest is Test { }); } + function test_redeemAndSwap_reenabled() public { + disableLegacyBridge(); + enableLegacyBridge(); + test_redeemAndSwap(); + } + function test_redeemAndSwap_revert_disabled() public { disableLegacyBridge(); vm.expectRevert("Legacy bridge is disabled"); @@ -235,6 +263,12 @@ contract SynapseBridgeLegacyTest is Test { }); } + function test_redeemAndRemove_reenabled() public { + disableLegacyBridge(); + enableLegacyBridge(); + test_redeemAndRemove(); + } + function test_redeemAndRemove_revert_disabled() public { disableLegacyBridge(); vm.expectRevert("Legacy bridge is disabled"); @@ -257,6 +291,12 @@ contract SynapseBridgeLegacyTest is Test { bridge.redeemV2({to: bytes32(uint256(1)), chainId: 2, token: ERC20Burnable(address(token)), amount: 3}); } + function test_redeemV2_reenabled() public { + disableLegacyBridge(); + enableLegacyBridge(); + test_redeemV2(); + } + function test_redeemV2_revert_disabled() public { disableLegacyBridge(); vm.expectRevert("Legacy bridge is disabled"); From e9bb7016f2e643f1b1c458dc6fcd1ea4ab68ce84 Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Tue, 22 Apr 2025 11:52:53 +0100 Subject: [PATCH 07/22] chore: revert docs formatting changes --- contracts/bridge/SynapseBridge.sol | 30 ++++++++++-------------------- 1 file changed, 10 insertions(+), 20 deletions(-) diff --git a/contracts/bridge/SynapseBridge.sol b/contracts/bridge/SynapseBridge.sol index 964958264..21883cce8 100644 --- a/contracts/bridge/SynapseBridge.sol +++ b/contracts/bridge/SynapseBridge.sol @@ -178,8 +178,7 @@ contract SynapseBridge is Initializable, AccessControlUpgradeable, ReentrancyGua * @param chainId which chain to bridge assets onto * @param token ERC20 compatible token to deposit into the bridge * @param amount Amount in native token decimals to transfer cross-chain pre-fees - * - */ + **/ function deposit( address to, uint256 chainId, @@ -196,8 +195,7 @@ contract SynapseBridge is Initializable, AccessControlUpgradeable, ReentrancyGua * @param chainId which underlying chain to bridge assets onto * @param token ERC20 compatible token to deposit into the bridge * @param amount Amount in native token decimals to transfer cross-chain pre-fees - * - */ + **/ function redeem( address to, uint256 chainId, @@ -215,8 +213,7 @@ contract SynapseBridge is Initializable, AccessControlUpgradeable, ReentrancyGua * @param amount Amount in native token decimals to withdraw * @param fee Amount in native token decimals to save to the contract as fees * @param kappa kappa - * - */ + **/ function withdraw( address to, IERC20 token, @@ -248,8 +245,7 @@ contract SynapseBridge is Initializable, AccessControlUpgradeable, ReentrancyGua * @param amount Amount in native token decimals to transfer cross-chain post-fees * @param fee Amount in native token decimals to save to the contract as fees * @param kappa kappa - * - */ + **/ function mint( address payable to, IERC20Mintable token, @@ -277,8 +273,7 @@ contract SynapseBridge is Initializable, AccessControlUpgradeable, ReentrancyGua * @param tokenIndexTo the token the user wants to swap to * @param minDy the min amount the user would like to receive, or revert to only minting the SynERC20 token crosschain. * @param deadline latest timestamp to accept this transaction - * - */ + **/ function depositAndSwap( address to, uint256 chainId, @@ -303,8 +298,7 @@ contract SynapseBridge is Initializable, AccessControlUpgradeable, ReentrancyGua * @param tokenIndexTo the token the user wants to swap to * @param minDy the min amount the user would like to receive, or revert to only minting the SynERC20 token crosschain. * @param deadline latest timestamp to accept this transaction - * - */ + **/ function redeemAndSwap( address to, uint256 chainId, @@ -328,8 +322,7 @@ contract SynapseBridge is Initializable, AccessControlUpgradeable, ReentrancyGua * @param swapTokenIndex Specifies which of the underlying LP assets the nodes should attempt to redeem for * @param swapMinAmount Specifies the minimum amount of the underlying asset needed for the nodes to execute the redeem/swap * @param swapDeadline Specificies the deadline that the nodes are allowed to try to redeem/swap the LP token - * - */ + **/ function redeemAndRemove( address to, uint256 chainId, @@ -356,8 +349,7 @@ contract SynapseBridge is Initializable, AccessControlUpgradeable, ReentrancyGua * @param minDy Minumum amount (in final asset decimals) that must be swapped for, otherwise the user will receive the SynERC20. * @param deadline Epoch time of the deadline that the swap is allowed to be executed. * @param kappa kappa - * - */ + **/ function mintAndSwap( address payable to, IERC20Mintable token, @@ -462,8 +454,7 @@ contract SynapseBridge is Initializable, AccessControlUpgradeable, ReentrancyGua * @param swapMinAmount Specifies the minimum amount of the underlying asset needed for the nodes to execute the redeem/swap * @param swapDeadline Specificies the deadline that the nodes are allowed to try to redeem/swap the LP token * @param kappa kappa - * - */ + **/ function withdrawAndRemove( address to, IERC20 token, @@ -539,8 +530,7 @@ contract SynapseBridge is Initializable, AccessControlUpgradeable, ReentrancyGua * @param chainId which underlying chain to bridge assets onto * @param token ERC20 compatible token to deposit into the bridge * @param amount Amount in native token decimals to transfer cross-chain pre-fees - * - */ + **/ function redeemV2( bytes32 to, uint256 chainId, From 189b72e6dc357036adbc5940e8e3dc1cec952589 Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Wed, 23 Apr 2025 12:08:48 +0100 Subject: [PATCH 08/22] chore: bump bridge version --- contracts/bridge/SynapseBridge.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/bridge/SynapseBridge.sol b/contracts/bridge/SynapseBridge.sol index 21883cce8..896b7df5f 100644 --- a/contracts/bridge/SynapseBridge.sol +++ b/contracts/bridge/SynapseBridge.sol @@ -29,7 +29,7 @@ contract SynapseBridge is Initializable, AccessControlUpgradeable, ReentrancyGua mapping(address => uint256) private fees; uint256 public startBlockNumber; - uint256 public constant bridgeVersion = 6; + uint256 public constant bridgeVersion = 8; uint256 public chainGasAmount; address payable public WETH_ADDRESS; From acbcaa84b8d9303bc693ba0e497c6f3621514b03 Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Wed, 23 Apr 2025 17:18:10 +0100 Subject: [PATCH 09/22] refactor: more explicit name for sending tokens --- contracts/bridge/SynapseBridge.sol | 22 ++++----- test/bridge/legacy/SynapseBridge.t.sol | 62 +++++++++++++------------- 2 files changed, 42 insertions(+), 42 deletions(-) diff --git a/contracts/bridge/SynapseBridge.sol b/contracts/bridge/SynapseBridge.sol index 896b7df5f..ae530f267 100644 --- a/contracts/bridge/SynapseBridge.sol +++ b/contracts/bridge/SynapseBridge.sol @@ -34,10 +34,10 @@ contract SynapseBridge is Initializable, AccessControlUpgradeable, ReentrancyGua address payable public WETH_ADDRESS; mapping(bytes32 => bool) private kappaMap; - bool public isLegacyBridgeDisabled; + bool public isLegacySendDisabled; - modifier legacyBridgeEnabled() { - require(!isLegacyBridgeDisabled, "Legacy bridge is disabled"); + modifier legacySendEnabled() { + require(!isLegacySendDisabled, "Legacy send is disabled"); _; } @@ -60,9 +60,9 @@ contract SynapseBridge is Initializable, AccessControlUpgradeable, ReentrancyGua require(success, "ETH_TRANSFER_FAILED"); } - function setLegacyBridgeDisabled(bool _isLegacyBridgeDisabled) external { + function setLegacySendDisabled(bool _isLegacySendDisabled) external { require(hasRole(GOVERNANCE_ROLE, msg.sender), "Not governance"); - isLegacyBridgeDisabled = _isLegacyBridgeDisabled; + isLegacySendDisabled = _isLegacySendDisabled; } function setWethAddress(address payable _wethAddress) external { @@ -184,7 +184,7 @@ contract SynapseBridge is Initializable, AccessControlUpgradeable, ReentrancyGua uint256 chainId, IERC20 token, uint256 amount - ) external nonReentrant whenNotPaused legacyBridgeEnabled { + ) external nonReentrant whenNotPaused legacySendEnabled { emit TokenDeposit(to, chainId, token, amount); token.safeTransferFrom(msg.sender, address(this), amount); } @@ -201,7 +201,7 @@ contract SynapseBridge is Initializable, AccessControlUpgradeable, ReentrancyGua uint256 chainId, ERC20Burnable token, uint256 amount - ) external nonReentrant whenNotPaused legacyBridgeEnabled { + ) external nonReentrant whenNotPaused legacySendEnabled { emit TokenRedeem(to, chainId, token, amount); token.burnFrom(msg.sender, amount); } @@ -283,7 +283,7 @@ contract SynapseBridge is Initializable, AccessControlUpgradeable, ReentrancyGua uint8 tokenIndexTo, uint256 minDy, uint256 deadline - ) external nonReentrant whenNotPaused legacyBridgeEnabled { + ) external nonReentrant whenNotPaused legacySendEnabled { emit TokenDepositAndSwap(to, chainId, token, amount, tokenIndexFrom, tokenIndexTo, minDy, deadline); token.safeTransferFrom(msg.sender, address(this), amount); } @@ -308,7 +308,7 @@ contract SynapseBridge is Initializable, AccessControlUpgradeable, ReentrancyGua uint8 tokenIndexTo, uint256 minDy, uint256 deadline - ) external nonReentrant whenNotPaused legacyBridgeEnabled { + ) external nonReentrant whenNotPaused legacySendEnabled { emit TokenRedeemAndSwap(to, chainId, token, amount, tokenIndexFrom, tokenIndexTo, minDy, deadline); token.burnFrom(msg.sender, amount); } @@ -331,7 +331,7 @@ contract SynapseBridge is Initializable, AccessControlUpgradeable, ReentrancyGua uint8 swapTokenIndex, uint256 swapMinAmount, uint256 swapDeadline - ) external nonReentrant whenNotPaused legacyBridgeEnabled { + ) external nonReentrant whenNotPaused legacySendEnabled { emit TokenRedeemAndRemove(to, chainId, token, amount, swapTokenIndex, swapMinAmount, swapDeadline); token.burnFrom(msg.sender, amount); } @@ -536,7 +536,7 @@ contract SynapseBridge is Initializable, AccessControlUpgradeable, ReentrancyGua uint256 chainId, ERC20Burnable token, uint256 amount - ) external nonReentrant whenNotPaused legacyBridgeEnabled { + ) external nonReentrant whenNotPaused legacySendEnabled { emit TokenRedeemV2(to, chainId, token, amount); token.burnFrom(msg.sender, amount); } diff --git a/test/bridge/legacy/SynapseBridge.t.sol b/test/bridge/legacy/SynapseBridge.t.sol index e8b0c99c6..6ea7c8d4b 100644 --- a/test/bridge/legacy/SynapseBridge.t.sol +++ b/test/bridge/legacy/SynapseBridge.t.sol @@ -63,32 +63,32 @@ contract SynapseBridgeLegacyTest is Test { token.approve(address(bridge), type(uint256).max); } - function disableLegacyBridge() public { + function disableLegacySend() public { vm.prank(governance); - bridge.setLegacyBridgeDisabled(true); + bridge.setLegacySendDisabled(true); } - function enableLegacyBridge() public { + function enableLegacySend() public { vm.prank(governance); - bridge.setLegacyBridgeDisabled(false); + bridge.setLegacySendDisabled(false); } - function test_disableLegacyBridge() public { - disableLegacyBridge(); - assertTrue(bridge.isLegacyBridgeDisabled()); + function test_disableLegacySend() public { + disableLegacySend(); + assertTrue(bridge.isLegacySendDisabled()); } - function test_enableLegacyBridge() public { - disableLegacyBridge(); - enableLegacyBridge(); - assertFalse(bridge.isLegacyBridgeDisabled()); + function test_enableLegacySend() public { + disableLegacySend(); + enableLegacySend(); + assertFalse(bridge.isLegacySendDisabled()); } - function test_setLegacyBridgeDisabled_revert_notGovernance(address caller) public { + function test_setLegacySendDisabled_revert_notGovernance(address caller) public { vm.assume(caller != governance); vm.prank(caller); vm.expectRevert("Not governance"); - bridge.setLegacyBridgeDisabled(true); + bridge.setLegacySendDisabled(true); } function test_withdrawChainGas() public { @@ -114,13 +114,13 @@ contract SynapseBridgeLegacyTest is Test { } function test_deposit_reenabled() public { - disableLegacyBridge(); - enableLegacyBridge(); + disableLegacySend(); + enableLegacySend(); test_deposit(); } function test_deposit_revert_disabled() public { - disableLegacyBridge(); + disableLegacySend(); vm.expectRevert("Legacy bridge is disabled"); vm.prank(user); bridge.deposit({to: address(1), chainId: 2, token: IERC20(address(token)), amount: 3}); @@ -152,13 +152,13 @@ contract SynapseBridgeLegacyTest is Test { } function test_depositAndSwap_reenabled() public { - disableLegacyBridge(); - enableLegacyBridge(); + disableLegacySend(); + enableLegacySend(); test_depositAndSwap(); } function test_depositAndSwap_revert_disabled() public { - disableLegacyBridge(); + disableLegacySend(); vm.expectRevert("Legacy bridge is disabled"); vm.prank(user); bridge.depositAndSwap({ @@ -181,13 +181,13 @@ contract SynapseBridgeLegacyTest is Test { } function test_redeem_reenabled() public { - disableLegacyBridge(); - enableLegacyBridge(); + disableLegacySend(); + enableLegacySend(); test_redeem(); } function test_redeem_revert_disabled() public { - disableLegacyBridge(); + disableLegacySend(); vm.expectRevert("Legacy bridge is disabled"); vm.prank(user); bridge.redeem({to: address(1), chainId: 2, token: ERC20Burnable(address(token)), amount: 3}); @@ -219,13 +219,13 @@ contract SynapseBridgeLegacyTest is Test { } function test_redeemAndSwap_reenabled() public { - disableLegacyBridge(); - enableLegacyBridge(); + disableLegacySend(); + enableLegacySend(); test_redeemAndSwap(); } function test_redeemAndSwap_revert_disabled() public { - disableLegacyBridge(); + disableLegacySend(); vm.expectRevert("Legacy bridge is disabled"); vm.prank(user); bridge.redeemAndSwap({ @@ -264,13 +264,13 @@ contract SynapseBridgeLegacyTest is Test { } function test_redeemAndRemove_reenabled() public { - disableLegacyBridge(); - enableLegacyBridge(); + disableLegacySend(); + enableLegacySend(); test_redeemAndRemove(); } function test_redeemAndRemove_revert_disabled() public { - disableLegacyBridge(); + disableLegacySend(); vm.expectRevert("Legacy bridge is disabled"); vm.prank(user); bridge.redeemAndRemove({ @@ -292,13 +292,13 @@ contract SynapseBridgeLegacyTest is Test { } function test_redeemV2_reenabled() public { - disableLegacyBridge(); - enableLegacyBridge(); + disableLegacySend(); + enableLegacySend(); test_redeemV2(); } function test_redeemV2_revert_disabled() public { - disableLegacyBridge(); + disableLegacySend(); vm.expectRevert("Legacy bridge is disabled"); vm.prank(user); bridge.redeemV2({to: bytes32(uint256(1)), chainId: 2, token: ERC20Burnable(address(token)), amount: 3}); From 0f34b971cdb86ec238fb3dbcf24c7f8408e7b67a Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Wed, 23 Apr 2025 17:30:03 +0100 Subject: [PATCH 10/22] chore: update actions/cache@v2 to v4 in GitHub workflows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude --- .github/workflows/coveralls.yaml | 2 +- .github/workflows/test.yaml | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/coveralls.yaml b/.github/workflows/coveralls.yaml index 8d8d6d634..59a00410c 100644 --- a/.github/workflows/coveralls.yaml +++ b/.github/workflows/coveralls.yaml @@ -31,7 +31,7 @@ jobs: node-version: "${{ steps.nvm.outputs.NVMRC }}" - name: Cache node modules - uses: actions/cache@v2 + uses: actions/cache@v4 env: cache-name: cache-node-modules with: diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index a482cb666..31bb242fb 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -51,7 +51,7 @@ jobs: submodules: recursive - name: Cache node modules - uses: actions/cache@v2 + uses: actions/cache@v4 env: cache-name: cache-node-modules with: @@ -82,7 +82,7 @@ jobs: submodules: recursive - name: Cache node modules - uses: actions/cache@v2 + uses: actions/cache@v4 env: cache-name: cache-node-modules with: From 34072987ee23244dafece22c1ba5c7b16bdd2e3e Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Wed, 23 Apr 2025 17:55:01 +0100 Subject: [PATCH 11/22] fix: update tests --- test/bridge/legacy/SynapseBridge.t.sol | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/test/bridge/legacy/SynapseBridge.t.sol b/test/bridge/legacy/SynapseBridge.t.sol index 6ea7c8d4b..212d9dd7f 100644 --- a/test/bridge/legacy/SynapseBridge.t.sol +++ b/test/bridge/legacy/SynapseBridge.t.sol @@ -121,7 +121,7 @@ contract SynapseBridgeLegacyTest is Test { function test_deposit_revert_disabled() public { disableLegacySend(); - vm.expectRevert("Legacy bridge is disabled"); + vm.expectRevert("Legacy send is disabled"); vm.prank(user); bridge.deposit({to: address(1), chainId: 2, token: IERC20(address(token)), amount: 3}); } @@ -159,7 +159,7 @@ contract SynapseBridgeLegacyTest is Test { function test_depositAndSwap_revert_disabled() public { disableLegacySend(); - vm.expectRevert("Legacy bridge is disabled"); + vm.expectRevert("Legacy send is disabled"); vm.prank(user); bridge.depositAndSwap({ to: address(1), @@ -188,7 +188,7 @@ contract SynapseBridgeLegacyTest is Test { function test_redeem_revert_disabled() public { disableLegacySend(); - vm.expectRevert("Legacy bridge is disabled"); + vm.expectRevert("Legacy send is disabled"); vm.prank(user); bridge.redeem({to: address(1), chainId: 2, token: ERC20Burnable(address(token)), amount: 3}); } @@ -226,7 +226,7 @@ contract SynapseBridgeLegacyTest is Test { function test_redeemAndSwap_revert_disabled() public { disableLegacySend(); - vm.expectRevert("Legacy bridge is disabled"); + vm.expectRevert("Legacy send is disabled"); vm.prank(user); bridge.redeemAndSwap({ to: address(1), @@ -271,7 +271,7 @@ contract SynapseBridgeLegacyTest is Test { function test_redeemAndRemove_revert_disabled() public { disableLegacySend(); - vm.expectRevert("Legacy bridge is disabled"); + vm.expectRevert("Legacy send is disabled"); vm.prank(user); bridge.redeemAndRemove({ to: address(1), @@ -299,7 +299,7 @@ contract SynapseBridgeLegacyTest is Test { function test_redeemV2_revert_disabled() public { disableLegacySend(); - vm.expectRevert("Legacy bridge is disabled"); + vm.expectRevert("Legacy send is disabled"); vm.prank(user); bridge.redeemV2({to: bytes32(uint256(1)), chainId: 2, token: ERC20Burnable(address(token)), amount: 3}); } From 84491d92dd2a1a4ae4af4f79e628b153a91f5ab1 Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Fri, 30 May 2025 17:51:49 +0100 Subject: [PATCH 12/22] test: expected behaviour for mint/withdraw pre/post legacy workflows --- test/bridge/legacy/PoolMock.sol | 16 ++ test/bridge/legacy/SynapseBridge.Dst.t.sol | 214 +++++++++++++++++++++ 2 files changed, 230 insertions(+) create mode 100644 test/bridge/legacy/PoolMock.sol create mode 100644 test/bridge/legacy/SynapseBridge.Dst.t.sol diff --git a/test/bridge/legacy/PoolMock.sol b/test/bridge/legacy/PoolMock.sol new file mode 100644 index 000000000..9d6928f4f --- /dev/null +++ b/test/bridge/legacy/PoolMock.sol @@ -0,0 +1,16 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.6.12; + +// solhint-disable no-empty-blocks, no-unused-vars +contract PoolMock { + /// @notice We include an empty "test" function so that this contract does not appear in the coverage report. + function testPoolMock() external {} + + function calculateSwap( + uint8 tokenIndexFrom, + uint8 tokenIndexTo, + uint256 amount + ) external returns (uint256) {} + + function calculateRemoveLiquidityOneToken(uint256 tokenAmount, uint8 tokenIndex) external returns (uint256) {} +} diff --git a/test/bridge/legacy/SynapseBridge.Dst.t.sol b/test/bridge/legacy/SynapseBridge.Dst.t.sol new file mode 100644 index 000000000..498999b0e --- /dev/null +++ b/test/bridge/legacy/SynapseBridge.Dst.t.sol @@ -0,0 +1,214 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.6.12; +pragma experimental ABIEncoderV2; + +import {SynapseBridge, IERC20, ERC20Burnable, IERC20Mintable, ISwap} from "../../../contracts/bridge/SynapseBridge.sol"; +import {SynapseERC20} from "../../../contracts/bridge/SynapseERC20.sol"; + +import {PoolMock} from "./PoolMock.sol"; + +import {Test} from "forge-std/Test.sol"; + +// solhint-disable func-name-mixedcase +contract SynapseBridgeLegacyDstTest is Test { + SynapseBridge internal bridge; + SynapseERC20 internal token; + address internal pool; + + address internal nodeGroup = makeAddr("NodeGroup"); + address internal user = makeAddr("User"); + bytes32 internal kappa = keccak256("kappa"); + + uint256 internal initialLockedBalance = 1000 ether; + uint256 internal amount = 1 ether; + uint256 internal fee = 0.01 ether; + + event TokenWithdraw(address indexed to, address token, uint256 amount, uint256 fee, bytes32 indexed kappa); + event TokenMint(address indexed to, address token, uint256 amount, uint256 fee, bytes32 indexed kappa); + event TokenMintAndSwap( + address indexed to, + address token, + uint256 amount, + uint256 fee, + uint8 tokenIndexFrom, + uint8 tokenIndexTo, + uint256 minDy, + uint256 deadline, + bool swapSuccess, + bytes32 indexed kappa + ); + event TokenWithdrawAndRemove( + address indexed to, + address token, + uint256 amount, + uint256 fee, + uint8 swapTokenIndex, + uint256 swapMinAmount, + uint256 swapDeadline, + bool swapSuccess, + bytes32 indexed kappa + ); + + modifier withMintToken() { + token.grantRole(token.MINTER_ROLE(), address(bridge)); + _; + } + + modifier withWithdrawToken() { + deal(address(token), address(bridge), initialLockedBalance, true); + _; + } + + modifier withRevertingPool() { + vm.mockCallRevert({callee: address(pool), data: "", revertData: "GM"}); + _; + } + + modifier withLegacySendDisabled() { + bridge.setLegacySendDisabled(true); + _; + } + + function expectMintEvent() public { + vm.expectEmit(address(bridge)); + emit TokenMint(user, address(token), amount - fee, fee, kappa); + } + + function expectMintAndSwapEvent() public { + vm.expectEmit(address(bridge)); + emit TokenMintAndSwap(user, address(token), amount - fee, fee, 0, 0, 1, 0, false, kappa); + } + + function expectWithdrawEvent() public { + // Note: TokenWithdraw emits amount before fees, which is left as is to preserve legacy behavior + vm.expectEmit(address(bridge)); + emit TokenWithdraw(user, address(token), amount, fee, kappa); + } + + function expectWithdrawAndRemoveEvent() public { + vm.expectEmit(address(bridge)); + emit TokenWithdrawAndRemove(user, address(token), amount - fee, fee, 0, 1, 0, false, kappa); + } + + function expectBalances( + uint256 bridgeBalance, + uint256 userBalance, + uint256 bridgeFees + ) public { + assertEq(token.balanceOf(address(bridge)), bridgeBalance); + assertEq(token.balanceOf(user), userBalance); + assertEq(bridge.getFeeBalance(address(token)), bridgeFees); + } + + function setUp() public { + bridge = new SynapseBridge(); + bridge.initialize(); + bridge.grantRole(bridge.GOVERNANCE_ROLE(), address(this)); + bridge.grantRole(bridge.NODEGROUP_ROLE(), nodeGroup); + + token = new SynapseERC20(); + token.initialize("Test", "TST", 18, address(this)); + + pool = address(new PoolMock()); + } + + function test_mint() public withMintToken { + expectMintEvent(); + vm.prank(nodeGroup); + bridge.mint(payable(user), IERC20Mintable(address(token)), amount, fee, kappa); + expectBalances({bridgeBalance: fee, userBalance: amount - fee, bridgeFees: fee}); + assertTrue(bridge.kappaExists(kappa)); + } + + function test_mintAndSwap() public withMintToken { + expectMintAndSwapEvent(); + vm.prank(nodeGroup); + // Use minDy > 0 so that it exceeds calculateSwap = 0 (swapSuccess = false). + bridge.mintAndSwap(payable(user), IERC20Mintable(address(token)), amount, fee, ISwap(pool), 0, 0, 1, 0, kappa); + expectBalances({bridgeBalance: fee, userBalance: amount - fee, bridgeFees: fee}); + assertTrue(bridge.kappaExists(kappa)); + } + + function test_mintAndSwap_reverts_withRevertingPool() public withMintToken withRevertingPool { + vm.expectRevert(bytes("GM")); + vm.prank(nodeGroup); + bridge.mintAndSwap(payable(user), IERC20Mintable(address(token)), amount, fee, ISwap(pool), 0, 0, 1, 0, kappa); + } + + /// @notice Should behave the same way as the legacy workflow. + function test_mint_legacySendDisabled() public withLegacySendDisabled { + test_mint(); + } + + /// @notice Should ignore andSwap instructions in legacySendDisabled mode. + function test_mintAndSwap_legacySendDisabled_mintFallback() public withLegacySendDisabled withMintToken { + expectMintEvent(); + vm.prank(nodeGroup); + bridge.mintAndSwap(payable(user), IERC20Mintable(address(token)), amount, fee, ISwap(pool), 0, 0, 1, 0, kappa); + expectBalances({bridgeBalance: fee, userBalance: amount - fee, bridgeFees: fee}); + assertTrue(bridge.kappaExists(kappa)); + } + + /// @notice Pool is never called in legacySendDisabled mode, so should be identical to mint fallback. + function test_mintAndSwap_legacySendDisabled_mintFallback_withRevertingPool() public withRevertingPool { + test_mintAndSwap_legacySendDisabled_mintFallback(); + } + + function test_withdraw() public withWithdrawToken { + expectWithdrawEvent(); + vm.prank(nodeGroup); + bridge.withdraw(payable(user), IERC20(address(token)), amount, fee, kappa); + expectBalances({ + bridgeBalance: initialLockedBalance - amount + fee, + userBalance: amount - fee, + bridgeFees: fee + }); + assertTrue(bridge.kappaExists(kappa)); + } + + function test_withdrawAndRemove() public withWithdrawToken { + expectWithdrawAndRemoveEvent(); + vm.prank(nodeGroup); + // Use swapMinAmount > 0 so that it exceeds calculateRemoveLiquidityOneToken = 0 (swapSuccess = false). + bridge.withdrawAndRemove(payable(user), IERC20(address(token)), amount, fee, ISwap(pool), 0, 1, 0, kappa); + expectBalances({ + bridgeBalance: initialLockedBalance - amount + fee, + userBalance: amount - fee, + bridgeFees: fee + }); + assertTrue(bridge.kappaExists(kappa)); + } + + function test_withdrawAndRemove_reverts_withRevertingPool() public withRevertingPool { + vm.expectRevert(bytes("GM")); + vm.prank(nodeGroup); + bridge.withdrawAndRemove(payable(user), IERC20(address(token)), amount, fee, ISwap(pool), 0, 1, 0, kappa); + } + + /// @notice Should behave the same way as the legacy workflow. + function test_withdraw_legacySendDisabled() public withLegacySendDisabled { + test_withdraw(); + } + + /// @notice Should ignore andRemove instructions in legacySendDisabled mode. + function test_withdrawAndRemove_legacySendDisabled_withdrawFallback() + public + withLegacySendDisabled + withWithdrawToken + { + expectWithdrawEvent(); + vm.prank(nodeGroup); + bridge.withdrawAndRemove(payable(user), IERC20(address(token)), amount, fee, ISwap(pool), 0, 1, 0, kappa); + expectBalances({ + bridgeBalance: initialLockedBalance - amount + fee, + userBalance: amount - fee, + bridgeFees: fee + }); + assertTrue(bridge.kappaExists(kappa)); + } + + /// @notice Pool is never called in legacySendDisabled mode, so should be identical to withdraw fallback. + function test_withdrawAndRemove_legacySendDisabled_withdrawFallback_withRevertingPool() public withRevertingPool { + test_withdrawAndRemove_legacySendDisabled_withdrawFallback(); + } +} From 2d66d6989267bfb18ede55d6bbcd0cf8661b8ac5 Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Fri, 30 May 2025 18:01:20 +0100 Subject: [PATCH 13/22] feat: fallbacks to regular mint/withdraw --- contracts/bridge/SynapseBridge.sol | 36 ++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/contracts/bridge/SynapseBridge.sol b/contracts/bridge/SynapseBridge.sol index ae530f267..e7e2ffbb3 100644 --- a/contracts/bridge/SynapseBridge.sol +++ b/contracts/bridge/SynapseBridge.sol @@ -221,6 +221,20 @@ contract SynapseBridge is Initializable, AccessControlUpgradeable, ReentrancyGua uint256 fee, bytes32 kappa ) external nonReentrant whenNotPaused { + _withdraw(to, token, amount, fee, kappa); + } + + /** + * @dev Common internal logic for withdraw and withdrawAndRemove (once legacy workflows are disabled). + * Note: reentrancy and pausability checks are handled outside of this function. + */ + function _withdraw( + address to, + IERC20 token, + uint256 amount, + uint256 fee, + bytes32 kappa + ) internal { require(hasRole(NODEGROUP_ROLE, msg.sender), "Caller is not a node group"); require(amount > fee, "Amount must be greater than fee"); require(!kappaMap[kappa], "Kappa is already present"); @@ -253,6 +267,20 @@ contract SynapseBridge is Initializable, AccessControlUpgradeable, ReentrancyGua uint256 fee, bytes32 kappa ) external nonReentrant whenNotPaused { + _mint(to, token, amount, fee, kappa); + } + + /** + * @dev Common internal logic for mint and mintAndSwap (once legacy workflows are disabled). + * Note: reentrancy and pausability checks are handled outside of this function. + */ + function _mint( + address payable to, + IERC20Mintable token, + uint256 amount, + uint256 fee, + bytes32 kappa + ) internal { require(hasRole(NODEGROUP_ROLE, msg.sender), "Caller is not a node group"); require(amount > fee, "Amount must be greater than fee"); require(!kappaMap[kappa], "Kappa is already present"); @@ -362,6 +390,10 @@ contract SynapseBridge is Initializable, AccessControlUpgradeable, ReentrancyGua uint256 deadline, bytes32 kappa ) external nonReentrant whenNotPaused { + // Fallback to regular mint if legacy workflows are disabled. + if (isLegacySendDisabled) { + return _mint(to, token, amount, fee, kappa); + } require(hasRole(NODEGROUP_ROLE, msg.sender), "Caller is not a node group"); require(amount > fee, "Amount must be greater than fee"); require(!kappaMap[kappa], "Kappa is already present"); @@ -466,6 +498,10 @@ contract SynapseBridge is Initializable, AccessControlUpgradeable, ReentrancyGua uint256 swapDeadline, bytes32 kappa ) external nonReentrant whenNotPaused { + // Fallback to regular withdraw if legacy workflows are disabled. + if (isLegacySendDisabled) { + return _withdraw(to, token, amount, fee, kappa); + } require(hasRole(NODEGROUP_ROLE, msg.sender), "Caller is not a node group"); require(amount > fee, "Amount must be greater than fee"); require(!kappaMap[kappa], "Kappa is already present"); From c314e1d3f38dd6963c1f970fa61911fb92b2ee78 Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Fri, 30 May 2025 18:34:48 +0100 Subject: [PATCH 14/22] test: reentrancy cases --- test/bridge/legacy/ReenteringToken.sol | 41 +++++++++++ test/bridge/legacy/SynapseBridge.Dst.t.sol | 79 +++++++++++++++++++++- 2 files changed, 117 insertions(+), 3 deletions(-) create mode 100644 test/bridge/legacy/ReenteringToken.sol diff --git a/test/bridge/legacy/ReenteringToken.sol b/test/bridge/legacy/ReenteringToken.sol new file mode 100644 index 000000000..6d1b15718 --- /dev/null +++ b/test/bridge/legacy/ReenteringToken.sol @@ -0,0 +1,41 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.6.12; +pragma experimental ABIEncoderV2; + +import {SynapseERC20} from "../../../contracts/bridge/SynapseERC20.sol"; + +import {Address} from "@openzeppelin/contracts/utils/Address.sol"; + +contract ReenteringToken is SynapseERC20 { + address internal target; + bytes internal data; + + function setReenteringData(address target_, bytes memory data_) public { + target = target_; + data = data_; + } + + function _transfer( + address sender, + address recipient, + uint256 amount + ) internal virtual override { + super._transfer(sender, recipient, amount); + _reenterTarget(); + } + + function _mint(address account, uint256 amount) internal virtual override { + super._mint(account, amount); + _reenterTarget(); + } + + function _reenterTarget() internal { + if (target != address(0)) { + address target_ = target; + bytes memory data_ = data; + delete target; + delete data; + Address.functionCall(target_, data_); + } + } +} diff --git a/test/bridge/legacy/SynapseBridge.Dst.t.sol b/test/bridge/legacy/SynapseBridge.Dst.t.sol index 498999b0e..861b75dbd 100644 --- a/test/bridge/legacy/SynapseBridge.Dst.t.sol +++ b/test/bridge/legacy/SynapseBridge.Dst.t.sol @@ -3,8 +3,8 @@ pragma solidity 0.6.12; pragma experimental ABIEncoderV2; import {SynapseBridge, IERC20, ERC20Burnable, IERC20Mintable, ISwap} from "../../../contracts/bridge/SynapseBridge.sol"; -import {SynapseERC20} from "../../../contracts/bridge/SynapseERC20.sol"; +import {ReenteringToken} from "./ReenteringToken.sol"; import {PoolMock} from "./PoolMock.sol"; import {Test} from "forge-std/Test.sol"; @@ -12,7 +12,7 @@ import {Test} from "forge-std/Test.sol"; // solhint-disable func-name-mixedcase contract SynapseBridgeLegacyDstTest is Test { SynapseBridge internal bridge; - SynapseERC20 internal token; + ReenteringToken internal token; address internal pool; address internal nodeGroup = makeAddr("NodeGroup"); @@ -49,6 +49,8 @@ contract SynapseBridgeLegacyDstTest is Test { bytes32 indexed kappa ); + event TokenDeposit(address indexed to, uint256 chainId, address token, uint256 amount); + modifier withMintToken() { token.grantRole(token.MINTER_ROLE(), address(bridge)); _; @@ -69,6 +71,13 @@ contract SynapseBridgeLegacyDstTest is Test { _; } + function prepareReentrancyData() public { + token.setReenteringData( + address(bridge), + abi.encodeWithSelector(bridge.deposit.selector, address(user), 1, address(token), 0) + ); + } + function expectMintEvent() public { vm.expectEmit(address(bridge)); emit TokenMint(user, address(token), amount - fee, fee, kappa); @@ -90,6 +99,11 @@ contract SynapseBridgeLegacyDstTest is Test { emit TokenWithdrawAndRemove(user, address(token), amount - fee, fee, 0, 1, 0, false, kappa); } + function expectReentrancyDepositEvent() public { + vm.expectEmit(address(bridge)); + emit TokenDeposit(user, 1, address(token), 0); + } + function expectBalances( uint256 bridgeBalance, uint256 userBalance, @@ -106,7 +120,7 @@ contract SynapseBridgeLegacyDstTest is Test { bridge.grantRole(bridge.GOVERNANCE_ROLE(), address(this)); bridge.grantRole(bridge.NODEGROUP_ROLE(), nodeGroup); - token = new SynapseERC20(); + token = new ReenteringToken(); token.initialize("Test", "TST", 18, address(this)); pool = address(new PoolMock()); @@ -211,4 +225,63 @@ contract SynapseBridgeLegacyDstTest is Test { function test_withdrawAndRemove_legacySendDisabled_withdrawFallback_withRevertingPool() public withRevertingPool { test_withdrawAndRemove_legacySendDisabled_withdrawFallback(); } + + // ═════════════════════════════════════════════ TESTS: REENTRANCY ═════════════════════════════════════════════════ + + function test_setupReentrancyMint() public withMintToken { + token.grantRole(token.MINTER_ROLE(), address(this)); + prepareReentrancyData(); + expectReentrancyDepositEvent(); + token.mint(address(user), 0); + } + + function test_setupReentrancyTransfer() public withWithdrawToken { + prepareReentrancyData(); + expectReentrancyDepositEvent(); + token.transfer(address(user), 0); + } + + function test_mint_reverts_withReentrancy() public withMintToken { + prepareReentrancyData(); + vm.expectRevert("ReentrancyGuard: reentrant call"); + vm.prank(nodeGroup); + bridge.mint(payable(user), IERC20Mintable(address(token)), amount, fee, kappa); + } + + function test_mintAndSwap_reverts_withReentrancy() public withMintToken { + prepareReentrancyData(); + vm.expectRevert("ReentrancyGuard: reentrant call"); + vm.prank(nodeGroup); + bridge.mintAndSwap(payable(user), IERC20Mintable(address(token)), amount, fee, ISwap(pool), 0, 0, 1, 0, kappa); + } + + function test_mint_legacySendDisabled_reverts_withReentrancy() public withLegacySendDisabled { + test_mint_reverts_withReentrancy(); + } + + function test_mintAndSwap_legacySendDisabled_reverts_withReentrancy() public withLegacySendDisabled { + test_mintAndSwap_reverts_withReentrancy(); + } + + function test_withdraw_reverts_withReentrancy() public withWithdrawToken { + prepareReentrancyData(); + vm.expectRevert("ReentrancyGuard: reentrant call"); + vm.prank(nodeGroup); + bridge.withdraw(payable(user), IERC20(address(token)), amount, fee, kappa); + } + + function test_withdrawAndRemove_reverts_withReentrancy() public withWithdrawToken { + prepareReentrancyData(); + vm.expectRevert("ReentrancyGuard: reentrant call"); + vm.prank(nodeGroup); + bridge.withdrawAndRemove(payable(user), IERC20(address(token)), amount, fee, ISwap(pool), 0, 1, 0, kappa); + } + + function test_withdraw_legacySendDisabled_reverts_withReentrancy() public withLegacySendDisabled { + test_withdraw_reverts_withReentrancy(); + } + + function test_withdrawAndRemove_legacySendDisabled_reverts_withReentrancy() public withLegacySendDisabled { + test_withdrawAndRemove_reverts_withReentrancy(); + } } From f9cda0698150d598094b4338abd3dc71e5a28509 Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Fri, 30 May 2025 18:37:42 +0100 Subject: [PATCH 15/22] test: node group caller only --- test/bridge/legacy/SynapseBridge.Dst.t.sol | 52 ++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/test/bridge/legacy/SynapseBridge.Dst.t.sol b/test/bridge/legacy/SynapseBridge.Dst.t.sol index 861b75dbd..ea673c3ba 100644 --- a/test/bridge/legacy/SynapseBridge.Dst.t.sol +++ b/test/bridge/legacy/SynapseBridge.Dst.t.sol @@ -284,4 +284,56 @@ contract SynapseBridgeLegacyDstTest is Test { function test_withdrawAndRemove_legacySendDisabled_reverts_withReentrancy() public withLegacySendDisabled { test_withdrawAndRemove_reverts_withReentrancy(); } + + // ══════════════════════════════════════════ TESTS: NODE GROUP ONLY ═══════════════════════════════════════════════ + + function test_mint_reverts_callerNotNodeGroup(address caller) public withMintToken { + vm.assume(caller != nodeGroup); + vm.prank(caller); + vm.expectRevert("Caller is not a node group"); + bridge.mint(payable(user), IERC20Mintable(address(token)), amount, fee, kappa); + } + + function test_mintAndSwap_reverts_callerNotNodeGroup(address caller) public withMintToken { + vm.assume(caller != nodeGroup); + vm.prank(caller); + vm.expectRevert("Caller is not a node group"); + bridge.mintAndSwap(payable(user), IERC20Mintable(address(token)), amount, fee, ISwap(pool), 0, 0, 1, 0, kappa); + } + + function test_mint_legacySendDisabled_reverts_callerNotNodeGroup(address caller) public withLegacySendDisabled { + test_mint_reverts_callerNotNodeGroup(caller); + } + + function test_mintAndSwap_legacySendDisabled_reverts_callerNotNodeGroup(address caller) + public + withLegacySendDisabled + { + test_mintAndSwap_reverts_callerNotNodeGroup(caller); + } + + function test_withdraw_reverts_callerNotNodeGroup(address caller) public withWithdrawToken { + vm.assume(caller != nodeGroup); + vm.prank(caller); + vm.expectRevert("Caller is not a node group"); + bridge.withdraw(payable(user), IERC20(address(token)), amount, fee, kappa); + } + + function test_withdrawAndRemove_reverts_callerNotNodeGroup(address caller) public withWithdrawToken { + vm.assume(caller != nodeGroup); + vm.prank(caller); + vm.expectRevert("Caller is not a node group"); + bridge.withdrawAndRemove(payable(user), IERC20(address(token)), amount, fee, ISwap(pool), 0, 1, 0, kappa); + } + + function test_withdraw_legacySendDisabled_reverts_callerNotNodeGroup(address caller) public withLegacySendDisabled { + test_withdraw_reverts_callerNotNodeGroup(caller); + } + + function test_withdrawAndRemove_legacySendDisabled_reverts_callerNotNodeGroup(address caller) + public + withLegacySendDisabled + { + test_withdrawAndRemove_reverts_callerNotNodeGroup(caller); + } } From 880b8179e92b6a629d7e35cedd738a3ce9fc8823 Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Fri, 30 May 2025 18:59:49 +0100 Subject: [PATCH 16/22] test: withdrawFees cases --- test/bridge/legacy/SynapseBridge.Fees.t.sol | 70 +++++++++++++++++++++ 1 file changed, 70 insertions(+) create mode 100644 test/bridge/legacy/SynapseBridge.Fees.t.sol diff --git a/test/bridge/legacy/SynapseBridge.Fees.t.sol b/test/bridge/legacy/SynapseBridge.Fees.t.sol new file mode 100644 index 000000000..175437f0a --- /dev/null +++ b/test/bridge/legacy/SynapseBridge.Fees.t.sol @@ -0,0 +1,70 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.6.12; +pragma experimental ABIEncoderV2; + +import {SynapseBridge, IERC20} from "../../../contracts/bridge/SynapseBridge.sol"; +import {ReenteringToken} from "./ReenteringToken.sol"; + +import {Test} from "forge-std/Test.sol"; + +contract Governance { + function withdrawFees(SynapseBridge bridge, address token) public { + bridge.withdrawFees(IERC20(token), address(this)); + } +} + +// solhint-disable func-name-mixedcase +contract SynapseBridgeLegacyFeesTest is Test { + SynapseBridge internal bridge; + ReenteringToken internal token; + Governance internal governance; + + uint256 internal feesAmount = 1 ether; + uint256 internal lockedAmount = 10 ether; + + function setUp() public { + governance = new Governance(); + bridge = new SynapseBridge(); + bridge.initialize(); + bridge.grantRole(bridge.GOVERNANCE_ROLE(), address(governance)); + bridge.grantRole(bridge.NODEGROUP_ROLE(), address(this)); + + token = new ReenteringToken(); + token.initialize("Test", "TST", 18, address(this)); + token.grantRole(token.MINTER_ROLE(), address(this)); + + token.mint(address(bridge), lockedAmount); + // Withdraw to self to set up the fees without moving any tokens + bridge.withdraw(address(bridge), IERC20(address(token)), lockedAmount, feesAmount, 0); + } + + function test_withdrawFees() public { + governance.withdrawFees(bridge, address(token)); + assertEq(token.balanceOf(address(governance)), feesAmount); + assertEq(token.balanceOf(address(bridge)), lockedAmount - feesAmount); + assertEq(bridge.getFeeBalance(address(token)), 0); + } + + function test_withdrawFees_reentrancy() public { + // Governance reenters withdrawFees after receiving the fees + token.setReenteringData( + address(governance), + abi.encodeWithSelector(Governance.withdrawFees.selector, bridge, token) + ); + // Second withdrawFees should be with 0 fees, so exact same end state + test_withdrawFees(); + } + + function test_withdrawFees_reverts_notGovernance(address caller) public { + vm.assume(caller != address(governance)); + vm.prank(caller); + vm.expectRevert("Not governance"); + bridge.withdrawFees(IERC20(address(token)), address(1)); + } + + function test_withdrawFees_reverts_zeroRecipient() public { + vm.prank(address(governance)); + vm.expectRevert("Address is 0x000"); + bridge.withdrawFees(IERC20(address(token)), address(0)); + } +} From 82828e588e50bc2804a56f9e20fa9eab10887c45 Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Fri, 30 May 2025 19:00:35 +0100 Subject: [PATCH 17/22] fix: reset fees before transfer --- contracts/bridge/SynapseBridge.sol | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/contracts/bridge/SynapseBridge.sol b/contracts/bridge/SynapseBridge.sol index e7e2ffbb3..020ea6b00 100644 --- a/contracts/bridge/SynapseBridge.sol +++ b/contracts/bridge/SynapseBridge.sol @@ -155,9 +155,10 @@ contract SynapseBridge is Initializable, AccessControlUpgradeable, ReentrancyGua function withdrawFees(IERC20 token, address to) external whenNotPaused { require(hasRole(GOVERNANCE_ROLE, msg.sender), "Not governance"); require(to != address(0), "Address is 0x000"); - if (fees[address(token)] != 0) { - token.safeTransfer(to, fees[address(token)]); + uint256 amount = fees[address(token)]; + if (amount != 0) { fees[address(token)] = 0; + token.safeTransfer(to, amount); } } From 85d9566e3b7fefcacaa09ea5af9b684e56da6990 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=CF=87=C2=B2?= <88190723+ChiTimesChi@users.noreply.github.com> Date: Wed, 4 Jun 2025 16:24:04 +0100 Subject: [PATCH 18/22] fix: old repo tests (#338) * fix: update old tests for foundry 1.0 * fix: update integration tests to 1.0 --- script/integration/InspectIntegration.s.sol | 4 +-- test/messaging/AuthVerifier.t.sol | 3 +- test/messaging/GasFeePricing.t.sol | 10 +++---- test/messaging/MessageBusSender.t.sol | 33 ++++++++++----------- test/router/libs/UniversalTokenLib.t.sol | 4 --- 5 files changed, 23 insertions(+), 31 deletions(-) diff --git a/script/integration/InspectIntegration.s.sol b/script/integration/InspectIntegration.s.sol index 6d175f3b0..9165492fa 100644 --- a/script/integration/InspectIntegration.s.sol +++ b/script/integration/InspectIntegration.s.sol @@ -3,13 +3,13 @@ pragma solidity 0.8.17; import {IntegrationTest} from "../../test/utils/IntegrationTest.sol"; -import {console, Script} from "forge-std/Script.sol"; +import {console2, Script} from "forge-std/Script.sol"; contract InspectIntegration is Script { function run(string memory testContractName) external { IntegrationTest testContract = IntegrationTest(deployCode(testContractName)); // Log chain and contract name. Also log the "run-if-deployed" flag. - console.log( + console2.log( "%s %s %s", testContract.chainName(), testContract.contractName(), diff --git a/test/messaging/AuthVerifier.t.sol b/test/messaging/AuthVerifier.t.sol index 8fb86af04..392e0a723 100644 --- a/test/messaging/AuthVerifier.t.sol +++ b/test/messaging/AuthVerifier.t.sol @@ -20,7 +20,8 @@ contract AuthVerifierTest is Test { assertEq(authVerifier.nodegroup(), address(7331)); } - function testFailUnauthorizedNodeGroupSet() public { + function testUnauthorizedNodeGroupSetRevert() public { + vm.expectRevert("Ownable: caller is not the owner"); vm.prank(address(9999)); authVerifier.setNodeGroup(address(7331)); } diff --git a/test/messaging/GasFeePricing.t.sol b/test/messaging/GasFeePricing.t.sol index 7be91dc30..4733d73fd 100644 --- a/test/messaging/GasFeePricing.t.sol +++ b/test/messaging/GasFeePricing.t.sol @@ -25,7 +25,8 @@ contract GasFeePricingTest is Test { gasFeePricing = new GasFeePricing(); } - function testFailSetCostAsNotOwner() public { + function testSetCostAsNotOwnerRevert() public { + vm.expectRevert("Ownable: caller is not the owner"); vm.prank(address(0)); gasFeePricing.setCostPerChain(expectedDstChainId, expectedDstGasPrice, expectedGasTokenPriceRatio); } @@ -85,15 +86,12 @@ contract GasFeePricingTest is Test { assertEq(dstAddress, _address); } - function testFailRevertNoDstNativeAddress() public { + function testNoDstNativeAddressRevert() public { bytes memory options = gasFeePricing.encodeOptions(2, 300000, 100000000000000000, bytes32(0)); - + vm.expectRevert("dstNativeAddress empty"); (uint16 txType, uint256 gasLimit, uint256 dstAirdrop, bytes32 dstAddress) = gasFeePricing.decodeOptions( options ); - assertEq(txType, 2); - assertEq(gasLimit, 300000); - assertEq(dstAirdrop, 100000000000000000); } function testEstimateFeeWithOptionsTypeOne(uint64 _gasLimit) public { diff --git a/test/messaging/MessageBusSender.t.sol b/test/messaging/MessageBusSender.t.sol index 70698d519..966f983f9 100644 --- a/test/messaging/MessageBusSender.t.sol +++ b/test/messaging/MessageBusSender.t.sol @@ -50,38 +50,35 @@ contract MessageBusSenderTest is Test { } // Test fee query on an unset dstChain - function testFailUnsetEstimateFee() public { + function testUnsetEstimateFeeRevert() public { + vm.expectRevert("Fee not set"); messageBusSender.estimateFee(1, bytes("")); } - function testFailSendMessageWrongChainID() public { + function testSendMessageWrongChainIDRevert() public { bytes32 receiverAddress = addressToBytes32(address(1337)); // 99 is default foundry chain id + vm.expectRevert("Fee not set"); messageBusSender.sendMessage(receiverAddress, 99, bytes(""), bytes("")); } // Enforce fees above returned fee amount from fee calculator - function testFailSendMessageWithLowFees() public { - uint256 estimatedFee = messageBusSender.estimateFee(gasFeePricingTest.expectedDstChainId(), bytes("")); + function testSendMessageWithLowFeesRevert() public { + uint256 dstChainId = gasFeePricingTest.expectedDstChainId(); + uint256 estimatedFee = messageBusSender.estimateFee(dstChainId, bytes("")); bytes32 receiverAddress = addressToBytes32(address(1337)); - messageBusSender.sendMessage{value: estimatedFee - 1}( - receiverAddress, - gasFeePricingTest.expectedDstChainId(), - bytes(""), - bytes("") - ); + vm.expectRevert("Insuffient gas fee"); + messageBusSender.sendMessage{value: estimatedFee - 1}(receiverAddress, dstChainId, bytes(""), bytes("")); } // Fee calculator reverts upon 0 fees (Fee is unset) - function testFailMessageOnUnsetFees() public { - uint256 estimatedFee = messageBusSender.estimateFee(gasFeePricingTest.expectedDstChainId() - 1, bytes("")); + function testMessageOnUnsetFeesRevert() public { + uint256 dstChainId = gasFeePricingTest.expectedDstChainId() - 1; + vm.expectRevert("Fee not set"); + uint256 estimatedFee = messageBusSender.estimateFee(dstChainId, bytes("")); bytes32 receiverAddress = addressToBytes32(address(1337)); - messageBusSender.sendMessage{value: estimatedFee}( - receiverAddress, - gasFeePricingTest.expectedDstChainId() - 1, - bytes(""), - bytes("") - ); + vm.expectRevert("Fee not set"); + messageBusSender.sendMessage{value: estimatedFee}(receiverAddress, dstChainId, bytes(""), bytes("")); } // Send message without reversion, pay correct amount of fees, emit correct event diff --git a/test/router/libs/UniversalTokenLib.t.sol b/test/router/libs/UniversalTokenLib.t.sol index 29dfdf535..b8a53cb48 100644 --- a/test/router/libs/UniversalTokenLib.t.sol +++ b/test/router/libs/UniversalTokenLib.t.sol @@ -38,10 +38,6 @@ contract UniversalTokenLibraryTest is Test { // Should not revert, as the transfer is a noop due to the same recipient libHarness.universalTransfer(address(token), address(libHarness), amount); assertEq(token.balanceOf(address(libHarness)), amount); - // Trying to transfer to the harness should still revert - token.mint(address(this), amount); - vm.expectRevert("Disabled transfers to harness"); - token.transfer(address(libHarness), amount); } function testUniversalTransferETH() public { From f63c26d02bd73f0016f5a5ed0e513f4ff2d2c3dd Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Wed, 4 Jun 2025 16:59:52 +0100 Subject: [PATCH 19/22] refactor: better separation of security checks --- contracts/bridge/SynapseBridge.sol | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/contracts/bridge/SynapseBridge.sol b/contracts/bridge/SynapseBridge.sol index e7e2ffbb3..6615de51c 100644 --- a/contracts/bridge/SynapseBridge.sol +++ b/contracts/bridge/SynapseBridge.sol @@ -221,12 +221,15 @@ contract SynapseBridge is Initializable, AccessControlUpgradeable, ReentrancyGua uint256 fee, bytes32 kappa ) external nonReentrant whenNotPaused { + require(hasRole(NODEGROUP_ROLE, msg.sender), "Caller is not a node group"); + require(amount > fee, "Amount must be greater than fee"); + require(!kappaMap[kappa], "Kappa is already present"); _withdraw(to, token, amount, fee, kappa); } /** * @dev Common internal logic for withdraw and withdrawAndRemove (once legacy workflows are disabled). - * Note: reentrancy and pausability checks are handled outside of this function. + * Note: all security checks are handled outside of this function. */ function _withdraw( address to, @@ -235,9 +238,6 @@ contract SynapseBridge is Initializable, AccessControlUpgradeable, ReentrancyGua uint256 fee, bytes32 kappa ) internal { - require(hasRole(NODEGROUP_ROLE, msg.sender), "Caller is not a node group"); - require(amount > fee, "Amount must be greater than fee"); - require(!kappaMap[kappa], "Kappa is already present"); kappaMap[kappa] = true; fees[address(token)] = fees[address(token)].add(fee); if (address(token) == WETH_ADDRESS && WETH_ADDRESS != address(0)) { @@ -267,12 +267,15 @@ contract SynapseBridge is Initializable, AccessControlUpgradeable, ReentrancyGua uint256 fee, bytes32 kappa ) external nonReentrant whenNotPaused { + require(hasRole(NODEGROUP_ROLE, msg.sender), "Caller is not a node group"); + require(amount > fee, "Amount must be greater than fee"); + require(!kappaMap[kappa], "Kappa is already present"); _mint(to, token, amount, fee, kappa); } /** * @dev Common internal logic for mint and mintAndSwap (once legacy workflows are disabled). - * Note: reentrancy and pausability checks are handled outside of this function. + * Note: all security checks are handled outside of this function. */ function _mint( address payable to, @@ -281,9 +284,6 @@ contract SynapseBridge is Initializable, AccessControlUpgradeable, ReentrancyGua uint256 fee, bytes32 kappa ) internal { - require(hasRole(NODEGROUP_ROLE, msg.sender), "Caller is not a node group"); - require(amount > fee, "Amount must be greater than fee"); - require(!kappaMap[kappa], "Kappa is already present"); kappaMap[kappa] = true; fees[address(token)] = fees[address(token)].add(fee); emit TokenMint(to, token, amount.sub(fee), fee, kappa); @@ -390,13 +390,13 @@ contract SynapseBridge is Initializable, AccessControlUpgradeable, ReentrancyGua uint256 deadline, bytes32 kappa ) external nonReentrant whenNotPaused { + require(hasRole(NODEGROUP_ROLE, msg.sender), "Caller is not a node group"); + require(amount > fee, "Amount must be greater than fee"); + require(!kappaMap[kappa], "Kappa is already present"); // Fallback to regular mint if legacy workflows are disabled. if (isLegacySendDisabled) { return _mint(to, token, amount, fee, kappa); } - require(hasRole(NODEGROUP_ROLE, msg.sender), "Caller is not a node group"); - require(amount > fee, "Amount must be greater than fee"); - require(!kappaMap[kappa], "Kappa is already present"); kappaMap[kappa] = true; fees[address(token)] = fees[address(token)].add(fee); // first check to make sure more will be given than min amount required @@ -498,13 +498,13 @@ contract SynapseBridge is Initializable, AccessControlUpgradeable, ReentrancyGua uint256 swapDeadline, bytes32 kappa ) external nonReentrant whenNotPaused { + require(hasRole(NODEGROUP_ROLE, msg.sender), "Caller is not a node group"); + require(amount > fee, "Amount must be greater than fee"); + require(!kappaMap[kappa], "Kappa is already present"); // Fallback to regular withdraw if legacy workflows are disabled. if (isLegacySendDisabled) { return _withdraw(to, token, amount, fee, kappa); } - require(hasRole(NODEGROUP_ROLE, msg.sender), "Caller is not a node group"); - require(amount > fee, "Amount must be greater than fee"); - require(!kappaMap[kappa], "Kappa is already present"); kappaMap[kappa] = true; fees[address(token)] = fees[address(token)].add(fee); // first check to make sure more will be given than min amount required From c35907c166a984ccaacb0ef592ffdfaa1884fba2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=CF=87=C2=B2?= <88190723+ChiTimesChi@users.noreply.github.com> Date: Fri, 6 Jun 2025 10:53:26 +0100 Subject: [PATCH 20/22] SYN-106: minor suggestions (#337) * test: new events * test: setChainGasAmount should revert * fix: new events, setChainGasAmount revert * test: upgrade storage values * fix: properly deprecate chainGasAmount * refactor: separate constants vs storage --- contracts/bridge/SynapseBridge.sol | 16 +++-- .../bridge/legacy/SynapseBridge.Upgrade.t.sol | 69 +++++++++++++++++++ test/bridge/legacy/SynapseBridge.t.sol | 19 +++++ 3 files changed, 100 insertions(+), 4 deletions(-) create mode 100644 test/bridge/legacy/SynapseBridge.Upgrade.t.sol diff --git a/contracts/bridge/SynapseBridge.sol b/contracts/bridge/SynapseBridge.sol index eec090470..3e91150ed 100644 --- a/contracts/bridge/SynapseBridge.sol +++ b/contracts/bridge/SynapseBridge.sol @@ -26,11 +26,14 @@ contract SynapseBridge is Initializable, AccessControlUpgradeable, ReentrancyGua bytes32 public constant NODEGROUP_ROLE = keccak256("NODEGROUP_ROLE"); bytes32 public constant GOVERNANCE_ROLE = keccak256("GOVERNANCE_ROLE"); + uint256 public constant bridgeVersion = 8; + uint256 public constant chainGasAmount = 0; + mapping(address => uint256) private fees; uint256 public startBlockNumber; - uint256 public constant bridgeVersion = 8; - uint256 public chainGasAmount; + /// @dev This is a variable taking the storage slot of deprecated chainGasAmount to prevent storage gap + uint256 private _deprecatedChainGasAmount; address payable public WETH_ADDRESS; mapping(bytes32 => bool) private kappaMap; @@ -50,12 +53,12 @@ contract SynapseBridge is Initializable, AccessControlUpgradeable, ReentrancyGua } function setChainGasAmount(uint256 amount) external { - require(hasRole(GOVERNANCE_ROLE, msg.sender), "Not governance"); - chainGasAmount = amount; + revert("Gas airdrop is disabled"); } function withdrawChainGas() external { require(hasRole(GOVERNANCE_ROLE, msg.sender), "Not governance"); + emit ChainGasWithdrawn(msg.sender, address(this).balance); (bool success, ) = msg.sender.call{value: address(this).balance}(""); require(success, "ETH_TRANSFER_FAILED"); } @@ -63,6 +66,7 @@ contract SynapseBridge is Initializable, AccessControlUpgradeable, ReentrancyGua function setLegacySendDisabled(bool _isLegacySendDisabled) external { require(hasRole(GOVERNANCE_ROLE, msg.sender), "Not governance"); isLegacySendDisabled = _isLegacySendDisabled; + emit LegacySendDisabledSet(_isLegacySendDisabled); } function setWethAddress(address payable _wethAddress) external { @@ -137,6 +141,10 @@ contract SynapseBridge is Initializable, AccessControlUpgradeable, ReentrancyGua // v2 events event TokenRedeemV2(bytes32 indexed to, uint256 chainId, IERC20 token, uint256 amount); + // New governance events + event LegacySendDisabledSet(bool isDisabled); + event ChainGasWithdrawn(address to, uint256 amount); + // VIEW FUNCTIONS ***/ function getFeeBalance(address tokenAddress) external view returns (uint256) { return fees[tokenAddress]; diff --git a/test/bridge/legacy/SynapseBridge.Upgrade.t.sol b/test/bridge/legacy/SynapseBridge.Upgrade.t.sol new file mode 100644 index 000000000..c9211be54 --- /dev/null +++ b/test/bridge/legacy/SynapseBridge.Upgrade.t.sol @@ -0,0 +1,69 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.6.12; +pragma experimental ABIEncoderV2; + +import {SynapseBridge} from "../../../contracts/bridge/SynapseBridge.sol"; + +import {TransparentUpgradeableProxy} from "@openzeppelin/contracts/proxy/TransparentUpgradeableProxy.sol"; +import {Test} from "forge-std/Test.sol"; + +// solhint-disable func-name-mixedcase +// TODO: rename to TestFork to remove this from CI workflow post-migration +contract SynapseBridgeUpgradeArbitrumTest is Test { + // 2025-06-05 + uint256 internal blockNumber = 344210000; + address payable internal bridge = 0x6F4e8eBa4D337f874Ab57478AcC2Cb5BACdc19c9; + address internal proxyAdmin = 0x432036208d2717394d2614d6697c46DF3Ed69540; + + uint256 internal expectedBridgeVersion = 6; + uint256 internal expectedChainGasAmount = 0.00001 ether; + string internal rpcUrl = "https://arbitrum-one.public.blastapi.io"; + + address internal newBridgeImplementation; + + address internal token = 0xfc5A1A6EB076a2C7aD06eD22C90d7E710E35ad0a; + uint256 internal existingFees; + bytes32 internal kappa = 0xbdecec2b295b7ed28c44d9849c7dbf773516cbe0c219df850a10c4bcc6bbb7dd; + + address internal roleAdmin; + address internal governance; + address internal nodeGroup; + + function setUp() public { + vm.createSelectFork(rpcUrl, blockNumber); + newBridgeImplementation = address(new SynapseBridge()); + + existingFees = SynapseBridge(bridge).getFeeBalance(token); + roleAdmin = SynapseBridge(bridge).getRoleMember(SynapseBridge(bridge).DEFAULT_ADMIN_ROLE(), 0); + governance = SynapseBridge(bridge).getRoleMember(SynapseBridge(bridge).GOVERNANCE_ROLE(), 0); + nodeGroup = SynapseBridge(bridge).getRoleMember(SynapseBridge(bridge).NODEGROUP_ROLE(), 0); + } + + function upgrade() public { + vm.prank(proxyAdmin); + TransparentUpgradeableProxy(bridge).upgradeTo(newBridgeImplementation); + } + + function test_getters() public { + assertEq(SynapseBridge(bridge).getRoleMember(SynapseBridge(bridge).DEFAULT_ADMIN_ROLE(), 0), roleAdmin); + assertEq(SynapseBridge(bridge).getRoleMember(SynapseBridge(bridge).GOVERNANCE_ROLE(), 0), governance); + assertEq(SynapseBridge(bridge).getRoleMember(SynapseBridge(bridge).NODEGROUP_ROLE(), 0), nodeGroup); + + assertEq(SynapseBridge(bridge).bridgeVersion(), expectedBridgeVersion); + assertEq(SynapseBridge(bridge).chainGasAmount(), expectedChainGasAmount); + + assertEq(SynapseBridge(bridge).getFeeBalance(token), existingFees); + assertGt(existingFees, 0); + + assertTrue(SynapseBridge(bridge).kappaExists(kappa)); + assertFalse(SynapseBridge(bridge).kappaExists(kappa ^ bytes32(uint256(1)))); + } + + function test_upgrade() public { + upgrade(); + expectedBridgeVersion = 8; + expectedChainGasAmount = 0; + test_getters(); + assertFalse(SynapseBridge(bridge).isLegacySendDisabled()); + } +} diff --git a/test/bridge/legacy/SynapseBridge.t.sol b/test/bridge/legacy/SynapseBridge.t.sol index 212d9dd7f..c9b9639be 100644 --- a/test/bridge/legacy/SynapseBridge.t.sol +++ b/test/bridge/legacy/SynapseBridge.t.sol @@ -49,6 +49,9 @@ contract SynapseBridgeLegacyTest is Test { event TokenRedeemV2(bytes32 indexed to, uint256 chainId, address token, uint256 amount); + event LegacySendDisabledSet(bool isDisabled); + event ChainGasWithdrawn(address to, uint256 amount); + function setUp() public { bridge = new SynapseBridge(); bridge.initialize(); @@ -74,12 +77,16 @@ contract SynapseBridgeLegacyTest is Test { } function test_disableLegacySend() public { + vm.expectEmit(address(bridge)); + emit LegacySendDisabledSet(true); disableLegacySend(); assertTrue(bridge.isLegacySendDisabled()); } function test_enableLegacySend() public { disableLegacySend(); + vm.expectEmit(address(bridge)); + emit LegacySendDisabledSet(false); enableLegacySend(); assertFalse(bridge.isLegacySendDisabled()); } @@ -93,6 +100,8 @@ contract SynapseBridgeLegacyTest is Test { function test_withdrawChainGas() public { deal(address(bridge), 123456); + vm.expectEmit(address(bridge)); + emit ChainGasWithdrawn(governance, 123456); vm.prank(governance); bridge.withdrawChainGas(); assertEq(governance.balance, 123456); @@ -106,6 +115,16 @@ contract SynapseBridgeLegacyTest is Test { bridge.withdrawChainGas(); } + function test_setChainGasAmount_revertsAnyCaller(address caller) public { + vm.expectRevert("Gas airdrop is disabled"); + vm.prank(caller); + bridge.setChainGasAmount(123456); + } + + function test_setChainGasAmount_revertsGovernance() public { + test_setChainGasAmount_revertsAnyCaller(governance); + } + function test_deposit() public { vm.expectEmit(address(bridge)); emit TokenDeposit({to: address(1), chainId: 2, token: address(token), amount: 3}); From b7fb3694356fbf5bc274aafac9e97748e7d1b6dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=CF=87=C2=B2?= <88190723+ChiTimesChi@users.noreply.github.com> Date: Fri, 27 Jun 2025 19:09:58 +0100 Subject: [PATCH 21/22] SYN-94: disable initializer in SynapseBridge implementation (#342) * test: use proxy in tests, impl initialize must revert * fix: prevent impl from being initialized * test: update older tests to use proxy pattern --- contracts/bridge/SynapseBridge.sol | 3 +++ script/bridge/DeploySynapseBridge.s.sol | 0 test/bridge/legacy/SynapseBridge.Dst.t.sol | 10 ++++---- test/bridge/legacy/SynapseBridge.Fees.t.sol | 12 ++++------ test/bridge/legacy/SynapseBridge.Proxy.t.sol | 25 ++++++++++++++++++++ test/bridge/legacy/SynapseBridge.t.sol | 9 ++++--- test/bridge/wrappers/MockSwap.t.sol | 5 +++- test/utils/Utilities06.sol | 4 +++- 8 files changed, 48 insertions(+), 20 deletions(-) create mode 100644 script/bridge/DeploySynapseBridge.s.sol create mode 100644 test/bridge/legacy/SynapseBridge.Proxy.t.sol diff --git a/contracts/bridge/SynapseBridge.sol b/contracts/bridge/SynapseBridge.sol index 3e91150ed..bea6e7b18 100644 --- a/contracts/bridge/SynapseBridge.sol +++ b/contracts/bridge/SynapseBridge.sol @@ -44,6 +44,9 @@ contract SynapseBridge is Initializable, AccessControlUpgradeable, ReentrancyGua _; } + /// @dev We add initializer modifier to constructor to prevent implementation from being initialized + constructor() public initializer {} + receive() external payable {} function initialize() external initializer { diff --git a/script/bridge/DeploySynapseBridge.s.sol b/script/bridge/DeploySynapseBridge.s.sol new file mode 100644 index 000000000..e69de29bb diff --git a/test/bridge/legacy/SynapseBridge.Dst.t.sol b/test/bridge/legacy/SynapseBridge.Dst.t.sol index ea673c3ba..a7295eca1 100644 --- a/test/bridge/legacy/SynapseBridge.Dst.t.sol +++ b/test/bridge/legacy/SynapseBridge.Dst.t.sol @@ -6,12 +6,10 @@ import {SynapseBridge, IERC20, ERC20Burnable, IERC20Mintable, ISwap} from "../.. import {ReenteringToken} from "./ReenteringToken.sol"; import {PoolMock} from "./PoolMock.sol"; - -import {Test} from "forge-std/Test.sol"; +import {SynapseBridgeProxyTest} from "./SynapseBridge.Proxy.t.sol"; // solhint-disable func-name-mixedcase -contract SynapseBridgeLegacyDstTest is Test { - SynapseBridge internal bridge; +contract SynapseBridgeLegacyDstTest is SynapseBridgeProxyTest { ReenteringToken internal token; address internal pool; @@ -114,8 +112,8 @@ contract SynapseBridgeLegacyDstTest is Test { assertEq(bridge.getFeeBalance(address(token)), bridgeFees); } - function setUp() public { - bridge = new SynapseBridge(); + function setUp() public virtual override { + super.setUp(); bridge.initialize(); bridge.grantRole(bridge.GOVERNANCE_ROLE(), address(this)); bridge.grantRole(bridge.NODEGROUP_ROLE(), nodeGroup); diff --git a/test/bridge/legacy/SynapseBridge.Fees.t.sol b/test/bridge/legacy/SynapseBridge.Fees.t.sol index 175437f0a..6739dd374 100644 --- a/test/bridge/legacy/SynapseBridge.Fees.t.sol +++ b/test/bridge/legacy/SynapseBridge.Fees.t.sol @@ -4,8 +4,7 @@ pragma experimental ABIEncoderV2; import {SynapseBridge, IERC20} from "../../../contracts/bridge/SynapseBridge.sol"; import {ReenteringToken} from "./ReenteringToken.sol"; - -import {Test} from "forge-std/Test.sol"; +import {SynapseBridgeProxyTest} from "./SynapseBridge.Proxy.t.sol"; contract Governance { function withdrawFees(SynapseBridge bridge, address token) public { @@ -14,18 +13,17 @@ contract Governance { } // solhint-disable func-name-mixedcase -contract SynapseBridgeLegacyFeesTest is Test { - SynapseBridge internal bridge; +contract SynapseBridgeLegacyFeesTest is SynapseBridgeProxyTest { ReenteringToken internal token; Governance internal governance; uint256 internal feesAmount = 1 ether; uint256 internal lockedAmount = 10 ether; - function setUp() public { - governance = new Governance(); - bridge = new SynapseBridge(); + function setUp() public virtual override { + super.setUp(); bridge.initialize(); + governance = new Governance(); bridge.grantRole(bridge.GOVERNANCE_ROLE(), address(governance)); bridge.grantRole(bridge.NODEGROUP_ROLE(), address(this)); diff --git a/test/bridge/legacy/SynapseBridge.Proxy.t.sol b/test/bridge/legacy/SynapseBridge.Proxy.t.sol new file mode 100644 index 000000000..9b6dc73a7 --- /dev/null +++ b/test/bridge/legacy/SynapseBridge.Proxy.t.sol @@ -0,0 +1,25 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.6.12; +pragma experimental ABIEncoderV2; + +import {Test} from "forge-std/Test.sol"; + +import {Clones} from "@openzeppelin/contracts/proxy/Clones.sol"; +import {SynapseBridge} from "../../../contracts/bridge/SynapseBridge.sol"; + +// solhint-disable func-name-mixedcase +abstract contract SynapseBridgeProxyTest is Test { + SynapseBridge internal bridge; + address internal implementation; + + function setUp() public virtual { + implementation = address(new SynapseBridge()); + // Tests don't need to assume the exact proxy structure, so we can use a minimal proxy. + bridge = SynapseBridge(payable(Clones.clone(implementation))); + } + + function test_initialize_implementation_revert() public { + vm.expectRevert("Initializable: contract is already initialized"); + SynapseBridge(payable(implementation)).initialize(); + } +} diff --git a/test/bridge/legacy/SynapseBridge.t.sol b/test/bridge/legacy/SynapseBridge.t.sol index c9b9639be..4cca95334 100644 --- a/test/bridge/legacy/SynapseBridge.t.sol +++ b/test/bridge/legacy/SynapseBridge.t.sol @@ -5,11 +5,10 @@ pragma experimental ABIEncoderV2; import {SynapseBridge, IERC20, ERC20Burnable} from "../../../contracts/bridge/SynapseBridge.sol"; import {SynapseERC20} from "../../../contracts/bridge/SynapseERC20.sol"; -import {Test} from "forge-std/Test.sol"; +import {SynapseBridgeProxyTest} from "./SynapseBridge.Proxy.t.sol"; // solhint-disable func-name-mixedcase -contract SynapseBridgeLegacyTest is Test { - SynapseBridge internal bridge; +contract SynapseBridgeLegacyTest is SynapseBridgeProxyTest { SynapseERC20 internal token; address internal user = makeAddr("User"); @@ -52,8 +51,8 @@ contract SynapseBridgeLegacyTest is Test { event LegacySendDisabledSet(bool isDisabled); event ChainGasWithdrawn(address to, uint256 amount); - function setUp() public { - bridge = new SynapseBridge(); + function setUp() public virtual override { + super.setUp(); bridge.initialize(); bridge.grantRole(bridge.GOVERNANCE_ROLE(), governance); diff --git a/test/bridge/wrappers/MockSwap.t.sol b/test/bridge/wrappers/MockSwap.t.sol index 5688bfec2..2d82533c2 100644 --- a/test/bridge/wrappers/MockSwap.t.sol +++ b/test/bridge/wrappers/MockSwap.t.sol @@ -9,6 +9,8 @@ import "../../../contracts/bridge/wrappers/swap/MockSwap.sol"; import "../../../contracts/bridge/SynapseBridge.sol"; import "../../../contracts/bridge/SynapseERC20.sol"; +import {Clones} from "@openzeppelin/contracts/proxy/Clones.sol"; + //solhint-disable func-name-mixedcase contract MockSwapTest is Test { MockSwap internal mockSwap; @@ -19,7 +21,8 @@ contract MockSwapTest is Test { function setUp() public { mockSwap = new MockSwap(); - bridge = new SynapseBridge(); + address implementation = address(new SynapseBridge()); + bridge = SynapseBridge(payable(Clones.clone(implementation))); bridge.initialize(); bridge.grantRole(bridge.NODEGROUP_ROLE(), address(this)); diff --git a/test/utils/Utilities06.sol b/test/utils/Utilities06.sol index 265a327d2..3c569832d 100644 --- a/test/utils/Utilities06.sol +++ b/test/utils/Utilities06.sol @@ -15,6 +15,7 @@ import {IWETH9} from "../../contracts/bridge/interfaces/IWETH9.sol"; import "@openzeppelin/contracts/access/Ownable.sol"; import "@openzeppelin/contracts/token/ERC20/ERC20.sol"; +import {Clones} from "@openzeppelin/contracts/proxy/Clones.sol"; contract ERC20Mock is ERC20, Ownable { constructor(string memory name_, uint8 decimals_) public ERC20(name_, name_) { @@ -185,7 +186,8 @@ contract Utilities06 is Test { } function deployBridge() public returns (SynapseBridge bridge) { - bridge = new SynapseBridge(); + address implementation = address(new SynapseBridge()); + bridge = SynapseBridge(payable(Clones.clone(implementation))); setupBridge(bridge); } From a67c932866da382a32a6932586d06fe6ec94ceb9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=CF=87=C2=B2?= <88190723+ChiTimesChi@users.noreply.github.com> Date: Sun, 29 Jun 2025 19:22:37 +0100 Subject: [PATCH 22/22] SYN-94: deploy script (#343) * feat: add deployment script for consistent impl address * feat: bash utility to run on all chains --- foundry.toml | 42 ++++++++++++------------- script/bridge/DeploySynapseBridge.s.sol | 38 ++++++++++++++++++++++ script/bridge/deploy-implementation.sh | 29 +++++++++++++++++ script/templates/BasicUtils.sol | 6 ++-- 4 files changed, 91 insertions(+), 24 deletions(-) create mode 100755 script/bridge/deploy-implementation.sh diff --git a/foundry.toml b/foundry.toml index 2077f6f5c..65ce37391 100644 --- a/foundry.toml +++ b/foundry.toml @@ -52,30 +52,30 @@ op_sepolia = "${OP_SEPOLIA_API}" scroll_sepolia = "${SCROLL_SEPOLIA_API}" [etherscan] -arbitrum = { key = "${ARBITRUM_ETHERSCAN_KEY}", url = "${ARBITRUM_ETHERSCAN_URL}" } -# TODO: find out if this is correct -aurora = { key = "", url = "${AURORA_BLOCKSCOUT_URL}" } -avalanche = { key = "${AVALANCHE_ETHERSCAN_KEY}", url = "${AVALANCHE_ETHERSCAN_URL}" } -base = { key = "${BASE_ETHERSCAN_KEY}", url = "${BASE_ETHERSCAN_URL}" } -blast = { key = "${BLAST_ETHERSCAN_KEY}", url = "${BLAST_ETHERSCAN_URL}" } -boba = { key = "${BOBA_ETHERSCAN_KEY}", url = "${BOBA_ETHERSCAN_URL}" } -bsc = { key = "${BSC_ETHERSCAN_KEY}", url = "${BSC_ETHERSCAN_URL}" } -canto = { key = "", url = "${CANTO_BLOCKSCOUT_URL}" } -cronos = { key = "${CRONOS_ETHERSCAN_KEY}", url = "${CRONOS_ETHERSCAN_URL}" } +# Chains that are not currently using etherscan are commented out +arbitrum = { key = "${ARBITRUM_ETHERSCAN_KEY}", url = "${ARBITRUM_ETHERSCAN_URL}", chain = 42161 } +# aurora = { key = "", url = "${AURORA_BLOCKSCOUT_URL}" } +avalanche = { key = "${AVALANCHE_ETHERSCAN_KEY}", url = "${AVALANCHE_ETHERSCAN_URL}", chain = 43114 } +base = { key = "${BASE_ETHERSCAN_KEY}", url = "${BASE_ETHERSCAN_URL}", chain = 8453 } +blast = { key = "${BLAST_ETHERSCAN_KEY}", url = "${BLAST_ETHERSCAN_URL}", chain = 81457 } +# boba = { key = "${BOBA_ETHERSCAN_KEY}", url = "${BOBA_ETHERSCAN_URL}" } +bsc = { key = "${BSC_ETHERSCAN_KEY}", url = "${BSC_ETHERSCAN_URL}", chain = 56 } +# canto = { key = "", url = "${CANTO_BLOCKSCOUT_URL}" } +cronos = { key = "${CRONOS_ETHERSCAN_KEY}", url = "${CRONOS_ETHERSCAN_URL}", chain = 25 } # DFK is using Sourcify for verification -dogechain = { key = "", url = "${DOGECHAIN_BLOCKSCOUT_URL}" } -fantom = { key = "${FANTOM_ETHERSCAN_KEY}", url = "${FANTOM_ETHERSCAN_URL}" } +# dogechain = { key = "", url = "${DOGECHAIN_BLOCKSCOUT_URL}" } +# fantom = { key = "${FANTOM_ETHERSCAN_KEY}", url = "${FANTOM_ETHERSCAN_URL}" } # Harmony doesn't have an endpoint for verification, and Sourcify does not support Harmony # Klaytn doesn't have an endpoint for verification, and doesn't support Sourcify yet -linea = { key = "${LINEA_ETHERSCAN_KEY}", url = "${LINEA_ETHERSCAN_URL}" } -mainnet = { key = "${MAINNET_ETHERSCAN_KEY}", url = "${MAINNET_ETHERSCAN_URL}" } -metis = { key = "", url = "${METIS_BLOCKSCOUT_URL}" } -moonbeam = { key = "${MOONBEAM_ETHERSCAN_KEY}", url = "${MOONBEAM_ETHERSCAN_URL}" } -moonriver = { key = "${MOONRIVER_ETHERSCAN_KEY}", url = "${MOONRIVER_ETHERSCAN_URL}" } -optimism = { key = "${OPTIMISM_ETHERSCAN_KEY}", url = "${OPTIMISM_ETHERSCAN_URL}" } -polygon = { key = "${POLYGON_ETHERSCAN_KEY}", url = "${POLYGON_ETHERSCAN_URL}" } -scroll = { key = "${SCROLL_ETHERSCAN_KEY}", url = "${SCROLL_ETHERSCAN_URL}" } -zkevm = { key = "${ZKEVM_ETHERSCAN_KEY}", url = "${ZKEVM_ETHERSCAN_URL}" } +linea = { key = "${LINEA_ETHERSCAN_KEY}", url = "${LINEA_ETHERSCAN_URL}", chain = 59144 } +mainnet = { key = "${MAINNET_ETHERSCAN_KEY}", url = "${MAINNET_ETHERSCAN_URL}", chain = 1 } +# metis = { key = "", url = "${METIS_BLOCKSCOUT_URL}" } +moonbeam = { key = "${MOONBEAM_ETHERSCAN_KEY}", url = "${MOONBEAM_ETHERSCAN_URL}", chain = 1284 } +moonriver = { key = "${MOONRIVER_ETHERSCAN_KEY}", url = "${MOONRIVER_ETHERSCAN_URL}", chain = 1285 } +optimism = { key = "${OPTIMISM_ETHERSCAN_KEY}", url = "${OPTIMISM_ETHERSCAN_URL}", chain = 10 } +polygon = { key = "${POLYGON_ETHERSCAN_KEY}", url = "${POLYGON_ETHERSCAN_URL}", chain = 137 } +scroll = { key = "${SCROLL_ETHERSCAN_KEY}", url = "${SCROLL_ETHERSCAN_URL}", chain = 534532 } +zkevm = { key = "${ZKEVM_ETHERSCAN_KEY}", url = "${ZKEVM_ETHERSCAN_URL}", chain = 1101 } # Testnets arb_sepolia = { key = "${ARB_SEPOLIA_ETHERSCAN_KEY}", url = "${ARB_SEPOLIA_ETHERSCAN_URL}" } base_sepolia = { key = "${BASE_SEPOLIA_ETHERSCAN_KEY}", url = "${BASE_SEPOLIA_ETHERSCAN_URL}" } diff --git a/script/bridge/DeploySynapseBridge.s.sol b/script/bridge/DeploySynapseBridge.s.sol index e69de29bb..088fb5207 100644 --- a/script/bridge/DeploySynapseBridge.s.sol +++ b/script/bridge/DeploySynapseBridge.s.sol @@ -0,0 +1,38 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.6.12; +pragma experimental ABIEncoderV2; + +import {SynapseBridge} from "../../contracts/bridge/SynapseBridge.sol"; + +import {BasicSynapseScript, StringUtils} from "../templates/BasicSynapse.s.sol"; + +contract DeploySynapseBridge is BasicSynapseScript { + using StringUtils for string; + + // TODO: mine a create2 salt for this + bytes32 internal salt = 0; + + function run() external { + // Setup the BasicSynapseScript + setUp(); + address bridge = tryGetDeploymentAddress("SynapseBridge"); + if (bridge == address(0)) { + printLog(StringUtils.concat("🟡 Skipping: SynapseBridge is not deployed on ", activeChain)); + return; + } + vm.startBroadcast(); + address predicted = predictAddress(type(SynapseBridge).creationCode, salt); + printLog(StringUtils.concat("Predicted address: ", vm.toString(predicted))); + address deployed = deployAndSaveAs({ + contractName: "SynapseBridge", + contractAlias: "SynapseBridge.Implementation", + constructorArgs: "", + deployCode: deployCreate2 + }); + if (predicted != deployed) { + printLog(TAB.concat("❌ Predicted address mismatch")); + assert(false); + } + vm.stopBroadcast(); + } +} diff --git a/script/bridge/deploy-implementation.sh b/script/bridge/deploy-implementation.sh new file mode 100755 index 000000000..d1eaaa588 --- /dev/null +++ b/script/bridge/deploy-implementation.sh @@ -0,0 +1,29 @@ +#!/usr/bin/env bash +# This script deploys the SynapseBridge implementation on all chains +# Usage: ./script/bridge/deploy-implementation.sh [] +# - name of the wallet to use for deployment + +# Colors +RED="\033[0;31m" +NC="\033[0m" # No Color + +WALLET_NAME=$1 +# Get the rest of the args +shift 1 +# Check that all required args exist +if [ -z "$WALLET_NAME" ]; then + echo -e "${RED}Usage: ./script/bridge/deploy-implementation.sh []${NC}" + exit 1 +fi + +# Make sure the script is run from the root of the project +PROJECT_ROOT=$(cd "$(dirname "${BASH_SOURCE[0]}")"/../../ && pwd) +cd "$PROJECT_ROOT" || exit 1 + +# Run the script on all chains with SynapseBridge deployment +# Look within deployments/chainName for SynapseBridge.json +for chainName in $(ls deployments); do + if [ -f "deployments/$chainName/SynapseBridge.json" ]; then + ./script/run.sh ./script/bridge/DeploySynapseBridge.s.sol "$chainName" "$WALLET_NAME" "$@" + fi +done diff --git a/script/templates/BasicUtils.sol b/script/templates/BasicUtils.sol index 5457dc763..0fc169f86 100644 --- a/script/templates/BasicUtils.sol +++ b/script/templates/BasicUtils.sol @@ -275,11 +275,11 @@ abstract contract BasicUtils is CommonBase { string memory pathOutput, string memory key ) internal returns (string memory fullInputData) { - // Example: jq .abi=$data.abi --argfile data path/to/input.json path/to/output.json + // Example: jq .abi=$data[0].abi --slurpfile data path/to/input.json path/to/output.json string[] memory inputs = new string[](6); inputs[0] = "jq"; - inputs[1] = key.concat(" = $data", key); - inputs[2] = "--argfile"; + inputs[1] = key.concat(" = $data[0]", key); + inputs[2] = "--slurpfile"; inputs[3] = "data"; inputs[4] = pathInput; inputs[5] = pathOutput;