filecoin-project / consensus

Filecoin consensus work
Other
42 stars 5 forks source link

Pin down precise slashing rules for EC upgrade #45

Closed ZenGround0 closed 5 years ago

ZenGround0 commented 5 years ago

After spec read-through today it appears that the latest EC construction could benefit from a new type of slashing rule that prevents miners from mining multiple chains when one of those chains is made of null blocks.

Output is a spec draft PR outlining these rules.

sternhenri commented 5 years ago

@ZenGround0 I'm trying to pin down the specific attack you mean, which I now realize I lost. I had assumed it was related to the fact that this new construction (with a single ElectionProof as opposed to an array of proofs) has the miner mine off of every ticket (including null ones) rather than repeatedly only off of the valid ones.

In order to validate a block we must make sure that the valid ElectionProof is derived from the ticket at blockHeight - K + |Tickets| - 1. However we still account for BlockHeight the same way, so I am not sure I understand what attack you mean.

If a miner mines a valid block on top of the block at height N-1. and also mines a valid block on top of a null block at height N-2, I believe current slashing rules would catch her...

What do you have in mind more precisely?

ZenGround0 commented 5 years ago

A restatement of the issue is that we are now losing out on slashing some cases where it is 100% certain that a miner mined on the same ticket. The reason for this is that before we were assuming that all null block tickets would be available for verification and we would throw out as invalid any chain with a null block containing a winning ticket.

It takes a little bit of work to demonstrate cases where the same "block height" rule doesn't catch these. So here's an attempt to clarify. Please don't stop asking for clarification until this is crystal clear.

Here's an ascii graph. Election lookback parameter is 6. x means a ticket created by an attacker o means a ticket made by someone else. | delineates that a block is mined with all tickets left of the symbol and right of the previous |. Multiple tickets can be between the lines if null blocks are being mined. -, indicates ticket links.

The base chain (block-height = B, ticket-height = T ): | - o | - x | - o - o | - o | - o |

Now imagine two forks are made on the head (right side) of the base chain. Fork 1 (block-height = B + 2, ticket-height = T + 2): | - o | - x |

Fork 2 (block-height = B + 1, ticket-height = T + 3): |- x - x - x |

Under the unmodified slashing rules the attacker will not be slashed because the block height of Fork 2 is not equal to the block height of Fork 1 whether you measure ticket height or block height. Furthermore the system can know for absolute certain that the attacker is mining on a fork because the second ticket of the base chain is being scratched off in both forks at the same time. The attacker has no deniability that they weren't aware of both forks when releasing the blocks

This is not an issue in the previous construction with lookback parameter because the second null block in fork 2 would not pass validation as the election proof would show that it was a winner.

ZenGround0 commented 5 years ago

One way to add in a slashing rule that catches this is to generalize the "block height" rule. One straightforward way of defining this is to define the "ticket set" of a block as the tickets that were scratched off to elect the block (including the losing "null" tickets). Then the slashing requirement simply becomes that a miner can be slashed if any two blocks exist that are both signed by that miner and have a non-overlapping ticket set.

In the example above the ticket set of the attackers block on fork 1 is the second ticket of the base chain and the ticket set of the attacker block on fork 2 is the first three tickets of the base chain. The intersection of the two sets is the second ticket. As this is non-empty the attacker can be slashed.

Note that this does not prevent the attacker from getting away with a little more grinding than it could in the previous construction. Specifically the attacker could still release a block with secretly "winning" null tickets as long as they did not release another chain with an overlapping ticket set. For example in the previous construction Fork 2 would be invalid even if the attacker did not submit the last block of Fork 1. However in the proposed construction with the above modified slashing rule Fork 2 would be a valid block that the attacker could submit to the network.

My opinion is that the penalty that the attacker pays by waiting the extra block delay to grind probably makes this form of grinding expensive enough that it is not a problem and worth the throughput gains that we get by not containing and checking every null election proof in the header.

ZenGround0 commented 5 years ago

Realization -- the ticket set idea is unnecessarily complicated and doesn't address the case where the forks diverge past the leader election lookback parameter which the previous construction does.

Proposal -- each block is defined to have a "ticket height interval" which is the smallest interval that includes the ticket height of each ticket in the block (including null tickets). The slashing condition is that a miner signed two blocks with intersecting ticket-height-intervals.

sternhenri commented 5 years ago

So a few things here:

