mirror of
https://github.com/ethereum/go-ethereum.git
synced 2026-02-26 15:47:21 +00:00
This PR proposes a change to the authorizations' validation introduced
in commit cdb66c8. These changes make the expected behavior independent
of the order of admission of authorizations, improving the
predictability of the resulting state and the usability of the system
with it.
The current implementation behavior is dependent on the transaction
submission order: This issue is related to authorities and the sender of
a transaction, and can be reproduced respecting the normal nonce rules.
The issue can be reproduced by the two following cases:
**First case**
- Given an empty pool.
- Submit transaction `{ from: B, auths [ A ] }`: is accepted.
- Submit transaction `{ from: A }`: Is accepted: it becomes the one
in-flight transaction allowed.
**Second case**
- Given an empty pool.
- Submit transaction `{ from: A }`: is accepted
- Submit transaction `{ from: B, auths [ A ] }`: is rejected since there
is already a queued/pending transaction from A.
The expected behavior is that both sequences of events would lead to the
same sets of accepted and rejected transactions.
**Proposed changes**
The queued/pending transactions issued from any authority of the
transaction being validated have to be counted, allowing one transaction
from accounts submitting an authorization.
- Notice that the expected behavior was explicitly forbidden in the case
"reject-delegation-from-pending-account", I believe that this behavior
conflicts to the definition of the limitation, and it is removed in this
PR. The expected behavior is tested in
"accept-authorization-from-sender-of-one-inflight-tx".
- Replacement tests have been separated to improve readability of the
acceptance test.
- The test "allow-more-than-one-tx-from-replaced-authority" has been
extended with one extra transaction, since the system would always have
accepted one transaction (but not two).
- The test "accept-one-inflight-tx-of-delegated-account" is extended to
clean-up state, avoiding leaking the delegation used into the other
tests. Additionally, replacement check is removed to be tested in its
own test case.
**Expected behavior**
The expected behavior of the authorizations' validation shall be as
follows:

