-
Notifications
You must be signed in to change notification settings - Fork 40
feat: sector extension manager, new sectors webui #818
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?
Conversation
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.
This PR is being reviewed by Cursor Bugbot
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| err = tx.QueryRow(`SELECT id FROM harmony_task WHERE id = $1 AND name = $2`, existingTaskID, taskName).Scan(&htTaskID) | ||
| if err != nil { | ||
| return false, err | ||
| } |
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.
Bug: Missing ErrNoRows handling breaks singleton task rescheduling
The query to check if a task is still active in harmony_task doesn't handle pgx.ErrNoRows. When a singleton task completes, it gets deleted from harmony_task. On subsequent scheduling attempts, this query returns no rows, causing ErrNoRows to be treated as an error and returned, which prevents the singleton from ever being rescheduled. The code expects htTaskID to be nil when the task isn't found, allowing shouldRun to be evaluated, but ErrNoRows short-circuits this logic.
|
|
||
| estimatedGas, err := e.chain.GasEstimateMessageGas(ctx, msg, nil, types.EmptyTSK) | ||
| if err != nil { | ||
| if strings.Contains(err.Error(), "call ran out of gas") || (err == nil && estimatedGas.GasLimit > MaxExtendMsgGasLimit) { |
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.
Bug: Gas limit check is dead code inside error block
The condition (err == nil && estimatedGas.GasLimit > MaxExtendMsgGasLimit) is placed inside if err != nil, making it unreachable since err is guaranteed to be non-nil in that block. This means the gas limit check will never trigger message splitting proactively. Only the "call ran out of gas" error string check will work. The gas limit check should be evaluated separately outside the error handling block.
| if len(claimIdsBySector) == 0 { | ||
| // No claims for this SP | ||
| continue | ||
| } |
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.
Bug: Stale claim data not cleared when SP loses all claims
When an SP has no claims on chain, claimIdsBySector is empty and the code executes continue at line 314, skipping the entire SP. This prevents the cleanup loop at lines 412-433 from running, which is responsible for clearing min_claim_epoch and max_claim_epoch from sectors that previously had claims. As a result, sectors that lose all their Fil+ claims retain stale claim data in the database, causing the extension manager to incorrectly believe these sectors still have active claims.
Note
Cursor Bugbot is generating a summary for commit fb17708. Configure here.