fix: sealing pipeline commit batch robustness and allocation validation - #13632
marco-storswift wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR aims to improve robustness and correctness across the sealing pipeline’s commit batching/allocation validation logic, and also includes changes to Ethereum receipt/log retrieval backed by the chain indexer, plus a set of release/version/documentation updates.
Changes:
- Sealing: adds commit batch “result completeness” handling, failure tracking/eviction, and makes allocation checks deterministic; adds allocation term validation in piece intake and precommit policy.
- ETH/indexer: narrows
eth_getTransactionReceiptlog retrieval to a single message at the indexer/SQL level; adjusts chain indexer filtering semantics and optimizes event-to-log conversion. - Release/config/docs: bumps versions to
1.36.0, updates changelog, OpenRPC JSON versions, and documentsEvents.MaxFilterResultsbehavior; updates mainnet upgrade height constant.
Reviewed changes
Copilot reviewed 31 out of 33 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| storage/pipeline/commit_batch.go | Commit batching robustness changes (fallback result delivery, failure tracking/eviction, deterministic allocation checks). |
| storage/pipeline/input.go | Adds allocation TermMax validation based on seal proof/minimum lifetime requirements. |
| storage/pipeline/precommit_policy.go | Validates allocation piece CID/size and clamps expiration against allocation term bounds. |
| storage/wdpost/wdpost_run_test.go | Updates test tipset construction to include a non-nil ticket. |
| chain/types/tipset.go | Enforces non-nil Ticket in NewTipSet. |
| chain/sync.go | Adds nil-safety checks when computing message metadata CIDs. |
| chain/events/tscache_test.go | Updates tipset creation in tests to include a ticket. |
| chain/events/state/mock/tipset.go | Updates mock tipset creation to include a ticket. |
| node/impl/eth/api.go | Adds ErrEventsNotYetAvailable and updates internal ETH events interface signature. |
| node/impl/eth/events.go | Adds message-CID–scoped log retrieval via chain indexer; refines “events not yet available” behavior; optimizes log conversion. |
| node/impl/eth/utils.go | Passes message CID into receipt construction to fetch logs for the specific message. |
| node/impl/eth/transaction.go | Updates receipt creation call sites to provide the message CID. |
| chain/index/interface.go | Extends EventFilter with optional MsgCid restriction. |
| chain/index/events.go | Adds MsgCid filtering, adjusts MaxResults semantics, and adds row-level memoization for event assembly. |
| chain/index/events_test.go | Updates/extends tests for new MaxResults semantics and MsgCid filtering. |
| node/config/types.go | Expands documentation for EventsConfig.MaxFilterResults semantics. |
| node/config/doc_gen.go | Regenerates config docs to match updated MaxFilterResults comment. |
| documentation/en/default-lotus-config.toml | Updates default config comments for MaxFilterResults. |
| go.mod | Bumps github.com/filecoin-project/go-f3 to v0.8.13. |
| go.sum | Updates checksums for go-f3 v0.8.13. |
| build/version.go | Bumps build versions to 1.36.0. |
| build/openrpc/worker.json | Updates OpenRPC version string to 1.36.0. |
| build/openrpc/v2/gateway.json | Updates OpenRPC version string to 1.36.0. |
| build/openrpc/v2/full.json | Updates OpenRPC version string to 1.36.0. |
| build/openrpc/v0/gateway.json | Updates OpenRPC version string to 1.36.0. |
| build/openrpc/miner.json | Updates OpenRPC version string to 1.36.0. |
| build/openrpc/gateway.json | Updates OpenRPC version string to 1.36.0. |
| build/openrpc/full.json | Updates OpenRPC version string to 1.36.0. |
| documentation/en/cli-lotus.md | Updates CLI docs version string to 1.36.0. |
| documentation/en/cli-lotus-worker.md | Updates CLI docs version string to 1.36.0. |
| documentation/en/cli-lotus-miner.md | Updates CLI docs version string to 1.36.0. |
| CHANGELOG.md | Adds v1.36.0 release notes and related changelog content. |
| build/buildconstants/params_mainnet.go | Sets mainnet Fire Horse upgrade epoch/height. |
Files not reviewed (1)
- node/config/doc_gen.go: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| // Defensive fallback: every queued sector must receive a result, otherwise | ||
| // AddCommit callers can remain blocked forever. | ||
| processed := map[abi.SectorNumber]struct{}{} | ||
| for _, r := range res { | ||
| for _, sn := range r.Sectors { | ||
| processed[sn] = struct{}{} | ||
| } | ||
| } | ||
|
|
||
| if len(processed) != total { | ||
| missingRes := sealiface.CommitBatchRes{ | ||
| FailedSectors: map[abi.SectorNumber]string{}, | ||
| } | ||
|
|
||
| for _, sn := range sectors { | ||
| if _, ok := processed[sn]; ok { | ||
| continue | ||
| } | ||
|
|
||
| missingRes.Sectors = append(missingRes.Sectors, sn) | ||
| missingRes.FailedSectors[sn] = "commit batcher dropped sector from processing" | ||
| } | ||
|
|
||
| if len(missingRes.Sectors) > 0 { | ||
| res = append(res, missingRes) | ||
| } |
| if precomitInfo.Info.Expiration < precomitInfo.PreCommitEpoch+alloc.TermMin { | ||
| log.Warnf("sector expiration %d is before than allocation TermMin %d for piece %s", precomitInfo.Info.Expiration, precomitInfo.PreCommitEpoch+alloc.TermMin, p.CID.String()) | ||
| } | ||
| if precomitInfo.Info.Expiration > ts.Height()+alloc.TermMax { | ||
| return xerrors.Errorf("sector expiration %d is later than allocation TermMax %d for piece %s", precomitInfo.Info.Expiration, ts.Height()+alloc.TermMax, p.CID.String()) | ||
| if precomitInfo.Info.Expiration > precomitInfo.PreCommitEpoch+alloc.TermMax { | ||
| log.Warnf("sector expiration %d is later than allocation TermMax %d for piece %s", precomitInfo.Info.Expiration, precomitInfo.PreCommitEpoch+alloc.TermMax, p.CID.String()) |
There was a problem hiding this comment.
ts.Height() + alloc.TermMax → startEpoch + alloc.TermMax, to be consistent with getClaimTerms, using the deal's StartEpoch instead of the current chain height for calculation
| maxAllowed := ts.Height() + alloc.TermMax | ||
| if maxAllowed < endEpoch { | ||
| log.Warnf("deal end epoch %d is after allocation term maximum %d, clamping to maximum", endEpoch, maxAllowed) | ||
| endEpoch = maxAllowed | ||
| } |
| if string(row.tipsetKeyCid) != lastTsKeyCid.KeyString() { | ||
| lastTsKeyCid, err = cid.Cast(row.tipsetKeyCid) | ||
| if err != nil { | ||
| return nil, xerrors.Errorf("parse tipsetkey cid: %w", err) | ||
| } |
| if string(row.messageCid) != lastMsgCid.KeyString() { | ||
| lastMsgCid, err = cid.Cast(row.messageCid) | ||
| if err != nil { | ||
| return nil, xerrors.Errorf("parse message cid: %w", err) | ||
| } |
|
|
||
| // ???? | ||
| var UpgradeFireHorseHeight = abi.ChainEpoch(9999999999) | ||
| // 2026-05-27T:14:00:00Z |
| func (e *ethEvents) GetEthLogsForBlockAndTransaction(ctx context.Context, blockHash *ethtypes.EthHash, msgCid cid.Cid) ([]ethtypes.EthLog, error) { | ||
| if e.eventFilterManager == nil { | ||
| return nil, api.ErrNotSupported | ||
| } | ||
| if e.chainIndexer == nil { | ||
| return nil, ErrChainIndexerDisabled |
|
@marco-storswift can you get this rebased on current master please, you have a bunch of conflicts that make it really noisy for review. Check out the copilot suggestions too. |
Copy that |
|
@marco-storswift are you able to rebase this soon? Then we can probably get this shipped in the upcoming Lotus v1.36.1 release |
- Track consecutive non-retryable failures in CommitBatcher and evict sectors after 5 failures to prevent infinite retry loops - Add defensive fallback to ensure every queued sector receives a result, preventing AddCommit callers from blocking indefinitely - Use PreCommitEpoch instead of live chain height for allocation TermMin/TermMax checks to make validation deterministic - Validate allocation term max meets minimum sector lifetime requirements in getClaimTerms to reject unsuitable allocations early - Validate piece CID/size against on-chain allocation and clamp deal end epoch to TermMax in BasicPreCommitPolicy.Expiration Fixes #13631 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
7c03260 to
b0fbbeb
Compare
- Guard defensive fallback with err==nil so transient processBatchV2 errors don't permanently fail and evict all queued sectors (sectors retry instead) - Return errors from allocationCheck on TermMin/TermMax violations instead of logging warnings; invalid sectors are now rejected before ProveCommitSectors3 - Use deal StartEpoch instead of ts.Height() for TermMax clamp in BasicPreCommitPolicy.Expiration for consistency with getClaimTerms Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
lint failures in CI, needs a |
Summary
Fixes several bugs in the Lotus sealing pipeline that cause sectors to get stuck in perpetual retry loops, block callers indefinitely, and perform non-deterministic allocation validation.
Closes #13631
Changes
1. Sectors stuck in infinite retry loop (
commit_batch.go)Track consecutive non-retryable failures per sector in the CommitBatcher. Sectors that fail 5 times consecutively are evicted from the batch queue with a failure result delivered to all waiters, preventing infinite retry loops on sectors that will never succeed.
2. AddCommit callers blocked forever (
commit_batch.go)Added a defensive fallback after
processBatchV2that ensures every sector in the batch receives a result. If a sector is missing from the response, an explicit failure result is generated. Also fixedprocessBatchV2to return thereswheninfosis empty but there are failed sectors (previously returned nil).3. Non-deterministic allocation TermMin/TermMax check (
commit_batch.go)Changed
allocationCheckto usePreCommitEpochas the fixed reference point instead of livets.Height(). This makes the TermMin/TermMax validation deterministic regardless of when the batch is processed.4. Missing allocation term validation (
input.go)Added
minAllocationTermForSealProofto compute the minimum allocation term required based on seal proof parameters.getClaimTermsnow rejects allocations whoseTermMaxis below this minimum, catching unsuitable allocations early.5. Missing allocation validation in precommit policy (
precommit_policy.go)Added allocation lookup in
BasicPreCommitPolicy.Expirationto validate piece CID/size against the on-chain allocation and clamp the deal end epoch toTermMaxwhen it exceeds the allowed maximum.Affected Files
storage/pipeline/commit_batch.go— failure tracking, eviction, defensive fallback, deterministic allocation checksstorage/pipeline/input.go— allocation term validationstorage/pipeline/precommit_policy.go— allocation validation and epoch clampingTest plan
🤖 Generated with Claude Code