convert ChunkFault to an error type

Author: peterargue

engine/execution/computation/execution_verification_test.go

@@ -851,9 +851,8 @@ func executeBlockAndVerifyWithParameters(t *testing.T,
 	require.Len(t, vcds, len(txs)+1) // +1 for system chunk
 
 	for _, vcd := range vcds {
-		_, fault, err := verifier.Verify(vcd)
+		_, err := verifier.Verify(vcd)
 		assert.NoError(t, err)
-		assert.Nil(t, fault)
 	}
 
 	return computationResult

engine/verification/verifier/engine.go

@@ -188,47 +188,46 @@ func (e *Engine) verify(ctx context.Context, originID flow.Identifier,
 	// execute the assigned chunk
 	span, _ := e.tracer.StartSpanFromContext(ctx, trace.VERVerChunkVerify)
 
-	spockSecret, chFault, err := e.chVerif.Verify(vc)
+	spockSecret, err := e.chVerif.Verify(vc)
 	span.End()
-	// Any err means that something went wrong when verify the chunk
-	// the outcome of the verification is captured inside the chFault and not the err
+
 	if err != nil {
-		return fmt.Errorf("cannot verify chunk: %w", err)
-	}
+		if !chmodels.IsChunkFaultError(err) {
+			return fmt.Errorf("cannot verify chunk: %w", err)
+		}
 
-	// if any fault found with the chunk
-	if chFault != nil {
-		switch chFault.(type) {
+		// if any fault found with the chunk
+		switch chFault := err.(type) {
 		case *chmodels.CFMissingRegisterTouch:
 			e.log.Warn().
 				Str("chunk_fault_type", "missing_register_touch").
-				Str("chunk_fault", chFault.String()).
+				Str("chunk_fault", chFault.Error()).
 				Msg("chunk fault found, could not verify chunk")
 			// still create approvals for this case
 		case *chmodels.CFNonMatchingFinalState:
 			// TODO raise challenge
 			e.log.Warn().
 				Str("chunk_fault_type", "final_state_mismatch").
-				Str("chunk_fault", chFault.String()).
+				Str("chunk_fault", chFault.Error()).
 				Msg("chunk fault found, could not verify chunk")
 			return nil
 		case *chmodels.CFInvalidVerifiableChunk:
 			// TODO raise challenge
 			e.log.Error().
 				Str("chunk_fault_type", "invalid_verifiable_chunk").
-				Str("chunk_fault", chFault.String()).
+				Str("chunk_fault", chFault.Error()).
 				Msg("chunk fault found, could not verify chunk")
 			return nil
 		case *chmodels.CFInvalidEventsCollection:
 			// TODO raise challenge
 			e.log.Error().
 				Str("chunk_fault_type", "invalid_event_collection").
-				Str("chunk_fault", chFault.String()).
+				Str("chunk_fault", chFault.Error()).
 				Msg("chunk fault found, could not verify chunk")
 			return nil
 		default:
 			return engine.NewInvalidInputErrorf("unknown type of chunk fault is received (type: %T) : %v",
-				chFault, chFault.String())
+				chFault, chFault.Error())
 		}
 	}
 

engine/verification/verifier/engine_test.go

@@ -211,40 +211,40 @@ func (suite *VerifierEngineTestSuite) TestVerifyUnhappyPaths() {
 type ChunkVerifierMock struct {
 }
 
-func (v ChunkVerifierMock) Verify(vc *verification.VerifiableChunkData) ([]byte, chmodel.ChunkFault, error) {
+func (v ChunkVerifierMock) Verify(vc *verification.VerifiableChunkData) ([]byte, error) {
 	if vc.IsSystemChunk {
-		return nil, nil, nil
+		return nil, nil
 	}
 
 	switch vc.Chunk.Index {
 	case 0:
-		return []byte{}, nil, nil
+		return []byte{}, nil
 	// return error
 	case 1:
 		return nil, chmodel.NewCFMissingRegisterTouch(
 			[]string{"test missing register touch"},
 			vc.Chunk.Index,
 			vc.Result.ID(),
-			unittest.TransactionFixture().ID()), nil
+			unittest.TransactionFixture().ID())
 
 	case 2:
 		return nil, chmodel.NewCFInvalidVerifiableChunk(
 			"test",
 			errors.New("test invalid verifiable chunk"),
 			vc.Chunk.Index,
-			vc.Result.ID()), nil
+			vc.Result.ID())
 
 	case 3:
 		return nil, chmodel.NewCFNonMatchingFinalState(
 			unittest.StateCommitmentFixture(),
 			unittest.StateCommitmentFixture(),
 			vc.Chunk.Index,
-			vc.Result.ID()), nil
+			vc.Result.ID())
 
 	// TODO add cases for challenges
 	// return successful by default
 	default:
-		return nil, nil, nil
+		return nil, nil
 	}
 
 }

model/chunks/chunkFaults.go

@@ -2,19 +2,26 @@ package chunks
 
 import (
 	"encoding/hex"
+	"errors"
 	"fmt"
 
 	"github.com/onflow/flow-go/model/flow"
 )
 
-// ChunkFault holds information about a fault that is found while
+// ChunkFaultError holds information about a fault that is found while
 // verifying a chunk
-type ChunkFault interface {
+type ChunkFaultError interface {
+	error
 	ChunkIndex() uint64
 	ExecutionResultID() flow.Identifier
 	String() string
 }
 
+func IsChunkFaultError(err error) bool {
+	var cfErr ChunkFaultError
+	return errors.As(err, &cfErr)
+}
+
 // CFMissingRegisterTouch is returned when a register touch is missing (read or update)
 type CFMissingRegisterTouch struct {
 	regsterIDs []string
@@ -23,6 +30,8 @@ type CFMissingRegisterTouch struct {
 	txID       flow.Identifier // very first transaction inside the chunk that required this register
 }
 
+var _ ChunkFaultError = (*CFMissingRegisterTouch)(nil)
+
 func (cf CFMissingRegisterTouch) String() string {
 	hexStrings := make([]string, len(cf.regsterIDs))
 	for i, s := range cf.regsterIDs {
@@ -32,6 +41,10 @@ func (cf CFMissingRegisterTouch) String() string {
 	return fmt.Sprintf("at least one register touch was missing inside the chunk data package that was needed while running transactions of chunk %d of result %s (tx hash of one of them: %s), hex-encoded register ids: %s", cf.chunkIndex, cf.execResID.String(), cf.txID.String(), hexStrings)
 }
 
+func (cf CFMissingRegisterTouch) Error() string {
+	return cf.String()
+}
+
 // ChunkIndex returns chunk index of the faulty chunk
 func (cf CFMissingRegisterTouch) ChunkIndex() uint64 {
 	return cf.chunkIndex
@@ -59,10 +72,16 @@ type CFNonMatchingFinalState struct {
 	execResID  flow.Identifier
 }
 
+var _ ChunkFaultError = (*CFNonMatchingFinalState)(nil)
+
 func (cf CFNonMatchingFinalState) String() string {
 	return fmt.Sprintf("final state commitment doesn't match, expected [%x] but computed [%x]", cf.expected, cf.computed)
 }
 
+func (cf CFNonMatchingFinalState) Error() string {
+	return cf.String()
+}
+
 // ChunkIndex returns chunk index of the faulty chunk
 func (cf CFNonMatchingFinalState) ChunkIndex() uint64 {
 	return cf.chunkIndex
@@ -90,6 +109,8 @@ type CFInvalidEventsCollection struct {
 	eventIDs   flow.IdentifierList
 }
 
+var _ ChunkFaultError = (*CFInvalidEventsCollection)(nil)
+
 func NewCFInvalidEventsCollection(expected flow.Identifier, computed flow.Identifier, chInx uint64, execResID flow.Identifier, events flow.EventsList) *CFInvalidEventsCollection {
 	return &CFInvalidEventsCollection{
 		expected:   expected,
@@ -113,6 +134,10 @@ func (c *CFInvalidEventsCollection) String() string {
 		c.chunkIndex, c.resultID, c.eventIDs)
 }
 
+func (cf CFInvalidEventsCollection) Error() string {
+	return cf.String()
+}
+
 // CFInvalidServiceEventsEmitted is returned when service events are different from the chunk's one
 type CFInvalidServiceEventsEmitted struct {
 	expected   flow.ServiceEventList
@@ -121,6 +146,8 @@ type CFInvalidServiceEventsEmitted struct {
 	resultID   flow.Identifier
 }
 
+var _ ChunkFaultError = (*CFInvalidServiceEventsEmitted)(nil)
+
 func CFInvalidServiceSystemEventsEmitted(expected flow.ServiceEventList, computed flow.ServiceEventList, chInx uint64, execResID flow.Identifier) *CFInvalidServiceEventsEmitted {
 	return &CFInvalidServiceEventsEmitted{
 		expected:   expected,
@@ -142,6 +169,10 @@ func (c *CFInvalidServiceEventsEmitted) String() string {
 	return fmt.Sprintf("service events differs, got [%s] expected [%s] for chunk %d with result ID %s", c.computed, c.expected, c.chunkIndex, c.resultID)
 }
 
+func (cf CFInvalidServiceEventsEmitted) Error() string {
+	return cf.String()
+}
+
 // CFInvalidVerifiableChunk is returned when a verifiable chunk is invalid
 // this includes cases that code fails to construct a partial trie,
 // collection hashes doesn't match
@@ -153,10 +184,16 @@ type CFInvalidVerifiableChunk struct {
 	execResID  flow.Identifier
 }
 
+var _ ChunkFaultError = (*CFInvalidVerifiableChunk)(nil)
+
 func (cf CFInvalidVerifiableChunk) String() string {
 	return fmt.Sprint("invalid verifiable chunk due to ", cf.reason, cf.details.Error())
 }
 
+func (cf CFInvalidVerifiableChunk) Error() string {
+	return cf.String()
+}
+
 // ChunkIndex returns chunk index of the faulty chunk
 func (cf CFInvalidVerifiableChunk) ChunkIndex() uint64 {
 	return cf.chunkIndex

model/chunks/executionDataFaults.go

@@ -8,8 +8,6 @@ import (
 	"github.com/onflow/flow-go/model/flow"
 )
 
-var _ ChunkFault = (*CFExecutionDataBlockIDMismatch)(nil)
-
 // CFExecutionDataBlockIDMismatch is returned when the block ID contained in the execution data
 // root is different from chunk's block ID
 type CFExecutionDataBlockIDMismatch struct {
@@ -19,11 +17,17 @@ type CFExecutionDataBlockIDMismatch struct {
 	chunkBlockID             flow.Identifier // chunk's blockID
 }
 
+var _ ChunkFaultError = (*CFExecutionDataBlockIDMismatch)(nil)
+
 func (cf CFExecutionDataBlockIDMismatch) String() string {
 	return fmt.Sprintf("execution data root's block ID (%s) is different than chunk's block ID (%s) for chunk %d with result ID %s",
 		cf.executionDataRootBlockID, cf.chunkBlockID, cf.chunkIndex, cf.execResID.String())
 }
 
+func (cf CFExecutionDataBlockIDMismatch) Error() string {
+	return cf.String()
+}
+
 // ChunkIndex returns chunk index of the faulty chunk
 func (cf CFExecutionDataBlockIDMismatch) ChunkIndex() uint64 {
 	return cf.chunkIndex
@@ -49,8 +53,6 @@ func NewCFExecutionDataBlockIDMismatch(
 	}
 }
 
-var _ ChunkFault = (*CFExecutionDataChunksLengthMismatch)(nil)
-
 // CFExecutionDataChunksLengthMismatch is returned when execution data chunks list has different length than number of chunks for a block
 type CFExecutionDataChunksLengthMismatch struct {
 	chunkIndex                     uint64          // chunk's index
@@ -59,11 +61,17 @@ type CFExecutionDataChunksLengthMismatch struct {
 	executionResultChunkListLength int             // number of chunks in ExecutionResult
 }
 
+var _ ChunkFaultError = (*CFExecutionDataChunksLengthMismatch)(nil)
+
 func (cf CFExecutionDataChunksLengthMismatch) String() string {
 	return fmt.Sprintf("execution data root chunk length (%d) is different then execution result chunk list length (%d) for chunk %d with result ID %s",
 		cf.executionDataRootChunkLength, cf.executionResultChunkListLength, cf.chunkIndex, cf.execResID.String())
 }
 
+func (cf CFExecutionDataChunksLengthMismatch) Error() string {
+	return cf.String()
+}
+
 // ChunkIndex returns chunk index of the faulty chunk
 func (cf CFExecutionDataChunksLengthMismatch) ChunkIndex() uint64 {
 	return cf.chunkIndex
@@ -89,8 +97,6 @@ func NewCFExecutionDataChunksLengthMismatch(
 	}
 }
 
-var _ ChunkFault = (*CFExecutionDataInvalidChunkCID)(nil)
-
 // CFExecutionDataInvalidChunkCID is returned when execution data chunk's CID is different from computed
 type CFExecutionDataInvalidChunkCID struct {
 	chunkIndex                uint64          // chunk's index
@@ -99,11 +105,17 @@ type CFExecutionDataInvalidChunkCID struct {
 	computedChunkCID          cid.Cid         // computed CID for the chunk
 }
 
+var _ ChunkFaultError = (*CFExecutionDataInvalidChunkCID)(nil)
+
 func (cf CFExecutionDataInvalidChunkCID) String() string {
 	return fmt.Sprintf("execution data chunk CID (%s) is different then computed (%s) for chunk %d with result ID %s",
 		cf.executionDataRootChunkCID, cf.computedChunkCID, cf.chunkIndex, cf.execResID.String())
 }
 
+func (cf CFExecutionDataInvalidChunkCID) Error() string {
+	return cf.String()
+}
+
 // ChunkIndex returns chunk index of the faulty chunk
 func (cf CFExecutionDataInvalidChunkCID) ChunkIndex() uint64 {
 	return cf.chunkIndex
@@ -129,8 +141,6 @@ func NewCFExecutionDataInvalidChunkCID(
 	}
 }
 
-var _ ChunkFault = (*CFInvalidExecutionDataID)(nil)
-
 // CFInvalidExecutionDataID is returned when ExecutionResult's ExecutionDataID is different from computed
 type CFInvalidExecutionDataID struct {
 	chunkIndex              uint64          // chunk's index
@@ -139,11 +149,17 @@ type CFInvalidExecutionDataID struct {
 	computedExecutionDataID flow.Identifier // computed ExecutionDataID
 }
 
+var _ ChunkFaultError = (*CFInvalidExecutionDataID)(nil)
+
 func (cf CFInvalidExecutionDataID) String() string {
 	return fmt.Sprintf("execution data ID (%s) is different then computed (%s) for chunk %d with result ID %s",
 		cf.erExecutionDataID, cf.computedExecutionDataID, cf.chunkIndex, cf.execResID.String())
 }
 
+func (cf CFInvalidExecutionDataID) Error() string {
+	return cf.String()
+}
+
 // ChunkIndex returns chunk index of the faulty chunk
 func (cf CFInvalidExecutionDataID) ChunkIndex() uint64 {
 	return cf.chunkIndex

module/chunks.go

@@ -26,7 +26,6 @@ type ChunkVerifier interface {
 		ch *verification.VerifiableChunkData,
 	) (
 		[]byte,
-		chmodels.ChunkFault,
 		error,
 	)
 }