feat: account for other vouchers when calculating voucher validity

paychmgr/paych.go

@@ -5,6 +5,8 @@ import (
 	"context"
 	"fmt"
 
+	"github.com/filecoin-project/specs-actors/actors/abi/big"
+
 	"github.com/filecoin-project/lotus/api"
 
 	cborutil "github.com/filecoin-project/go-cbor-util"
@@ -99,14 +101,14 @@ func (pm *Manager) CheckVoucherValid(ctx context.Context, ch address.Address, sv
 	return err
 }
 
-func (pm *Manager) checkVoucherValid(ctx context.Context, ch address.Address, sv *paych.SignedVoucher) (*paych.State, error) {
+func (pm *Manager) checkVoucherValid(ctx context.Context, ch address.Address, sv *paych.SignedVoucher) (map[uint64]*paych.LaneState, error) {
-	act, pca, err := pm.loadPaychState(ctx, ch)
+	act, pchState, err := pm.loadPaychState(ctx, ch)
 	if err != nil {
 		return nil, err
 	}
 
 	var account account.State
-	_, err = pm.sm.LoadActorState(ctx, pca.From, &account, nil)
+	_, err = pm.sm.LoadActorState(ctx, pchState.From, &account, nil)
 	if err != nil {
 		return nil, err
 	}
@@ -125,29 +127,47 @@ func (pm *Manager) checkVoucherValid(ctx context.Context, ch address.Address, sv
 		return nil, err
 	}
 
-	sendAmount := sv.Amount
-
 	// Check the voucher against the highest known voucher nonce / value
-	ls, err := pm.laneState(pca, ch, sv.Lane)
+	laneStates, err := pm.laneState(pchState, ch)
 	if err != nil {
 		return nil, err
 	}
-	// If there has been at least once voucher redeemed, and the voucher
+
-	// nonce value is less than the highest known nonce
+	// If the new voucher nonce value is less than the highest known
-	if ls.Redeemed.Int64() > 0 && sv.Nonce <= ls.Nonce {
+	// nonce for the lane
+	ls, lsExists := laneStates[sv.Lane]
+	if lsExists && sv.Nonce <= ls.Nonce {
 		return nil, fmt.Errorf("nonce too low")
 	}
+
 	// If the voucher amount is less than the highest known voucher amount
-	if sv.Amount.LessThanEqual(ls.Redeemed) {
+	if lsExists && sv.Amount.LessThanEqual(ls.Redeemed) {
 		return nil, fmt.Errorf("voucher amount is lower than amount for voucher with lower nonce")
 	}
 
-	// Only send the difference between the voucher amount and what has already
+	// Total redeemed is the total redeemed amount for all lanes, including
-	// been redeemed
+	// the new voucher
-	sendAmount = types.BigSub(sv.Amount, ls.Redeemed)
+	// eg
+	//
+	// lane 1 redeemed:            3
+	// lane 2 redeemed:            2
+	// voucher for lane 1:         5
+	//
+	// Voucher supersedes lane 1 redeemed, therefore
+	// effective lane 1 redeemed:  5
+	//
+	// lane 1:  5
+	// lane 2:  2
+	//          -
+	// total:   7
+	totalRedeemed, err := pm.totalRedeemedWithVoucher(laneStates, sv)
+	if err != nil {
+		return nil, err
+	}
 
-	// TODO: also account for vouchers on other lanes we've received
+	// Total required balance = total redeemed + toSend
-	newTotal := types.BigAdd(sendAmount, pca.ToSend)
+	// Must not exceed actor balance
+	newTotal := types.BigAdd(totalRedeemed, pchState.ToSend)
 	if act.Balance.LessThan(newTotal) {
 		return nil, fmt.Errorf("not enough funds in channel to cover voucher")
 	}
@@ -156,7 +176,7 @@ func (pm *Manager) checkVoucherValid(ctx context.Context, ch address.Address, sv
 		return nil, fmt.Errorf("dont currently support paych lane merges")
 	}
 
-	return pca, nil
+	return laneStates, nil
 }
 
 // CheckVoucherSpendable checks if the given voucher is currently spendable
@@ -260,21 +280,22 @@ func (pm *Manager) AddVoucher(ctx context.Context, ch address.Address, sv *paych
 	}
 
 	// Check voucher validity
-	pchState, err := pm.checkVoucherValid(ctx, ch, sv)
+	laneStates, err := pm.checkVoucherValid(ctx, ch, sv)
 	if err != nil {
 		return types.NewInt(0), err
 	}
 
 	// The change in value is the delta between the voucher amount and
-	// the highest previous voucher amount
+	// the highest previous voucher amount for the lane
-	laneState, err := pm.laneState(pchState, ch, sv.Lane)
+	laneState, exists := laneStates[sv.Lane]
-	if err != nil {
+	redeemed := big.NewInt(0)
-		return types.NewInt(0), err
+	if exists {
+		redeemed = laneState.Redeemed
 	}
 
-	delta := types.BigSub(sv.Amount, laneState.Redeemed)
+	delta := types.BigSub(sv.Amount, redeemed)
 	if minDelta.GreaterThan(delta) {
-		return delta, xerrors.Errorf("addVoucher: supplied token amount too low; minD=%s, D=%s; laneAmt=%s; v.Amt=%s", minDelta, delta, laneState.Redeemed, sv.Amount)
+		return delta, xerrors.Errorf("addVoucher: supplied token amount too low; minD=%s, D=%s; laneAmt=%s; v.Amt=%s", minDelta, delta, redeemed, sv.Amount)
 	}
 
 	ci.Vouchers = append(ci.Vouchers, &VoucherInfo{

paychmgr/paych_test.go

@@ -262,21 +262,42 @@ func TestCheckVoucherValid(t *testing.T) {
 		toSend:        big.NewInt(9),
 		voucherAmount: big.NewInt(2),
 	}, {
-		// required balance = toSend + (voucher - redeemed)
+		// voucher supersedes lane 1 redeemed so
-		//                  = 0 + (11 - 2)
+		// lane 1 effective redeemed = voucher amount
-		//                  = 9
+		//
-		// So required balance: 9 < actor balance: 10
+		// required balance = toSend + total redeemed
-		name:          "passes when voucher - redeemed < balance",
+		//                  = 1 + 6 (lane1)
+		//                  = 7
+		// So required balance: 7 < actor balance: 10
+		name:          "passes when voucher + total redeemed <= balance",
 		key:           fromKeyPrivate,
 		actorBalance:  big.NewInt(10),
-		toSend:        big.NewInt(0),
+		toSend:        big.NewInt(1),
-		voucherAmount: big.NewInt(11),
+		voucherAmount: big.NewInt(6),
 		voucherLane:   1,
-		voucherNonce:  3,
+		voucherNonce:  2,
 		laneStates: []*paych.LaneState{{
-			ID:       1,
+			ID:       1, // Lane 1 (same as voucher lane 1)
-			Redeemed: big.NewInt(2),
+			Redeemed: big.NewInt(4),
-			Nonce:    2,
+			Nonce:    1,
+		}},
+	}, {
+		// required balance = toSend + total redeemed
+		//                  = 1 + 4 (lane 2) + 6 (voucher lane 1)
+		//                  = 11
+		// So required balance: 11 > actor balance: 10
+		name:          "fails when voucher + total redeemed > balance",
+		expectError:   true,
+		key:           fromKeyPrivate,
+		actorBalance:  big.NewInt(10),
+		toSend:        big.NewInt(1),
+		voucherAmount: big.NewInt(6),
+		voucherLane:   1,
+		voucherNonce:  1,
+		laneStates: []*paych.LaneState{{
+			ID:       2, // Lane 2 (different from voucher lane 1)
+			Redeemed: big.NewInt(4),
+			Nonce:    1,
 		}},
 	}}
 
@@ -316,6 +337,139 @@ func TestCheckVoucherValid(t *testing.T) {
 	}
 }
 
+func TestCheckVoucherValidCountingAllLanes(t *testing.T) {
+	ctx := context.Background()
+
+	fromKeyPrivate, fromKeyPublic := testGenerateKeyPair(t)
+
+	ch := tutils.NewIDAddr(t, 100)
+	from := tutils.NewSECP256K1Addr(t, string(fromKeyPublic))
+	to := tutils.NewSECP256K1Addr(t, "secpTo")
+	fromAcct := tutils.NewActorAddr(t, "fromAct")
+	toAcct := tutils.NewActorAddr(t, "toAct")
+	minDelta := big.NewInt(0)
+
+	sm := newMockStateManager()
+	sm.setAccountState(fromAcct, account.State{Address: from})
+	sm.setAccountState(toAcct, account.State{Address: to})
+
+	store := NewStore(ds_sync.MutexWrap(ds.NewMapDatastore()))
+
+	actorBalance := big.NewInt(10)
+	toSend := big.NewInt(1)
+	laneStates := []*paych.LaneState{{
+		ID:       1,
+		Nonce:    1,
+		Redeemed: big.NewInt(3),
+	}, {
+		ID:       2,
+		Nonce:    1,
+		Redeemed: big.NewInt(4),
+	}}
+
+	act := &types.Actor{
+		Code:    builtin.AccountActorCodeID,
+		Head:    cid.Cid{},
+		Nonce:   0,
+		Balance: actorBalance,
+	}
+	sm.setPaychState(ch, act, paych.State{
+		From:            fromAcct,
+		To:              toAcct,
+		ToSend:          toSend,
+		SettlingAt:      abi.ChainEpoch(0),
+		MinSettleHeight: abi.ChainEpoch(0),
+		LaneStates:      laneStates,
+	})
+
+	mgr := newManager(sm, store)
+	err := mgr.TrackInboundChannel(ctx, ch)
+	require.NoError(t, err)
+
+	//
+	// Should not be possible to add a voucher with a value such that
+	// <total lane Redeemed> + toSend > <actor balance>
+	//
+	// lane 1 redeemed:                   3
+	// voucher amount (lane 1):           6
+	// lane 1 redeemed (with voucher):    6
+	//
+	// Lane 1:             6
+	// Lane 2:             4
+	// toSend:             1
+	//                     --
+	// total:              11
+	//
+	// actor balance is 10 so total is too high.
+	//
+	voucherLane := uint64(1)
+	voucherNonce := uint64(2)
+	voucherAmount := big.NewInt(6)
+	sv := testCreateVoucher(t, voucherLane, voucherNonce, voucherAmount, fromKeyPrivate)
+	err = mgr.CheckVoucherValid(ctx, ch, sv)
+	require.Error(t, err)
+
+	//
+	// lane 1 redeemed:                   3
+	// voucher amount (lane 1):           4
+	// lane 1 redeemed (with voucher):    4
+	//
+	// Lane 1:             4
+	// Lane 2:             4
+	// toSend:             1
+	//                     --
+	// total:              9
+	//
+	// actor balance is 10 so total is ok.
+	//
+	voucherAmount = big.NewInt(4)
+	sv = testCreateVoucher(t, voucherLane, voucherNonce, voucherAmount, fromKeyPrivate)
+	err = mgr.CheckVoucherValid(ctx, ch, sv)
+	require.NoError(t, err)
+
+	// Add voucher to lane 1, so Lane 1 effective redeemed
+	// (with first voucher) is now 4
+	_, err = mgr.AddVoucher(ctx, ch, sv, nil, minDelta)
+	require.NoError(t, err)
+
+	//
+	// lane 1 redeemed:                   4
+	// voucher amount (lane 1):           6
+	// lane 1 redeemed (with voucher):    6
+	//
+	// Lane 1:             6
+	// Lane 2:             4
+	// toSend:             1
+	//                     --
+	// total:              11
+	//
+	// actor balance is 10 so total is too high.
+	//
+	voucherNonce++
+	voucherAmount = big.NewInt(6)
+	sv = testCreateVoucher(t, voucherLane, voucherNonce, voucherAmount, fromKeyPrivate)
+	err = mgr.CheckVoucherValid(ctx, ch, sv)
+	require.Error(t, err)
+
+	//
+	// lane 1 redeemed:                   4
+	// voucher amount (lane 1):           5
+	// lane 1 redeemed (with voucher):    5
+	//
+	// Lane 1:             5
+	// Lane 2:             4
+	// toSend:             1
+	//                     --
+	// total:              10
+	//
+	// actor balance is 10 so total is ok.
+	//
+	voucherAmount = big.NewInt(5)
+	sv = testCreateVoucher(t, voucherLane, voucherNonce, voucherAmount, fromKeyPrivate)
+	err = mgr.CheckVoucherValid(ctx, ch, sv)
+	require.NoError(t, err)
+}
+
 func TestAddVoucherDelta(t *testing.T) {
 	ctx := context.Background()
 
@@ -524,7 +678,7 @@ func testSetupMgrWithChannel(ctx context.Context, t *testing.T) (*Manager, addre
 		Code:    builtin.AccountActorCodeID,
 		Head:    cid.Cid{},
 		Nonce:   0,
-		Balance: big.NewInt(10),
+		Balance: big.NewInt(20),
 	}
 	sm.setPaychState(ch, act, paych.State{
 		From:            fromAcct,

paychmgr/state.go

@@ -3,6 +3,8 @@ package paychmgr
 import (
 	"context"
 
+	"github.com/filecoin-project/specs-actors/actors/abi/big"
+
 	"github.com/filecoin-project/specs-actors/actors/builtin/account"
 
 	"github.com/filecoin-project/go-address"
@@ -71,52 +73,42 @@ func nextLaneFromState(st *paych.State) uint64 {
 	return maxLane + 1
 }
 
-func findLane(states []*paych.LaneState, lane uint64) *paych.LaneState {
+// laneState gets the LaneStates from chain, then applies all vouchers in
-	var ls *paych.LaneState
+// the data store over the chain state
-	for _, laneState := range states {
+func (pm *Manager) laneState(state *paych.State, ch address.Address) (map[uint64]*paych.LaneState, error) {
-		if laneState.ID == lane {
-			ls = laneState
-			break
-		}
-	}
-	return ls
-}
-
-func (pm *Manager) laneState(state *paych.State, ch address.Address, lane uint64) (paych.LaneState, error) {
 	// TODO: we probably want to call UpdateChannelState with all vouchers to be fully correct
 	//  (but technically dont't need to)
-	// TODO: make sure this is correct
+	laneStates := make(map[uint64]*paych.LaneState, len(state.LaneStates))
 
 	// Get the lane state from the chain
-	ls := findLane(state.LaneStates, lane)
+	for _, laneState := range state.LaneStates {
-	if ls == nil {
+		laneStates[laneState.ID] = laneState
-		ls = &paych.LaneState{
-			ID:       lane,
-			Redeemed: types.NewInt(0),
-			Nonce:    0,
-		}
 	}
 
 	// Apply locally stored vouchers
 	vouchers, err := pm.store.VouchersForPaych(ch)
-	if err != nil {
+	if err != nil && err != ErrChannelNotTracked {
-		if err == ErrChannelNotTracked {
+		return nil, err
-			return *ls, nil
-		}
-		return paych.LaneState{}, err
 	}
 
 	for _, v := range vouchers {
 		for range v.Voucher.Merges {
-			return paych.LaneState{}, xerrors.Errorf("paych merges not handled yet")
+			return nil, xerrors.Errorf("paych merges not handled yet")
 		}
 
-		if v.Voucher.Lane != lane {
+		// If there's a voucher for a lane that isn't in chain state just
-			continue
+		// create it
+		ls, ok := laneStates[v.Voucher.Lane]
+		if !ok {
+			ls = &paych.LaneState{
+				ID:       v.Voucher.Lane,
+				Redeemed: types.NewInt(0),
+				Nonce:    0,
+			}
+			laneStates[v.Voucher.Lane] = ls
 		}
 
 		if v.Voucher.Nonce < ls.Nonce {
-			log.Warnf("Found outdated voucher: ch=%s, lane=%d, v.nonce=%d lane.nonce=%d", ch, lane, v.Voucher.Nonce, ls.Nonce)
 			continue
 		}
 
@@ -124,5 +116,31 @@ func (pm *Manager) laneState(state *paych.State, ch address.Address, lane uint64
 		ls.Redeemed = v.Voucher.Amount
 	}
 
-	return *ls, nil
+	return laneStates, nil
+}
+
+// Get the total redeemed amount across all lanes, after applying the voucher
+func (pm *Manager) totalRedeemedWithVoucher(laneStates map[uint64]*paych.LaneState, sv *paych.SignedVoucher) (big.Int, error) {
+	total := big.NewInt(0)
+	for _, ls := range laneStates {
+		total = big.Add(total, ls.Redeemed)
+	}
+
+	lane, ok := laneStates[sv.Lane]
+	if ok {
+		// If the voucher is for an existing lane, and the voucher nonce
+		// and is higher than the lane nonce
+		if sv.Nonce > lane.Nonce {
+			// Add the delta between the redeemed amount and the voucher
+			// amount to the total
+			delta := big.Sub(sv.Amount, lane.Redeemed)
+			total = big.Add(total, delta)
+		}
+	} else {
+		// If the voucher is *not* for an existing lane, just add its
+		// value (implicitly a new lane will be created for the voucher)
+		total = big.Add(total, sv.Amount)
+	}
+
+	return total, nil
 }