[Consensus] SignerIndices Optimization Interface changes (1/4) (#2047)

* update interface

* remove IdentitiesByIndices

* remove TODO

* address review comments

* [Consensus] SignerIndices Optimization (2/4) (#2101)

* implement IdentitiesByIndices

* convert signer index

* implement encoding and decoding

* add test cases

* remove signer indices file

* fix Packer

* add comments

* remove IdentitiesByIndices

* update mock

* fix test cases

* add comments

* remove fields

* fix ParentVoterIDs

* fix verifier

* fix mock

* implement staking vote progressor

* fix tests

* move to module/packer

* fix tests

* fix validator test

* fix validator test

* use ParentVoterIndices

* add QuorumCertificateWithSignerIDs

* update blockSummary

* move module

* add test cases

* remove comments

* add check to EncodeSignerIndices

* Apply suggestions from code review

Co-authored-by: Jordan Schalm <jordan@dapperlabs.com>

* Apply suggestions from code review

Co-authored-by: Alexander Hentschel <alex.hentschel@axiomzen.co>

* Apply suggestions from code review

Co-authored-by: Alexander Hentschel <alex.hentschel@axiomzen.co>

* [Consensus] Use SignerIndices in CollectionGuarantee (3/4) (#2140)

* implement IdentitiesByIndices

* implement encoding and decoding

* remove IdentitiesByIndices

* use ParentVoterIndices

* use chainID

* add ChainID

* fix test case

* remove file

* sort assignment to canonical order

* fix guarantee

* fix execution ingestion core

* use signer indices

* add identifierOrder

* use guarantor.FindGuarantors

* fix tests

* fix tests

* fix tests

* disable tests

* fix tests

* fix tests, add validation

* fix fvm test

* fix access tests

* fix tests

* fix tests

* fix select_filter_test tests

* fix access tests

* fix access integration

* fix access integration tests

* revert fvm_test changes

* update fvm_test

* fix exec tests

* fix ingestion engine tests

* fix test cases

* remove todo

* fix error handling

* add comment

* fix tests

* update logging

* refactor

* fix tests

* remove function

* fix error wrapping

* update comment

* update tests

* remove unverified signature

* revert mutator tests change

* update comment

* address comments

* add tests

* add check in NewClusterList to ensure the assignments are sorted in
canonical order

* rename method

* consensus ingestion core log signer indices

* remove tests log

* refactor canonical order check

* replace order.ByNodeIDAsc with order.Canonical

* [Consensus] Refactor for guarantee signer indices (4/4) (#2204)

* wip

* wip

* starting to fix tests

* adding tests

* happy path test

* Added toDo for fixing tests for unhappy paths

* • fixed packer tests
• consolidated logic for checking the padded bits

* re-gen mocks

* fix validPadding

* fix findguarantors

* refactor ingestion/core.go

* move FindGuarantors to protocol

* remove commented code

* fix name

* fix tests

* small refactor

* fix import

* Apply suggestions from code review

Co-authored-by: Jordan Schalm <jordan@dapperlabs.com>

* fix error type

* fix type

* fix error message

* update comments

* update comment

* update tests

* fix identity_test

* fix tests

* fix unittest

* fix cluster tests

* fix tests

* fixtures ingestion engine tests

* fix execution_test

* fix consensus inclusion tests

* fix bootstrap constraint check

* fix cycle dep from NewClusterList (#2225)

Co-authored-by: Alexander Hentschel <alex.hentschel@axiomzen.co>
Co-authored-by: Jordan Schalm <jordan@dapperlabs.com>

Co-authored-by: Alexander Hentschel <alex.hentschel@axiomzen.co>
Co-authored-by: Jordan Schalm <jordan@dapperlabs.com>

Co-authored-by: Jordan Schalm <jordan@dapperlabs.com>
Co-authored-by: Alexander Hentschel <alex.hentschel@axiomzen.co>

Co-authored-by: Jordan Schalm <jordan@dapperlabs.com>
Co-authored-by: Alexander Hentschel <alex.hentschel@axiomzen.co>

cmd/bootstrap/cmd/clusters.go

@@ -5,6 +5,8 @@ import (
 	model "github.com/onflow/flow-go/model/bootstrap"
 	"github.com/onflow/flow-go/model/cluster"
 	"github.com/onflow/flow-go/model/flow"
+	"github.com/onflow/flow-go/model/flow/assignment"
+	"github.com/onflow/flow-go/model/flow/factory"
 	"github.com/onflow/flow-go/model/flow/filter"
 )
 
@@ -23,20 +25,22 @@ func constructClusterAssignment(partnerNodes, internalNodes []model.NodeInfo, se
 	internals = internals.DeterministicShuffle(seed)
 
 	nClusters := flagCollectionClusters
-	assignments := make(flow.AssignmentList, nClusters)
+	identifierLists := make([]flow.IdentifierList, nClusters)
 
 	// first, round-robin internal nodes into each cluster
 	for i, node := range internals {
-		assignments[i%len(assignments)] = append(assignments[i%len(assignments)], node.NodeID)
+		identifierLists[i%len(identifierLists)] = append(identifierLists[i%len(identifierLists)], node.NodeID)
 	}
 
 	// next, round-robin partner nodes into each cluster
 	for i, node := range partners {
-		assignments[i%len(assignments)] = append(assignments[i%len(assignments)], node.NodeID)
+		identifierLists[i%len(identifierLists)] = append(identifierLists[i%len(identifierLists)], node.NodeID)
 	}
 
+	assignments := assignment.FromIdentifierLists(identifierLists)
+
 	collectors := append(partners, internals...)
-	clusters, err := flow.NewClusterList(assignments, collectors)
+	clusters, err := factory.NewClusterList(assignments, collectors)
 	if err != nil {
 		log.Fatal().Err(err).Msg("could not create cluster list")
 	}

cmd/bootstrap/cmd/constraints.go

@@ -50,12 +50,12 @@ func checkConstraints(partnerNodes, internalNodes []model.NodeInfo) {
 			if _, exists := internals.ByNodeID(node.NodeID); exists {
 				clusterInternalCount++
 			}
-			if clusterInternalCount <= clusterPartnerCount*2 {
-				log.Fatal().Msgf(
-					"will not bootstrap configuration without Byzantine majority within cluster: "+
-						"(partners=%d, internals=%d, min_internals=%d)",
-					clusterPartnerCount, clusterInternalCount, clusterPartnerCount*2+1)
-			}
+		}
+		if clusterInternalCount <= clusterPartnerCount*2 {
+			log.Fatal().Msgf(
+				"will not bootstrap configuration without Byzantine majority within cluster: "+
+					"(partners=%d, internals=%d, min_internals=%d)",
+				clusterPartnerCount, clusterInternalCount, clusterPartnerCount*2+1)
 		}
 		partnerCOLCount += clusterPartnerCount
 		internalCOLCount += clusterInternalCount

cmd/bootstrap/cmd/seal.go

@@ -7,6 +7,7 @@ import (
 	"github.com/onflow/flow-go/model/dkg"
 	"github.com/onflow/flow-go/model/flow"
 	"github.com/onflow/flow-go/model/flow/order"
+	"github.com/onflow/flow-go/module/signature"
 )
 
 func constructRootResultAndSeal(
@@ -43,9 +44,24 @@ func constructRootResultAndSeal(
 		RandomSource:       getRandomSource(flagBootstrapRandomSeed),
 	}
 
+	qcsWithSignerIDs := make([]*flow.QuorumCertificateWithSignerIDs, 0, len(clusterQCs))
+	for i, clusterQC := range clusterQCs {
+		members := assignments[i]
+		signerIDs, err := signature.DecodeSignerIndicesToIdentifiers(members, clusterQC.SignerIndices)
+		if err != nil {
+			log.Fatal().Err(err).Msgf("could not decode signer IDs from clusterQC at index %v", i)
+		}
+		qcsWithSignerIDs = append(qcsWithSignerIDs, &flow.QuorumCertificateWithSignerIDs{
+			View:      clusterQC.View,
+			BlockID:   clusterQC.BlockID,
+			SignerIDs: signerIDs,
+			SigData:   clusterQC.SigData,
+		})
+	}
+
 	epochCommit := &flow.EpochCommit{
 		Counter:            flagEpochCounter,
-		ClusterQCs:         flow.ClusterQCVoteDatasFromQCs(clusterQCs),
+		ClusterQCs:         flow.ClusterQCVoteDatasFromQCs(qcsWithSignerIDs),
 		DKGGroupKey:        dkgData.PubGroupKey,
 		DKGParticipantKeys: dkgData.PubKeyShares,
 	}

cmd/bootstrap/run/block.go

@@ -19,7 +19,7 @@ func GenerateRootBlock(chainID flow.ChainID, parentID flow.Identifier, height ui
 		PayloadHash:        payload.Hash(),
 		Timestamp:          timestamp,
 		View:               0,
-		ParentVoterIDs:     nil,
+		ParentVoterIndices: nil,
 		ParentVoterSigData: nil,
 		ProposerID:         flow.ZeroID,
 		ProposerSigData:    nil,

cmd/util/cmd/epochs/cmd/reset_test.go

@@ -30,7 +30,7 @@ func TestReset_LocalSnapshot(t *testing.T) {
 		unittest.RunWithTempDir(t, func(bootDir string) {
 
 			// create a root snapshot
-			rootSnapshot := unittest.RootSnapshotFixture(unittest.IdentityListFixture(10))
+			rootSnapshot := unittest.RootSnapshotFixture(unittest.IdentityListFixture(10, unittest.WithAllRoles()))
 
 			// write snapshot to correct path in bootDir
 			err := writeRootSnapshot(bootDir, rootSnapshot)
@@ -62,7 +62,7 @@ func TestReset_LocalSnapshot(t *testing.T) {
 		unittest.RunWithTempDir(t, func(bootDir string) {
 
 			// create a root snapshot
-			rootSnapshot := unittest.RootSnapshotFixture(unittest.IdentityListFixture(10))
+			rootSnapshot := unittest.RootSnapshotFixture(unittest.IdentityListFixture(10, unittest.WithAllRoles()))
 
 			// write snapshot to correct path in bootDir
 			err := writeRootSnapshot(bootDir, rootSnapshot)

cmd/util/cmd/exec-data-json-export/block_exporter.go

@@ -16,12 +16,12 @@ import (
 )
 
 type blockSummary struct {
-	BlockHeight    uint64   `json:"block_height"`
-	BlockID        string   `json:"block_id"`
-	ParentBlockID  string   `json:"parent_block_id"`
-	ParentVoterIDs []string `json:"parent_voter_ids"`
-	// ParentVoterSigData []string  `json:"parent_voter_sig"`
-	ProposerID string `json:"proposer_id"`
+	BlockHeight        uint64 `json:"block_height"`
+	BlockID            string `json:"block_id"`
+	ParentBlockID      string `json:"parent_block_id"`
+	ParentVoterIndices string `json:"parent_voter_indices"`
+	ParentVoterSigData string `json:"parent_voter_sig"`
+	ProposerID         string `json:"proposer_id"`
 	// ProposerSigData    string  `json:"proposer_sig"`
 	Timestamp         time.Time `json:"timestamp"`
 	CollectionIDs     []string  `json:"collection_ids"`
@@ -87,22 +87,18 @@ func ExportBlocks(blockID flow.Identifier, dbPath string, outputPath string) (fl
 			sealsStates = append(sealsStates, hex.EncodeToString(s.FinalState[:]))
 		}
 
-		pvIDs := make([]string, 0)
-		for _, i := range header.ParentVoterIDs {
-			pvIDs = append(pvIDs, hex.EncodeToString(i[:]))
-		}
-
 		b := blockSummary{
-			BlockID:           hex.EncodeToString(activeBlockID[:]),
-			BlockHeight:       header.Height,
-			ParentBlockID:     hex.EncodeToString(header.ParentID[:]),
-			ParentVoterIDs:    pvIDs,
-			ProposerID:        hex.EncodeToString(header.ProposerID[:]),
-			Timestamp:         header.Timestamp,
-			CollectionIDs:     cols,
-			SealedBlocks:      seals,
-			SealedResults:     sealsResults,
-			SealedFinalStates: sealsStates,
+			BlockID:            hex.EncodeToString(activeBlockID[:]),
+			BlockHeight:        header.Height,
+			ParentBlockID:      hex.EncodeToString(header.ParentID[:]),
+			ParentVoterIndices: hex.EncodeToString(header.ParentVoterIndices),
+			ParentVoterSigData: hex.EncodeToString(header.ParentVoterSigData),
+			ProposerID:         hex.EncodeToString(header.ProposerID[:]),
+			Timestamp:          header.Timestamp,
+			CollectionIDs:      cols,
+			SealedBlocks:       seals,
+			SealedResults:      sealsResults,
+			SealedFinalStates:  sealsStates,
 		}
 
 		jsonData, err := json.Marshal(b)

consensus/follower_test.go

@@ -5,6 +5,8 @@ import (
 	"testing"
 	"time"
 
+	"github.com/onflow/flow-go/module/signature"
+
 	"github.com/rs/zerolog"
 	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/require"
@@ -68,9 +70,9 @@ func (s *HotStuffFollowerSuite) SetupTest() {
 
 	// mock consensus committee
 	s.committee = &mockhotstuff.Committee{}
-	s.committee.On("Identities", mock.Anything, mock.Anything).Return(
-		func(blockID flow.Identifier, selector flow.IdentityFilter) flow.IdentityList {
-			return identities.Filter(selector)
+	s.committee.On("Identities", mock.Anything).Return(
+		func(blockID flow.Identifier) flow.IdentityList {
+			return identities
 		},
 		nil,
 	)
@@ -105,10 +107,13 @@ func (s *HotStuffFollowerSuite) SetupTest() {
 		Height:    21053,
 		View:      52078,
 	}
+
+	signerIndices, err := signature.EncodeSignersToIndices(identities.NodeIDs(), identities.NodeIDs()[:3])
+	require.NoError(s.T(), err)
 	s.rootQC = &flow.QuorumCertificate{
-		View:      s.rootHeader.View,
-		BlockID:   s.rootHeader.ID(),
-		SignerIDs: identities.NodeIDs()[:3],
+		View:          s.rootHeader.View,
+		BlockID:       s.rootHeader.ID(),
+		SignerIndices: signerIndices,
 	}
 
 	// we start with the latest finalized block being the root block
@@ -335,6 +340,7 @@ func (mc *MockConsensus) extendBlock(blockView uint64, parent *flow.Header) *flo
 	nextBlock := unittest.BlockHeaderWithParentFixture(parent)
 	nextBlock.View = blockView
 	nextBlock.ProposerID = mc.identities[int(blockView)%len(mc.identities)].NodeID
-	nextBlock.ParentVoterIDs = mc.identities.NodeIDs()
+	signerIndices, _ := signature.EncodeSignersToIndices(mc.identities.NodeIDs(), mc.identities.NodeIDs())
+	nextBlock.ParentVoterIndices = signerIndices
 	return &nextBlock
 }

consensus/hotstuff/blockproducer/block_producer.go

@@ -33,7 +33,7 @@ func (bp *BlockProducer) MakeBlockProposal(qc *flow.QuorumCertificate, view uint
 	// in hotstuff, we use this for view number and signature-related fields
 	setHotstuffFields := func(header *flow.Header) error {
 		header.View = view
-		header.ParentVoterIDs = qc.SignerIDs
+		header.ParentVoterIndices = qc.SignerIndices
 		header.ParentVoterSigData = qc.SigData
 		header.ProposerID = bp.committee.Self()
 

consensus/hotstuff/committee.go

@@ -17,14 +17,12 @@ import (
 // Given a collector block, some logic is required to find the main consensus block
 // for determining the valid collector-HotStuff participants.
 type Committee interface {
-
 	// Identities returns a IdentityList with legitimate HotStuff participants for the specified block.
-	// The list of participants is filtered by the provided selector. The returned list of HotStuff participants
+	// The returned list of HotStuff participants
 	//   * contains nodes that are allowed to sign the specified block (legitimate participants with NON-ZERO STAKE)
 	//   * is ordered in the canonical order
 	//   * contains no duplicates.
-	// The list of all legitimate HotStuff participants for the specified block can be obtained by using `filter.Any`
-	Identities(blockID flow.Identifier, selector flow.IdentityFilter) (flow.IdentityList, error)
+	Identities(blockID flow.Identifier) (flow.IdentityList, error)
 
 	// Identity returns the full Identity for specified HotStuff participant.
 	// The node must be a legitimate HotStuff participant with NON-ZERO STAKE at the specified block.

consensus/hotstuff/committees/cluster_committee.go

@@ -61,8 +61,12 @@ func NewClusterCommittee(
 	return com, nil
 }
 
-func (c *Cluster) Identities(blockID flow.Identifier, selector flow.IdentityFilter) (flow.IdentityList, error) {
-
+// Identities returns the identities of all cluster members that are authorized to
+// participate at the given block. The order of the identities is the canonical order.
+func (c *Cluster) Identities(blockID flow.Identifier) (flow.IdentityList, error) {
+	// blockID is a collection block not a block produced by consensus,
+	// to query the identities from protocol state, we need to use the reference block id from the payload
+	//
 	// first retrieve the cluster block payload
 	payload, err := c.payloads.ByBlockID(blockID)
 	if err != nil {
@@ -74,14 +78,12 @@ func (c *Cluster) Identities(blockID flow.Identifier, selector flow.IdentityFilt
 
 	// use the initial cluster members for root block
 	if isRootBlock {
-		return c.initialClusterMembers.Filter(selector), nil
+		return c.initialClusterMembers, nil
 	}
 
 	// otherwise use the snapshot given by the reference block
-	identities, err := c.state.AtBlockID(payload.ReferenceBlockID).Identities(filter.And(
-		selector,
-		c.clusterMemberFilter,
-	))
+	identities, err := c.state.AtBlockID(payload.ReferenceBlockID).Identities(c.clusterMemberFilter) // remove ejected nodes
+
 	return identities, err
 }
 

consensus/hotstuff/committees/consensus_committee.go

@@ -68,11 +68,10 @@ func NewConsensusCommittee(state protocol.State, me flow.Identifier) (*Consensus
 	return com, nil
 }
 
-func (c *Consensus) Identities(blockID flow.Identifier, selector flow.IdentityFilter) (flow.IdentityList, error) {
-	il, err := c.state.AtBlockID(blockID).Identities(filter.And(
-		filter.IsVotingConsensusCommitteeMember,
-		selector,
-	))
+// Identities returns the identities of all authorized consensus participants at the given block.
+// The order of the identities is the canonical order.
+func (c *Consensus) Identities(blockID flow.Identifier) (flow.IdentityList, error) {
+	il, err := c.state.AtBlockID(blockID).Identities(filter.IsVotingConsensusCommitteeMember)
 	return il, err
 }
 

consensus/hotstuff/committees/leader/leader_selection_test.go

@@ -304,59 +304,55 @@ func TestZeroStakedNodeWillNotBeSelected(t *testing.T) {
 		rng, err := random.NewChacha20PRG(someSeed, []byte("leader_selec"))
 		require.NoError(t, err)
 
-		for i := 0; i < 100; i++ {
-			// create 1002 nodes with all 0 stake
-			identities := unittest.IdentityListFixture(1002, unittest.WithStake(0))
-
-			// create 2 nodes with 1 stake, and place them in between
-			// index 233-777
-			n := rng.UintN(777-233) + 233
-			m := rng.UintN(777-233) + 233
-			identities[n].Stake = 1
-			identities[m].Stake = 1
-
-			// the following code check the zero staker should not be selected
-			stakeful := identities.Filter(filter.HasStake(true))
-
-			count := 1000
-			selectionFromAll, err := ComputeLeaderSelectionFromSeed(0, someSeed, count, identities)
-			require.NoError(t, err)
+		// create 1002 nodes with all 0 stake
+		identities := unittest.IdentityListFixture(1002, unittest.WithStake(0))
 
-			selectionFromStakeful, err := ComputeLeaderSelectionFromSeed(0, someSeed, count, stakeful)
-			require.NoError(t, err)
+		// create 2 nodes with 1 stake, and place them in between
+		// index 233-777
+		n := rng.UintN(777-233) + 233
+		m := rng.UintN(777-233) + 233
+		identities[n].Stake = 1
+		identities[m].Stake = 1
 
-			for i := 0; i < count; i++ {
-				nodeIDFromAll, err := selectionFromAll.LeaderForView(uint64(i))
-				require.NoError(t, err)
+		// the following code check the zero staker should not be selected
+		stakeful := identities.Filter(filter.HasStake(true))
 
-				nodeIDFromStakeful, err := selectionFromStakeful.LeaderForView(uint64(i))
-				require.NoError(t, err)
+		count := 1000
+		selectionFromAll, err := ComputeLeaderSelectionFromSeed(0, someSeed, count, identities)
+		require.NoError(t, err)
 
-				// the selection should be the same
-				require.Equal(t, nodeIDFromStakeful, nodeIDFromAll)
-			}
+		selectionFromStakeful, err := ComputeLeaderSelectionFromSeed(0, someSeed, count, stakeful)
+		require.NoError(t, err)
+
+		for i := 0; i < count; i++ {
+			nodeIDFromAll, err := selectionFromAll.LeaderForView(uint64(i))
+			require.NoError(t, err)
+
+			nodeIDFromStakeful, err := selectionFromStakeful.LeaderForView(uint64(i))
+			require.NoError(t, err)
+
+			// the selection should be the same
+			require.Equal(t, nodeIDFromStakeful, nodeIDFromAll)
 		}
 
 		t.Run("if there is only 1 node has stake, then it will be always be the leader and the only leader", func(t *testing.T) {
 			rng, err := random.NewChacha20PRG(someSeed, []byte("leader_selec"))
 			require.NoError(t, err)
 
-			for i := 0; i < 100; i++ {
-				identities := unittest.IdentityListFixture(1000, unittest.WithStake(0))
+			identities := unittest.IdentityListFixture(1000, unittest.WithStake(0))
 
-				n := rng.UintN(1000)
-				stake := n + 1
-				identities[n].Stake = stake
-				onlyStaked := identities[n]
+			n := rng.UintN(1000)
+			stake := n + 1
+			identities[n].Stake = stake
+			onlyStaked := identities[n]
 
-				selections, err := ComputeLeaderSelectionFromSeed(0, someSeed, 1000, identities)
-				require.NoError(t, err)
+			selections, err := ComputeLeaderSelectionFromSeed(0, someSeed, 1000, identities)
+			require.NoError(t, err)
 
-				for i := 0; i < 1000; i++ {
-					nodeID, err := selections.LeaderForView(uint64(i))
-					require.NoError(t, err)
-					require.Equal(t, onlyStaked.NodeID, nodeID)
-				}
+			for i := 0; i < 1000; i++ {
+				nodeID, err := selections.LeaderForView(uint64(i))
+				require.NoError(t, err)
+				require.Equal(t, onlyStaked.NodeID, nodeID)
 			}
 		})
 	})

consensus/hotstuff/committees/metrics_wrapper.go

@@ -28,9 +28,9 @@ func NewMetricsWrapper(committee hotstuff.Committee, metrics module.HotstuffMetr
 	}
 }
 
-func (w CommitteeMetricsWrapper) Identities(blockID flow.Identifier, selector flow.IdentityFilter) (flow.IdentityList, error) {
+func (w CommitteeMetricsWrapper) Identities(blockID flow.Identifier) (flow.IdentityList, error) {
 	processStart := time.Now()
-	identities, err := w.committee.Identities(blockID, selector)
+	identities, err := w.committee.Identities(blockID)
 	w.metrics.CommitteeProcessingDuration(time.Since(processStart))
 	return identities, err
 }