deals: Address review

build/params.go

@@ -9,6 +9,8 @@ const SectorSize = 1024
 
 const PaymentChannelClosingDelay = 6 * 60 * 2 // six hours
 
+const DealVoucherSkewLimit = 10
+
 const ForkLengthThreshold = 20
 
 // TODO: Move other important consts here

chain/deals/handler_states.go

@@ -3,6 +3,7 @@ package deals
 import (
 	"bytes"
 	"context"
+	"github.com/filecoin-project/go-lotus/build"
 
 	cbor "github.com/ipfs/go-ipld-cbor"
 	"github.com/ipfs/go-merkledag"
@@ -34,6 +35,68 @@ func (h *Handler) handle(ctx context.Context, deal MinerDeal, cb handlerFunc, ne
 
 // ACCEPTED
 
+func (h *Handler) validateVouchers(ctx context.Context, deal MinerDeal) error {
+	curHead, err := h.full.ChainHead(ctx)
+	if err != nil {
+		return err
+	}
+
+	for i, voucher := range deal.Proposal.Payment.Vouchers {
+		err := h.full.PaychVoucherCheckValid(ctx, deal.Proposal.Payment.PayChActor, voucher)
+		if err != nil {
+			return xerrors.Errorf("validating payment voucher %d: %w", i, err)
+		}
+
+		if voucher.Extra == nil {
+			return xerrors.Errorf("validating payment voucher %d: voucher.Extra not set")
+		}
+
+		if voucher.Extra.Actor != deal.Proposal.MinerAddress {
+			return xerrors.Errorf("validating payment voucher %d: extra params actor didn't match miner address in proposal: '%s' != '%s'", i, voucher.Extra.Actor, deal.Proposal.MinerAddress)
+		}
+		if voucher.Extra.Method != actors.MAMethods.PaymentVerifyInclusion {
+			return xerrors.Errorf("validating payment voucher %d: expected extra method %d, got %d", i, actors.MAMethods.PaymentVerifyInclusion, voucher.Extra.Method)
+		}
+
+		var inclChallenge actors.PieceInclVoucherData
+		if err := cbor.DecodeInto(voucher.Extra.Data, &inclChallenge); err != nil {
+			return xerrors.Errorf("validating payment voucher %d: failed to decode storage voucher data for verification: %w", i, err)
+		}
+		if inclChallenge.PieceSize.Uint64() != deal.Proposal.Size {
+			return xerrors.Errorf("validating payment voucher %d: paych challenge piece size didn't match deal proposal size: %d != %d", i, inclChallenge.PieceSize.Uint64(), deal.Proposal.Size)
+		}
+		if !bytes.Equal(inclChallenge.CommP, deal.Proposal.CommP) {
+			return xerrors.Errorf("validating payment voucher %d: paych challenge commP didn't match deal proposal", i)
+		}
+
+		maxClose := curHead.Height() + deal.Proposal.Duration + build.DealVoucherSkewLimit
+		if voucher.MinCloseHeight > maxClose {
+			return xerrors.Errorf("validating payment voucher %d: MinCloseHeight too high (%d), max expected: %d", i, voucher.MinCloseHeight, maxClose)
+		}
+
+		if voucher.TimeLock > maxClose {
+			return xerrors.Errorf("validating payment voucher %d: TimeLock too high (%d), max expected: %d", i, voucher.TimeLock, maxClose)
+		}
+
+		if len(voucher.Merges) > 0 {
+			return xerrors.Errorf("validating payment voucher %d: didn't expect any merges", i)
+		}
+
+		// TODO: make sure that current laneStatus.Amount == 0
+
+		if types.BigCmp(voucher.Amount, deal.Proposal.TotalPrice) < 0 {
+			return xerrors.Errorf("validating payment voucher %d: not enough funds in the voucher", i)
+		}
+
+		minPrice := types.BigMul(types.BigMul(h.pricePerByteBlock, types.NewInt(deal.Proposal.Size)), types.NewInt(deal.Proposal.Duration))
+		if types.BigCmp(minPrice, deal.Proposal.TotalPrice) > 0 {
+			return xerrors.Errorf("validating payment voucher %d: minimum price: %s", i, minPrice)
+		}
+	}
+
+	return nil
+}
+
 func (h *Handler) accept(ctx context.Context, deal MinerDeal) (func(*MinerDeal), error) {
 	switch deal.Proposal.SerializationMode {
 	//case SerializationRaw:
@@ -43,61 +106,10 @@ func (h *Handler) accept(ctx context.Context, deal MinerDeal) (func(*MinerDeal),
 		return nil, xerrors.Errorf("deal proposal with unsupported serialization: %s", deal.Proposal.SerializationMode)
 	}
 
-	curHead, err := h.full.ChainHead(ctx)
+	if err := h.validateVouchers(ctx, deal); err != nil {
-	if err != nil {
 		return nil, err
 	}
 
-	for i, voucher := range deal.Proposal.Payment.Vouchers {
-		err := h.full.PaychVoucherCheckValid(ctx, deal.Proposal.Payment.PayChActor, voucher)
-		if err != nil {
-			return nil, xerrors.Errorf("validating payment voucher %d: %w", i, err)
-		}
-
-		if voucher.Extra != nil { // TODO: should we reject vouchers with no extra params?
-			if voucher.Extra.Actor != deal.Proposal.MinerAddress {
-				return nil, xerrors.Errorf("validating payment voucher %d: extra params actor didn't match miner address in proposal: '%s' != '%s'", i, voucher.Extra.Actor, deal.Proposal.MinerAddress)
-			}
-			if voucher.Extra.Method != actors.MAMethods.PaymentVerifyInclusion {
-				return nil, xerrors.Errorf("validating payment voucher %d: expected extra method %d, got %d", i, actors.MAMethods.PaymentVerifyInclusion, voucher.Extra.Method)
-			}
-
-			var inclChallenge actors.PieceInclVoucherData
-			if err := cbor.DecodeInto(voucher.Extra.Data, &inclChallenge); err != nil {
-				return nil, xerrors.Errorf("validating payment voucher %d: failed to decode storage voucher data for verification: %w", i, err)
-			}
-			if inclChallenge.PieceSize.Uint64() != deal.Proposal.Size {
-				return nil, xerrors.Errorf("validating payment voucher %d: paych challenge piece size didn't match deal proposal size: %d != %d", i, inclChallenge.PieceSize.Uint64(), deal.Proposal.Size)
-			}
-			if !bytes.Equal(inclChallenge.CommP, deal.Proposal.CommP) {
-				return nil, xerrors.Errorf("validating payment voucher %d: paych challenge commP didn't match deal proposal", i)
-			}
-		}
-
-		if voucher.MinCloseHeight > curHead.Height()+deal.Proposal.Duration {
-			return nil, xerrors.Errorf("validating payment voucher %d: MinCloseHeight too high (%d), max expected: %d", i, voucher.MinCloseHeight, curHead.Height()+deal.Proposal.Duration)
-		}
-
-		if voucher.TimeLock > curHead.Height()+deal.Proposal.Duration {
-			return nil, xerrors.Errorf("validating payment voucher %d: TimeLock too high (%d), max expected: %d", i, voucher.TimeLock, curHead.Height()+deal.Proposal.Duration)
-		}
-
-		if len(voucher.Merges) > 0 {
-			return nil, xerrors.Errorf("validating payment voucher %d: didn't expect any merges", i)
-		}
-
-		// TODO: make sure that current laneStatus.Amount == 0
-
-		if types.BigCmp(voucher.Amount, deal.Proposal.TotalPrice) < 0 {
-			return nil, xerrors.Errorf("validating payment voucher %d: not enough funds in the voucher", i)
-		}
-
-		minPrice := types.BigMul(types.BigMul(h.pricePerByteBlock, types.NewInt(deal.Proposal.Size)), types.NewInt(deal.Proposal.Duration))
-		if types.BigCmp(minPrice, deal.Proposal.TotalPrice) > 0 {
-			return nil, xerrors.Errorf("validating payment voucher %d: minimum price: %s", i, minPrice)
-		}
-	}
-
 	for i, voucher := range deal.Proposal.Payment.Vouchers {
 		if err := h.full.PaychVoucherAdd(ctx, deal.Proposal.Payment.PayChActor, voucher, nil); err != nil {
 			return nil, xerrors.Errorf("consuming payment voucher %d: %w", i, err)
@@ -105,7 +117,7 @@ func (h *Handler) accept(ctx context.Context, deal MinerDeal) (func(*MinerDeal),
 	}
 
 	log.Info("fetching data for a deal")
-	err = h.sendSignedResponse(StorageDealResponse{
+	err := h.sendSignedResponse(StorageDealResponse{
 		State:    Accepted,
 		Message:  "",
 		Proposal: deal.ProposalCid,

miner/miner.go

@@ -336,7 +336,7 @@ func (m *Miner) createBlock(base *MiningBase, ticket *types.Ticket, proof types.
 
 	msgs := m.selectMessages(pending)
 
-	uts := time.Now().Unix() // TODO: review: is this what we want here?
+	uts := time.Now().Unix() // TODO: put smallest valid timestamp
 
 	// why even return this? that api call could just submit it for us
 	return m.api.MinerCreateBlock(context.TODO(), m.addresses[0], base.ts, append(base.tickets, ticket), proof, msgs, uint64(uts))

paych/store.go

@@ -131,26 +131,17 @@ func (ps *Store) AddVoucher(ch address.Address, sv *types.SignedVoucher, proof [
 		return err
 	}
 
-	svs, err := sv.EncodedString()
-	if err != nil {
-		return err
-	}
-
 	// look for duplicates
 	for i, v := range ci.Vouchers {
-		osvs, err := v.Voucher.EncodedString()
+		if !sv.Equals(v.Voucher) {
-		if err != nil {
-			return err
-		}
-		if osvs != svs {
 			continue
 		}
 		if v.Proof != nil {
 			if !bytes.Equal(v.Proof, proof) {
-				log.Warnf("AddVoucher: multiple proofs for single voucher: v:'%s', storing both", svs)
+				log.Warnf("AddVoucher: multiple proofs for single voucher, storing both")
 				break
 			}
-			log.Warnf("AddVoucher: voucher re-added with matching proof: v:'%s'", svs)
+			log.Warnf("AddVoucher: voucher re-added with matching proof")
 			return nil
 		}
 

paych/watcher.go

@@ -1,26 +0,0 @@
-package paych
-
-import (
-	"github.com/filecoin-project/go-lotus/chain/address"
-	"github.com/filecoin-project/go-lotus/chain/events"
-	"github.com/filecoin-project/go-lotus/chain/types"
-)
-
-type watchedLane struct {
-	bestVoucher *types.SignedVoucher
-	closed      bool
-}
-
-type inboundWatcher struct {
-	ev *events.Events
-
-	paych *address.Address
-
-	control address.Address
-
-	lanes map[uint64]*watchedLane
-
-	closeAt   uint64 // at what H do we plan to call close
-	collectAt uint64 // at what H do we plan to call collect
-
-}