Merge pull request #3197 from filecoin-project/fix/paych-spendable-su…

…bmit-voucher

paych: fix issue where `voucher spendable` fails for submitted vouchers

api/api_full.go

@@ -427,7 +427,7 @@ type FullNode interface {
 	PaychVoucherCreate(context.Context, address.Address, types.BigInt, uint64) (*paych.SignedVoucher, error)
 	PaychVoucherAdd(context.Context, address.Address, *paych.SignedVoucher, []byte, types.BigInt) (types.BigInt, error)
 	PaychVoucherList(context.Context, address.Address) ([]*paych.SignedVoucher, error)
-	PaychVoucherSubmit(context.Context, address.Address, *paych.SignedVoucher) (cid.Cid, error)
+	PaychVoucherSubmit(context.Context, address.Address, *paych.SignedVoucher, []byte, []byte) (cid.Cid, error)
 }
 
 type FileRef struct {

api/apistruct/struct.go

@@ -214,7 +214,7 @@ type FullNodeStruct struct {
 		PaychVoucherAdd            func(context.Context, address.Address, *paych.SignedVoucher, []byte, types.BigInt) (types.BigInt, error)  `perm:"write"`
 		PaychVoucherCreate         func(context.Context, address.Address, big.Int, uint64) (*paych.SignedVoucher, error)                     `perm:"sign"`
 		PaychVoucherList           func(context.Context, address.Address) ([]*paych.SignedVoucher, error)                                    `perm:"write"`
-		PaychVoucherSubmit         func(context.Context, address.Address, *paych.SignedVoucher) (cid.Cid, error)                             `perm:"sign"`
+		PaychVoucherSubmit         func(context.Context, address.Address, *paych.SignedVoucher, []byte, []byte) (cid.Cid, error)             `perm:"sign"`
 	}
 }
 
@@ -920,8 +920,8 @@ func (c *FullNodeStruct) PaychNewPayment(ctx context.Context, from, to address.A
 	return c.Internal.PaychNewPayment(ctx, from, to, vouchers)
 }
 
