address PR comments

ChainSafe · Sep 11, 2024 · b8bbeb2 · b8bbeb2
1 parent 020fb6f
commit b8bbeb2
Show file tree

Hide file tree

Showing 6 changed files with 10 additions and 11 deletions.
diff --git a/dot/parachain/candidate-validation/candidate_validation.go b/dot/parachain/candidate-validation/candidate_validation.go
@@ -18,7 +18,7 @@ import (
 type CandidateValidation struct {
 	SubsystemToOverseer chan<- any
 	BlockState          BlockState
-	pvfHost             *host
+	pvfHost             *host // pvfHost is the host for the parachain validation function
 }
 
 type BlockState interface {

diff --git a/dot/parachain/candidate-validation/candidate_validation_test.go b/dot/parachain/candidate-validation/candidate_validation_test.go
@@ -368,7 +368,6 @@ func TestCandidateValidation_processMessageValidateFromChainState(t *testing.T)
 		BlockData: bd,
 	}
 
-	sender := make(chan parachaintypes.OverseerFuncRes[ValidationResult])
 	toSubsystem := make(chan any)
 	candidateValidationSubsystem := CandidateValidation{
 		pvfHost:    newValidationHost(),
@@ -387,7 +386,6 @@ func TestCandidateValidation_processMessageValidateFromChainState(t *testing.T)
 			msg: ValidateFromChainState{
 				CandidateReceipt: candidateReceipt2,
 				Pov:              pov,
-				Ch:               sender,
 			},
 			want: &ValidationResult{
 				InvalidResult: &povHashMismatch,
@@ -397,7 +395,6 @@ func TestCandidateValidation_processMessageValidateFromChainState(t *testing.T)
 			msg: ValidateFromChainState{
 				CandidateReceipt: candidateReceipt3,
 				Pov:              pov,
-				Ch:               sender,
 			},
 			want: &ValidationResult{
 				InvalidResult: &paramsTooLarge,
@@ -407,7 +404,6 @@ func TestCandidateValidation_processMessageValidateFromChainState(t *testing.T)
 			msg: ValidateFromChainState{
 				CandidateReceipt: candidateReceipt4,
 				Pov:              pov,
-				Ch:               sender,
 			},
 			want: &ValidationResult{
 				InvalidResult: &codeHashMismatch,
@@ -417,7 +413,6 @@ func TestCandidateValidation_processMessageValidateFromChainState(t *testing.T)
 			msg: ValidateFromChainState{
 				CandidateReceipt: candidateReceipt5,
 				Pov:              pov,
-				Ch:               sender,
 			},
 			want: &ValidationResult{
 				InvalidResult: &badSignature,
@@ -427,7 +422,6 @@ func TestCandidateValidation_processMessageValidateFromChainState(t *testing.T)
 			msg: ValidateFromChainState{
 				CandidateReceipt: candidateReceipt,
 				Pov:              pov,
-				Ch:               sender,
 			},
 			want: &ValidationResult{
 				ValidResult: &Valid{
@@ -459,6 +453,9 @@ func TestCandidateValidation_processMessageValidateFromChainState(t *testing.T)
 		t.Run(name, func(t *testing.T) {
 			t.Parallel()
 
+			sender := make(chan parachaintypes.OverseerFuncRes[ValidationResult])
+			tt.msg.Ch = sender
+
 			toSubsystem <- tt.msg
 			result := <-sender
 			require.Equal(t, tt.want, &result.Data)

diff --git a/dot/parachain/candidate-validation/host.go b/dot/parachain/candidate-validation/host.go
@@ -10,6 +10,7 @@ import (
 
 var logger = log.NewFromGlobal(log.AddContext("pkg", "pvf"), log.SetLevel(log.Debug))
 
+// host is the struct that holds the workerPool which is responsible for executing the validation tasks
 type host struct {
 	workerPool *workerPool
 }

diff --git a/dot/parachain/candidate-validation/host_test.go b/dot/parachain/candidate-validation/host_test.go
@@ -214,6 +214,7 @@ func TestHost_validate(t *testing.T) {
 }
 
 func TestHost_performBasicChecks(t *testing.T) {
+	t.Parallel()
 	paramsTooLarge := ParamsTooLarge
 	povHashMismatch := PoVHashMismatch
 	codeHashMismatch := CodeHashMismatch
@@ -313,9 +314,12 @@ func TestHost_performBasicChecks(t *testing.T) {
 		},
 	}
 	for name, tt := range tests {
+		tt := tt
 		t.Run(name, func(t *testing.T) {
-			validationError, _ := performBasicChecks(tt.args.candidate, tt.args.maxPoVSize, tt.args.pov,
+			t.Parallel()
+			validationError, internalError := performBasicChecks(tt.args.candidate, tt.args.maxPoVSize, tt.args.pov,
 				tt.args.validationCodeHash)
+			require.NoError(t, internalError)
 			if tt.expectedError != nil {
 				require.EqualError(t, validationError, tt.expectedError.Error())
 			} else {

diff --git a/dot/parachain/candidate-validation/worker.go b/dot/parachain/candidate-validation/worker.go
@@ -5,7 +5,6 @@ import (
 	parachaintypes "github.com/ChainSafe/gossamer/dot/parachain/types"
 )
 
-// TODO(ed): figure out a better name for this that describes what it does
 type worker struct {
 	workerID    parachaintypes.ValidationCodeHash
 	instance    *parachainruntime.Instance

diff --git a/dot/parachain/candidate-validation/worker_pool.go b/dot/parachain/candidate-validation/worker_pool.go
@@ -8,8 +8,6 @@ import (
 )
 
 type workerPool struct {
-
-	// todo, make sure other functions work with paraID
 	workers map[parachaintypes.ValidationCodeHash]*worker
 }