Merge pull request #5609 from filecoin-project/fix/vouchers

Verify Voucher locks in VoucherValidUnlocked

paychmgr/manager.go

@@ -319,6 +319,7 @@ func (pm *Manager) trackInboundChannel(ctx context.Context, ch address.Address)
 	return pm.store.TrackChannel(stateCi)
 }
 
+// TODO: secret vs proof doesn't make sense, there is only one, not two
 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

paychmgr/paych.go

@@ -184,6 +184,20 @@ func (ca *channelAccessor) checkVoucherValidUnlocked(ctx context.Context, ch add
 		return nil, xerrors.Errorf("voucher ChannelAddr doesn't match channel address, got %s, expected %s", sv.ChannelAddr, ch)
 	}
 
+	// check voucher is unlocked
+	if sv.Extra != nil {
+		return nil, xerrors.Errorf("voucher is Message Locked")
+	}
+	if sv.TimeLockMax != 0 {
+		return nil, xerrors.Errorf("voucher is Max Time Locked")
+	}
+	if sv.TimeLockMin != 0 {
+		return nil, xerrors.Errorf("voucher is Min Time Locked")
+	}
+	if len(sv.SecretPreimage) != 0 {
+		return nil, xerrors.Errorf("voucher is Hash Locked")
+	}
+
 	// Load payment channel actor state
 	act, pchState, err := ca.sa.loadPaychActorState(ctx, ch)
 	if err != nil {

paychmgr/paych_test.go

@@ -596,7 +596,7 @@ func TestCheckSpendable(t *testing.T) {
 	voucherLane := uint64(1)
 	nonce := uint64(1)
 	voucherAmount := big.NewInt(1)
-	voucher := createTestVoucherWithExtra(t, s.ch, voucherLane, nonce, voucherAmount, s.fromKeyPrivate)
+	voucher := createTestVoucher(t, s.ch, voucherLane, nonce, voucherAmount, s.fromKeyPrivate)
 
 	// Add voucher
 	minDelta := big.NewInt(0)
@@ -660,38 +660,34 @@ func TestSubmitVoucher(t *testing.T) {
 	voucherLane := uint64(1)
 	nonce := uint64(1)
 	voucherAmount := big.NewInt(1)
-	voucher := createTestVoucherWithExtra(t, s.ch, voucherLane, nonce, voucherAmount, s.fromKeyPrivate)
+	voucher := createTestVoucher(t, s.ch, voucherLane, nonce, voucherAmount, s.fromKeyPrivate)
 
 	// Add voucher
 	minDelta := big.NewInt(0)
 	_, err := s.mgr.AddVoucherInbound(ctx, s.ch, voucher, nil, minDelta)
 	require.NoError(t, err)
 
 	// Submit voucher
-	secret := []byte("secret")
-	submitCid, err := s.mgr.SubmitVoucher(ctx, s.ch, voucher, secret, nil)
+	submitCid, err := s.mgr.SubmitVoucher(ctx, s.ch, voucher, nil, nil)
 	require.NoError(t, err)
 
 	// Check that the secret was passed through correctly
 	msg := s.mock.pushedMessages(submitCid)
 	var p paych2.UpdateChannelStateParams
 	err = p.UnmarshalCBOR(bytes.NewReader(msg.Message.Params))
 	require.NoError(t, err)
-	require.Equal(t, secret, p.Secret)
 
 	// Submit a voucher without first adding it
 	nonce++
 	voucherAmount = big.NewInt(3)
-	secret3 := []byte("secret2")
-	voucher = createTestVoucherWithExtra(t, s.ch, voucherLane, nonce, voucherAmount, s.fromKeyPrivate)
-	submitCid, err = s.mgr.SubmitVoucher(ctx, s.ch, voucher, secret3, nil)
+	voucher = createTestVoucher(t, s.ch, voucherLane, nonce, voucherAmount, s.fromKeyPrivate)
+	submitCid, err = s.mgr.SubmitVoucher(ctx, s.ch, voucher, nil, nil)
 	require.NoError(t, err)
 
 	msg = s.mock.pushedMessages(submitCid)
 	var p3 paych2.UpdateChannelStateParams
 	err = p3.UnmarshalCBOR(bytes.NewReader(msg.Message.Params))
 	require.NoError(t, err)
-	require.Equal(t, secret3, p3.Secret)
 
 	// Verify that vouchers are marked as submitted
 	vis, err := s.mgr.ListVouchers(ctx, s.ch)
@@ -703,7 +699,7 @@ func TestSubmitVoucher(t *testing.T) {
 	}
 
 	// Attempting to submit the same voucher again should fail
-	_, err = s.mgr.SubmitVoucher(ctx, s.ch, voucher, secret3, nil)
+	_, err = s.mgr.SubmitVoucher(ctx, s.ch, voucher, nil, nil)
 	require.Error(t, err)
 }
 
@@ -790,7 +786,7 @@ func createTestVoucher(t *testing.T, ch address.Address, voucherLane uint64, non
 	return sv
 }
 
-func createTestVoucherWithExtra(t *testing.T, ch address.Address, voucherLane uint64, nonce uint64, voucherAmount big.Int, key []byte) *paych2.SignedVoucher {
+func createTestVoucherWithExtra(t *testing.T, ch address.Address, voucherLane uint64, nonce uint64, voucherAmount big.Int, key []byte) *paych2.SignedVoucher { //nolint:deadcode
 	sv := &paych2.SignedVoucher{
 		ChannelAddr: ch,
 		Lane:        voucherLane,