diff --git a/state/protocol/badger/mutator.go b/state/protocol/badger/mutator.go index 07bc9eaceb2..f54c6ccb82f 100644 --- a/state/protocol/badger/mutator.go +++ b/state/protocol/badger/mutator.go @@ -549,8 +549,9 @@ func (m *ParticipantState) checkOutdatedExtension(header flow.HeaderBody) error } // guaranteeExtend verifies the validity of the collection guarantees that are -// included in the block. Specifically, we check for expired collections and -// duplicated collections (also including ancestor blocks). +// included in the block. Specifically, we check for expired guarantees, duplicated +// guarantees (also including ancestor blocks), and conflicting guarantees for the same +// collection (a different guarantee already indexed for a collection referenced in the payload). // Expected errors during normal operations: // - state.InvalidExtensionError if the candidate block contains invalid collection guarantees func (m *ParticipantState) guaranteeExtend(ctx context.Context, candidate *flow.Block) error { @@ -593,6 +594,11 @@ func (m *ParticipantState) guaranteeExtend(ctx context.Context, candidate *flow. ancestorID = ancestor.ParentID } + // Track the guarantee ID included for each collection within this payload, so that we can reject a + // block that references the same collection more than once — whether via an identical or a + // conflicting guarantee (see the collection-uniqueness check below). + guaranteesByCollection := make(map[flow.Identifier]flow.Identifier, len(payload.Guarantees)) + // check each guarantee included in the payload for duplication and expiry for _, guarantee := range payload.Guarantees { @@ -627,6 +633,42 @@ func (m *ParticipantState) guaranteeExtend(ctx context.Context, candidate *flow. } return fmt.Errorf("could not find guarantor for guarantee %v: %w", guarantee.ID(), err) } + + // A collection may be guaranteed at most once. We enforce this here for two scopes: + // + // 1. Within this payload: the same collection must not appear twice, whether via an identical + // guarantee (double execution / double fee charge) or via two *different* guarantees for the + // same collection (conflicting guarantees). The duplicate check above only inspects ancestor + // blocks, never the candidate's own guarantees, so it does not catch within-payload repeats. + // + // 2. Against all previously persisted guarantees: the storage layer enforces a single guarantee ID + // per collection ID globally, across all forks (see operation.IndexGuarantee). A *different* + // guarantee for an already-guaranteed collection therefore cannot be persisted. + // We reject it here as a typed InvalidExtensionError instead. An identical guarantee already + // persisted on another fork is legitimate (it re-indexes to the same value), so at this scope + // only a differing guarantee ID is a conflict. + // + // This check is performed last, so that a guarantee that is malformed for a more fundamental reason + // (e.g. invalid guarantors or an expired reference block) is reported with that more specific reason. + if priorGuaranteeID, ok := guaranteesByCollection[guarantee.CollectionID]; ok { + if priorGuaranteeID == guarantee.ID() { + return state.NewInvalidExtensionErrorf("payload includes duplicate guarantee (%x) for collection %x", + guarantee.ID(), guarantee.CollectionID) + } + return state.NewInvalidExtensionErrorf("payload includes conflicting guarantees (%x and %x) for the same collection %x", + priorGuaranteeID, guarantee.ID(), guarantee.CollectionID) + } + var indexedGuaranteeID flow.Identifier + err = operation.LookupGuarantee(m.db.Reader(), guarantee.CollectionID, &indexedGuaranteeID) + if err == nil { + if indexedGuaranteeID != guarantee.ID() { + return state.NewInvalidExtensionErrorf("payload includes guarantee %x for collection %x that is already guaranteed by a different guarantee %x", + guarantee.ID(), guarantee.CollectionID, indexedGuaranteeID) + } + } else if !errors.Is(err, storage.ErrNotFound) { + return fmt.Errorf("could not look up existing guarantee for collection %x: %w", guarantee.CollectionID, err) + } + guaranteesByCollection[guarantee.CollectionID] = guarantee.ID() } return nil diff --git a/state/protocol/badger/mutator_test.go b/state/protocol/badger/mutator_test.go index eb9c7a2df07..3d629dfa6f6 100644 --- a/state/protocol/badger/mutator_test.go +++ b/state/protocol/badger/mutator_test.go @@ -3361,6 +3361,185 @@ func TestExtendInvalidGuarantee(t *testing.T) { }) } +// TestExtendConflictingGuaranteeSameCollection verifies that a block carrying a guarantee for a +// collection that a different guarantee was already ingested for is rejected as a typed +// state.InvalidExtensionError, rather than crashing the node. +// +// The storage layer (operation.IndexGuarantee) enforces one guarantee per collection globally. +// A proposer can include a guarantee for an already-guaranteed collection by changing the +// signature — producing a fresh guarantee.ID() that slips past the ancestor-dedup check (which +// is keyed by guarantee ID). Without an early check in guaranteeExtend, the conflict surfaces +// later as an untyped storage.ErrDataMismatch from the deferred storage batch. The compliance +// engine treats any unrecognized error from state.Extend as a fatal exception and crashes the +// node. The fix detects the conflict in guaranteeExtend and rejects it as InvalidExtensionError. +func TestExtendConflictingGuaranteeSameCollection(t *testing.T) { + rootSnapshot := unittest.RootSnapshotFixture(participants) + rootProtocolStateID := getRootProtocolStateID(t, rootSnapshot) + util.RunWithFullProtocolState(t, rootSnapshot, func(db storage.DB, state *protocol.ParticipantState) { + head, err := rootSnapshot.Head() + require.NoError(t, err) + + cluster, err := unittest.SnapshotClusterByIndex(rootSnapshot, 0) + require.NoError(t, err) + + all := cluster.Members().NodeIDs() + validSignerIndices, err := signature.EncodeSignersToIndices(all, all) + require.NoError(t, err) + + usedViews := make(map[uint64]struct{}) + usedViews[head.View] = struct{}{} + + collectionID := unittest.IdentifierFixture() + + // G1: a valid guarantee for collection C. + g1 := &flow.CollectionGuarantee{ + CollectionID: collectionID, + ClusterChainID: cluster.ChainID(), + ReferenceBlockID: head.ID(), + SignerIndices: validSignerIndices, + } + + block1 := unittest.BlockWithParentAndPayloadAndUniqueView( + head, + flow.Payload{ + Guarantees: []*flow.CollectionGuarantee{g1}, + ProtocolStateID: rootProtocolStateID, + }, + usedViews, + ) + err = state.Extend(context.Background(), unittest.ProposalFromBlock(block1)) + require.NoError(t, err) + + // G2: a guarantee for the same collection C with a mutated Signature, yielding a fresh + // guarantee.ID(). Guarantee signatures are not verified by consensus nodes, so this is + // sufficient to produce a distinct ID while keeping all other fields valid. + g2 := &flow.CollectionGuarantee{ + CollectionID: collectionID, + ClusterChainID: cluster.ChainID(), + ReferenceBlockID: head.ID(), + SignerIndices: validSignerIndices, + Signature: unittest.SignatureFixture(), + } + require.NotEqual(t, g1.ID(), g2.ID(), + "mutating the guarantee signature must yield a fresh guarantee ID") + + block2 := unittest.BlockWithParentAndPayloadAndUniqueView( + block1.ToHeader(), + flow.Payload{ + Guarantees: []*flow.CollectionGuarantee{g2}, + ProtocolStateID: rootProtocolStateID, + }, + usedViews, + ) + err = state.Extend(context.Background(), unittest.ProposalFromBlock(block2)) + + require.Error(t, err) + require.True(t, st.IsInvalidExtensionError(err), + "conflicting guarantee for an already-guaranteed collection must be rejected as InvalidExtensionError, got: %v", err) + require.NotErrorIs(t, err, storage.ErrDataMismatch, + "storage sentinel must not escape Extend") + }) +} + +// TestExtendConflictingGuaranteesWithinSamePayload verifies that a block carrying two different +// guarantees for the same collection within a single payload is rejected as InvalidExtensionError. +func TestExtendConflictingGuaranteesWithinSamePayload(t *testing.T) { + rootSnapshot := unittest.RootSnapshotFixture(participants) + rootProtocolStateID := getRootProtocolStateID(t, rootSnapshot) + util.RunWithFullProtocolState(t, rootSnapshot, func(db storage.DB, state *protocol.ParticipantState) { + head, err := rootSnapshot.Head() + require.NoError(t, err) + + cluster, err := unittest.SnapshotClusterByIndex(rootSnapshot, 0) + require.NoError(t, err) + + all := cluster.Members().NodeIDs() + validSignerIndices, err := signature.EncodeSignersToIndices(all, all) + require.NoError(t, err) + + usedViews := make(map[uint64]struct{}) + usedViews[head.View] = struct{}{} + + collectionID := unittest.IdentifierFixture() + + g1 := &flow.CollectionGuarantee{ + CollectionID: collectionID, + ClusterChainID: cluster.ChainID(), + ReferenceBlockID: head.ID(), + SignerIndices: validSignerIndices, + } + g2 := &flow.CollectionGuarantee{ + CollectionID: collectionID, + ClusterChainID: cluster.ChainID(), + ReferenceBlockID: head.ID(), + SignerIndices: validSignerIndices, + Signature: unittest.SignatureFixture(), + } + require.NotEqual(t, g1.ID(), g2.ID()) + + block := unittest.BlockWithParentAndPayloadAndUniqueView( + head, + flow.Payload{ + Guarantees: []*flow.CollectionGuarantee{g1, g2}, + ProtocolStateID: rootProtocolStateID, + }, + usedViews, + ) + err = state.Extend(context.Background(), unittest.ProposalFromBlock(block)) + + require.Error(t, err) + require.True(t, st.IsInvalidExtensionError(err), + "two conflicting guarantees for the same collection in one payload must be rejected as InvalidExtensionError, got: %v", err) + require.NotErrorIs(t, err, storage.ErrDataMismatch) + }) +} + +// TestExtendDuplicateGuaranteeWithinSamePayload verifies that a block carrying the same guarantee +// twice within a single payload is rejected as InvalidExtensionError. The ancestor-based +// duplicate check does not cover within-payload repeats. +func TestExtendDuplicateGuaranteeWithinSamePayload(t *testing.T) { + rootSnapshot := unittest.RootSnapshotFixture(participants) + rootProtocolStateID := getRootProtocolStateID(t, rootSnapshot) + util.RunWithFullProtocolState(t, rootSnapshot, func(db storage.DB, state *protocol.ParticipantState) { + head, err := rootSnapshot.Head() + require.NoError(t, err) + + cluster, err := unittest.SnapshotClusterByIndex(rootSnapshot, 0) + require.NoError(t, err) + + all := cluster.Members().NodeIDs() + validSignerIndices, err := signature.EncodeSignersToIndices(all, all) + require.NoError(t, err) + + usedViews := make(map[uint64]struct{}) + usedViews[head.View] = struct{}{} + + guarantee := &flow.CollectionGuarantee{ + CollectionID: unittest.IdentifierFixture(), + ClusterChainID: cluster.ChainID(), + ReferenceBlockID: head.ID(), + SignerIndices: validSignerIndices, + } + + block := unittest.BlockWithParentAndPayloadAndUniqueView( + head, + flow.Payload{ + Guarantees: []*flow.CollectionGuarantee{guarantee, guarantee}, // [G, G] + ProtocolStateID: rootProtocolStateID, + }, + usedViews, + ) + err = state.Extend(context.Background(), unittest.ProposalFromBlock(block)) + + require.Error(t, err) + require.True(t, st.IsInvalidExtensionError(err), + "identical guarantee twice in one payload must be rejected as InvalidExtensionError, got: %v", err) + + _, err = state.AtBlockID(block.ID()).Head() + require.Error(t, err) + }) +} + // If block B is finalized and contains a seal for block A, then A is the last sealed block func TestSealed(t *testing.T) { rootSnapshot := unittest.RootSnapshotFixture(participants)