fix(content): FIL VM System Actors Update #1061
Conversation
There was a problem hiding this comment.
I think it is a really bad idea to copy actor code into the spec. The specs-actors repo is the spec. These code snippets will rot and cause confusion.
- Just link to the code in the specs-actors repo
- Improve prose by adding code-comments to the specs-actors repo
A very high level description of the role of each actor in the system would be appropriate in this repo, but I think anything more will just add confusion and maintenance burden.
| The `CronActor` is responsible for sending messages to other registered actors at the end of every epoch. It interfaces with the Storage Power Actor on `OnEpochTickEnd` to check: | ||
| - `CronEventPreCommitExpiry`: if the miner has submitted their `ProveCommit` proofs, | ||
| The `CronActor` is responsible for scheduling jobs for several operations that need to take place between actors. This is primarily realised in the form of sending messages to other registered actors at the end of every epoch. It interfaces with the Storage Power Actor on `OnEpochTickEnd` to check: | ||
| - `CronEventPreCommitExpiry`: if the miner has submitted their `ProveCommit` proofs (given enough time for the miner to complete the proofs of replication and finalize the commit phase). If the miner has completed their commit phase on time, they can claim the `PreCommiDeposit` back. If not, the `PreCommitDeposit` is lost and no power is added for this miner to the power table. |
There was a problem hiding this comment.
These events are all really internal details of the power and miner actor. The cron actor itself has its own registry of calls, and it contains two items: a call to the power and a call to the market actor.
Note that names of things like methods and cron event tags are liable to change, which will lead to confusion here (the names aren't part of the "spec").
There was a problem hiding this comment.
These events are all really internal details of the power and miner actor.
I included those to provide some links between the different sections and description of actors. I thought it's going to be useful for the reader. But I'm fine removing, which I have done in the latest commit.
The cron actor itself has its own registry of calls, and it contains two items: a call to the power and a call to the market actor.
The latest description is more general to say what the CronActor could do. I think this is what we want from the spec? Should I just mention these two?
Note that names of things like methods and cron event tags are liable to change, which will lead to confusion here (the names aren't part of the "spec").
Agreed, but I think these are descriptive-enough, so even if the actual names in the implementation change, this still provides the concept. Do you want me to come up with more generic ones?
There was a problem hiding this comment.
It depends what you want to mean by "stable". I would not say "will not change before mainnet", that is way too strong.
These bits are all ~80% right. We've done the 20% work for the 80% benefit. Getting them complete and correct 100% will be much more effort, especially from the implementers who know all the gnarly details.
Co-authored-by: Alex North <445306+anorth@users.noreply.github.com>
|
I've addressed all of @anorth's comments. This is ready to merge and is labelled as "reliable". |
hugomrdias
changed the title
* master: (27 commits) fix(content): filecoin VM interface update (#1056) fix(content): chainsync section update (#1123) fix(content): libraries section update (#1115) fix: show hugo errors on build docs: tidy up the readme, add toc. (#1141) fix: reword stacked drg missing link text" fix: fix broken refs and change warns to errors (#1139) fix: import meter css (#1140) feat: build toc and dashboard in hugo (#1122) feat: show wip and reliable in progress bar (#1137) content: fix typo plus remove definition of weight (#1138) fix: css for label style and dashboard link (#1136) feat: add symbol embeds and listing (#1126) v1.0.1 feat(tooling): release scripts v1.0.0 chore: update readme and package.json fix: update dns link fix: fix broken links fix(content): FIL VM System Actors Update (#1061) ...
This PR is updating the description of the system actors that interact with the VM (init, cron, account, reward).
There is more to the reward actor than is said in this and the previous version of the spec, so I would welcome opinions on whether we should add more, e.g., related to the calculation of the reward, the penalties and so on. On the other hand, some of this is text for the cryptoecon part, so an option is to include it there.