Skip to content

Commit

Permalink
fix(datatransfer): Fix validators
Browse files Browse the repository at this point in the history
Validators were checking payload cid against commP -- which are not the same any more. Added a
payloadCid to client deal to maintain the record, fixed validator logic
  • Loading branch information
hannahhoward authored and shannonwells committed Dec 12, 2019
1 parent 4b6f5cc commit 7f4a718
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 42 deletions.
22 changes: 20 additions & 2 deletions chain/deals/cbor_gen.go
Original file line number Diff line number Diff line change
Expand Up @@ -518,7 +518,7 @@ func (t *ClientDeal) MarshalCBOR(w io.Writer) error {
_, err := w.Write(cbg.CborNull)
return err
}
if _, err := w.Write([]byte{135}); err != nil {
if _, err := w.Write([]byte{136}); err != nil {
return err
}

Expand Down Expand Up @@ -556,6 +556,12 @@ func (t *ClientDeal) MarshalCBOR(w io.Writer) error {
return err
}

// t.t.PayloadCid (cid.Cid) (struct)

if err := cbg.WriteCid(w, t.PayloadCid); err != nil {
return xerrors.Errorf("failed to write cid field t.PayloadCid: %w", err)
}

// t.PublishMessage (types.SignedMessage) (struct)
if err := t.PublishMessage.MarshalCBOR(w); err != nil {
return err
Expand All @@ -574,7 +580,7 @@ func (t *ClientDeal) UnmarshalCBOR(r io.Reader) error {
return fmt.Errorf("cbor input should be of type array")
}

if extra != 7 {
if extra != 8 {
return fmt.Errorf("cbor input had wrong number of fields")
}

Expand Down Expand Up @@ -638,6 +644,18 @@ func (t *ClientDeal) UnmarshalCBOR(r io.Reader) error {
return fmt.Errorf("wrong type for uint64 field")
}
t.DealID = uint64(extra)
// t.t.PayloadCid (cid.Cid) (struct)

{

c, err := cbg.ReadCid(br)
if err != nil {
return xerrors.Errorf("failed to read cid field t.PayloadCid: %w", err)
}

t.PayloadCid = c

}
// t.PublishMessage (types.SignedMessage) (struct)

{
Expand Down
5 changes: 3 additions & 2 deletions chain/deals/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ type ClientDeal struct {
Miner peer.ID
MinerWorker address.Address
DealID uint64
PayloadCid cid.Cid

PublishMessage *types.SignedMessage

Expand Down Expand Up @@ -245,8 +246,8 @@ func (c *Client) Start(ctx context.Context, p ClientDealProposal) (cid.Cid, erro
State: api.DealUnknown,
Miner: p.MinerID,
MinerWorker: p.MinerWorker,

s: s,
PayloadCid: p.Data,
s: s,
}

c.incoming <- deal
Expand Down
3 changes: 1 addition & 2 deletions chain/deals/client_utils.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package deals

import (
"bytes"
"context"
"runtime"

Expand Down Expand Up @@ -158,7 +157,7 @@ func (c *ClientRequestValidator) ValidatePull(
if deal.Miner != receiver {
return xerrors.Errorf("Deal Peer %s, Data Transfer Peer %s: %w", deal.Miner.String(), receiver.String(), ErrWrongPeer)
}
if !bytes.Equal(deal.Proposal.PieceRef, baseCid.Bytes()) {
if !deal.PayloadCid.Equals(baseCid) {
return xerrors.Errorf("Deal Payload CID %s, Data Transfer CID %s: %w", string(deal.Proposal.PieceRef), baseCid.String(), ErrWrongPiece)
}
for _, state := range DataTransferStates {
Expand Down
3 changes: 1 addition & 2 deletions chain/deals/provider_utils.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package deals

import (
"bytes"
"context"
"runtime"

Expand Down Expand Up @@ -175,7 +174,7 @@ func (m *ProviderRequestValidator) ValidatePush(
return xerrors.Errorf("Deal Peer %s, Data Transfer Peer %s: %w", deal.Client.String(), sender.String(), ErrWrongPeer)
}

if !bytes.Equal(deal.Proposal.PieceRef, baseCid.Bytes()) {
if !deal.Ref.Equals(baseCid) {
return xerrors.Errorf("Deal Payload CID %s, Data Transfer CID %s: %w", string(deal.Proposal.PieceRef), baseCid.String(), ErrWrongPiece)
}
for _, state := range DataTransferStates {
Expand Down
48 changes: 14 additions & 34 deletions chain/deals/request_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ func newClientDeal(minerID peer.ID, state api.DealState) (deals.ClientDeal, erro
return deals.ClientDeal{
Proposal: newProposal,
ProposalCid: proposalNd.Cid(),
PayloadCid: blockGenerator.Next().Cid(),
Miner: minerID,
MinerWorker: minerAddr,
State: state,
Expand All @@ -91,10 +92,7 @@ func newMinerDeal(clientID peer.ID, state api.DealState) (deals.MinerDeal, error
if err != nil {
return deals.MinerDeal{}, err
}
ref, err := cid.Cast(newProposal.PieceRef)
if err != nil {
return deals.MinerDeal{}, err
}
ref := blockGenerator.Next().Cid()

return deals.MinerDeal{
Proposal: newProposal,
Expand Down Expand Up @@ -143,11 +141,8 @@ func TestClientRequestValidation(t *testing.T) {
if err := state.Begin(clientDeal.ProposalCid, &clientDeal); err != nil {
t.Fatal("deal tracking failed")
}
pieceRef, err := cid.Cast(clientDeal.Proposal.PieceRef)
if err != nil {
t.Fatal("unable to construct piece cid")
}
if !xerrors.Is(crv.ValidatePull(minerID, &deals.StorageDataTransferVoucher{clientDeal.ProposalCid, 1}, pieceRef, nil), deals.ErrWrongPeer) {
payloadCid := clientDeal.PayloadCid
if !xerrors.Is(crv.ValidatePull(minerID, &deals.StorageDataTransferVoucher{clientDeal.ProposalCid, 1}, payloadCid, nil), deals.ErrWrongPeer) {
t.Fatal("Pull should fail if miner address is incorrect")
}
})
Expand All @@ -171,11 +166,8 @@ func TestClientRequestValidation(t *testing.T) {
if err := state.Begin(clientDeal.ProposalCid, &clientDeal); err != nil {
t.Fatal("deal tracking failed")
}
pieceRef, err := cid.Cast(clientDeal.Proposal.PieceRef)
if err != nil {
t.Fatal("unable to construct piece cid")
}
if !xerrors.Is(crv.ValidatePull(minerID, &deals.StorageDataTransferVoucher{clientDeal.ProposalCid, 1}, pieceRef, nil), deals.ErrInacceptableDealState) {
payloadCid := clientDeal.PayloadCid
if !xerrors.Is(crv.ValidatePull(minerID, &deals.StorageDataTransferVoucher{clientDeal.ProposalCid, 1}, payloadCid, nil), deals.ErrInacceptableDealState) {
t.Fatal("Pull should fail if deal is in a state that cannot be data transferred")
}
})
Expand All @@ -187,11 +179,8 @@ func TestClientRequestValidation(t *testing.T) {
if err := state.Begin(clientDeal.ProposalCid, &clientDeal); err != nil {
t.Fatal("deal tracking failed")
}
pieceRef, err := cid.Cast(clientDeal.Proposal.PieceRef)
if err != nil {
t.Fatal("unable to construct piece cid")
}
if crv.ValidatePull(minerID, &deals.StorageDataTransferVoucher{clientDeal.ProposalCid, 1}, pieceRef, nil) != nil {
payloadCid := clientDeal.PayloadCid
if crv.ValidatePull(minerID, &deals.StorageDataTransferVoucher{clientDeal.ProposalCid, 1}, payloadCid, nil) != nil {
t.Fatal("Pull should should succeed when all parameters are correct")
}
})
Expand Down Expand Up @@ -236,11 +225,8 @@ func TestProviderRequestValidation(t *testing.T) {
if err := state.Begin(minerDeal.ProposalCid, &minerDeal); err != nil {
t.Fatal("deal tracking failed")
}
pieceRef, err := cid.Cast(minerDeal.Proposal.PieceRef)
if err != nil {
t.Fatal("unable to construct piece cid")
}
if !xerrors.Is(mrv.ValidatePush(clientID, &deals.StorageDataTransferVoucher{minerDeal.ProposalCid, 1}, pieceRef, nil), deals.ErrWrongPeer) {
ref := minerDeal.Ref
if !xerrors.Is(mrv.ValidatePush(clientID, &deals.StorageDataTransferVoucher{minerDeal.ProposalCid, 1}, ref, nil), deals.ErrWrongPeer) {
t.Fatal("Push should fail if miner address is incorrect")
}
})
Expand All @@ -264,11 +250,8 @@ func TestProviderRequestValidation(t *testing.T) {
if err := state.Begin(minerDeal.ProposalCid, &minerDeal); err != nil {
t.Fatal("deal tracking failed")
}
pieceRef, err := cid.Cast(minerDeal.Proposal.PieceRef)
if err != nil {
t.Fatal("unable to construct piece cid")
}
if !xerrors.Is(mrv.ValidatePush(clientID, &deals.StorageDataTransferVoucher{minerDeal.ProposalCid, 1}, pieceRef, nil), deals.ErrInacceptableDealState) {
ref := minerDeal.Ref
if !xerrors.Is(mrv.ValidatePush(clientID, &deals.StorageDataTransferVoucher{minerDeal.ProposalCid, 1}, ref, nil), deals.ErrInacceptableDealState) {
t.Fatal("Push should fail if deal is in a state that cannot be data transferred")
}
})
Expand All @@ -280,11 +263,8 @@ func TestProviderRequestValidation(t *testing.T) {
if err := state.Begin(minerDeal.ProposalCid, &minerDeal); err != nil {
t.Fatal("deal tracking failed")
}
pieceRef, err := cid.Cast(minerDeal.Proposal.PieceRef)
if err != nil {
t.Fatal("unable to construct piece cid")
}
if mrv.ValidatePush(clientID, &deals.StorageDataTransferVoucher{minerDeal.ProposalCid, 1}, pieceRef, nil) != nil {
ref := minerDeal.Ref
if mrv.ValidatePush(clientID, &deals.StorageDataTransferVoucher{minerDeal.ProposalCid, 1}, ref, nil) != nil {
t.Fatal("Push should should succeed when all parameters are correct")
}
})
Expand Down

0 comments on commit 7f4a718

Please sign in to comment.