diff --git a/src/solidity/Fees.sol b/src/solidity/Fees.sol index bcd6172..6b70b45 100644 --- a/src/solidity/Fees.sol +++ b/src/solidity/Fees.sol @@ -1,37 +1,29 @@ // SPDX-License-Identifier: Apache-2.0. pragma solidity ^0.8.0; +// Estimate cost for L1-L2 message handler. uint256 constant DEPOSIT_FEE_GAS = 20000; uint256 constant DEPLOYMENT_FEE_GAS = 100000; -uint256 constant MIN_FEE_MARGIN = 100000; -uint256 constant MAX_FEE_MARGIN = 10**14; -library Fees { - function estimateDepositFee() internal view returns (uint256) { - return DEPOSIT_FEE_GAS * block.basefee; - } +// We don't have a solid way to gauge block gas price +// (block.basefee cannot be used). +uint256 constant DEFAULT_WEI_PER_GAS = 5 * 10**9; - function estimateEnrollmentFee() internal view returns (uint256) { - return DEPLOYMENT_FEE_GAS * block.basefee; - } +abstract contract Fees { + // Effectively no minimum fee on testnet. + uint256 immutable MIN_FEE = (block.chainid == 1) ? 10**12 : 1; + uint256 constant MAX_FEE = 10**16; - function checkDepositFee(uint256 feeWei) internal view { - checkFee(feeWei, estimateDepositFee()); + function estimateDepositFee() internal pure returns (uint256) { + return DEPOSIT_FEE_GAS * DEFAULT_WEI_PER_GAS; } - function checkEnrollmentFee(uint256 feeWei) internal view { - checkFee(feeWei, estimateEnrollmentFee()); + function estimateEnrollmentFee() internal pure returns (uint256) { + return DEPLOYMENT_FEE_GAS * DEFAULT_WEI_PER_GAS; } - /* - The fee should be within margins from the estimated fee: - max(5, estimate/2 - MIN_FEE_MARGIN) <= fee <= 2*estimate + MAX_FEE_MARGIN. - */ - function checkFee(uint256 feeWei, uint256 feeEstimate) internal pure { - uint256 minFee = feeEstimate >> 1; - minFee = (minFee < MIN_FEE_MARGIN + 5) ? 5 : (minFee - MIN_FEE_MARGIN); - uint256 maxFee = MAX_FEE_MARGIN + (feeEstimate << 1); - require(feeWei >= minFee, "INSUFFICIENT_FEE_VALUE"); - require(feeWei <= maxFee, "FEE_VALUE_TOO_HIGH"); + function checkFee(uint256 feeWei) internal view { + require(feeWei >= MIN_FEE, "INSUFFICIENT_FEE_VALUE"); + require(feeWei <= MAX_FEE, "FEE_VALUE_TOO_HIGH"); } } diff --git a/src/solidity/StarknetERC20Bridge.sol b/src/solidity/StarknetERC20Bridge.sol index 723f993..5d7616c 100644 --- a/src/solidity/StarknetERC20Bridge.sol +++ b/src/solidity/StarknetERC20Bridge.sol @@ -5,6 +5,6 @@ import "src/solidity/LegacyBridge.sol"; contract StarknetERC20Bridge is LegacyBridge { function identify() external pure override returns (string memory) { - return "StarkWare_StarknetERC20Bridge_2.0_3"; + return "StarkWare_StarknetERC20Bridge_2.0_4"; } } diff --git a/src/solidity/StarknetEthBridge.sol b/src/solidity/StarknetEthBridge.sol index 79843cc..50391c0 100644 --- a/src/solidity/StarknetEthBridge.sol +++ b/src/solidity/StarknetEthBridge.sol @@ -9,7 +9,7 @@ contract StarknetEthBridge is LegacyBridge { using Addresses for address; function identify() external pure override returns (string memory) { - return "StarkWare_StarknetEthBridge_2.0_3"; + return "StarkWare_StarknetEthBridge_2.0_4"; } function acceptDeposit( @@ -19,7 +19,7 @@ contract StarknetEthBridge is LegacyBridge { // Make sure msg.value is enough to cover amount. The remaining value is fee. require(msg.value >= amount, "INSUFFICIENT_VALUE"); uint256 fee = msg.value - amount; - Fees.checkDepositFee(fee); + Fees.checkFee(fee); // The msg.value was already credited to this contract. Fee will be passed to Starknet. require(address(this).balance - fee <= getMaxTotalBalance(ETH), "MAX_BALANCE_EXCEEDED"); return fee; diff --git a/src/solidity/StarknetTokenBridge.sol b/src/solidity/StarknetTokenBridge.sol index dbe244b..8f432ff 100644 --- a/src/solidity/StarknetTokenBridge.sol +++ b/src/solidity/StarknetTokenBridge.sol @@ -24,6 +24,7 @@ contract StarknetTokenBridge is IStarkgateBridge, IStarkgateService, Identity, + Fees, StarknetTokenStorage, ProxySupport { @@ -90,7 +91,7 @@ contract StarknetTokenBridge is uint256 constant DEPOSIT_MESSAGE_FIXED_SIZE = 1; function identify() external pure virtual returns (string memory) { - return "StarkWare_StarknetTokenBridge_2.0_3"; + return "StarkWare_StarknetTokenBridge_2.0_4"; } function validateInitData(bytes calldata data) internal view virtual override { @@ -139,17 +140,17 @@ contract StarknetTokenBridge is _; } - function estimateDepositFeeWei() external view returns (uint256) { + function estimateDepositFeeWei() external pure returns (uint256) { return Fees.estimateDepositFee(); } - function estimateEnrollmentFeeWei() external view returns (uint256) { + function estimateEnrollmentFeeWei() external pure returns (uint256) { return Fees.estimateEnrollmentFee(); } // Virtual functions. function acceptDeposit(address token, uint256 amount) internal virtual returns (uint256) { - Fees.checkDepositFee(msg.value); + Fees.checkFee(msg.value); uint256 currentBalance = IERC20(token).balanceOf(address(this)); require(currentBalance + amount <= getMaxTotalBalance(token), "MAX_BALANCE_EXCEEDED"); Transfers.transferIn(token, msg.sender, amount); @@ -429,7 +430,7 @@ contract StarknetTokenBridge is function sendDeployMessage(address token) internal returns (bytes32) { require(l2TokenBridge() != 0, "L2_BRIDGE_NOT_SET"); - Fees.checkEnrollmentFee(msg.value); + Fees.checkFee(msg.value); (bytes32 deploymentMsgHash, ) = messagingContract().sendMessageToL2{value: msg.value}( l2TokenBridge(), diff --git a/src/solidity/conftest.py b/src/solidity/conftest.py index 9434d09..856c1e8 100644 --- a/src/solidity/conftest.py +++ b/src/solidity/conftest.py @@ -22,6 +22,7 @@ StarknetTokenBridgeTester, StarknetERC20BridgeTester, SelfRemoveTester, + FeeTester, ) DYNAMIC_FEE = -1 @@ -191,6 +192,11 @@ def regular_user(eth_test_utils: EthTestUtils) -> EthContract: return eth_test_utils.accounts[3] +@pytest.fixture +def fee_tester(governor: EthAccount) -> EthContract: + return governor.deploy(FeeTester) + + @pytest.fixture def mock_erc20_contract(governor: EthAccount) -> EthContract: erc20_contract = governor.deploy(TestERC20) diff --git a/src/solidity/files_to_compile.txt b/src/solidity/files_to_compile.txt index dcacdac..9d7f216 100644 --- a/src/solidity/files_to_compile.txt +++ b/src/solidity/files_to_compile.txt @@ -12,3 +12,4 @@ starkware/solidity/test_contracts/TestERC20.sol starkware/solidity/upgrade/Proxy.sol starkware/starknet/testing/MockStarknetMessaging.sol src/solidity/StarkgateUpgradeAssistExternalInitializer.sol +src/solidity/test_contracts/TestFees.sol diff --git a/src/solidity/legacy_token_bridge_test.py b/src/solidity/legacy_token_bridge_test.py index ff8cde6..df5cc11 100644 --- a/src/solidity/legacy_token_bridge_test.py +++ b/src/solidity/legacy_token_bridge_test.py @@ -1,6 +1,5 @@ import pytest from web3 import Web3 -from starkware.cairo.lang.cairo_constants import DEFAULT_PRIME from starkware.eth.eth_test_utils import EthContract, EthRevertException, EthTestUtils, EthAccount from solidity.conftest import ( add_implementation_and_upgrade, @@ -11,16 +10,8 @@ deploy_proxy, messaging_contract, mock_erc20_contract, - StarknetTokenBridgeWrapper, - EthBridgeWrapper, - StarknetERC20BridgeWrapper, - TokenBridgeWrapper, L1_TOKEN_ADDRESS_OF_ETH, - L2_TOKEN_CONTRACT, MAX_UINT, - HANDLE_TOKEN_DEPOSIT_SELECTOR, - HANDLE_DEPOSIT_WITH_MESSAGE_SELECTOR, - HANDLE_TOKEN_DEPLOYMENT_SELECTOR, TOKEN_ADDRESS, UpgradeAssistEIC, StarknetTokenBridge, @@ -48,6 +39,8 @@ WITHDRAW_AMOUNT = 3 MESSAGE_CANCEL_DELAY = 1000 MAX_TVL = 496351 +MAX_FEE = 10**16 +MIN_FEE = 10**12 @pytest.fixture(scope="session") @@ -136,6 +129,23 @@ def legacy_tester_erc20_new_proxy_bridge( ) +def test_fee_values(fee_tester): + with pytest.raises(EthRevertException, match="INSUFFICIENT_FEE_VALUE"): + fee_tester.testCheckFee.call(0) + + for valid_fee in [MIN_FEE, MAX_FEE]: + fee_tester.testCheckFee.call(valid_fee) + + deposit_fee = fee_tester.estimateDepositFeeWei.call() + assert MIN_FEE * 10 < deposit_fee < MAX_FEE // 10 + + enrollment_fee = fee_tester.estimateEnrollmentFeeWei.call() + assert MIN_FEE * 10 < enrollment_fee < MAX_FEE // 10 + + with pytest.raises(EthRevertException, match="FEE_VALUE_TOO_HIGH"): + fee_tester.testCheckFee.call(1 + MAX_FEE) + + def test_erc20_bridge_deposit_cancel_upgrade( governor: EthAccount, mock_erc20_contract: EthContract, diff --git a/src/solidity/test_contracts.py b/src/solidity/test_contracts.py index d883f0a..6d85a16 100644 --- a/src/solidity/test_contracts.py +++ b/src/solidity/test_contracts.py @@ -5,3 +5,4 @@ StarknetERC20BridgeTester = load_contract("StarknetERC20BridgeTester") SelfRemoveTester = load_contract("SelfRemoveTester") StarkgateRegistry = load_contract("StarkgateRegistry") +FeeTester = load_contract("TestFees") diff --git a/src/solidity/test_contracts/TestFees.sol b/src/solidity/test_contracts/TestFees.sol new file mode 100644 index 0000000..abb4855 --- /dev/null +++ b/src/solidity/test_contracts/TestFees.sol @@ -0,0 +1,18 @@ +// SPDX-License-Identifier: Apache-2.0. +pragma solidity ^0.8.0; + +import "src/solidity/Fees.sol"; + +contract TestFees is Fees { + function estimateDepositFeeWei() external pure returns (uint256) { + return Fees.estimateDepositFee(); + } + + function estimateEnrollmentFeeWei() external pure returns (uint256) { + return Fees.estimateEnrollmentFee(); + } + + function testCheckFee(uint256 feeWei) external view { + Fees.checkFee(feeWei); + } +} diff --git a/src/solidity/token_bridge_test.py b/src/solidity/token_bridge_test.py index 20a4548..af601b5 100644 --- a/src/solidity/token_bridge_test.py +++ b/src/solidity/token_bridge_test.py @@ -191,18 +191,17 @@ def test_deposit_l2_token_contract_not_set(token_bridge_wrapper: TokenBridgeWrap def test_deposit_fee_too_low(token_bridge_wrapper: TokenBridgeWrapper): setup_contracts(token_bridge_wrapper=token_bridge_wrapper) with pytest.raises(EthRevertException, match="INSUFFICIENT_FEE_VALUE"): - token_bridge_wrapper.deposit(amount=DEPOSIT_AMOUNT, l2_recipient=L2_RECIPIENT, fee=1) + token_bridge_wrapper.deposit(amount=DEPOSIT_AMOUNT, l2_recipient=L2_RECIPIENT, fee=0) with pytest.raises(EthRevertException, match="INSUFFICIENT_FEE_VALUE"): token_bridge_wrapper.deposit( - amount=DEPOSIT_AMOUNT, l2_recipient=L2_RECIPIENT, fee=1, message=MESSAGE + amount=DEPOSIT_AMOUNT, l2_recipient=L2_RECIPIENT, fee=0, message=MESSAGE ) def test_deposit_fee_too_high(token_bridge_wrapper: TokenBridgeWrapper): setup_contracts(token_bridge_wrapper=token_bridge_wrapper) - base_fee = token_bridge_wrapper.contract.estimateDepositFeeWei.call() - too_high_fee = 2 * base_fee + 2 * 10**14 + too_high_fee = 2 + 10**16 with pytest.raises(EthRevertException, match="FEE_VALUE_TOO_HIGH"): token_bridge_wrapper.deposit( amount=DEPOSIT_AMOUNT, l2_recipient=L2_RECIPIENT, fee=too_high_fee