Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Draft] Slash malevolent validators #294

Merged
merged 29 commits into from
Aug 21, 2023
Merged

Conversation

akildemir
Copy link
Contributor

@akildemir akildemir commented May 15, 2023

This is a draft pr to slash validators with bad behavior. Comments and/or reviews are appreciated.

in tributary, tendermint is used for the consensus. And there are bunch of defined invalid or bad behaviors for validators that participate in the tendermint consensus algorithm. We slash those nodes to disincentive those behaviors.

In this case, once a node catches an invalid/bad behavior from another validator, it either sends an evidence for this bad behavior or a vote to slash the bad node. If an evidence provided and valid, it is solely enough to slash the node by the network participants even if other nodes didn't see this behavior. Once the evidence is on the chain, the bad node is slashed. If a vote is sent, then we wait vote from 2/3 of the nodes to slash the node.

For now following points are completed;

  • create a special tx type to publish bad validator behavior data and/or votes for slashing
  • modify the tributary to hold the both app txs and this integral tx
  • ignore unsigned tendermint tx replays
  • verify that provided evidence was actually valid(sender isn't falsely try to claim someone else with invalid evidence).
  • scan the slash evidence/votes in the chain
  • support unsigned app txs while at it(not really related to this pr but hey! why not)
  • enforce provided, unsigned, signed tx ordering within a block
  • add unit tests for tendermint tx validation
  • update existing mempool/blockchain tests for new tx types

Copy link
Member

@kayabaNerve kayabaNerve left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few comments while I had a second. I don't think GH is showing me the full edits, so I'll have to re-open this to make the rest of my comments...

coordinator/tributary/src/blockchain.rs Outdated Show resolved Hide resolved
coordinator/tributary/src/blockchain.rs Outdated Show resolved Hide resolved
coordinator/tributary/src/mempool.rs Outdated Show resolved Hide resolved
Copy link
Member

@kayabaNerve kayabaNerve left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed about half of this. Looks solid so far.

coordinator/src/tributary/db.rs Show resolved Hide resolved
coordinator/src/tributary/db.rs Outdated Show resolved Hide resolved
coordinator/src/tributary/db.rs Show resolved Hide resolved
coordinator/tributary/src/block.rs Show resolved Hide resolved
coordinator/tributary/src/blockchain.rs Outdated Show resolved Hide resolved
coordinator/tributary/src/mempool.rs Outdated Show resolved Hide resolved
coordinator/tributary/src/mempool.rs Outdated Show resolved Hide resolved
coordinator/tributary/src/mempool.rs Outdated Show resolved Hide resolved
coordinator/tributary/src/mempool.rs Outdated Show resolved Hide resolved
coordinator/tributary/src/mempool.rs Outdated Show resolved Hide resolved
@kayabaNerve kayabaNerve force-pushed the tm-slash branch 3 times, most recently from 04749ca to 4700ac6 Compare August 19, 2023 20:34
akildemir and others added 25 commits August 20, 2023 00:15
Also moves Evidence from Vec<Message> to (Message, Option<Message>). That
should meet all requirements while being a bit safer.
One use of sort was inefficient, sorting unsigned || signed when unsigned was
already properly sorted. Given how the unsigned TXs were given a nonce of 0, an
unstable sort may swap places with an unsigned TX and a signed TX with a nonce
of 0 (leading to a faulty block).

The extra protection added here sorts signed, then concats.
empty_signature led to corrupted local state histories. Unfortunately, the API
is only sane with a signature.

We now use the actual signature, which risks creating a signature over a
malicious message if we have ever have an invariant producing malicious
messages. Prior, we only signed the message after the local machine confirmed
it was okay per the local view of consensus.

This is tolerated/preferred over a corrupt state history since production of
such messages is already an invariant. TODOs are added to make handling of this
theoretical invariant further robust.
There was no competition for resources forcing them to be run sequentially.
@kayabaNerve kayabaNerve merged commit 39ce819 into serai-dex:develop Aug 21, 2023
6 of 7 checks passed
@akildemir akildemir deleted the tm-slash branch August 21, 2023 09:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants