fix: verify voucher amount check

api/test/paych.go

@@ -67,7 +67,7 @@ func TestPaymentChannels(t *testing.T, b APIBuilder, blocktime time.Duration) {
 		t.Fatal(err)
 	}
 
-	channelAmt := int64(100000)
+	channelAmt := int64(7000)
 	channelInfo, err := paymentCreator.PaychGet(ctx, createrAddr, receiverAddr, abi.NewTokenAmount(channelAmt))
 	if err != nil {
 		t.Fatal(err)
@@ -169,6 +169,51 @@ func TestPaymentChannels(t *testing.T, b APIBuilder, blocktime time.Duration) {
 		t.Fatal("Timed out waiting for receiver to submit vouchers")
 	}
 
+	// Create a new voucher now that some vouchers have already been submitted
+	vouchRes, err := paymentCreator.PaychVoucherCreate(ctx, channel, abi.NewTokenAmount(1000), 3)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if vouchRes.Voucher == nil {
+		t.Fatal(fmt.Errorf("Not enough funds to create voucher: missing %d", vouchRes.Shortfall))
+	}
+	vdelta, err := paymentReceiver.PaychVoucherAdd(ctx, channel, vouchRes.Voucher, nil, abi.NewTokenAmount(1000))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !vdelta.Equals(abi.NewTokenAmount(1000)) {
+		t.Fatal("voucher didn't have the right amount")
+	}
+
+	// Create a new voucher whose value would exceed the channel balance
+	excessAmt := abi.NewTokenAmount(1000)
+	vouchRes, err = paymentCreator.PaychVoucherCreate(ctx, channel, excessAmt, 4)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if vouchRes.Voucher != nil {
+		t.Fatal("Expected not to be able to create voucher whose value would exceed channel balance")
+	}
+	if !vouchRes.Shortfall.Equals(excessAmt) {
+		t.Fatal(fmt.Errorf("Expected voucher shortfall of %d, got %d", excessAmt, vouchRes.Shortfall))
+	}
+
+	// Add a voucher whose value would exceed the channel balance
+	vouch := &paych.SignedVoucher{ChannelAddr: channel, Amount: excessAmt, Lane: 4, Nonce: 1}
+	vb, err := vouch.SigningBytes()
+	if err != nil {
+		t.Fatal(err)
+	}
+	sig, err := paymentCreator.WalletSign(ctx, createrAddr, vb)
+	if err != nil {
+		t.Fatal(err)
+	}
+	vouch.Signature = sig
+	_, err = paymentReceiver.PaychVoucherAdd(ctx, channel, vouch, nil, abi.NewTokenAmount(1000))
+	if err == nil {
+		t.Fatal(fmt.Errorf("Expected shortfall error of %d", excessAmt))
+	}
+
 	// wait for the settlement period to pass before collecting
 	waitForBlocks(ctx, t, bm, paymentReceiver, receiverAddr, paych.SettleDelay)
 

paychmgr/mock_test.go

@@ -68,6 +68,13 @@ func (sm *mockStateManager) setAccountState(a address.Address, state account.Sta
 func (sm *mockStateManager) setPaychState(a address.Address, actor *types.Actor, state paych.State) {
 	sm.lk.Lock()
 	defer sm.lk.Unlock()
+
+	laneStates, err := adt.MakeEmptyArray(sm.store).Root()
+	if err != nil {
+		panic(err)
+	}
+	state.LaneStates = laneStates
+
 	sm.paychState[a] = mockPchState{actor, state}
 }
 

paychmgr/paych.go

@@ -7,8 +7,6 @@ import (
 
 	"github.com/filecoin-project/lotus/api"
 
-	"github.com/filecoin-project/specs-actors/actors/util/adt"
-
 	"github.com/ipfs/go-cid"
 
 	"github.com/filecoin-project/go-address"
@@ -191,7 +189,7 @@ func (ca *channelAccessor) checkVoucherValidUnlocked(ctx context.Context, ch add
 	}
 
 	// Check the voucher against the highest known voucher nonce / value
-	laneStates, err := ca.laneState(ctx, pchState, ch)
+	laneStates, err := ca.laneState(ch)
 	if err != nil {
 		return nil, err
 	}
@@ -228,11 +226,9 @@ func (ca *channelAccessor) checkVoucherValidUnlocked(ctx context.Context, ch add
 		return nil, err
 	}
 
-	// Total required balance = total redeemed + toSend
-	// Must not exceed actor balance
-	newTotal := types.BigAdd(totalRedeemed, pchState.ToSend)
-	if act.Balance.LessThan(newTotal) {
-		return nil, newErrInsufficientFunds(types.BigSub(newTotal, act.Balance))
+	// Total required balance must not exceed actor balance
+	if act.Balance.LessThan(totalRedeemed) {
+		return nil, newErrInsufficientFunds(types.BigSub(totalRedeemed, act.Balance))
 	}
 
 	if len(sv.Merges) != 0 {
@@ -472,7 +468,6 @@ func (ca *channelAccessor) allocateLane(ch address.Address) (uint64, error) {
 	ca.lk.Lock()
 	defer ca.lk.Unlock()
 
-	// TODO: should this take into account lane state?
 	return ca.store.AllocateLane(ch)
 }
 
@@ -487,44 +482,23 @@ func (ca *channelAccessor) listVouchers(ctx context.Context, ch address.Address)
 
 // laneState gets the LaneStates from chain, then applies all vouchers in
 // the data store over the chain state
-func (ca *channelAccessor) laneState(ctx context.Context, state *paych.State, ch address.Address) (map[uint64]*paych.LaneState, error) {
-	// TODO: we probably want to call UpdateChannelState with all vouchers to be fully correct
-	//  (but technically dont't need to)
-
-	// Get the lane state from the chain
-	store := ca.api.AdtStore(ctx)
-	lsamt, err := adt.AsArray(store, state.LaneStates)
-	if err != nil {
-		return nil, err
-	}
-
-	// Note: we use a map instead of an array to store laneStates because the
-	// client sets the lane ID (the index) and potentially they could use a
-	// very large index.
-	var ls paych.LaneState
-	laneStates := make(map[uint64]*paych.LaneState, lsamt.Length())
-	err = lsamt.ForEach(&ls, func(i int64) error {
-		current := ls
-		laneStates[uint64(i)] = &current
-		return nil
-	})
-	if err != nil {
-		return nil, err
-	}
-
+func (ca *channelAccessor) laneState(ch address.Address) (map[uint64]*paych.LaneState, error) {
 	// Apply locally stored vouchers
 	vouchers, err := ca.store.VouchersForPaych(ch)
 	if err != nil && err != ErrChannelNotTracked {
 		return nil, err
 	}
 
+	// Note: we use a map instead of an array to store laneStates because the
+	// client sets the lane ID (the index) and potentially they could use a
+	// very large index.
+	laneStates := make(map[uint64]*paych.LaneState, len(vouchers))
 	for _, v := range vouchers {
 		for range v.Voucher.Merges {
 			return nil, xerrors.Errorf("paych merges not handled yet")
 		}
 
-		// If there's a voucher for a lane that isn't in chain state just
-		// create it
+		// Create lane for voucher if it hasn't already been created
 		ls, ok := laneStates[v.Voucher.Lane]
 		if !ok {
 			ls = &paych.LaneState{
@@ -534,6 +508,7 @@ func (ca *channelAccessor) laneState(ctx context.Context, state *paych.State, ch
 			laneStates[v.Voucher.Lane] = ls
 		}
 
+		// Vouchers with a higher nonce overwrite vouchers with a lower nonce
 		if v.Voucher.Nonce < ls.Nonce {
 			continue
 		}