fix: bug bypassing basic filters (#1217)

* fix bug bypassing basic filters

* update code comments

storagemarket/provider_dealfilter.go

@@ -13,14 +13,30 @@ import (
 
 func (p *Provider) getDealFilterParams(deal *types.ProviderDealState) (*dealfilter.DealFilterParams, *acceptError) {
 
-	// Check cached sealing pipeline status and error
-	sealingStatus, err := p.sealingPipelineStatus()
-	if err != nil {
-		return nil, &acceptError{
-			error:         fmt.Errorf("storage deal filter: failed to fetch sealing pipeline status: %w", err),
-			reason:        "server error: storage deal filter: getting sealing status",
-			isSevereError: true,
-		}
+	params := types.DealParams{
+		DealUUID:           deal.DealUuid,
+		ClientDealProposal: deal.ClientDealProposal,
+		DealDataRoot:       deal.DealDataRoot,
+		Transfer:           deal.Transfer,
+		IsOffline:          deal.IsOffline,
+		RemoveUnsealedCopy: !deal.FastRetrieval,
+		SkipIPNIAnnounce:   !deal.AnnounceToIPNI,
+	}
+
+	// Clear transfer params in case it contains sensitive information
+	// (eg Authorization header)
+	params.Transfer.Params = []byte{}
+
+	// If no external deal filter is set then return empty value for SealingPipelineState,
+	// FundsState and StorageState to shorten the execution. This also avoids the expensive
+	// p.sealingPipelineStatus() call
+	if p.config.StorageFilter == "" {
+		return &dealfilter.DealFilterParams{
+			DealParams:           params,
+			SealingPipelineState: sealingpipeline.Status{},
+			FundsState:           funds.Status{},
+			StorageState:         storagespace.Status{},
+		}, nil
 	}
 
 	// Get the status of funds in the collateral and publish message wallets
@@ -43,20 +59,16 @@ func (p *Provider) getDealFilterParams(deal *types.ProviderDealState) (*dealfilt
 		}
 	}
 
-	params := types.DealParams{
-		DealUUID:           deal.DealUuid,
-		ClientDealProposal: deal.ClientDealProposal,
-		DealDataRoot:       deal.DealDataRoot,
-		Transfer:           deal.Transfer,
-		IsOffline:          deal.IsOffline,
-		RemoveUnsealedCopy: !deal.FastRetrieval,
-		SkipIPNIAnnounce:   !deal.AnnounceToIPNI,
+	// Check cached sealing pipeline status and error
+	sealingStatus, err := p.sealingPipelineStatus()
+	if err != nil {
+		return nil, &acceptError{
+			error:         fmt.Errorf("storage deal filter: failed to fetch sealing pipeline status: %w", err),
+			reason:        "server error: storage deal filter: getting sealing status",
+			isSevereError: true,
+		}
 	}
 
-	// Clear transfer params in case it contains sensitive information
-	// (eg Authorization header)
-	params.Transfer.Params = []byte{}
-
 	return &dealfilter.DealFilterParams{
 		DealParams:           params,
 		SealingPipelineState: sealingStatus,

storagemarket/provider_loop.go

@@ -69,10 +69,10 @@ type acceptError struct {
 	reason string
 }
 
+// we still need to call the BasicDealFilter() even when external deal filter is not set.
+// Once BasicDealFilter() completes the checks like are we accepting online deal, verified deal etc.
+// Then it runs the external "cmd" filter. Thus, runDealFilters is not optional of any type of deal
 func (p *Provider) runDealFilters(deal *types.ProviderDealState) *acceptError {
-	if p.config.StorageFilter == "" {
-		return nil
-	}
 
 	// run custom storage deal filter decision logic
 	dealFilterParams, aerr := p.getDealFilterParams(deal)
@@ -118,7 +118,7 @@ func (p *Provider) processDealProposal(deal *types.ProviderDealState) *acceptErr
 		return aerr
 	}
 
-	// Run deal through the filter if deal filter is set
+	// we still need to call runDealFilters() even when external deal filter is not set
 	if aerr := p.runDealFilters(deal); aerr != nil {
 		return aerr
 	}
@@ -229,7 +229,7 @@ func (p *Provider) processOfflineDealProposal(ds *smtypes.ProviderDealState, dh
 		return aerr
 	}
 
-	// Run deal through the filter if deal filter is set
+	// we still need to call runDealFilters() even when external deal filter is not set
 	if aerr := p.runDealFilters(ds); aerr != nil {
 		return aerr
 	}