FRC0051 Implementation Proposal

[Stebalien]

Added by @arajasek

Implement https://github.com/filecoin-project/FIPs/blob/master/FRCs/frc-0051.md

Technical Breakdown

Development

Preview Give feedback

1. Chainstore: drop all blocks from the same miner in the same tipset
2. Open design question: Decide how to catch equivocations in the miner (see discussion below)
3. Implement equivocation-checking in the miner loop
4. Add metrics to the miner loop to track how long mining takes

Testing

Preview Give feedback

1. Unit test: Chainstore rejects all blocks from the same miner when forming or expanding tipsets
2. Functional test: Confirm that miners still produce blocks
3. Performance test: Confirm that the time to produce a block does not increase by more than 2 seconds with this change

---

created by @Stebalien
FRC0051 proposes rejecting equivocations. I.e., multiple blocks at the same height with the same ticket.

This was previously implemented in the sync engine but reverted in #10777 due to DoS and other issues.

Here, I propose that re-implementing this FRC, but inside the chainstore, re-using all the existing tipset tracking logic. Specifically, when we expand a tipset, we currently drop duplicate blocks from the same miner. Instead, we should drop all blocks at a given height from a miner if more than one block is produced (or drop all blocks if we see multiple blocks with the same ticket? I think these are equivalent).

However, to actually catch equivocations, we need a delay after we receive the last block in a tipset but before we create the block. Otherwise, an equivocating miner could send out multiple blocks to different miners at the last possible moment, right before the end of the propagation delay period.

Unfortunately, it looks like any reasonable solution will require modifying the block miner. But it sould be a simple modification. Basically:

1. As described above, drop blocks from miners that equivocate (inside the chainstore).
2. Inside the miner, after the block propagation delay (10s) take the current tipset.
3. Wait 2s (the equivocation delay).
4. Ask the client for the latest tipset, again.
5. Treat the intersection between these two tipsets as the base tipset for mining.

Most of this logic should center around the waitFunc call in the miner.

Alternatively, we could (and probably should?) insert this sleep right after computing the proof. Effectively, we'd pass a "proposed" base tipset into mineOne, then have mineOne prune the proposed tipset right before mining a block.

In the future, we could even make createBlock interruptable.

[arajasek]

@Stebalien Thanks for the write-up!

Here, I propose that re-implementing this FRC, but inside the chainstore, re-using all the existing tipset tracking logic. Specifically, when we expand a tipset, we currently drop duplicate blocks from the same miner. Instead, we should drop all blocks at a given height from a miner if more than one block is produced (or drop all blocks if we see multiple blocks with the same ticket? I think these are equivalent).

I think this part is fairly uncontroversial, safe, and easy to do. Regardless of the equivocation attack that motivated FRC-0051, let's do it.

Unfortunately, it looks like any reasonable solution will require modifying the block miner. But it sould be a simple modification. Basically:

1. As described above, drop blocks from miners that equivocate (inside the chainstore).
2. Inside the miner, after the block propagation delay (10s) take the current tipset.
3. Wait 2s (the equivocation delay).
4. Ask the client for the latest tipset, again.
5. Treat the intersection between these two tipsets as the base tipset for mining.

Most of this logic should center around the waitFunc call in the miner.

Alternatively, we could (and probably should?) insert this sleep right after computing the proof. Effectively, we'd pass a "proposed" base tipset into mineOne, then have mineOne prune the proposed tipset right before mining a block.

I think what would be great is if this didn't have to be a sleep, but instead a timer that we check before submitting our block.

I'd propose changing MiningBase to include a time at which it was constructed. In the mineOne method, we can then:

1. fully prepare the block (proofs, messages, signature, etc.)
2. ask for a refreshed MiningBase
3. If it's changed, redo message selection & state computation and produce a new block
4. If it hasn't changed, and the difference in production time of our MiningBases is > 2s, Submit the block
5. If it hasn't changed, and the difference is < 2s, sleep and go to step 2

There's some slight concern equivocating then becomes an attack on miners to slow them down, but I think overall this is better than 2 seconds of dead time.

[Stebalien]

I'd propose changing MiningBase to include a time at which it was constructed. In the mineOne method, we can then:

We can do that. Although, unless we have a way to interrupt tipset computation, long block execution times could be a bit of an issue. But that's not currently an issue.

[arajasek]

It might be worth splitting this into 2 issues: one for the chainstore changes in the Lotus node, and one for the changes relevant to the lotus-miner's block production process. Let's discuss this further in sync.

[jennijuju]

Let's discuss this further in sync.

(just a small nit for future that the triaging (trying to not saying the g word lolol) session last friady was a better place for discussing scope instead of sprint planning

[jennijuju]

It might be worth splitting this into 2 issues: one for the chainstore changes in the Lotus node, and one for the changes relevant to the lotus-miner's block production process

Sounds reasonable, would recommend using this as the epic issue that tracks both node/miner issue if we ended up splitting.

[jennijuju]

For testing, should we deploy this to the long-term-lotus team ndoe (we need a name) and confirm it runs for 48hr?

[arajasek]

For testing, should we deploy this to the long-term-lotus team ndoe (we need a name) and confirm it runs for 48hr?

I'm gonna run this on my local node to confirm sync works, but I think the more notable test will be that miners still produce blocks well. Hoping TSE gang can help with that.

[arajasek]

#11104 implements the first half of this (the chainstore changes).

[arajasek]

@jennijuju ^Unlinking that PR because it won't close the issue

[arajasek]

So, even the first half of this proposal (the chainstore changes) are a bit harder than they appear. We need to consider the possibility that the weight of our current head can decrease because we need to evict blocks from our current head due to equivocation. This is a new problem. \

We can deal with this by:

• "Reforming" our current head every time a new tipset is formed
• If the weight does NOT decrease, simply compare weights and pick the heavier one
• If the weight DOES decrease, form the best possible tipset at heights +/-5 of our current head, and pick that
• (tune the number 5)?

[arajasek]

The first half of this change has now landed in #11104.

[arajasek]

The second half of this change has been opened in #11157