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

ChainUpdates: model bad peer behavior #492

Draft
wants to merge 15 commits into
base: main
Choose a base branch
from
Draft

Conversation

amesgen
Copy link
Member

@amesgen amesgen commented Nov 6, 2023

Closes CAD-4314

Previously, we did not generate any ChainUpdate behavior modeling a malicious or ill-configured peer. This meant that existing tests could only check that our disconnection routines are not overly restrictive, but not that they are strict enough.

This PR adds this possibility, namely by randomly toggling some blocks to be invalid. As the result is not necessarily modeling bad behavior, we add a function to "classify" a sequence of chain updates after the fact.

Concerning the BlockFetch client test: as certain invalid behavior is only caught by the ChainSync client, we now also run it in the background.


Historical remark: This PR has been migrated from IntersectMBO/ouroboros-network#3856.

The BFT protocol uses block numbers as its `SelectView`, so the induced order on
blocks is not total. When we generate a rollback, we usually end up with the
same chain length, which is then not improving upon the previous chain.
Right now, the peer disconnection logic relies on the interplay of the
BlockFetch and the ChainSync client to catch invalid behavior, so this commit
adds an actual ChainSync client for every peer.

Concretely, consider the case when a peer wants to extend an invalid block. In
that case, the ChainSync client will disconnect, either when the extending
header is received, or via the invalid block rejector in a background thread.

In contrast, when we simply add a block (together with a punishment) to the
ChainDB, this punishment will *not* be enacted, as the block is not validated as
it is not reachable via any (valid) block in the VolDB.
The `serverUpdates` only depend on the `securityParam`, and the `clientUpdates`
additionally depend on `serverUpdates` due to `removeLateClientUpdates`.
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.

1 participant