diff --git a/eth/api_admin.go b/eth/api_admin.go index 4a3ccb84e8..468f22b7df 100644 --- a/eth/api_admin.go +++ b/eth/api_admin.go @@ -24,6 +24,7 @@ import ( "os" "strings" + "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/core" "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/rlp" @@ -141,3 +142,35 @@ func (api *AdminAPI) ImportChain(file string) (bool, error) { } return true, nil } + +// GetBlocklistFeeCurrencies returns the fee currencies in the blocklist mapped +// to their expiry Unix timestamp. Note that the parameter 'includeDisabled' +// determines if the map should contain fee currencies for which blocking has +// been manually disabled. +func (api *AdminAPI) GetBlocklistFeeCurrencies(includeDisabled bool) map[common.Address]uint64 { + return api.eth.Miner().BlocklistFeeCurrencies(includeDisabled) +} + +// DisableBlocklistFeeCurrencies disables blocking on the given currencies, for currencies that +// are already disabled this is a no-op. +func (api *AdminAPI) DisableBlocklistFeeCurrencies(currencies []common.Address) { + api.eth.Miner().DisableBlocklistFeeCurrencies(currencies) +} + +// EnableBlocklistFeeCurrencies enables blocking on the given currencies, for currencies that +// are already enabled this is a no-op. +func (api *AdminAPI) EnableBlocklistFeeCurrencies(currencies []common.Address) { + api.eth.Miner().EnableBlocklistFeeCurrencies(currencies) +} + +// GetDisabledBlocklistFeeCurrencies returns the currencies for which blocking is currently +// manually disabled. +func (api *AdminAPI) GetDisabledBlocklistFeeCurrencies() []common.Address { + return api.eth.Miner().DisabledBlocklistFeeCurrencies() +} + +// UnblockFeeCurrency removes a fee currency from the fee currency blocklist, +// if the currency was present true is returned. +func (api *AdminAPI) UnblockFeeCurrency(address common.Address) bool { + return api.eth.Miner().UnblockFeeCurrency(address) +} diff --git a/miner/currency_blocklist.go b/miner/currency_blocklist.go index 4081a2eb81..60d5b46aa9 100644 --- a/miner/currency_blocklist.go +++ b/miner/currency_blocklist.go @@ -18,24 +18,91 @@ type AddressBlocklist struct { // will get evicted when evict() is called headerEvictionTimeoutSeconds uint64 oldestHeader *types.Header + + // disabledCurrencies is the set of currencies for which the blocklist + // functionality has been manually disabled. + disabledCurrencies map[common.Address]struct{} } func NewAddressBlocklist() *AddressBlocklist { - return &AddressBlocklist{ + bl := &AddressBlocklist{ mux: &sync.RWMutex{}, currencies: map[common.Address]*types.Header{}, headerEvictionTimeoutSeconds: EvictionTimeoutSeconds, oldestHeader: nil, + disabledCurrencies: make(map[common.Address]struct{}), + } + return bl +} + +// Blocklist returns the current blocklist, a set of fee currency +// addresses mapped to their expiry Unix timestamp. Note that the parameter +// 'includeDisabled' determines if the map should contain fee currencies for +// which blocking has been manually disabled. +func (b *AddressBlocklist) Blocklist(includeDisabled bool) map[common.Address]uint64 { + b.mux.RLock() + defer b.mux.RUnlock() + result := make(map[common.Address]uint64, len(b.currencies)) + for currency, addedHeader := range b.currencies { + if _, disabled := b.disabledCurrencies[currency]; disabled && !includeDisabled { + continue + } + result[currency] = addedHeader.Time + b.headerEvictionTimeoutSeconds } + return result +} + +// DisableBlocking disables blocking on the given currencies, for currencies that +// are already disabled this is a no-op. +func (b *AddressBlocklist) DisableBlocking(currencies []common.Address) { + b.mux.Lock() + defer b.mux.Unlock() + for _, currency := range currencies { + b.disabledCurrencies[currency] = struct{}{} + } +} + +// EnableBlocking enables blocking on the given currencies, for currencies that +// are already enabled this is a no-op. +func (b *AddressBlocklist) EnableBlocking(currencies []common.Address) { + b.mux.Lock() + defer b.mux.Unlock() + for _, currency := range currencies { + delete(b.disabledCurrencies, currency) + } +} + +// DisabledCurrencies returns the currencies for which blocking is currently +// manually disabled. +func (b *AddressBlocklist) DisabledCurrencies() []common.Address { + b.mux.RLock() + defer b.mux.RUnlock() + result := make([]common.Address, 0, len(b.disabledCurrencies)) + for currency := range b.disabledCurrencies { + result = append(result, currency) + } + return result +} + +// BlockingEnabled returns whether blocking is enabled for the given currency. +func (b *AddressBlocklist) BlockingEnabled(currency common.Address) bool { + b.mux.RLock() + defer b.mux.RUnlock() + _, disabled := b.disabledCurrencies[currency] + return !disabled } +// FilterAllowlist returns allowlist with any blocked and not disabled +// currencies removed. It accepts the latest header so that it may provide a +// view of the blocklist consistent with that header. func (b *AddressBlocklist) FilterAllowlist(allowlist common.AddressSet, latest *types.Header) common.AddressSet { b.mux.RLock() defer b.mux.RUnlock() filtered := common.AddressSet{} for a := range allowlist { - if !b.isBlocked(a, latest) { + _, disabled := b.disabledCurrencies[a] + if !b.isBlocked(a, latest) || disabled { filtered[a] = struct{}{} } } @@ -64,14 +131,16 @@ func (b *AddressBlocklist) Remove(currency common.Address) bool { return ok } -func (b *AddressBlocklist) Add(currency common.Address, head types.Header) { +func (b *AddressBlocklist) Add(currency common.Address, head types.Header) bool { b.mux.Lock() defer b.mux.Unlock() + _, existed := b.currencies[currency] if b.oldestHeader == nil || b.oldestHeader.Time > head.Time { b.oldestHeader = &head } b.currencies[currency] = &head + return !existed } func (b *AddressBlocklist) Evict(latest *types.Header) []common.Address { diff --git a/miner/currency_blocklist_test.go b/miner/currency_blocklist_test.go index a26d7392ae..b070945af5 100644 --- a/miner/currency_blocklist_test.go +++ b/miner/currency_blocklist_test.go @@ -70,6 +70,11 @@ func TestBlocklistAddAfterEviction(t *testing.T) { func TestBlocklistRemove(t *testing.T) { bl := NewAddressBlocklist() + + // Check that removing a fee currency from an empty blocklist doesn't panic. + bl.Remove(feeCurrency1) + + // Check that removal of existing fee currency works. bl.Add(feeCurrency1, header) bl.Add(feeCurrency2, header) bl.Remove(feeCurrency1) @@ -91,3 +96,64 @@ func TestBlocklistAddAfterRemove(t *testing.T) { assert.True(t, bl.IsBlocked(feeCurrency2, HeaderAfter(*header2, int64(EvictionTimeoutSeconds)-1))) assert.False(t, bl.IsBlocked(feeCurrency2, HeaderAfter(*header2, int64(EvictionTimeoutSeconds)+1))) } + +func TestDisableEnableBlocking(t *testing.T) { + bl := NewAddressBlocklist() + bl.Add(feeCurrency1, header) + bl.Add(feeCurrency2, header) + + assert.True(t, bl.BlockingEnabled(feeCurrency1)) + assert.True(t, bl.BlockingEnabled(feeCurrency2)) + + allowlist := common.NewAddressSet(feeCurrency1, feeCurrency2) + + assert.Equal(t, common.NewAddressSet(), bl.FilterAllowlist(allowlist, &header)) + + bl.DisableBlocking([]common.Address{feeCurrency1}) + assert.False(t, bl.BlockingEnabled(feeCurrency1)) + assert.Equal(t, common.NewAddressSet(feeCurrency1), bl.FilterAllowlist(allowlist, &header)) + + bl.DisableBlocking([]common.Address{feeCurrency2}) + assert.False(t, bl.BlockingEnabled(feeCurrency2)) + assert.Equal(t, allowlist, bl.FilterAllowlist(allowlist, &header)) + + bl.EnableBlocking([]common.Address{feeCurrency2, feeCurrency1}) + assert.True(t, bl.BlockingEnabled(feeCurrency2)) + assert.True(t, bl.BlockingEnabled(feeCurrency1)) + assert.Equal(t, common.NewAddressSet(), bl.FilterAllowlist(allowlist, &header)) +} + +func TestBlocklistRetrieval(t *testing.T) { + bl := NewAddressBlocklist() + bl.Add(feeCurrency1, header) + bl.Add(feeCurrency2, header) + + expiryTime := header.Time + EvictionTimeoutSeconds + + allCurrencies := map[common.Address]uint64{ + feeCurrency1: expiryTime, + feeCurrency2: expiryTime, + } + assert.Equal(t, allCurrencies, bl.Blocklist(true)) + assert.Equal(t, allCurrencies, bl.Blocklist(false)) + + bl.DisableBlocking([]common.Address{feeCurrency1}) + assert.Equal(t, allCurrencies, bl.Blocklist(true)) + oneCurrency := map[common.Address]uint64{ + feeCurrency2: expiryTime, + } + assert.Equal(t, oneCurrency, bl.Blocklist(false)) +} + +func TestDisabledCurrenciesRetrieval(t *testing.T) { + bl := NewAddressBlocklist() + disabledCurrencies := []common.Address{feeCurrency1, feeCurrency2} + + assert.Empty(t, bl.DisabledCurrencies()) + + bl.DisableBlocking(disabledCurrencies) + assert.ElementsMatch(t, disabledCurrencies, bl.DisabledCurrencies()) + + bl.EnableBlocking([]common.Address{feeCurrency1}) + assert.ElementsMatch(t, []common.Address{feeCurrency2}, bl.DisabledCurrencies()) +} diff --git a/miner/miner.go b/miner/miner.go index 080000a530..cc04a7de07 100644 --- a/miner/miner.go +++ b/miner/miner.go @@ -231,6 +231,42 @@ func (miner *Miner) getPending() *newPayloadResult { return ret } +// BlocklistFeeCurrencies returns the fee currencies in the blocklist mapped to +// their expiry Unix timestamp. Note that the parameter 'includeDisabled' +// determines if the map should contain fee currencies for which blocking has +// been manually disabled. +func (miner *Miner) BlocklistFeeCurrencies(includeDisabled bool) map[common.Address]uint64 { + return miner.feeCurrencyBlocklist.Blocklist(includeDisabled) +} + +// DisableBlocklistFeeCurrencies disables blocking on the given currencies, for currencies that +// are already disabled this is a no-op. +func (miner *Miner) DisableBlocklistFeeCurrencies(currencies []common.Address) { + miner.feeCurrencyBlocklist.DisableBlocking(currencies) +} + +// DisabledBlocklistFeeCurrencies returns the currencies for which blocking is currently +// manually disabled. +func (miner *Miner) DisabledBlocklistFeeCurrencies() []common.Address { + return miner.feeCurrencyBlocklist.DisabledCurrencies() +} + +// EnableBlocklistFeeCurrencies enables blocking on the given currencies, for currencies that +// are already enabled this is a no-op. +func (miner *Miner) EnableBlocklistFeeCurrencies(currencies []common.Address) { + miner.feeCurrencyBlocklist.EnableBlocking(currencies) +} + +// UnblockFeeCurrency removes a fee currency from the fee currency blocklist, +// if the currency was present true is returned. +func (miner *Miner) UnblockFeeCurrency(address common.Address) bool { + removed := miner.feeCurrencyBlocklist.Remove(address) + if removed { + feeCurrenciesInBlocklistCounter.Dec(1) + } + return removed +} + func (miner *Miner) Close() { miner.lifeCtxCancel() } diff --git a/miner/worker.go b/miner/worker.go index 2515c99c1a..da607529f2 100644 --- a/miner/worker.go +++ b/miner/worker.go @@ -61,6 +61,8 @@ var ( txConditionalMinedTimer = metrics.NewRegisteredTimer("miner/transactionConditional/elapsedtime", nil) txInteropRejectedCounter = metrics.NewRegisteredCounter("miner/transactionInterop/rejected", nil) + + feeCurrenciesInBlocklistCounter = metrics.NewRegisteredCounter("miner/blocklist/feeCurrency/blocked", nil) ) // environment is the worker's current environment and holds all @@ -340,6 +342,7 @@ func (miner *Miner) prepareWork(genParams *generateParams, witness bool) (*envir "evicted-fee-currencies", evicted, "eviction-timeout-seconds", EvictionTimeoutSeconds, ) + feeCurrenciesInBlocklistCounter.Dec(int64(len(evicted))) } env.feeCurrencyAllowlist = miner.feeCurrencyBlocklist.FilterAllowlist( common.CurrencyAllowlist(env.feeCurrencyContext.ExchangeRates), @@ -424,7 +427,7 @@ func (miner *Miner) commitTransaction(env *environment, tx *types.Transaction) e "fee-currency", tx.FeeCurrency(), "error", err.Error(), ) - miner.blockFeeCurrency(env, *tx.FeeCurrency(), err) + miner.registerFeeCurrencyTxFailure(env, tx, err) } return err } @@ -834,14 +837,18 @@ func (miner *Miner) validateParams(genParams *generateParams) (time.Duration, er return time.Duration(blockTime) * time.Second, nil } -func (miner *Miner) blockFeeCurrency(env *environment, feeCurrency common.Address, err error) { +func (miner *Miner) registerFeeCurrencyTxFailure(env *environment, tx *types.Transaction, err error) { // the fee-currency is still in the allowlist of this environment, // so set the fee-currency block gas limit to 0 to prevent other // transactions. - pool := env.multiGasPool.PoolFor(&feeCurrency) - pool.SetGas(0) + if miner.feeCurrencyBlocklist.BlockingEnabled(*tx.FeeCurrency()) { + pool := env.multiGasPool.PoolFor(tx.FeeCurrency()) + pool.SetGas(0) + } // also add the fee-currency to a worker-wide blocklist, // so that they are not allowlisted in the following blocks // (only locally in the txpool, not consensus-critical) - miner.feeCurrencyBlocklist.Add(feeCurrency, *env.header) + if miner.feeCurrencyBlocklist.Add(*tx.FeeCurrency(), *env.header) { + feeCurrenciesInBlocklistCounter.Inc(1) + } }