-
Notifications
You must be signed in to change notification settings - Fork 887
feat(data): integrate LittDB-backed BlockDB into autobahn data layer (CON-272) #3707
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
40e8335
039c935
9bf41f8
64a2608
c179ece
cd75fbd
18c3c3c
f7ff617
a2c2efd
e4b398f
2ab8c62
048ae1e
36fb633
c1de32b
50ff709
61280b3
d519368
6ba6dd7
1838665
e2bb015
ec6bbcb
2ab6f38
0a29226
f3775e2
8803170
7ffd047
3abb764
fa50942
674d934
88a10c4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,6 +5,7 @@ import ( | |
| "fmt" | ||
| "net/url" | ||
|
|
||
| "github.com/sei-protocol/sei-chain/sei-db/ledger_db/block/littblock" | ||
| atypes "github.com/sei-protocol/sei-chain/sei-tendermint/autobahn/types" | ||
| "github.com/sei-protocol/sei-chain/sei-tendermint/internal/p2p" | ||
| "github.com/sei-protocol/sei-chain/sei-tendermint/libs/utils" | ||
|
|
@@ -34,6 +35,26 @@ type AutobahnValidator struct { | |
| EVMRPC URL `json:"evmrpc"` | ||
| } | ||
|
|
||
| // AutobahnBlockDBConfig holds optional overrides for the LittDB-backed BlockDB | ||
| // opened under persistent_state_dir/blockdb. Paths are never taken from here — | ||
| // Autobahn always roots BlockDB at <persistent_state_dir>/blockdb. | ||
| // | ||
| // Each field is independently optional. Absent fields keep whatever | ||
| // littblock.DefaultConfig currently uses (do not duplicate those values here — | ||
| // they live in the littblock / LittDB packages and may change). | ||
| // | ||
| // Disk impact (most → least useful for bounding usage): Retention, then | ||
| // GCPeriod. Segment sizing is intentionally not exposed (engine-internal). | ||
| type AutobahnBlockDBConfig struct { | ||
| // Retention is the failsafe minimum age before pruned records may be | ||
| // reclaimed. Primary knob for worst-case disk after PruneBefore advances. | ||
| // Absent ⇒ littblock.DefaultConfig Retention. | ||
| Retention utils.Option[utils.Duration] `json:"retention"` | ||
| // GCPeriod is how often GC runs once data is eligible (reclaim latency). | ||
| // Absent ⇒ littblock.DefaultConfig / LittDB GCPeriod. | ||
| GCPeriod utils.Option[utils.Duration] `json:"gc_period"` | ||
| } | ||
|
|
||
| // AutobahnFileConfig is the JSON structure of the autobahn config file. | ||
| type AutobahnFileConfig struct { | ||
| Validators []AutobahnValidator `json:"validators"` | ||
|
|
@@ -49,6 +70,11 @@ type AutobahnFileConfig struct { | |
| // fullnodes serving downstream block-sync are subject to the same | ||
| // cap). Absent ⇒ DefaultMaxInboundFullnodePeers. Some(0) ⇒ reject all. | ||
| MaxInboundFullnodePeers utils.Option[uint64] `json:"max_inbound_fullnode_peers"` | ||
| // BlockDB optionally overlays AutobahnBlockDBConfig onto littblock.DefaultConfig | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if all fields are optional, then does it really make sense to make it optional as a whole? You can just add omitzero to the json spec to avoid having it serialized. |
||
| // when PersistentStateDir is set. Zero value ⇒ littblock.DefaultConfig unchanged | ||
| // (see AutobahnBlockDBConfig for field semantics). Ignored when | ||
| // PersistentStateDir is absent (memblock). Omitted from JSON when empty. | ||
| BlockDB AutobahnBlockDBConfig `json:"block_db,omitzero"` | ||
| } | ||
|
|
||
| // DefaultMaxInboundFullnodePeers is the built-in cap used when | ||
|
|
@@ -80,5 +106,40 @@ func (fc *AutobahnFileConfig) Validate() error { | |
| if fc.DialInterval <= 0 { | ||
| return errors.New("dial_interval must be > 0") | ||
| } | ||
| if err := fc.BlockDB.Validate(); err != nil { | ||
| return fmt.Errorf("block_db: %w", err) | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| // Validate checks optional BlockDB overrides. Absent fields are fine. | ||
| func (c AutobahnBlockDBConfig) Validate() error { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [suggestion]
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. how about making Validate() return an instantiated littDB config (with the defaults applied)?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (or add a separate method which resolves the littDB config) |
||
| if r, ok := c.Retention.Get(); ok && r <= 0 { | ||
| return errors.New("retention must be > 0 when set") | ||
| } | ||
| if p, ok := c.GCPeriod.Get(); ok && p <= 0 { | ||
| return errors.New("gc_period must be > 0 when set") | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| // LittBlockConfig returns littblock.DefaultConfig(dir) with this config's | ||
| // optional overrides applied. Fsync is always forced on. | ||
| func (c AutobahnBlockDBConfig) LittBlockConfig(dir string) (littblock.LittBlockConfig, error) { | ||
| if err := c.Validate(); err != nil { | ||
| return littblock.LittBlockConfig{}, err | ||
| } | ||
| cfg, err := littblock.DefaultConfig(dir) | ||
| if err != nil { | ||
| return littblock.LittBlockConfig{}, fmt.Errorf("littblock.DefaultConfig: %w", err) | ||
| } | ||
| if r, ok := c.Retention.Get(); ok { | ||
| cfg.Retention = r.Duration() | ||
| } | ||
| if p, ok := c.GCPeriod.Get(); ok { | ||
| cfg.Litt.GCPeriod = p.Duration() | ||
| } | ||
| // NOT SAFE to set false: crash can lose acknowledged BlockDB writes. | ||
| cfg.Litt.Fsync = true | ||
| return *cfg, nil | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 TestLittblockNoBlockWithoutQCAfterTornTail and TestLittblockFlushSurvivesHardKill dropped their
require.True(t, qc.IsPresent(), ...)assertion, leaving onlyrequire.NoErroron the result ofReadQCByBlockNumber. Per the BlockDB contract, a block with no covering QC returns(None, nil)— a nil error — so both tests now pass even if a block survives crash recovery without its covering QC, which is exactly the invariant they exist to verify.Extended reasoning...
What changed. In
sei-db/ledger_db/block/littblock_crash_test.go, bothTestLittblockNoBlockWithoutQCAfterTornTail(torn-tail recovery) andTestLittblockFlushSurvivesHardKill(hard-kill recovery) previously did:and now do:
Why this is vacuous. Per the
BlockDB.ReadQCByBlockNumbercontract (sei-tendermint/autobahn/types/block_db.go:262-277), when a block number is at or above the retention watermark but has no covering QC, the method returnsutils.Nonewith a nil error — err is only non-nil for genuine failures (ErrPruned, I/O/decode errors). The littblock implementation (litt_block_db.goaround line 410-416) matches this contract exactly: whens.table.Get(qcKey(n))reports!exists, it returns(utils.None, nil).In both of these crash-recovery tests, the watermark stays at genesis (no pruning occurs in either torn-tail or hard-kill scenarios), so every block number iterated is at or above the watermark. That means a block which survives crash recovery without its covering QC — exactly the failure mode these tests are named to catch — now produces
(None, nil)fromReadQCByBlockNumber, andrequire.NoError(t, err, ...)passes without complaint. The droppedrequire.True(t, qc.IsPresent(), ...)was the only assertion that actually checked for QC presence.Why nothing else catches this. These are the only two assertions in the codebase directly verifying the 'every persisted block is still covered by a persisted QC' invariant after a simulated crash (torn WAL tail / hard kill). No other test path exercises this specific recovery scenario, so weakening these two removes the only regression guard for this invariant.
Step-by-step proof:
nsurvives on disk but its covering QC does not.db2.ReadQCByBlockNumber(n)is called. Sincenis at or above the (genesis) watermark and no QC covers it, the method returns(utils.None, nil)per contract._, err = db2.ReadQCByBlockNumber(n)thenrequire.NoError(t, err, ...). Sinceerris nil, this assertion passes.present++and continues; the test function returns without failing.Impact and fix. This is test-only: no production/runtime/consensus code path is affected, and the underlying littblock recovery logic itself is unchanged by this PR (all three verifiers independently confirmed the current implementation still satisfies the invariant in practice). The regression is purely in the tests' ability to detect a future violation. The fix is to restore the dropped assertion, e.g.:
at both call sites (lines ~85 and ~232 pre-diff).