Skip to content

Conversation

@Niols
Copy link
Contributor

@Niols Niols commented Jun 19, 2024

This attack is only triggered if one replaces FetchModeDeadline by FetchModeBulkSync and reduces the bfcMaxConcurrency* constant to be lower or equal to the number of adversaries. (Typically, in the upcoming BlockFetch change, bfcMaxConcurrencyBulkSync will be 1.)

Niols and others added 19 commits May 30, 2024 19:06
This also involves some fairly important changes to the leaky bucket and
to the leaky bucket API. These changes make the leaky bucket
significantly more robust.

Co-authored-by: Alexander Esgen <[email protected]>
Also:

* Simplify chainDiffs computation in chainSelectionForBlock
* Revert "Don't allow to extend the selection by more than k blocks"
  This reverts commit c8468c2.
* Revert "Retrigger GDD when the selection changes"
  This reverts commit f990d41.
* Rewrite `Peers` to accept arbitrary number of peers
* Actually generate honest peers in CSJ happy path
* Support a field for extra honest peers in `GenesisTest`
* Allow `uniformPoints` to generate schedules with multiple honest peers
* Adapt CSJ test to use native multiple honest peers generation
* Share partial accessor functions used in tests
* Use partial accessor to retrieve the only honest peer
Also:
* Remove ill-defined LoELimit
* Remove unused updateLoEFragStall
* Rename runGdd to runGDDGovernor
* Print GDD in traces instead of GDG
* Remove the UpdateLoEFrag callback
* Reorder functions in the Governor module
…' and disable timeouts as they are supposed to be
Also:
* LoEEnabled: make payload strict to avoid NoThunks failures
* LoE: allow to dynamically en-/disable
* Depending on the state of the GSM, we want to either en- or disable the LoE.
* Refactor `runGdd` to use `Watcher`, share trigger logic
  Previously, we would duplicate the logic for when to trigger the GDD between the
  NodeKernel and the peer simulator.
* Add GDD tracing
  The `Show` instances are probably way too large ATM, but there are currently
  unused, so that isn't a pressing concern.
* LoP rate: fix typo
  500/s = 1/2ms, not 2/ms 🤦
Previously, it wasn't possible to eg run *just* CSJ.
Modify the honest shrinking function by no longer speeding up the other
schedules when an honest tick is deleted. This simplifies a lot of code
in the `Shrinking` module, at the cost of no longer ensuring that shrunk
schedules preserve the overall order of events.

Additionally, we re-enable shrinking in CSJ tests

Also:
* Extend adversarial schedules when shrinking an honest one
* Document the cases when we don't use shrinking
Instead of a type alias. This could help catching bugs, but, more
importantly, this paves the way to making it a data-type and adding more
fields to it, although a lot of the fixes of this commit would then
crumble immediately.
This commit brings several LoE-related improvements, namely:

- The precondition of `followsLoEFrag` stipulating that the given
  candidate fragment had to intersect with the LoE fragment was
  often violated in the code, because the candidate fragments were
  anchored at the tip of the selection.

- `followsLoEFrag` used to accept or reject fragments depending on
  whether they were LoE-compliant or not. The new verson trims them
  to their longest LoE-compliant prefix. Following this change,
  `followsLoEFrag` has been renamed to `timToLoE`. It does not filter
  candidates out anymore.

- This last change makes `computeLoEMaxExtra` redundant. We get rid of
  it entirely and clean up the functions `maximalCandidates` and
  `extendWithSuccessors` accordingly.
This commit refactors the `PointSchedule` type from a `newtype` of
`Peers (PeerSchedule blk)` to a datatype containing this schedule as
well as an additional field `psMinEndTime`. This field describes a
minimal absolute time that the test must reach. If all ticks are
executed before this time is reached, an extra delay is inserted.

This allows getting rid of all the places in which we added extra ticks
only for the purpose of making the test run longer. At the cost of
complexifying a bit the implementation of point schedules, this makes
the semantics much clearer.

The fact that `PointSchedule` is now a datatype makes it easy to later
add other fields, for instance a field stating the initial tip points,
instead of having to add zero-duration ticks at the beginning to set
those up, or a field describing the origin of the absolute clock,
instead of shifting the tick times.

Co-authored-by: Nicolas “Niols” Jeannerod <[email protected]>
@Niols Niols added the Genesis PRs related to Genesis testing and implementation label Jun 19, 2024
@Niols
Copy link
Contributor Author

Niols commented Jun 19, 2024

Note that tests fail, as discussed on Tweag's Slack.

@Niols Niols force-pushed the niols/blockfetch-leashing branch from 7f6759c to b1e649a Compare June 28, 2024 14:24
Base automatically changed from genesis/milestone-13 to main July 11, 2024 19:12
@amesgen
Copy link
Member

amesgen commented Sep 9, 2024

Superseded by #1179

@amesgen amesgen closed this Sep 9, 2024
@amesgen amesgen deleted the niols/blockfetch-leashing branch September 9, 2024 15:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Genesis PRs related to Genesis testing and implementation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants