Merge pull request #193 from bitfinity-network/remove_approved_fee_sp…

…enders

[EPROD-1064] FeeCharge without approved spenders

solidity/src/BTFBridge.sol

@@ -246,7 +246,7 @@ contract BTFBridge is TokenManager, UUPSUpgradeable, OwnableUpgradeable, Pausabl
 
             // If user can't pay required fee, skip his order.
             if (_isFeeRequired()) {
-                bool canPayFee = feeChargeContract.canPayFee(order.feePayer, order.senderID, MIN_FEE_DEPOSIT_AMOUNT);
+                bool canPayFee = feeChargeContract.canPayFee(order.feePayer, MIN_FEE_DEPOSIT_AMOUNT);
                 if (!canPayFee) {
                     processedOrderIndexes[i] = MINT_ERROR_CODE_INSUFFICIENT_FEE_DEPOSIT;
                     continue;
@@ -280,7 +280,7 @@ contract BTFBridge is TokenManager, UUPSUpgradeable, OwnableUpgradeable, Pausabl
                 MintOrderData memory order =
                     _decodeOrder(encodedOrders[MINT_ORDER_DATA_LEN * i:MINT_ORDER_DATA_LEN * i + MINT_ORDER_DATA_LEN]);
                 if (_isFeeRequired()) {
-                    _chargeFee(order.feePayer, order.senderID, feePerUser);
+                    _chargeFee(order.feePayer, feePerUser);
                 }
                 _emitMintedEvent(order, feePerUser);
             }
@@ -311,9 +311,9 @@ contract BTFBridge is TokenManager, UUPSUpgradeable, OwnableUpgradeable, Pausabl
     }
 
     /// Charge fee from the user.