Notice that replacement shall be allowed, and behavior shall remain
coherent with the table, according to the replaced transaction.
---------
Co-authored-by: lightclient <lightclient@protonmail.com>
138 lines
4.7 KiB
Go
138 lines
4.7 KiB
Go
// Copyright 2025 The go-ethereum Authors
|
|
// This file is part of the go-ethereum library.
|
|
//
|
|
// The go-ethereum library is free software: you can redistribute it and/or modify
|
|
// it under the terms of the GNU Lesser General Public License as published by
|
|
// the Free Software Foundation, either version 3 of the License, or
|
|
// (at your option) any later version.
|
|
//
|
|
// The go-ethereum library is distributed in the hope that it will be useful,
|
|
// but WITHOUT ANY WARRANTY; without even the implied warranty of
|
|
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
|
|
// GNU Lesser General Public License for more details.
|
|
//
|
|
// You should have received a copy of the GNU Lesser General Public License
|
|
// along with the go-ethereum library. If not, see <http://www.gnu.org/licenses/>.
|
|
|
|
package txpool
|
|
|
|
import (
|
|
"errors"
|
|
"fmt"
|
|
"sync"
|
|
|
|
"github.com/ethereum/go-ethereum/common"
|
|
"github.com/ethereum/go-ethereum/log"
|
|
"github.com/ethereum/go-ethereum/metrics"
|
|
)
|
|
|
|
var (
|
|
// reservationsGaugeName is the prefix of a per-subpool address reservation
|
|
// metric.
|
|
//
|
|
// This is mostly a sanity metric to ensure there's no bug that would make
|
|
// some subpool hog all the reservations due to mis-accounting.
|
|
reservationsGaugeName = "txpool/reservations"
|
|
)
|
|
|
|
// ReservationTracker is a struct shared between different subpools. It is used to reserve
|
|
// the account and ensure that one address cannot initiate transactions, authorizations,
|
|
// and other state-changing behaviors in different pools at the same time.
|
|
type ReservationTracker struct {
|
|
accounts map[common.Address]int
|
|
lock sync.RWMutex
|
|
}
|
|
|
|
// NewReservationTracker initializes the account reservation tracker.
|
|
func NewReservationTracker() *ReservationTracker {
|
|
return &ReservationTracker{
|
|
accounts: make(map[common.Address]int),
|
|
}
|
|
}
|
|
|
|
// NewHandle creates a named handle on the ReservationTracker. The handle
|
|
// identifies the subpool so ownership of reservations can be determined.
|
|
func (r *ReservationTracker) NewHandle(id int) *ReservationHandle {
|
|
return &ReservationHandle{r, id}
|
|
}
|
|
|
|
// Reserver is an interface for creating and releasing owned reservations in the
|
|
// ReservationTracker struct, which is shared between subpools.
|
|
type Reserver interface {
|
|
// Hold attempts to reserve the specified account address for the given pool.
|
|
// Returns an error if the account is already reserved.
|
|
Hold(addr common.Address) error
|
|
|
|
// Release attempts to release the reservation for the specified account.
|
|
// Returns an error if the address is not reserved or is reserved by another pool.
|
|
Release(addr common.Address) error
|
|
|
|
// Has returns a flag indicating if the address has been reserved by a pool
|
|
// other than one with the current Reserver handle.
|
|
Has(address common.Address) bool
|
|
}
|
|
|
|
// ReservationHandle is a named handle on ReservationTracker. It is held by subpools to
|
|
// make reservations for accounts it is tracking. The id is used to determine
|
|
// which pool owns an address and disallows non-owners to hold or release
|
|
// addresses it doesn't own.
|
|
type ReservationHandle struct {
|
|
tracker *ReservationTracker
|
|
id int
|
|
}
|
|
|
|
// Hold implements the Reserver interface.
|
|
func (h *ReservationHandle) Hold(addr common.Address) error {
|
|
h.tracker.lock.Lock()
|
|
defer h.tracker.lock.Unlock()
|
|
|
|
// Double reservations are forbidden even from the same pool to
|
|
// avoid subtle bugs in the long term.
|
|
owner, exists := h.tracker.accounts[addr]
|
|
if exists {
|
|
if owner == h.id {
|
|
log.Error("pool attempted to reserve already-owned address", "address", addr)
|
|
return nil // Ignore fault to give the pool a chance to recover while the bug gets fixed
|
|
}
|
|
return ErrAlreadyReserved
|
|
}
|
|
h.tracker.accounts[addr] = h.id
|
|
if metrics.Enabled() {
|
|
m := fmt.Sprintf("%s/%d", reservationsGaugeName, h.id)
|
|
metrics.GetOrRegisterGauge(m, nil).Inc(1)
|
|
}
|
|
return nil
|
|
}
|
|
|
|
// Release implements the Reserver interface.
|
|
func (h *ReservationHandle) Release(addr common.Address) error {
|
|
h.tracker.lock.Lock()
|
|
defer h.tracker.lock.Unlock()
|
|
|
|
// Ensure subpools only attempt to unreserve their own owned addresses,
|
|
// otherwise flag as a programming error.
|
|
owner, exists := h.tracker.accounts[addr]
|
|
if !exists {
|
|
log.Error("pool attempted to unreserve non-reserved address", "address", addr)
|
|
return errors.New("address not reserved")
|
|
}
|
|
if owner != h.id {
|
|
log.Error("pool attempted to unreserve non-owned address", "address", addr)
|
|
return errors.New("address not owned")
|
|
}
|
|
delete(h.tracker.accounts, addr)
|
|
if metrics.Enabled() {
|
|
m := fmt.Sprintf("%s/%d", reservationsGaugeName, h.id)
|
|
metrics.GetOrRegisterGauge(m, nil).Dec(1)
|
|
}
|
|
return nil
|
|
}
|
|
|
|
// Has implements the Reserver interface.
|
|
func (h *ReservationHandle) Has(address common.Address) bool {
|
|
h.tracker.lock.RLock()
|
|
defer h.tracker.lock.RUnlock()
|
|
|
|
id, exists := h.tracker.accounts[address]
|
|
return exists && id != h.id
|
|
}
|