address PR comments, rename variables and functions

ChainSafe · Sep 13, 2024 · 25d95bd · 25d95bd
1 parent b8bbeb2
commit 25d95bd
Show file tree

Hide file tree

Showing 10 changed files with 67 additions and 60 deletions.
diff --git a/dot/parachain/backing/candidate_backing_test.go b/dot/parachain/backing/candidate_backing_test.go
@@ -731,7 +731,7 @@ func TestValidateAndMakeAvailable(t *testing.T) {
 						ci := candidatevalidation.ExecutionError
 						data.Ch <- parachaintypes.OverseerFuncRes[candidatevalidation.ValidationResult]{
 							Data: candidatevalidation.ValidationResult{
-								InvalidResult: &ci,
+								Invalid: &ci,
 							},
 						}
 					default:
@@ -768,7 +768,7 @@ func TestValidateAndMakeAvailable(t *testing.T) {
 					case candidatevalidation.ValidateFromExhaustive:
 						data.Ch <- parachaintypes.OverseerFuncRes[candidatevalidation.ValidationResult]{
 							Data: candidatevalidation.ValidationResult{
-								ValidResult: &candidatevalidation.Valid{},
+								Valid: &candidatevalidation.Valid{},
 							},
 						}
 					case availabilitystore.StoreAvailableData:

diff --git a/dot/parachain/backing/integration_test.go b/dot/parachain/backing/integration_test.go
@@ -227,7 +227,7 @@ func validResponseForValidateFromExhaustive(
 
 		msgValidate.Ch <- parachaintypes.OverseerFuncRes[candidatevalidation.ValidationResult]{
 			Data: candidatevalidation.ValidationResult{
-				ValidResult: &candidatevalidation.Valid{
+				Valid: &candidatevalidation.Valid{
 					CandidateCommitments: parachaintypes.CandidateCommitments{
 						HeadData:                  headData,
 						UpwardMessages:            []parachaintypes.UpwardMessage{},
@@ -337,7 +337,7 @@ func TestSecondsValidCandidate(t *testing.T) {
 		badReturn := candidatevalidation.BadReturn
 		validateFromExhaustive.Ch <- parachaintypes.OverseerFuncRes[candidatevalidation.ValidationResult]{
 			Data: candidatevalidation.ValidationResult{
-				InvalidResult: &badReturn,
+				Invalid: &badReturn,
 			},
 		}
 		return true

diff --git a/dot/parachain/backing/per_relay_parent_state.go b/dot/parachain/backing/per_relay_parent_state.go
@@ -340,8 +340,8 @@ func (rpState *perRelayParentState) validateAndMakeAvailable(
 			bgValidationResult = backgroundValidationResult{
 				outputs: &backgroundValidationOutputs{
 					candidateReceipt:        candidateReceipt,
-					candidateCommitments:    validationResultRes.Data.ValidResult.CandidateCommitments,
-					persistedValidationData: validationResultRes.Data.ValidResult.PersistedValidationData,
+					candidateCommitments:    validationResultRes.Data.Valid.CandidateCommitments,
+					persistedValidationData: validationResultRes.Data.Valid.PersistedValidationData,
 				},
 				candidate: nil,
 				err:       nil,
@@ -358,11 +358,11 @@ func (rpState *perRelayParentState) validateAndMakeAvailable(
 		}
 
 	} else { // Invalid
-		logger.Error(validationResultRes.Data.InvalidResult.Error())
+		logger.Error(validationResultRes.Data.Invalid.Error())
 		bgValidationResult = backgroundValidationResult{
 			outputs:   nil,
 			candidate: &candidateReceipt,
-			err:       fmt.Errorf(validationResultRes.Data.InvalidResult.Error()),
+			err:       fmt.Errorf(validationResultRes.Data.Invalid.Error()),
 		}
 	}
 

diff --git a/dot/parachain/candidate-validation/candidate_validation_test.go b/dot/parachain/candidate-validation/candidate_validation_test.go
@@ -164,7 +164,7 @@ func TestCandidateValidation_processMessageValidateFromExhaustive(t *testing.T)
 			},
 			want: parachaintypes.OverseerFuncRes[ValidationResult]{
 				Data: ValidationResult{
-					InvalidResult: &povHashMismatch,
+					Invalid: &povHashMismatch,
 				},
 				Err: nil,
 			},
@@ -184,7 +184,7 @@ func TestCandidateValidation_processMessageValidateFromExhaustive(t *testing.T)
 			},
 			want: parachaintypes.OverseerFuncRes[ValidationResult]{
 				Data: ValidationResult{
-					InvalidResult: &paramsTooLarge,
+					Invalid: &paramsTooLarge,
 				},
 			},
 		},
@@ -203,7 +203,7 @@ func TestCandidateValidation_processMessageValidateFromExhaustive(t *testing.T)
 			},
 			want: parachaintypes.OverseerFuncRes[ValidationResult]{
 				Data: ValidationResult{
-					InvalidResult: &codeHashMismatch,
+					Invalid: &codeHashMismatch,
 				},
 			},
 		},
@@ -222,7 +222,7 @@ func TestCandidateValidation_processMessageValidateFromExhaustive(t *testing.T)
 			},
 			want: parachaintypes.OverseerFuncRes[ValidationResult]{
 				Data: ValidationResult{
-					ValidResult: &Valid{
+					Valid: &Valid{
 						CandidateCommitments: parachaintypes.CandidateCommitments{
 							HeadData: parachaintypes.HeadData{Data: []byte{2, 0, 0, 0, 0, 0, 0, 0, 123,
 								207, 206, 8, 219, 227, 136, 82, 236, 169, 14, 100, 45, 100, 31, 177, 154, 160, 220, 245,
@@ -388,7 +388,7 @@ func TestCandidateValidation_processMessageValidateFromChainState(t *testing.T)
 				Pov:              pov,
 			},
 			want: &ValidationResult{
-				InvalidResult: &povHashMismatch,
+				Invalid: &povHashMismatch,
 			},
 		},
 		"invalid_pov_size": {
@@ -397,7 +397,7 @@ func TestCandidateValidation_processMessageValidateFromChainState(t *testing.T)
 				Pov:              pov,
 			},
 			want: &ValidationResult{
-				InvalidResult: &paramsTooLarge,
+				Invalid: &paramsTooLarge,
 			},
 		},
 		"code_mismatch": {
@@ -406,7 +406,7 @@ func TestCandidateValidation_processMessageValidateFromChainState(t *testing.T)
 				Pov:              pov,
 			},
 			want: &ValidationResult{
-				InvalidResult: &codeHashMismatch,
+				Invalid: &codeHashMismatch,
 			},
 		},
 		"bad_signature": {
@@ -415,7 +415,7 @@ func TestCandidateValidation_processMessageValidateFromChainState(t *testing.T)
 				Pov:              pov,
 			},
 			want: &ValidationResult{
-				InvalidResult: &badSignature,
+				Invalid: &badSignature,
 			},
 		},
 		"happy_path": {
@@ -424,7 +424,7 @@ func TestCandidateValidation_processMessageValidateFromChainState(t *testing.T)
 				Pov:              pov,
 			},
 			want: &ValidationResult{
-				ValidResult: &Valid{
+				Valid: &Valid{
 					CandidateCommitments: parachaintypes.CandidateCommitments{
 						HeadData: parachaintypes.HeadData{Data: []byte{2, 0, 0, 0, 0, 0, 0, 0, 123, 207, 206, 8, 219, 227,
 							136, 82, 236, 169, 14, 100, 45, 100, 31, 177, 154, 160, 220, 245, 59, 106, 76, 168, 122, 109,

diff --git a/dot/parachain/candidate-validation/host.go b/dot/parachain/candidate-validation/host.go
@@ -8,7 +8,7 @@ import (
 	"github.com/ChainSafe/gossamer/pkg/scale"
 )
 
-var logger = log.NewFromGlobal(log.AddContext("pkg", "pvf"), log.SetLevel(log.Debug))
+var logger = log.NewFromGlobal(log.AddContext("pkg", "candidatevalidation"), log.SetLevel(log.Debug))
 
 // host is the struct that holds the workerPool which is responsible for executing the validation tasks
 type host struct {
@@ -17,13 +17,12 @@ type host struct {
 
 func newValidationHost() *host {
 	return &host{
-		workerPool: newValidationWorkerPool(),
+		workerPool: newWorkerPool(),
 	}
 }
 
 func (v *host) validate(msg *ValidationTask) (*ValidationResult, error) {
 	validationCodeHash := msg.ValidationCode.Hash()
-	// performBasicChecks
 	validationErr, internalErr := performBasicChecks(&msg.CandidateReceipt.Descriptor,
 		msg.PersistedValidationData.MaxPovSize,
 		msg.PoV,
@@ -33,15 +32,15 @@ func (v *host) validate(msg *ValidationTask) (*ValidationResult, error) {
 		return nil, internalErr
 	}
 	if validationErr != nil {
-		return &ValidationResult{InvalidResult: validationErr}, nil //nolint
+		return &ValidationResult{Invalid: validationErr}, nil //nolint
 	}
 
 	// submit request
 	return v.workerPool.executeRequest(msg)
 }
 
-// performBasicChecks Does basic checks of a candidate. Provide the encoded PoV-block.
-// Returns ReasonForInvalidity and internal error if any.
+// performBasicChecks does basic checks of a candidate. Provided the encoded PoV-block it returns ReasonForInvalidity
+// and internal error if any.
 func performBasicChecks(candidate *parachaintypes.CandidateDescriptor, maxPoVSize uint32,
 	pov parachaintypes.PoV, validationCodeHash parachaintypes.ValidationCodeHash) (
 	validationError *ReasonForInvalidity, internalError error) {
@@ -76,5 +75,6 @@ func performBasicChecks(candidate *parachaintypes.CandidateDescriptor, maxPoVSiz
 		ci := BadSignature
 		return &ci, nil
 	}
+
 	return nil, nil
 }
diff --git a/dot/parachain/candidate-validation/host_test.go b/dot/parachain/candidate-validation/host_test.go
@@ -74,7 +74,7 @@ func TestHost_validate(t *testing.T) {
 				ValidationCode:     &validationCode,
 			},
 			want: &ValidationResult{
-				InvalidResult: &povHashMismatch,
+				Invalid: &povHashMismatch,
 			},
 			isValid: false,
 		},
@@ -91,7 +91,7 @@ func TestHost_validate(t *testing.T) {
 				PoV:              pov,
 			},
 			want: &ValidationResult{
-				InvalidResult: &paramsTooLarge,
+				Invalid: &paramsTooLarge,
 			},
 		},
 		"code_mismatch": {
@@ -107,7 +107,7 @@ func TestHost_validate(t *testing.T) {
 				PoV:              pov,
 			},
 			want: &ValidationResult{
-				InvalidResult: &codeHashMismatch,
+				Invalid: &codeHashMismatch,
 			},
 			isValid: false,
 		},
@@ -121,7 +121,7 @@ func TestHost_validate(t *testing.T) {
 				PoV:              pov,
 			},
 			want: &ValidationResult{
-				InvalidResult: &executionError,
+				Invalid: &executionError,
 			},
 		},
 		"para_head_hash_mismatch": {
@@ -137,7 +137,7 @@ func TestHost_validate(t *testing.T) {
 				PoV:              pov,
 			},
 			want: &ValidationResult{
-				InvalidResult: &paraHedHashMismatch,
+				Invalid: &paraHedHashMismatch,
 			},
 			isValid: false,
 		},
@@ -154,7 +154,7 @@ func TestHost_validate(t *testing.T) {
 				PoV:              pov,
 			},
 			want: &ValidationResult{
-				InvalidResult: &commitmentsHashMismatch,
+				Invalid: &commitmentsHashMismatch,
 			},
 			isValid: false,
 		},
@@ -171,7 +171,7 @@ func TestHost_validate(t *testing.T) {
 				PoV:              pov,
 			},
 			want: &ValidationResult{
-				ValidResult: &Valid{
+				Valid: &Valid{
 					CandidateCommitments: parachaintypes.CandidateCommitments{
 						UpwardMessages:     nil,
 						HorizontalMessages: nil,
@@ -323,7 +323,7 @@ func TestHost_performBasicChecks(t *testing.T) {
 			if tt.expectedError != nil {
 				require.EqualError(t, validationError, tt.expectedError.Error())
 			} else {
-				require.Nil(t, validationError)
+				require.NoError(t, validationError)
 			}
 		})
 	}

diff --git a/dot/parachain/candidate-validation/worker.go b/dot/parachain/candidate-validation/worker.go
@@ -5,9 +5,10 @@ import (
 	parachaintypes "github.com/ChainSafe/gossamer/dot/parachain/types"
 )
 
+// worker is the thing that can execute a validation request
 type worker struct {
 	workerID    parachaintypes.ValidationCodeHash
-	instance    *parachainruntime.Instance
+	instance    parachainruntime.ValidatorInstance
 	isProcessed map[parachaintypes.CandidateHash]*ValidationResult
 }
 
@@ -18,14 +19,15 @@ type workerTask struct {
 }
 
 func newWorker(validationCode parachaintypes.ValidationCode) (*worker, error) {
-	validationRuntime, err := parachainruntime.SetupVM(validationCode)
+	parachainRuntime, err := parachainruntime.SetupVM(validationCode)
 
 	if err != nil {
 		return nil, err
 	}
+
 	return &worker{
 		workerID:    validationCode.Hash(),
-		instance:    validationRuntime,
+		instance:    parachainRuntime,
 		isProcessed: make(map[parachaintypes.CandidateHash]*ValidationResult),
 	}, nil
 }
@@ -37,7 +39,6 @@ func (w *worker) executeRequest(task *workerTask) (*ValidationResult, error) {
 		return nil, err
 	}
 
-	// do isProcessed check here
 	if processed, ok := w.isProcessed[candidateHash]; ok {
 		logger.Debugf("candidate %x already processed", candidateHash)
 		return processed, nil
@@ -46,19 +47,19 @@ func (w *worker) executeRequest(task *workerTask) (*ValidationResult, error) {
 	if err != nil {
 		logger.Errorf("executing validate_block: %w", err)
 		reasonForInvalidity := ExecutionError
-		return &ValidationResult{InvalidResult: &reasonForInvalidity}, nil
+		return &ValidationResult{Invalid: &reasonForInvalidity}, nil
 	}
 
 	headDataHash, err := validationResult.HeadData.Hash()
 	if err != nil {
 		logger.Errorf("hashing head data: %w", err)
 		reasonForInvalidity := ExecutionError
-		return &ValidationResult{InvalidResult: &reasonForInvalidity}, nil
+		return &ValidationResult{Invalid: &reasonForInvalidity}, nil
 	}
 
 	if headDataHash != task.candidateReceipt.Descriptor.ParaHead {
 		reasonForInvalidity := ParaHeadHashMismatch
-		return &ValidationResult{InvalidResult: &reasonForInvalidity}, nil
+		return &ValidationResult{Invalid: &reasonForInvalidity}, nil
 	}
 	candidateCommitments := parachaintypes.CandidateCommitments{
 		UpwardMessages:            validationResult.UpwardMessages,
@@ -72,7 +73,7 @@ func (w *worker) executeRequest(task *workerTask) (*ValidationResult, error) {
 	// if validation produced a new set of commitments, we treat the candidate as invalid
 	if task.candidateReceipt.CommitmentsHash != candidateCommitments.Hash() {
 		reasonForInvalidity := CommitmentsHashMismatch
-		return &ValidationResult{InvalidResult: &reasonForInvalidity}, nil
+		return &ValidationResult{Invalid: &reasonForInvalidity}, nil
 	}
 	pvd := parachaintypes.PersistedValidationData{
 		ParentHead:             task.work.ParentHeadData,
@@ -81,7 +82,7 @@ func (w *worker) executeRequest(task *workerTask) (*ValidationResult, error) {
 		MaxPovSize:             task.maxPoVSize,
 	}
 	result := &ValidationResult{
-		ValidResult: &Valid{
+		Valid: &Valid{
 			CandidateCommitments:    candidateCommitments,
 			PersistedValidationData: pvd,
 		},