fix(content): chain manager and block producer updates

[yiannisbot]

This PR updates the chain manager and block producer sections, which were mostly in a good state. @schomatis, as discussed, please edit directly to update the text where needed.

Two notes:

• I would probably exclude completely current section 2.4.1.3 "Chain" as I don't see what it adds. @schomatis let me know what you think.
• In section 2.4.1.5.2 on Block Rewards, I have excluded all text and have instead pointed to the correct sections.

@irenegia I would welcome a quick opinion on whether the Election PoSt text in section 2.4.1.5 Block Producer is accurate or needs update.

[schomatis]

I'm not finding much to review here apart from some minor prose edits (I may be missing something though).

I would probably exclude completely current section 2.4.1.3 "Chain" as I don't see what it adds. @schomatis let me know what you think.

Sure.

In section 2.4.1.5.2 on Block Rewards, I have excluded all text and have instead pointed to the correct sections.

I can't find that section index.

[yiannisbot]

I'm not finding much to review here apart from some minor prose edits (I may be missing something though).

No, you're not missing anything. As discussed last week, these sections are mostly up to date as they are. The purpose of this PR is to:

1. I do the minor updates you see,
2. you edit the PR to add any further important details that are currently missing.

If you think there is little to add wrt point 2) then we can merge and mark the sections as "reliable".

[irenegia]

@yiannisbot

@irenegia I would welcome a quick opinion on whether the Election PoSt text in section 2.4.1.5 Block Producer is accurate or needs update

I can not find any text about Election PoSt in the "Block Producer" section..

[yiannisbot]

@irenegia it's this text in line 20:

"Producing a block for epoch H requires computing a tipset for epoch H-1 (or possibly a prior epoch, if no blocks were received for that epoch). Using the state produced by this tipset, a miner can scratch winning ElectionPoSt ticket(s). Armed with the requisite ElectionPoStOutput, as well as a new randomness ticket generated in this epoch, a miner can produce a new block."

[irenegia]

@irenegia it's this text in line 20:

"Producing a block for epoch H requires computing a tipset for epoch H-1 (or possibly a prior epoch, if no blocks were received for that epoch). Using the state produced by this tipset, a miner can scratch winning ElectionPoSt ticket(s). Armed with the requisite ElectionPoStOutput, as well as a new randomness ticket generated in this epoch, a miner can produce a new block."

I see, this part should be removed.
Instead we should write something like:

"Producing a block for epoch H requires waiting for the beacon entry for that epoch and using it to run GenerateElectionProof. If WinCount ≥ 1 (the miner is elected), the same beacon entry is used to run WinningPoSt. Armed by the ElectionProof ticket (output of GenerateElectionProof) and the WinningPoSt proof, the minere can produce an new block."

[schomatis]

So, after another pass I think we should drop the entire Chain Manager, it doesn't add much value from the spec perspective. At most we can keep the Go API.

[schomatis]

Suggested change

Once a block has been received and passes syntactic and semantic validation (as explained above) it must be added to the local datastore, regardless of whether it is understood as the best tip at this point. Future blocks from other miners may be mined on top of it and in that case we will want to have it around to avoid refetching.

This entire line sounds like implementation details. We can rework it if we think it's sound advise and worth staying here but the term must should be used with care in a spec I think.

[yiannisbot]

I think the following is an important detail:

regardless of whether it is understood as the best tip at this point. Future blocks from other miners may be mined on top of it and in that case we will want to have it around to avoid refetching.

If we want to rephrase from must to should we can do that instead of deleting.

[schomatis]

I'm fine with just changing the word if you think it's useful. It still seems to me too much of an implementation detail, should we really store those tipsets preemptively? Is it really worth the disk space to save some bandwidth? Maybe yes, maybe no, but you see we are already talking about details that don't reflect how the protocol works. They sound to me more of an engineering issue than anything else.

[yiannisbot]

Yes, it is a detail, I agree. I also think it will not hurt as a recommendation, which is what I have added it as. It will be interesting to see what is cheaper to do in terms of resources. I have a feeling that as the network grows, it will be better if everyone kept a copy temporarily, instead of having to fetch thousands of copies of the same thing to thousands of nodes.

[yiannisbot]

So, after another pass I think we should drop the entire Chain Manager, it doesn't add much value from the spec perspective. At most we can keep the Go API.

@schomatis do you suggest deleting the whole of (current) section 2.4.1.4 on Chain Manager, or only the introductory paragraphs before 2.4.1.4.1?

[schomatis]

I'll leave that up to you. When I started the review I marked those two for delete for sure, and after looking at what was left I didn't see much more remaining so I (confusingly) summed up my review as 'just remove it all', but I'm fine with anything in between those two extremes.

[yiannisbot]

I have done a restructure of the whole subsection as it was very confusing with lots of redundant text. It now reads more clearly. I have excluded most of the parts you suggested and added the rest as "Notes & Recommendations".