Conversation
Embed Shovel as a Go library to index PDPVerifier contract events into Postgres tables, replacing the linear scan of all proof sets that made 4+ RPC calls per proof set per cycle. The tracker now reads from Shovel's inbox tables (transient event rows), materializes state into a new pdp_proof_sets table and existing Deal records, then deletes processed rows. Remaining RPC calls are bounded: one getDataSetListener per new proof set, one getActivePieces per PiecesAdded/Removed event.
0e68f4e to
60def83
Compare
also add ./service/pdptracker/... to CI integration test path
Shovel only defaults PollDuration to 1s during JSON unmarshal, not when the struct is constructed directly. Zero value propagates to jrpc2.Client and panics in time.NewTicker(0).
…nvil-tests pdp anvil tests
|
|
clears shovel cursor and truncates integration tables so the indexer restarts from the PDPVerifier deployment block. requires an archival RPC endpoint since public nodes limit lookback depth. also fixes Start block propagation to shovel tasks: the integration source refs were only carrying the Name field, so WithRange always got Start=0 and shovel fell through to "start at latest". deploy blocks: mainnet 5441432, calibnet 3140755 (from filfox).
eb3a4c8 to
c49adb9
Compare
|
ok, this is ready to review, tradeoffs:
the upside is that this is... scalable and performant, we only make 1 bare RPC call per proofset creation and subsequently rely only on logs in my mind this is a valid set of tradeoffs |
| } | ||
|
|
||
| // failed rows are retained for retry on next cycle | ||
| func processInbox[R inboxRow](db *gorm.DB, query, table string, fn func(R) error) error { |
There was a problem hiding this comment.
This inbox pattern (SELECT rows, process, then broad DELETE) can race with concurrent inserts from Shovel and delete rows we never processed.
Can we switch to deleting only the exact claimed rows (by deterministic event key) in the same transaction, instead of DELETE FROM table / NOT IN failed?
|
|
||
| func (r nextProvingPeriodRow) setID() uint64 { return r.SetID_ } | ||
|
|
||
| func processNextProvingPeriod(ctx context.Context, db *gorm.DB) error { |
There was a problem hiding this comment.
If pdp_proof_sets doesn’t contain set_id yet, this update affects 0 rows but still returns nil, and the inbox row gets deleted.
That can lose ordering-dependent events when DataSetCreated is delayed.
Can we retain this row by checking existence / rows affected and returning an error when proof set is missing?
|
|
||
| func (r possessionProvenRow) setID() uint64 { return r.SetID_ } | ||
|
|
||
| func processPossessionProven(ctx context.Context, db *gorm.DB) error { |
There was a problem hiding this comment.
Same concern as NextProvingPeriod: missing proof set currently results in “successful no-op update” and row deletion.
Can we make missing proof set a retry condition (return error), so these events are retained until DataSetCreated materializes the set?
service/pdptracker/eventprocessor.go
Outdated
|
|
||
| func processDataSetCreated(ctx context.Context, db *gorm.DB, rpcClient *ChainPDPClient) error { | ||
| return processInbox(db, | ||
| "SELECT set_id, storage_provider, block_num FROM pdp_dataset_created", |
There was a problem hiding this comment.
Can we add deterministic ordering (ORDER BY block_num, tx_idx, log_idx, abi_idx) to preserve on-chain event order?
Without ordering, state outcomes can depend on DB row return order when multiple events are pending.
(this applies to all the sql queries)
|
Strong architectural improvement; not approving yet due to event-loss edge cases. Please fix atomic dequeue + missing-proofset retention, and add deterministic ordering. Ready after that i think |
|
yep all good points, have been hunting these down (similar issues but under slightly different scenarios) but there's a lot -- will update shortly |
|
ok I have cleanup but I'm working on 3 separate projects at once and my head is hurting so I'm prone to make mistakes, it being friday afternoon I am going to take the safe route and defer fixes until monday |
- delete by exact event key (block_num, tx_idx, log_idx) instead of broad DELETE to prevent racing with concurrent shovel inserts - check RowsAffected on proofset updates; retain inbox rows when proofset not yet materialized - add ORDER BY block_num, tx_idx, log_idx to all inbox queries
|
@anjor well I lied and apparently I like working on weekends 🫠 feeling ok about current state |
No description provided.