Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

provisioner: prioritize backing votes for disputes #6274

Closed
1 task
ordian opened this issue Nov 12, 2022 · 11 comments · Fixed by #6460 or #6494
Closed
1 task

provisioner: prioritize backing votes for disputes #6274

ordian opened this issue Nov 12, 2022 · 11 comments · Fixed by #6460 or #6494
Assignees
Labels
T5-parachains_protocol This PR/Issue is related to Parachains features and protocol changes.
Milestone

Comments

@ordian
Copy link
Member

ordian commented Nov 12, 2022

See #6249 for the context.

With our slashing design centered around backers for both valid and invalid cases and in the light of #6249, we should always prioritize missing backing votes when selecting votes for a dispute set.

  • Add a test in provisioner that we if there are missing backing votes on-chain, we always select them.
@ordian ordian added the T5-parachains_protocol This PR/Issue is related to Parachains features and protocol changes. label Nov 12, 2022
@ordian ordian moved this to To do in Parachains-core Nov 12, 2022
@ordian ordian added this to the slashing milestone Nov 12, 2022
@eskimor
Copy link
Member

eskimor commented Nov 15, 2022

@BradleyOlson64 This should be fairly simple as well.

@eskimor
Copy link
Member

eskimor commented Nov 15, 2022

Not only prioritize, but make sure there are always imported - even if the chain already knows about some valid votes for those validators.

@BradleyOlson64 BradleyOlson64 self-assigned this Nov 15, 2022
@BradleyOlson64
Copy link
Contributor

The first issue I'm seeing is that we import votes to the provisioner from the disputes subsystem in an order that can't account for backing votes specifically. It only knows about which disputes to prioritize, not which votes to prioritize.

I think a solution might be to set up another call similar to request_votes called request_backing_votes. Not sure how easy it would be to support such a call from the dispute subsystem side, but without it I don't see how we could guarantee all unknown backing votes get into the dispute set.

@ordian
Copy link
Member Author

ordian commented Nov 26, 2022

Heads up: in #6249 I implemented the following logic

// We allow backing statements to be imported after
// explicit "for" vote, but not the other way around.

This makes it easier to implement slashing with our currrent design and should be fine considering

/// Backers should not participate in explicit voting so this only possible on malicious backers.

That means if we select a backing vote (if it's not present on chain already), we should not select an explicit "Valid" vote in the same set from the same validator.

It only knows about which disputes to prioritize, not which votes to prioritize.

My understanding is vote selection in provisioner (in the new version) happens in

@ordian
Copy link
Member Author

ordian commented Jan 2, 2023

The issue was closed, but I don't see it being actually fixed or am I missing something (and the test is missing)?
If an attacker places an explicit "for" and "against" votes on chain (or just an explicit "for" vote), wouldn't its backing vote be filtered by the next honest block author?
Here

if in_validators_for && in_validators_against {
// The validator has double voted and runtime knows about this. Ignore this vote.
return false
}

!in_validators_for && !in_validators_against

@ordian ordian reopened this Jan 2, 2023
@ordian
Copy link
Member Author

ordian commented Jan 2, 2023

Another way to solve this problem is to ensure backers never participate in disputes in the first place as they shouldn't. But that requires the runtime being able to tell who's a backer easily, which might be harder to achieve.

@ordian
Copy link
Member Author

ordian commented Jan 2, 2023

As discussed on the call, we can get away with this problem by not allowing importing disputes votes without backing threshold backing votes. We already check it's non-empty in #6249, but we can also check for a threshold.

@ordian ordian closed this as completed Jan 2, 2023
@ordian
Copy link
Member Author

ordian commented Jan 2, 2023

We already check it's non-empty in #6249, but we can also check for a threshold.

Actually, checking for a minimum_backing_votes is a bit tricky. We'd need to know the validator group size, which is not readily available in the disputes context.

It would still be nice for the provisioner to include the remaining backing votes even if explicit "for" votes has been placed on chain, so we can slash as many backers as possible. Provisioner has the information about kinds of valid votes.

@eskimor
Copy link
Member

eskimor commented Jan 2, 2023

Yes, let's modify the filtering in the provisioner to never filter backing votes, even if the chain knows about them.

@eskimor
Copy link
Member

eskimor commented Jan 2, 2023

@BradleyOlson64 We do indeed filter votes and not only full disputes here. We need to modify this to never filter backing votes, even if the chain knows about valid votes for that validator.

@eskimor eskimor reopened this Jan 2, 2023
@BradleyOlson64
Copy link
Contributor

This should do the trick #6494

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
T5-parachains_protocol This PR/Issue is related to Parachains features and protocol changes.
Projects
No open projects
3 participants