refactor filter decision code

storage/paths/db_index.go

@@ -15,7 +15,6 @@ import (
 	"go.opencensus.io/tag"
 	"golang.org/x/xerrors"
 
-	"github.com/filecoin-project/go-address"
 	"github.com/filecoin-project/go-state-types/abi"
 
 	"github.com/filecoin-project/lotus/journal/alerting"
@@ -578,52 +577,23 @@ func (dbi *DBIndex) StorageFindSector(ctx context.Context, s abi.SectorID, ft st
 				log.Debugf("not selecting on %s, not allowed by file type filters", row.StorageId)
 				continue
 			}
-
+			allowMiners := splitString(row.AllowMiners)
-			if len(row.AllowMiners) > 0 {
+			proceed, err := MinerFilter(allowMiners, false, s.Miner)
-				found := false
+			if err != nil {
-				allowMiners := splitString(row.AllowMiners)
+				return nil, err
-				for _, m := range allowMiners {
-					m := m
-					maddr, err := address.NewFromString(m)
-					if err != nil {
-						return nil, xerrors.Errorf("parsing miner address: %w", err)
-					}
-					mid, err := address.IDFromAddress(maddr)
-					if err != nil {
-						return nil, xerrors.Errorf("parsing miner ID: %w", err)
-					}
-					if abi.ActorID(mid) == s.Miner {
-						found = true
-						break
-					}
-				}
-				if !found {
-					log.Debugf("not selecting on %s, not allowed by allow miners filters", row.StorageId)
-					continue
-				}
 			}
-			if len(row.DenyMiners) > 0 {
+			if !proceed {
-				found := false
+				log.Debugf("not allocating on %s, miner %s not allowed", row.StorageId, s.Miner.String())
-				denyMiners := splitString(row.DenyMiners)
+				continue
-				for _, m := range denyMiners {
+			}
-					m := m
+			denyMiners := splitString(row.DenyMiners)
-					maddr, err := address.NewFromString(m)
+			proceed, err = MinerFilter(denyMiners, true, s.Miner)
-					if err != nil {
+			if err != nil {
-						return nil, xerrors.Errorf("parsing miner address: %w", err)
+				return nil, err
-					}
+			}
-					mid, err := address.IDFromAddress(maddr)
+			if !proceed {
-					if err != nil {
+				log.Debugf("not allocating on %s, miner %s denied", row.StorageId, s.Miner.String())
-						return nil, xerrors.Errorf("parsing miner ID: %w", err)
+				continue
-					}
-					if abi.ActorID(mid) == s.Miner {
-						found = true
-						break
-					}
-				}
-				if found {
-					log.Debugf("not selecting on %s, not allowed by deny miners filters", row.StorageId)
-					continue
-				}
 			}
 
 			if allowList != nil {
@@ -776,49 +746,23 @@ func (dbi *DBIndex) StorageBestAlloc(ctx context.Context, allocate storiface.Sec
 		// Matching with 0 as a workaround to avoid having minerID
 		// present when calling TaskStorage.HasCapacity()
 		if !(miner == abi.ActorID(0)) {
-			if len(row.AllowMiners) > 0 {
+			allowMiners := splitString(row.AllowMiners)
-				found := false
+			proceed, err := MinerFilter(allowMiners, false, miner)
-				allowMiners := splitString(row.AllowMiners)
+			if err != nil {
-				for _, m := range allowMiners {
+				return nil, err
-					m := m
-					maddr, err := address.NewFromString(m)
-					if err != nil {
-						return nil, xerrors.Errorf("parsing miner address: %w", err)
-					}
-					mid, err := address.IDFromAddress(maddr)
-					if err != nil {
-						return nil, xerrors.Errorf("parsing miner ID: %w", err)
-					}
-					if abi.ActorID(mid) == miner {
-						found = true
-						break
-					}
-				}
-				if !found {
-					continue
-				}
 			}
-			if len(row.DenyMiners) > 0 {
+			if !proceed {
-				found := false
+				log.Debugf("not allocating on %s, miner %s not allowed", row.StorageId, miner.String())
-				denyMiners := splitString(row.DenyMiners)
+				continue
-				for _, m := range denyMiners {
+			}
-					m := m
+			denyMiners := splitString(row.DenyMiners)
-					maddr, err := address.NewFromString(m)
+			proceed, err = MinerFilter(denyMiners, true, miner)
-					if err != nil {
+			if err != nil {
-						return nil, xerrors.Errorf("parsing miner address: %w", err)
+				return nil, err
-					}
+			}
-					mid, err := address.IDFromAddress(maddr)
+			if !proceed {
-					if err != nil {
+				log.Debugf("not allocating on %s, miner %s denied", row.StorageId, miner.String())
-						return nil, xerrors.Errorf("parsing miner ID: %w", err)
+				continue
-					}
-					if abi.ActorID(mid) == miner {
-						found = true
-						break
-					}
-				}
-				if found {
-					continue
-				}
 			}
 		}
 

storage/paths/index.go

@@ -498,50 +498,24 @@ func (i *MemIndex) StorageFindSector(ctx context.Context, s abi.SectorID, ft sto
 				continue
 			}
 
-			if len(st.info.AllowMiners) > 0 {
+			proceed, err := MinerFilter(st.info.AllowMiners, false, s.Miner)
-				found := false
+			if err != nil {
-				for _, m := range st.info.AllowMiners {
+				return nil, err
-					minerIDStr := m
-					maddr, err := address.NewFromString(minerIDStr)
-					if err != nil {
-						return nil, xerrors.Errorf("parsing miner address: %w", err)
-					}
-					mid, err := address.IDFromAddress(maddr)
-					if err != nil {
-						return nil, xerrors.Errorf("converting miner address to ID: %w", err)
-					}
-					if abi.ActorID(mid) == s.Miner {
-						found = true
-						break
-					}
-				}
-				if !found {
-					log.Debugf("not allocating on %s, miner %s not allowed", st.info.ID, s.Miner.String())
-					continue
-				}
 			}
 
-			if len(st.info.DenyMiners) > 0 {
+			if !proceed {
-				found := false
+				log.Debugf("not allocating on %s, miner %s not allowed", st.info.ID, s.Miner.String())
-				for _, m := range st.info.DenyMiners {
+				continue
-					minerIDStr := m
+			}
-					maddr, err := address.NewFromString(minerIDStr)
+
-					if err != nil {
+			proceed, err = MinerFilter(st.info.DenyMiners, true, s.Miner)
-						return nil, xerrors.Errorf("parsing miner address: %w", err)
+			if err != nil {
-					}
+				return nil, err
-					mid, err := address.IDFromAddress(maddr)
+			}
-					if err != nil {
+
-						return nil, xerrors.Errorf("converting miner address to ID: %w", err)
+			if !proceed {
-					}
+				log.Debugf("not allocating on %s, miner %s denied", st.info.ID, s.Miner.String())
-					if abi.ActorID(mid) == s.Miner {
+				continue
-						found = true
-						break
-					}
-				}
-				if found {
-					log.Debugf("not allocating on %s, miner %s denied", st.info.ID, s.Miner.String())
-					continue
-				}
 			}
 
 			if spaceReq > uint64(st.fsi.Available) {
@@ -699,50 +673,24 @@ func (i *MemIndex) StorageBestAlloc(ctx context.Context, allocate storiface.Sect
 			continue
 		}
 
-		if len(p.info.AllowMiners) > 0 {
+		proceed, err := MinerFilter(p.info.AllowMiners, false, miner)
-			found := false
+		if err != nil {
-			for _, m := range p.info.AllowMiners {
+			return nil, err
-				minerIDStr := m
-				maddr, err := address.NewFromString(minerIDStr)
-				if err != nil {
-					return nil, xerrors.Errorf("parsing miner address: %w", err)
-				}
-				mid, err := address.IDFromAddress(maddr)
-				if err != nil {
-					return nil, xerrors.Errorf("converting miner address to ID: %w", err)
-				}
-				if abi.ActorID(mid) == miner {
-					found = true
-					break
-				}
-			}
-			if !found {
-				log.Debugf("not allocating on %s, miner %s not allowed", p.info.ID, miner.String())
-				continue
-			}
 		}
 
-		if len(p.info.DenyMiners) > 0 {
+		if !proceed {
-			found := false
+			log.Debugf("not allocating on %s, miner %s not allowed", p.info.ID, miner.String())
-			for _, m := range p.info.DenyMiners {
+			continue
-				minerIDStr := m
+		}
-				maddr, err := address.NewFromString(minerIDStr)
+
-				if err != nil {
+		proceed, err = MinerFilter(p.info.DenyMiners, true, miner)
-					return nil, xerrors.Errorf("parsing miner address: %w", err)
+		if err != nil {
-				}
+			return nil, err
-				mid, err := address.IDFromAddress(maddr)
+		}
-				if err != nil {
+
-					return nil, xerrors.Errorf("converting miner address to ID: %w", err)
+		if !proceed {
-				}
+			log.Debugf("not allocating on %s, miner %s denied", p.info.ID, miner.String())
-				if abi.ActorID(mid) == miner {
+			continue
-					found = true
-					break
-				}
-			}
-			if found {
-				log.Debugf("not allocating on %s, miner %s denied", p.info.ID, miner.String())
-				continue
-			}
 		}
 
 		if spaceReq > uint64(p.fsi.Available) {
@@ -802,3 +750,41 @@ func (i *MemIndex) FindSector(id abi.SectorID, typ storiface.SectorFileType) ([]
 }
 
 var _ SectorIndex = &MemIndex{}
+
+func MinerFilter(miners []string, deny bool, miner abi.ActorID) (bool, error) {
+	if len(miners) > 0 {
+		found := false
+		for _, m := range miners {
+			minerIDStr := m
+			maddr, err := address.NewFromString(minerIDStr)
+			if err != nil {
+				return false, xerrors.Errorf("parsing miner address: %w", err)
+			}
+			mid, err := address.IDFromAddress(maddr)
+			if err != nil {
+				return false, xerrors.Errorf("converting miner address to ID: %w", err)
+			}
+			if abi.ActorID(mid) == miner {
+				found = true
+				break
+			}
+		}
+		// If not found in list
+		if !found {
+			// If this is allowed list
+			if !deny {
+				return false, nil
+			}
+			// If this is denied list
+			return true, nil
+		}
+		// If found in list and
+		// If this is allowed list
+		if !deny {
+			return true, nil
+		}
+		// If this is denied list
+		return false, nil
+	}
+	return true, nil
+}

storage/paths/local.go

@@ -14,7 +14,6 @@ import (
 	"golang.org/x/xerrors"
 
 	ffi "github.com/filecoin-project/filecoin-ffi"
-	"github.com/filecoin-project/go-address"
 	"github.com/filecoin-project/go-state-types/abi"
 	"github.com/filecoin-project/go-state-types/proof"
 
@@ -512,49 +511,19 @@ func (st *Local) AcquireSector(ctx context.Context, sid storiface.SectorRef, exi
 		if !fileType.Allowed(allowTypes, denyTypes) {
 			return false, nil
 		}
-
+		proceed, err := MinerFilter(allowMiners, false, miner)
-		if len(allowMiners) > 0 {
+		if err != nil {
-			found := false
+			return false, err
-			for _, m := range allowMiners {
-				minerIDStr := m
-				maddr, err := address.NewFromString(minerIDStr)
-				if err != nil {
-					return false, xerrors.Errorf("parsing miner address: %w", err)
-				}
-				mid, err := address.IDFromAddress(maddr)
-				if err != nil {
-					return false, xerrors.Errorf("converting miner address to ID: %w", err)
-				}
-				if abi.ActorID(mid) == miner {
-					found = true
-					break
-				}
-			}
-			if !found {
-				return false, nil
-			}
 		}
-
+		if !proceed {
-		if len(denyMiners) > 0 {
+			return false, nil
-			found := false
+		}
-			for _, m := range denyMiners {
+		proceed, err = MinerFilter(denyMiners, true, miner)
-				minerIDStr := m
+		if err != nil {
-				maddr, err := address.NewFromString(minerIDStr)
+			return false, err
-				if err != nil {
+		}
-					return false, xerrors.Errorf("parsing miner address: %w", err)
+		if !proceed {
-				}
+			return false, nil
-				mid, err := address.IDFromAddress(maddr)
-				if err != nil {
-					return false, xerrors.Errorf("converting miner address to ID: %w", err)
-				}
-				if abi.ActorID(mid) == miner {
-					found = true
-					break
-				}
-			}
-			if found {
-				return false, nil
-			}
 		}
 
 		return true, nil