Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes several issues in the logpoller replay mechanism and hardens data validation:
Changes:
- Refactors replay data model to separately track
fromBlockandprevBlockinstead of a singlerequestBlock, fixing off-by-one errors and enabling block 1 replay - Adds
ValidateBlockIDExtfunction to validate RootHash/FileHash are 32 bytes at ingestion and deserialization points - Hardens BOC handling by rejecting nil log data and detecting inconsistent header/payload states
- Fixes replay status access using
ReplayStatus()method instead of direct field access - Updates test fixtures to include required RootHash/FileHash fields
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/logpoller/store/postgres/models_test.go | Adds required RootHash/FileHash fields to test BlockIDExt fixture |
| pkg/logpoller/store/postgres/models.go | Adds nil check for log.Data, validates BlockIDExt, and detects inconsistent BOC header/payload states |
| pkg/logpoller/service.go | Updates replay logic to use new fromBlock/prevBlock model and fixes status access |
| pkg/logpoller/replay_test.go | Updates tests to reflect new replay data model with fromBlock/prevBlock separation |
| pkg/logpoller/replay.go | Refactors replay state tracking to use fromBlock+prevBlock instead of single requestBlock |
| pkg/logpoller/models/models_test.go | Adds comprehensive tests for ValidateBlockIDExt function |
| pkg/logpoller/models/models.go | Adds ValidateBlockIDExt function with int256Len constant |
| pkg/logpoller/loader/loader.go | Adds ValidateBlockIDExt call during transaction loading |
| integration-tests/smoke/chainaccessor/accessor_test.go | Adds testBlockIDExt helper and updates fixtures to include hash fields |
| integration-tests/logpoller/pg_pruning_test.go | Updates test fixtures to use testBlockIDExt helper |
| integration-tests/logpoller/pg_logs_test.go | Adds testBlockIDExt helper and updates fixtures |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -99,18 +99,21 @@ type logModel struct { | |||
|
|
|||
| // FromLog converts a models.Log to logModel | |||
| func (l *logModel) FromLog(log lptypes.Log) (logModel, error) { | |||
There was a problem hiding this comment.
Nit: I noticed both value and pointer receivers are used for logModel (and filterModel above). The Go convention if any method needs a pointer receiver (state changes), make them all pointer receivers for that type.
| ) | ||
|
|
||
| // testBlockIDExt creates a valid BlockIDExt for testing with required hash fields | ||
| func testBlockIDExt(seqNo uint32) *ton.BlockIDExt { |
There was a problem hiding this comment.
This duplicates with integration-tests/logpoller/pg_logs_test.go, could consider moving into testutil package
huangzhen1997
left a comment
There was a problem hiding this comment.
lgtm, just few minor nits
210452b
Summary
fromBlock(uint32) andprevBlock(*BlockIDExt) separately instead of a singlerequestBlock, fixing off-by-one edge cases in (prev, to] blockrange semantics and enabling block 1 replay
ValidateBlockIDExtto validate RootHash/FileHash lengths (32 bytes) at ingestion and deserialization boundariesReplayStatus()accessor