-
Notifications
You must be signed in to change notification settings - Fork 33
bor: fall back to eligible producers when veblop candidate set is empty #618
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
Open
pratikspatil024
wants to merge
3
commits into
develop
Choose a base branch
from
ppatil/veblop-producer-selection-fallback
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,217 @@ | ||
| package app | ||
|
|
||
| import ( | ||
| "strings" | ||
| "testing" | ||
|
|
||
| abci "github.com/cometbft/cometbft/abci/types" | ||
| cmtsecp256k1 "github.com/cometbft/cometbft/crypto/secp256k1" | ||
| cmtproto "github.com/cometbft/cometbft/proto/tendermint/types" | ||
| sdk "github.com/cosmos/cosmos-sdk/types" | ||
| "github.com/ethereum/go-ethereum/common" | ||
| "github.com/stretchr/testify/require" | ||
|
|
||
| "github.com/0xPolygon/heimdall-v2/helper" | ||
| "github.com/0xPolygon/heimdall-v2/sidetxs" | ||
| borKeeper "github.com/0xPolygon/heimdall-v2/x/bor/keeper" | ||
| borTypes "github.com/0xPolygon/heimdall-v2/x/bor/types" | ||
| milestoneTypes "github.com/0xPolygon/heimdall-v2/x/milestone/types" | ||
| stakeTypes "github.com/0xPolygon/heimdall-v2/x/stake/types" | ||
| ) | ||
|
|
||
| // singletonScenario holds the staged degenerate VEBLOP state: the elected producer set | ||
| // has collapsed to the single current producer, and a >2/3 milestone is finalized by the | ||
| // other validators while the current producer withholds its milestone vote (so it is | ||
| // excluded from the supporting set). Future-span creation in PreBlocker then asks | ||
| // SelectNextSpanProducer for a replacement and the candidate set (singleton minus the | ||
| // current producer) is empty. | ||
| type singletonScenario struct { | ||
| app *HeimdallApp | ||
| ctx sdk.Context | ||
| height int64 | ||
| lastSpan borTypes.Span | ||
| currentProducerID uint64 | ||
| validators []*stakeTypes.Validator | ||
| extCommitBytes []byte | ||
| } | ||
|
|
||
| func stageSingletonProducer(t *testing.T) *singletonScenario { | ||
| t.Helper() | ||
| _, app, ctx, validatorPrivKeys := SetupAppWithABCICtxAndValidators(t, 4) | ||
|
|
||
| oldRio := helper.GetRioHeight() | ||
| helper.SetRioHeight(100) | ||
| t.Cleanup(func() { helper.SetRioHeight(oldRio) }) | ||
|
|
||
| height := app.LastBlockHeight() + 1 | ||
| ctx = ctx.WithBlockHeight(height) | ||
|
|
||
| validators := app.StakeKeeper.GetAllValidators(ctx) | ||
| require.Len(t, validators, 4) | ||
|
|
||
| currentProducerID := validators[0].ValId | ||
| lastSpan := borTypes.Span{ | ||
| Id: 1, | ||
| StartBlock: 100, | ||
| EndBlock: 199, | ||
| BorChainId: "1", | ||
| ValidatorSet: stakeTypes.ValidatorSet{Validators: validators, Proposer: validators[0]}, | ||
| SelectedProducers: []stakeTypes.Validator{*validators[0]}, | ||
| } | ||
| require.NoError(t, app.BorKeeper.AddNewSpan(ctx, &lastSpan)) | ||
|
|
||
| // Every validator casts a one-entry ballot for the current producer, collapsing | ||
| // CalculateProducerSet to the singleton [currentProducer]. | ||
| msgServer := borKeeper.NewMsgServerImpl(app.BorKeeper) | ||
| for _, validator := range validators { | ||
| _, err := msgServer.VoteProducers(ctx, &borTypes.MsgVoteProducers{ | ||
| Voter: validator.Signer, | ||
| VoterId: validator.ValId, | ||
| Votes: borTypes.ProducerVotes{Votes: []uint64{currentProducerID}}, | ||
| }) | ||
| require.NoError(t, err) | ||
| } | ||
| candidates, err := app.BorKeeper.CalculateProducerSet(ctx, helper.GetProducerSetLimit(ctx)) | ||
| require.NoError(t, err) | ||
| require.Equal(t, []uint64{currentProducerID}, candidates) | ||
|
|
||
| previousMilestoneHash := common.HexToHash("0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa").Bytes() | ||
| require.NoError(t, app.MilestoneKeeper.AddMilestone(ctx, milestoneTypes.Milestone{ | ||
| Proposer: validators[1].Signer, | ||
| Hash: previousMilestoneHash, | ||
| StartBlock: 1, | ||
| EndBlock: lastSpan.StartBlock - 1, | ||
| BorChainId: "1", | ||
| MilestoneId: "previous", | ||
| Timestamp: 1, | ||
| TotalDifficulty: 1, | ||
| })) | ||
|
|
||
| winningMilestone := &milestoneTypes.MilestoneProposition{ | ||
| StartBlockNumber: lastSpan.StartBlock, | ||
| ParentHash: previousMilestoneHash, | ||
| BlockHashes: [][]byte{common.HexToHash("0xbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb").Bytes()}, | ||
| BlockTds: []uint64{2}, | ||
| } | ||
|
|
||
| // Validators 1, 2 and 3 (75% of voting power) finalize the milestone; the current | ||
| // producer (validator 0) withholds, so it is absent from supportingValidatorIDs. | ||
| extCommitBytes := milestoneExtendedCommit(t, app.ChainID(), height, validators, validatorPrivKeys, currentProducerID, winningMilestone) | ||
|
|
||
| return &singletonScenario{ | ||
| app: app, | ||
| ctx: ctx, | ||
| height: height, | ||
| lastSpan: lastSpan, | ||
| currentProducerID: currentProducerID, | ||
| validators: validators, | ||
| extCommitBytes: extCommitBytes, | ||
| } | ||
| } | ||
|
|
||
| // Pre-Ithaca the fallback is gated off, so the singleton-minus-current empty set still | ||
| // errors out of PreBlocker. This pins the gate boundary: the fix changes nothing before | ||
| // the fork activates. | ||
| func TestVeBlopEmptyElectedSetPreIthacaErrors(t *testing.T) { | ||
| s := stageSingletonProducer(t) | ||
|
|
||
| oldIthaca := helper.GetIthacaHeight() | ||
| helper.SetIthacaHeight(0) // disabled: IsIthaca is always false | ||
| t.Cleanup(func() { helper.SetIthacaHeight(oldIthaca) }) | ||
|
|
||
| _, err := s.app.PreBlocker(s.ctx, &abci.RequestFinalizeBlock{ | ||
| Height: s.height, | ||
| Txs: [][]byte{s.extCommitBytes}, | ||
| ProposerAddress: common.FromHex(s.validators[1].Signer), | ||
| }) | ||
| require.Error(t, err) | ||
| require.True(t, strings.Contains(err.Error(), "no candidates found"), err.Error()) | ||
|
|
||
| // Nothing committed: the failing height replays identically. | ||
| milestoneCount, err := s.app.MilestoneKeeper.GetMilestoneCount(s.ctx) | ||
| require.NoError(t, err) | ||
| require.Equal(t, uint64(1), milestoneCount) | ||
| span, err := s.app.BorKeeper.GetLastSpan(s.ctx) | ||
| require.NoError(t, err) | ||
| require.Equal(t, s.lastSpan.Id, span.Id) | ||
| } | ||
|
|
||
| // Post-Ithaca the fallback supplies a deterministic non-empty candidate set drawn from | ||
| // the milestone supporters, so future-span creation succeeds, the milestone/span commit, | ||
| // and the new producer is a supporting validator rather than the excluded incumbent. | ||
| func TestVeBlopEmptyElectedSetPostIthacaFallsBack(t *testing.T) { | ||
| s := stageSingletonProducer(t) | ||
|
|
||
| oldIthaca := helper.GetIthacaHeight() | ||
| helper.SetIthacaHeight(s.height) | ||
| t.Cleanup(func() { helper.SetIthacaHeight(oldIthaca) }) | ||
|
|
||
| _, err := s.app.PreBlocker(s.ctx, &abci.RequestFinalizeBlock{ | ||
| Height: s.height, | ||
| Txs: [][]byte{s.extCommitBytes}, | ||
| ProposerAddress: common.FromHex(s.validators[1].Signer), | ||
| }) | ||
| require.NoError(t, err) | ||
|
|
||
| // The milestone committed and a new span was frozen. | ||
| milestoneCount, err := s.app.MilestoneKeeper.GetMilestoneCount(s.ctx) | ||
| require.NoError(t, err) | ||
| require.Equal(t, uint64(2), milestoneCount) | ||
|
|
||
| span, err := s.app.BorKeeper.GetLastSpan(s.ctx) | ||
| require.NoError(t, err) | ||
| require.Equal(t, s.lastSpan.Id+1, span.Id) | ||
| require.Len(t, span.SelectedProducers, 1) | ||
|
|
||
| // The replacement must be a supporting validator, never the excluded incumbent. | ||
| newProducer := span.SelectedProducers[0].ValId | ||
| require.NotEqual(t, s.currentProducerID, newProducer) | ||
| supporters := map[uint64]struct{}{ | ||
| s.validators[1].ValId: {}, | ||
| s.validators[2].ValId: {}, | ||
| s.validators[3].ValId: {}, | ||
| } | ||
| _, ok := supporters[newProducer] | ||
| require.True(t, ok, "new producer %d should be one of the milestone supporters", newProducer) | ||
| } | ||
|
|
||
| func milestoneExtendedCommit(t *testing.T, chainID string, height int64, validators []*stakeTypes.Validator, validatorPrivKeys []cmtsecp256k1.PrivKey, currentProducerID uint64, milestone *milestoneTypes.MilestoneProposition) []byte { | ||
| t.Helper() | ||
| require.Len(t, validatorPrivKeys, len(validators)) | ||
|
|
||
| dummyNonRpExt, err := GetDummyNonRpVoteExtension(height-1, chainID) | ||
| require.NoError(t, err) | ||
|
|
||
| extCommit := &abci.ExtendedCommitInfo{Round: 0, Votes: make([]abci.ExtendedVoteInfo, 0, len(validators))} | ||
| for i, validator := range validators { | ||
| var prop *milestoneTypes.MilestoneProposition | ||
| if validator.ValId != currentProducerID { | ||
| prop = milestone | ||
| } | ||
|
|
||
| voteExt := sidetxs.VoteExtension{ | ||
| BlockHash: []byte("repro-previous-block"), | ||
| Height: height - 1, | ||
| MilestoneProposition: prop, | ||
| SideTxResponses: []sidetxs.SideTxResponse{}, | ||
| } | ||
| voteExtBytes, err := voteExt.Marshal() | ||
| require.NoError(t, err) | ||
|
|
||
| voteInfo := abci.ExtendedVoteInfo{ | ||
| Validator: abci.Validator{ | ||
| Address: common.FromHex(validator.Signer), | ||
| Power: validator.VotingPower, | ||
| }, | ||
| VoteExtension: voteExtBytes, | ||
| NonRpVoteExtension: dummyNonRpExt, | ||
| BlockIdFlag: cmtproto.BlockIDFlagCommit, | ||
| } | ||
| createSignatureForVoteExtension(t, height-1, validatorPrivKeys[i], voteExtBytes, dummyNonRpExt, &voteInfo) | ||
| extCommit.Votes = append(extCommit.Votes, voteInfo) | ||
| } | ||
|
|
||
| bz, err := extCommit.Marshal() | ||
| require.NoError(t, err) | ||
| return bz | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,57 @@ | ||
| package keeper | ||
|
|
||
| import ( | ||
| "slices" | ||
|
|
||
| sdk "github.com/cosmos/cosmos-sdk/types" | ||
| ) | ||
|
|
||
| // eligibleProducerFallback returns a deterministic candidate set for the degenerate case | ||
| // where producer election leaves no selectable candidate other than the current producer | ||
| // (the elected set has collapsed to the lone incumbent, or every alternative was filtered | ||
| // out). It draws from the caller's active set (milestone supporters in the future-span | ||
| // path, latest active producers in the rotation paths) first, then the full validator set, | ||
| // each sorted ascending with the current and excluded producers removed. | ||
| // | ||
| // In the future-span (milestone) path this is always non-empty: a >2/3 milestone that | ||
| // excludes the current producer leaves a non-empty supporting set, so SelectProducer never | ||
| // receives an empty slice there and the PreBlocker halt cannot occur. In the non-fatal | ||
| // rotation / pending-stall paths it may legitimately return empty (e.g. every producer is | ||
| // failed/excluded); those callers already treat an empty selection as "skip this rotation | ||
| // and retry", so the result is intentionally not forced non-empty — reinstalling an | ||
| // excluded producer would defeat the rotation those paths exist to perform. | ||
| func (k *Keeper) eligibleProducerFallback(ctx sdk.Context, currentProducer uint64, activeValidatorIDs, excludedProducerIDs map[uint64]struct{}) []uint64 { | ||
| if c := sortedEligibleProducers(activeValidatorIDs, currentProducer, excludedProducerIDs); len(c) > 0 { | ||
| return c | ||
| } | ||
|
|
||
| valSet, err := k.sk.GetValidatorSet(ctx) | ||
| if err != nil { | ||
| k.Logger(ctx).Error("Failed to get validator set for producer fallback", "error", err) | ||
| return nil | ||
| } | ||
|
|
||
| valIDs := make(map[uint64]struct{}, len(valSet.Validators)) | ||
| for _, v := range valSet.Validators { | ||
| valIDs[v.ValId] = struct{}{} | ||
| } | ||
| return sortedEligibleProducers(valIDs, currentProducer, excludedProducerIDs) | ||
| } | ||
|
Check failure on line 39 in x/bor/keeper/veblop_producer_fallback.go
|
||
|
pratikspatil024 marked this conversation as resolved.
|
||
|
|
||
| // sortedEligibleProducers returns the ids in set, ascending, omitting the current producer | ||
| // and any excluded producer. Sorting makes the result deterministic across validators (Go | ||
| // map iteration order is not). | ||
| func sortedEligibleProducers(set map[uint64]struct{}, currentProducer uint64, excluded map[uint64]struct{}) []uint64 { | ||
| ids := make([]uint64, 0, len(set)) | ||
| for id := range set { | ||
| if id == currentProducer { | ||
| continue | ||
| } | ||
| if _, isExcluded := excluded[id]; isExcluded { | ||
| continue | ||
| } | ||
| ids = append(ids, id) | ||
| } | ||
| slices.Sort(ids) | ||
| return ids | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.