Skip to content

Commit

Permalink
Fix bugs in TestLogPoller_BackupPollAndSaveLogsSkippingLogsThatAreTooOld
Browse files Browse the repository at this point in the history
This fixes 2 bugs on develop branch in this test, and removes some
unused commented code.

First Bug
=========
The first bug was causing a false positive PASS on develop branch, which
was obscuring a (very minor) bug in BackupPoller that's been fixed in this PR.

The comment here about what the test was intended to test is still correct:

 // Only the 2nd batch + 1 log from a previous batch should be backfilled, because we perform backfill starting
 // from one block behind the latest finalized block

Contrary to the comment, the code was returning 2 logs from the 1st batch (Data=9 & Data=10), plus 9 of 10 logs
from the 2nd batch. This was incorrect behavior, but the test was also checking for the same incorrect behavior
(looking for 11 logs with first one being Data=9) instead of what's described in the comment.

The bug in the production code was that it starts the Backup Poller at Finalized - 1 instead of Finalized.
This is a harmless "bug", just unnecessarily starting a block too early, since there's no reason for backup
logpoller to re-request the same finalized logs that's already been processed.

Now, the code returns the last log from the 1st batch + all but one logs
from the 2nd batch, which is correct. (It can't return the last log
because that goes beyond the last safe block.) So the test checks that
there are 10 logs with first one being Data=10 (last log from the first
batch.)

Second Bug
==========
The second bug was passing firstBatchBlock and secondBatchBlock directly
to markBlockAsFinalized() instead of passing firstBatchBlock - 1 and
secondBatchBlock - 1. This was only working because of a bug in the
version of geth we're currently using: when you request the pending
block from simulated geth, it gives you back the current block (1 block
prior) instead of the current block. (For example, in the first case,
even though we wanted block 11, the latest current block, we request
block 12 and get back block 11.) This has been fixed in the latest
version of geth... so presumably if we don't fix this here the test
would have started failing as soon as we upgrade to the latest version
of geth. It doesn't change any behavior of the test for the present
version of geth, just makes it more clear that we want block 11 not 12.
  • Loading branch information
reductionista committed Apr 10, 2024
1 parent d7d0169 commit 7a59051
Showing 1 changed file with 4 additions and 6 deletions.
10 changes: 4 additions & 6 deletions core/chains/evm/logpoller/log_poller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -545,8 +545,6 @@ func TestLogPoller_BackupPollAndSaveLogsSkippingLogsThatAreTooOld(t *testing.T)
BackupPollerBlockDelay: 1,
}
th := SetupTH(t, lpOpts)
//header, err := th.Client.HeaderByNumber(ctx, nil)
//require.NoError(t, err)

// Emit some logs in blocks
for i := 1; i <= logsBatch; i++ {
Expand All @@ -559,7 +557,7 @@ func TestLogPoller_BackupPollAndSaveLogsSkippingLogsThatAreTooOld(t *testing.T)
// 1 -> 2 -> ... -> firstBatchBlock
firstBatchBlock := th.PollAndSaveLogs(ctx, 1)
// Mark current tip of the chain as finalized (after emitting 10 logs)
markBlockAsFinalized(t, th, firstBatchBlock)
markBlockAsFinalized(t, th, firstBatchBlock-1)

// Emit 2nd batch of block
for i := 1; i <= logsBatch; i++ {
Expand All @@ -571,7 +569,7 @@ func TestLogPoller_BackupPollAndSaveLogsSkippingLogsThatAreTooOld(t *testing.T)
// 1 -> 2 -> ... -> firstBatchBlock (finalized) -> .. -> firstBatchBlock + emitted logs
secondBatchBlock := th.PollAndSaveLogs(ctx, firstBatchBlock)
// Mark current tip of the block as finalized (after emitting 20 logs)
markBlockAsFinalized(t, th, secondBatchBlock)
markBlockAsFinalized(t, th, secondBatchBlock-1)

// Register filter
err := th.LogPoller.RegisterFilter(ctx, logpoller.Filter{
Expand All @@ -595,8 +593,8 @@ func TestLogPoller_BackupPollAndSaveLogsSkippingLogsThatAreTooOld(t *testing.T)
th.EmitterAddress1,
)
require.NoError(t, err)
require.Len(t, logs, logsBatch+1)
require.Equal(t, hexutil.MustDecode(`0x0000000000000000000000000000000000000000000000000000000000000009`), logs[0].Data)
require.Len(t, logs, logsBatch)
require.Equal(t, hexutil.MustDecode(`0x000000000000000000000000000000000000000000000000000000000000000a`), logs[0].Data)
}

func TestLogPoller_BlockTimestamps(t *testing.T) {
Expand Down

0 comments on commit 7a59051

Please sign in to comment.