better reserves calcs. other review fixes

client/core/bond.go

@@ -6,6 +6,7 @@ import (
 	"errors"
 	"fmt"
 	"math"
+	"sort"
 	"time"
 
 	"decred.org/dcrdex/client/asset"
@@ -25,7 +26,6 @@ const (
 	defaultBondAsset = 42 // DCR
 
 	maxBondedMult    = 4
-	bondOverlap      = 2
 	bondTickInterval = 20 * time.Second
 )
 
@@ -202,17 +202,13 @@ func (c *Core) updateBondReserves(balanceCheckID ...uint32) {
 		if dc.acct.targetTier == 0 {
 			return
 		}
+
 		bondAsset := bondAssets[dc.acct.bondAsset]
 		if bondAsset == nil {
 			// Logged at login auth.
 			return
 		}
-		inBonds, _ := dc.bondTotalInternal(bondAsset.ID)
-		totalReserves := bondOverlap * dc.acct.targetTier * bondAsset.Amt
-		var future uint64
-		if inBonds < totalReserves {
-			future = totalReserves - inBonds
-		}
+		future := c.minBondReserves(dc, bondAsset)
 		reserves[bondAsset.ID] = append(reserves[bondAsset.ID], future)
 	}
 
@@ -262,6 +258,89 @@ func (c *Core) updateBondReserves(balanceCheckID ...uint32) {
 	}
 }
 
+// minBondReserveTiers calculates the minimum number of tiers that we need to
+// reserve funds for. minBondReserveTiers must be called with the authMtx
+// RLocked.
+func (c *Core) minBondReserves(dc *dexConnection, bondAsset *BondAsset) uint64 {
+	acct, targetTier := dc.acct, dc.acct.targetTier
+	if targetTier == 0 {
+		return 0
+	}
+	// Keep a list of tuples of [weakTime, bondStrength]. Later, we'll checks
+	// these against expired bonds, to see how many tiers we can expect to have
+	// refunded funds avilable for.
+	activeTiers := make([][2]uint64, 0)
+	dexCfg := dc.config()
+	bondExpiry := dexCfg.BondExpiry
+	pBuffer := uint64(pendingBuffer(c.net))
+	var tierSum uint64
+	for _, bond := range append(acct.pendingBonds, acct.bonds...) {
+		weakTime := bond.LockTime - bondExpiry - pBuffer
+		ba := dexCfg.BondAssets[dex.BipIDSymbol(bond.AssetID)]
+		if ba == nil {
+			// Bond asset no longer supported Can't calculate strength. Consdier
+			// it strength one.
+			activeTiers = append(activeTiers, [2]uint64{weakTime, 1})
+			continue
+		}
+
+		tiers := bond.Amount / ba.Amt
+		// We won't count any active bond strength > our tier target.
+		if tiers > targetTier-tierSum {
+			tiers = targetTier - tierSum
+		}
+		tierSum += tiers
+		activeTiers = append(activeTiers, [2]uint64{weakTime, tiers})
+		if tierSum == targetTier {
+			break
+		}
+	}
+	// If our active+pending bonds don't cover our target tier for some reason,
+	// we need to add the missing bond strength.
+	reserveTiers := targetTier - tierSum
+	sort.Slice(activeTiers, func(i, j int) bool {
+		return activeTiers[i][0] < activeTiers[j][1]
+	})
+	sort.Slice(acct.expiredBonds, func(i, j int) bool { // probably already is sorted, but whatever
+		return acct.expiredBonds[i].LockTime < acct.expiredBonds[j].LockTime
+	})
+	sBuffer := uint64(spendableDelay(c.net))
+out:
+	for _, bond := range acct.expiredBonds {
+		if bond.AssetID != bondAsset.ID {
+			continue
+		}
+		strength := bond.Amount / bondAsset.Amt
+		refundableTime := bond.LockTime + sBuffer
+		for i, pair := range activeTiers {
+			weakTime, tiers := pair[0], pair[1]
+			if tiers == 0 {
+				continue
+			}
+			if refundableTime >= weakTime {
+				// Everything is time-sorted. If this bond won't be refunded
+				// in time, none of the others will either.
+				break out
+			}
+			// Modify the activeTiers strengths in-place. Will cause some
+			// extra iteration, but beats the complexity of trying to modify
+			// the slice somehow.
+			if tiers < strength {
+				strength -= tiers
+				activeTiers[i][1] = 0
+			} else {
+				activeTiers[i][1] = tiers - strength
+				// strength = 0
+				break
+			}
+		}
+	}
+	for _, pair := range activeTiers {
+		reserveTiers += pair[1]
+	}
+	return reserveTiers * bondAsset.Amt
+}
+
 // dexBondConfig retrieves a dex's configuration related to bonds.
 func (c *Core) dexBondConfig(dc *dexConnection, now int64) *dexBondCfg {
 	lockTimeThresh := now // in case dex is down, expire (to refund when lock time is passed)
@@ -374,7 +453,6 @@ func (c *Core) bondStateOfDEX(dc *dexConnection, bondCfg *dexBondCfg) *dexAcctBo
 		}
 	}
 	state.mustPost += state.toComp
