paych: fix voucher amount verification

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)
 

chain/actors/builtin/paych/mock/mock.go

@@ -27,10 +27,9 @@ type mockLaneState struct {
 func NewMockPayChState(from address.Address,
 	to address.Address,
 	settlingAt abi.ChainEpoch,
-	toSend abi.TokenAmount,
 	lanes map[uint64]paych.LaneState,
 ) paych.State {
-	return &mockState{from, to, settlingAt, toSend, lanes}
+	return &mockState{from: from, to: to, settlingAt: settlingAt, toSend: big.NewInt(0), lanes: lanes}
 }
 
 // NewMockLaneState constructs a state for a payment channel lane with the set fixed values

paychmgr/paych.go

@@ -205,7 +205,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(pchState, ch)
 	if err != nil {
 		return nil, err
 	}
@@ -253,16 +253,9 @@ func (ca *channelAccessor) checkVoucherValidUnlocked(ctx context.Context, ch add
 		return nil, err
 	}
 
-	// Total required balance = total redeemed + toSend
-	// Must not exceed actor balance
-	ts, err := pchState.ToSend()
-	if err != nil {
-		return nil, err
-	}
-
-	newTotal := types.BigAdd(totalRedeemed, ts)
-	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 {
@@ -505,7 +498,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)
 }
 
@@ -520,7 +512,7 @@ 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) {
+func (ca *channelAccessor) laneState(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)
 
@@ -552,9 +544,12 @@ func (ca *channelAccessor) laneState(ctx context.Context, state paych.State, ch
 			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
+		// Check if there is an existing laneState in the payment channel
+		// for this voucher's lane
 		ls, ok := laneStates[v.Voucher.Lane]
+
+		// If the voucher does not have a higher nonce than the existing
+		// laneState for this lane, ignore it
 		if ok {
 			n, err := ls.Nonce()
 			if err != nil {
@@ -565,6 +560,7 @@ func (ca *channelAccessor) laneState(ctx context.Context, state paych.State, ch
 			}
 		}
 
+		// Voucher has a higher nonce, so replace laneState with this voucher
 		laneStates[v.Voucher.Lane] = laneState{v.Voucher.Amount, v.Voucher.Nonce}
 	}