Skip to content

Commit ec962be

Browse files
Krish-vemulaKrish-vemula
committed
multinode: cache finalized-state checker, tighten mocks/logs, drop base no-op, gate threshold on poll interval
1 parent de28b86 commit ec962be

5 files changed

Lines changed: 55 additions & 39 deletions

File tree

‎multinode/mock_rpc_client_ext_test.go‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,28 @@ package multinode
22

33
import (
44
"context"
5+
6+
mock "github.com/stretchr/testify/mock"
57
)
68

9+
// mockRPCClient_CheckFinalizedStateAvailability_Call mirrors mockery-generated *Call types for
10+
// CheckFinalizedStateAvailability (added manually on mockRPCClient, not on RPCClient interface).
11+
type mockRPCClient_CheckFinalizedStateAvailability_Call[CHAIN_ID ID, HEAD Head] struct {
12+
*mock.Call
13+
}
14+
15+
// CheckFinalizedStateAvailability is a helper to define EXPECT().CheckFinalizedStateAvailability(...).
16+
func (_e *mockRPCClient_Expecter[CHAIN_ID, HEAD]) CheckFinalizedStateAvailability(ctx interface{}) *mockRPCClient_CheckFinalizedStateAvailability_Call[CHAIN_ID, HEAD] {
17+
return &mockRPCClient_CheckFinalizedStateAvailability_Call[CHAIN_ID, HEAD]{Call: _e.mock.On("CheckFinalizedStateAvailability", ctx)}
18+
}
19+
20+
func (_c *mockRPCClient_CheckFinalizedStateAvailability_Call[CHAIN_ID, HEAD]) Return(err error) *mockRPCClient_CheckFinalizedStateAvailability_Call[CHAIN_ID, HEAD] {
21+
_c.Call.Return(err)
22+
return _c
23+
}
24+
725
// CheckFinalizedStateAvailability extends mockRPCClient to also satisfy FinalizedStateChecker,
8-
// allowing the type assertion any(n.rpc).(FinalizedStateChecker) to succeed in tests.
26+
// so NewNode populates finalizedStateChecker for tests that exercise finalized-state checks.
927
func (_m *mockRPCClient[CHAIN_ID, HEAD]) CheckFinalizedStateAvailability(ctx context.Context) error {
1028
ret := _m.Called(ctx)
1129

‎multinode/node.go‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,11 +39,13 @@ type ChainConfig interface {
3939
}
4040

4141
// FinalizedStateCheckConfig is an optional interface for enabling finalized state availability checking.
42+
// It is optional (not part of NodeConfig) so non-EVM multinode consumers are not forced to extend their config types; see aliveLoop in node_lifecycle.go.
4243
type FinalizedStateCheckConfig interface {
4344
FinalizedStateCheckFailureThreshold() uint32
4445
}
4546

4647
// FinalizedStateChecker is an optional interface for RPCClients that support finalized state checks.
48+
// It is optional (not part of RPCClient) so non-EVM multinode consumers avoid boilerplate and review churn; see aliveLoop in node_lifecycle.go.
4749
type FinalizedStateChecker interface {
4850
CheckFinalizedStateAvailability(ctx context.Context) error
4951
}
@@ -118,8 +120,9 @@ type node[
118120
ws *url.URL
119121
http *url.URL
120122

121-
rpc RPC
122-
isLoadBalancedRPC bool
123+
rpc RPC
124+
finalizedStateChecker FinalizedStateChecker // set in NewNode when rpc implements FinalizedStateChecker
125+
isLoadBalancedRPC bool
123126

124127
stateMu sync.RWMutex // protects state* fields
125128
state nodeState
@@ -177,6 +180,9 @@ func NewNode[
177180
)
178181
n.lfcLog = logger.Named(lggr, "Lifecycle")
179182
n.rpc = rpc
183+
if fs, ok := any(rpc).(FinalizedStateChecker); ok {
184+
n.finalizedStateChecker = fs
185+
}
180186
n.isLoadBalancedRPC = isLoadBalancedRPC
181187
n.chainFamily = chainFamily
182188
return n

‎multinode/node_lifecycle.go‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -103,11 +103,11 @@ func (n *node[CHAIN_ID, HEAD, RPC]) aliveLoop() {
103103
localHighestChainInfo, _ := n.rpc.GetInterceptedChainInfo()
104104
var pollFailures uint32
105105

106-
// Finalized state availability check via optional interfaces
106+
// Finalized-state availability is for EVM-style stacks, but multinode is a shared framework package: mandatory RPCClient/NodeConfig APIs would make every consumer (e.g. Solana)
107+
// inherit stubs or real implementations trigger unrelated security review. Optional interfaces keep the feature opt-in—only callers that need it implement FinalizedStateCheckConfig and FinalizedStateChecker; everyone else stays untouched.
107108
var finalizedStateCheckFailureThreshold uint32
108109
finalizedStateCfg, hasFinalizedStateCfg := n.nodePoolCfg.(FinalizedStateCheckConfig)
109-
_, hasFinalizedStateChecker := any(n.rpc).(FinalizedStateChecker)
110-
if hasFinalizedStateCfg && hasFinalizedStateChecker {
110+
if hasFinalizedStateCfg && n.finalizedStateChecker != nil && pollInterval > 0 {
111111
finalizedStateCheckFailureThreshold = finalizedStateCfg.FinalizedStateCheckFailureThreshold()
112112
}
113113
var finalizedStateFailures uint32
@@ -165,7 +165,7 @@ func (n *node[CHAIN_ID, HEAD, RPC]) aliveLoop() {
165165
}
166166
if finalizedStateCheckFailureThreshold > 0 {
167167
stateCheckCtx, stateCheckCancel := context.WithTimeout(ctx, pollInterval)
168-
stateErr := any(n.rpc).(FinalizedStateChecker).CheckFinalizedStateAvailability(stateCheckCtx)
168+
stateErr := n.finalizedStateChecker.CheckFinalizedStateAvailability(stateCheckCtx)
169169
stateCheckCancel()
170170
if stateErr != nil {
171171
if errors.Is(stateErr, ErrFinalizedStateUnavailable) {
@@ -802,8 +802,7 @@ func (n *node[CHAIN_ID, HEAD, RPC]) finalizedStateNotAvailableLoop() {
802802
return
803803
}
804804

805-
checker, ok := any(n.rpc).(FinalizedStateChecker)
806-
if !ok {
805+
if n.finalizedStateChecker == nil {
807806
lggr.Infow("RPC does not implement FinalizedStateChecker, transitioning to alive")
808807
n.declareAlive()
809808
return
@@ -817,14 +816,14 @@ func (n *node[CHAIN_ID, HEAD, RPC]) finalizedStateNotAvailableLoop() {
817816
return
818817
case <-time.After(recheckBackoff.Duration()):
819818
stateCheckCtx, stateCheckCancel := context.WithTimeout(ctx, n.nodePoolCfg.PollInterval())
820-
stateErr := checker.CheckFinalizedStateAvailability(stateCheckCtx)
819+
stateErr := n.finalizedStateChecker.CheckFinalizedStateAvailability(stateCheckCtx)
821820
stateCheckCancel()
822821
if stateErr != nil {
823822
if errors.Is(stateErr, ErrFinalizedStateUnavailable) {
824823
lggr.Warnw("Finalized state still not available", "err", stateErr)
825824
continue
826825
}
827-
lggr.Warnw("Finalized state check failed with RPC error", "err", stateErr)
826+
lggr.Errorw("Finalized state check failed with RPC error", "err", stateErr)
828827
n.declareUnreachable()
829828
return
830829
}

‎multinode/node_lifecycle_test.go‎

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2386,7 +2386,7 @@ func TestUnit_NodeLifecycle_finalizedStateNotAvailableLoop(t *testing.T) {
23862386

23872387
newFinalizedStateNotAvailableNode := func(t *testing.T, opts testNodeOpts) testNode {
23882388
node := newTestNode(t, opts)
2389-
opts.rpc.On("Close").Return(nil)
2389+
opts.rpc.EXPECT().Close().Return()
23902390
node.setState(nodeStateFinalizedStateNotAvailable)
23912391
return node
23922392
}
@@ -2411,7 +2411,7 @@ func TestUnit_NodeLifecycle_finalizedStateNotAvailableLoop(t *testing.T) {
24112411
})
24122412
defer func() { assert.NoError(t, node.close()) }()
24132413

2414-
rpc.On("Dial", mock.Anything).Return(errors.New("failed to dial"))
2414+
rpc.EXPECT().Dial(mock.Anything).Return(errors.New("failed to dial"))
24152415
node.wg.Add(1)
24162416
go node.finalizedStateNotAvailableLoop()
24172417
tests.AssertLogCountEventually(t, observedLogs, "Node is unreachable", 2)
@@ -2429,9 +2429,9 @@ func TestUnit_NodeLifecycle_finalizedStateNotAvailableLoop(t *testing.T) {
24292429
})
24302430
defer func() { assert.NoError(t, node.close()) }()
24312431

2432-
rpc.On("Dial", mock.Anything).Return(nil)
2433-
rpc.On("ChainID", mock.Anything).Return(nodeChainID, nil)
2434-
rpc.On("CheckFinalizedStateAvailability", mock.Anything).Return(fmt.Errorf("%w: missing trie node", ErrFinalizedStateUnavailable))
2432+
rpc.EXPECT().Dial(mock.Anything).Return(nil)
2433+
rpc.EXPECT().ChainID(mock.Anything).Return(nodeChainID, nil)
2434+
rpc.EXPECT().CheckFinalizedStateAvailability(mock.Anything).Return(fmt.Errorf("%w: missing trie node", ErrFinalizedStateUnavailable))
24352435

24362436
node.wg.Add(1)
24372437
go node.finalizedStateNotAvailableLoop()
@@ -2448,9 +2448,9 @@ func TestUnit_NodeLifecycle_finalizedStateNotAvailableLoop(t *testing.T) {
24482448
})
24492449
defer func() { assert.NoError(t, node.close()) }()
24502450

2451-
rpc.On("Dial", mock.Anything).Return(nil)
2452-
rpc.On("ChainID", mock.Anything).Return(nodeChainID, nil)
2453-
rpc.On("CheckFinalizedStateAvailability", mock.Anything).Return(nil)
2451+
rpc.EXPECT().Dial(mock.Anything).Return(nil)
2452+
rpc.EXPECT().ChainID(mock.Anything).Return(nodeChainID, nil)
2453+
rpc.EXPECT().CheckFinalizedStateAvailability(mock.Anything).Return(nil)
24542454

24552455
setupRPCForAliveLoop(t, rpc)
24562456

@@ -2477,26 +2477,26 @@ func TestUnit_NodeLifecycle_finalizedStateNotAvailableLoop(t *testing.T) {
24772477
})
24782478
defer func() { assert.NoError(t, node.close()) }()
24792479

2480-
rpc.On("Close").Return(nil)
2481-
rpc.On("Dial", mock.Anything).Return(nil).Maybe()
2482-
rpc.On("ChainID", mock.Anything).Return(nodeChainID, nil).Maybe()
2480+
rpc.EXPECT().Close().Return()
2481+
rpc.EXPECT().Dial(mock.Anything).Return(nil).Maybe()
2482+
rpc.EXPECT().ChainID(mock.Anything).Return(nodeChainID, nil).Maybe()
24832483

24842484
sub := newMockSubscription(t)
2485-
sub.On("Err").Return(nil).Maybe()
2486-
sub.On("Unsubscribe").Maybe()
2485+
sub.EXPECT().Err().Return(nil).Maybe()
2486+
sub.EXPECT().Unsubscribe().Return().Maybe()
24872487
headsCh := make(chan Head)
2488-
rpc.On("SubscribeToHeads", mock.Anything).Return((<-chan Head)(headsCh), sub, nil).Maybe()
2489-
rpc.On("SubscribeToFinalizedHeads", mock.Anything).Return((<-chan Head)(headsCh), sub, nil).Maybe()
2490-
rpc.On("GetInterceptedChainInfo").Return(ChainInfo{}, ChainInfo{}).Maybe()
2491-
rpc.On("ClientVersion", mock.Anything).Return("", nil).Maybe()
2488+
rpc.EXPECT().SubscribeToHeads(mock.Anything).Return((<-chan Head)(headsCh), sub, nil).Maybe()
2489+
rpc.EXPECT().SubscribeToFinalizedHeads(mock.Anything).Return((<-chan Head)(headsCh), sub, nil).Maybe()
2490+
rpc.EXPECT().GetInterceptedChainInfo().Return(ChainInfo{}, ChainInfo{}).Maybe()
2491+
rpc.EXPECT().ClientVersion(mock.Anything).Return("", nil).Maybe()
24922492

2493-
rpc.On("CheckFinalizedStateAvailability", mock.Anything).Return(
2493+
rpc.EXPECT().CheckFinalizedStateAvailability(mock.Anything).Return(
24942494
fmt.Errorf("%w: missing trie node", ErrFinalizedStateUnavailable),
24952495
).Times(3)
24962496

24972497
poolInfo := newMockPoolChainInfoProvider(t)
2498-
poolInfo.On("LatestChainInfo", mock.Anything).Return(5, ChainInfo{}).Maybe()
2499-
poolInfo.On("HighestUserObservations").Return(ChainInfo{}).Maybe()
2498+
poolInfo.EXPECT().LatestChainInfo(mock.Anything).Return(5, ChainInfo{}).Maybe()
2499+
poolInfo.EXPECT().HighestUserObservations().Return(ChainInfo{}).Maybe()
25002500
node.SetPoolChainInfoProvider(poolInfo)
25012501

25022502
node.setState(nodeStateDialed)
@@ -2506,7 +2506,7 @@ func TestUnit_NodeLifecycle_finalizedStateNotAvailableLoop(t *testing.T) {
25062506
return node.State() == nodeStateFinalizedStateNotAvailable
25072507
})
25082508

2509-
rpc.On("CheckFinalizedStateAvailability", mock.Anything).Return(nil).Maybe()
2509+
rpc.EXPECT().CheckFinalizedStateAvailability(mock.Anything).Return(nil).Maybe()
25102510

25112511
tests.AssertEventually(t, func() bool {
25122512
return node.State() == nodeStateAlive

‎multinode/rpc_client_base.go‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -331,10 +331,3 @@ func (m *RPCClientBase[HEAD]) GetInterceptedChainInfo() (latest, highestUserObse
331331
defer m.chainInfoLock.RUnlock()
332332
return m.latestChainInfo, m.highestUserObservations
333333
}
334-
335-
// CheckFinalizedStateAvailability provides a default no-op implementation for the FinalizedStateChecker interface.
336-
// Chain-specific RPC clients can override this method to verify that the RPC can serve
337-
// historical state at the finalized block (e.g., by calling eth_getBalance at the finalized block).
338-
func (m *RPCClientBase[HEAD]) CheckFinalizedStateAvailability(ctx context.Context) error {
339-
return nil
340-
}

0 commit comments

Comments
 (0)