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

Implement divergent EC chain in simulations #187

Merged
merged 1 commit into from May 13, 2024
Merged

Conversation

masih
Copy link
Member

@masih masih commented May 3, 2024

So far simulations used the same EC chain across all participants after the first instance. This approach was simple but not representative of the real-world where the EC chain may diverge across multiple subsets of participants.

The changes here implement a comprehensive set of APIs that allow a set of EC generation strategies to mix-and-match and replicate complex EC chain evolution through iterations of a simulation. The chain evolution is controllable by instance by actor ID. This allows us to simulate fluctuating EC evolution patterns throughout a simulation. The changes include an initial set of EC generators that cover:

  • Fixed EC for all instances and participants.
  • Random EC per instance per participant.
  • Random EC per instance but uniform across all participants.
  • Common prefix EC per instance with probabilistic length
  • Majority common prefix across a set of participants with probabilistic length

The EC generators can be aggregated and set per number of participants, which enables simulation of a flexible set of scenarios including evolving majority common prefix.

Fixes #115

@masih masih marked this pull request as ready for review May 3, 2024 14:45
@masih masih requested a review from Kubuxu May 3, 2024 14:45
@masih masih enabled auto-merge May 3, 2024 15:57
@masih masih force-pushed the masih/sim-ec-divergence branch from 5a3a566 to 2953820 Compare May 3, 2024 17:34
Copy link
Member

@anorth anorth left a comment

Choose a reason for hiding this comment

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

I skimmed, looking mainly at the tests and how they changed. The main code still needs a proper review.

Looks good except for some confusion around which value is being expected, or how it's fetched, in the multi-instance tests.

sim/host.go Show resolved Hide resolved
test/honest_test.go Outdated Show resolved Hide resolved
sim/options.go Outdated Show resolved Hide resolved
test/multi_instance_test.go Outdated Show resolved Hide resolved
@masih masih disabled auto-merge May 7, 2024 11:48
@masih masih force-pushed the masih/sim-ec-divergence branch from 2953820 to 0c0caaa Compare May 7, 2024 13:06
@masih masih requested a review from anorth May 7, 2024 13:07
test/util_test.go Outdated Show resolved Hide resolved
So far simulations used the same EC chain across all participants after
the first instance. This approach was simple but not representative of
the real-world where the EC chain may diverge across multiple subsets
of participants.

The changes here implement a comprehensive set of APIs that allow a set
of EC generation strategies to mix-and-match and replicate complex EC
chain evolution through iterations of a simulation. The chain evolution
is controllable by instance by actor ID. This allows us to simulate
fluctuating EC evolution patterns throughout a simulation. The changes
include an initial set of EC generators that cover:

* Fixed EC for all instances and participants.
* Random EC per instance per participant.
* Random EC per instance but uniform across all participants.
* Common prefix EC per instance with probabilistic length
* Majority common prefix across a set of participants with probabilistic
  length

The EC generators can be aggregated and set per number of participants,
which enables simulation of a flexible set of scenarios including
evolving majority common prefix.

Fixes #115
@masih masih force-pushed the masih/sim-ec-divergence branch 2 times, most recently from 280f454 to 1519aec Compare May 13, 2024 16:10
Copy link
Collaborator

@Kubuxu Kubuxu left a comment

Choose a reason for hiding this comment

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

SGTM

@Kubuxu Kubuxu added this pull request to the merge queue May 13, 2024
Merged via the queue into main with commit 8f7cfef May 13, 2024
6 checks passed
@Kubuxu Kubuxu deleted the masih/sim-ec-divergence branch May 13, 2024 18:47
@anorth
Copy link
Member

anorth commented May 13, 2024

Thanks for pushing forward to land something.

I think what the suggested code snippet meant to be is requireConsensusAtInstance(t, sm, instanceCount-1, <basechain-of-instance-after-last>.Head())

Yes. basechain-of-instance-after-last is what I meant by expected.

which is an unreasonable expectation in scenarios with EC divergence among participants

But what about scenarios without EC divergence? I'm just trying to preserve the behaviour in those.

The code that landed in multi_instance_test looks fine. If the same thing can be achieved without running the sim into instance one-past-the-end, that will be fine too. But if what I'm saying sounds like nonsense to you, probably best we leave it as-is.

@masih
Copy link
Member Author

masih commented May 14, 2024

If the same thing can be achieved without running the sim into instance one-past-the-end, that will be fine too.

Thanks Alex, I think at the depth current tests assert things it makes no difference.

We might want to revisit this if we decide to simulate failure scenarios, where we expect the simulation to not converge and eventually converge back. The current simulation logic simply errors out in such cases. For now what's there seems reasonable. 🙏

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.

Support divergent EC chain views in simulator
3 participants