Skip to content

Commit 000a6e6

Browse files
julienrbrtjulienrbrt
committed
fix unit tests, implement syncer verification
1 parent 07705e3 commit 000a6e6

File tree

4 files changed

+54
-17
lines changed

4 files changed

+54
-17
lines changed

‎block/internal/syncing/syncer.go‎

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -905,6 +905,26 @@ func (s *Syncer) VerifyForcedInclusionTxs(ctx context.Context, currentState type
905905
return fmt.Errorf("failed to retrieve forced included txs from DA: %w", err)
906906
}
907907

908+
// Filter out invalid forced inclusion transactions using the executor's FilterTxs.
909+
// This ensures we don't mark the sequencer as malicious for not including txs that
910+
// were legitimately filtered (e.g., malformed, unparseable, or otherwise invalid).
911+
validForcedTxs := forcedIncludedTxsEvent.Txs
912+
if len(forcedIncludedTxsEvent.Txs) > 0 {
913+
filterStatuses, filterErr := s.exec.FilterTxs(ctx, forcedIncludedTxsEvent.Txs, 0, 0, true)
914+
if filterErr != nil {
915+
s.logger.Warn().Err(filterErr).Msg("failed to filter forced inclusion txs, checking no txs")
916+
validForcedTxs = [][]byte{}
917+
} else {
918+
validForcedTxs = make([][]byte, 0, len(forcedIncludedTxsEvent.Txs))
919+
for i, status := range filterStatuses {
920+
if status != coreexecutor.FilterOK {
921+
continue
922+
}
923+
validForcedTxs = append(validForcedTxs, forcedIncludedTxsEvent.Txs[i])
924+
}
925+
}
926+
}
927+
908928
// Build map of transactions in current block
909929
blockTxMap := make(map[string]struct{})
910930
for _, tx := range data.Txs {
@@ -929,9 +949,9 @@ func (s *Syncer) VerifyForcedInclusionTxs(ctx context.Context, currentState type
929949
return true
930950
})
931951

