remove proof parameter from payment channels

It never worked properly, and will be removed in actors v2.

paychmgr/manager.go

@@ -2,6 +2,7 @@ package paychmgr
 
 import (
 	"context"
+	"errors"
 	"sync"
 
 	"github.com/ipfs/go-cid"
@@ -24,6 +25,8 @@ import (
 
 var log = logging.Logger("paych")
 
+var errProofNotSupported = errors.New("payment channel proof parameter is not supported")
+
 // PaychAPI is used by dependency injection to pass the consituent APIs to NewManager()
 type PaychAPI struct {
 	fx.In
@@ -245,34 +248,43 @@ func (pm *Manager) CheckVoucherValid(ctx context.Context, ch address.Address, sv
 
 // CheckVoucherSpendable checks if the given voucher is currently spendable
 func (pm *Manager) CheckVoucherSpendable(ctx context.Context, ch address.Address, sv *paych.SignedVoucher, secret []byte, proof []byte) (bool, error) {
+	if len(proof) > 0 {
+		return false, errProofNotSupported
+	}
 	ca, err := pm.accessorByAddress(ch)
 	if err != nil {
 		return false, err
 	}
 
-	return ca.checkVoucherSpendable(ctx, ch, sv, secret, proof)
+	return ca.checkVoucherSpendable(ctx, ch, sv, secret)
 }
 
 // AddVoucherOutbound adds a voucher for an outbound channel.
 // Returns an error if the channel is not already in the store.
 func (pm *Manager) AddVoucherOutbound(ctx context.Context, ch address.Address, sv *paych.SignedVoucher, proof []byte, minDelta types.BigInt) (types.BigInt, error) {
+	if len(proof) > 0 {
+		return types.NewInt(0), errProofNotSupported
+	}
 	ca, err := pm.accessorByAddress(ch)
 	if err != nil {
 		return types.NewInt(0), err
 	}
-	return ca.addVoucher(ctx, ch, sv, proof, minDelta)
+	return ca.addVoucher(ctx, ch, sv, minDelta)
 }
 
 // AddVoucherInbound adds a voucher for an inbound channel.
 // If the channel is not in the store, fetches the channel from state (and checks that
 // the channel To address is owned by the wallet).
 func (pm *Manager) AddVoucherInbound(ctx context.Context, ch address.Address, sv *paych.SignedVoucher, proof []byte, minDelta types.BigInt) (types.BigInt, error) {
+	if len(proof) > 0 {
+		return types.NewInt(0), errProofNotSupported
+	}
 	// Get an accessor for the channel, creating it from state if necessary
 	ca, err := pm.inboundChannelAccessor(ctx, ch)
 	if err != nil {
 		return types.BigInt{}, err
 	}
-	return ca.addVoucher(ctx, ch, sv, proof, minDelta)
+	return ca.addVoucher(ctx, ch, sv, minDelta)
 }
 
 // inboundChannelAccessor gets an accessor for the given channel. The channel
@@ -336,11 +348,14 @@ func (pm *Manager) trackInboundChannel(ctx context.Context, ch address.Address)
 }
 
 func (pm *Manager) SubmitVoucher(ctx context.Context, ch address.Address, sv *paych.SignedVoucher, secret []byte, proof []byte) (cid.Cid, error) {
+	if len(proof) > 0 {
+		return cid.Undef, errProofNotSupported
+	}
 	ca, err := pm.accessorByAddress(ch)
 	if err != nil {
 		return cid.Undef, err
 	}
-	return ca.submitVoucher(ctx, ch, sv, secret, proof)
+	return ca.submitVoucher(ctx, ch, sv, secret)
 }
 
 func (pm *Manager) AllocateLane(ch address.Address) (uint64, error) {

paychmgr/paych.go

@@ -1,7 +1,6 @@
 package paychmgr
 
 import (
-	"bytes"
 	"context"
 	"fmt"
 
@@ -133,7 +132,7 @@ func (ca *channelAccessor) createVoucher(ctx context.Context, ch address.Address
 	sv.Signature = sig
 
 	// Store the voucher
-	if _, err := ca.addVoucherUnlocked(ctx, ch, sv, nil, types.NewInt(0)); err != nil {
+	if _, err := ca.addVoucherUnlocked(ctx, ch, sv, types.NewInt(0)); err != nil {
 		// If there are not enough funds in the channel to cover the voucher,
 		// return a voucher create result with the shortfall
 		var ife insufficientFundsErr
@@ -272,7 +271,7 @@ func (ca *channelAccessor) checkVoucherValidUnlocked(ctx context.Context, ch add
 	return laneStates, nil
 }
 
-func (ca *channelAccessor) checkVoucherSpendable(ctx context.Context, ch address.Address, sv *paych.SignedVoucher, secret []byte, proof []byte) (bool, error) {
+func (ca *channelAccessor) checkVoucherSpendable(ctx context.Context, ch address.Address, sv *paych.SignedVoucher, secret []byte) (bool, error) {
 	ca.lk.Lock()
 	defer ca.lk.Unlock()
 
@@ -295,26 +294,9 @@ func (ca *channelAccessor) checkVoucherSpendable(ctx context.Context, ch address
 		return false, nil
 	}
 
-	// If proof is needed and wasn't supplied as a parameter, get it from the
-	// datastore
-	if sv.Extra != nil && proof == nil {
-		vi, err := ci.infoForVoucher(sv)
-		if err != nil {
-			return false, err
-		}
-
-		if vi.Proof != nil {
-			log.Info("CheckVoucherSpendable: using stored proof")
-			proof = vi.Proof
-		} else {
-			log.Warn("CheckVoucherSpendable: nil proof for voucher with validation")
-		}
-	}
-
 	enc, err := actors.SerializeParams(&paych0.UpdateChannelStateParams{
 		Sv:     *sv,
 		Secret: secret,
-		Proof:  proof,
 	})
 	if err != nil {
 		return false, err
@@ -346,44 +328,31 @@ func (ca *channelAccessor) getPaychRecipient(ctx context.Context, ch address.Add
 	return state.To()
 }
 
-func (ca *channelAccessor) addVoucher(ctx context.Context, ch address.Address, sv *paych.SignedVoucher, proof []byte, minDelta types.BigInt) (types.BigInt, error) {
+func (ca *channelAccessor) addVoucher(ctx context.Context, ch address.Address, sv *paych.SignedVoucher, minDelta types.BigInt) (types.BigInt, error) {
 	ca.lk.Lock()
 	defer ca.lk.Unlock()
 
-	return ca.addVoucherUnlocked(ctx, ch, sv, proof, minDelta)
+	return ca.addVoucherUnlocked(ctx, ch, sv, minDelta)
 }
 
-func (ca *channelAccessor) addVoucherUnlocked(ctx context.Context, ch address.Address, sv *paych.SignedVoucher, proof []byte, minDelta types.BigInt) (types.BigInt, error) {
+func (ca *channelAccessor) addVoucherUnlocked(ctx context.Context, ch address.Address, sv *paych.SignedVoucher, minDelta types.BigInt) (types.BigInt, error) {
 	ci, err := ca.store.ByAddress(ch)
 	if err != nil {
 		return types.BigInt{}, err
 	}
 
 	// Check if the voucher has already been added
-	for i, v := range ci.Vouchers {
+	for _, v := range ci.Vouchers {
 		eq, err := cborutil.Equals(sv, v.Voucher)
 		if err != nil {
 			return types.BigInt{}, err
 		}
-		if !eq {
+		if eq {
-			continue
+			// Ignore the duplicate voucher.
+			log.Warnf("AddVoucher: voucher re-added")
+			return types.NewInt(0), nil
 		}
 
-		// This is a duplicate voucher.
-		// Update the proof on the existing voucher
-		if len(proof) > 0 && !bytes.Equal(v.Proof, proof) {
-			log.Warnf("AddVoucher: adding proof to stored voucher")
-			ci.Vouchers[i] = &VoucherInfo{
-				Voucher: v.Voucher,
-				Proof:   proof,
-			}
-
-			return types.NewInt(0), ca.store.putChannelInfo(ci)
-		}
-
-		// Otherwise just ignore the duplicate voucher
-		log.Warnf("AddVoucher: voucher re-added with matching proof")
-		return types.NewInt(0), nil
 	}
 
 	// Check voucher validity
@@ -410,7 +379,6 @@ func (ca *channelAccessor) addVoucherUnlocked(ctx context.Context, ch address.Ad
 
 	ci.Vouchers = append(ci.Vouchers, &VoucherInfo{
 		Voucher: sv,
-		Proof:   proof,
 	})
 
 	if ci.NextLane <= sv.Lane {
@@ -420,7 +388,7 @@ func (ca *channelAccessor) addVoucherUnlocked(ctx context.Context, ch address.Ad
 	return delta, ca.store.putChannelInfo(ci)
 }
 
-func (ca *channelAccessor) submitVoucher(ctx context.Context, ch address.Address, sv *paych.SignedVoucher, secret []byte, proof []byte) (cid.Cid, error) {
+func (ca *channelAccessor) submitVoucher(ctx context.Context, ch address.Address, sv *paych.SignedVoucher, secret []byte) (cid.Cid, error) {
 	ca.lk.Lock()
 	defer ca.lk.Unlock()
 
@@ -429,21 +397,6 @@ func (ca *channelAccessor) submitVoucher(ctx context.Context, ch address.Address
 		return cid.Undef, err
 	}
 
-	// If voucher needs proof, and none was supplied, check datastore for proof
-	if sv.Extra != nil && proof == nil {
-		vi, err := ci.infoForVoucher(sv)
-		if err != nil {
-			return cid.Undef, err
-		}
-
-		if vi.Proof != nil {
-			log.Info("SubmitVoucher: using stored proof")
-			proof = vi.Proof
-		} else {
-			log.Warn("SubmitVoucher: nil proof for voucher with validation")
-		}
-	}
-
 	has, err := ci.hasVoucher(sv)
 	if err != nil {
 		return cid.Undef, err
@@ -462,13 +415,9 @@ func (ca *channelAccessor) submitVoucher(ctx context.Context, ch address.Address
 	}
 
 	// TODO: ActorUpgrade
-	// The "proof" field is going away. We will need to abstract over the
-	// network version here.
-	// Alternatively, we'd need to support the "old" method on-chain.
 	enc, err := actors.SerializeParams(&paych0.UpdateChannelStateParams{
 		Sv:     *sv,
 		Secret: secret,
-		Proof:  proof,
 	})
 	if err != nil {
 		return cid.Undef, err
@@ -492,7 +441,6 @@ func (ca *channelAccessor) submitVoucher(ctx context.Context, ch address.Address
 		// Add the voucher to the channel
 		ci.Vouchers = append(ci.Vouchers, &VoucherInfo{
 			Voucher: sv,
-			Proof:   proof,
 		})
 	}
 

paychmgr/paych_test.go

@@ -555,53 +555,6 @@ func TestAllocateLaneWithExistingLaneState(t *testing.T) {
 	require.EqualValues(t, 3, lane)
 }
 
-func TestAddVoucherProof(t *testing.T) {
-	ctx := context.Background()
-
-	// Set up a manager with a single payment channel
-	s := testSetupMgrWithChannel(ctx, t)
-
-	nonce := uint64(1)
-	voucherAmount := big.NewInt(1)
-	minDelta := big.NewInt(0)
-	voucherAmount = big.NewInt(2)
-	voucherLane := uint64(1)
-
-	// Add a voucher with no proof
-	var proof []byte
-	sv := createTestVoucher(t, s.ch, voucherLane, nonce, voucherAmount, s.fromKeyPrivate)
-	_, err := s.mgr.AddVoucherOutbound(ctx, s.ch, sv, nil, minDelta)
-	require.NoError(t, err)
-
-	// Expect one voucher with no proof
-	ci, err := s.mgr.GetChannelInfo(s.ch)
-	require.NoError(t, err)
-	require.Len(t, ci.Vouchers, 1)
-	require.Len(t, ci.Vouchers[0].Proof, 0)
-
-	// Add same voucher with no proof
-	voucherLane = uint64(1)
-	_, err = s.mgr.AddVoucherOutbound(ctx, s.ch, sv, proof, minDelta)
-	require.NoError(t, err)
-
-	// Expect one voucher with no proof
-	ci, err = s.mgr.GetChannelInfo(s.ch)
-	require.NoError(t, err)
-	require.Len(t, ci.Vouchers, 1)
-	require.Len(t, ci.Vouchers[0].Proof, 0)
-
-	// Add same voucher with proof
-	proof = []byte{1}
-	_, err = s.mgr.AddVoucherOutbound(ctx, s.ch, sv, proof, minDelta)
-	require.NoError(t, err)
-
-	// Should add proof to existing voucher
-	ci, err = s.mgr.GetChannelInfo(s.ch)
-	require.NoError(t, err)
-	require.Len(t, ci.Vouchers, 1)
-	require.Len(t, ci.Vouchers[0].Proof, 1)
-}
-
 func TestAddVoucherInboundWalletKey(t *testing.T) {
 	ctx := context.Background()
 
@@ -748,10 +701,9 @@ func TestCheckSpendable(t *testing.T) {
 	voucherAmount := big.NewInt(1)
 	voucher := createTestVoucherWithExtra(t, s.ch, voucherLane, nonce, voucherAmount, s.fromKeyPrivate)
 
-	// Add voucher with proof
+	// Add voucher
 	minDelta := big.NewInt(0)
-	proof := []byte("proof")
+	_, err := s.mgr.AddVoucherInbound(ctx, s.ch, voucher, nil, minDelta)
-	_, err := s.mgr.AddVoucherInbound(ctx, s.ch, voucher, proof, minDelta)
 	require.NoError(t, err)
 
 	// Return success exit code from VM call, which indicates that voucher is
@@ -765,33 +717,17 @@ func TestCheckSpendable(t *testing.T) {
 
 	// Check that spendable is true
 	secret := []byte("secret")
-	otherProof := []byte("other proof")
+	spendable, err := s.mgr.CheckVoucherSpendable(ctx, s.ch, voucher, secret, nil)
-	spendable, err := s.mgr.CheckVoucherSpendable(ctx, s.ch, voucher, secret, otherProof)
 	require.NoError(t, err)
 	require.True(t, spendable)
 
-	// Check that the secret and proof were passed through correctly
+	// Check that the secret was passed through correctly
 	lastCall := s.mock.getLastCall()
 	var p paych0.UpdateChannelStateParams
 	err = p.UnmarshalCBOR(bytes.NewReader(lastCall.Params))
 	require.NoError(t, err)
-	require.Equal(t, otherProof, p.Proof)
 	require.Equal(t, secret, p.Secret)
 
-	// Check that if no proof is supplied, the proof supplied to add voucher
-	// above is used
-	secret2 := []byte("secret2")
-	spendable, err = s.mgr.CheckVoucherSpendable(ctx, s.ch, voucher, secret2, nil)
-	require.NoError(t, err)
-	require.True(t, spendable)
-
-	lastCall = s.mock.getLastCall()
-	var p2 paych0.UpdateChannelStateParams
-	err = p2.UnmarshalCBOR(bytes.NewReader(lastCall.Params))
-	require.NoError(t, err)
-	require.Equal(t, proof, p2.Proof)
-	require.Equal(t, secret2, p2.Secret)
-
 	// Check that if VM call returns non-success exit code, spendable is false
 	s.mock.setCallResponse(&api.InvocResult{
 		MsgRct: &types.MessageReceipt{
@@ -829,73 +765,48 @@ func TestSubmitVoucher(t *testing.T) {
 	voucherAmount := big.NewInt(1)
 	voucher := createTestVoucherWithExtra(t, s.ch, voucherLane, nonce, voucherAmount, s.fromKeyPrivate)
 
-	// Add voucher with proof
+	// Add voucher
 	minDelta := big.NewInt(0)
-	addVoucherProof := []byte("proof")
+	_, err := s.mgr.AddVoucherInbound(ctx, s.ch, voucher, nil, minDelta)
-	_, err := s.mgr.AddVoucherInbound(ctx, s.ch, voucher, addVoucherProof, minDelta)
 	require.NoError(t, err)
 
 	// Submit voucher
 	secret := []byte("secret")
-	submitProof := []byte("submit proof")
+	submitCid, err := s.mgr.SubmitVoucher(ctx, s.ch, voucher, secret, nil)
-	submitCid, err := s.mgr.SubmitVoucher(ctx, s.ch, voucher, secret, submitProof)
 	require.NoError(t, err)
 
-	// Check that the secret and proof were passed through correctly
+	// Check that the secret was passed through correctly
 	msg := s.mock.pushedMessages(submitCid)
 	var p paych0.UpdateChannelStateParams
 	err = p.UnmarshalCBOR(bytes.NewReader(msg.Message.Params))
 	require.NoError(t, err)
-	require.Equal(t, submitProof, p.Proof)
 	require.Equal(t, secret, p.Secret)
 
-	// Check that if no proof is supplied to submit voucher, the proof supplied
-	// to add voucher is used
-	nonce++
-	voucherAmount = big.NewInt(2)
-	addVoucherProof2 := []byte("proof2")
-	secret2 := []byte("secret2")
-	voucher = createTestVoucherWithExtra(t, s.ch, voucherLane, nonce, voucherAmount, s.fromKeyPrivate)
-	_, err = s.mgr.AddVoucherInbound(ctx, s.ch, voucher, addVoucherProof2, minDelta)
-	require.NoError(t, err)
-
-	submitCid, err = s.mgr.SubmitVoucher(ctx, s.ch, voucher, secret2, nil)
-	require.NoError(t, err)
-
-	msg = s.mock.pushedMessages(submitCid)
-	var p2 paych0.UpdateChannelStateParams
-	err = p2.UnmarshalCBOR(bytes.NewReader(msg.Message.Params))
-	require.NoError(t, err)
-	require.Equal(t, addVoucherProof2, p2.Proof)
-	require.Equal(t, secret2, p2.Secret)
-
 	// Submit a voucher without first adding it
 	nonce++
 	voucherAmount = big.NewInt(3)
 	secret3 := []byte("secret2")
-	proof3 := []byte("proof3")
 	voucher = createTestVoucherWithExtra(t, s.ch, voucherLane, nonce, voucherAmount, s.fromKeyPrivate)
-	submitCid, err = s.mgr.SubmitVoucher(ctx, s.ch, voucher, secret3, proof3)
+	submitCid, err = s.mgr.SubmitVoucher(ctx, s.ch, voucher, secret3, nil)
 	require.NoError(t, err)
 
 	msg = s.mock.pushedMessages(submitCid)
 	var p3 paych0.UpdateChannelStateParams
 	err = p3.UnmarshalCBOR(bytes.NewReader(msg.Message.Params))
 	require.NoError(t, err)
-	require.Equal(t, proof3, p3.Proof)
 	require.Equal(t, secret3, p3.Secret)
 
 	// Verify that vouchers are marked as submitted
 	vis, err := s.mgr.ListVouchers(ctx, s.ch)
 	require.NoError(t, err)
-	require.Len(t, vis, 3)
+	require.Len(t, vis, 2)
 
 	for _, vi := range vis {
 		require.True(t, vi.Submitted)
 	}
 
 	// Attempting to submit the same voucher again should fail
-	_, err = s.mgr.SubmitVoucher(ctx, s.ch, voucher, secret2, nil)
+	_, err = s.mgr.SubmitVoucher(ctx, s.ch, voucher, secret3, nil)
 	require.Error(t, err)
 }
 

paychmgr/store.go

@@ -49,7 +49,7 @@ const (
 
 type VoucherInfo struct {
 	Voucher   *paych.SignedVoucher
-	Proof     []byte
+	Proof     []byte // ignored
 	Submitted bool
 }