diff --git a/contracts/.env.example b/contracts/.env.example index 0f05f5e7..adbfbbe7 100644 --- a/contracts/.env.example +++ b/contracts/.env.example @@ -1,9 +1,21 @@ # PROJECT ID from https://infura.io/dashboard -INFURA_KEY=01117e8ede8e4f36801a6a838b24f36c +INFURA_KEY= -# Private keys +# Private keys for deployers +PRIVATEKEY_OWNER_AMB= +PRIVATEKEY_OWNER_ETH= +PRIVATEKEY_OWNER_BSC= -# Deployer -PRIVATEKEY_OWNER_AMB=34d8e83fca265e9ab5bcc1094fa64e98692375bf8980d066a9edcf4953f0f2f5 -PRIVATEKEY_OWNER_ETH=34d8e83fca265e9ab5bcc1094fa64e98692375bf8980d066a9edcf4953f0f2f5 -PRIVATEKEY_OWNER_BSC=34d8e83fca265e9ab5bcc1094fa64e98692375bf8980d066a9edcf4953f0f2f5 +PRIVATE_KEY_DEV= + + +# verification apis +ETHERSCAN_API_KEY= +BSCSCAN_API_KEY= + + +# for integration tests +PRIVATEKEY_INTEGR1= +PRIVATEKEY_INTEGR2= +PRIVATEKEY_INTEGR3= +PRIVATEKEY_INTEGR4= diff --git a/contracts/contracts/checks/CheckUntrustless2.sol b/contracts/contracts/checks/CheckUntrustless2.sol index 2dbf6938..69896f59 100644 --- a/contracts/contracts/checks/CheckUntrustless2.sol +++ b/contracts/contracts/checks/CheckUntrustless2.sol @@ -6,6 +6,5 @@ import "../common/CommonStructs.sol"; contract CheckUntrustless2 { // this contract do nothing, but i think it's required to inheritance from it - // todo check this uint256[15] private ___gap; } diff --git a/contracts/contracts/checks/SignatureCheck.sol b/contracts/contracts/checks/SignatureCheck.sol deleted file mode 100644 index 1aa2dbb5..00000000 --- a/contracts/contracts/checks/SignatureCheck.sol +++ /dev/null @@ -1,15 +0,0 @@ -// SPDX-License-Identifier: MIT -pragma solidity 0.8.6; - -function ecdsaRecover(bytes32 messageHash, bytes memory signature) pure returns(address) { - bytes32 r; - bytes32 s; - uint8 v; - assembly { - r := mload(add(signature, 32)) - s := mload(add(signature, 64)) - v := byte(0, mload(add(signature, 96))) - if lt(v, 27) {v := add(v, 27)} - } - return ecrecover(messageHash, v, r, s); -} diff --git a/contracts/contracts/common/CommonBridge.sol b/contracts/contracts/common/CommonBridge.sol index 251958d0..83192bb5 100644 --- a/contracts/contracts/common/CommonBridge.sol +++ b/contracts/contracts/common/CommonBridge.sol @@ -7,10 +7,13 @@ import "@openzeppelin/contracts-upgradeable/security/PausableUpgradeable.sol"; import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; import "./CommonStructs.sol"; import "../tokens/IWrapper.sol"; -import "../checks/SignatureCheck.sol"; +import "@openzeppelin/contracts/utils/cryptography/ECDSA.sol"; +import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; contract CommonBridge is Initializable, AccessControlUpgradeable, PausableUpgradeable { + using SafeERC20 for IERC20; + // DEFAULT_ADMIN_ROLE can grants and revokes all roles below; Set to multisig (proxy contract address) bytes32 public constant ADMIN_ROLE = keccak256("ADMIN_ROLE"); // can change tokens; unpause contract; change params like lockTime, minSafetyBlocks, ... bytes32 public constant RELAY_ROLE = keccak256("RELAY_ROLE"); // can submit transfers @@ -95,7 +98,7 @@ contract CommonBridge is Initializable, AccessControlUpgradeable, PausableUpgrad /// @param feeSignature Signature signed by relay that confirms that the fee values are valid function wrapWithdraw(address toAddress, bytes calldata feeSignature, uint transferFee, uint bridgeFee - ) public payable { + ) public payable whenNotPaused { address tokenSideAddress = tokenAddresses[wrapperAddress]; require(tokenSideAddress != address(0), "Unknown token address"); @@ -103,8 +106,10 @@ contract CommonBridge is Initializable, AccessControlUpgradeable, PausableUpgrad uint amount = msg.value - transferFee - bridgeFee; feeCheck(wrapperAddress, feeSignature, transferFee, bridgeFee, amount); - transferFeeRecipient.transfer(transferFee); - bridgeFeeRecipient.transfer(bridgeFee); + (bool sent, ) = payable(transferFeeRecipient).call{value: transferFee}(""); + require(sent, "Transfer failed (transferFee)"); + (sent, ) = payable(bridgeFeeRecipient).call{value: bridgeFee}(""); + require(sent, "Transfer failed (bridgeFee)"); IWrapper(wrapperAddress).deposit{value : amount}(); @@ -129,7 +134,7 @@ contract CommonBridge is Initializable, AccessControlUpgradeable, PausableUpgrad function withdraw( address tokenThisAddress, address toAddress, uint amount, bool unwrapSide, bytes calldata feeSignature, uint transferFee, uint bridgeFee - ) payable public { + ) payable public whenNotPaused { address tokenSideAddress; if (unwrapSide) { require(tokenAddresses[address(0)] == tokenThisAddress, "Token not point to native token"); @@ -144,10 +149,12 @@ contract CommonBridge is Initializable, AccessControlUpgradeable, PausableUpgrad require(amount > 0, "Cannot withdraw 0"); feeCheck(tokenThisAddress, feeSignature, transferFee, bridgeFee, amount); - transferFeeRecipient.transfer(transferFee); - bridgeFeeRecipient.transfer(bridgeFee); + (bool sent, ) = payable(transferFeeRecipient).call{value: transferFee}(""); + require(sent, "Transfer failed (transferFee)"); + (sent, ) = payable(bridgeFeeRecipient).call{value: bridgeFee}(""); + require(sent, "Transfer failed (bridgeFee)"); - require(IERC20(tokenThisAddress).transferFrom(msg.sender, address(this), amount), "Fail transfer coins"); + IERC20(tokenThisAddress).safeTransferFrom(msg.sender, address(this), amount); queue.push(CommonStructs.Transfer(tokenSideAddress, toAddress, amount)); emit Withdraw(msg.sender, outputEventId, tokenThisAddress, tokenSideAddress, amount, transferFee, bridgeFee); @@ -156,7 +163,7 @@ contract CommonBridge is Initializable, AccessControlUpgradeable, PausableUpgrad } // can be called to force emit `Transfer` event, without waiting for withdraw in next timeframe - function triggerTransfers() public { + function triggerTransfers() public whenNotPaused { require(queue.length != 0, "Queue is empty"); emit Transfer(outputEventId++, queue); @@ -172,12 +179,13 @@ contract CommonBridge is Initializable, AccessControlUpgradeable, PausableUpgrad require(transfersLocked.endTimestamp > 0, "no locked transfers with this id"); require(transfersLocked.endTimestamp < block.timestamp, "lockTime has not yet passed"); - proceedTransfers(transfersLocked.transfers); - delete lockedTransfers[eventId]; emit TransferFinish(eventId); oldestLockedEventId = eventId + 1; + + // delete lockedTransfers[eventId] first to prevent reentrancy + proceedTransfers(transfersLocked.transfers); } // optimized version of unlockTransfers that unlock all transfer that can be unlocked in one call @@ -187,10 +195,11 @@ contract CommonBridge is Initializable, AccessControlUpgradeable, PausableUpgrad CommonStructs.LockedTransfers memory transfersLocked = lockedTransfers[eventId]; if (transfersLocked.endTimestamp == 0 || transfersLocked.endTimestamp > block.timestamp) break; - proceedTransfers(transfersLocked.transfers); - delete lockedTransfers[eventId]; emit TransferFinish(eventId); + + // delete lockedTransfers[eventId] first to prevent reentrancy + proceedTransfers(transfersLocked.transfers); } oldestLockedEventId = eventId; } @@ -315,6 +324,7 @@ contract CommonBridge is Initializable, AccessControlUpgradeable, PausableUpgrad // submitted transfers saves in `lockedTransfers` for `lockTime` period function lockTransfers(CommonStructs.Transfer[] calldata events, uint eventId) internal { + require(lockedTransfers[eventId].endTimestamp == 0, "lockedTransfers[eventId] not empty"); lockedTransfers[eventId].endTimestamp = block.timestamp + lockTime; for (uint i = 0; i < events.length; i++) lockedTransfers[eventId].transfers.push(events[i]); @@ -328,11 +338,10 @@ contract CommonBridge is Initializable, AccessControlUpgradeable, PausableUpgrad if (transfers[i].tokenAddress == address(0)) {// native token IWrapper(wrapperAddress).withdraw(transfers[i].amount); - payable(transfers[i].toAddress).transfer(transfers[i].amount); + (bool sent, ) = payable(transfers[i].toAddress).call{value: transfers[i].amount}(""); + require(sent, "Transfer failed"); } else {// ERC20 token - require( - IERC20(transfers[i].tokenAddress).transfer(transfers[i].toAddress, transfers[i].amount), - "Fail transfer coins"); + IERC20(transfers[i].tokenAddress).safeTransfer(transfers[i].toAddress, transfers[i].amount); } } @@ -359,12 +368,11 @@ contract CommonBridge is Initializable, AccessControlUpgradeable, PausableUpgrad uint timestampEpoch = block.timestamp / SIGNATURE_FEE_TIMESTAMP; for (uint i = 0; i < signatureFeeCheckNumber; i++) { - messageHash = keccak256(abi.encodePacked( - "\x19Ethereum Signed Message:\n32", + messageHash = ECDSA.toEthSignedMessageHash( keccak256(abi.encodePacked(token, timestampEpoch, transferFee, bridgeFee, amount)) - )); + ); - signer = ecdsaRecover(messageHash, signature); + (signer, ) = ECDSA.tryRecover(messageHash, signature); if (hasRole(FEE_PROVIDER_ROLE, signer)) return; timestampEpoch--; diff --git a/contracts/contracts/contracts_for_tests/BridgeERC20_AmbTest.sol b/contracts/contracts/contracts_for_tests/BridgeERC20_AmbTest.sol deleted file mode 100644 index 3390243b..00000000 --- a/contracts/contracts/contracts_for_tests/BridgeERC20_AmbTest.sol +++ /dev/null @@ -1,19 +0,0 @@ -// SPDX-License-Identifier: MIT -pragma solidity 0.8.6; - -import "../tokens/BridgeERC20_Amb.sol"; - -contract BridgeERC20_AmbTest is BridgeERC20_Amb { - constructor(string memory name_, string memory symbol_, uint8 decimals_, - address[] memory bridgeAddresses_, uint8[] memory sideTokenDecimals_ - ) - BridgeERC20_Amb(name_, symbol_, decimals_, bridgeAddresses_, sideTokenDecimals_) {} - - function mint(address to, uint256 amount) public { - _mint(to, amount); - } - - function changeBridgeBalance(address bridge, uint balance) public { - bridgeBalances[bridge] = balance; - } -} diff --git a/contracts/contracts/contracts_for_tests/CommonBridgeTest.sol b/contracts/contracts/contracts_for_tests/CommonBridgeTest.sol index a8a26237..3d41a23b 100644 --- a/contracts/contracts/contracts_for_tests/CommonBridgeTest.sol +++ b/contracts/contracts/contracts_for_tests/CommonBridgeTest.sol @@ -2,6 +2,7 @@ pragma solidity 0.8.6; import "../common/CommonBridge.sol"; +import "hardhat/console.sol"; contract CommonBridgeTest is CommonBridge { @@ -24,7 +25,7 @@ contract CommonBridgeTest is CommonBridge { function checkSignatureTest(bytes32 hash, bytes memory signature) public view returns(address) { - return ecdsaRecover(hash, signature); + return ECDSA.recover(hash, signature); } diff --git a/contracts/contracts/networks/BSC_AmbBridge.sol b/contracts/contracts/networks/BSC_AmbBridge.sol index e729b63c..d449e6e1 100644 --- a/contracts/contracts/networks/BSC_AmbBridge.sol +++ b/contracts/contracts/networks/BSC_AmbBridge.sol @@ -13,30 +13,6 @@ contract BSC_AmbBridge is CommonBridge, CheckUntrustless2 { __CommonBridge_init(args); } - function upgrade( - address[] calldata _watchdogs, - address _fee_provider, - address mpcRelay, - address oldDefaultAdmin, - address oldRelay - ) public { - require(msg.sender == address(this), "This method require multisig"); - - // add DEFAULT_ADMIN_ROLE to multisig - _setupRole(DEFAULT_ADMIN_ROLE, msg.sender); - - // revoke RELAY_ROLE from old relay - revokeRole(RELAY_ROLE, oldRelay); - - // new roles for untrustless mpc - _setupRoles(WATCHDOG_ROLE, _watchdogs); - _setupRole(FEE_PROVIDER_ROLE, _fee_provider); - _setupRole(RELAY_ROLE, mpcRelay); - - // revoke DEFAULT_ADMIN_ROLE from deployer - revokeRole(DEFAULT_ADMIN_ROLE, oldDefaultAdmin); - } - function submitTransferUntrustless(uint eventId, CommonStructs.Transfer[] calldata transfers) public onlyRole(RELAY_ROLE) whenNotPaused { checkEventId(eventId); diff --git a/contracts/contracts/networks/BSC_BscBridge.sol b/contracts/contracts/networks/BSC_BscBridge.sol index ce72c808..2b3dfe3e 100644 --- a/contracts/contracts/networks/BSC_BscBridge.sol +++ b/contracts/contracts/networks/BSC_BscBridge.sol @@ -13,29 +13,6 @@ contract BSC_BscBridge is CommonBridge, CheckUntrustless2 { __CommonBridge_init(args); } - function upgrade( - address[] calldata _watchdogs, - address _fee_provider, - address mpcRelay, - address oldDefaultAdmin, - address oldRelay - ) public { - require(msg.sender == address(this), "This method require multisig"); - - // add DEFAULT_ADMIN_ROLE to multisig - _setupRole(DEFAULT_ADMIN_ROLE, msg.sender); - - // revoke RELAY_ROLE from old relay - revokeRole(RELAY_ROLE, oldRelay); - - // new roles for untrustless mpc - _setupRoles(WATCHDOG_ROLE, _watchdogs); - _setupRole(FEE_PROVIDER_ROLE, _fee_provider); - _setupRole(RELAY_ROLE, mpcRelay); - - // revoke DEFAULT_ADMIN_ROLE from deployer - revokeRole(DEFAULT_ADMIN_ROLE, oldDefaultAdmin); - } function submitTransferUntrustless(uint eventId, CommonStructs.Transfer[] calldata transfers) public onlyRole(RELAY_ROLE) whenNotPaused { checkEventId(eventId); diff --git a/contracts/contracts/networks/ETH_AmbBridge.sol b/contracts/contracts/networks/ETH_AmbBridge.sol index de5e1426..c576eadf 100644 --- a/contracts/contracts/networks/ETH_AmbBridge.sol +++ b/contracts/contracts/networks/ETH_AmbBridge.sol @@ -13,29 +13,6 @@ contract ETH_AmbBridge is CommonBridge, CheckUntrustless2 { __CommonBridge_init(args); } - function upgrade( - address[] calldata _watchdogs, - address _fee_provider, - address mpcRelay, - address oldDefaultAdmin, - address oldRelay - ) public { - require(msg.sender == address(this), "This method require multisig"); - - // add DEFAULT_ADMIN_ROLE to multisig - _setupRole(DEFAULT_ADMIN_ROLE, msg.sender); - - // revoke RELAY_ROLE from old relay - revokeRole(RELAY_ROLE, oldRelay); - - // new roles for untrustless mpc - _setupRoles(WATCHDOG_ROLE, _watchdogs); - _setupRole(FEE_PROVIDER_ROLE, _fee_provider); - _setupRole(RELAY_ROLE, mpcRelay); - - // revoke DEFAULT_ADMIN_ROLE from deployer - revokeRole(DEFAULT_ADMIN_ROLE, oldDefaultAdmin); - } function submitTransferUntrustless(uint eventId, CommonStructs.Transfer[] calldata transfers) public onlyRole(RELAY_ROLE) whenNotPaused { diff --git a/contracts/contracts/networks/ETH_EthBridge.sol b/contracts/contracts/networks/ETH_EthBridge.sol index c09bda4e..0007baaf 100644 --- a/contracts/contracts/networks/ETH_EthBridge.sol +++ b/contracts/contracts/networks/ETH_EthBridge.sol @@ -13,22 +13,6 @@ contract ETH_EthBridge is CommonBridge, CheckUntrustless2 { __CommonBridge_init(args); } - function upgrade( - address[] calldata _watchdogs, - address _fee_provider, - address oldDefaultAdmin - ) public { - require(msg.sender == address(this), "This method require multisig"); - - // new roles for untrustless mpc - _setupRoles(WATCHDOG_ROLE, _watchdogs); - _setupRole(FEE_PROVIDER_ROLE, _fee_provider); - - // add DEFAULT_ADMIN_ROLE to multisig - _setupRole(DEFAULT_ADMIN_ROLE, msg.sender); - // revoke DEFAULT_ADMIN_ROLE from deployer - revokeRole(DEFAULT_ADMIN_ROLE, oldDefaultAdmin); - } function submitTransferUntrustless(uint eventId, CommonStructs.Transfer[] calldata transfers) public onlyRole(RELAY_ROLE) whenNotPaused { checkEventId(eventId); diff --git a/contracts/contracts/tokens/BridgeERC20_Amb.sol b/contracts/contracts/tokens/BridgeERC20_Amb.sol index dc3606f3..efeb5cd0 100644 --- a/contracts/contracts/tokens/BridgeERC20_Amb.sol +++ b/contracts/contracts/tokens/BridgeERC20_Amb.sol @@ -34,7 +34,6 @@ contract BridgeERC20_Amb is ERC20, Ownable { _setSideTokenDecimals(bridgeAddresses_, sideTokenDecimals_); } - // todo check if we need this func function _setSideTokenDecimals(address[] memory bridgeAddresses_, uint8[] memory sideTokenDecimals_) private { require(bridgeAddresses_.length == sideTokenDecimals_.length, "wrong array lengths"); for (uint i = 0; i < bridgeAddresses_.length; i++) diff --git a/contracts/contracts/tokens/BridgeERC20_Amb_OLD.sol b/contracts/contracts/tokens/BridgeERC20_Amb_OLD.sol deleted file mode 100644 index 09c8a356..00000000 --- a/contracts/contracts/tokens/BridgeERC20_Amb_OLD.sol +++ /dev/null @@ -1,94 +0,0 @@ -// SPDX-License-Identifier: MIT -pragma solidity 0.8.6; - -import "@openzeppelin/contracts/token/ERC20/ERC20.sol"; -import "@openzeppelin/contracts/access/Ownable.sol"; - - -// this contract is already deployed on mainnets as USDC and BUSD tokens, but -// it has an unpleasant feature: allowance need to be set with ANOTHER token denomination -// and user will see counterintuitive amount in increaseAllowance() function. -// SO, FRONT SHOULD CHECK FOR USDC AND BUSD TOKENS AND USE THIS LEGACY LOGIC FOR THEM -contract BridgeERC20_Amb_OLD is ERC20, Ownable { - - // decimals of token in side network - // example: - // 0xBSC_Amb => 18 (AMB contract of BSC bridge => 18 decimals) - // 0xETH_Amb => 6 (AMB contract of ETH bridge => 6 decimals) - // now, token will auto convert self _decimals to side _decimals (or vice versa) on bridge transfer - // NOTE: value 0 means that address is not a bridge; DON'T SET NON ZERO VALUES FOR NON BRIDGE ADDRESSES - mapping(address => uint8) public sideTokenDecimals; - - mapping(address => uint) public bridgeBalances; // locked tokens on the side bridge - - uint8 _decimals; - - constructor( - string memory name_, string memory symbol_, uint8 decimals_, - address[] memory bridgeAddresses_, uint8[] memory sideTokenDecimals_ - ) ERC20(name_, symbol_) Ownable() { - _setSideTokenDecimals(bridgeAddresses_, sideTokenDecimals_); - _decimals = decimals_; - } - - function decimals() public view override returns (uint8) { - return _decimals; - } - - function setSideTokenDecimals(address[] memory bridgeAddresses_, uint8[] memory sideTokenDecimals_) public onlyOwner() { - _setSideTokenDecimals(bridgeAddresses_, sideTokenDecimals_); - } - - // todo check if we need this func - function _setSideTokenDecimals(address[] memory bridgeAddresses_, uint8[] memory sideTokenDecimals_) private { - require(bridgeAddresses_.length == sideTokenDecimals_.length, "wrong array lengths"); - for (uint i = 0; i < bridgeAddresses_.length; i++) - sideTokenDecimals[bridgeAddresses_[i]] = sideTokenDecimals_[i]; - } - - function _transfer( - address sender, - address recipient, - uint amount - ) internal virtual override { - // todo events - if (sideTokenDecimals[sender] != 0) { // sender is bridge - // user transfer tokens to ambrosus => need to mint it - - // we receive tokens from network where token have sideTokenDecimals[sender] decimals - // convert amount with SIDE network decimals form to SELF decimals form - uint amount_this = _convertDecimals(amount, sideTokenDecimals[sender], _decimals); - - - // bridge mint money to user; same amount locked on side bridge - bridgeBalances[sender] += amount_this; - - _mint(recipient, amount_this); - } else if (sideTokenDecimals[recipient] != 0) { // recipient is bridge - // user withdraw tokens from ambrosus => need to burn it - - // we transfer tokens to network where token have sideTokenDecimals[sender] decimals - // convert amount with SIDE network decimals form to SELF decimals form - uint amount_this = _convertDecimals(amount, sideTokenDecimals[recipient], _decimals); - - - // user burn tokens; side bridge must have enough tokens to send - require(bridgeBalances[recipient] >= amount_this, "not enough locked tokens on bridge"); - bridgeBalances[recipient] -= amount_this; - - _burn(sender, amount_this); - } else { - super._transfer(sender, recipient, amount); - } - } - - function _convertDecimals(uint256 amount, uint8 dFrom, uint8 dTo) internal pure returns (uint256) { - if (dTo == dFrom) - return amount; - if (dTo > dFrom) - return amount * (10 ** (dTo - dFrom)); - else - return amount / (10 ** (dFrom - dTo)); - } - -} diff --git a/contracts/contracts/tokens/BridgeERC20_Fix.sol b/contracts/contracts/tokens/BridgeERC20_Fix.sol deleted file mode 100644 index 5a864419..00000000 --- a/contracts/contracts/tokens/BridgeERC20_Fix.sol +++ /dev/null @@ -1,14 +0,0 @@ -// SPDX-License-Identifier: MIT -pragma solidity 0.8.6; - -import "./BridgeERC20.sol"; - -contract BridgeERC20_Fix is BridgeERC20 { - constructor(string memory name_, string memory symbol_, uint8 decimals_, address bridgeAddress_, - address[] memory oldAddresses, uint[] memory oldBalances - ) - BridgeERC20(name_, symbol_, decimals_, bridgeAddress_) { - for (uint i = 0; i < oldAddresses.length; i++) - _mint(oldAddresses[i], oldBalances[i]); - } -} diff --git a/contracts/contracts/tokens/sAMB.sol b/contracts/contracts/tokens/sAMB.sol index b2c24c58..70bbb09f 100644 --- a/contracts/contracts/tokens/sAMB.sol +++ b/contracts/contracts/tokens/sAMB.sol @@ -15,9 +15,10 @@ contract sAMB is IWrapper, ERC20 { function withdraw(uint amount) public override { _burn(msg.sender, amount); - payable(msg.sender).transfer(amount); - emit Withdrawal(msg.sender, amount); + + (bool sent, ) = payable(msg.sender).call{value: amount}(""); + require(sent, "Transfer failed"); } } diff --git a/contracts/contracts/utils/Faucet.sol b/contracts/contracts/utils/Faucet.sol index d2350de3..eae025e5 100644 --- a/contracts/contracts/utils/Faucet.sol +++ b/contracts/contracts/utils/Faucet.sol @@ -18,7 +18,8 @@ contract Faucet is AccessControl { function withdraw(address toAddress, uint256 amount) public onlyRole(DEFAULT_ADMIN_ROLE) { require(address(this).balance >= amount, "not enough funds"); - payable(toAddress).transfer(amount); + (bool sent, ) = payable(toAddress).call{value: amount}(""); + require(sent, "Transfer failed"); } receive() external payable {} diff --git a/contracts/contracts/utils/MultiSigWallet.sol b/contracts/contracts/utils/MultiSigWallet.sol index b984f6ca..4b7fd97d 100644 --- a/contracts/contracts/utils/MultiSigWallet.sol +++ b/contracts/contracts/utils/MultiSigWallet.sol @@ -238,7 +238,7 @@ contract MultiSigWallet { let x := mload(0x40) // "Allocate" memory for output (0x40 is where "free memory" pointer is stored by convention) let d := add(data, 32) // First 32 bytes are the padded length of data, so exclude that result := call( - sub(gas(), 36810), // 34710 is the value that solidity is currently emitting // todo + sub(gas(), 36810), // 34710 is the value that solidity is currently emitting // It includes callGas (700) + callVeryLow (3, to pay for SUB) + callValueTransferGas (9000) + // callNewAccountGas (25000, in case the destination address does not exist and needs creating) + // gasSLoadEIP2929 (2100) diff --git a/contracts/contracts/utils/Proxy.sol b/contracts/contracts/utils/Proxy.sol index 32c87a4d..00d71dc8 100644 --- a/contracts/contracts/utils/Proxy.sol +++ b/contracts/contracts/utils/Proxy.sol @@ -5,7 +5,6 @@ import "@openzeppelin/contracts/proxy/Proxy.sol"; import "@openzeppelin/contracts/utils/StorageSlot.sol"; import "@openzeppelin/contracts/utils/Address.sol"; import "./MultiSigWallet.sol"; -import "hardhat/console.sol"; contract ProxyMultiSig is Proxy, MultiSigWallet { diff --git a/contracts/deploy/for_tests.ts b/contracts/deploy/for_tests.ts index 6b1d742f..20163d56 100644 --- a/contracts/deploy/for_tests.ts +++ b/contracts/deploy/for_tests.ts @@ -3,7 +3,7 @@ import {DeployFunction} from "hardhat-deploy/types"; import {ethers} from "hardhat"; const func: DeployFunction = async function (hre: HardhatRuntimeEnvironment) { - const {owner, admin} = await hre.getNamedAccounts(); + const {owner, admin, relay} = await hre.getNamedAccounts(); const {address: mockAddr} = await hre.deployments.deploy("BridgeERC20Test", { from: owner, @@ -12,7 +12,7 @@ const func: DeployFunction = async function (hre: HardhatRuntimeEnvironment) { ethers.constants.AddressZero, // bridgeAddress ], }); - await hre.deployments.deploy("BridgeERC20_AmbTest", { + await hre.deployments.deploy("BridgeERC20_Amb", { from: owner, args: [ "Mock", "Mock", 18, @@ -20,6 +20,12 @@ const func: DeployFunction = async function (hre: HardhatRuntimeEnvironment) { [0], // bridgeDecimals ], }); + await hre.deployments.deploy("MintableERC20", { + from: owner, + args: [ + "Mock", "Mock", 18, + ], + }); const {address: wrapperAddr} = await hre.deployments.deploy("sAMB", { from: owner, @@ -27,8 +33,7 @@ const func: DeployFunction = async function (hre: HardhatRuntimeEnvironment) { }); - await hre.deployments.deploy("CommonBridgeTest", {from: owner}); // can't use calldata in normal constructor, so ... - await hre.deployments.execute("CommonBridgeTest", {from: owner}, "constructor_", { + const commonArgs = { sideBridgeAddress: ethers.constants.AddressZero, relayAddress: ethers.constants.AddressZero, feeProviderAddress: ethers.constants.AddressZero, @@ -41,7 +46,33 @@ const func: DeployFunction = async function (hre: HardhatRuntimeEnvironment) { timeframeSeconds: 14400, lockTime: 1000, minSafetyBlocks: 10, - }); + } + + await hre.deployments.deploy("CommonBridgeTest", {from: owner}); // can't use calldata in normal constructor, so ... + await hre.deployments.execute("CommonBridgeTest", {from: owner}, "constructor_", commonArgs); + + + const realDeploymentsOptions = { + from: owner, + proxy: { + owner: owner, + proxyArgs: ["{implementation}", "{data}", [owner], [1]], + proxyContract: "ProxyMultiSig", + execute: { + init: { + methodName: "initialize", + args: [{...commonArgs, relayAddress: relay}] + } + } + }, + log: true + } + + await hre.deployments.deploy("BSC_AmbBridge", realDeploymentsOptions); + await hre.deployments.deploy("BSC_BscBridge", realDeploymentsOptions); + await hre.deployments.deploy("ETH_AmbBridge", realDeploymentsOptions); + await hre.deployments.deploy("ETH_EthBridge", realDeploymentsOptions); + await hre.deployments.deploy("CheckUntrustlessTest", { from: owner, @@ -57,6 +88,15 @@ const func: DeployFunction = async function (hre: HardhatRuntimeEnvironment) { from: owner, }); + await hre.deployments.deploy("MultiSigWallet", { + from: owner, + args: [ + [owner], + 1 + ], + }); + + await hre.deployments.deploy("ProxyMultiSig", { from: owner, args: [ diff --git a/contracts/hardhat.config.ts b/contracts/hardhat.config.ts index bde96a86..57a07112 100644 --- a/contracts/hardhat.config.ts +++ b/contracts/hardhat.config.ts @@ -9,16 +9,16 @@ import {ethers} from "ethers"; dotenv.config(); -// 0x295C2707319ad4BecA6b5bb4086617fD6F240CfE, used instead of empty PK -const devPK = "34d8e83fca265e9ab5bcc1094fa64e98692375bf8980d066a9edcf4953f0f2f5" -const bscScanApiKey = "NFH875QU828E37MQD7XB3QHFBE4XTC2AKH" +const devPK = process.env.PRIVATE_KEY_DEV || ethers.constants.HashZero; // used for dev networks +const bscScanApiKey = process.env.ETHERSCAN_API_KEY; +const etherScanApiKey = process.env.ETHERSCAN_API_KEY; const config: HardhatUserConfig = { networks: { hardhat: { blockGasLimit: 40000000, // amb value - hardfork: "byzantium", + hardfork: "petersburg", companionNetworks: {amb: 'hardhat'}, initialDate: "13 May 2022 18:10:36 GMT", @@ -26,7 +26,6 @@ const config: HardhatUserConfig = { forking: { enabled: false, url: "https://network.ambrosus-dev.io", - // url: "https://eth-rinkeby.alchemyapi.io/v2/e1F5R9XuWDU2-zCtzaMDg4Ybb5SuoEDA" blockNumber: 0xb00ba, } }, @@ -52,7 +51,7 @@ const config: HardhatUserConfig = { }, "integr/eth": { url: "http://127.0.0.1:8502", - accounts: ["0x51d098d8aee092622149d8f3a79cc7b1ce36ff97fadaa2fbd623c65badeefadc", "e7420b6492b8c876d23cd8a1156e35af4bc5dc5703fb4b79b376cb268a718e2e"], + accounts: [process.env.PRIVATEKEY_INTEGR1 || ethers.constants.HashZero, process.env.PRIVATEKEY_INTEGR2 || ethers.constants.HashZero], tags: ["eth", "integr"], companionNetworks: {amb: 'integr/amb'}, }, @@ -60,29 +59,29 @@ const config: HardhatUserConfig = { "dev/amb": { url: "https://network.ambrosus-dev.io", tags: ["amb", "devnet"], - hardfork: "byzantium", + hardfork: "petersburg", companionNetworks: {eth: 'dev/eth', bsc: 'dev/bsc'}, accounts: [devPK], // todo devPk }, "test/amb": { url: "https://network.ambrosus-test.io", tags: ["amb", "testnet"], - hardfork: "byzantium", + hardfork: "petersburg", companionNetworks: {eth: 'test/eth', bsc: 'test/bsc'}, accounts: [process.env.PRIVATEKEY_OWNER_AMB || ethers.constants.HashZero], }, "main/amb": { url: "https://network.ambrosus.io", tags: ["amb", "mainnet"], - hardfork: "byzantium", + hardfork: "petersburg", companionNetworks: {eth: 'main/eth', bsc: 'main/bsc'}, accounts: [process.env.PRIVATEKEY_OWNER_AMB || ethers.constants.HashZero], }, "integr/amb": { url: "http://127.0.0.1:8545", - accounts: ["0x80f702eb861f36fe8fbbe1a7ccceb04ef7ddef714604010501a5f67c8065d446", "0x5b18f0adcca221f65373b20158f95313ecd51bde42b96a4c16f5eb851576bc06"], + accounts: [process.env.PRIVATEKEY_INTEGR3 || ethers.constants.HashZero, process.env.PRIVATEKEY_INTEGR4 || ethers.constants.HashZero], tags: ["amb", "integr"], - hardfork: "byzantium", + hardfork: "petersburg", }, @@ -131,7 +130,7 @@ const config: HardhatUserConfig = { verify: { etherscan: { - apiKey: "DY4Z86MQ2D9E24C6HB98PTA79EKJ5TQIFX", + apiKey: etherScanApiKey, }, }, @@ -145,7 +144,7 @@ const config: HardhatUserConfig = { runs: 200, // todo bigger }, // Note: for amb deploy - evmVersion: "byzantium" + evmVersion: "petersburg" }, }, { version: "0.4.22", @@ -155,7 +154,7 @@ const config: HardhatUserConfig = { runs: 200, }, // Note: for amb deploy - evmVersion: "byzantium" + evmVersion: "petersburg" }, }, ], diff --git a/contracts/test/bridge-networks.ts b/contracts/test/bridge-networks.ts new file mode 100644 index 00000000..7a098434 --- /dev/null +++ b/contracts/test/bridge-networks.ts @@ -0,0 +1,76 @@ +import { deployments, ethers, getNamedAccounts } from "hardhat"; +import type { Contract, Signer } from "ethers"; + +import chai from "chai"; + +chai.should(); +export const expect = chai.expect; + + +describe("Check Bridge Networks Contracts", () => { + let ownerS: Signer; + let owner: string; + let relayS: Signer; + let relay: string; + + let bridgeAmbBsc: Contract; + let bridgeBscAmb: Contract; + let bridgeAmbEth: Contract; + let bridgeEthAmb: Contract; + + const transfer = [ + "0x0000000000000000000000000000000000000001", + "0x0000000000000000000000000000000000000002", + 3, + ]; + + + before(async () => { + await deployments.fixture(["for_tests"]); + ({ owner, relay } = await getNamedAccounts()); + ownerS = await ethers.getSigner(owner); + relayS = await ethers.getSigner(relay); + + bridgeAmbBsc = await ethers.getContract("BSC_AmbBridge", ownerS); + bridgeBscAmb = await ethers.getContract("BSC_BscBridge", ownerS); + bridgeAmbEth = await ethers.getContract("ETH_AmbBridge", ownerS); + bridgeEthAmb = await ethers.getContract("ETH_EthBridge", ownerS); + }); + + beforeEach(async () => { + await deployments.fixture(["for_tests"]); // reset contracts state + }); + + it("bsc->amb", async function () { + await checkSubmitUntrustless(bridgeBscAmb); + }); + it("amb->bsc", async function () { + await checkSubmitUntrustless(bridgeAmbBsc); + await checkSetSideBridge(bridgeAmbBsc) + }); + + it("eth->amb", async function () { + await checkSubmitUntrustless(bridgeEthAmb); + }); + it("amb->eth", async function () { + await checkSubmitUntrustless(bridgeAmbEth); + await checkSetSideBridge(bridgeAmbEth) + }); + + async function checkSubmitUntrustless(contract: Contract) { + await expect(contract.submitTransferUntrustless(1, [transfer])).to.be.reverted; + + await contract.connect(relayS).submitTransferUntrustless(1, [transfer]); + expect(await contract.inputEventId()).to.eq(1); + } + + + async function checkSetSideBridge(contract: Contract) { + await contract.setSideBridge(bridgeAmbBsc.address); + expect(await contract.sideBridgeAddress()).to.eq(bridgeAmbBsc.address); + await expect(contract.setSideBridge(bridgeAmbBsc.address)).to.be.reverted; + } + + +}); + diff --git a/contracts/test/bridgeErc20_Amb.ts b/contracts/test/bridgeErc20_Amb.ts index 14481bf2..68f6ac4e 100644 --- a/contracts/test/bridgeErc20_Amb.ts +++ b/contracts/test/bridgeErc20_Amb.ts @@ -24,7 +24,7 @@ describe("BridgeERC20_Amb", () => { userS = await ethers.getSigner(user); bridge6S = await ethers.getSigner(bridge6); - ambERC20 = await ethers.getContract("BridgeERC20_AmbTest", ownerS); + ambERC20 = await ethers.getContract("BridgeERC20_Amb", ownerS); }); beforeEach(async () => { diff --git a/contracts/test/common.ts b/contracts/test/common.ts index db104bfe..3297a9da 100644 --- a/contracts/test/common.ts +++ b/contracts/test/common.ts @@ -423,12 +423,11 @@ describe("Common tests", () => { it('Test checkSignature', async () => { - const hash = "0x1d0a6ca42217dc9f0560840b3eb91a3879b836cb7ec5a8055e265a520e6839d0"; - const signature = "0x5c1974f609035dc81319f058a8b9428b7ce26b366fadf9768b8ca19e3014c759467d732731a58a2ad9f3e9efedc56275427cd4a2fd7a6de59007b0bdb2e95f7d00"; - const needAddress = "0xc89C669357D161d57B0b255C94eA96E179999919"; - expect(ethers.utils.recoverAddress(ethers.utils.arrayify(hash), signature)).eq(needAddress); - - expect(await commonBridge.checkSignatureTest(hash, signature)).eq(needAddress); + const message = ethers.utils.arrayify("0x1d0a6ca42217dc9f0560840b3eb91a3879b836cb7ec5a8055e265a520e6839d0"); + const hash = ethers.utils.hashMessage(message); + const signature = await ownerS.signMessage(message); + expect(ethers.utils.recoverAddress(hash, signature)).eq(owner); + expect(await commonBridge.checkSignatureTest(hash, signature)).eq(owner); }); }); diff --git a/contracts/test/mintableErc20.ts b/contracts/test/mintableErc20.ts new file mode 100644 index 00000000..0d861a4b --- /dev/null +++ b/contracts/test/mintableErc20.ts @@ -0,0 +1,35 @@ +import {deployments, ethers, getNamedAccounts} from "hardhat"; +import type {Contract, Signer} from "ethers"; + +import chai from "chai"; + +chai.should(); +export const expect = chai.expect; + +describe("MintableERC20", () => { + let userS: Signer; + let user: string; + let mintableErc20: Contract; + + before(async () => { + await deployments.fixture(["for_tests"]); + ({user} = await getNamedAccounts()); + userS = await ethers.getSigner(user); + mintableErc20 = await ethers.getContract("MintableERC20"); + }); + + beforeEach(async () => { + await deployments.fixture(["for_tests"]); // reset contracts state + }); + + it("should mint", async () => { + await mintableErc20.mint(user, 1); + expect(await mintableErc20.balanceOf(user)).eq(1); + }); + + it("decimals", async () => { + expect(await mintableErc20.decimals()).eq(18); + }); + +}); + diff --git a/contracts/test/multisig.ts b/contracts/test/multisig.ts new file mode 100644 index 00000000..7fc1f57b --- /dev/null +++ b/contracts/test/multisig.ts @@ -0,0 +1,131 @@ +import { deployments, ethers, getNamedAccounts } from "hardhat"; +import { BigNumber, Contract, Signer } from "ethers"; + +import chai from "chai"; + +chai.should(); +export const expect = chai.expect; + + +describe("MultiSig test", () => { + let ownerS: Signer; + let owner: string; + let adminS: Signer; + let admin: string; + let user: string; + let userS: Signer; + + let multisig: Contract; + + + before(async () => { + ({ owner, user, admin } = await getNamedAccounts()); + ownerS = await ethers.getSigner(owner); + adminS = await ethers.getSigner(admin); + userS = await ethers.getSigner(user); + + multisig = await ethers.getContract("MultiSigWallet", ownerS); + }); + + beforeEach(async () => { + await deployments.fixture(["for_tests"]); + }); + + + it("makeTransactionSolo", async () => { + await multisig.connect(ownerS).submitTransaction(user, 1, "0x"); + + expect(await multisig.getTransactionCount(true, true)).to.eq(1); + expect((await multisig.getTransactionIds(0, 1, true, true))[0]).to.eq(BigNumber.from(0)); + + expect(await multisig.getConfirmationCount(0)).to.eq(1); + expect(await multisig.getConfirmations(0)).to.eql([owner] ); + + expect(await multisig.isConfirmed(0)).to.eq(true); + + // can't confirm twice + await expect(multisig.connect(ownerS).confirmTransaction(0)).to.be.reverted; + // can't execute twice + // await expect(multisig.connect(ownerS).executeTransaction(0)).to.be.reverted; + + }); + + it("makeTransactionDuo", async () => { + // can't change requirement to 2 while there is only 1 owner + await submitTx(multisig.populateTransaction.changeRequirement(2)); + expect((await multisig.transactions(latestTxId())).executed).to.be.false; + expect(await multisig.required()).to.eq(1); + + // successfully change requirement to 2 + await submitTx(multisig.populateTransaction.addOwner(admin)); + await submitTx(multisig.populateTransaction.changeRequirement(2)); + expect(await multisig.required()).to.eq(2); + + + // create tx + await multisig.connect(ownerS).submitTransaction(user, 1, "0x"); + const txId = await latestTxId(); + + expect(await multisig.getConfirmationCount(txId)).to.eq(1); + expect(await multisig.isConfirmed(txId)).to.eq(false); + + // can't confirm twice + await expect(multisig.connect(ownerS).confirmTransaction(txId)).to.be.reverted; + + // can't confirm without permissions + await expect(multisig.connect(userS).confirmTransaction(txId)).to.be.reverted; + + await multisig.connect(ownerS).revokeConfirmation(txId); + // can't revoke twice + await expect(multisig.connect(userS).revokeConfirmation(txId)).to.be.reverted; + + + await multisig.connect(adminS).confirmTransaction(txId); + await multisig.connect(ownerS).confirmTransaction(txId); + }); + + + it("changeOwners", async () => { + await expect(multisig.addOwner(admin)).to.be.reverted; + + await submitTx(multisig.populateTransaction.addOwner(admin)); + expect(await multisig.getOwners()).to.eql([owner, admin]); + + // can't add twice + await submitTx(multisig.populateTransaction.addOwner(admin)); + expect((await multisig.transactions(latestTxId())).executed).to.be.false; + expect(await multisig.getOwners()).to.eql([owner, admin]); + + + await submitTx(multisig.populateTransaction.replaceOwner(admin, user)); + expect(await multisig.getOwners()).to.eql([owner, user]); + + + + await submitTx(multisig.populateTransaction.addOwner(admin)); + expect(await multisig.getOwners()).to.eql([owner, user, admin]); + + await submitTx(multisig.populateTransaction.removeOwner(user)); + expect(await multisig.getOwners()).to.eql([owner, admin]); + + // can't remove twice + await submitTx(multisig.populateTransaction.removeOwner(user)) + expect((await multisig.transactions(latestTxId())).executed).to.be.false; + expect(await multisig.getOwners()).to.eql([owner, admin]); + + + + await submitTx(multisig.populateTransaction.removeOwner(admin)); + expect(await multisig.getOwners()).to.eql([owner]); + }); + + + async function submitTx(populateTx: any, value=0) { + return multisig.connect(ownerS).submitTransaction(multisig.address, value, (await populateTx).data); + } + + async function latestTxId() { + return (await multisig.transactionCount()) - 1; + } + +}); diff --git a/contracts/test/multiSigTest.ts b/contracts/test/proxyMultisig.ts similarity index 73% rename from contracts/test/multiSigTest.ts rename to contracts/test/proxyMultisig.ts index 3f0e9972..d0086192 100644 --- a/contracts/test/multiSigTest.ts +++ b/contracts/test/proxyMultisig.ts @@ -1,5 +1,5 @@ -import {deployments, ethers, getNamedAccounts} from "hardhat"; -import type {Contract, Signer} from "ethers"; +import { deployments, ethers, getNamedAccounts } from "hardhat"; +import type { Contract, Signer } from "ethers"; import chai from "chai"; @@ -7,7 +7,7 @@ chai.should(); export const expect = chai.expect; -describe("MultiSig test", () => { +describe("Proxy MultiSig test", () => { let ownerS: Signer; let owner: string; let adminS: Signer; @@ -21,7 +21,7 @@ describe("MultiSig test", () => { before(async () => { - ({owner, user, admin} = await getNamedAccounts()); + ({ owner, user, admin } = await getNamedAccounts()); ownerS = await ethers.getSigner(owner); adminS = await ethers.getSigner(admin); userS = await ethers.getSigner(user); @@ -38,19 +38,14 @@ describe("MultiSig test", () => { it("upgradeTo", async () => { await proxy.upgradeTo(implementation.address); - await expect(proxy.connect(adminS).confirmTransaction(0)). - to.emit(proxy, "Upgraded") + await expect(proxy.connect(adminS).confirmTransaction(0)).to.emit(proxy, "Upgraded") .withArgs(implementation.address); const callData = implementation.interface.encodeFunctionData( implementation.interface.functions["changeValue(bytes4)"], ["0x11223344"] ); - await proxy.submitTransaction( - proxy.address, - 0, - callData - ); + await proxy.submitTransaction(proxy.address, 0, callData); await expect(proxy.connect(adminS).confirmTransaction(1)) .to.emit(proxy, "Execution") @@ -60,40 +55,31 @@ describe("MultiSig test", () => { }); - it ("UpgradeToAndCall", async () => { + it("UpgradeToAndCall", async () => { const callData = implementation.interface.encodeFunctionData( implementation.interface.functions["changeValue(bytes4)"], ["0x44332211"] ); await proxy.upgradeToAndCall(implementation.address, callData); - await expect(proxy.connect(adminS).confirmTransaction(0)). - to.emit(proxy, "Upgraded") + await expect(proxy.connect(adminS).confirmTransaction(0)).to.emit(proxy, "Upgraded") .withArgs(implementation.address); expect(await implementation.attach(proxy.address).value()).eq("0x44332211"); }); - it ("MultiSig submitTransaction", async () => { + it("MultiSig submitTransaction", async () => { await mockERC20.mint(proxy.address, 1000); const callData = mockERC20.interface.encodeFunctionData( mockERC20.interface.functions["transfer(address,uint256)"], [admin, 500] ); - await proxy.submitTransaction( - mockERC20.address, - 0, - callData - ); + await proxy.submitTransaction(mockERC20.address, 0, callData); await expect(() => proxy.connect(adminS).confirmTransaction(0)) .to.changeTokenBalance(mockERC20, adminS, 500); }); - it ("Non admin submitTransaction Call", async () => { - await expect(proxy.connect(userS).submitTransaction(proxy.address, 0, "0x1234")) - .to.be.revertedWith(""); - }); }); diff --git a/relay/internal/service_fee/fee_helper/fee_helper.go b/relay/internal/service_fee/fee_helper/fee_helper.go index b2722e7a..74937e54 100644 --- a/relay/internal/service_fee/fee_helper/fee_helper.go +++ b/relay/internal/service_fee/fee_helper/fee_helper.go @@ -45,7 +45,12 @@ func NewFeeHelper(bridge networks.Bridge, cfg config.FeeApiNetwork) (*FeeHelper, } func (b *FeeHelper) Sign(digestHash []byte) ([]byte, error) { - return crypto.Sign(digestHash, b.privateKey) + sign, err := crypto.Sign(digestHash, b.privateKey) + if err != nil { + return nil, err + } + sign[64] += 27 + return sign, nil } func (b *FeeHelper) GetTransferFee() (thisGas, sideGas decimal.Decimal, err error) {