932-
// Add new forced inclusion transactions from current epoch
952+
// Add new forced inclusion transactions from current epoch (only valid ones)
933953
var newPendingCount, includedCount int
934-
for _, forcedTx := range forcedIncludedTxsEvent.Txs {
954+
for _, forcedTx := range validForcedTxs {
935955
txHash := hashTx(forcedTx)
936956
if _, ok := blockTxMap[txHash]; ok {
937957
// Transaction is included in this block
@@ -1005,7 +1025,7 @@ func (s *Syncer) VerifyForcedInclusionTxs(ctx context.Context, currentState type
10051025
}
10061026

10071027
// Log current state
1008-
if len(forcedIncludedTxsEvent.Txs) > 0 {
1028+
if len(validForcedTxs) > 0 {
10091029
if newPendingCount > 0 {
10101030
totalPending := 0
10111031
s.pendingForcedInclusionTxs.Range(func(key, value any) bool {
@@ -1021,11 +1041,13 @@ func (s *Syncer) VerifyForcedInclusionTxs(ctx context.Context, currentState type
10211041
Int("included_count", includedCount).
10221042
Int("deferred_count", newPendingCount).
10231043
Int("total_pending", totalPending).
1044+
Int("filtered_invalid", len(forcedIncludedTxsEvent.Txs)-len(validForcedTxs)).
10241045
Msg("forced inclusion transactions processed - some deferred due to block size constraints")
10251046
} else {
10261047
s.logger.Debug().
10271048
Uint64("height", data.Height()).
1028-
Int("forced_txs", len(forcedIncludedTxsEvent.Txs)).
1049+
Int("forced_txs", len(validForcedTxs)).
1050+
Int("filtered_invalid", len(forcedIncludedTxsEvent.Txs)-len(validForcedTxs)).
10291051
Msg("all forced inclusion transactions included in block")
10301052
}
10311053
}

‎block/internal/syncing/syncer_forced_inclusion_test.go‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515
"github.com/evstack/ev-node/block/internal/cache"
1616
"github.com/evstack/ev-node/block/internal/common"
1717
"github.com/evstack/ev-node/block/internal/da"
18+
"github.com/evstack/ev-node/core/execution"
1819
"github.com/evstack/ev-node/pkg/config"
1920
datypes "github.com/evstack/ev-node/pkg/da/types"
2021
"github.com/evstack/ev-node/pkg/genesis"
@@ -23,6 +24,21 @@ import (
2324
"github.com/evstack/ev-node/types"
2425
)
2526

27+
// setupFilterTxsMock sets up the FilterTxs mock to return FilterOK for all transactions.
28+
// This is the default behavior for tests that don't specifically test filtering.
29+
func setupFilterTxsMock(mockExec *testmocks.MockExecutor) {
30+
mockExec.On("FilterTxs", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(
31+
func(ctx context.Context, txs [][]byte, maxBytes, maxGas uint64, hasForceIncludedTransaction bool) []execution.FilterStatus {
32+
result := make([]execution.FilterStatus, len(txs))
33+
for i := range result {
34+
result[i] = execution.FilterOK
35+
}
36+
return result
37+
},
38+
nil,
39+
).Maybe()
40+
}
41+
2642
func TestCalculateBlockFullness_HalfFull(t *testing.T) {
2743
s := &Syncer{}
2844

@@ -363,6 +379,7 @@ func TestVerifyForcedInclusionTxs_AllTransactionsIncluded(t *testing.T) {
363379
mockExec := testmocks.NewMockExecutor(t)
364380
mockExec.EXPECT().InitChain(mock.Anything, mock.Anything, uint64(1), "tchain").
365381
Return([]byte("app0"), nil).Once()
382+
setupFilterTxsMock(mockExec)
366383

367384
client := testmocks.NewMockClient(t)
368385
client.On("GetHeaderNamespace").Return([]byte(cfg.DA.Namespace)).Maybe()
@@ -437,6 +454,7 @@ func TestVerifyForcedInclusionTxs_MissingTransactions(t *testing.T) {
437454
mockExec := testmocks.NewMockExecutor(t)
438455
mockExec.EXPECT().InitChain(mock.Anything, mock.Anything, uint64(1), "tchain").
439456
Return([]byte("app0"), nil).Once()
457+
setupFilterTxsMock(mockExec)
440458

441459
client := testmocks.NewMockClient(t)
442460
client.On("GetHeaderNamespace").Return([]byte(cfg.DA.Namespace)).Maybe()
@@ -541,6 +559,7 @@ func TestVerifyForcedInclusionTxs_PartiallyIncluded(t *testing.T) {
541559
mockExec := testmocks.NewMockExecutor(t)
542560
mockExec.EXPECT().InitChain(mock.Anything, mock.Anything, uint64(1), "tchain").
543561
Return([]byte("app0"), nil).Once()
562+
setupFilterTxsMock(mockExec)
544563

545564
client := testmocks.NewMockClient(t)
546565
client.On("GetHeaderNamespace").Return([]byte(cfg.DA.Namespace)).Maybe()
@@ -649,6 +668,7 @@ func TestVerifyForcedInclusionTxs_NoForcedTransactions(t *testing.T) {
649668
mockExec := testmocks.NewMockExecutor(t)
650669
mockExec.EXPECT().InitChain(mock.Anything, mock.Anything, uint64(1), "tchain").
651670
Return([]byte("app0"), nil).Once()
671+
setupFilterTxsMock(mockExec)
652672

653673
client := testmocks.NewMockClient(t)
654674
client.On("GetHeaderNamespace").Return([]byte(cfg.DA.Namespace)).Maybe()
@@ -716,6 +736,7 @@ func TestVerifyForcedInclusionTxs_NamespaceNotConfigured(t *testing.T) {
716736
mockExec := testmocks.NewMockExecutor(t)
717737
mockExec.EXPECT().InitChain(mock.Anything, mock.Anything, uint64(1), "tchain").
718738
Return([]byte("app0"), nil).Once()
739+
setupFilterTxsMock(mockExec)
719740

720741
client := testmocks.NewMockClient(t)
721742
client.On("GetHeaderNamespace").Return([]byte(cfg.DA.Namespace)).Maybe()
@@ -783,6 +804,7 @@ func TestVerifyForcedInclusionTxs_DeferralWithinEpoch(t *testing.T) {
783804
mockExec := testmocks.NewMockExecutor(t)
784805
mockExec.EXPECT().InitChain(mock.Anything, mock.Anything, uint64(1), "tchain").
785806
Return([]byte("app0"), nil).Once()
807+
setupFilterTxsMock(mockExec)
786808

787809
client := testmocks.NewMockClient(t)
788810
client.On("GetHeaderNamespace").Return([]byte(cfg.DA.Namespace)).Maybe()
@@ -907,6 +929,7 @@ func TestVerifyForcedInclusionTxs_MaliciousAfterEpochEnd(t *testing.T) {
907929
mockExec := testmocks.NewMockExecutor(t)
908930
mockExec.EXPECT().InitChain(mock.Anything, mock.Anything, uint64(1), "tchain").
909931
Return([]byte("app0"), nil).Once()
932+
setupFilterTxsMock(mockExec)
910933

911934
client := testmocks.NewMockClient(t)
912935
client.On("GetHeaderNamespace").Return([]byte(cfg.DA.Namespace)).Maybe()
@@ -996,6 +1019,7 @@ func TestVerifyForcedInclusionTxs_SmoothingExceedsEpoch(t *testing.T) {
9961019
mockExec := testmocks.NewMockExecutor(t)
9971020
mockExec.EXPECT().InitChain(mock.Anything, mock.Anything, uint64(1), "tchain").
9981021
Return([]byte("app0"), nil).Once()
1022+
setupFilterTxsMock(mockExec)
9991023

10001024
client := testmocks.NewMockClient(t)
10011025
client.On("GetHeaderNamespace").Return([]byte(cfg.DA.Namespace)).Maybe()

‎execution/evm/test/execution_test.go‎

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -80,10 +80,9 @@ func TestEngineExecution(t *testing.T) {
8080

8181
ctx, cancel := context.WithTimeout(context.Background(), 300*time.Second)
8282
defer cancel()
83-
stateRoot, gasLimit, err := executionClient.InitChain(ctx, genesisTime, initialHeight, CHAIN_ID)
83+
stateRoot, err := executionClient.InitChain(ctx, genesisTime, initialHeight, CHAIN_ID)
8484
require.NoError(t, err)
8585
require.Equal(t, GenesisStateRoot, stateRoot)
86-
require.NotZero(t, gasLimit)
8786

8887
prevStateRoot := GenesisStateRoot
8988
lastHeight, lastHash, lastTxs := checkLatestBlock(tt, ctx, ethURL)
@@ -125,11 +124,8 @@ func TestEngineExecution(t *testing.T) {
125124
allTimestamps = append(allTimestamps, blockTimestamp)
126125

127126
// Execute transactions and get the new state root
128-
newStateRoot, maxBytes, err := executionClient.ExecuteTxs(ctx, payload, blockHeight, blockTimestamp, prevStateRoot)
127+
newStateRoot, err := executionClient.ExecuteTxs(ctx, payload, blockHeight, blockTimestamp, prevStateRoot)
129128
require.NoError(tt, err)
130-
if nTxs > 0 {
131-
require.NotZero(tt, maxBytes)
132-
}
133129

134130
err = executionClient.SetFinal(ctx, blockHeight)
135131
require.NoError(tt, err)
@@ -181,10 +177,9 @@ func TestEngineExecution(t *testing.T) {
181177

182178
ctx, cancel := context.WithTimeout(context.Background(), 300*time.Second)
183179
defer cancel()
184-
stateRoot, gasLimit, err := executionClient.InitChain(ctx, genesisTime, initialHeight, CHAIN_ID)
180+
stateRoot, err := executionClient.InitChain(ctx, genesisTime, initialHeight, CHAIN_ID)
185181
require.NoError(t, err)
186182
require.Equal(t, GenesisStateRoot, stateRoot)
187-
require.NotZero(t, gasLimit)
188183

189184
prevStateRoot := GenesisStateRoot
190185
lastHeight, lastHash, lastTxs := checkLatestBlock(tt, ctx, ethURL)
@@ -200,11 +195,8 @@ func TestEngineExecution(t *testing.T) {
200195

201196
// Use timestamp from build phase for each block to ensure proper ordering
202197
blockTimestamp := allTimestamps[blockHeight-1]
203-
newStateRoot, maxBytes, err := executionClient.ExecuteTxs(ctx, payload, blockHeight, blockTimestamp, prevStateRoot)
198+
newStateRoot, err := executionClient.ExecuteTxs(ctx, payload, blockHeight, blockTimestamp, prevStateRoot)
204199
require.NoError(t, err)
205-
if len(payload) > 0 {
206-
require.NotZero(tt, maxBytes)
207-
}
208200
if len(payload) == 0 {
209201
require.Equal(tt, prevStateRoot, newStateRoot)
210202
} else {

‎pkg/telemetry/sequencer_tracing_test.go‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,6 @@ func TestTracedSequencer_GetNextBatch_Success(t *testing.T) {
150150

151151
attrs := span.Attributes()
152152
testutil.RequireAttribute(t, attrs, "tx.count", 2)
153-
testutil.RequireAttribute(t, attrs, "forced_inclusion.count", 1)
154153
testutil.RequireAttribute(t, attrs, "max_bytes", int64(1000))
155154
}
156155

0 commit comments

Comments
 (0)