I now understand that the issue you mean is that we used to have all the VRFs of a miner on null blocks, we now no longer have them and so cannot check if the miner is willingly ignoring a winning block. My understanding is that we didn't previously check null blocks for winning but unclaimed tickets. @whyrusleeping and I never thought that was an issue since it seemed irrational for a miner not to claim a winning ticket (as in fork 2) given added block delay (as you mention later).

However I do agree that a continuation like this: Fork 1 (block-height = B + 2, ticket-height = T + 2): | - x | - x |

Fork 2 (block-height = B + 1, ticket-height = T + 3): |- x - x - x |

where only a single miner has mined both is problematic indeed.

I disagree with this claim. I believe we always depended on delay for punishment.

I think what you're getting at with the 'ticket height interval' is that a miner shouldn't be able to mine multiple blocks off of the same ticket set. I agree with that (so long as no one else has also mined in the interim), but I think that gets caught appropriately by the 'no two blocks at same height constraint'.

Conclusion: I'm not sure I agree there is an issue here. Or at least, I don't understand it.

Maybe this requires sync chat :/

sternhenri commented 5 years ago

In fact I posit that mining multiple blocks at same height should not be slashed (could be due to network delay), but mining multiple blocks at same round should.

With that said I have a fix for my priori issue: Fork 1 (block-height = B + 2, ticket-height = T + 2): | - x | - x |

Fork 2 (block-height = B + 1, ticket-height = T + 3): |- x - x - x |

where it can be proven two blocks at same round were mined, we can use your idea @ZenGround0 which is to show that the contents of the Tickets arrays here will overlap (wouldn't be the case if another miner mines the block in the interim). This captures back what we had with ElectionProofs since just as we could check old electionproofs for a valid one, any valid one used means a new block at a different height, but with the particularity that no new miner will have mined between this miner's tickets.

so the new slashing rule should be:

(maybe that's what you meant by ticket height interval, I'm not sure I got it)

ZenGround0 commented 5 years ago

Looks like we are getting closer. In particular the ticket interval proposal is close to the statement "If any miner is found to have mined two blocks at different heights but whose tickets arrays intersect, slash". I am proposing "If any miner is found to have mined two blocks whose ticket arrays have heights that intersect, slash"

Some intermediate points to sync about

Given that there is another miner mining on fork 1, there could be a legit reason for changing chains that we wouldn't want to prevent here.

The | - o | - x | chain is slashable. It is really important we are on the same page about this. There is no legit reason for a single miner to generate two tickets of the same height whether or not the miner is working off of chains built by different miners. I understand that we need to allow legit reasons for changing chains and that this restricts slashing rules. We cannot disallow miners from "catching up" by switching to newer chains and there is always uncertainty about which chain was newer from the miner's perspective as long as the ticket heights are different. We can only slash if the mining rounds are the same. I believe making the proposed change will catch more instances where the network can prove mining at the same ticket height and not interfere with restrictions that protect miners from switching chains.

I disagree with this claim. I believe we always depended on delay for punishment.

Looking back at the spec looks like validating that null blocks fail was never written down. This means that this has always been a problem but it doesn't change that these changes are needed.

I think that gets caught appropriately by the 'no two blocks at same height constraint'

I think we both disagree with this now because you say in the next comment that sometimes we can't even slash blocks of the same height.

In fact I posit that mining multiple blocks at same height should not be slashed (could be due to network delay), but mining multiple blocks at same round should.

I think "mining multiple blocks at the same round" is the core of slashable conditions and we agree on this.

sternhenri commented 5 years ago

yes agreed,

I believe the height interval and common tickets idea are functionally the exact same solution. If so, we should do the common tickets: it's a bit easier to grok I think...

That said I disagree that it catches this use-case:

| - o | - x |

or that it should...

Likewise, the null block validation omission was one I discussed with @whyrusleeping and was intentional. I'm still not sure why it is rational to grind this way.

Let's wrap up quickly in person? I think we can converge quickly.

sternhenri commented 5 years ago

Converged on the following:

THis is good because it imitates old slashing rule. It also carries with it the issues that has: if a miner has published a block and then hears about a heavier chain in the past, they must wait to publish a new block on top of that one (lest they get slashed). This promotes selfish mining (releasing late) but timing, entropy and weight mitigate this.

@ZenGround0 will make a PR relating this new slashing rule and link it/close this issue