Do not commit failing payment txs (ethereum#43)

gnosis · Mar 9, 2023 · 9af2d97 · 9af2d97
1 parent bb7042b
commit 9af2d97
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 19 deletions.
diff --git a/miner/algo_common.go b/miner/algo_common.go
@@ -293,50 +293,61 @@ func estimatePayoutTxGas(env *environment, sender, receiver common.Address, prv
 }
 
 func insertPayoutTx(env *environment, sender, receiver common.Address, gas uint64, isEOA bool, availableFunds *big.Int, prv *ecdsa.PrivateKey, chData chainData) (*types.Receipt, error) {
-	diff := newEnvironmentDiff(env)
-	applyTx := func(gas uint64) (*types.Receipt, error) {
+	applyTx := func(envDiff *environmentDiff, gas uint64) (*types.Receipt, error) {
 		fee := new(big.Int).Mul(env.header.BaseFee, new(big.Int).SetUint64(gas))
 		amount := new(big.Int).Sub(availableFunds, fee)
 		if amount.Sign() < 0 {
 			return nil, errors.New("not enough funds available")
 		}
-		rec, err := diff.commitPayoutTx(amount, sender, receiver, gas, prv, chData)
+		rec, err := envDiff.commitPayoutTx(amount, sender, receiver, gas, prv, chData)
 		if err != nil {
 			return nil, fmt.Errorf("failed to commit payment tx: %w", err)
-		}
-		if rec.Status != types.ReceiptStatusSuccessful {
+		} else if rec.Status != types.ReceiptStatusSuccessful {
 			return nil, fmt.Errorf("payment tx failed")
 		}
 		return rec, nil
 	}
 
 	if isEOA {
-		rec, err := applyTx(gas)
+		diff := newEnvironmentDiff(env)
+		rec, err := applyTx(diff, gas)
 		if err != nil {
 			return nil, err
 		}
 		diff.applyToBaseEnv()
 		return rec, nil
 	}
 
-	var (
-		rec *types.Receipt
-		err error
-	)
+	var err error
 	for i := 0; i < 6; i++ {
-		rec, err = applyTx(gas)
+		diff := newEnvironmentDiff(env)
+		var rec *types.Receipt
+		rec, err = applyTx(diff, gas)
 		if err != nil {
 			gas += 1000
-		} else {
-			break
+			continue
+		}
+
+		if gas == rec.GasUsed {
+			diff.applyToBaseEnv()
+			return rec, nil
+		}
+
+		exactEnvDiff := newEnvironmentDiff(env)
+		exactRec, err := applyTx(exactEnvDiff, rec.GasUsed)
+		if err != nil {
+			diff.applyToBaseEnv()
+			return rec, nil
 		}
+		exactEnvDiff.applyToBaseEnv()
+		return exactRec, nil
 	}
 
-	if err != nil {
-		return nil, err
+	if err == nil {
+		return nil, errors.New("could not estimate gas")
 	}
-	diff.applyToBaseEnv()
-	return rec, nil
+
+	return nil, err
 }
 
 func (envDiff *environmentDiff) commitPayoutTx(amount *big.Int, sender, receiver common.Address, gas uint64, prv *ecdsa.PrivateKey, chData chainData) (*types.Receipt, error) {

diff --git a/miner/algo_common_test.go b/miner/algo_common_test.go
@@ -548,6 +548,7 @@ func TestPayoutTxUtils(t *testing.T) {
 	require.Equal(t, types.ReceiptStatusSuccessful, rec.Status)
 	require.Equal(t, uint64(21000), rec.GasUsed)
 	require.True(t, new(big.Int).Sub(balanceAfter, balanceBefore).Cmp(expectedPayment) == 0)
+	require.Equal(t, env.state.GetNonce(signers.addresses[1]), uint64(1))
 
 	// Sending payment to the contract that logs event of the amount
 	gas, isEOA, err = estimatePayoutTxGas(env, signers.addresses[1], logContractAddress, signers.signers[1], chData)
@@ -564,10 +565,10 @@ func TestPayoutTxUtils(t *testing.T) {
 	require.Equal(t, types.ReceiptStatusSuccessful, rec.Status)
 	require.Equal(t, uint64(22025), rec.GasUsed)
 	require.True(t, new(big.Int).Sub(balanceAfter, balanceBefore).Cmp(expectedPayment) == 0)
+	require.Equal(t, env.state.GetNonce(signers.addresses[1]), uint64(2))
 
 	// Try requesting less gas for contract tx. We request 21k gas, but we must pay 22025
-	// iteration logic should set gas limit to 23k
-	expectedPayment = new(big.Int).Sub(availableFunds, big.NewInt(23000))
+	expectedPayment = new(big.Int).Sub(availableFunds, big.NewInt(22025))
 	balanceBefore = env.state.GetBalance(logContractAddress)
 	rec, err = insertPayoutTx(env, signers.addresses[1], logContractAddress, 21000, isEOA, availableFunds, signers.signers[1], chData)
 	balanceAfter = env.state.GetBalance(logContractAddress)
@@ -576,6 +577,7 @@ func TestPayoutTxUtils(t *testing.T) {
 	require.Equal(t, types.ReceiptStatusSuccessful, rec.Status)
 	require.Equal(t, uint64(22025), rec.GasUsed)
 	require.True(t, new(big.Int).Sub(balanceAfter, balanceBefore).Cmp(expectedPayment) == 0)
+	require.Equal(t, env.state.GetNonce(signers.addresses[1]), uint64(3))
 
 	// errors
 
@@ -591,4 +593,6 @@ func TestPayoutTxUtils(t *testing.T) {
 
 	_, err = insertPayoutTx(env, signers.addresses[1], signers.addresses[2], 20000, true, availableFunds, signers.signers[1], chData)
 	require.ErrorContains(t, err, "not enough gas")
+
+	require.Equal(t, env.state.GetNonce(signers.addresses[1]), uint64(3))
 }