Fix out-of-collateral algorithm

autopilot/contractor.go

@@ -762,12 +762,7 @@ func (c *contractor) runContractChecks(ctx context.Context, w Worker, contracts
 
 		// decide whether the contract is still good
 		ci := contractInfo{contract: contract, priceTable: host.PriceTable.HostPriceTable, settings: host.Settings}
-		renterFunds, err := c.renewFundingEstimate(ctx, ci, state.fee, false)
-		if err != nil {
-			c.logger.Errorw(fmt.Sprintf("failed to compute renterFunds for contract: %v", err))
-		}
-
-		usable, recoverable, refresh, renew, reasons := c.isUsableContract(state.cfg, ci, cs.BlockHeight, renterFunds, ipFilter)
+		usable, recoverable, refresh, renew, reasons := c.isUsableContract(state.cfg, state, ci, cs.BlockHeight, ipFilter)
 		ci.usable = usable
 		ci.recoverable = recoverable
 		if !usable {
@@ -1406,7 +1401,7 @@ func (c *contractor) refreshContract(ctx context.Context, w Worker, ci contractI
 
 	// calculate the renter funds
 	var renterFunds types.Currency
-	if isOutOfFunds(state.cfg, ci.settings, ci.contract) {
+	if isOutOfFunds(state.cfg, ci.priceTable, ci.contract) {
 		renterFunds, err = c.refreshFundingEstimate(ctx, state.cfg, ci, state.fee)
 		if err != nil {
 			c.logger.Errorw(fmt.Sprintf("could not get refresh funding estimate, err: %v", err), "hk", hk, "fcid", fcid)
@@ -1425,24 +1420,18 @@ func (c *contractor) refreshContract(ctx context.Context, w Worker, ci contractI
 	expectedStorage := renterFundsToExpectedStorage(renterFunds, contract.EndHeight()-cs.BlockHeight, ci.priceTable)
 	unallocatedCollateral := rev.MissedHostPayout().Sub(contract.ContractPrice)
 
-	// calculate the expected new collateral to determine the minNewCollateral.
-	// If the contract isn't below the min collateral, we don't enforce a
-	// minimum.
-	var minNewColl types.Currency
-	_, _, expectedNewCollateral := rhpv3.RenewalCosts(contract.Revision.FileContract, ci.priceTable, expectedStorage, contract.EndHeight())
-	if isBelowCollateralThreshold(expectedNewCollateral, unallocatedCollateral) {
-		minNewColl = minNewCollateral(unallocatedCollateral)
-	}
+	// a refresh should always result in a contract that has enough collateral
+	minNewCollateral := minRemainingCollateral(state.cfg, state.rs, renterFunds, settings, ci.priceTable).Mul64(2)
 
 	// renew the contract
-	resp, err := w.RHPRenew(ctx, contract.ID, contract.EndHeight(), hk, contract.SiamuxAddr, settings.Address, state.address, renterFunds, minNewColl, expectedStorage, settings.WindowSize)
+	resp, err := w.RHPRenew(ctx, contract.ID, contract.EndHeight(), hk, contract.SiamuxAddr, settings.Address, state.address, renterFunds, minNewCollateral, expectedStorage, settings.WindowSize)
 	if err != nil {
 		if strings.Contains(err.Error(), "new collateral is too low") {
 			c.logger.Debugw("refresh failed: contract wouldn't have enough collateral after refresh",
 				"hk", hk,
 				"fcid", fcid,
 				"unallocatedCollateral", unallocatedCollateral.String(),
-				"minNewCollateral", minNewColl.String(),
+				"minNewCollateral", minNewCollateral.String(),
 			)
 			return api.ContractMetadata{}, true, err
 		}
@@ -1469,7 +1458,7 @@ func (c *contractor) refreshContract(ctx context.Context, w Worker, ci contractI
 		"fcid", refreshedContract.ID,
 		"renewedFrom", contract.ID,
 		"renterFunds", renterFunds.String(),
-		"minNewCollateral", minNewColl.String(),
+		"minNewCollateral", minNewCollateral.String(),
 		"newCollateral", newCollateral.String(),
 	)
 	return refreshedContract, true, nil

autopilot/hostfilter.go

@@ -20,12 +20,11 @@ const (
 	// remaining at which the contract gets marked as not good for upload
 	minContractFundUploadThreshold = float64(0.05) // 5%
 
-	// minContractCollateralThreshold is 10% of the collateral that we would put
-	// into a contract upon renewing it. That means, we consider a contract
-	// worth renewing when that results in 10x the collateral of what it
-	// currently has remaining.
-	minContractCollateralThresholdNumerator   = 1
-	minContractCollateralThresholdDenominator = 10
+	// minContractCollateralDenominator is used to define the percentage of
+	// remaining collateral in a contract in relation to its potential
+	// acquirable storage below which the contract is considered to be
+	// out-of-collateral.
+	minContractCollateralDenominator = 20 // 5%
 
 	// contractConfirmationDeadline is the number of blocks since its start
 	// height we wait for a contract to appear on chain.
@@ -227,7 +226,7 @@ func isUsableHost(cfg api.AutopilotConfig, rs api.RedundancySettings, gc worker.
 // - recoverable -> can be usable in the contract set if it is refreshed/renewed
 // - refresh -> should be refreshed
 // - renew -> should be renewed
-func (c *contractor) isUsableContract(cfg api.AutopilotConfig, ci contractInfo, bh uint64, renterFunds types.Currency, f *ipFilter) (usable, recoverable, refresh, renew bool, reasons []string) {
+func (c *contractor) isUsableContract(cfg api.AutopilotConfig, state state, ci contractInfo, bh uint64, f *ipFilter) (usable, recoverable, refresh, renew bool, reasons []string) {
 	contract, s, pt := ci.contract, ci.settings, ci.priceTable
 
 	usable = true
@@ -244,14 +243,14 @@ func (c *contractor) isUsableContract(cfg api.AutopilotConfig, ci contractInfo,
 		refresh = false
 		renew = false
 	} else {
-		if isOutOfCollateral(contract, s, pt, renterFunds, cfg.Contracts.Period, bh) {
+		if isOutOfCollateral(cfg, state.rs, contract, s, pt) {
 			reasons = append(reasons, errContractOutOfCollateral.Error())
 			usable = false
 			recoverable = true
 			refresh = true
 			renew = false
 		}
-		if isOutOfFunds(cfg, s, contract) {
+		if isOutOfFunds(cfg, pt, contract) {
 			reasons = append(reasons, errContractOutOfFunds.Error())
 			usable = false
 			recoverable = true
@@ -278,19 +277,17 @@ func (c *contractor) isUsableContract(cfg api.AutopilotConfig, ci contractInfo,
 	return
 }
 
-func isOutOfFunds(cfg api.AutopilotConfig, s rhpv2.HostSettings, c api.Contract) bool {
+func isOutOfFunds(cfg api.AutopilotConfig, pt rhpv3.HostPriceTable, c api.Contract) bool {
 	// TotalCost should never be zero but for legacy reasons we check and return
 	// true should it be the case
 	if c.TotalCost.IsZero() {
 		return true
 	}
 
-	blockBytes := types.NewCurrency64(rhpv2.SectorSize * cfg.Contracts.Period)
-	sectorStoragePrice := s.StoragePrice.Mul(blockBytes)
-	sectorUploadBandwidthPrice := s.UploadBandwidthPrice.Mul64(rhpv2.SectorSize)
-	sectorDownloadBandwidthPrice := s.DownloadBandwidthPrice.Mul64(rhpv2.SectorSize)
-	sectorBandwidthPrice := sectorUploadBandwidthPrice.Add(sectorDownloadBandwidthPrice)
-	sectorPrice := sectorStoragePrice.Add(sectorBandwidthPrice)
+	sectorPrice, _ := pt.BaseCost().
+		Add(pt.AppendSectorCost(cfg.Contracts.Period)).
+		Add(pt.ReadSectorCost(rhpv2.SectorSize)).
+		Total()
 	percentRemaining, _ := big.NewRat(0, 1).SetFrac(c.RenterFunds().Big(), c.TotalCost.Big()).Float64()
 
 	return c.RenterFunds().Cmp(sectorPrice.Mul64(3)) < 0 || percentRemaining < minContractFundUploadThreshold
@@ -299,50 +296,61 @@ func isOutOfFunds(cfg api.AutopilotConfig, s rhpv2.HostSettings, c api.Contract)
 // isOutOfCollateral returns 'true' if the remaining/unallocated collateral in
 // the contract is below a certain threshold of the collateral we would try to
 // put into a contract upon renew.
-func isOutOfCollateral(c api.Contract, s rhpv2.HostSettings, pt rhpv3.HostPriceTable, renterFunds types.Currency, period, blockHeight uint64) bool {
-	// Compute the expected storage for the contract given the funds we are
-	// willing to put into it.
+func isOutOfCollateral(cfg api.AutopilotConfig, rs api.RedundancySettings, c api.Contract, s rhpv2.HostSettings, pt rhpv3.HostPriceTable) bool {
+	min := minRemainingCollateral(cfg, rs, c.RenterFunds(), s, pt)
+	return c.RemainingCollateral().Cmp(min) < 0
+}
+
+// minNewCollateral returns the minimum amount of unallocated collateral that a
+// contract should contain after a refresh given the current amount of
+// unallocated collateral.
+func minRemainingCollateral(cfg api.AutopilotConfig, rs api.RedundancySettings, renterFunds types.Currency, s rhpv2.HostSettings, pt rhpv3.HostPriceTable) types.Currency {
+	// Compute the expected storage for the contract given its remaining funds.
 	// Note: we use the full period here even though we are checking whether to
 	// do a refresh. Otherwise, the 'expectedStorage' would would become
 	// ridiculously large the closer the contract is to its end height.
-	expectedStorage := renterFundsToExpectedStorage(renterFunds, period, pt)
+	expectedStorage := renterFundsToExpectedStorage(renterFunds, cfg.Contracts.Period, pt)
+
+	// Cap the expected storage at twice the ideal amount of data we expect to
+	// store on a host. Even if we could afford more storage, there is no point
+	// in locking up more collateral than we expect to require.
+	idealDataPerHost := float64(cfg.Contracts.Storage) * rs.Redundancy() / float64(cfg.Contracts.Amount)
+	allocationPerHost := idealDataPerHost * 2
+	if expectedStorage > uint64(allocationPerHost) {
+		expectedStorage = uint64(allocationPerHost)
+	}
+
 	// Cap the expected storage at the remaining storage of the host. If the
 	// host doesn't have any storage left, there is no point in adding
 	// collateral.
 	if expectedStorage > s.RemainingStorage {
 		expectedStorage = s.RemainingStorage
 	}
-	_, _, newCollateral := rhpv3.RenewalCosts(c.Revision.FileContract, pt, expectedStorage, c.EndHeight())
-	return isBelowCollateralThreshold(newCollateral, c.RemainingCollateral())
-}
 
-// isBelowCollateralThreshold returns true if the remainingCollateral is below a
-// certain percentage of newCollateral. The newCollateral is the amount of
-// unallocated collateral in a contract after refreshing it and the
-// remainingCollateral is the current amount of unallocated collateral in the
-// contract.
-func isBelowCollateralThreshold(newCollateral, remainingCollateral types.Currency) bool {
-	if newCollateral.IsZero() {
-		// Protect against division-by-zero. This can happen for 2 reasons.
-		// 1. the collateral is already at the host's max collateral so a
-		// refresh wouldn't result in any new unallocated collateral.
-		// 2. the host has no more remaining storage so a refresh would only
-		// lead to unallocated collateral that we can't use.
-		// In both cases we don't gain anything from refreshing the contract.
-		// NOTE: This causes us to not immediately consider contracts as bad
-		// even though we can't upload to them anymore. This is fine since the
-		// collateral score or remaining storage score should filter these
-		// contracts out eventually.
-		return false
+	// If no storage is expected, return zero.
+	if expectedStorage == 0 {
+		return types.ZeroCurrency
 	}
-	return newCollateral.Cmp(minNewCollateral(remainingCollateral)) >= 0
-}
 
-// minNewCollateral returns the minimum amount of unallocated collateral that a
-// contract should contain after a refresh given the current amount of
-// unallocated collateral.
-func minNewCollateral(unallocatedCollateral types.Currency) types.Currency {
-	return unallocatedCollateral.Mul64(minContractCollateralThresholdDenominator).Div64(minContractCollateralThresholdNumerator)
+	// Computet the collateral for a single sector.
+	_, sectorCollateral := pt.BaseCost().
+		Add(pt.AppendSectorCost(cfg.Contracts.Period)).
+		Add(pt.ReadSectorCost(rhpv2.SectorSize)).
+		Total()
+
+	// The expectedStorageCollateral is 5% of the collateral we'd need to store
+	// all of the expectedStorage.
+	minExpectedStorageCollateral := sectorCollateral.Mul64(expectedStorage / rhpv2.SectorSize).Div64(minContractCollateralDenominator)
+
+	// The absolute minimum collateral we want to put into a contract is 3
+	// sectors worth of collateral.
+	minCollateral := sectorCollateral.Mul64(3)
+
+	// Return the larger of the two.
+	if minExpectedStorageCollateral.Cmp(minCollateral) > 0 {
+		minCollateral = minExpectedStorageCollateral
+	}
+	return minCollateral
 }
 
 func isUpForRenewal(cfg api.AutopilotConfig, r types.FileContractRevision, blockHeight uint64) (shouldRenew, secondHalf bool) {

autopilot/hostfilter_test.go

@@ -1,53 +1,88 @@
 package autopilot
 
 import (
+	"math"
 	"testing"
 
+	rhpv2 "go.sia.tech/core/rhp/v2"
+	rhpv3 "go.sia.tech/core/rhp/v3"
 	"go.sia.tech/core/types"
+	"go.sia.tech/renterd/api"
 )
 
-func TestMinNewCollateral(t *testing.T) {
+func TestMinRemainingCollateral(t *testing.T) {
 	t.Parallel()
 
-	// The collateral threshold is 10% meaning that we expect 10 times the
-	// remaining collateral to be the minimum to trigger a renewal.
-	if min := minNewCollateral(types.Siacoins(1)); !min.Equals(types.Siacoins(10)) {
-		t.Fatalf("expected 10, got %v", min)
+	// consts
+	rs := api.RedundancySettings{MinShards: 1, TotalShards: 2} // 2x redundancy
+	cfg := api.AutopilotConfig{
+		Contracts: api.ContractsConfig{
+			Amount: 5,
+			Period: 100,
+		},
 	}
-}
-
-func TestIsBelowCollateralThreshold(t *testing.T) {
-	t.Parallel()
+	one := types.NewCurrency64(1)
+	pt := rhpv3.HostPriceTable{
+		CollateralCost:        one,
+		InitBaseCost:          one,
+		WriteBaseCost:         one,
+		ReadBaseCost:          one,
+		WriteLengthCost:       one,
+		WriteStoreCost:        one,
+		ReadLengthCost:        one,
+		UploadBandwidthCost:   one,
+		DownloadBandwidthCost: one,
+	}
+	s := rhpv2.HostSettings{}
+	_, sectorCollateral := pt.BaseCost().
+		Add(pt.AppendSectorCost(cfg.Contracts.Period)).
+		Add(pt.ReadSectorCost(rhpv2.SectorSize)).
+		Total()
 
+		// testcases
 	tests := []struct {
-		newCollateral       types.Currency
-		remainingCollateral types.Currency
-		isBelow             bool
+		expectedStorage  uint64
+		remainingStorage uint64
+		renterFunds      types.Currency
+		expected         types.Currency
 	}{
 		{
-			remainingCollateral: types.NewCurrency64(1),
-			newCollateral:       types.NewCurrency64(10),
-			isBelow:             true,
+			// lots of funds but no remaining storage
+			expected:         types.ZeroCurrency,
+			expectedStorage:  100,
+			remainingStorage: 0,
+			renterFunds:      types.Siacoins(1000),
 		},
 		{
-			remainingCollateral: types.NewCurrency64(1),
-			newCollateral:       types.NewCurrency64(9),
-			isBelow:             false,
+			// lots of funds but only 1 byte of remaining storage
+			expected:         sectorCollateral.Mul64(3),
+			expectedStorage:  100,
+			remainingStorage: 1,
+			renterFunds:      types.Siacoins(1000),
 		},
 		{
-			remainingCollateral: types.NewCurrency64(1),
-			newCollateral:       types.NewCurrency64(11),
-			isBelow:             true,
+			// ideal data is capping the collateral
+			// 100 sectors * 2 (redundancy) * 2 (buffer) / 5 (hosts) / 20 (denom) = 4 sectors of collateral
+			expected:         sectorCollateral.Mul64(4),                               // 100 sectors * 2 (redundancy) * 2 (buffer)
+			expectedStorage:  5 * rhpv2.SectorSize * minContractCollateralDenominator, // 100 sectors
+			remainingStorage: math.MaxUint64,
+			renterFunds:      types.Siacoins(1000),
 		},
 		{
-			remainingCollateral: types.NewCurrency64(1),
-			newCollateral:       types.ZeroCurrency,
-			isBelow:             false,
+			// nothing is capping the expected storage
+			expected:         types.NewCurrency64(17175674880), // ~13.65 x the previous 'expected'
+			expectedStorage:  math.MaxUint32,
+			remainingStorage: math.MaxUint64,
+			renterFunds:      types.Siacoins(1000),
 		},
 	}
+
 	for i, test := range tests {
-		if isBelow := isBelowCollateralThreshold(test.newCollateral, test.remainingCollateral); isBelow != test.isBelow {
-			t.Fatalf("%v: expected %v, got %v", i+1, test.isBelow, isBelow)
+		cfg.Contracts.Storage = test.expectedStorage
+		s.RemainingStorage = test.remainingStorage
+		min := minRemainingCollateral(cfg, rs, test.renterFunds, s, pt)
+		if min.Cmp(test.expected) != 0 {
+			t.Fatalf("%v: expected %v, got %v", i+1, test.expected, min)
 		}
 	}
 }