pdpv0: Document PDP task inventory and dependency tree #998
pdpv0: Document PDP task inventory and dependency tree #998ZenGround0 merged 3 commits intopdpv0from
Conversation
pdpv0: document PDP task inventory and dependency tree
rjan90
added
the
team/fs-wg
ZenGround0
left a comment
ZenGround0
left a comment
There was a problem hiding this comment.
I'm happy to help contribute to this PR to get it merged. The main thing I'd like to see is some changes to the ASCII diagrams for clarity in the Task Dependency Tree section. I think the high level categories are pretty good. But I would probably move from ASCII diagrams to text given that the diagrams are quite dense and they mostly break down into linear sequences of steps.
|
|
||
| # PDP Harmony Tasks | ||
|
|
||
| All PDP-related functionality is implemented as harmony tasks and chain-handler watchers. Tasks implement the `TaskInterface` and are scheduled by the harmony task engine. Chain-handler watchers are registered via `chainsched.AddHandler` and fire on every new chain head. |
There was a problem hiding this comment.
I love seeing the chainsched handlers mentioned. Since we are getting into the mechanis I think we should probably have more exposition on how exactly harmony tasks are scheduled by the task engine -- i.e. IAMBored or constructors creating chain handlers that spawn harmony tasks.
There was a problem hiding this comment.
Ok nice I see that this is called out in the registered task table's Trigger column. But some exposition on the interaction between chain handler and tasks and the broader points of poller vs IAMBored from harmony task would be useful here
| |-----------|--------|------|---------|-------------| | ||
| | `PDPv0_InitPP` | `InitProvingPeriodTask` | `tasks/pdp/task_init_pp.go` | Chain handler | Initializes the first proving period for a dataset by calling `PDPVerifier.nextProvingPeriod()`. Picks up datasets where `init_ready=TRUE` and `prove_at_epoch IS NULL`. | | ||
| | `PDPv0_ProvPeriod` | `NextProvingPeriodTask` | `tasks/pdp/task_next_pp.go` | Chain handler | Schedules subsequent proving periods after each challenge window completes. Fires when `prove_at_epoch + challenge_window <= current_height`. Detects `ProvingPeriodNotInitialized` errors and resets dataset back to init state. | | ||
| | `PDPv0_Prove` | `ProveTask` | `tasks/pdp/task_prove.go` | Chain handler | Generates SHA-256 Merkle tree proofs and submits them via `PDPVerifier.provePossession()`. Scheduled when the `challenge_request_msg_hash` transaction lands on-chain and `prove_at_epoch` is reached. Most resource-intensive task (2GB RAM for memtree). | |
There was a problem hiding this comment.
From my POV we could just drop the description field. The descriptions there are ok but mixed between very high level and fine details which makes reading them a bit tricky at least for me. My thinking would be that the rest of this document should be describing these tasks in enough detail that anyone reading this will already understand what they are doing. A reference table listing trigger type is quite useful though.
| ├── Contract revert ──► exponential backoff | ||
| │ (100 blocks * 2^(failures-1), max 28800 blocks) | ||
| │ after 5 consecutive failures ──► unrecoverable | ||
| │ |
There was a problem hiding this comment.
Perhaps just my opinion but I get the sense that these ASCII diagrams are too crowded to be very useful. I think a bulleted list or text description would probably better suite presenting this information. Alternatively a more simplified ascii diagram with only arrows between tasks could also be a good direction to take this.
| | `PDPv0_PullPiece` | `PDPPullPieceTask` | `tasks/pdp/task_pull_piece.go` | Polling (10s) | Downloads pieces from external URLs, streams data while computing CommP, verifies against expected piece CID, and stores in `parked_pieces` for long-term storage. | | ||
| | `PDPv0_TermFWSS` | `TerminateFWSSTask` | `tasks/pdp/task_terminate_fwss.go` | IAmBored (1 min) | Calls `FWSS.terminateService()` for datasets pending service termination. Retrieves `PdpEndEpoch` from the FWSS contract. | | ||
| | `PDPv0_DelDataSet` | `DeleteDataSetTask` | `tasks/pdp/task_delete_data_set.go` | IAmBored (1 hour) | Calls `PDPVerifier.deleteDataSet()` for datasets where `service_termination_epoch <= current_block` and `deletion_allowed=TRUE`. | | ||
| | `Settle` | `SettleTask` | `tasks/pay/settle_task.go` | IAmBored (12 hours) | Settles the FWSS lockup period for the PDP operator. Calls `filecoinpayment.SettleLockupPeriod()` with the operator and payee addresses. | |
There was a problem hiding this comment.
We need to add PDPv0_Indexing and PDPv0_IPNI
pdpv0: address PR review feedback on PDP spec
I pushed some changes that addressed some of your feedback, but also feel free to push directly to the branch fi there are things you would want to change! |
| **If a rail is fully settled/finalized:** | ||
| 3. SettleWatcher calls `ensureDataSetDeletion()`, which sets `deletion_allowed = TRUE` in `pdp_delete_data_set`. This is then picked up by **PDPv0_DelDataSet** at step 6 above. |
There was a problem hiding this comment.
This piece is redundant, I'll delete
github-project-automation
bot
moved this from 🔎 Awaiting review
to ✔️ Approved by reviewer
in FOC
2 participants
Summary
Closes remaining parts of #904