From df6f73e1b9c34a2fe5b272a4259ddbe63020c34a Mon Sep 17 00:00:00 2001 From: ganeshvanahalli Date: Mon, 16 Oct 2023 09:38:51 -0500 Subject: [PATCH 1/6] test failure --- .github/workflows/waitForNitro.sh | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/.github/workflows/waitForNitro.sh b/.github/workflows/waitForNitro.sh index e196b38d88..422083e38e 100755 --- a/.github/workflows/waitForNitro.sh +++ b/.github/workflows/waitForNitro.sh @@ -1,10 +1,14 @@ -# poll the nitro endpoint until we get a 0 return code -while true -do +# poll the nitro endpoint until we get a 0 return code or 30mins have passed, in that case exit 1 +start_time=$(date +%s) +timeout=20 + +while (( $(date +%s) - start_time <= timeout )); do curl -X POST -H 'Content-Type: application/json' -d '{"jsonrpc":"2.0","id":45678,"method":"eth_chainId","params":[]}' 'http://localhost:8547' if [ "$?" -eq "0" ]; then exit 0 else sleep 20 fi -done \ No newline at end of file +done + +exit 1 \ No newline at end of file From 82663481ad99b6ea6eef6440aed735e3cdedd243 Mon Sep 17 00:00:00 2001 From: ganeshvanahalli Date: Mon, 16 Oct 2023 09:58:24 -0500 Subject: [PATCH 2/6] test success --- .github/workflows/waitForNitro.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/waitForNitro.sh b/.github/workflows/waitForNitro.sh index 422083e38e..a1b7f2ad0f 100755 --- a/.github/workflows/waitForNitro.sh +++ b/.github/workflows/waitForNitro.sh @@ -1,6 +1,6 @@ # poll the nitro endpoint until we get a 0 return code or 30mins have passed, in that case exit 1 start_time=$(date +%s) -timeout=20 +timeout=1800 while (( $(date +%s) - start_time <= timeout )); do curl -X POST -H 'Content-Type: application/json' -d '{"jsonrpc":"2.0","id":45678,"method":"eth_chainId","params":[]}' 'http://localhost:8547' From f99cfe673e5799404186a60ce254a37d48935397 Mon Sep 17 00:00:00 2001 From: ganeshvanahalli Date: Mon, 16 Oct 2023 10:31:24 -0500 Subject: [PATCH 3/6] refactor --- .github/workflows/waitForNitro.sh | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/.github/workflows/waitForNitro.sh b/.github/workflows/waitForNitro.sh index a1b7f2ad0f..add83e24bc 100755 --- a/.github/workflows/waitForNitro.sh +++ b/.github/workflows/waitForNitro.sh @@ -1,8 +1,7 @@ # poll the nitro endpoint until we get a 0 return code or 30mins have passed, in that case exit 1 -start_time=$(date +%s) -timeout=1800 +timeout_time=$(($(date +%s) + 1800)) -while (( $(date +%s) - start_time <= timeout )); do +while (( $(date +%s) <= timeout_time )); do curl -X POST -H 'Content-Type: application/json' -d '{"jsonrpc":"2.0","id":45678,"method":"eth_chainId","params":[]}' 'http://localhost:8547' if [ "$?" -eq "0" ]; then exit 0 From f5593692885c7401195a6907339ea1d03df95c35 Mon Sep 17 00:00:00 2001 From: ganeshvanahalli Date: Mon, 16 Oct 2023 10:46:04 -0500 Subject: [PATCH 4/6] refactor --- .github/workflows/waitForNitro.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/waitForNitro.sh b/.github/workflows/waitForNitro.sh index add83e24bc..cf3f6484fc 100755 --- a/.github/workflows/waitForNitro.sh +++ b/.github/workflows/waitForNitro.sh @@ -1,9 +1,9 @@ +#!/bin/bash # poll the nitro endpoint until we get a 0 return code or 30mins have passed, in that case exit 1 timeout_time=$(($(date +%s) + 1800)) while (( $(date +%s) <= timeout_time )); do - curl -X POST -H 'Content-Type: application/json' -d '{"jsonrpc":"2.0","id":45678,"method":"eth_chainId","params":[]}' 'http://localhost:8547' - if [ "$?" -eq "0" ]; then + if curl -X POST -H 'Content-Type: application/json' -d '{"jsonrpc":"2.0","id":45678,"method":"eth_chainId","params":[]}' 'http://localhost:8547'; then exit 0 else sleep 20 From 487f6eeba172805c8be6ce2a0bb8914e55c30e5d Mon Sep 17 00:00:00 2001 From: Lee Bousfield Date: Mon, 16 Oct 2023 23:07:06 -0600 Subject: [PATCH 5/6] Fix retryable gas estimation when overriding gas price to zero --- arbos/tx_processor.go | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/arbos/tx_processor.go b/arbos/tx_processor.go index 4eeffc679e..436998dfb5 100644 --- a/arbos/tx_processor.go +++ b/arbos/tx_processor.go @@ -245,16 +245,17 @@ func (p *TxProcessor) StartTxHook() (endTxNow bool, gasUsed uint64, err error, r } balance := statedb.GetBalance(tx.From) - basefee := evm.Context.BaseFee + effectiveBaseFee := evm.Context.BaseFee usergas := p.msg.GasLimit - maxGasCost := arbmath.BigMulByUint(tx.GasFeeCap, usergas) - maxFeePerGasTooLow := arbmath.BigLessThan(tx.GasFeeCap, basefee) - if p.msg.TxRunMode == core.MessageGasEstimationMode && tx.GasFeeCap.BitLen() == 0 { - // In gas estimation mode, we permit a zero gas fee cap. - // This matches behavior with normal tx gas estimation. - maxFeePerGasTooLow = false + if p.msg.TxRunMode != core.MessageCommitMode && tx.GasFeeCap.BitLen() == 0 { + // In gas estimation or eth_call mode, we permit a zero gas fee cap. + // This matches behavior with normal tx gas estimation and eth_call. + effectiveBaseFee = common.Big0 } + + maxGasCost := arbmath.BigMulByUint(tx.GasFeeCap, usergas) + maxFeePerGasTooLow := arbmath.BigLessThan(tx.GasFeeCap, effectiveBaseFee) if arbmath.BigLessThan(balance, maxGasCost) || usergas < params.TxGas || maxFeePerGasTooLow { // User either specified too low of a gas fee cap, didn't have enough balance to pay for gas, // or the specified gas limit is below the minimum transaction gas cost. @@ -268,7 +269,7 @@ func (p *TxProcessor) StartTxHook() (endTxNow bool, gasUsed uint64, err error, r } // pay for the retryable's gas and update the pools - gascost := arbmath.BigMulByUint(basefee, usergas) + gascost := arbmath.BigMulByUint(effectiveBaseFee, usergas) networkCost := gascost if p.state.ArbOSVersion() >= 11 { infraFeeAccount, err := p.state.InfraFeeAccount() @@ -276,7 +277,7 @@ func (p *TxProcessor) StartTxHook() (endTxNow bool, gasUsed uint64, err error, r if infraFeeAccount != (common.Address{}) { minBaseFee, err := p.state.L2PricingState().MinBaseFeeWei() p.state.Restrict(err) - infraFee := arbmath.BigMin(minBaseFee, basefee) + infraFee := arbmath.BigMin(minBaseFee, effectiveBaseFee) infraCost := arbmath.BigMulByUint(infraFee, usergas) infraCost = takeFunds(networkCost, infraCost) if err := transfer(&tx.From, &infraFeeAccount, infraCost); err != nil { @@ -294,7 +295,7 @@ func (p *TxProcessor) StartTxHook() (endTxNow bool, gasUsed uint64, err error, r } withheldGasFunds := takeFunds(availableRefund, gascost) // gascost is conceptually charged before the gas price refund - gasPriceRefund := arbmath.BigMulByUint(arbmath.BigSub(tx.GasFeeCap, basefee), tx.Gas) + gasPriceRefund := arbmath.BigMulByUint(arbmath.BigSub(tx.GasFeeCap, effectiveBaseFee), tx.Gas) if gasPriceRefund.Sign() < 0 { // This should only be possible during gas estimation mode gasPriceRefund.SetInt64(0) @@ -310,7 +311,7 @@ func (p *TxProcessor) StartTxHook() (endTxNow bool, gasUsed uint64, err error, r retryTxInner, err := retryable.MakeTx( underlyingTx.ChainId(), 0, - basefee, + effectiveBaseFee, usergas, ticketId, tx.FeeRefundAddr, From 0a3e0d8bbcedad1ccae5a987928875c588d38835 Mon Sep 17 00:00:00 2001 From: Lee Bousfield Date: Thu, 19 Oct 2023 12:10:34 -0600 Subject: [PATCH 6/6] Fix basefee in EndTxHook --- arbos/tx_processor.go | 31 ++++++++++++++++++++++++------- go-ethereum | 2 +- 2 files changed, 25 insertions(+), 8 deletions(-) diff --git a/arbos/tx_processor.go b/arbos/tx_processor.go index 436998dfb5..569edb7c63 100644 --- a/arbos/tx_processor.go +++ b/arbos/tx_processor.go @@ -248,7 +248,7 @@ func (p *TxProcessor) StartTxHook() (endTxNow bool, gasUsed uint64, err error, r effectiveBaseFee := evm.Context.BaseFee usergas := p.msg.GasLimit - if p.msg.TxRunMode != core.MessageCommitMode && tx.GasFeeCap.BitLen() == 0 { + if p.msg.TxRunMode != core.MessageCommitMode && p.msg.GasFeeCap.BitLen() == 0 { // In gas estimation or eth_call mode, we permit a zero gas fee cap. // This matches behavior with normal tx gas estimation and eth_call. effectiveBaseFee = common.Big0 @@ -457,7 +457,6 @@ func (p *TxProcessor) EndTxHook(gasLeft uint64, success bool) { underlyingTx := p.msg.Tx networkFeeAccount, _ := p.state.NetworkFeeAccount() - basefee := p.evm.Context.BaseFee scenario := util.TracingAfterEVM if gasLeft > p.msg.GasLimit { @@ -467,9 +466,20 @@ func (p *TxProcessor) EndTxHook(gasLeft uint64, success bool) { if underlyingTx != nil && underlyingTx.Type() == types.ArbitrumRetryTxType { inner, _ := underlyingTx.GetInner().(*types.ArbitrumRetryTx) + effectiveBaseFee := inner.GasFeeCap + if p.msg.TxRunMode == core.MessageCommitMode && !arbmath.BigEquals(effectiveBaseFee, p.evm.Context.BaseFee) { + log.Error( + "ArbitrumRetryTx GasFeeCap doesn't match basefee in commit mode", + "txHash", underlyingTx.Hash(), + "gasFeeCap", inner.GasFeeCap, + "baseFee", p.evm.Context.BaseFee, + ) + // revert to the old behavior to avoid diverging from older nodes + effectiveBaseFee = p.evm.Context.BaseFee + } // undo Geth's refund to the From address - gasRefund := arbmath.BigMulByUint(basefee, gasLeft) + gasRefund := arbmath.BigMulByUint(effectiveBaseFee, gasLeft) err := util.BurnBalance(&inner.From, gasRefund, p.evm, scenario, "undoRefund") if err != nil { log.Error("Uh oh, Geth didn't refund the user", inner.From, gasRefund) @@ -504,7 +514,7 @@ func (p *TxProcessor) EndTxHook(gasLeft uint64, success bool) { takeFunds(maxRefund, inner.SubmissionFeeRefund) } // Conceptually, the gas charge is taken from the L1 deposit pool if possible. - takeFunds(maxRefund, arbmath.BigMulByUint(basefee, gasUsed)) + takeFunds(maxRefund, arbmath.BigMulByUint(effectiveBaseFee, gasUsed)) // Refund any unused gas, without overdrafting the L1 deposit. networkRefund := gasRefund if p.state.ArbOSVersion() >= 11 { @@ -514,7 +524,7 @@ func (p *TxProcessor) EndTxHook(gasLeft uint64, success bool) { minBaseFee, err := p.state.L2PricingState().MinBaseFeeWei() p.state.Restrict(err) // TODO MinBaseFeeWei change during RetryTx execution may cause incorrect calculation of the part of the refund that should be taken from infraFeeAccount. Unless the balances of network and infra fee accounts are too low, the amount transferred to refund address should remain correct. - infraFee := arbmath.BigMin(minBaseFee, basefee) + infraFee := arbmath.BigMin(minBaseFee, effectiveBaseFee) infraRefund := arbmath.BigMulByUint(infraFee, gasLeft) infraRefund = takeFunds(networkRefund, infraRefund) refund(infraFeeAccount, infraRefund) @@ -542,6 +552,7 @@ func (p *TxProcessor) EndTxHook(gasLeft uint64, success bool) { return } + basefee := p.evm.Context.BaseFee totalCost := arbmath.BigMul(basefee, arbmath.UintToBig(gasUsed)) // total cost = price of gas * gas burnt computeCost := arbmath.BigSub(totalCost, p.PosterFee) // total cost = network's compute + poster's L1 costs if computeCost.Sign() < 0 { @@ -603,9 +614,15 @@ func (p *TxProcessor) EndTxHook(gasLeft uint64, success bool) { func (p *TxProcessor) ScheduledTxes() types.Transactions { scheduled := types.Transactions{} time := p.evm.Context.Time - basefee := p.evm.Context.BaseFee + effectiveBaseFee := p.evm.Context.BaseFee chainID := p.evm.ChainConfig().ChainID + if p.msg.TxRunMode != core.MessageCommitMode && p.msg.GasFeeCap.BitLen() == 0 { + // In gas estimation or eth_call mode, we permit a zero gas fee cap. + // This matches behavior with normal tx gas estimation and eth_call. + effectiveBaseFee = common.Big0 + } + logs := p.evm.StateDB.GetCurrentTxLogs() for _, log := range logs { if log.Address != ArbRetryableTxAddress || log.Topics[0] != RedeemScheduledEventID { @@ -624,7 +641,7 @@ func (p *TxProcessor) ScheduledTxes() types.Transactions { redeem, _ := retryable.MakeTx( chainID, event.SequenceNum, - basefee, + effectiveBaseFee, event.DonatedGas, event.TicketId, event.GasDonor, diff --git a/go-ethereum b/go-ethereum index b4221631e1..e8c8827c0b 160000 --- a/go-ethereum +++ b/go-ethereum @@ -1 +1 @@ -Subproject commit b4221631e1e5eac86f01582bd74234e3c0f7f5c7 +Subproject commit e8c8827c0b9e22e60829da1945cba9c451cda85a