Merge pull request #4063 from onflow/yurii/decode-signer-ids-by-block…

…-id-fallback

[Committee] Fallback mechanism for `DecodeSignerIDs`

consensus/hotstuff/committee.go

@@ -130,7 +130,6 @@ type BlockSignerDecoder interface {
 	// consensus committee has reached agreement on validity of parent block. Consequently, the
 	// returned IdentifierList contains the consensus participants that signed the parent block.
 	// Expected Error returns during normal operations:
-	//   - model.ErrViewForUnknownEpoch if the given block's parent is within an unknown epoch
 	//   - signature.InvalidSignerIndicesError if signer indices included in the header do
 	//     not encode a valid subset of the consensus committee
 	DecodeSignerIDs(header *flow.Header) (flow.IdentifierList, error)

consensus/hotstuff/signature/block_signer_decoder.go

@@ -27,7 +27,6 @@ var _ hotstuff.BlockSignerDecoder = (*BlockSignerDecoder)(nil)
 // consensus committee has reached agreement on validity of parent block. Consequently, the
 // returned IdentifierList contains the consensus participants that signed the parent block.
 // Expected Error returns during normal operations:
-//   - model.ErrViewForUnknownEpoch if the given block is within an unknown epoch
 //   - signature.InvalidSignerIndicesError if signer indices included in the header do
 //     not encode a valid subset of the consensus committee
 func (b *BlockSignerDecoder) DecodeSignerIDs(header *flow.Header) (flow.IdentifierList, error) {
@@ -36,12 +35,20 @@ func (b *BlockSignerDecoder) DecodeSignerIDs(header *flow.Header) (flow.Identifi
 		return []flow.Identifier{}, nil
 	}
 
+	// we will use IdentitiesByEpoch since it's a faster call and avoids DB lookup
 	members, err := b.IdentitiesByEpoch(header.ParentView)
 	if err != nil {
 		if errors.Is(err, model.ErrViewForUnknownEpoch) {
-			return nil, fmt.Errorf("could not retrieve identities for block %x with QC view %d: %w", header.ID(), header.ParentView, err)
+			// possibly, we request epoch which is far behind in the past, in this case we won't have it in cache.
+			// try asking by parent ID
+			members, err = b.IdentitiesByBlock(header.ParentID)
+			if err != nil {
+				return nil, fmt.Errorf("could not retrieve identities for block %x with QC view %d for parent %x: %w",
+					header.ID(), header.ParentView, header.ParentID, err)
+			}
+		} else {
+			return nil, fmt.Errorf("unexpected error retrieving identities for block %v: %w", header.ID(), err)
 		}
-		return nil, fmt.Errorf("unexpected error retrieving identities for block %v: %w", header.ID(), err)
 	}
 	signerIDs, err := signature.DecodeSignerIndicesToIdentifiers(members.NodeIDs(), header.ParentVoterIndices)
 	if err != nil {

consensus/hotstuff/signature/block_signer_decoder_test.go

@@ -2,6 +2,7 @@ package signature
 
 import (
 	"errors"
+	"fmt"
 	"testing"
 
 	"github.com/stretchr/testify/mock"
@@ -13,6 +14,7 @@ import (
 	"github.com/onflow/flow-go/model/flow"
 	"github.com/onflow/flow-go/model/flow/order"
 	"github.com/onflow/flow-go/module/signature"
+	"github.com/onflow/flow-go/state"
 	"github.com/onflow/flow-go/utils/unittest"
 )
 
@@ -82,11 +84,12 @@ func (s *blockSignerDecoderSuite) Test_UnexpectedCommitteeException() {
 // It should propagate the sentinel error model.ErrViewForUnknownEpoch from Committee.
 func (s *blockSignerDecoderSuite) Test_UnknownEpoch() {
 	*s.committee = *hotstuff.NewDynamicCommittee(s.T())
-	s.committee.On("IdentitiesByEpoch", mock.Anything).Return(nil, model.ErrViewForUnknownEpoch)
+	s.committee.On("IdentitiesByEpoch", s.block.Header.ParentView).Return(nil, model.ErrViewForUnknownEpoch)
+	s.committee.On("IdentitiesByBlock", s.block.Header.ParentID).Return(nil, fmt.Errorf(""))
 
 	ids, err := s.decoder.DecodeSignerIDs(s.block.Header)
 	require.Empty(s.T(), ids)
-	require.ErrorIs(s.T(), err, model.ErrViewForUnknownEpoch)
+	require.Error(s.T(), err)
 }
 
 // Test_InvalidIndices verifies that `BlockSignerDecoder` returns