(Requires Audit) PLEX-2473 LP Sparce Block Store (Part 3)#374
Merged
dhaidashenko merged 14 commits intodevelopfrom Apr 24, 2026
Merged
(Requires Audit) PLEX-2473 LP Sparce Block Store (Part 3)#374dhaidashenko merged 14 commits intodevelopfrom
dhaidashenko merged 14 commits intodevelopfrom
Conversation
dhaidashenko
changed the title
dhaidashenko
changed the base branch from
develop
to
feature/PLEX-2473-lp-batch-insert-blocks
Contributor
|
dhaidashenko
force-pushed
the
feature/PLEX-2473-lp-sparce-block-store
branch
from
498dfff to
4cad2fc
Compare
Unheilbar
previously approved these changes
Feb 26, 2026
silaslenihan
previously approved these changes
Feb 26, 2026
dhaidashenko
force-pushed
the
feature/PLEX-2473-lp-batch-insert-blocks
branch
from
0833d36 to
715d9ba
Compare
dhaidashenko
force-pushed
the
feature/PLEX-2473-lp-sparce-block-store
branch
from
4cad2fc to
3fb2047
Compare
dhaidashenko
force-pushed
the
feature/PLEX-2473-lp-sparce-block-store
branch
from
3fb2047 to
0c4981b
Compare
dhaidashenko
force-pushed
the
feature/PLEX-2473-lp-sparce-block-store
branch
from
0c4981b to
8d8ff31
Compare
dhaidashenko
changed the title
dhaidashenko
force-pushed
the
feature/PLEX-2473-lp-sparce-block-store
branch
from
8d8ff31 to
c681f60
Compare
Contributor
|
This PR is stale because it has been open 30 days with no activity. |
Contributor
|
This PR has been automatically closed because it has been stale for > 30 days. |
Base automatically changed from
feature/PLEX-2473-lp-batch-insert-blocks
to
develop
April 24, 2026 14:49
dhaidashenko
dismissed stale reviews from silaslenihan and Unheilbar
The base branch was changed.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds support for a “sparse blocks” storage mode in the LogPoller to reduce DB writes by optionally skipping persistence of empty (no-log) blocks, and updates reorg/LCA logic plus configuration plumbing to support it.
Changes:
- Add
SelectNewestBlockto the LogPoller ORM and ObservedORM to support ancestor lookup in a sparse blocks table. - Introduce
Opts.SkipEmptyBlocks/LogPollerSkipEmptyBlocksconfiguration and wire it through chain config -> LogPoller. - Update LogPoller reorg handling/LCA search and expand tests for sparse-block scenarios (including skip-empty-blocks).
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/logpoller/orm.go | Adds SelectNewestBlock query for newest block at/below a given height. |
| pkg/logpoller/observability.go | Adds observed wrapper for SelectNewestBlock. |
| pkg/logpoller/log_poller.go | Adds SkipEmptyBlocks option and updates unfinalized log/block collection + LCA search for sparse DB blocks. |
| pkg/logpoller/log_poller_test.go | Refactors/extends tests for skip-empty-blocks and adds DB-vs-geth consistency helpers. |
| pkg/logpoller/log_poller_internal_test.go | Adds dedicated unit tests for findBlockAfterLCA under sparse DB conditions. |
| pkg/config/toml/docs.toml | Documents LogPollerSkipEmptyBlocks. |
| pkg/config/toml/defaults/fallback.toml | Provides default for LogPollerSkipEmptyBlocks. |
| pkg/config/toml/defaults.go | Ensures defaults copy includes LogPollerSkipEmptyBlocks. |
| pkg/config/toml/testdata/config-full.toml | Adds LogPollerSkipEmptyBlocks to full config fixture. |
| pkg/config/toml/config.go | Adds LogPollerSkipEmptyBlocks to TOML Chain struct. |
| pkg/config/toml/config_test.go | Updates config test expectations to include the new field. |
| pkg/config/config.go | Extends EVM config interface with LogPollerSkipEmptyBlocks(). |
| pkg/config/chain_scoped.go | Implements LogPollerSkipEmptyBlocks() accessor. |
| pkg/config/mocks/evm.go | Updates generated mock for new config method. |
| pkg/chains/legacyevm/chain.go | Wires config flag into logpoller.Opts.SkipEmptyBlocks. |
| CONFIG.md | Documents the new TOML setting. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
1250
to
+1277
| @@ -1262,8 +1270,11 @@ func (lp *logPoller) getUnfinalizedLogs(ctx context.Context, currentBlock *evmty | |||
| FinalizedBlockNumber: finalized, | |||
| SafeBlockNumber: safe, | |||
| } | |||
| logs = append(logs, convertLogs(rpcLogs, []Block{block}, lp.lggr, lp.ec.ConfiguredChainID())...) | |||
| blocks = append(blocks, block) | |||
| logs = append(logs, convertLogs(rpcLogs, []Block{*block}, lp.lggr, lp.ec.ConfiguredChainID())...) | |||
| // Skip empty blocks if configured to do so. | |||
| if len(rpcLogs) > 0 || !lp.skipEmptyBlocks { | |||
| blocks = append(blocks, *block) | |||
| } | |||
There was a problem hiding this comment.
getUnfinalizedLogs currently will not compile: it declares var block *Block and then shadows it with block := Block{...} inside the loop, but later dereferences *block and appends *block to slices. This also means the deferred checkpoint logic never sees the last block. Avoid shadowing and keep a single *Block (or use a differently named value like blk) and pass/append by value without dereferencing a non-pointer.
Comment on lines
+2266
to
+2289
| func checkDBMatchesGeth(t *testing.T, orm logpoller.ORM, client logpoller.Client) bool { | ||
| // Check every block is identical | ||
| latest, err1 := client.HeadByNumber(testutils.Context(t), nil) | ||
| require.NoError(t, err1) | ||
| dbBlocks, err := orm.SelectLogsByBlockRange(t.Context(), 0, latest.Number) | ||
| require.NoError(t, err) | ||
| // ensure all blocks present in db are on geth canonical chain | ||
| for _, ourBlock := range dbBlocks { | ||
| gethBlock, err1 := client.HeadByNumber(testutils.Context(t), big.NewInt(ourBlock.BlockNumber)) | ||
| require.NoError(t, err1) | ||
| if ourBlock.BlockHash != gethBlock.Hash { | ||
| t.Logf("Initial poll our block differs at height %d got %x want %x\n", ourBlock.BlockNumber, ourBlock.BlockHash, gethBlock.Hash) | ||
| return false | ||
| } | ||
| } | ||
|
|
||
| latestDB, err := orm.SelectLatestBlock(t.Context()) | ||
| require.NoError(t, err) | ||
| require.Equal(t, latest.Number, latestDB.BlockNumber, "latest block number in db should match geth") | ||
|
|
||
| // ensure all logs present in db are on geth canonical chain | ||
| logs, err1 := orm.SelectLogsByBlockRange(t.Context(), 0, latest.Number) | ||
| require.NoError(t, err1) | ||
| for _, log := range logs { |
There was a problem hiding this comment.
checkDBMatchesGeth attempts to validate DB blocks against the canonical chain, but it calls orm.SelectLogsByBlockRange(...) and iterates those results as dbBlocks. This ends up checking log rows (and will miss blocks with no logs), and it duplicates the same logs query again later. Use a blocks query (e.g., orm.GetBlocksRange(...) or similar) for the blocks-table check, and keep the logs query only for validating log rows.
Contributor
|
fernandezlautaro
approved these changes
Apr 24, 2026
ilija42
approved these changes
Apr 24, 2026
dhaidashenko
added a commit
that referenced
this pull request
Apr 28, 2026
This reverts commit 938a963.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Required to reduce LP's DB usage.
Depends on #356