-
 	return state
 }
 
@@ -979,7 +1057,8 @@ func (c *Core) UpdateBondOptions(form *BondOptionsForm) error {
 	dbAcct.PenaltyComps = penaltyComps
 
 	var bondAssetAmt uint64 // because to disable we must proceed even with no config
-	if bondAsset := bondAssets[bondAssetID]; bondAsset == nil {
+	bondAsset := bondAssets[bondAssetID]
+	if bondAsset == nil {
 		if targetTier > 0 || assetChanged {
 			return fmt.Errorf("dex %v is does not support %v as a bond asset (or we lack their config)",
 				dbAcct.Host, unbip(bondAssetID))
@@ -1023,12 +1102,7 @@ func (c *Core) UpdateBondOptions(form *BondOptionsForm) error {
 		// We're under the dc.acct.authMtx lock, so we'll add our contribution
 		// first and then iterate the others in a loop where we're okay to lock
 		// their authMtx (via bondTotal).
-		var nominalReserves uint64
-		inBonds, _ := dc.bondTotalInternal(bondAssetID)
-		totalReserves := bondOverlap * bondAssetAmt * targetTier // this dexConnection
-		if totalReserves > inBonds {
-			nominalReserves += totalReserves - inBonds
-		}
+		nominalReserves := c.minBondReserves(dc, bondAsset)
 		var n uint64
 		if targetTier > 0 {
 			n = 1
@@ -1038,26 +1112,24 @@ func (c *Core) UpdateBondOptions(form *BondOptionsForm) error {
 			if otherDC.acct.host == dc.acct.host { // Only adding others
 				continue
 			}
-			assetID, targetTier, _ := otherDC.bondOpts()
+			assetID, _, _ := otherDC.bondOpts()
 			if assetID != bondAssetID {
 				continue
 			}
 			bondAsset, _ := otherDC.bondAsset(assetID)
 			if bondAsset == nil {
 				continue
 			}
-			inBonds, _ := otherDC.bondTotal(assetID)
-			totalReserves := bondOverlap * targetTier * bondAsset.Amt
 			n++
 			tiers += targetTier
-			if inBonds >= totalReserves {
-				continue
-			}
-			nominalReserves += totalReserves - inBonds
+			ba := BondAsset(*bondAsset)
+			otherDC.acct.authMtx.RLock()
+			nominalReserves += c.minBondReserves(dc, &ba)
+			otherDC.acct.authMtx.RUnlock()
 		}
 
 		var feeReserves uint64
-		if tiers > 0 {
+		if n > 0 {
 			feeBuffer := bonder.BondsFeeBuffer(c.feeSuggestionAny(bondAssetID))
 			feeReserves = n * feeBuffer
 			req := nominalReserves + feeReserves

client/core/core_test.go

@@ -10507,7 +10507,7 @@ func TestUpdateBondOptions(t *testing.T) {
 	var targetTierZero uint64 = 0
 	defaultMaxBondedAmt := maxBondedMult * bondAsset.Amt * targetTier
 	tooLowMaxBonded := defaultMaxBondedAmt - 1
-	singlyBondedReserves := bondAsset.Amt*bondOverlap*targetTier + bondFeeBuffer
+	singlyBondedReserves := bondAsset.Amt*targetTier + bondFeeBuffer
 
 	type acctState struct {
 		targetTier   uint64
@@ -10597,7 +10597,7 @@ func TestUpdateBondOptions(t *testing.T) {
 			},
 			addOtherDC:  true,
 			after:       acctState{},
-			expReserves: singlyBondedReserves,
+			expReserves: bondFeeBuffer,
 		},
 	} {
 		t.Run(tt.name, func(t *testing.T) {
@@ -10654,17 +10654,17 @@ func TestRotateBonds(t *testing.T) {
 	bondAsset := dcrBondAsset
 	bondFeeBuffer := tDcrWallet.BondsFeeBuffer(feeRate)
 	maxBondedPerTier := maxBondedMult * bondAsset.Amt
-	overlappedReservesPerTier := bondAsset.Amt * bondOverlap
-	singlyTieredMaxReserves := overlappedReservesPerTier + bondFeeBuffer
 
 	now := uint64(time.Now().Unix())
 	bondExpiry := rig.dc.config().BondExpiry
 	// bondDuration := minBondLifetime(rig.core.net, bondExpiry)
 	locktimeThresh := now + bondExpiry
-	mergeableLocktimeThresh := locktimeThresh + bondExpiry/4 + uint64(pendingBuffer(rig.core.net))
+	pBuffer := uint64(pendingBuffer(rig.core.net))
+	mergeableLocktimeThresh := locktimeThresh + bondExpiry/4 + pBuffer
 	// unexpired := locktimeThresh + 1
 	locktimeExpired := locktimeThresh - 1
 	locktimeRefundable := now - 1
+	weakTimeThresh := locktimeThresh + pBuffer
 
 	run := func(wantPending, wantExpired int, expectedReserves uint64) {
 		ctx, cancel := context.WithTimeout(rig.core.ctx, time.Second)
@@ -10690,32 +10690,47 @@ func TestRotateBonds(t *testing.T) {
 	acct.bondAsset = bondAsset.ID
 	tDcrWallet.bal = &asset.Balance{Available: bondAsset.Amt*targetTier + bondFeeBuffer}
 	rig.queuePrevalidateBond()
-	run(1, 0, singlyTieredMaxReserves-bondAsset.Amt)
+	run(1, 0, bondAsset.Amt+bondFeeBuffer)
 
 	// Post and then expire the bond. This first bond should move to expired and we
 	// should create another bond.
 	acct.bonds, acct.pendingBonds = acct.pendingBonds, nil
 	acct.bonds[0].LockTime = locktimeExpired
 	rig.queuePrevalidateBond()
-	run(1, 1, singlyTieredMaxReserves-2*bondAsset.Amt)
+	// The newly expired bond will be refunded in time to fund our next round,
+	// so we only need fees reserved.
+	run(1, 1, bondFeeBuffer)
+
+	// If the live bond is closer to expiration, the expired bond won't be
+	// ready in time, so we'll need more reserves.
+	acct.bonds, acct.pendingBonds = acct.pendingBonds, nil
+	acct.bonds[0].LockTime = weakTimeThresh + 1
+	run(0, 1, bondAsset.Amt+bondFeeBuffer)
+
+	// Make the live bond weak. Should get a pending bond. Only fees reserves,
+	// because we still have an expired bond.
+	acct.bonds[0].LockTime = weakTimeThresh - 1
+	rig.queuePrevalidateBond()
+	run(1, 1, bondFeeBuffer)
 
 	// Refund the expired bond
 	acct.expiredBonds[0].LockTime = locktimeRefundable
 	tDcrWallet.contractExpired = true
 	tDcrWallet.refundBondCoin = &tCoin{}
-	run(1, 0, singlyTieredMaxReserves-bondAsset.Amt)
+	run(1, 0, bondAsset.Amt+bondFeeBuffer)
 
 	acct.targetTier = 2
+	acct.bonds = nil
 	rig.queuePrevalidateBond()
-	run(2, 0, (overlappedReservesPerTier*2+bondFeeBuffer)-2*bondAsset.Amt)
+	run(2, 0, bondAsset.Amt*2+bondFeeBuffer)
 
 	// Check that a new bond will be scheduled for merge with an existing bond
 	// if the locktime is not too soon.
 	acct.bonds = append(acct.bonds, acct.pendingBonds[0])
 	acct.pendingBonds = nil
 	acct.bonds[0].LockTime = mergeableLocktimeThresh + 1
 	rig.queuePrevalidateBond()
-	run(1, 0, (overlappedReservesPerTier*2+bondFeeBuffer)-2*bondAsset.Amt)
+	run(1, 0, 2*bondAsset.Amt+bondFeeBuffer)
 	mergingBond := acct.pendingBonds[0]
 	if mergingBond.LockTime != acct.bonds[0].LockTime {
 		t.Fatalf("Mergeable bond was not merged")
@@ -10725,7 +10740,7 @@ func TestRotateBonds(t *testing.T) {
 	acct.pendingBonds = nil
 	acct.bonds[0].LockTime = mergeableLocktimeThresh - 1
 	rig.queuePrevalidateBond()
-	run(1, 0, (overlappedReservesPerTier*2+bondFeeBuffer)-2*bondAsset.Amt)
+	run(1, 0, 2*bondAsset.Amt+bondFeeBuffer)
 	unmergingBond := acct.pendingBonds[0]
 	if unmergingBond.LockTime == acct.bonds[0].LockTime {
 		t.Fatalf("Unmergeable bond was scheduled for merged")

client/core/types.go

@@ -650,18 +650,35 @@ type BondOptions struct {
 	PenaltyComps uint16 `json:"PenaltyComps"`
 }
 
+// ExchangeAuth is data characterizing the state of client bonding.
 type ExchangeAuth struct {
-	Rep             account.Reputation  `json:"rep"`
-	BondAssetID     uint32              `json:"bondAssetID"`
-	PendingStrength int64               `json:"pendingStrength"`
-	WeakStrength    int64               `json:"weakStrength"`
-	LiveStrength    int64               `json:"liveStrength"`
-	TargetTier      uint64              `json:"targetTier"`
-	EffectiveTier   int64               `json:"effectiveTier"`
-	MaxBondedAmt    uint64              `json:"maxBondedAmt"`
-	PenaltyComps    uint16              `json:"penaltyComps"`
-	PendingBonds    []*PendingBondState `json:"pendingBonds"`
-	Compensation    int64               `json:"compensation"`
+	// Rep is the user's Reputation as reported by the DEX server.
+	Rep account.Reputation `json:"rep"`
+	// BondAssetID is the user's currently configured bond asset.
+	BondAssetID uint32 `json:"bondAssetID"`
+	// PendingStrength counts how many tiers are in unconfirmed bonds.
+	PendingStrength int64 `json:"pendingStrength"`
+	// WeakStrength counts the number of tiers that are about to expire.
+	WeakStrength int64 `json:"weakStrength"`
+	// LiveStrength counts all active bond tiers, including weak.
+	LiveStrength int64 `json:"liveStrength"`
+	// TargetTier is the user's current configured tier level.
+	TargetTier uint64 `json:"targetTier"`
+	// EffectiveTier is the user's current tier, after considering reputation.
+	EffectiveTier int64 `json:"effectiveTier"`
+	// MaxBondedAmt is the maximum amount that can be locked in bonds at a given
+	// time. If not provided, a default is calculated based on TargetTier and
+	// PenaltyComps.
+	MaxBondedAmt uint64 `json:"maxBondedAmt"`
+	// PenaltyComps is the maximum number of penalized tiers to automatically
+	// compensate.
+	PenaltyComps uint16 `json:"penaltyComps"`
+	// PendingBonds are currently pending bonds and their confirmation count.
+	PendingBonds []*PendingBondState `json:"pendingBonds"`
+	// Compensation is the amount we have locked in bonds greater than what
+	// is needed to maintain our target tier. This could be from penalty
+	// compensation, or it could be due to the user lowering their target tier.
+	Compensation int64 `json:"compensation"`
 }
 
 // Exchange represents a single DEX with any number of markets.

server/account/account.go

@@ -170,10 +170,18 @@ func (r Rule) Punishable() bool {
 // Reputation is a part of a number of server-originating messages. It was
 // introduced with the v2 ConnectResult.
 type Reputation struct {
-	BondedTier int64  `json:"bondedTier"`
-	Penalties  uint16 `json:"penalties"`
-	Legacy     bool   `json:"legacyTier"`
-	Score      int32  `json:"score"`
+	// BondedTier is the tier indicated by the users active bonds. BondedTier
+	// does not account for penalties.
+	BondedTier int64 `json:"bondedTier"`
+	// Penalties are the number of tiers that are currently revoked due to low
+	// user score.
+	Penalties uint16 `json:"penalties"`
+	// Legacy is true if the server recognizes a legacy registration for this
+	// user. Legacy registration increases effective tier by 1.
+	Legacy bool `json:"legacyTier"`
+	// Score is the user's current score. Score must be evaluated against a
+	// server's configured penalty threshold to calculate penalties.
+	Score int32 `json:"score"`
 }
 
 // Effective calculates the effective tier for trading limit calculations.

server/auth/auth.go

@@ -430,7 +430,7 @@ const (
 func NewAuthManager(cfg *Config) *AuthManager {
 	// A penalty threshold of 0 is not sensible, so have a default.
 	penaltyThreshold := int32(cfg.PenaltyThreshold)
-	if penaltyThreshold == 0 {
+	if penaltyThreshold <= 0 {
 		penaltyThreshold = DefaultPenaltyThreshold
 	}
 	// Invert sign for internal use.