From 61c6098da8beca21e6f9e28552c2b0cc54678834 Mon Sep 17 00:00:00 2001 From: Jason Yellick Date: Tue, 7 May 2024 15:45:48 -0400 Subject: [PATCH 1/2] Add additional logging around l1cost --- core/state_transition.go | 15 ++++++++++++--- erigon-lib/opstack/rollup_cost.go | 16 +++++++++------- erigon-lib/opstack/rollup_cost_test.go | 4 ++-- erigon-lib/txpool/pool.go | 6 +++--- 4 files changed, 26 insertions(+), 15 deletions(-) diff --git a/core/state_transition.go b/core/state_transition.go index 12f1528a3c4..288a922ad31 100644 --- a/core/state_transition.go +++ b/core/state_transition.go @@ -255,7 +255,11 @@ func (st *StateTransition) buyGas(gasBailout bool) error { } } var subBalance = false - if have, want := st.state.GetBalance(st.msg.From()), balanceCheck; have.Cmp(want) < 0 { + have, want := st.state.GetBalance(st.msg.From()), balanceCheck + if !gasBailout { + log.Info("Validating balance requirements in buyGas", "from", st.msg.From(), "nonce", st.msg.Nonce(), "have", have, "want", want) + } + if have.Cmp(want) < 0 { if !gasBailout { return fmt.Errorf("%w: address %v have %v want %v", ErrInsufficientFunds, st.msg.From().Hex(), have, want) } @@ -296,9 +300,14 @@ func (st *StateTransition) preCheck(gasBailout bool) error { // buyGas method originally handled balance check, but deposit tx does not use it // Therefore explicit check required for separating consensus error and evm internal error. // If not check it here, it will trigger evm internal error and break consensus. - if have, want := st.state.GetBalance(st.msg.From()), st.msg.Value(); have.Cmp(want) < 0 { + + have, want := st.state.GetBalance(st.msg.From()), st.msg.Value() + if !want.IsZero() { + log.Info("Validating balance requirements for depositTx in preCheck", "from", st.msg.From(), "nonce", st.msg.Nonce(), "have", have, "want", want) + } + if have.Cmp(want) < 0 { if !gasBailout { - return fmt.Errorf("%w: address %v have %v want %v", ErrInsufficientFunds, st.msg.From().Hex(), have, want) + return fmt.Errorf("%w: precheck address %v have %v want %v", ErrInsufficientFunds, st.msg.From().Hex(), have, want) } } diff --git a/erigon-lib/opstack/rollup_cost.go b/erigon-lib/opstack/rollup_cost.go index bf972342aae..3dd2c21b18f 100644 --- a/erigon-lib/opstack/rollup_cost.go +++ b/erigon-lib/opstack/rollup_cost.go @@ -141,7 +141,7 @@ func NewL1CostFunc(config *chain.Config, statedb StateGetter) L1CostFunc { offset := scalarSectionStart l1BaseFeeScalar := new(uint256.Int).SetBytes(l1FeeScalars[offset : offset+4]) l1BlobBaseFeeScalar := new(uint256.Int).SetBytes(l1FeeScalars[offset+4 : offset+8]) - cachedFunc = newL1CostFuncEcotone(&l1BaseFee, &l1BlobBaseFee, l1BaseFeeScalar, l1BlobBaseFeeScalar) + cachedFunc = newL1CostFuncEcotone(blockTime, &l1BaseFee, &l1BlobBaseFee, l1BaseFeeScalar, l1BlobBaseFeeScalar) } } } @@ -158,6 +158,7 @@ func newL1CostFuncBedrock(config *chain.Config, statedb StateGetter, blockTime u statedb.GetState(L1BlockAddr, &OverheadSlot, &overhead) statedb.GetState(L1BlockAddr, &ScalarSlot, &scalar) isRegolith := config.IsRegolith(blockTime) + log.Info("Caching l1cost parameters for bedrock", "isRegolith", isRegolith, "blockTime", blockTime, "l1BaseFee", l1BaseFee, "overhead", overhead, "l1BaseFeeScalar", scalar) return newL1CostFuncBedrockHelper(&l1BaseFee, &overhead, &scalar, isRegolith) } @@ -183,7 +184,8 @@ func newL1CostFuncBedrockHelper(l1BaseFee, overhead, scalar *uint256.Int, isRego // newL1CostFuncEcotone returns an l1 cost function suitable for the Ecotone upgrade except for the // very first block of the upgrade. -func newL1CostFuncEcotone(l1BaseFee, l1BlobBaseFee, l1BaseFeeScalar, l1BlobBaseFeeScalar *uint256.Int) l1CostFunc { +func newL1CostFuncEcotone(blockTime uint64, l1BaseFee, l1BlobBaseFee, l1BaseFeeScalar, l1BlobBaseFeeScalar *uint256.Int) l1CostFunc { + log.Info("Caching l1cost parameters for ecotone", "blockTime", blockTime, "l1BaseFee", l1BaseFee, "l1BlobBaseFee", l1BlobBaseFee, "l1BaseFeeScalar", l1BaseFeeScalar, "l1BlobBaseFeeScalar", l1BlobBaseFeeScalar) return func(costData types.RollupCostData) (fee, calldataGasUsed *uint256.Int) { calldataGas := (costData.Zeroes * fixedgas.TxDataZeroGas) + (costData.Ones * fixedgas.TxDataNonZeroGasEIP2028) calldataGasUsed = new(uint256.Int).SetUint64(calldataGas) @@ -221,7 +223,7 @@ func ExtractL1GasParams(config *chain.Config, time uint64, data []byte) (l1BaseF // edge case: for the very first Ecotone block we still need to use the Bedrock // function. We detect this edge case by seeing if the function selector is the old one if len(data) >= 4 && !bytes.Equal(data[0:4], BedrockL1AttributesSelector) { - l1BaseFee, costFunc, err = extractL1GasParamsEcotone(data) + l1BaseFee, costFunc, err = extractL1GasParamsEcotone(time, data) return } } @@ -246,7 +248,7 @@ func extractL1GasParamsLegacy(isRegolith bool, data []byte) (l1BaseFee *uint256. // extractEcotoneL1GasParams extracts the gas parameters necessary to compute gas from L1 attribute // info calldata after the Ecotone upgrade, but not for the very first Ecotone block. -func extractL1GasParamsEcotone(data []byte) (l1BaseFee *uint256.Int, costFunc l1CostFunc, err error) { +func extractL1GasParamsEcotone(time uint64, data []byte) (l1BaseFee *uint256.Int, costFunc l1CostFunc, err error) { if len(data) != EcotoneL1InfoBytes { return nil, nil, fmt.Errorf("expected 164 L1 info bytes, got %d", len(data)) } @@ -266,7 +268,7 @@ func extractL1GasParamsEcotone(data []byte) (l1BaseFee *uint256.Int, costFunc l1 l1BlobBaseFee := new(uint256.Int).SetBytes(data[68:100]) l1BaseFeeScalar := new(uint256.Int).SetBytes(data[4:8]) l1BlobBaseFeeScalar := new(uint256.Int).SetBytes(data[8:12]) - costFunc = newL1CostFuncEcotone(l1BaseFee, l1BlobBaseFee, l1BaseFeeScalar, l1BlobBaseFeeScalar) + costFunc = newL1CostFuncEcotone(time, l1BaseFee, l1BlobBaseFee, l1BaseFeeScalar, l1BlobBaseFeeScalar) return } @@ -284,11 +286,11 @@ func l1CostHelper(gasWithOverhead, l1BaseFee, scalar *uint256.Int) *uint256.Int return fee } -func L1CostFnForTxPool(data []byte) (types.L1CostFn, error) { +func L1CostFnForTxPool(time uint64, data []byte) (types.L1CostFn, error) { var costFunc l1CostFunc var err error if len(data) == EcotoneL1InfoBytes { - _, costFunc, err = extractL1GasParamsEcotone(data) + _, costFunc, err = extractL1GasParamsEcotone(time, data) if err != nil { return nil, err } diff --git a/erigon-lib/opstack/rollup_cost_test.go b/erigon-lib/opstack/rollup_cost_test.go index 741803422e3..173b957baf4 100644 --- a/erigon-lib/opstack/rollup_cost_test.go +++ b/erigon-lib/opstack/rollup_cost_test.go @@ -55,7 +55,7 @@ func TestBedrockL1CostFunc(t *testing.T) { } func TestEcotoneL1CostFunc(t *testing.T) { - costFunc := newL1CostFuncEcotone(basefee, blobBasefee, basefeeScalar, blobBasefeeScalar) + costFunc := newL1CostFuncEcotone(0, basefee, blobBasefee, basefeeScalar, blobBasefeeScalar) c, g := costFunc(emptyTxRollupCostData) require.Equal(t, ecotoneGas, g) require.Equal(t, ecotoneFee, c) @@ -115,7 +115,7 @@ func TestExtractEcotoneGasParams(t *testing.T) { // make sure wrong amont of data results in error data = append(data, 0x00) // tack on garbage byte - _, _, err = extractL1GasParamsEcotone(data) + _, _, err = extractL1GasParamsEcotone(0, data) require.Error(t, err) } diff --git a/erigon-lib/txpool/pool.go b/erigon-lib/txpool/pool.go index bb18912f462..1856826a072 100644 --- a/erigon-lib/txpool/pool.go +++ b/erigon-lib/txpool/pool.go @@ -349,7 +349,7 @@ func (p *TxPool) Start(ctx context.Context, db kv.RwDB) error { }) } -func RawRLPTxToOptimismL1CostFn(payload []byte) (types.L1CostFn, error) { +func RawRLPTxToOptimismL1CostFn(blockTime uint64, payload []byte) (types.L1CostFn, error) { // skip prefix byte if len(payload) == 0 { return nil, fmt.Errorf("empty tx payload") @@ -396,7 +396,7 @@ func RawRLPTxToOptimismL1CostFn(payload []byte) (types.L1CostFn, error) { return nil, fmt.Errorf("failed to read tx data entry rlp prefix: %w", err) } txCalldata := payload[dataPos : dataPos+dataLen] - return opstack.L1CostFnForTxPool(txCalldata) + return opstack.L1CostFnForTxPool(blockTime, txCalldata) } func (p *TxPool) OnNewBlock(ctx context.Context, stateChanges *remote.StateChangeBatch, unwindTxs, unwindBlobTxs, minedTxs types.TxSlots, tx kv.Tx) error { @@ -450,7 +450,7 @@ func (p *TxPool) OnNewBlock(ctx context.Context, stateChanges *remote.StateChang if p.cfg.Optimism { lastChangeBatch := stateChanges.ChangeBatch[len(stateChanges.ChangeBatch)-1] if len(lastChangeBatch.Txs) > 0 { - l1CostFn, err := RawRLPTxToOptimismL1CostFn(lastChangeBatch.Txs[0]) + l1CostFn, err := RawRLPTxToOptimismL1CostFn(uint64(time.Now().Unix()), lastChangeBatch.Txs[0]) if err == nil { p.l1Cost = l1CostFn } else { From 7262d2041da3a931f301d09484bd9deac827349e Mon Sep 17 00:00:00 2001 From: Jason Yellick Date: Wed, 8 May 2024 17:22:38 -0400 Subject: [PATCH 2/2] Add additional check that deposit txes included This additionally removes the 'abort' path inside of the add mining txes code when dealing with deposit txes. --- eth/stagedsync/stage_mining_exec.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/eth/stagedsync/stage_mining_exec.go b/eth/stagedsync/stage_mining_exec.go index 90395618d3c..d140b072e1d 100644 --- a/eth/stagedsync/stage_mining_exec.go +++ b/eth/stagedsync/stage_mining_exec.go @@ -125,11 +125,16 @@ func SpawnMiningExecStage(s *StageState, tx kv.RwTx, cfg MiningExecCfg, quit <-c } depTS := types.NewTransactionsFixedOrder(txs) - logs, _, err := addTransactionsToMiningBlock(logPrefix, current, cfg.chainConfig, cfg.vmConfig, getHeader, cfg.engine, depTS, cfg.miningState.MiningConfig.Etherbase, ibs, quit, cfg.interrupt, cfg.payloadId, logger) + // Note, we do not allow early interrupt of adding deposit txes to the + // block, as they must all be included. + logs, _, err := addTransactionsToMiningBlock(logPrefix, current, cfg.chainConfig, cfg.vmConfig, getHeader, cfg.engine, depTS, cfg.miningState.MiningConfig.Etherbase, ibs, quit, nil, cfg.payloadId, logger) log.Debug("addTransactionsToMiningBlock (deposit) result", "err", err, "logs", logs) if err != nil { return err } + if len(current.Txs) != len(current.Deposits) { + return fmt.Errorf("all deposit transactions must commit but only %d of %d did", len(current.Txs), len(txs)) + } } if txs != nil && !txs.Empty() {