Skip to content

Add support + mission for new leader election algorithm - #205

Merged
marta-lokhova merged 2 commits into
stellar:mainfrom
bboston7:scp-nomination
Apr 21, 2025
Merged

marta-lokhova merged 2 commits into
stellar:mainfrom
bboston7:scp-nomination

Conversation

@bboston7

Copy link
Copy Markdown
Contributor

Part of stellar/stellar-core#4387

This change adds support for testing the new leader election algorithm by generating configs that make use of auto quorum set configuration where possible. In doing so, it switches many tests over to auto quorum set configuration.

This change also adds a new set of missions that blend nodes running the old and new leader election algorithm to assess the impact of nodes using these different algorithms simultaneously. The good news is that in running the test I did not see much increase in timeouts with either majority (no ledger had more than 1 timeout).

@marta-lokhova marta-lokhova left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

thanks! looks like there are some merge conflicts, but otherwise looks good. We can also add the two new missions to the CI once this PR lands.

Part of stellar/stellar-core#4387

This change adds support for testing the new leader election algorithm
by generating configs that make use of auto quorum set configuration
where possible. In doing so, it switches many tests over to auto quorum
set configuration.

This change also adds a new set of missions that blend nodes running the
old and new leader election algorithm to assess the impact of nodes using
these different algorithms simultaneously. The good news is that in
running the test I did not see much increase in timeouts with either
majority (no ledger had more than 1 timeout).
@marta-lokhova
marta-lokhova merged commit 260fa66 into stellar:main Apr 21, 2025
unsafeQuorum = true
awaitSync = true
validate = true
homeDomain = Some "stellar.org"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should this be None so we don't accidentally create a single organization by default?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I went down a bit of a rabbit hole here. Even before this change, all of the "simple" quorum types result in a flat quorum. That means that any missions that use AllPeersQuorum, CoreSetQuorum, CoreSetQuorumList, or CoreSetQuorumListWithThreshold get flattened into a single qset. I would have guessed that the *List* QuorumSetSpecs to end up as hierarchical qsets, but I also understand the logic for flattening them based on how we use the list-based QuorumSetSpecs in practice.

At the same time, setting a default homeDomain makes it easier to misconfigure AutoQuorums. I think what we want is:

  1. Default homeDomain to None as you suggest, and
  2. When using AllPeersQuorum, CoreSetQuorum, or CoreSetQuorumList, if the homeDomain is None, create a home domain to use for the resulting flat quorum. This matches the behavior before this change, but also allows us to use the new nomination algorithm for most missions.

What do you think?

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