add configurable historical balance health check for finalized block …#352
add configurable historical balance health check for finalized block …#352Krish-vemula wants to merge 4 commits intodevelopfrom
Conversation
📊 API Diff Results
|
product-security-plaid-production
bot
requested review from
DylanTinianov,
amit-momin and
justinkaseman
| return tc.NodeFinalizedBlockPollInterval | ||
| } | ||
|
|
||
| func (tc TestNodePoolConfig) HistoricalBalanceCheckEnabled() bool { |
There was a problem hiding this comment.
This flag should be part of the multinode config. As multinode will perform the polling
pkg/client/rpc_client.go
Outdated
| return version, nil | ||
| } | ||
|
|
||
| func (r *RPCClient) checkHistoricalStateAtFinalized(ctx context.Context) error { |
There was a problem hiding this comment.
This should be public method, so that we can poll it from multinode
…cept probeAddress parameter, fall back to EVM config if empty.
pkg/client/helpers_test.go
Outdated
| return tc.ExternalRequestMaxResponseSizeVal | ||
| } | ||
|
|
||
| func (tc TestNodePoolConfig) FinalizedStateCheckEnabled() bool { |
There was a problem hiding this comment.
Why do we need two feature flags?
pkg/client/rpc_client.go
Outdated
| // This is used to detect non-archive nodes that cannot serve state queries for older blocks. | ||
| // If probeAddress is provided, it uses that address; otherwise falls back to the configured HistoricalBalanceCheckAddress. | ||
| // Returns nil immediately if historical balance check is not enabled and no probeAddress is provided. | ||
| func (r *RPCClient) CheckFinalizedStateAvailability(ctx context.Context, probeAddress string) error { |
There was a problem hiding this comment.
IMO, there is no need to allow overriding the address. It adds complexity but we do not know if it'll be used.
| blockNumber = big.NewInt(finalizedHeight) | ||
| } | ||
| _, err := r.BalanceAt(ctx, addr, blockNumber) | ||
| if err != nil { |
There was a problem hiding this comment.
We need to check the error against the regex here and return a dedicated error defined in the framework repo.
This way, we abstract away the EVM-specific way of checking the availability of finalized states.
pkg/config/chain_scoped_node_pool.go
Outdated
| } | ||
|
|
||
| func (n *NodePoolConfig) HistoricalBalanceCheckEnabled() bool { | ||
| if n.C.HistoricalBalanceCheckEnabled == nil { |
There was a problem hiding this comment.
Configs are built in a way that the value should never be nil. So here we should skip checks to keep everything consistent.
Also, you need to add new config fields to setFrom method.
It is required to handle overrides properly.
By overrides I mean the ability to define a common value in the fallback file and override it in a chain-specific TOML file or during runtime via a custom TOML file.
pkg/client/helpers_test.go
Outdated
| FinalizedStateCheckEnabledVal bool | ||
| FinalizedStateCheckAddressVal string | ||
| FinalizedStateCheckFailureThresholdVal uint32 | ||
| FinalizedStateUnavailableRegexVal string |
There was a problem hiding this comment.
This regex should be defined as part of ClientErrors
3 participants
Summary
Adds an optional health check that verifies RPC nodes can access historical blockchain state at finalized blocks.
Changes
New Configuration Options ([EVM.NodePool]):
eth_getBalanceprobeImplementation:
PollInterval), if enabled, executeseth_getBalanceat the finalized blockFinalityTagEnabled = true: queries using the"finalized"block tagFinalityTagEnabled = false: queries atlatest - FinalityDepthPollFailureThreshold- after enough consecutive failures, the node is marked unreachable and traffic fails over to healthy nodesConfiguration Example