-
Notifications
You must be signed in to change notification settings - Fork 13
Refactor & harden the nonce-aware tx mempool (stacked on #971) #974
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
de58a9e
9cce00c
24a6b3f
b535dc1
54fd7d3
9c6f6c5
4019174
328ccf4
d1ab394
d1db5d8
8a9053c
c705722
ad71012
7303ab9
b04c8ed
82306fe
8cbdfce
46cf137
ec791ad
f001141
8b3b742
fb426a5
968afb1
ac59fc4
7123a4e
59383db
17417b0
32a1f15
ab7fa95
294c631
60b0bb6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,8 @@ | ||
| package requester | ||
|
|
||
| import ( | ||
| "sync" | ||
|
|
||
| gethCommon "github.com/ethereum/go-ethereum/common" | ||
| "github.com/onflow/flow-go/fvm/evm" | ||
| "github.com/onflow/flow-go/fvm/evm/offchain/query" | ||
|
|
@@ -10,25 +12,49 @@ import ( | |
| "github.com/onflow/flow-evm-gateway/storage/pebble" | ||
| ) | ||
|
|
||
| // NonceProvider returns the current nonce of the given EOA address. | ||
| // The transaction mempool uses it to determine the expected next nonce. | ||
| // NonceView reads EOA nonces at a single, fixed EVM state (one built block | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These are changes to incorporate @m-Peter comment in the earlier PR about reusing the block view to find the last nonce for an EOA. Caching the view for a given block height ensures we are not performing the expensive operation of reading the full block view each time when figuring out the latest on-chain nonce for an EOA. |
||
| // view). The mempool reads many EOAs' nonces from one view per flush tick | ||
| // rather than rebuilding the (expensive) view per address. It is an interface | ||
| // so tests can fake it without constructing a real query.View. | ||
| type NonceView interface { | ||
| // GetNonce returns the EOA's account nonce (the next nonce to use) at this | ||
| // view's state. Named GetNonce — not GetNextNonce — because this interface is | ||
| // satisfied directly by flow-go's query.View, whose method is GetNonce. | ||
| GetNonce(address gethCommon.Address) (uint64, error) | ||
| } | ||
|
|
||
| // NonceProvider returns the next nonce of the given EOA address. The transaction | ||
| // mempool uses it to determine the expected next nonce. | ||
| type NonceProvider interface { | ||
| // GetNonce returns the current nonce of the given EOA address. | ||
| // GetNextNonce returns the account nonce of the given EOA — its transaction | ||
| // count, i.e. the next nonce the EOA should use (matches eth_getTransactionCount). | ||
| // | ||
| // A non-nil error represents an EXCEPTION, not an expected condition: | ||
| // the underlying read is a local state-index lookup that should not | ||
| // fail under normal operation. Callers must therefore treat an error | ||
| // as a hard failure (reject the transaction / abort the operation) | ||
| // rather than a routine, recoverable condition to swallow. | ||
| GetNonce(address gethCommon.Address) (uint64, error) | ||
| GetNextNonce(address gethCommon.Address) (uint64, error) | ||
|
|
||
| // GetBlockView returns a NonceView over the latest indexed EVM state. A | ||
| // non-nil error is an EXCEPTION, same contract as GetNextNonce. | ||
| GetBlockView() (NonceView, error) | ||
| } | ||
|
|
||
| // LocalNonceProvider reads the EOA nonce from the latest height of the | ||
| // local state index. | ||
| // local state index. It caches the built block view and reuses it while the | ||
| // indexed height is unchanged (see GetBlockView). | ||
| type LocalNonceProvider struct { | ||
| chainID flowGo.ChainID | ||
| registerStore *pebble.RegisterStorage | ||
| blocks storage.BlockIndexer | ||
|
|
||
| // mu guards the cached view below. The cached view is shared across reads; | ||
| // callers that read it concurrently must serialize (the mempool does, via | ||
| // its queueMux). | ||
| mu sync.Mutex | ||
| cachedView NonceView | ||
| cachedHeight uint64 | ||
| } | ||
|
|
||
| var _ NonceProvider = &LocalNonceProvider{} | ||
|
|
@@ -45,10 +71,23 @@ func NewLocalNonceProvider( | |
| } | ||
| } | ||
|
|
||
| func (p *LocalNonceProvider) GetNonce(address gethCommon.Address) (uint64, error) { | ||
| // GetBlockView returns a NonceView over the latest indexed EVM height. The view | ||
| // is cached and reused while the indexed height is unchanged, so a burst of | ||
| // reads within one block — many Add calls, or a collectDueBatches pass — builds | ||
| // the (expensive) view only once. It is rebuilt when a new block is indexed. | ||
| // Reuse is safe because an EOA's on-chain nonce cannot change without a new | ||
| // block being indexed. | ||
| func (p *LocalNonceProvider) GetBlockView() (NonceView, error) { | ||
| height, err := p.blocks.LatestEVMHeight() | ||
| if err != nil { | ||
| return 0, err | ||
| return nil, err | ||
| } | ||
|
|
||
| p.mu.Lock() | ||
| defer p.mu.Unlock() | ||
|
|
||
| if p.cachedView != nil && p.cachedHeight == height { | ||
| return p.cachedView, nil | ||
| } | ||
|
|
||
| viewProvider := query.NewViewProvider( | ||
|
|
@@ -60,6 +99,18 @@ func (p *LocalNonceProvider) GetNonce(address gethCommon.Address) (uint64, error | |
| ) | ||
|
|
||
| view, err := viewProvider.GetBlockView(height) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| p.cachedView = view | ||
| p.cachedHeight = height | ||
|
|
||
| return view, nil | ||
| } | ||
|
|
||
| func (p *LocalNonceProvider) GetNextNonce(address gethCommon.Address) (uint64, error) { | ||
| view, err := p.GetBlockView() | ||
| if err != nil { | ||
| return 0, err | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this change is to address @zhangchiqing suggestion that we should reject nonce that are ridiculously higher than the current nonce. e.g. 1M nonce in the future.