diff --git a/dot/parachain/dispute/common/helpers_test.go b/dot/parachain/dispute/common/helpers_test.go new file mode 100644 index 00000000000..eb17a541f35 --- /dev/null +++ b/dot/parachain/dispute/common/helpers_test.go @@ -0,0 +1,50 @@ +package common + +import ( + "github.com/stretchr/testify/require" + "testing" +) + +func TestGetByzantineThreshold(t *testing.T) { + cases := []struct { + n, expected int + }{ + {0, 0}, + {1, 0}, + {2, 0}, + {3, 0}, + {4, 1}, + {5, 1}, + {6, 1}, + {9, 2}, + {10, 3}, + // Additional cases can be added here + } + + for _, c := range cases { + got := GetByzantineThreshold(c.n) + require.Equal(t, c.expected, got) + } +} + +func TestGetSuperMajorityThreshold(t *testing.T) { + cases := []struct { + n, expected int + }{ + {0, 0}, + {1, 1}, + {2, 2}, + {3, 3}, + {4, 3}, + {5, 4}, + {6, 5}, + {9, 7}, + {10, 7}, + // Additional cases can be added here + } + + for _, c := range cases { + got := GetSuperMajorityThreshold(c.n) + require.Equal(t, c.expected, got) + } +} diff --git a/dot/parachain/dispute/coordinator_test.go b/dot/parachain/dispute/coordinator_test.go index 8496072f34e..84e9fd57616 100644 --- a/dot/parachain/dispute/coordinator_test.go +++ b/dot/parachain/dispute/coordinator_test.go @@ -2989,7 +2989,140 @@ func TestDisputesCoordinator(t *testing.T) { ts.conclude(t) }) t.Run("informs_chain_selection_when_dispute_concluded_against", func(t *testing.T) { + t.Parallel() + ts := newTestState(t) + session := parachaintypes.SessionIndex(1) + initialised := false + restarted := false + sessionEvents, err := parachaintypes.NewCandidateEvents() + require.NoError(t, err) + ts.mockRuntimeCalls(t, session, nil, &sessionEvents, nil, &initialised, &restarted) + + wg := sync.WaitGroup{} + wg.Add(2) + go func() { + defer wg.Done() + ts.mockResumeSync(t, &session) + }() + go func() { + defer wg.Done() + ts.run(t) + }() + wg.Wait() + initialised = true + candidateReceipt := getValidCandidateReceipt(t) + candidateHash, err := candidateReceipt.Hash() + require.NoError(t, err) + parent1Number := uint32(1) + parent2Number := uint32(2) + block1Header := types.Header{ + ParentHash: ts.lastBlock, + Number: uint(parent1Number), + StateRoot: common.Hash{}, + ExtrinsicsRoot: common.Hash{}, + Digest: types.NewDigest(), + } + parent1Hash := block1Header.Hash() + event := getCandidateIncludedEvent(t, candidateReceipt) + value, err := event.Value() + require.NoError(t, err) + err = sessionEvents.Add(value) + require.NoError(t, err) + ts.activateLeafAtSession(t, session, uint(parent1Number)) + + block2Header := types.Header{ + ParentHash: ts.lastBlock, + Number: uint(parent2Number), + StateRoot: common.Hash{}, + ExtrinsicsRoot: common.Hash{}, + Digest: types.NewDigest(), + } + parent2Hash := block2Header.Hash() + ts.activateLeafAtSession(t, session, uint(parent2Number)) + + byzantineThreshold := disputesCommon.GetByzantineThreshold(len(ts.validators)) + validVote, invalidVote := ts.generateOpposingVotesPair(t, + 2, + 1, + candidateHash, + session, + ExplicitVote, + ) + wg = sync.WaitGroup{} + wg.Add(1) + go func() { + defer wg.Done() + ts.handleApprovalVoteRequest(t, candidateHash, []overseer.ApprovalSignature{}) + }() + statements := []disputetypes.Statement{ + { + SignedDisputeStatement: validVote, + ValidatorIndex: 2, + }, + { + SignedDisputeStatement: invalidVote, + ValidatorIndex: 1, + }, + } + importResult := ts.sendImportStatementsMessage(t, candidateReceipt, session, statements, make(chan any)) + require.Equal(t, ValidImport, importResult) + wg.Wait() + + // Use a different expected commitments hash to ensure the candidate validation returns + // invalid. + handleParticipationWithDistribution(t, ts.mockOverseer, ts.runtime, candidateHash, common.Hash{1}) + + statements = []disputetypes.Statement{} + for i := 3; i < byzantineThreshold+3; i++ { + vote := ts.issueExplicitStatementWithIndex(t, parachaintypes.ValidatorIndex(i), candidateHash, session, false) + statements = append(statements, disputetypes.Statement{ + SignedDisputeStatement: vote, + ValidatorIndex: parachaintypes.ValidatorIndex(i), + }) + } + importResult = ts.sendImportStatementsMessage(t, candidateReceipt, session, statements, make(chan any)) + require.Equal(t, ValidImport, importResult) + + ctx, cancel := context.WithTimeout(context.Background(), timeout) + defer cancel() + select { + case msg := <-ts.mockOverseer: + switch data := msg.(type) { + case overseer.ChainSelectionMessage[overseer.RevertBlocks]: + parent1Exists := false + parent2Exists := false + for _, b := range data.Message.Blocks { + if b.Hash == parent1Hash && b.Number == parent1Number { + parent1Exists = true + } + if b.Hash == parent2Hash && b.Number == parent2Number { + parent2Exists = true + } + } + require.True(t, parent1Exists) + require.True(t, parent2Exists) + default: + err := fmt.Errorf("unexpected message type: %T", msg) + require.NoError(t, err) + } + case <-ctx.Done(): + err := fmt.Errorf("timeout waiting for chain selection message") + require.NoError(t, err) + } + + // One more import which should not trigger reversion + // Validator index is `byzantineThreshold + 4` + vote := ts.issueExplicitStatementWithIndex(t, parachaintypes.ValidatorIndex(byzantineThreshold+4), candidateHash, session, false) + statements = []disputetypes.Statement{ + { + SignedDisputeStatement: vote, + ValidatorIndex: parachaintypes.ValidatorIndex(byzantineThreshold + 4), + }, + } + _ = ts.sendImportStatementsMessage(t, candidateReceipt, session, statements, make(chan any)) + ts.awaitConclude(t) + ts.conclude(t) }) } diff --git a/dot/parachain/dispute/import.go b/dot/parachain/dispute/import.go index ab76a74d4fb..6b0680545bb 100644 --- a/dot/parachain/dispute/import.go +++ b/dot/parachain/dispute/import.go @@ -34,6 +34,8 @@ type ImportResult interface { ) (ImportResult, error) // IntoUpdatedVotes returns the updated votes after the import IntoUpdatedVotes() *types.CandidateVotes + // HasFreshByzantineThresholdAgainst returns true if there are byzantineThreshold + 1 invalid votes + HasFreshByzantineThresholdAgainst() bool } // ImportResultHandler implements ImportResult interface @@ -207,6 +209,10 @@ func (i ImportResultHandler) IntoUpdatedVotes() *types.CandidateVotes { return &i.newState.Votes } +func (i ImportResultHandler) HasFreshByzantineThresholdAgainst() bool { + return !i.oldState.ByzantineThresholdAgainst && i.newState.ByzantineThresholdAgainst +} + var _ ImportResult = (*ImportResultHandler)(nil) func NewImportResultFromStatements( diff --git a/dot/parachain/dispute/initialized.go b/dot/parachain/dispute/initialized.go index 1b6defbc679..dffc69b2bdd 100644 --- a/dot/parachain/dispute/initialized.go +++ b/dot/parachain/dispute/initialized.go @@ -659,6 +659,13 @@ func (i *Initialized) HandleImportStatements( candidateHash, session, ) + var ownStatements []types.Statement + for _, statement := range statements { + _, ok := env.ControlledIndices[statement.ValidatorIndex] + if ok && statement.SignedDisputeStatement.CandidateHash == candidateHash { + ownStatements = append(ownStatements, statement) + } + } var importResult *ImportResultHandler intermediateResult, err := NewImportResultFromStatements(env, statements, oldState, now) @@ -920,17 +927,11 @@ func (i *Initialized) HandleImportStatements( } } - // Notify ChainSelection if a dispute has concluded against a candidate. ChainSelection - // will need to mark the candidate's relay parent as reverted. - isFreshlyConcludedAgainst, err := importResult.IsFreshlyConcludedAgainst() - if err != nil { - return InvalidImport, fmt.Errorf("is freshly concluded against: %w", err) - } - if isFreshlyConcludedAgainst { - inclusions := i.Scraper.GetBlocksIncludingCandidate(candidateHash) - blocks := make([]overseer.Block, len(inclusions)) - for _, inclusion := range inclusions { - logger.Tracef("dispute has just concluded against the candidate hash noted."+ + if importResult.HasFreshByzantineThresholdAgainst() { + blocksIncluding := i.Scraper.GetBlocksIncludingCandidate(candidateHash) + var blocks []overseer.Block + for _, inclusion := range blocksIncluding { + logger.Warnf("Dispute has just concluded against the candidate hash noted. "+ "Its parent will be marked as reverted. candidateHash: %v, parentBlockNumber: %v, parentBlockHash: %v", candidateHash, inclusion.BlockNumber, @@ -942,7 +943,7 @@ func (i *Initialized) HandleImportStatements( }) } - if len(blocks) > 0 { + if len(blocksIncluding) > 0 { message := overseer.ChainSelectionMessage[overseer.RevertBlocks]{ Message: overseer.RevertBlocks{Blocks: blocks}, } @@ -958,6 +959,42 @@ func (i *Initialized) HandleImportStatements( } } + isFreshlyConcludedFor, err := importResult.IsFreshlyConcludedFor() + if err != nil { + return InvalidImport, fmt.Errorf("is freshly concluded for: %w", err) + } + if isFreshlyConcludedFor { + logger.Infof("Dispute on candidate with valid result. candidateHash: %v, session: %v", + candidateHash, + session, + ) + for _, statement := range ownStatements { + logger.Warnf("Voted against a candidate that was concluded valid. CandidateHash: %v, validatorIndex:%v", + candidateHash, + statement.ValidatorIndex, + ) + } + } + + // Notify ChainSelection if a dispute has concluded against a candidate. ChainSelection + // will need to mark the candidate's relay parent as reverted. + isFreshlyConcludedAgainst, err := importResult.IsFreshlyConcludedAgainst() + if err != nil { + return InvalidImport, fmt.Errorf("is freshly concluded against: %w", err) + } + if isFreshlyConcludedAgainst { + logger.Infof("Dispute on candidate with invalid result. candidateHash: %v, session: %v", + candidateHash, + session, + ) + for _, statement := range ownStatements { + logger.Warnf("Voted against a candidate that was concluded invalid. CandidateHash: %v, validatorIndex:%v", + candidateHash, + statement.ValidatorIndex, + ) + } + } + // TODO: update metrics // Only write when votes have changed. diff --git a/dot/parachain/dispute/participation.go b/dot/parachain/dispute/participation.go index 343197b0ffd..d50d4b3cd8b 100644 --- a/dot/parachain/dispute/participation.go +++ b/dot/parachain/dispute/participation.go @@ -217,6 +217,8 @@ func (p *ParticipationHandler) forkParticipation(sender chan<- any, request Part } func (p *ParticipationHandler) participate(sender chan<- any, blockHash common.Hash, request ParticipationRequest) error { + // TODO: determine if this has any effect on performance + // also look into how we can enable this only for tests. using ENVs maybe? time.Sleep(100 * time.Millisecond) // get available data from the overseer respCh := make(chan any, 1) diff --git a/dot/parachain/dispute/types/vote.go b/dot/parachain/dispute/types/vote.go index 702fb1ddcbd..0148eaffc77 100644 --- a/dot/parachain/dispute/types/vote.go +++ b/dot/parachain/dispute/types/vote.go @@ -210,9 +210,10 @@ func NewOwnVoteStateVDTWithVotes(voteState CandidateVotes, env *CandidateEnviron // CandidateVoteState is the state of the votes for a candidate type CandidateVoteState struct { - Votes CandidateVotes - Own OwnVoteStateVDT - DisputeStatus *DisputeStatusVDT + Votes CandidateVotes + Own OwnVoteStateVDT + DisputeStatus *DisputeStatusVDT + ByzantineThresholdAgainst bool } // IsDisputed returns true if we have an ongoing dispute @@ -250,17 +251,19 @@ func (c *CandidateVoteState) IsConcludedAgainst() (bool, error) { // IntoOldState Extracts `CandidateVotes` for handling import of new statements. func (c *CandidateVoteState) IntoOldState() (CandidateVotes, CandidateVoteState) { return c.Votes.Copy(), CandidateVoteState{ - Votes: NewCandidateVotes(), - Own: c.Own, - DisputeStatus: c.DisputeStatus, + Votes: NewCandidateVotes(), + Own: c.Own, + DisputeStatus: c.DisputeStatus, + ByzantineThresholdAgainst: c.ByzantineThresholdAgainst, } } // NewCandidateVoteState creates a new CandidateVoteState func NewCandidateVoteState(votes CandidateVotes, env *CandidateEnvironment, now uint64) (CandidateVoteState, error) { var ( - disputeStatus *DisputeStatusVDT - err error + disputeStatus *DisputeStatusVDT + byzantineThresholdAgainst bool + err error ) ownVoteState, err := NewOwnVoteStateVDTWithVotes(votes, env) @@ -303,12 +306,14 @@ func NewCandidateVoteState(votes CandidateVotes, env *CandidateEnvironment, now } } disputeStatus = &status + byzantineThresholdAgainst = votes.Invalid.Len() > byzantineThreshold } return CandidateVoteState{ - Votes: votes, - Own: ownVoteState, - DisputeStatus: disputeStatus, + Votes: votes, + Own: ownVoteState, + DisputeStatus: disputeStatus, + ByzantineThresholdAgainst: byzantineThresholdAgainst, }, nil }