-    function _chargeFee(address from, bytes32 senderID, uint256 amount) private {
+    function _chargeFee(address from, uint256 amount) private {
         if (amount != 0) {
-            feeChargeContract.chargeFee(from, payable(minterCanisterAddress), senderID, amount);
+            feeChargeContract.chargeFee(from, payable(minterCanisterAddress), amount);
         }
     }
 

solidity/src/FeeCharge.sol

@@ -10,9 +10,7 @@ contract FeeCharge is IFeeCharge {
     // Mapping from user address to amount of native tokens on his deposit.
     mapping(address => uint256) private _userBalance;
 
-    // Mapping from user address to list of senderIDs, which are able to spend native deposit.
-    mapping(address => mapping(bytes32 => bool)) private _approvedIDs;
-
+    // Addresses allowed to charge fee from users.
     mapping(address => bool) private _canChargeFee;
 
     constructor(
@@ -25,18 +23,11 @@ contract FeeCharge is IFeeCharge {
         }
     }
 
-    // Deposit `msg.value` amount of native token to user's address.
-    // The deposit could be used to pay fees by the approvedSenderIDs.
+    // Deposit `msg.value` amount of native token to `msg.sender` address.
     // Returns user's balance after the operation.
-    function nativeTokenDeposit(
-        bytes32[] calldata approvedSenderIDs
-    ) external payable returns (uint256 balance) {
+    function nativeTokenDeposit() external payable returns (uint256 balance) {
         address to = msg.sender;
-
-        // Add approved SpenderIDs
-        for (uint256 i = 0; i < approvedSenderIDs.length; i++) {
-            _approvedIDs[to][approvedSenderIDs[i]] = true;
-        }
+        require(to != address(0), "expected non-zero to address");
 
         balance = _userBalance[to];
         balance += msg.value;
@@ -68,30 +59,20 @@ contract FeeCharge is IFeeCharge {
         balance = _userBalance[user];
     }
 
-    // Remove approved SpenderIDs
-    function removeApprovedSenderIDs(
-        bytes32[] calldata approvedSenderIDs
-    ) external {
-        for (uint256 i = 0; i < approvedSenderIDs.length; i++) {
-            delete _approvedIDs[msg.sender][approvedSenderIDs[i]];
-        }
-    }
-
     // Take the given amount of fee from the user.
     // Require the user to have enough native token balance and approval for senderID.
-    function chargeFee(address from, address payable to, bytes32 senderID, uint256 amount) external {
+    function chargeFee(address from, address payable to, uint256 amount) external {
         require(_canChargeFee[msg.sender], "fee charger is not present in allow list");
         uint256 balance = _userBalance[from];
         require(balance >= amount, "insufficient balance to pay fee");
-        require(_approvedIDs[from][senderID], "senderID is not approved");
 
         uint256 newBalance = balance - amount;
         _userBalance[from] = newBalance;
         to.transfer(amount);
     }
 
     /// Function to check if fee charge operation can be performed.
-    function canPayFee(address payer, bytes32 senderID, uint256 amount) external view returns (bool) {
+    function canPayFee(address payer, uint256 amount) external view returns (bool) {
         /// Check if the msg.sender is able to charge fee.
         if (!_canChargeFee[msg.sender]) {
             return false;
@@ -103,11 +84,6 @@ contract FeeCharge is IFeeCharge {
             return false;
         }
 
-        /// Check if the payer approved fee charge for the senderID.
-        if (!_approvedIDs[payer][senderID]) {
-            return false;
-        }
-
         return true;
     }
 }

solidity/src/interfaces/IFeeCharge.sol

@@ -3,12 +3,9 @@ pragma solidity >=0.5.0;
 
 // Allows to depsit/withdraw native tokens and use the deposit to charge fee.
 interface IFeeCharge {
-    // Deposit `msg.value` amount of native token to user's address.
-    // The deposit could be used to pay fees by the approvedSenderIDs.
+    // Deposit `msg.value` amount of native token to `msg.sender` address.
     // Returns user's balance after the operation.
-    function nativeTokenDeposit(
-        bytes32[] calldata approvedSenderIDs
-    ) external payable returns (uint256 balance);
+    function nativeTokenDeposit() external payable returns (uint256 balance);
 
     // Withdraw the amount of native token to user's address.
     // Returns user's balance after the operation.
@@ -21,15 +18,10 @@ interface IFeeCharge {
         address user
     ) external view returns (uint256 balance);
 
-    // Remove approved SpenderIDs
-    function removeApprovedSenderIDs(
-        bytes32[] calldata approvedSenderIDs
-    ) external;
-
-    // Take the given amount of fee from the user.
-    // Require the user to have enough native token balance and approval for senderID.
-    function chargeFee(address from, address payable to, bytes32 senderID, uint256 amount) external;
+    // Charge the given amount of fee from the user.
+    // Require the user to have enough native token deposit balance.
+    function chargeFee(address from, address payable to, uint256 amount) external;
 
     /// Function to check if fee charge operation can be performed.
-    function canPayFee(address payer, bytes32 senderID, uint256 amount) external view returns (bool);
+    function canPayFee(address from, uint256 amount) external view returns (bool);
 }

solidity/test/contract_tests/FeeCharge.t.sol

@@ -30,7 +30,7 @@ contract ChargeFeeTest is Test {
         aliceSenderIDs[1] = _aliceSender2;
         vm.deal(_alice, _aliceInitBalance);
         vm.prank(_alice);
-        _feeCharge.nativeTokenDeposit{ value: _aliceInitDeposit }(aliceSenderIDs);
+        _feeCharge.nativeTokenDeposit{ value: _aliceInitDeposit }();
     }
 
     function testDeposit() public view {
@@ -58,25 +58,21 @@ contract ChargeFeeTest is Test {
         uint256 fee = 1000;
 
         vm.prank(_charger);
-        _feeCharge.chargeFee(_alice, payable(_recepient), _aliceSender1, fee);
+        _feeCharge.chargeFee(_alice, payable(_recepient), fee);
         uint256 newBalance = _feeCharge.nativeTokenBalance(_alice);
         assertEq(newBalance, _aliceInitDeposit - fee, "deposit balance should decrese");
         assertEq(_alice.balance, _aliceInitBalance - _aliceInitDeposit, "alice balance should not change");
         assertEq(_recepient.balance, fee, "recepient balance should increase");
 
         vm.prank(_charger);
-        _feeCharge.chargeFee(_alice, payable(_recepient), _aliceSender2, fee);
+        _feeCharge.chargeFee(_alice, payable(_recepient), fee);
         uint256 newBalance2 = _feeCharge.nativeTokenBalance(_alice);
         assertEq(newBalance2, _aliceInitDeposit - fee * 2, "deposit balance should decrese");
         assertEq(_alice.balance, _aliceInitBalance - _aliceInitDeposit, "alice balance should not change");
         assertEq(_recepient.balance, fee * 2, "recepient balance should increase");
 
         vm.prank(_alice);
         vm.expectRevert();
-        _feeCharge.chargeFee(_alice, payable(_recepient), _aliceSender1, fee);
-
-        vm.prank(_charger);
-        vm.expectRevert();
-        _feeCharge.chargeFee(_alice, payable(_recepient), _bobSender1, fee);
+        _feeCharge.chargeFee(_alice, payable(_recepient), fee);
     }
 }

src/bridge-tool/src/flow/mod.rs

@@ -288,15 +288,8 @@ impl<'a> Erc20BridgeFlow<'a> {
         let memo = Self::generate_memo();
 
         self.approve_erc20(amount, side).await?;
-
-        let chain_id = self.chain_id(side).await?;
-        let sender_id = Id256::from_evm_address(&self.wallet.address().into(), chain_id as u32);
-
-        self.approve_fee(side.other(), sender_id, FEE_APPROVE_AMOUNT)
-            .await?;
-
+        self.approve_fee(side.other(), FEE_APPROVE_AMOUNT).await?;
         self.burn_btf(side, amount, &recipient, memo).await?;
-
         self.track_operation(memo, side.other()).await
     }
 
@@ -332,12 +325,7 @@ impl<'a> Erc20BridgeFlow<'a> {
         Ok(address)
     }
 
-    async fn approve_fee(
-        &self,
-        evm_side: EvmSide,
-        sender_id: Id256,
-        amount: u128,
-    ) -> anyhow::Result<()> {
+    async fn approve_fee(&self, evm_side: EvmSide, amount: u128) -> anyhow::Result<()> {
         info!("Requesting fee charge approve");
 
         let (client, _, _) = self.get_side(evm_side);
@@ -350,12 +338,7 @@ impl<'a> Erc20BridgeFlow<'a> {
             .await?;
         let chain_id = self.chain_id(evm_side).await?;
 
-        let input = FeeCharge::nativeTokenDepositCall {
-            approvedSenderIDs: vec![alloy_sol_types::private::FixedBytes::from_slice(
-                &sender_id.0,
-            )],
-        }
-        .abi_encode();
+        let input = FeeCharge::nativeTokenDepositCall {}.abi_encode();
 
         let approve_tx = TransactionBuilder {
             from: &self.wallet.address().into(),

src/integration-tests/tests/context/mod.rs

@@ -610,15 +610,9 @@ pub trait TestContext {
         evm_client: &EvmCanisterClient<Self::Client>,
         fee_charge: H160,
         user_wallet: &Wallet<'static, SigningKey>,
-        sender_ids: &[Id256],
         amount: u128,
     ) -> Result<U256> {
-        let sender_ids = sender_ids.iter().map(|id| id.0.into()).collect();
-
-        let input = FeeCharge::nativeTokenDepositCall {
-            approvedSenderIDs: sender_ids,
-        }
-        .abi_encode();
+        let input = FeeCharge::nativeTokenDepositCall {}.abi_encode();
 
         let receipt = self
             .call_contract_on_evm(evm_client, user_wallet, &fee_charge, input, amount)

src/integration-tests/tests/context/stress.rs

@@ -179,14 +179,12 @@ impl<B: BaseTokens> StressTestState<B> {
 
             // Deposit native token to charge fee.
             let evm_client = base_tokens.ctx().evm_client(base_tokens.ctx().admin_name());
-            let user_id256 = base_tokens.user_id256(user_id.clone());
             base_tokens
                 .ctx()
                 .native_token_deposit(
                     &evm_client,
                     fee_charge_address.clone(),
                     &wallet,
-                    &[user_id256],
                     10_u128.pow(15),
                 )
                 .await?;

src/integration-tests/tests/pocket_ic_integration_test/erc20_bridge.rs

@@ -206,13 +206,11 @@ async fn test_external_bridging() {
 
     // spender should deposit native tokens to btf bridge, to pay fee.
     let wrapped_evm_client = ctx.context.evm_client(ADMIN);
-    let bob_id = Id256::from_evm_address(&ctx.bob_address, CHAIN_ID as _);
     ctx.context
         .native_token_deposit(
             &wrapped_evm_client,
             ctx.fee_charge_address.clone(),
             &ctx.bob_wallet,
-            &[bob_id],
             10_u64.pow(15).into(),
         )
         .await
@@ -323,14 +321,12 @@ async fn native_token_deposit_increase_and_decrease() {
 
     // spender should deposit native tokens to btf bridge, to pay fee.
     let native_balance_after_deposit = 10_u64.pow(15);
-    let bob_id = Id256::from_evm_address(&ctx.bob_address, CHAIN_ID as _);
     let init_native_balance = ctx
         .context
         .native_token_deposit(
             &wrapped_evm_client,
             ctx.fee_charge_address.clone(),
             &ctx.bob_wallet,
-            &[bob_id],
             native_balance_after_deposit.into(),
         )
         .await
@@ -568,13 +564,11 @@ async fn native_token_deposit_should_increase_fee_charge_contract_balance() {
     // Deposit native tokens to btf bridge.
     let native_token_deposit = 10_000_000_u64;
     let wrapped_evm_client = ctx.context.evm_client(ADMIN);
-    let bob_id = Id256::from_evm_address(&ctx.bob_address, CHAIN_ID as _);
     ctx.context
         .native_token_deposit(
             &wrapped_evm_client,
             ctx.fee_charge_address.clone(),
             &ctx.bob_wallet,
-            &[bob_id],
             native_token_deposit.into(),
         )
         .await

src/integration-tests/tests/pocket_ic_integration_test/icrc2_bridge.rs

@@ -45,13 +45,11 @@ async fn test_icrc2_tokens_roundtrip() {
     let amount = 300_000u64;
 
     let evm_client = ctx.evm_client(ADMIN);
-    let john_principal_id = Id256::from(&john());
     let native_token_amount = 10_u64.pow(17);
     ctx.native_token_deposit(
         &evm_client,
         fee_charge.clone(),
         &john_wallet,
-        &[john_principal_id],
         native_token_amount.into(),
     )
     .await
@@ -140,13 +138,11 @@ async fn test_icrc2_token_canister_stopped() {
     let amount = 3_000_000u64;
 
     let evm_client = ctx.evm_client(ADMIN);
-    let john_principal_id = Id256::from(&john());
     let native_token_amount = 10_u64.pow(17);
     ctx.native_token_deposit(
         &evm_client,
         fee_charge.clone(),
         &john_wallet,
-        &[john_principal_id],
         native_token_amount.into(),
     )
     .await
@@ -339,13 +335,11 @@ async fn test_icrc2_tokens_approve_after_mint() {
     let spender_wallet = ctx.new_wallet(0).await.unwrap();
 
     let evm_client = ctx.evm_client(ADMIN);
-    let john_principal_id = Id256::from(&john());
     let native_token_amount = 10_u64.pow(17);
     ctx.native_token_deposit(
         &evm_client,
         fee_charge.clone(),
         &john_wallet,
-        &[john_principal_id],
         native_token_amount.into(),
     )
     .await
@@ -439,13 +433,11 @@ async fn icrc2_token_bridge(
     let amount = 300_000u64;
 
     let evm_client = ctx.evm_client(ADMIN);
-    let john_principal_id = Id256::from(&john());
     let native_token_amount = 10_u64.pow(10);
     ctx.native_token_deposit(
         &evm_client,
         fee_charge.clone(),
         &john_wallet,
-        &[john_principal_id],
         native_token_amount.into(),
     )
     .await
@@ -495,13 +487,11 @@ async fn test_minter_canister_address_balances_gets_replenished_after_roundtrip(
         .await
         .unwrap();
 
-    let john_principal_id = Id256::from(&john());
     let native_token_amount = 10_u64.pow(17);
     ctx.native_token_deposit(
         &evm_client,
         fee_charge.clone(),
         &john_wallet,
-        &[john_principal_id],
         native_token_amount.into(),
     )
     .await
@@ -561,13 +551,11 @@ async fn rescheduling_deposit_operation() {
     let amount = 300_000u64;
 
     let evm_client = ctx.evm_client(ADMIN);
-    let john_principal_id = Id256::from(&john());
     let native_token_amount = 10_u64.pow(17);
     ctx.native_token_deposit(
         &evm_client,
         fee_charge.clone(),
         &john_wallet,
-        &[john_principal_id],
         native_token_amount.into(),
     )
     .await
@@ -750,13 +738,11 @@ async fn test_icrc2_tokens_roundtrip_with_reschedule_spam() {
     let amount = 300_000u64;
 
     let evm_client = ctx.evm_client(ADMIN);
-    let john_principal_id = Id256::from(&john());
     let native_token_amount = 10_u64.pow(17);
     ctx.native_token_deposit(
         &evm_client,
         fee_charge.clone(),
         &john_wallet,
-        &[john_principal_id],
         native_token_amount.into(),
     )
     .await