address PR comments

dot/parachain/backing/candidate_backing_test.go

@@ -768,7 +768,7 @@ func TestValidateAndMakeAvailable(t *testing.T) {
 					case candidatevalidation.ValidateFromExhaustive:
 						data.Ch <- parachaintypes.OverseerFuncRes[candidatevalidation.ValidationResult]{
 							Data: candidatevalidation.ValidationResult{
-								ValidResult: &candidatevalidation.ValidValidationResult{},
+								ValidResult: &candidatevalidation.Valid{},
 							},
 						}
 					case availabilitystore.StoreAvailableData:

dot/parachain/backing/integration_test.go

@@ -227,7 +227,7 @@ func validResponseForValidateFromExhaustive(
 
 		msgValidate.Ch <- parachaintypes.OverseerFuncRes[candidatevalidation.ValidationResult]{
 			Data: candidatevalidation.ValidationResult{
-				ValidResult: &candidatevalidation.ValidValidationResult{
+				ValidResult: &candidatevalidation.Valid{
 					CandidateCommitments: parachaintypes.CandidateCommitments{
 						HeadData:                  headData,
 						UpwardMessages:            []parachaintypes.UpwardMessage{},

dot/parachain/candidate-validation/candidate_validation_test.go

@@ -222,7 +222,7 @@ func TestCandidateValidation_processMessageValidateFromExhaustive(t *testing.T)
 			},
 			want: parachaintypes.OverseerFuncRes[ValidationResult]{
 				Data: ValidationResult{
-					ValidResult: &ValidValidationResult{
+					ValidResult: &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,
@@ -430,7 +430,7 @@ func TestCandidateValidation_processMessageValidateFromChainState(t *testing.T)
 				Ch:               sender,
 			},
 			want: &ValidationResult{
-				ValidResult: &ValidValidationResult{
+				ValidResult: &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,

dot/parachain/candidate-validation/host.go

@@ -3,7 +3,6 @@ package candidatevalidation
 import (
 	"fmt"
 
-	parachainruntime "github.com/ChainSafe/gossamer/dot/parachain/runtime"
 	parachaintypes "github.com/ChainSafe/gossamer/dot/parachain/types"
 	"github.com/ChainSafe/gossamer/internal/log"
 	"github.com/ChainSafe/gossamer/pkg/scale"
@@ -35,32 +34,9 @@ func (v *Host) Validate(msg *ValidationTask) (*ValidationResult, error) {
 	if validationErr != nil {
 		return &ValidationResult{InvalidResult: validationErr}, nil //nolint
 	}
-	// create worker if not in pool
-	if !v.workerPool.containsWorker(validationCodeHash) {
-		worker, err := v.workerPool.newValidationWorker(*msg.ValidationCode)
-		if err != nil {
-			return nil, err
-		}
-
-		// sanity check
-		if worker.workerID != validationCodeHash {
-			return nil, fmt.Errorf("workerID does not match validationCodeHash")
-		}
-	}
 
 	// submit request
-	validationParams := parachainruntime.ValidationParameters{
-		ParentHeadData:         msg.PersistedValidationData.ParentHead,
-		BlockData:              msg.PoV.BlockData,
-		RelayParentNumber:      msg.PersistedValidationData.RelayParentNumber,
-		RelayParentStorageRoot: msg.PersistedValidationData.RelayParentStorageRoot,
-	}
-	workTask := &workerTask{
-		work:             validationParams,
-		maxPoVSize:       msg.PersistedValidationData.MaxPovSize,
-		candidateReceipt: msg.CandidateReceipt,
-	}
-	return v.workerPool.submitRequest(validationCodeHash, workTask)
+	return v.workerPool.submitRequest(msg)
 }
 
 // performBasicChecks Does basic checks of a candidate. Provide the encoded PoV-block.

dot/parachain/candidate-validation/host_test.go

@@ -171,7 +171,7 @@ func TestHost_validate(t *testing.T) {
 				PoV:              pov,
 			},
 			want: &ValidationResult{
-				ValidResult: &ValidValidationResult{
+				ValidResult: &Valid{
 					CandidateCommitments: parachaintypes.CandidateCommitments{
 						UpwardMessages:     nil,
 						HorizontalMessages: nil,

dot/parachain/candidate-validation/worker.go

@@ -7,10 +7,9 @@ import (
 
 // TODO(ed): figure out a better name for this that describes what it does
 type worker struct {
-	workerID parachaintypes.ValidationCodeHash
-	instance *parachainruntime.Instance
-	// TODO(ed): determine if wasProcessed is stored here or in host
-	isProcessed map[parachaintypes.CandidateHash]struct{}
+	workerID    parachaintypes.ValidationCodeHash
+	instance    *parachainruntime.Instance
+	isProcessed map[parachaintypes.CandidateHash]*ValidationResult
 }
 
 type workerTask struct {
@@ -26,8 +25,9 @@ func newWorker(validationCode parachaintypes.ValidationCode) (*worker, error) {
 		return nil, err
 	}
 	return &worker{
-		workerID: validationCode.Hash(),
-		instance: validationRuntime,
+		workerID:    validationCode.Hash(),
+		instance:    validationRuntime,
+		isProcessed: make(map[parachaintypes.CandidateHash]*ValidationResult),
 	}, nil
 }
 
@@ -39,10 +39,9 @@ func (w *worker) executeRequest(task *workerTask) (*ValidationResult, error) {
 	}
 
 	// do isProcessed check here
-	if _, ok := w.isProcessed[candidateHash]; ok {
-		// TODO: determine what the isPreccessed check should return, and if re-trying is allowed
-		//  get a better understanding of what the isProcessed check should be checking for
+	if processed, ok := w.isProcessed[candidateHash]; ok {
 		logger.Debugf("candidate %x already processed", candidateHash)
+		return processed, nil
 	}
 	validationResult, err := w.instance.ValidateBlock(task.work)
 
@@ -83,11 +82,12 @@ func (w *worker) executeRequest(task *workerTask) (*ValidationResult, error) {
 		RelayParentStorageRoot: task.work.RelayParentStorageRoot,
 		MaxPovSize:             task.maxPoVSize,
 	}
-	return &ValidationResult{
-		ValidResult: &ValidValidationResult{
+	result := &ValidationResult{
+		ValidResult: &Valid{
 			CandidateCommitments:    candidateCommitments,
 			PersistedValidationData: pvd,
 		},
-	}, nil
-
+	}
+	w.isProcessed[candidateHash] = result
+	return result, nil
 }

dot/parachain/candidate-validation/worker_pool.go

@@ -3,6 +3,7 @@ package candidatevalidation
 import (
 	"fmt"
 
+	parachainruntime "github.com/ChainSafe/gossamer/dot/parachain/runtime"
 	parachaintypes "github.com/ChainSafe/gossamer/dot/parachain/types"
 )
 
@@ -23,23 +24,23 @@ type ValidationTask struct {
 }
 
 // ValidationResult represents the result coming from the candidate validation subsystem.
-// Validation results can be either a ValidValidationResult or InvalidValidationResult.
+// Validation results can be either a Valid or InvalidValidationResult.
 //
 // If the result is invalid,
 // store the reason for invalidity in the InvalidResult field of ValidationResult.
 //
 // If the result is valid,
-// set the values of the ValidResult field of ValidValidationResult.
+// set the values of the ValidResult field of Valid.
 type ValidationResult struct {
-	ValidResult   *ValidValidationResult
+	ValidResult   *Valid
 	InvalidResult *ReasonForInvalidity
 }
 
 func (vr ValidationResult) IsValid() bool {
 	return vr.ValidResult != nil
 }
 
-type ValidValidationResult struct {
+type Valid struct {
 	CandidateCommitments    parachaintypes.CandidateCommitments
 	PersistedValidationData parachaintypes.PersistedValidationData
 }
@@ -114,33 +115,47 @@ func newValidationWorkerPool() *workerPool {
 	}
 }
 
-func (v *workerPool) newValidationWorker(validationCode parachaintypes.ValidationCode) (*worker, error) {
+func (v *workerPool) newValidationWorker(validationCode parachaintypes.ValidationCode) error {
 
 	worker, err := newWorker(validationCode)
 	if err != nil {
-		return nil, fmt.Errorf("failed to create a new worker: %w", err)
+		return fmt.Errorf("failed to create a new worker: %w", err)
 	}
 
 	v.workers[worker.workerID] = worker
 
-	return worker, nil
+	return nil
 }
 
 // submitRequest given a request, the worker pool will get the worker for a given workerID
 // a channel in returned that the response will be dispatch on
-func (v *workerPool) submitRequest(workerID parachaintypes.ValidationCodeHash,
-	request *workerTask) (*ValidationResult, error) {
-	logger.Debugf("pool submit request workerID %x", workerID)
-
-	syncWorker, inMap := v.workers[workerID]
-	if inMap {
-		if syncWorker == nil {
-			panic("sync worker should not be nil")
+func (v *workerPool) submitRequest(msg *ValidationTask) (*ValidationResult, error) {
+	validationCodeHash := msg.ValidationCode.Hash()
+
+	// create worker if not in pool
+	if !v.containsWorker(validationCodeHash) {
+		err := v.newValidationWorker(*msg.ValidationCode)
+		if err != nil {
+			return nil, err
 		}
-		logger.Debugf("sending request", workerID)
-		return syncWorker.executeRequest(request)
 	}
-	return nil, fmt.Errorf("worker not found")
+	syncWorker := v.workers[validationCodeHash]
+
+	logger.Debugf("sending request", validationCodeHash)
+
+	validationParams := parachainruntime.ValidationParameters{
+		ParentHeadData:         msg.PersistedValidationData.ParentHead,
+		BlockData:              msg.PoV.BlockData,
+		RelayParentNumber:      msg.PersistedValidationData.RelayParentNumber,
+		RelayParentStorageRoot: msg.PersistedValidationData.RelayParentStorageRoot,
+	}
+	workTask := &workerTask{
+		work:             validationParams,
+		maxPoVSize:       msg.PersistedValidationData.MaxPovSize,
+		candidateReceipt: msg.CandidateReceipt,
+	}
+	return syncWorker.executeRequest(workTask)
+
 }
 
 func (v *workerPool) containsWorker(workerID parachaintypes.ValidationCodeHash) bool {

dot/parachain/candidate-validation/worker_pool_test.go

@@ -30,7 +30,7 @@ func TestValidationWorkerPool_newValidationWorker(t *testing.T) {
 		"add_one_invalid_worker": {
 			setupWorkerPool: func(t *testing.T) *workerPool {
 				pool := newValidationWorkerPool()
-				_, err := pool.newValidationWorker(parachaintypes.ValidationCode{1, 2, 3, 4})
+				err := pool.newValidationWorker(parachaintypes.ValidationCode{1, 2, 3, 4})
 				require.Error(t, err)
 				return pool
 			},
@@ -39,7 +39,7 @@ func TestValidationWorkerPool_newValidationWorker(t *testing.T) {
 		"add_one_valid_worker": {
 			setupWorkerPool: func(t *testing.T) *workerPool {
 				pool := newValidationWorkerPool()
-				_, err := pool.newValidationWorker(testValidationCode)
+				err := pool.newValidationWorker(testValidationCode)
 				require.NoError(t, err)
 				return pool
 			},