Fix account balance check

core/state_transition.go

@@ -316,7 +316,8 @@ func (st *StateTransition) buyGas() error {
 		balanceCheck.SetUint64(st.msg.GasLimit)
 		balanceCheck = balanceCheck.Mul(balanceCheck, st.msg.GasFeeCap)
 	}
-	balanceCheck.Add(balanceCheck, st.msg.Value)
+	// Moved to canPayFee
+	// balanceCheck.Add(balanceCheck, st.msg.Value)
 	if l1Cost != nil {
 		balanceCheck.Add(balanceCheck, l1Cost)
 	}
@@ -333,12 +334,7 @@ func (st *StateTransition) buyGas() error {
 			mgval.Add(mgval, blobFee)
 		}
 	}
-	balanceCheckU256, overflow := uint256.FromBig(balanceCheck)
-	if overflow {
-		return fmt.Errorf("%w: address %v required balance exceeds 256 bits", ErrInsufficientFunds, st.msg.From.Hex())
-	}
-
-	if err := st.canPayFee(balanceCheckU256); err != nil {
+	if err := st.canPayFee(balanceCheck); err != nil {
 		return err
 	}
 	if err := st.gp.SubGas(st.msg.GasLimit); err != nil {
@@ -356,14 +352,33 @@ func (st *StateTransition) buyGas() error {
 }
 
 // canPayFee checks whether accountOwner's balance can cover transaction fee.
-func (st *StateTransition) canPayFee(checkAmount *uint256.Int) error {
+func (st *StateTransition) canPayFee(checkAmountForGas *big.Int) error {
+	var checkAmountInCelo, checkAmountInAlternativeCurrency *big.Int
 	if st.msg.FeeCurrency == nil {
+		checkAmountInCelo = new(big.Int).Add(checkAmountForGas, st.msg.Value)
+		checkAmountInAlternativeCurrency = common.Big0
+	} else {
+		checkAmountInCelo = st.msg.Value
+		checkAmountInAlternativeCurrency = checkAmountForGas
+	}
+
+	if checkAmountInCelo.Cmp(common.Big0) > 0 {
+		balanceInCeloU256, overflow := uint256.FromBig(checkAmountInCelo)
+		if overflow {
+			return fmt.Errorf("%w: address %v required balance exceeds 256 bits", ErrInsufficientFunds, st.msg.From.Hex())
+		}
+
 		balance := st.state.GetBalance(st.msg.From)
 
-		if balance.Cmp(checkAmount) < 0 {
-			return fmt.Errorf("%w: address %v have %v want %v", ErrInsufficientFunds, st.msg.From.Hex(), balance, checkAmount)
+		if balance.Cmp(balanceInCeloU256) < 0 {
+			return fmt.Errorf("%w: address %v have %v want %v", ErrInsufficientFunds, st.msg.From.Hex(), balance, checkAmountInCelo)
+		}
+	}
+	if checkAmountInAlternativeCurrency.Cmp(common.Big0) > 0 {
+		_, overflow := uint256.FromBig(checkAmountInAlternativeCurrency)
+		if overflow {
+			return fmt.Errorf("%w: address %v required balance exceeds 256 bits", ErrInsufficientFunds, st.msg.From.Hex())
 		}
-	} else {
 		backend := &contracts.CeloBackend{
 			ChainConfig: st.evm.ChainConfig(),
 			State:       st.state,
@@ -373,10 +388,8 @@ func (st *StateTransition) canPayFee(checkAmount *uint256.Int) error {
 			return err
 		}
 
-		// Token amount can't be bigger than 256 bit
-		balanceU256, _ := uint256.FromBig(balance)
-		if balanceU256.Cmp(checkAmount) < 0 {
-			return fmt.Errorf("%w: address %v have %v want %v, fee currency: %v", ErrInsufficientFunds, st.msg.From.Hex(), balance, checkAmount, st.msg.FeeCurrency.Hex())
+		if balance.Cmp(checkAmountInAlternativeCurrency) < 0 {
+			return fmt.Errorf("%w: address %v have %v want %v, fee currency: %v", ErrInsufficientFunds, st.msg.From.Hex(), balance, checkAmountInAlternativeCurrency, st.msg.FeeCurrency.Hex())
 		}
 	}
 	return nil

e2e_test/debug-fee-currency/lib.sh

@@ -49,8 +49,10 @@ function cleanup_fee_currency() {
 # 	$3: replaceTransaction (bool):
 # 		replace the transaction with a transaction of higher priority-fee when
 # 		there is no receipt after the `waitBlocks` time passed
+#   $4: value (num):
+#     value to send in the transaction
 function cip_64_tx() {
-	$SCRIPT_DIR/js-tests/send_tx.mjs "$(cast chain-id)" $ACC_PRIVKEY $1 $2 $3
+	$SCRIPT_DIR/js-tests/send_tx.mjs "$(cast chain-id)" $ACC_PRIVKEY $1 $2 $3 $4
 }
 
 # use this function to assert the cip_64_tx return value, by using a pipe like

e2e_test/js-tests/send_tx.mjs

@@ -9,7 +9,7 @@ import {
 import { celoAlfajores } from "viem/chains";
 import { privateKeyToAccount } from "viem/accounts";
 
-const [chainId, privateKey, feeCurrency, waitBlocks, replaceTxAfterWait] =
+const [chainId, privateKey, feeCurrency, waitBlocks, replaceTxAfterWait, celoValue] =
   process.argv.slice(2);
 const devChain = defineChain({
   ...celoAlfajores,
@@ -89,13 +89,17 @@ async function replaceTransaction(tx) {
 }
 
 async function main() {
+  let value = 2n
+  if (celoValue !== "") {
+    value = BigInt(celoValue)
+  }
   const request = await walletClient.prepareTransactionRequest({
     account,
     to: "0x00000000000000000000000000000000DeaDBeef",
-    value: 2n,
+    value: value,
     gas: 90000,
     feeCurrency,
-    maxFeePerGas: 2000000000n,
+    maxFeePerGas: 25000000000n,
     maxPriorityFeePerGas: 100n, // should be >= 1wei even after conversion to native tokens
   });
 

e2e_test/run_all_tests.sh

@@ -39,7 +39,7 @@ for f in test_*"$TEST_GLOB"*; do
 	if [[ -n $NETWORK ]]; then
 		case $f in
 		  # Skip tests that require a local network.
-		  test_fee_currency_fails_on_credit.sh|test_fee_currency_fails_on_debit.sh|test_fee_currency_fails_intrinsic.sh)
+		  test_fee_currency_fails_on_credit.sh|test_fee_currency_fails_on_debit.sh|test_fee_currency_fails_intrinsic.sh|test_value_and_fee_currency_balance_check.sh)
 		  echo "skipping file $f"
 		  continue
 		  ;;

e2e_test/test_fee_currency_fails_intrinsic.sh

@@ -15,15 +15,15 @@ trap 'kill %%' EXIT # kill bg tail job on exit
 	# trigger the first failed call to the CreditFees(), causing the
 	# currency to get temporarily blocklisted.
 	# initial tx should not succeed, should have required a replacement transaction.
-	cip_64_tx $fee_currency 1 true | assert_cip_64_tx false
+	cip_64_tx $fee_currency 1 true 2 | assert_cip_64_tx false
 
 	sleep 2
 
 	# since the fee currency is temporarily blocked,
 	# this should NOT make the transaction execute anymore,
 	# but invalidate the transaction earlier.
 	# initial tx should not succeed, should have required a replacement transaction.
-	cip_64_tx $fee_currency 1 true | assert_cip_64_tx false
+	cip_64_tx $fee_currency 1 true 2 | assert_cip_64_tx false
 
 	cleanup_fee_currency $fee_currency
 )

e2e_test/test_fee_currency_fails_on_credit.sh

@@ -14,15 +14,15 @@ trap 'kill %%' EXIT # kill bg tail job on exit
 	# trigger the first failed call to the CreditFees(), causing the
 	# currency to get temporarily blocklisted.
 	# initial tx should not succeed, should have required a replacement transaction.
-	cip_64_tx $fee_currency 1 true | assert_cip_64_tx false
+	cip_64_tx $fee_currency 1 true 2 | assert_cip_64_tx false
 
 	sleep 2
 
 	# since the fee currency is temporarily blocked,
 	# this should NOT make the transaction execute anymore,
 	# but invalidate the transaction earlier.
 	# initial tx should not succeed, should have required a replacement transaction.
-	cip_64_tx $fee_currency 1 true | assert_cip_64_tx false
+	cip_64_tx $fee_currency 1 true 2 | assert_cip_64_tx false
 
 	cleanup_fee_currency $fee_currency
 )

e2e_test/test_fee_currency_fails_on_debit.sh

@@ -9,6 +9,6 @@ source debug-fee-currency/lib.sh
 #
 fee_currency=$(deploy_fee_currency true false false)
 # this fails during the RPC call, since the DebitFees() is part of the pre-validation
-cip_64_tx $fee_currency 1 false | assert_cip_64_tx false "fee-currency internal error"
+cip_64_tx $fee_currency 1 false 2 | assert_cip_64_tx false "fee-currency internal error"
 
 cleanup_fee_currency $fee_currency

e2e_test/test_value_and_fee_currency_balance_check.sh

@@ -0,0 +1,23 @@
+#!/bin/bash
+#shellcheck disable=SC2086
+set -eo pipefail
+set -x
+
+source shared.sh
+source debug-fee-currency/lib.sh
+
+TEST_ACCOUNT_ADDR=0xEa787f769d66B5C131319f262F07254790985BdC
+TEST_ACCOUNT_PRIVKEY=0xd36ad839c0bc4bfd8c718a3219591a791871dafad2391149153e6abb43a777fd
+
+fee_currency=$(deploy_fee_currency false false false)
+
+# Send 2.5e15 fee currency to test account
+cast send --private-key $ACC_PRIVKEY $fee_currency 'transfer(address to, uint256 value) returns (bool)' $TEST_ACCOUNT_ADDR 2500000000000000
+# Send 1e18 celo to test account
+cast send --private-key $ACC_PRIVKEY $TOKEN_ADDR 'transfer(address to, uint256 value) returns (bool)' $TEST_ACCOUNT_ADDR 1000000000000000000
+
+# balanceFeeCurrency=2.5e15, txCost=2.25e15, balanceCelo=1e18, valueCelo=1e15
+# this should succed because the value and the txCost should not be added (with the bug the total cost could be 3.25e15 will fail because the balanceFeeCurrency is 2.5e15)
+$SCRIPT_DIR/js-tests/send_tx.mjs "$(cast chain-id)" $TEST_ACCOUNT_PRIVKEY $fee_currency 1 false 1000000000000000 | assert_cip_64_tx true ""
+
+cleanup_fee_currency $fee_currency