-func (c *FullNodeStruct) PaychVoucherSubmit(ctx context.Context, ch address.Address, sv *paych.SignedVoucher) (cid.Cid, error) {
-	return c.Internal.PaychVoucherSubmit(ctx, ch, sv)
+func (c *FullNodeStruct) PaychVoucherSubmit(ctx context.Context, ch address.Address, sv *paych.SignedVoucher, secret []byte, proof []byte) (cid.Cid, error) {
+	return c.Internal.PaychVoucherSubmit(ctx, ch, sv, secret, proof)
 }
 
 // StorageMinerStruct

cli/paych.go

@@ -468,7 +468,7 @@ var paychVoucherSubmitCmd = &cli.Command{
 
 		ctx := ReqContext(cctx)
 
-		mcid, err := api.PaychVoucherSubmit(ctx, ch, sv)
+		mcid, err := api.PaychVoucherSubmit(ctx, ch, sv, nil, nil)
 		if err != nil {
 			return err
 		}

cli/paych_test.go

@@ -107,12 +107,14 @@ func TestPaymentChannelVouchers(t *testing.T) {
 	ctx := context.Background()
 	nodes, addrs := startTwoNodesOneMiner(ctx, t, blocktime)
 	paymentCreator := nodes[0]
+	paymentReceiver := nodes[1]
 	creatorAddr := addrs[0]
 	receiverAddr := addrs[1]
 
 	// Create mock CLI
 	mockCLI := newMockCLI(t)
 	creatorCLI := mockCLI.client(paymentCreator.ListenAddr)
+	receiverCLI := mockCLI.client(paymentReceiver.ListenAddr)
 
 	// creator: paych get <creator> <receiver> <amount>
 	channelAmt := "100000"
@@ -127,58 +129,144 @@ func TestPaymentChannelVouchers(t *testing.T) {
 	// creator: paych voucher create <channel> <amount>
 	// Note: implied --lane=0
 	voucherAmt1 := 100
-	vamt1 := strconv.Itoa(voucherAmt1)
-	cmd = []string{chAddr.String(), vamt1}
+	cmd = []string{chAddr.String(), strconv.Itoa(voucherAmt1)}
 	voucher1 := creatorCLI.runCmd(paychVoucherCreateCmd, cmd)
 	vouchers = append(vouchers, voucherSpec{serialized: voucher1, lane: 0, amt: voucherAmt1})
 
 	// creator: paych voucher create <channel> <amount> --lane=5
 	lane5 := "--lane=5"
 	voucherAmt2 := 50
-	vamt2 := strconv.Itoa(voucherAmt2)
-	cmd = []string{lane5, chAddr.String(), vamt2}
+	cmd = []string{lane5, chAddr.String(), strconv.Itoa(voucherAmt2)}
 	voucher2 := creatorCLI.runCmd(paychVoucherCreateCmd, cmd)
 	vouchers = append(vouchers, voucherSpec{serialized: voucher2, lane: 5, amt: voucherAmt2})
 
 	// creator: paych voucher create <channel> <amount> --lane=5
 	voucherAmt3 := 70
-	vamt3 := strconv.Itoa(voucherAmt3)
-	cmd = []string{lane5, chAddr.String(), vamt3}
+	cmd = []string{lane5, chAddr.String(), strconv.Itoa(voucherAmt3)}
 	voucher3 := creatorCLI.runCmd(paychVoucherCreateCmd, cmd)
 	vouchers = append(vouchers, voucherSpec{serialized: voucher3, lane: 5, amt: voucherAmt3})
 
+	// creator: paych voucher create <channel> <amount> --lane=5
+	voucherAmt4 := 80
+	cmd = []string{lane5, chAddr.String(), strconv.Itoa(voucherAmt4)}
+	voucher4 := creatorCLI.runCmd(paychVoucherCreateCmd, cmd)
+	vouchers = append(vouchers, voucherSpec{serialized: voucher4, lane: 5, amt: voucherAmt4})
+
 	// creator: paych voucher list <channel> --export
 	cmd = []string{"--export", chAddr.String()}
 	list := creatorCLI.runCmd(paychVoucherListCmd, cmd)
 
-	// Check that voucher list output is correct
+	// Check that voucher list output is correct on creator
 	checkVoucherOutput(t, list, vouchers)
 
 	// creator: paych voucher best-spendable <channel>
 	cmd = []string{"--export", chAddr.String()}
 	bestSpendable := creatorCLI.runCmd(paychVoucherBestSpendableCmd, cmd)
 
-	// Check that best spendable output is correct
+	// Check that best spendable output is correct on creator
 	bestVouchers := []voucherSpec{
 		{serialized: voucher1, lane: 0, amt: voucherAmt1},
-		{serialized: voucher3, lane: 5, amt: voucherAmt3},
+		{serialized: voucher4, lane: 5, amt: voucherAmt4},
+	}
+	checkVoucherOutput(t, bestSpendable, bestVouchers)
+
+	// receiver: paych voucher add <voucher>
+	cmd = []string{chAddr.String(), voucher1}
+	receiverCLI.runCmd(paychVoucherAddCmd, cmd)
+
+	// receiver: paych voucher add <voucher>
+	cmd = []string{chAddr.String(), voucher2}
+	receiverCLI.runCmd(paychVoucherAddCmd, cmd)
+
+	// receiver: paych voucher add <voucher>
+	cmd = []string{chAddr.String(), voucher3}
+	receiverCLI.runCmd(paychVoucherAddCmd, cmd)
+
+	// receiver: paych voucher add <voucher>
+	cmd = []string{chAddr.String(), voucher4}
+	receiverCLI.runCmd(paychVoucherAddCmd, cmd)
+
+	// receiver: paych voucher list <channel> --export
+	cmd = []string{"--export", chAddr.String()}
+	list = receiverCLI.runCmd(paychVoucherListCmd, cmd)
+
+	// Check that voucher list output is correct on receiver
+	checkVoucherOutput(t, list, vouchers)
+
+	// receiver: paych voucher best-spendable <channel>
+	cmd = []string{"--export", chAddr.String()}
+	bestSpendable = receiverCLI.runCmd(paychVoucherBestSpendableCmd, cmd)
+
+	// Check that best spendable output is correct on receiver
+	bestVouchers = []voucherSpec{
+		{serialized: voucher1, lane: 0, amt: voucherAmt1},
+		{serialized: voucher4, lane: 5, amt: voucherAmt4},
+	}
+	checkVoucherOutput(t, bestSpendable, bestVouchers)
+
+	// receiver: paych voucher submit <channel> <voucher>
+	cmd = []string{chAddr.String(), voucher1}
+	receiverCLI.runCmd(paychVoucherSubmitCmd, cmd)
+
+	// receiver: paych voucher best-spendable <channel>
+	cmd = []string{"--export", chAddr.String()}
+	bestSpendable = receiverCLI.runCmd(paychVoucherBestSpendableCmd, cmd)
+
+	// Check that best spendable output no longer includes submitted voucher
+	bestVouchers = []voucherSpec{
+		{serialized: voucher4, lane: 5, amt: voucherAmt4},
+	}
+	checkVoucherOutput(t, bestSpendable, bestVouchers)
+
+	// There are three vouchers in lane 5: 50, 70, 80
+	// Submit the voucher for 50. Best spendable should still be 80.
+	// receiver: paych voucher submit <channel> <voucher>
+	cmd = []string{chAddr.String(), voucher2}
+	receiverCLI.runCmd(paychVoucherSubmitCmd, cmd)
+
+	// receiver: paych voucher best-spendable <channel>
+	cmd = []string{"--export", chAddr.String()}
+	bestSpendable = receiverCLI.runCmd(paychVoucherBestSpendableCmd, cmd)
+
+	// Check that best spendable output still includes the voucher for 80
+	bestVouchers = []voucherSpec{
+		{serialized: voucher4, lane: 5, amt: voucherAmt4},
 	}
 	checkVoucherOutput(t, bestSpendable, bestVouchers)
+
+	// Submit the voucher for 80
+	// receiver: paych voucher submit <channel> <voucher>
+	cmd = []string{chAddr.String(), voucher4}
+	receiverCLI.runCmd(paychVoucherSubmitCmd, cmd)
+
+	// receiver: paych voucher best-spendable <channel>
+	cmd = []string{"--export", chAddr.String()}
+	bestSpendable = receiverCLI.runCmd(paychVoucherBestSpendableCmd, cmd)
+
+	// Check that best spendable output no longer includes submitted voucher
+	bestVouchers = []voucherSpec{}
+	checkVoucherOutput(t, bestSpendable, bestVouchers)
 }
 
 func checkVoucherOutput(t *testing.T, list string, vouchers []voucherSpec) {
 	lines := strings.Split(list, "\n")
 	listVouchers := make(map[string]string)
 	for _, line := range lines {
 		parts := strings.Split(line, ";")
-		serialized := strings.TrimSpace(parts[1])
-		listVouchers[serialized] = strings.TrimSpace(parts[0])
+		if len(parts) == 2 {
+			serialized := strings.TrimSpace(parts[1])
+			listVouchers[serialized] = strings.TrimSpace(parts[0])
+		}
 	}
 	for _, vchr := range vouchers {
 		res, ok := listVouchers[vchr.serialized]
 		require.True(t, ok)
 		require.Regexp(t, fmt.Sprintf("Lane %d", vchr.lane), res)
 		require.Regexp(t, fmt.Sprintf("%d", vchr.amt), res)
+		delete(listVouchers, vchr.serialized)
+	}
+	for _, vchr := range listVouchers {
+		require.Fail(t, "Extra voucher "+vchr)
 	}
 }
 

documentation/en/api-methods.md

@@ -2437,7 +2437,9 @@ Inputs:
       "Type": 2,
       "Data": "Ynl0ZSBhcnJheQ=="
     }
-  }
+  },
+  "Ynl0ZSBhcnJheQ==",
+  "Ynl0ZSBhcnJheQ=="
 ]
 ```
 

node/impl/paych/paych.go

@@ -2,18 +2,15 @@ package paych
 
 import (
 	"context"
-	"fmt"
 
 	"github.com/ipfs/go-cid"
 	"go.uber.org/fx"
 	"golang.org/x/xerrors"
 
 	"github.com/filecoin-project/go-address"
-	"github.com/filecoin-project/specs-actors/actors/builtin"
 	"github.com/filecoin-project/specs-actors/actors/builtin/paych"
 
 	"github.com/filecoin-project/lotus/api"
-	"github.com/filecoin-project/lotus/chain/actors"
 	"github.com/filecoin-project/lotus/chain/types"
 	full "github.com/filecoin-project/lotus/node/impl/full"
 	"github.com/filecoin-project/lotus/paychmgr"
@@ -183,35 +180,6 @@ func (a *PaychAPI) PaychVoucherList(ctx context.Context, pch address.Address) ([
 	return out, nil
 }
 
-func (a *PaychAPI) PaychVoucherSubmit(ctx context.Context, ch address.Address, sv *paych.SignedVoucher) (cid.Cid, error) {
-	ci, err := a.PaychMgr.GetChannelInfo(ch)
-	if err != nil {
-		return cid.Undef, err
-	}
-
-	if sv.Extra != nil || len(sv.SecretPreimage) > 0 {
-		return cid.Undef, fmt.Errorf("cant handle more advanced payment channel stuff yet")
-	}
-
-	enc, err := actors.SerializeParams(&paych.UpdateChannelStateParams{
-		Sv: *sv,
-	})
-	if err != nil {
-		return cid.Undef, err
-	}
-
-	msg := &types.Message{
-		From:   ci.Control,
-		To:     ch,
-		Value:  types.NewInt(0),
-		Method: builtin.MethodsPaych.UpdateChannelState,
-		Params: enc,
-	}
-
-	smsg, err := a.MpoolPushMessage(ctx, msg, nil)
-	if err != nil {
-		return cid.Undef, err
-	}
-
-	return smsg.Cid(), nil
+func (a *PaychAPI) PaychVoucherSubmit(ctx context.Context, ch address.Address, sv *paych.SignedVoucher, secret []byte, proof []byte) (cid.Cid, error) {
+	return a.PaychMgr.SubmitVoucher(ctx, ch, sv, secret, proof)
 }

paychmgr/manager.go

@@ -285,6 +285,14 @@ func (pm *Manager) trackInboundChannel(ctx context.Context, ch address.Address)
 	return pm.store.TrackChannel(stateCi)
 }
 
+func (pm *Manager) SubmitVoucher(ctx context.Context, ch address.Address, sv *paych.SignedVoucher, secret []byte, proof []byte) (cid.Cid, error) {
+	ca, err := pm.accessorByAddress(ch)
+	if err != nil {
+		return cid.Undef, err
+	}
+	return ca.submitVoucher(ctx, ch, sv, secret, proof)
+}
+
 func (pm *Manager) AllocateLane(ch address.Address) (uint64, error) {
 	ca, err := pm.accessorByAddress(ch)
 	if err != nil {

paychmgr/mock_test.go

@@ -40,6 +40,7 @@ type mockStateManager struct {
 	paychState   map[address.Address]mockPchState
 	store        adt.Store
 	response     *api.InvocResult
+	lastCall     *types.Message
 }
 
 func newMockStateManager() *mockStateManager {
@@ -93,7 +94,26 @@ func (sm *mockStateManager) LoadActorState(ctx context.Context, a address.Addres
 	panic(fmt.Sprintf("unexpected state type %v", out))
 }
 
+func (sm *mockStateManager) setCallResponse(response *api.InvocResult) {
+	sm.lk.Lock()
+	defer sm.lk.Unlock()
+
+	sm.response = response
+}
+
+func (sm *mockStateManager) getLastCall() *types.Message {
+	sm.lk.Lock()
+	defer sm.lk.Unlock()
+
+	return sm.lastCall
+}
+
 func (sm *mockStateManager) Call(ctx context.Context, msg *types.Message, ts *types.TipSet) (*api.InvocResult, error) {
+	sm.lk.Lock()
+	defer sm.lk.Unlock()
+
+	sm.lastCall = msg
+
 	return sm.response, nil
 }