From 841d699e3a3c6ddcf7841f6de71bd1b99ebe2600 Mon Sep 17 00:00:00 2001 From: Maximilian Langenfeld <15726643+ezdac@users.noreply.github.com> Date: Thu, 1 Aug 2024 10:55:14 +0200 Subject: [PATCH 1/8] Add FeeCurrencyContext object to BlockContext This replaces the `BlockContext.ExchangeRates` with a wrapper object `BlockContext.FeeCurrencyContext`. This enables easier addition of fee-currency related information to the block context and passing around that information. The exchange-rates are now part of the `FeeCurrencyContext` --- cmd/evm/internal/t8ntool/execution.go | 7 ++++++- common/celo_types.go | 4 ++++ contracts/fee_currencies.go | 11 +++++++++++ core/celo_evm.go | 4 ++-- core/state_prefetcher.go | 2 +- core/state_processor.go | 6 +++--- core/state_transition.go | 8 ++++---- core/txpool/blobpool/blobpool.go | 6 +++--- core/txpool/blobpool/celo_blobpool.go | 6 +++--- core/txpool/celo_validation.go | 8 ++++---- core/txpool/legacypool/celo_legacypool.go | 9 +++++---- core/txpool/legacypool/legacypool.go | 14 +++++++------- core/txpool/validation.go | 2 +- core/vm/evm.go | 4 ++-- eth/state_accessor.go | 2 +- eth/tracers/api.go | 14 +++++++------- eth/tracers/api_test.go | 2 +- eth/tracers/internal/tracetest/calltrace_test.go | 4 ++-- .../internal/tracetest/flat_calltrace_test.go | 2 +- eth/tracers/internal/tracetest/prestate_test.go | 2 +- eth/tracers/tracers_test.go | 2 +- internal/ethapi/api.go | 2 +- miner/worker.go | 2 +- 23 files changed, 72 insertions(+), 51 deletions(-) diff --git a/cmd/evm/internal/t8ntool/execution.go b/cmd/evm/internal/t8ntool/execution.go index 026b5ad159..e1ba606cc8 100644 --- a/cmd/evm/internal/t8ntool/execution.go +++ b/cmd/evm/internal/t8ntool/execution.go @@ -206,7 +206,12 @@ func (pre *Prestate) Apply(vmConfig vm.Config, chainConfig *params.ChainConfig, rejectedTxs = append(rejectedTxs, &rejectedTx{i, errMsg}) continue } - msg, err := core.TransactionToMessage(tx, signer, pre.Env.BaseFee, vmContext.ExchangeRates) + // NOTE: we can't provide exchange rates + // for fee-currencies here, since those are dynamically changing + // based on the oracle's exchange rates. + // When a Celo transaction with specified fee-currency is validated with this tool, + // this will thus result in a ErrNonWhitelistedFeeCurrency error for now. + msg, err := core.TransactionToMessage(tx, signer, pre.Env.BaseFee, vmContext.FeeCurrencyContext.ExchangeRates) if err != nil { log.Warn("rejected tx", "index", i, "hash", tx.Hash(), "error", err) rejectedTxs = append(rejectedTxs, &rejectedTx{i, err.Error()}) diff --git a/common/celo_types.go b/common/celo_types.go index 3ae8a2935a..a1b981ceb5 100644 --- a/common/celo_types.go +++ b/common/celo_types.go @@ -10,6 +10,10 @@ var ( type ExchangeRates = map[Address]*big.Rat +type FeeCurrencyContext struct { + ExchangeRates ExchangeRates +} + func CurrencyWhitelist(exchangeRates ExchangeRates) []Address { addrs := make([]Address, len(exchangeRates)) i := 0 diff --git a/contracts/fee_currencies.go b/contracts/fee_currencies.go index 1b5bd51729..b0ef991bd2 100644 --- a/contracts/fee_currencies.go +++ b/contracts/fee_currencies.go @@ -140,6 +140,17 @@ func GetExchangeRates(caller bind.ContractCaller) (common.ExchangeRates, error) return exchangeRates, nil } +// GetFeeCurrencyContext returns the fee-currency context for all gas currencies from CELO +func GetFeeCurrencyContext(caller bind.ContractCaller) (common.FeeCurrencyContext, error) { + rates, err := GetExchangeRates(caller) + if err != nil { + return common.FeeCurrencyContext{}, err + } + return common.FeeCurrencyContext{ + ExchangeRates: rates, + }, nil +} + // GetBalanceERC20 returns an account's balance on a given ERC20 currency func GetBalanceERC20(caller bind.ContractCaller, accountOwner common.Address, contractAddress common.Address) (result *big.Int, err error) { token, err := abigen.NewFeeCurrencyCaller(contractAddress, caller) diff --git a/core/celo_evm.go b/core/celo_evm.go index 2c17251939..402fc6baf3 100644 --- a/core/celo_evm.go +++ b/core/celo_evm.go @@ -15,9 +15,9 @@ func setCeloFieldsInBlockContext(blockContext *vm.BlockContext, header *types.He caller := &contracts.CeloBackend{ChainConfig: config, State: statedb} - // Add fee currency exchange rates + // Add fee currency context var err error - blockContext.ExchangeRates, err = contracts.GetExchangeRates(caller) + blockContext.FeeCurrencyContext, err = contracts.GetFeeCurrencyContext(caller) if err != nil { log.Error("Error fetching exchange rates!", "err", err) } diff --git a/core/state_prefetcher.go b/core/state_prefetcher.go index 45348979f6..b769dbb6e8 100644 --- a/core/state_prefetcher.go +++ b/core/state_prefetcher.go @@ -63,7 +63,7 @@ func (p *statePrefetcher) Prefetch(block *types.Block, statedb *state.StateDB, c return } // Convert the transaction into an executable message and pre-cache its sender - msg, err := TransactionToMessage(tx, signer, header.BaseFee, blockContext.ExchangeRates) + msg, err := TransactionToMessage(tx, signer, header.BaseFee, blockContext.FeeCurrencyContext.ExchangeRates) if err != nil { return // Also invalid block, bail out } diff --git a/core/state_processor.go b/core/state_processor.go index f0f443720a..0eed561e7d 100644 --- a/core/state_processor.go +++ b/core/state_processor.go @@ -83,7 +83,7 @@ func (p *StateProcessor) Process(block *types.Block, statedb *state.StateDB, cfg } // Iterate over and process the individual transactions for i, tx := range block.Transactions() { - msg, err := TransactionToMessage(tx, signer, header.BaseFee, context.ExchangeRates) + msg, err := TransactionToMessage(tx, signer, header.BaseFee, context.FeeCurrencyContext.ExchangeRates) if err != nil { return nil, nil, 0, fmt.Errorf("could not apply tx %d [%v]: %w", i, tx.Hash().Hex(), err) } @@ -156,7 +156,7 @@ func applyTransaction(msg *Message, config *params.ChainConfig, gp *GasPool, sta if tx.Type() == types.CeloDynamicFeeTxV2Type { alternativeBaseFee := evm.Context.BaseFee if msg.FeeCurrency != nil { - alternativeBaseFee, err = exchange.ConvertCeloToCurrency(evm.Context.ExchangeRates, msg.FeeCurrency, evm.Context.BaseFee) + alternativeBaseFee, err = exchange.ConvertCeloToCurrency(evm.Context.FeeCurrencyContext.ExchangeRates, msg.FeeCurrency, evm.Context.BaseFee) if err != nil { return nil, err } @@ -189,7 +189,7 @@ func applyTransaction(msg *Message, config *params.ChainConfig, gp *GasPool, sta func ApplyTransaction(config *params.ChainConfig, bc ChainContext, author *common.Address, gp *GasPool, statedb *state.StateDB, header *types.Header, tx *types.Transaction, usedGas *uint64, cfg vm.Config) (*types.Receipt, error) { // Create a new context to be used in the EVM environment blockContext := NewEVMBlockContext(header, bc, author, config, statedb) - msg, err := TransactionToMessage(tx, types.MakeSigner(config, header.Number, header.Time), header.BaseFee, blockContext.ExchangeRates) + msg, err := TransactionToMessage(tx, types.MakeSigner(config, header.Number, header.Time), header.BaseFee, blockContext.FeeCurrencyContext.ExchangeRates) if err != nil { return nil, err } diff --git a/core/state_transition.go b/core/state_transition.go index f0c5d3c6b9..592cd1e85d 100644 --- a/core/state_transition.go +++ b/core/state_transition.go @@ -427,7 +427,7 @@ func (st *StateTransition) preCheck() error { if !st.evm.ChainConfig().IsCel2(st.evm.Context.Time) { return ErrCel2NotEnabled } else { - isWhiteListed := common.IsCurrencyWhitelisted(st.evm.Context.ExchangeRates, msg.FeeCurrency) + isWhiteListed := common.IsCurrencyWhitelisted(st.evm.Context.FeeCurrencyContext.ExchangeRates, msg.FeeCurrency) if !isWhiteListed { log.Trace("fee currency not whitelisted", "fee currency address", msg.FeeCurrency) return exchange.ErrNonWhitelistedFeeCurrency @@ -455,7 +455,7 @@ func (st *StateTransition) preCheck() error { // This will panic if baseFee is nil, but basefee presence is verified // as part of header validation. - baseFeeInFeeCurrency, err := exchange.ConvertCeloToCurrency(st.evm.Context.ExchangeRates, msg.FeeCurrency, st.evm.Context.BaseFee) + baseFeeInFeeCurrency, err := exchange.ConvertCeloToCurrency(st.evm.Context.FeeCurrencyContext.ExchangeRates, msg.FeeCurrency, st.evm.Context.BaseFee) if err != nil { return fmt.Errorf("preCheck: %w", err) } @@ -768,7 +768,7 @@ func (st *StateTransition) distributeTxFees() error { } } else { if l1Cost != nil { - l1Cost, _ = exchange.ConvertCeloToCurrency(st.evm.Context.ExchangeRates, feeCurrency, l1Cost) + l1Cost, _ = exchange.ConvertCeloToCurrency(st.evm.Context.FeeCurrencyContext.ExchangeRates, feeCurrency, l1Cost) } if err := contracts.CreditFees(st.evm, feeCurrency, from, st.evm.Context.Coinbase, feeHandlerAddress, params.OptimismL1FeeRecipient, refund, tipTxFee, baseTxFee, l1Cost); err != nil { log.Error("Error crediting", "from", from, "coinbase", st.evm.Context.Coinbase, "feeHandler", feeHandlerAddress, "err", err) @@ -790,7 +790,7 @@ func (st *StateTransition) calculateBaseFee() *big.Int { if st.msg.FeeCurrency != nil { // Existence of the fee currency has been checked in `preCheck` - baseFee, _ = exchange.ConvertCeloToCurrency(st.evm.Context.ExchangeRates, st.msg.FeeCurrency, baseFee) + baseFee, _ = exchange.ConvertCeloToCurrency(st.evm.Context.FeeCurrencyContext.ExchangeRates, st.msg.FeeCurrency, baseFee) } return baseFee diff --git a/core/txpool/blobpool/blobpool.go b/core/txpool/blobpool/blobpool.go index 470cf7633b..1cbcc81e67 100644 --- a/core/txpool/blobpool/blobpool.go +++ b/core/txpool/blobpool/blobpool.go @@ -318,8 +318,8 @@ type BlobPool struct { lock sync.RWMutex // Mutex protecting the pool during reorg handling // Celo specific - celoBackend *contracts.CeloBackend // For fee currency balances & exchange rate calculation - currentRates common.ExchangeRates // current exchange rates for fee currencies + celoBackend *contracts.CeloBackend // For fee currency balances & exchange rate calculation + feeCurrencyContext common.FeeCurrencyContext } // New creates a new blob transaction pool to gather, sort and filter inbound @@ -1094,7 +1094,7 @@ func (p *BlobPool) validateTx(tx *types.Transaction) error { MaxSize: txMaxSize, MinTip: p.gasTip.ToBig(), } - if err := txpool.CeloValidateTransaction(tx, p.head, p.signer, baseOpts, p.currentRates); err != nil { + if err := txpool.CeloValidateTransaction(tx, p.head, p.signer, baseOpts, p.feeCurrencyContext); err != nil { return err } // Ensure the transaction adheres to the stateful pool filters (nonce, balance) diff --git a/core/txpool/blobpool/celo_blobpool.go b/core/txpool/blobpool/celo_blobpool.go index b3e4cc8cff..7b453a3ca4 100644 --- a/core/txpool/blobpool/celo_blobpool.go +++ b/core/txpool/blobpool/celo_blobpool.go @@ -10,9 +10,9 @@ func (pool *BlobPool) recreateCeloProperties() { ChainConfig: pool.chain.Config(), State: pool.state, } - currentRates, err := contracts.GetExchangeRates(pool.celoBackend) + currencyContext, err := contracts.GetFeeCurrencyContext(pool.celoBackend) if err != nil { - log.Error("Error trying to get exchange rates in txpool.", "cause", err) + log.Error("Error trying to get fee currency context in txpool.", "cause", err) } - pool.currentRates = currentRates + pool.feeCurrencyContext = currencyContext } diff --git a/core/txpool/celo_validation.go b/core/txpool/celo_validation.go index 280f5dc146..19441e4d44 100644 --- a/core/txpool/celo_validation.go +++ b/core/txpool/celo_validation.go @@ -50,12 +50,12 @@ func (cvo *CeloValidationOptions) Accepts(txType uint8) bool { // This check is public to allow different transaction pools to check the basic // rules without duplicating code and running the risk of missed updates. func CeloValidateTransaction(tx *types.Transaction, head *types.Header, - signer types.Signer, opts *CeloValidationOptions, rates common.ExchangeRates) error { - if err := ValidateTransaction(tx, head, signer, opts); err != nil { + signer types.Signer, opts *CeloValidationOptions, currencyCtx common.FeeCurrencyContext) error { + if err := ValidateTransaction(tx, head, signer, opts, currencyCtx); err != nil { return err } - if !common.IsCurrencyWhitelisted(rates, tx.FeeCurrency()) { - return NonWhitelistedFeeCurrencyError + if !common.IsCurrencyWhitelisted(currencyCtx.ExchangeRates, tx.FeeCurrency()) { + return exchange.ErrNonWhitelistedFeeCurrency } return nil diff --git a/core/txpool/legacypool/celo_legacypool.go b/core/txpool/legacypool/celo_legacypool.go index 847f2e09c5..6cfb957362 100644 --- a/core/txpool/legacypool/celo_legacypool.go +++ b/core/txpool/legacypool/celo_legacypool.go @@ -12,7 +12,7 @@ import ( // and gasLimit. Returns drops and invalid txs. func (pool *LegacyPool) filter(list *list, addr common.Address, gasLimit uint64) (types.Transactions, types.Transactions) { // CELO: drop all transactions that no longer have a whitelisted currency - dropsWhitelist, invalidsWhitelist := list.FilterWhitelisted(pool.currentRates) + dropsWhitelist, invalidsWhitelist := list.FilterWhitelisted(pool.feeCurrencyContext.ExchangeRates) // Check from which currencies we need to get balances currenciesInList := list.FeeCurrencies() drops, invalids := list.Filter(pool.getBalances(addr, currenciesInList), gasLimit) @@ -34,9 +34,10 @@ func (pool *LegacyPool) recreateCeloProperties() { ChainConfig: pool.chainconfig, State: pool.currentState, } - currentRates, err := contracts.GetExchangeRates(pool.celoBackend) + feeCurrencyContext, err := contracts.GetFeeCurrencyContext(pool.celoBackend) if err != nil { - log.Error("Error trying to get exchange rates in txpool.", "cause", err) + log.Error("Error trying to get fee currency context in txpool.", "cause", err) } - pool.currentRates = currentRates + + pool.feeCurrencyContext = feeCurrencyContext } diff --git a/core/txpool/legacypool/legacypool.go b/core/txpool/legacypool/legacypool.go index 1cdc513cd2..968402d317 100644 --- a/core/txpool/legacypool/legacypool.go +++ b/core/txpool/legacypool/legacypool.go @@ -242,8 +242,8 @@ type LegacyPool struct { l1CostFn txpool.L1CostFunc // To apply L1 costs as rollup, optional field, may be nil. // Celo specific - celoBackend *contracts.CeloBackend // For fee currency balances & exchange rate calculation - currentRates common.ExchangeRates // current exchange rates for fee currencies + celoBackend *contracts.CeloBackend // For fee currency balances & exchange rate calculation + feeCurrencyContext common.FeeCurrencyContext // context for fee currencies } type txpoolResetRequest struct { @@ -650,7 +650,7 @@ func (pool *LegacyPool) validateTxBasics(tx *types.Transaction, local bool) erro if local { opts.MinTip = new(big.Int) } - if err := txpool.CeloValidateTransaction(tx, pool.currentHead.Load(), pool.signer, opts, pool.currentRates); err != nil { + if err := txpool.CeloValidateTransaction(tx, pool.currentHead.Load(), pool.signer, opts, pool.feeCurrencyContext); err != nil { return err } return nil @@ -833,7 +833,7 @@ func (pool *LegacyPool) add(tx *types.Transaction, local bool) (replaced bool, e // Try to replace an existing transaction in the pending pool if list := pool.pending[from]; list != nil && list.Contains(tx.Nonce()) { // Nonce already pending, check if required price bump is met - inserted, old := list.Add(tx, pool.config.PriceBump, pool.l1CostFn, pool.currentRates) + inserted, old := list.Add(tx, pool.config.PriceBump, pool.l1CostFn, pool.feeCurrencyContext.ExchangeRates) if !inserted { pendingDiscardMeter.Mark(1) return false, txpool.ErrReplaceUnderpriced @@ -907,7 +907,7 @@ func (pool *LegacyPool) enqueueTx(hash common.Hash, tx *types.Transaction, local if pool.queue[from] == nil { pool.queue[from] = newList(false) } - inserted, old := pool.queue[from].Add(tx, pool.config.PriceBump, pool.l1CostFn, pool.currentRates) + inserted, old := pool.queue[from].Add(tx, pool.config.PriceBump, pool.l1CostFn, pool.feeCurrencyContext.ExchangeRates) if !inserted { // An older transaction was better, discard this queuedDiscardMeter.Mark(1) @@ -961,7 +961,7 @@ func (pool *LegacyPool) promoteTx(addr common.Address, hash common.Hash, tx *typ } list := pool.pending[addr] - inserted, old := list.Add(tx, pool.config.PriceBump, pool.l1CostFn, pool.currentRates) + inserted, old := list.Add(tx, pool.config.PriceBump, pool.l1CostFn, pool.feeCurrencyContext.ExchangeRates) if !inserted { // An older transaction was better, discard this pool.all.Remove(hash) @@ -1357,7 +1357,7 @@ func (pool *LegacyPool) runReorg(done chan struct{}, reset *txpoolResetRequest, if reset.newHead != nil { if pool.chainconfig.IsLondon(new(big.Int).Add(reset.newHead.Number, big.NewInt(1))) { pendingBaseFee := eip1559.CalcBaseFee(pool.chainconfig, reset.newHead, reset.newHead.Time+1) - pool.priced.SetBaseFeeAndRates(pendingBaseFee, pool.currentRates) + pool.priced.SetBaseFeeAndRates(pendingBaseFee, pool.feeCurrencyContext.ExchangeRates) } else { pool.priced.Reheap() } diff --git a/core/txpool/validation.go b/core/txpool/validation.go index 71633ef7a8..afd332b406 100644 --- a/core/txpool/validation.go +++ b/core/txpool/validation.go @@ -74,7 +74,7 @@ type ValidationOptions struct { // This check is public to allow different transaction pools to check the basic // rules without duplicating code and running the risk of missed updates. // ONLY TO BE CALLED FROM "CeloValidateTransaction" -func ValidateTransaction(tx *types.Transaction, head *types.Header, signer types.Signer, opts *CeloValidationOptions) error { +func ValidateTransaction(tx *types.Transaction, head *types.Header, signer types.Signer, opts *CeloValidationOptions, currencyCtx common.FeeCurrencyContext) error { // No unauthenticated deposits allowed in the transaction pool. // This is for spam protection, not consensus, // as the external engine-API user authenticates deposits. diff --git a/core/vm/evm.go b/core/vm/evm.go index da66d4262e..d0bb53dbc0 100644 --- a/core/vm/evm.go +++ b/core/vm/evm.go @@ -102,8 +102,8 @@ type BlockContext struct { Random *common.Hash // Provides information for PREVRANDAO // Celo specific information - ExchangeRates common.ExchangeRates - GasUsedForDebit uint64 + GasUsedForDebit uint64 + FeeCurrencyContext common.FeeCurrencyContext } // TxContext provides the EVM with information about a transaction. diff --git a/eth/state_accessor.go b/eth/state_accessor.go index f8b7fb3160..2857e50d46 100644 --- a/eth/state_accessor.go +++ b/eth/state_accessor.go @@ -241,7 +241,7 @@ func (eth *Ethereum) stateAtTransaction(ctx context.Context, block *types.Block, for idx, tx := range block.Transactions() { context := core.NewEVMBlockContext(block.Header(), eth.blockchain, nil, eth.blockchain.Config(), statedb) // Assemble the transaction call message and return if the requested offset - msg, _ := core.TransactionToMessage(tx, signer, block.BaseFee(), context.ExchangeRates) + msg, _ := core.TransactionToMessage(tx, signer, block.BaseFee(), context.FeeCurrencyContext.ExchangeRates) txContext := core.NewEVMTxContext(msg) if idx == txIndex { return msg, context, statedb, release, nil diff --git a/eth/tracers/api.go b/eth/tracers/api.go index 5d0702b81f..10528aed6e 100644 --- a/eth/tracers/api.go +++ b/eth/tracers/api.go @@ -271,7 +271,7 @@ func (api *API) traceChain(start, end *types.Block, config *TraceConfig, closed ) // Trace all the transactions contained within for i, tx := range task.block.Transactions() { - msg, _ := core.TransactionToMessage(tx, signer, task.block.BaseFee(), blockCtx.ExchangeRates) + msg, _ := core.TransactionToMessage(tx, signer, task.block.BaseFee(), blockCtx.FeeCurrencyContext.ExchangeRates) txctx := &Context{ BlockHash: task.block.Hash(), BlockNumber: task.block.Number(), @@ -561,7 +561,7 @@ func (api *API) IntermediateRoots(ctx context.Context, hash common.Hash, config return nil, err } var ( - msg, _ = core.TransactionToMessage(tx, signer, block.BaseFee(), vmctx.ExchangeRates) + msg, _ = core.TransactionToMessage(tx, signer, block.BaseFee(), vmctx.FeeCurrencyContext.ExchangeRates) txContext = core.NewEVMTxContext(msg) vmenv = vm.NewEVM(vmctx, txContext, statedb, chainConfig, vm.Config{}) ) @@ -635,7 +635,7 @@ func (api *API) traceBlock(ctx context.Context, block *types.Block, config *Trac ) for i, tx := range txs { // Generate the next state snapshot fast without tracing - msg, _ := core.TransactionToMessage(tx, signer, block.BaseFee(), blockCtx.ExchangeRates) + msg, _ := core.TransactionToMessage(tx, signer, block.BaseFee(), blockCtx.FeeCurrencyContext.ExchangeRates) txctx := &Context{ BlockHash: blockHash, BlockNumber: block.Number(), @@ -678,7 +678,7 @@ func (api *API) traceBlockParallel(ctx context.Context, block *types.Block, stat // Fetch and execute the next transaction trace tasks for task := range jobs { blockCtx := core.NewEVMBlockContext(block.Header(), api.chainContext(ctx), nil, api.backend.ChainConfig(), task.statedb) - msg, _ := core.TransactionToMessage(txs[task.index], signer, block.BaseFee(), blockCtx.ExchangeRates) + msg, _ := core.TransactionToMessage(txs[task.index], signer, block.BaseFee(), blockCtx.FeeCurrencyContext.ExchangeRates) txctx := &Context{ BlockHash: blockHash, BlockNumber: block.Number(), @@ -710,7 +710,7 @@ txloop: } // Generate the next state snapshot fast without tracing - msg, _ := core.TransactionToMessage(tx, signer, block.BaseFee(), blockCtx.ExchangeRates) + msg, _ := core.TransactionToMessage(tx, signer, block.BaseFee(), blockCtx.FeeCurrencyContext.ExchangeRates) statedb.SetTxContext(tx.Hash(), i) vmenv := vm.NewEVM(blockCtx, core.NewEVMTxContext(msg), statedb, api.backend.ChainConfig(), vm.Config{}) if _, err := core.ApplyMessage(vmenv, msg, new(core.GasPool).AddGas(msg.GasLimit)); err != nil { @@ -790,7 +790,7 @@ func (api *API) standardTraceBlockToFile(ctx context.Context, block *types.Block for i, tx := range block.Transactions() { // Prepare the transaction for un-traced execution var ( - msg, _ = core.TransactionToMessage(tx, signer, block.BaseFee(), vmctx.ExchangeRates) + msg, _ = core.TransactionToMessage(tx, signer, block.BaseFee(), vmctx.FeeCurrencyContext.ExchangeRates) txContext = core.NewEVMTxContext(msg) vmConf vm.Config dump *os.File @@ -966,7 +966,7 @@ func (api *API) TraceCall(ctx context.Context, args ethapi.TransactionArgs, bloc config.BlockOverrides.Apply(&vmctx) } // Execute the trace - msg, err := args.ToMessage(api.backend.RPCGasCap(), vmctx.BaseFee, vmctx.ExchangeRates) + msg, err := args.ToMessage(api.backend.RPCGasCap(), vmctx.BaseFee, vmctx.FeeCurrencyContext.ExchangeRates) if err != nil { return nil, err } diff --git a/eth/tracers/api_test.go b/eth/tracers/api_test.go index f5864eb431..8352d95122 100644 --- a/eth/tracers/api_test.go +++ b/eth/tracers/api_test.go @@ -249,7 +249,7 @@ func (b *testBackend) StateAtTransaction(ctx context.Context, block *types.Block signer := types.MakeSigner(b.chainConfig, block.Number(), block.Time()) for idx, tx := range block.Transactions() { context := core.NewEVMBlockContext(block.Header(), b.chain, nil, b.chainConfig, statedb) - msg, _ := core.TransactionToMessage(tx, signer, block.BaseFee(), context.ExchangeRates) + msg, _ := core.TransactionToMessage(tx, signer, block.BaseFee(), context.FeeCurrencyContext.ExchangeRates) txContext := core.NewEVMTxContext(msg) if idx == txIndex { return msg, context, statedb, release, nil diff --git a/eth/tracers/internal/tracetest/calltrace_test.go b/eth/tracers/internal/tracetest/calltrace_test.go index 3d66163729..e3950e5d97 100644 --- a/eth/tracers/internal/tracetest/calltrace_test.go +++ b/eth/tracers/internal/tracetest/calltrace_test.go @@ -141,7 +141,7 @@ func testCallTracer(tracerName string, dirPath string, t *testing.T) { if err != nil { t.Fatalf("failed to create call tracer: %v", err) } - msg, err := core.TransactionToMessage(tx, signer, context.BaseFee, context.ExchangeRates) + msg, err := core.TransactionToMessage(tx, signer, context.BaseFee, context.FeeCurrencyContext.ExchangeRates) if err != nil { t.Fatalf("failed to prepare transaction for tracing: %v", err) } @@ -231,7 +231,7 @@ func benchTracer(tracerName string, test *callTracerTest, b *testing.B) { Difficulty: (*big.Int)(test.Context.Difficulty), GasLimit: uint64(test.Context.GasLimit), } - msg, err := core.TransactionToMessage(tx, signer, context.BaseFee, context.ExchangeRates) + msg, err := core.TransactionToMessage(tx, signer, context.BaseFee, context.FeeCurrencyContext.ExchangeRates) if err != nil { b.Fatalf("failed to prepare transaction for tracing: %v", err) } diff --git a/eth/tracers/internal/tracetest/flat_calltrace_test.go b/eth/tracers/internal/tracetest/flat_calltrace_test.go index 2d140b3ff6..88b9bd087b 100644 --- a/eth/tracers/internal/tracetest/flat_calltrace_test.go +++ b/eth/tracers/internal/tracetest/flat_calltrace_test.go @@ -102,7 +102,7 @@ func flatCallTracerTestRunner(tracerName string, filename string, dirPath string if err != nil { return fmt.Errorf("failed to create call tracer: %v", err) } - msg, err := core.TransactionToMessage(tx, signer, context.BaseFee, context.ExchangeRates) + msg, err := core.TransactionToMessage(tx, signer, context.BaseFee, context.FeeCurrencyContext.ExchangeRates) if err != nil { return fmt.Errorf("failed to prepare transaction for tracing: %v", err) } diff --git a/eth/tracers/internal/tracetest/prestate_test.go b/eth/tracers/internal/tracetest/prestate_test.go index 412f7ec7d7..a41c02cfb7 100644 --- a/eth/tracers/internal/tracetest/prestate_test.go +++ b/eth/tracers/internal/tracetest/prestate_test.go @@ -111,7 +111,7 @@ func testPrestateDiffTracer(tracerName string, dirPath string, t *testing.T) { if err != nil { t.Fatalf("failed to create call tracer: %v", err) } - msg, err := core.TransactionToMessage(tx, signer, context.BaseFee, context.ExchangeRates) + msg, err := core.TransactionToMessage(tx, signer, context.BaseFee, context.FeeCurrencyContext.ExchangeRates) if err != nil { t.Fatalf("failed to prepare transaction for tracing: %v", err) } diff --git a/eth/tracers/tracers_test.go b/eth/tracers/tracers_test.go index c3c5d4746f..7930ee0261 100644 --- a/eth/tracers/tracers_test.go +++ b/eth/tracers/tracers_test.go @@ -90,7 +90,7 @@ func BenchmarkTransactionTrace(b *testing.B) { //EnableReturnData: false, }) evm := vm.NewEVM(context, txContext, state.StateDB, params.AllEthashProtocolChanges, vm.Config{Tracer: tracer}) - msg, err := core.TransactionToMessage(tx, signer, context.BaseFee, context.ExchangeRates) + msg, err := core.TransactionToMessage(tx, signer, context.BaseFee, context.FeeCurrencyContext.ExchangeRates) if err != nil { b.Fatalf("failed to prepare transaction for tracing: %v", err) } diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index 6d51d621ff..61f2b4e19e 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -1187,7 +1187,7 @@ func doCall(ctx context.Context, b Backend, args TransactionArgs, state *state.S if blockOverrides != nil { blockOverrides.Apply(&blockCtx) } - msg, err := args.ToMessage(globalGasCap, blockCtx.BaseFee, blockCtx.ExchangeRates) + msg, err := args.ToMessage(globalGasCap, blockCtx.BaseFee, blockCtx.FeeCurrencyContext.ExchangeRates) if err != nil { return nil, err } diff --git a/miner/worker.go b/miner/worker.go index 7a21bef643..0576346f9d 100644 --- a/miner/worker.go +++ b/miner/worker.go @@ -1137,7 +1137,7 @@ func (w *worker) prepareWork(genParams *generateParams) (*environment, error) { return nil, err } context := core.NewEVMBlockContext(header, w.chain, nil, w.chainConfig, env.state) - env.feeCurrencyWhitelist = common.CurrencyWhitelist(context.ExchangeRates) + env.feeCurrencyWhitelist = common.CurrencyWhitelist(context.FeeCurrencyContext.ExchangeRates) if header.ParentBeaconRoot != nil { vmenv := vm.NewEVM(context, vm.TxContext{}, env.state, w.chainConfig, vm.Config{}) core.ProcessBeaconBlockRoot(*header.ParentBeaconRoot, vmenv, env.state) From c70679f221ddd38a4000528021963be7c9cfc5ab Mon Sep 17 00:00:00 2001 From: Maximilian Langenfeld <15726643+ezdac@users.noreply.github.com> Date: Thu, 1 Aug 2024 12:40:22 +0200 Subject: [PATCH 2/8] Add intrinsic gas retrieval from FeeCurrencyDirectory --- common/celo_types.go | 24 +++++++++- contracts/fee_currencies.go | 92 ++++++++++++++++++++++++++++--------- 2 files changed, 93 insertions(+), 23 deletions(-) diff --git a/common/celo_types.go b/common/celo_types.go index a1b981ceb5..442d333438 100644 --- a/common/celo_types.go +++ b/common/celo_types.go @@ -9,9 +9,31 @@ var ( ) type ExchangeRates = map[Address]*big.Rat +type IntrinsicGasCosts = map[Address]uint64 type FeeCurrencyContext struct { - ExchangeRates ExchangeRates + ExchangeRates ExchangeRates + IntrinsicGasCosts IntrinsicGasCosts +} + +func MaxAllowedIntrinsicGasCost(i IntrinsicGasCosts, feeCurrency *Address) (uint64, bool) { + intrinsicGas, ok := CurrencyIntrinsicGasCost(i, feeCurrency) + if !ok { + return 0, false + } + // allow the contract to overshoot 2 times the deducted intrinsic gas + // during execution + return intrinsicGas * 3, true +} +func CurrencyIntrinsicGasCost(i IntrinsicGasCosts, feeCurrency *Address) (uint64, bool) { + if feeCurrency == nil { + return 0, true + } + gasCost, ok := i[*feeCurrency] + if !ok { + return 0, false + } + return gasCost, true } func CurrencyWhitelist(exchangeRates ExchangeRates) []Address { diff --git a/contracts/fee_currencies.go b/contracts/fee_currencies.go index b0ef991bd2..f52e54027e 100644 --- a/contracts/fee_currencies.go +++ b/contracts/fee_currencies.go @@ -2,6 +2,7 @@ package contracts import ( "fmt" + "math" "math/big" "github.com/ethereum/go-ethereum/accounts/abi" @@ -112,42 +113,51 @@ func CreditFees( return err } -// GetExchangeRates returns the exchange rates for all gas currencies from CELO +func GetRegisteredCurrencies(caller *abigen.FeeCurrencyDirectoryCaller) ([]common.Address, error) { + currencies, err := caller.GetCurrencies(&bind.CallOpts{}) + if err != nil { + return currencies, fmt.Errorf("Failed to get registered tokens: %w", err) + } + return currencies, nil +} + +// GetExchangeRates returns the exchange rates for the provided gas currencies from CELO func GetExchangeRates(caller bind.ContractCaller) (common.ExchangeRates, error) { exchangeRates := map[common.Address]*big.Rat{} directory, err := abigen.NewFeeCurrencyDirectoryCaller(addresses.FeeCurrencyDirectoryAddress, caller) if err != nil { return exchangeRates, fmt.Errorf("Failed to access FeeCurrencyDirectory: %w", err) } - - registeredTokens, err := directory.GetCurrencies(&bind.CallOpts{}) + currencies, err := GetRegisteredCurrencies(directory) if err != nil { - return exchangeRates, fmt.Errorf("Failed to get whitelisted tokens: %w", err) + return map[common.Address]*big.Rat{}, err } - for _, tokenAddress := range registeredTokens { - rate, err := directory.GetExchangeRate(&bind.CallOpts{}, tokenAddress) - if err != nil { - log.Error("Failed to get medianRate for gas currency!", "err", err, "tokenAddress", tokenAddress.Hex()) - continue - } - if rate.Numerator.Sign() <= 0 || rate.Denominator.Sign() <= 0 { - log.Error("Bad exchange rate for fee currency", "tokenAddress", tokenAddress.Hex(), "numerator", rate.Numerator, "denominator", rate.Denominator) - continue - } - exchangeRates[tokenAddress] = new(big.Rat).SetFrac(rate.Numerator, rate.Denominator) - } - - return exchangeRates, nil + return getExchangeRatesForTokens(directory, currencies) } -// GetFeeCurrencyContext returns the fee-currency context for all gas currencies from CELO +// GetFeeCurrencyContext returns the fee currency block context for all registered gas currencies from CELO func GetFeeCurrencyContext(caller bind.ContractCaller) (common.FeeCurrencyContext, error) { - rates, err := GetExchangeRates(caller) + var feeContext common.FeeCurrencyContext + directory, err := abigen.NewFeeCurrencyDirectoryCaller(addresses.FeeCurrencyDirectoryAddress, caller) + if err != nil { + return feeContext, fmt.Errorf("Failed to access FeeCurrencyDirectory: %w", err) + } + + currencies, err := GetRegisteredCurrencies(directory) + if err != nil { + return feeContext, err + } + rates, err := getExchangeRatesForTokens(directory, currencies) if err != nil { - return common.FeeCurrencyContext{}, err + return feeContext, err + } + intrinsicGas, err := getIntrinsicGasForTokens(directory, currencies) + if err != nil { + return feeContext, err } return common.FeeCurrencyContext{ - ExchangeRates: rates, + ExchangeRates: rates, + IntrinsicGasCosts: intrinsicGas, }, nil } @@ -178,3 +188,41 @@ func GetFeeBalance(backend *CeloBackend, account common.Address, feeCurrency *co } return balance } + +// getIntrinsicGasForTokens returns the intrinsic gas costs for the provided gas currencies from CELO +func getIntrinsicGasForTokens(caller *abigen.FeeCurrencyDirectoryCaller, tokens []common.Address) (common.IntrinsicGasCosts, error) { + gasCosts := common.IntrinsicGasCosts{} + for _, tokenAddress := range tokens { + config, err := caller.GetCurrencyConfig(&bind.CallOpts{}, tokenAddress) + if err != nil { + log.Error("Failed to get intrinsic gas cost for gas currency!", "err", err, "tokenAddress", tokenAddress.Hex()) + continue + } + if !config.IntrinsicGas.IsUint64() { + log.Error("Intrinsic gas cost exceeds MaxUint64 limit, capping at MaxUint64", "err", err, "tokenAddress", tokenAddress.Hex()) + gasCosts[tokenAddress] = math.MaxUint64 + } else { + gasCosts[tokenAddress] = config.IntrinsicGas.Uint64() + } + } + return gasCosts, nil +} + +// getExchangeRatesForTokens returns the exchange rates for the provided gas currencies from CELO +func getExchangeRatesForTokens(caller *abigen.FeeCurrencyDirectoryCaller, tokens []common.Address) (common.ExchangeRates, error) { + exchangeRates := common.ExchangeRates{} + for _, tokenAddress := range tokens { + rate, err := caller.GetExchangeRate(&bind.CallOpts{}, tokenAddress) + if err != nil { + log.Error("Failed to get medianRate for gas currency!", "err", err, "tokenAddress", tokenAddress.Hex()) + continue + } + if rate.Numerator.Sign() <= 0 || rate.Denominator.Sign() <= 0 { + log.Error("Bad exchange rate for fee currency", "tokenAddress", tokenAddress.Hex(), "numerator", rate.Numerator, "denominator", rate.Denominator) + continue + } + exchangeRates[tokenAddress] = new(big.Rat).SetFrac(rate.Numerator, rate.Denominator) + } + + return exchangeRates, nil +} From 1db60d701bff56867b0ba77a04bb0ca8acc41af4 Mon Sep 17 00:00:00 2001 From: Maximilian Langenfeld <15726643+ezdac@users.noreply.github.com> Date: Wed, 10 Jul 2024 16:58:50 +0200 Subject: [PATCH 3/8] Use intrinsic gas from FeeCurrencyDirectory in STF --- contracts/fee_currencies.go | 62 ++++++++++++++++++---------- core/state_transition.go | 42 ++++++++++++++++--- core/txpool/celo_validation.go | 4 +- core/txpool/legacypool/legacypool.go | 5 ++- core/txpool/validation.go | 11 ++++- core/vm/evm.go | 1 - e2e_test/js-tests/test_viem_tx.mjs | 5 +-- 7 files changed, 93 insertions(+), 37 deletions(-) diff --git a/contracts/fee_currencies.go b/contracts/fee_currencies.go index f52e54027e..66a04f589d 100644 --- a/contracts/fee_currencies.go +++ b/contracts/fee_currencies.go @@ -1,6 +1,7 @@ package contracts import ( + "errors" "fmt" "math" "math/big" @@ -8,6 +9,7 @@ import ( "github.com/ethereum/go-ethereum/accounts/abi" "github.com/ethereum/go-ethereum/accounts/abi/bind" "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/common/exchange" "github.com/ethereum/go-ethereum/contracts/addresses" "github.com/ethereum/go-ethereum/contracts/celo/abigen" "github.com/ethereum/go-ethereum/core/types" @@ -15,15 +17,6 @@ import ( "github.com/ethereum/go-ethereum/log" ) -const ( - Thousand = 1000 - - // Default intrinsic gas cost of transactions paying for gas in alternative currencies. - // Calculated to estimate 1 balance read, 1 debit, and 4 credit transactions. - IntrinsicGasForAlternativeFeeCurrency uint64 = 50 * Thousand - maxAllowedGasForDebitAndCredit uint64 = 3 * IntrinsicGasForAlternativeFeeCurrency -) - var feeCurrencyABI *abi.ABI func init() { @@ -35,31 +28,43 @@ func init() { } // Returns nil if debit is possible, used in tx pool validation -func TryDebitFees(tx *types.Transaction, from common.Address, backend *CeloBackend) error { +func TryDebitFees(tx *types.Transaction, from common.Address, backend *CeloBackend, feeContext common.FeeCurrencyContext) error { amount := new(big.Int).SetUint64(tx.Gas()) amount.Mul(amount, tx.GasFeeCap()) snapshot := backend.State.Snapshot() - err := DebitFees(backend.NewEVM(), tx.FeeCurrency(), from, amount) + evm := backend.NewEVM() + evm.Context.FeeCurrencyContext = feeContext + _, err := DebitFees(evm, tx.FeeCurrency(), from, amount) backend.State.RevertToSnapshot(snapshot) return err } // Debits transaction fees from the transaction sender and stores them in the temporary address -func DebitFees(evm *vm.EVM, feeCurrency *common.Address, address common.Address, amount *big.Int) error { +func DebitFees(evm *vm.EVM, feeCurrency *common.Address, address common.Address, amount *big.Int) (uint64, error) { if amount.Cmp(big.NewInt(0)) == 0 { - return nil + return 0, nil + } + + maxIntrinsicGasCost, ok := common.MaxAllowedIntrinsicGasCost(evm.Context.FeeCurrencyContext.IntrinsicGasCosts, feeCurrency) + if !ok { + return 0, exchange.ErrNonWhitelistedFeeCurrency } leftoverGas, err := evm.CallWithABI( - feeCurrencyABI, "debitGasFees", *feeCurrency, maxAllowedGasForDebitAndCredit, + feeCurrencyABI, "debitGasFees", *feeCurrency, maxIntrinsicGasCost, // debitGasFees(address from, uint256 value) parameters address, amount, ) - gasUsed := maxAllowedGasForDebitAndCredit - leftoverGas - evm.Context.GasUsedForDebit = gasUsed + if errors.Is(err, vm.ErrOutOfGas) { + // This basically is a configuration / contract error, since + // the contract itself used way more gas than was expected (including grace limit) + return 0, fmt.Errorf("surpassed maximum allowed intrinsic gas for fee currency: %w", err) + } + + gasUsed := maxIntrinsicGasCost - leftoverGas log.Trace("DebitFees called", "feeCurrency", *feeCurrency, "gasUsed", gasUsed) - return err + return gasUsed, err } // Credits fees to the respective parties @@ -72,6 +77,7 @@ func CreditFees( feeCurrency *common.Address, txSender, tipReceiver, baseFeeReceiver, l1DataFeeReceiver common.Address, refund, feeTip, baseFee, l1DataFee *big.Int, + gasUsedDebit uint64, ) error { // Our old `creditGasFees` function does not accept an l1DataFee and // the fee currencies do not implement the new interface yet. Since tip @@ -86,8 +92,12 @@ func CreditFees( if tipReceiver.Cmp(common.ZeroAddress) == 0 { tipReceiver = baseFeeReceiver } + maxAllowedGasForDebitAndCredit, ok := common.MaxAllowedIntrinsicGasCost(evm.Context.FeeCurrencyContext.IntrinsicGasCosts, feeCurrency) + if !ok { + return exchange.ErrNonWhitelistedFeeCurrency + } - maxAllowedGasForCredit := maxAllowedGasForDebitAndCredit - evm.Context.GasUsedForDebit + maxAllowedGasForCredit := maxAllowedGasForDebitAndCredit - gasUsedDebit leftoverGas, err := evm.CallWithABI( feeCurrencyABI, "creditGasFees", *feeCurrency, maxAllowedGasForCredit, // function creditGasFees( @@ -102,13 +112,23 @@ func CreditFees( // ) txSender, tipReceiver, common.ZeroAddress, baseFeeReceiver, refund, feeTip, common.Big0, baseFee, ) + if errors.Is(err, vm.ErrOutOfGas) { + // This is a configuration / contract error, since + // the contract itself used way more gas than was expected (including grace limit) + return fmt.Errorf("surpassed maximum allowed intrinsic gas for fee currency: %w", err) + } gasUsed := maxAllowedGasForCredit - leftoverGas log.Trace("CreditFees called", "feeCurrency", *feeCurrency, "gasUsed", gasUsed) - gasUsedForDebitAndCredit := evm.Context.GasUsedForDebit + gasUsed - if gasUsedForDebitAndCredit > IntrinsicGasForAlternativeFeeCurrency { - log.Info("Gas usage for debit+credit exceeds intrinsic gas!", "gasUsed", gasUsedForDebitAndCredit, "intrinsicGas", IntrinsicGasForAlternativeFeeCurrency, "feeCurrency", feeCurrency) + intrinsicGas, ok := common.CurrencyIntrinsicGasCost(evm.Context.FeeCurrencyContext.IntrinsicGasCosts, feeCurrency) + if !ok { + // this will never happen + return exchange.ErrNonWhitelistedFeeCurrency + } + gasUsedForDebitAndCredit := gasUsedDebit + gasUsed + if gasUsedForDebitAndCredit > intrinsicGas { + log.Info("Gas usage for debit+credit exceeds intrinsic gas!", "gasUsed", gasUsedForDebitAndCredit, "intrinsicGas", intrinsicGas, "feeCurrency", feeCurrency) } return err } diff --git a/core/state_transition.go b/core/state_transition.go index 592cd1e85d..80c44b7c2e 100644 --- a/core/state_transition.go +++ b/core/state_transition.go @@ -71,7 +71,7 @@ func (result *ExecutionResult) Revert() []byte { } // IntrinsicGas computes the 'intrinsic gas' for a message with the given data. -func IntrinsicGas(data []byte, accessList types.AccessList, isContractCreation bool, isHomestead, isEIP2028, isEIP3860 bool, feeCurrency *common.Address) (uint64, error) { +func IntrinsicGas(data []byte, accessList types.AccessList, isContractCreation bool, isHomestead, isEIP2028, isEIP3860 bool, feeCurrency *common.Address, feeIntrinsicGas common.IntrinsicGasCosts) (uint64, error) { // Set the starting gas for the raw transaction var gas uint64 if isContractCreation && isHomestead { @@ -129,10 +129,14 @@ func IntrinsicGas(data []byte, accessList types.AccessList, isContractCreation b // In this case, however, the user always ends up paying `maxGasForDebitAndCreditTransactions` // keeping it consistent. if feeCurrency != nil { - if (math.MaxUint64 - gas) < contracts.IntrinsicGasForAlternativeFeeCurrency { + intrinsicGasForFeeCurrency, ok := common.CurrencyIntrinsicGasCost(feeIntrinsicGas, feeCurrency) + if !ok { + return 0, exchange.ErrNonWhitelistedFeeCurrency + } + if (math.MaxUint64 - gas) < intrinsicGasForFeeCurrency { return 0, ErrGasUintOverflow } - gas += contracts.IntrinsicGasForAlternativeFeeCurrency + gas += intrinsicGasForFeeCurrency } if accessList != nil { @@ -274,6 +278,8 @@ type StateTransition struct { initialGas uint64 state vm.StateDB evm *vm.EVM + + feeCurrencyGasUsed uint64 } // NewStateTransition initialises and returns a new state transition object. @@ -379,7 +385,9 @@ func (st *StateTransition) subFees(effectiveFee *big.Int) (err error) { st.state.SubBalance(st.msg.From, effectiveFeeU256) return nil } else { - return contracts.DebitFees(st.evm, st.msg.FeeCurrency, st.msg.From, effectiveFee) + gasUsedDebit, err := contracts.DebitFees(st.evm, st.msg.FeeCurrency, st.msg.From, effectiveFee) + st.feeCurrencyGasUsed += gasUsedDebit + return err } } @@ -586,7 +594,16 @@ func (st *StateTransition) innerTransitionDb() (*ExecutionResult, error) { } // Check clauses 4-5, subtract intrinsic gas if everything is correct - gas, err := IntrinsicGas(msg.Data, msg.AccessList, contractCreation, rules.IsHomestead, rules.IsIstanbul, rules.IsShanghai, msg.FeeCurrency) + gas, err := IntrinsicGas( + msg.Data, + msg.AccessList, + contractCreation, + rules.IsHomestead, + rules.IsIstanbul, + rules.IsShanghai, + msg.FeeCurrency, + st.evm.Context.FeeCurrencyContext.IntrinsicGasCosts, + ) if err != nil { return nil, err } @@ -770,7 +787,20 @@ func (st *StateTransition) distributeTxFees() error { if l1Cost != nil { l1Cost, _ = exchange.ConvertCeloToCurrency(st.evm.Context.FeeCurrencyContext.ExchangeRates, feeCurrency, l1Cost) } - if err := contracts.CreditFees(st.evm, feeCurrency, from, st.evm.Context.Coinbase, feeHandlerAddress, params.OptimismL1FeeRecipient, refund, tipTxFee, baseTxFee, l1Cost); err != nil { + if err := contracts.CreditFees( + st.evm, + feeCurrency, + from, + st.evm.Context.Coinbase, + feeHandlerAddress, + params.OptimismL1FeeRecipient, + refund, + tipTxFee, + baseTxFee, + l1Cost, + st.feeCurrencyGasUsed, + ); err != nil { + err = fmt.Errorf("error crediting fee-currency: %w", err) log.Error("Error crediting", "from", from, "coinbase", st.evm.Context.Coinbase, "feeHandler", feeHandlerAddress, "err", err) return err } diff --git a/core/txpool/celo_validation.go b/core/txpool/celo_validation.go index 19441e4d44..0801c05ece 100644 --- a/core/txpool/celo_validation.go +++ b/core/txpool/celo_validation.go @@ -1,16 +1,14 @@ package txpool import ( - "errors" "math/big" "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/common/exchange" "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/params" ) -var NonWhitelistedFeeCurrencyError = errors.New("Fee currency given is not whitelisted at current block") - // AcceptSet is a set of accepted transaction types for a transaction subpool. type AcceptSet = map[uint8]struct{} diff --git a/core/txpool/legacypool/legacypool.go b/core/txpool/legacypool/legacypool.go index 968402d317..f26b03eb9b 100644 --- a/core/txpool/legacypool/legacypool.go +++ b/core/txpool/legacypool/legacypool.go @@ -713,7 +713,10 @@ func (pool *LegacyPool) validateTx(tx *types.Transaction, local bool) error { log.Error("Transaction sender recovery failed", "err", err) return err } - return contracts.TryDebitFees(tx, from, pool.celoBackend) + //NOTE: we only test the `debitFees` call here. + // If the `creditFees` reverts (or runs out of gas), the transaction will + // not be invalidated here and will be included in the block. + return contracts.TryDebitFees(tx, from, pool.celoBackend, pool.feeCurrencyContext) } return nil } diff --git a/core/txpool/validation.go b/core/txpool/validation.go index afd332b406..b2f438eea0 100644 --- a/core/txpool/validation.go +++ b/core/txpool/validation.go @@ -133,7 +133,16 @@ func ValidateTransaction(tx *types.Transaction, head *types.Header, signer types } // Ensure the transaction has more gas than the bare minimum needed to cover // the transaction metadata - intrGas, err := core.IntrinsicGas(tx.Data(), tx.AccessList(), tx.To() == nil, true, opts.Config.IsIstanbul(head.Number), opts.Config.IsShanghai(head.Number, head.Time), tx.FeeCurrency()) + intrGas, err := core.IntrinsicGas( + tx.Data(), + tx.AccessList(), + tx.To() == nil, + true, + opts.Config.IsIstanbul(head.Number), + opts.Config.IsShanghai(head.Number, head.Time), + tx.FeeCurrency(), + currencyCtx.IntrinsicGasCosts, + ) if err != nil { return err } diff --git a/core/vm/evm.go b/core/vm/evm.go index d0bb53dbc0..1c00927cbf 100644 --- a/core/vm/evm.go +++ b/core/vm/evm.go @@ -102,7 +102,6 @@ type BlockContext struct { Random *common.Hash // Provides information for PREVRANDAO // Celo specific information - GasUsedForDebit uint64 FeeCurrencyContext common.FeeCurrencyContext } diff --git a/e2e_test/js-tests/test_viem_tx.mjs b/e2e_test/js-tests/test_viem_tx.mjs index 91d5e425a2..01a511a219 100644 --- a/e2e_test/js-tests/test_viem_tx.mjs +++ b/e2e_test/js-tests/test_viem_tx.mjs @@ -263,10 +263,7 @@ describe("viem send tx", () => { assert.fail("Failed to filter nonwhitelisted feeCurrency"); } catch (err) { // TODO: find a better way to check the error type - if ( - err.cause.details == - "Fee currency given is not whitelisted at current block" - ) { + if (err.cause.details == "non-whitelisted fee currency address") { // Test success } else { throw err; From 8e525058ba3c981a79375022eeb9ff60824fa233 Mon Sep 17 00:00:00 2001 From: Maximilian Langenfeld <15726643+ezdac@users.noreply.github.com> Date: Thu, 11 Jul 2024 16:16:57 +0200 Subject: [PATCH 4/8] Add test for intrinsic gas too high in fee currency --- .../debug-fee-currency/DebugFeeCurrency.sol | 21 ++++++++++++++++++- .../debug-fee-currency/deploy_and_send_tx.sh | 2 +- e2e_test/test_fee_currency_fails_intrinsic.sh | 13 ++++++++++++ e2e_test/test_fee_currency_fails_on_credit.sh | 6 +++--- e2e_test/test_fee_currency_fails_on_debit.sh | 6 +++--- 5 files changed, 40 insertions(+), 8 deletions(-) create mode 100755 e2e_test/test_fee_currency_fails_intrinsic.sh diff --git a/e2e_test/debug-fee-currency/DebugFeeCurrency.sol b/e2e_test/debug-fee-currency/DebugFeeCurrency.sol index e5d6e1ce3a..7f269dd5f2 100644 --- a/e2e_test/debug-fee-currency/DebugFeeCurrency.sol +++ b/e2e_test/debug-fee-currency/DebugFeeCurrency.sol @@ -702,11 +702,14 @@ contract FeeCurrency is ERC20, IFeeCurrency { contract DebugFeeCurrency is ERC20, IFeeCurrency { bool failOnDebit; bool failOnCredit; + bool highGasOnCredit; + mapping(uint256 => uint256) private _dummyMap; - constructor(uint256 initialSupply, bool _failOnDebit, bool _failOnCredit) ERC20("DebugFeeCurrency", "DFC") { + constructor(uint256 initialSupply, bool _failOnDebit, bool _failOnCredit, bool _highGasOnCredit) ERC20("DebugFeeCurrency", "DFC") { _mint(msg.sender, initialSupply); failOnDebit = _failOnDebit; failOnCredit = _failOnCredit; + highGasOnCredit = _highGasOnCredit; } modifier onlyVm() { @@ -727,6 +730,18 @@ contract DebugFeeCurrency is ERC20, IFeeCurrency { for (uint256 i = 0; i < recipients.length; i++) { _mint(recipients[i], amounts[i]); } + + if (highGasOnCredit){ + induceHighGasCost(); + } + } + + function induceHighGasCost() internal view { + // SLOAD for non existing touched_storage_slots + // 2100 * 3000 gas = 630.0000 + for (uint256 i = 0; i < 3000; i++) { + _dummyMap[i]; + } } // Old function signature for backwards compatibility @@ -746,6 +761,10 @@ contract DebugFeeCurrency is ERC20, IFeeCurrency { _mint(from, refund); _mint(feeRecipient, tipTxFee); _mint(communityFund, baseTxFee); + + if (highGasOnCredit){ + induceHighGasCost(); + } } } diff --git a/e2e_test/debug-fee-currency/deploy_and_send_tx.sh b/e2e_test/debug-fee-currency/deploy_and_send_tx.sh index 76470fa53a..1cfd9eec73 100755 --- a/e2e_test/debug-fee-currency/deploy_and_send_tx.sh +++ b/e2e_test/debug-fee-currency/deploy_and_send_tx.sh @@ -3,7 +3,7 @@ set -xeo pipefail export FEE_CURRENCY=$(\ - forge create --root . --contracts . --private-key $ACC_PRIVKEY DebugFeeCurrency.sol:DebugFeeCurrency --constructor-args '100000000000000000000000000' $1 $2 --json\ + forge create --root . --contracts . --private-key $ACC_PRIVKEY DebugFeeCurrency.sol:DebugFeeCurrency --constructor-args '100000000000000000000000000' $1 $2 $3 --json\ | jq .deployedTo -r) cast send --private-key $ACC_PRIVKEY $ORACLE3 'setExchangeRate(address, uint256, uint256)' $FEE_CURRENCY 2ether 1ether diff --git a/e2e_test/test_fee_currency_fails_intrinsic.sh b/e2e_test/test_fee_currency_fails_intrinsic.sh new file mode 100755 index 0000000000..68213b5b7d --- /dev/null +++ b/e2e_test/test_fee_currency_fails_intrinsic.sh @@ -0,0 +1,13 @@ +#!/bin/bash +#shellcheck disable=SC2086 +set -eo pipefail + +source shared.sh + +# Expect that the creditGasFees failed and is logged by geth +tail -f -n0 geth.log >debug-fee-currency/geth.intrinsic.log & # start log capture +(cd debug-fee-currency && ./deploy_and_send_tx.sh false false true) +sleep 0.5 +kill %1 # stop log capture +grep "error crediting fee-currency: surpassed maximum allowed intrinsic gas for fee currency: out of gas" debug-fee-currency/geth.intrinsic.log +# echo $(grep "send_tx hash:" debug-fee-currency/send_tx.intrinsic.log) diff --git a/e2e_test/test_fee_currency_fails_on_credit.sh b/e2e_test/test_fee_currency_fails_on_credit.sh index 137ea6c227..dbf16dfa2d 100755 --- a/e2e_test/test_fee_currency_fails_on_credit.sh +++ b/e2e_test/test_fee_currency_fails_on_credit.sh @@ -5,8 +5,8 @@ set -eo pipefail source shared.sh # Expect that the creditGasFees failed and is logged by geth -tail -f -n0 geth.log > debug-fee-currency/geth.partial.log & # start log capture -(cd debug-fee-currency && ./deploy_and_send_tx.sh false true) -sleep 0.1 +tail -f -n0 geth.log >debug-fee-currency/geth.partial.log & # start log capture +(cd debug-fee-currency && ./deploy_and_send_tx.sh false true false) +sleep 0.5 kill %1 # stop log capture grep "This DebugFeeCurrency always fails in (old) creditGasFees!" debug-fee-currency/geth.partial.log diff --git a/e2e_test/test_fee_currency_fails_on_debit.sh b/e2e_test/test_fee_currency_fails_on_debit.sh index 0b4e4438f3..105c448e36 100755 --- a/e2e_test/test_fee_currency_fails_on_debit.sh +++ b/e2e_test/test_fee_currency_fails_on_debit.sh @@ -5,6 +5,6 @@ set -eo pipefail source shared.sh # Expect that the debitGasFees fails during tx submission -(cd debug-fee-currency && ./deploy_and_send_tx.sh true false) &> debug-fee-currency/send_tx.log || true -grep "debitGasFees reverted: This DebugFeeCurrency always fails in debitGasFees!" debug-fee-currency/send_tx.log \ - || (cat debug-fee-currency/send_tx.log && false) +(cd debug-fee-currency && ./deploy_and_send_tx.sh true false false) &>debug-fee-currency/send_tx.log || true +grep "debitGasFees reverted: This DebugFeeCurrency always fails in debitGasFees!" debug-fee-currency/send_tx.log || + (cat debug-fee-currency/send_tx.log && false) From 17473b34a54b41140e66e77e724a0de6f7f8bed5 Mon Sep 17 00:00:00 2001 From: Maximilian Langenfeld <15726643+ezdac@users.noreply.github.com> Date: Tue, 30 Jul 2024 17:27:38 +0200 Subject: [PATCH 5/8] Add empty fee-currency context to tools and tests --- cmd/evm/internal/t8ntool/transaction.go | 8 +++++++- core/bench_test.go | 3 ++- tests/transaction_test_util.go | 3 ++- 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/cmd/evm/internal/t8ntool/transaction.go b/cmd/evm/internal/t8ntool/transaction.go index 1ee2d5142a..379837eef5 100644 --- a/cmd/evm/internal/t8ntool/transaction.go +++ b/cmd/evm/internal/t8ntool/transaction.go @@ -133,8 +133,14 @@ func Transaction(ctx *cli.Context) error { r.Address = sender } // Check intrinsic gas + // NOTE: we can't provide specific intrinsic gas costs + // for fee-currencies here, since those are written to the + // FeeCurrencyDirectory contract and are chain-specific. + // When a Celo transaction with specified fee-currency is validated with this tool, + // this will thus result in a ErrNonWhitelistedFeeCurrency error for now. + var feeIntrinsic common.IntrinsicGasCosts if gas, err := core.IntrinsicGas(tx.Data(), tx.AccessList(), tx.To() == nil, - chainConfig.IsHomestead(new(big.Int)), chainConfig.IsIstanbul(new(big.Int)), chainConfig.IsShanghai(new(big.Int), 0), tx.FeeCurrency()); err != nil { + chainConfig.IsHomestead(new(big.Int)), chainConfig.IsIstanbul(new(big.Int)), chainConfig.IsShanghai(new(big.Int), 0), tx.FeeCurrency(), feeIntrinsic); err != nil { r.Error = err results = append(results, r) continue diff --git a/core/bench_test.go b/core/bench_test.go index 893758029f..55a1a9104f 100644 --- a/core/bench_test.go +++ b/core/bench_test.go @@ -83,7 +83,8 @@ func genValueTx(nbytes int) func(int, *BlockGen) { return func(i int, gen *BlockGen) { toaddr := common.Address{} data := make([]byte, nbytes) - gas, _ := IntrinsicGas(data, nil, false, false, false, false, nil) + var feeIntrinsic common.IntrinsicGasCosts + gas, _ := IntrinsicGas(data, nil, false, false, false, false, nil, feeIntrinsic) signer := gen.Signer() gasPrice := big.NewInt(0) if gen.header.BaseFee != nil { diff --git a/tests/transaction_test_util.go b/tests/transaction_test_util.go index 8f8c07ea53..22c60f7c57 100644 --- a/tests/transaction_test_util.go +++ b/tests/transaction_test_util.go @@ -55,7 +55,8 @@ func (tt *TransactionTest) Run(config *params.ChainConfig) error { return nil, nil, err } // Intrinsic gas - requiredGas, err := core.IntrinsicGas(tx.Data(), tx.AccessList(), tx.To() == nil, isHomestead, isIstanbul, false, nil) + var feeIntrinsic common.IntrinsicGasCosts + requiredGas, err := core.IntrinsicGas(tx.Data(), tx.AccessList(), tx.To() == nil, isHomestead, isIstanbul, false, nil, feeIntrinsic) if err != nil { return nil, nil, err } From ff0a17074c3339d703f0c4de43c6a76118972988 Mon Sep 17 00:00:00 2001 From: Maximilian Langenfeld <15726643+ezdac@users.noreply.github.com> Date: Mon, 5 Aug 2024 11:26:26 +0200 Subject: [PATCH 6/8] Add minor code improvements Co-authored-by: Paul Lange --- common/celo_types.go | 1 + contracts/fee_currencies.go | 4 ++-- core/bench_test.go | 3 +-- tests/transaction_test_util.go | 3 +-- 4 files changed, 5 insertions(+), 6 deletions(-) diff --git a/common/celo_types.go b/common/celo_types.go index 442d333438..44c59e857d 100644 --- a/common/celo_types.go +++ b/common/celo_types.go @@ -25,6 +25,7 @@ func MaxAllowedIntrinsicGasCost(i IntrinsicGasCosts, feeCurrency *Address) (uint // during execution return intrinsicGas * 3, true } + func CurrencyIntrinsicGasCost(i IntrinsicGasCosts, feeCurrency *Address) (uint64, bool) { if feeCurrency == nil { return 0, true diff --git a/contracts/fee_currencies.go b/contracts/fee_currencies.go index 66a04f589d..a92bd99d24 100644 --- a/contracts/fee_currencies.go +++ b/contracts/fee_currencies.go @@ -141,7 +141,7 @@ func GetRegisteredCurrencies(caller *abigen.FeeCurrencyDirectoryCaller) ([]commo return currencies, nil } -// GetExchangeRates returns the exchange rates for the provided gas currencies from CELO +// GetExchangeRates returns the exchange rates for the provided gas currencies func GetExchangeRates(caller bind.ContractCaller) (common.ExchangeRates, error) { exchangeRates := map[common.Address]*big.Rat{} directory, err := abigen.NewFeeCurrencyDirectoryCaller(addresses.FeeCurrencyDirectoryAddress, caller) @@ -150,7 +150,7 @@ func GetExchangeRates(caller bind.ContractCaller) (common.ExchangeRates, error) } currencies, err := GetRegisteredCurrencies(directory) if err != nil { - return map[common.Address]*big.Rat{}, err + return common.ExchangeRates{}, err } return getExchangeRatesForTokens(directory, currencies) } diff --git a/core/bench_test.go b/core/bench_test.go index 55a1a9104f..233287f7c8 100644 --- a/core/bench_test.go +++ b/core/bench_test.go @@ -83,8 +83,7 @@ func genValueTx(nbytes int) func(int, *BlockGen) { return func(i int, gen *BlockGen) { toaddr := common.Address{} data := make([]byte, nbytes) - var feeIntrinsic common.IntrinsicGasCosts - gas, _ := IntrinsicGas(data, nil, false, false, false, false, nil, feeIntrinsic) + gas, _ := IntrinsicGas(data, nil, false, false, false, false, nil, common.IntrinsicGasCosts{}) signer := gen.Signer() gasPrice := big.NewInt(0) if gen.header.BaseFee != nil { diff --git a/tests/transaction_test_util.go b/tests/transaction_test_util.go index 22c60f7c57..956be1fc25 100644 --- a/tests/transaction_test_util.go +++ b/tests/transaction_test_util.go @@ -55,8 +55,7 @@ func (tt *TransactionTest) Run(config *params.ChainConfig) error { return nil, nil, err } // Intrinsic gas - var feeIntrinsic common.IntrinsicGasCosts - requiredGas, err := core.IntrinsicGas(tx.Data(), tx.AccessList(), tx.To() == nil, isHomestead, isIstanbul, false, nil, feeIntrinsic) + requiredGas, err := core.IntrinsicGas(tx.Data(), tx.AccessList(), tx.To() == nil, isHomestead, isIstanbul, false, nil, common.IntrinsicGasCosts{}) if err != nil { return nil, nil, err } From d4176187c3e3a68f01ec912cafd76e103ee862de Mon Sep 17 00:00:00 2001 From: Maximilian Langenfeld <15726643+ezdac@users.noreply.github.com> Date: Mon, 5 Aug 2024 12:00:29 +0200 Subject: [PATCH 7/8] Add comment for nil fee-currency code path --- common/celo_types.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/common/celo_types.go b/common/celo_types.go index 44c59e857d..533c2416a5 100644 --- a/common/celo_types.go +++ b/common/celo_types.go @@ -21,12 +21,16 @@ func MaxAllowedIntrinsicGasCost(i IntrinsicGasCosts, feeCurrency *Address) (uint if !ok { return 0, false } - // allow the contract to overshoot 2 times the deducted intrinsic gas - // during execution + // Allow the contract to overshoot 2 times the deducted intrinsic gas + // during execution. + // If the feeCurrency is nil, then the max allowed intrinsic gas cost + // is 0 (i.e. not allowed) for a fee-currency specific EVM call within the STF. return intrinsicGas * 3, true } func CurrencyIntrinsicGasCost(i IntrinsicGasCosts, feeCurrency *Address) (uint64, bool) { + // the additional intrinsic gas cost for a non fee-currency + // transaction is 0 if feeCurrency == nil { return 0, true } From 111be39fd2c90e273e5ea4cd195d162a9780565c Mon Sep 17 00:00:00 2001 From: Maximilian Langenfeld <15726643+ezdac@users.noreply.github.com> Date: Mon, 5 Aug 2024 12:07:20 +0200 Subject: [PATCH 8/8] Add fee-currency-context to CeloBackend.NewEVM constructor --- contracts/celo_backend.go | 9 +++++++-- contracts/fee_currencies.go | 3 +-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/contracts/celo_backend.go b/contracts/celo_backend.go index d3133c783e..b6e8e1e008 100644 --- a/contracts/celo_backend.go +++ b/contracts/celo_backend.go @@ -57,14 +57,19 @@ func (b *CeloBackend) CallContract(ctx context.Context, call ethereum.CallMsg, b // Get a vm.EVM object of you can't use the abi wrapper via the ContractCaller interface // This is usually the case when executing functions that modify state. -func (b *CeloBackend) NewEVM() *vm.EVM { - blockCtx := vm.BlockContext{BlockNumber: new(big.Int), Time: 0, +func (b *CeloBackend) NewEVM(feeCurrencyContext *common.FeeCurrencyContext) *vm.EVM { + blockCtx := vm.BlockContext{ + BlockNumber: new(big.Int), + Time: 0, Transfer: func(state vm.StateDB, from common.Address, to common.Address, value *uint256.Int) { if value.Cmp(common.U2560) != 0 { panic("Non-zero transfers not implemented, yet.") } }, } + if feeCurrencyContext != nil { + blockCtx.FeeCurrencyContext = *feeCurrencyContext + } txCtx := vm.TxContext{} vmConfig := vm.Config{} return vm.NewEVM(blockCtx, txCtx, b.State, b.ChainConfig, vmConfig) diff --git a/contracts/fee_currencies.go b/contracts/fee_currencies.go index a92bd99d24..88e9771ae7 100644 --- a/contracts/fee_currencies.go +++ b/contracts/fee_currencies.go @@ -33,8 +33,7 @@ func TryDebitFees(tx *types.Transaction, from common.Address, backend *CeloBacke amount.Mul(amount, tx.GasFeeCap()) snapshot := backend.State.Snapshot() - evm := backend.NewEVM() - evm.Context.FeeCurrencyContext = feeContext + evm := backend.NewEVM(&feeContext) _, err := DebitFees(evm, tx.FeeCurrency(), from, amount) backend.State.RevertToSnapshot(snapshot) return err