Piece deletion - #1453
Conversation
FilOzzy
added
the
team/fs-wg
471ab1e to
5aa3aa2
Compare
| ], | ||
| "outputs": [], | ||
| "stateMutability": "nonpayable" | ||
| }, |
There was a problem hiding this comment.
Note: when we rebase on latest main this should go away since @snadrus 's PR included all of this
| const provingPeriodReconcileBatchLimit = 128 | ||
|
|
||
| // NewProvingPeriodWatcher reconciles confirmed proving-period side effects | ||
| // before the prove watcher runs. nextProvingPeriod is what finally applies |
There was a problem hiding this comment.
Lets update this comment
| // watcher runs in an earlier phase; complete here and let the next tipset | ||
| // re-schedule this task. A queue with no drain in flight is handled by the | ||
| // PendingPieceDeletions revert instead. | ||
| draining, err := hasDrainInFlight(ctx, n.db, dataSetId) |
There was a problem hiding this comment.
I don't want to handle it in this ad hoc way when this is really the core scheduling flow of the whole proving mechanism. I want to only schedule nextProvingPeriod tasks when there is no record of us needing to processRemovals.
| AND rm_message_hash = $3 | ||
| AND removed = FALSE | ||
| `, piece.DataSetID, piece.PieceID, piece.TxHash) | ||
| func enqueueDeletionDrainFromProvingWatcher(ctx context.Context, db *harmonydb.DB, dataSetID int64) error { |
There was a problem hiding this comment.
This function wrapper can be dropped and the body can be inlined into the only callsite
| SELECT data_set | ||
| FROM pdpv0_deletion_drain | ||
| WHERE task_id IS NULL | ||
| AND msg_hash IS NULL |
There was a problem hiding this comment.
This seems wrong.
Right now the proving period watcher enqueus these rows immediately if the dataset has any pieces to delete. This task then immediately processes them. This will happen before proving essentially every time. Because of our approach clearing out the challenge epoch on chain this means that every scheduled removal leads to a failed proof.
We need to run this between proving and nextProvingPeriod. IMO the right way to do this is to
- schedule this after the challenge window has passed for the dataset in question
- only schedule nextProvingPeriod task after any rows in this table have been removed
There was a problem hiding this comment.
One thing to note is that every nextPP is going to have to wait for process deletions upon startup because we are migrating every dataset to check for its removals after upgrade.
| // schedulePieceDeletions transaction only records delete intent; the piece | ||
| // should not be marked removed locally while PDPVerifier still reports it as | ||
| // scheduled or live. | ||
| func processPendingPieceDeletes(ctx context.Context, db *harmonydb.DB, ethClient ethchain.EthClient, dataSets []int64) error { |
There was a problem hiding this comment.
I am somewhat concerned about piece deletions that happen on the non-upgraded smart contract before the smart contract is upgraded to 3.5.0 with this version of curio software. It looks like we are not going to track them properly.
3 participants
No description provided.