From 7519bdde42f55ba2960246f87ff29c2724297f2b Mon Sep 17 00:00:00 2001 From: hannahhoward Date: Mon, 9 Nov 2020 16:30:47 -0800 Subject: [PATCH] feat(markets): check deal equality in OnDealSectorCommitted, verify that deals looked up match the deal proposals which were made --- go.mod | 2 +- go.sum | 4 +- markets/storageadapter/client.go | 5 +- markets/storageadapter/getcurrentdealinfo.go | 11 +++- .../storageadapter/getcurrentdealinfo_test.go | 56 ++++++++++++++++++- .../storageadapter/ondealsectorcommitted.go | 25 +++++---- .../ondealsectorcommitted_test.go | 10 +++- markets/storageadapter/provider.go | 4 +- 8 files changed, 94 insertions(+), 23 deletions(-) diff --git a/go.mod b/go.mod index d781a635e..88afa1a73 100644 --- a/go.mod +++ b/go.mod @@ -30,7 +30,7 @@ require ( github.com/filecoin-project/go-crypto v0.0.0-20191218222705-effae4ea9f03 github.com/filecoin-project/go-data-transfer v1.0.0 github.com/filecoin-project/go-fil-commcid v0.0.0-20200716160307-8f644712406f - github.com/filecoin-project/go-fil-markets v0.9.2-0.20201103030854-3d9aa08b3c2f + github.com/filecoin-project/go-fil-markets v0.9.2-0.20201109213926-58f0437275b7 github.com/filecoin-project/go-jsonrpc v0.1.2-0.20201008195726-68c6a2704e49 github.com/filecoin-project/go-multistore v0.0.3 github.com/filecoin-project/go-padreader v0.0.0-20200903213702-ed5fae088b20 diff --git a/go.sum b/go.sum index c949a71ea..ff8bd9656 100644 --- a/go.sum +++ b/go.sum @@ -255,8 +255,8 @@ github.com/filecoin-project/go-ds-versioning v0.1.0 h1:y/X6UksYTsK8TLCI7rttCKEvl github.com/filecoin-project/go-ds-versioning v0.1.0/go.mod h1:mp16rb4i2QPmxBnmanUx8i/XANp+PFCCJWiAb+VW4/s= github.com/filecoin-project/go-fil-commcid v0.0.0-20200716160307-8f644712406f h1:GxJzR3oRIMTPtpZ0b7QF8FKPK6/iPAc7trhlL5k/g+s= github.com/filecoin-project/go-fil-commcid v0.0.0-20200716160307-8f644712406f/go.mod h1:Eaox7Hvus1JgPrL5+M3+h7aSPHc0cVqpSxA+TxIEpZQ= -github.com/filecoin-project/go-fil-markets v0.9.2-0.20201103030854-3d9aa08b3c2f h1:SAyL9YllpB/2gyclDE0AXQenEQc4O2g2IP+DDF7L0Vg= -github.com/filecoin-project/go-fil-markets v0.9.2-0.20201103030854-3d9aa08b3c2f/go.mod h1:h+bJ/IUnYjnW5HMKyt9JQSnhslqetkpuzwwugc3K8vM= +github.com/filecoin-project/go-fil-markets v0.9.2-0.20201109213926-58f0437275b7 h1:oxqnqKP9LCI+D4EoszxC7xUYCQovgLmyDa53RgFm6ts= +github.com/filecoin-project/go-fil-markets v0.9.2-0.20201109213926-58f0437275b7/go.mod h1:h+bJ/IUnYjnW5HMKyt9JQSnhslqetkpuzwwugc3K8vM= github.com/filecoin-project/go-hamt-ipld v0.1.5 h1:uoXrKbCQZ49OHpsTCkrThPNelC4W3LPEk0OrS/ytIBM= github.com/filecoin-project/go-hamt-ipld v0.1.5 h1:uoXrKbCQZ49OHpsTCkrThPNelC4W3LPEk0OrS/ytIBM= github.com/filecoin-project/go-hamt-ipld v0.1.5/go.mod h1:6Is+ONR5Cd5R6XZoCse1CWaXZc0Hdb/JeX+EQCQzX24= diff --git a/markets/storageadapter/client.go b/markets/storageadapter/client.go index 9d450c21b..16d28f843 100644 --- a/markets/storageadapter/client.go +++ b/markets/storageadapter/client.go @@ -23,6 +23,7 @@ import ( "github.com/filecoin-project/lotus/api" "github.com/filecoin-project/lotus/build" + marketactor "github.com/filecoin-project/lotus/chain/actors/builtin/market" "github.com/filecoin-project/lotus/chain/events" "github.com/filecoin-project/lotus/chain/events/state" "github.com/filecoin-project/lotus/chain/market" @@ -212,8 +213,8 @@ func (c *ClientNodeAdapter) DealProviderCollateralBounds(ctx context.Context, si return big.Mul(bounds.Min, big.NewInt(clientOverestimation)), bounds.Max, nil } -func (c *ClientNodeAdapter) OnDealSectorCommitted(ctx context.Context, provider address.Address, dealID abi.DealID, publishCid *cid.Cid, cb storagemarket.DealSectorCommittedCallback) error { - return OnDealSectorCommitted(ctx, c, c.ev, provider, dealID, publishCid, cb) +func (c *ClientNodeAdapter) OnDealSectorCommitted(ctx context.Context, provider address.Address, dealID abi.DealID, proposal market2.DealProposal, publishCid *cid.Cid, cb storagemarket.DealSectorCommittedCallback) error { + return OnDealSectorCommitted(ctx, c, c.ev, provider, dealID, marketactor.DealProposal(proposal), publishCid, cb) } func (c *ClientNodeAdapter) OnDealExpiredOrSlashed(ctx context.Context, dealID abi.DealID, onDealExpired storagemarket.DealExpiredCallback, onDealSlashed storagemarket.DealSlashedCallback) error { diff --git a/markets/storageadapter/getcurrentdealinfo.go b/markets/storageadapter/getcurrentdealinfo.go index f4667fc05..91a55f9df 100644 --- a/markets/storageadapter/getcurrentdealinfo.go +++ b/markets/storageadapter/getcurrentdealinfo.go @@ -19,10 +19,13 @@ type getCurrentDealInfoAPI interface { } // GetCurrentDealInfo gets current information on a deal, and corrects the deal ID as needed -func GetCurrentDealInfo(ctx context.Context, ts *types.TipSet, api getCurrentDealInfoAPI, dealID abi.DealID, publishCid *cid.Cid) (abi.DealID, *api.MarketDeal, error) { +func GetCurrentDealInfo(ctx context.Context, ts *types.TipSet, api getCurrentDealInfoAPI, dealID abi.DealID, proposal market.DealProposal, publishCid *cid.Cid) (abi.DealID, *api.MarketDeal, error) { marketDeal, dealErr := api.StateMarketStorageDeal(ctx, dealID, ts.Key()) if dealErr == nil { - return dealID, marketDeal, nil + if marketDeal.Proposal == proposal { + return dealID, marketDeal, nil + } + dealErr = xerrors.Errorf("Deal proposals did not match") } if publishCid == nil { return dealID, nil, dealErr @@ -55,5 +58,9 @@ func GetCurrentDealInfo(ctx context.Context, ts *types.TipSet, api getCurrentDea dealID = retval.IDs[0] marketDeal, err = api.StateMarketStorageDeal(ctx, dealID, ts.Key()) + if err == nil && marketDeal.Proposal != proposal { + return dealID, nil, xerrors.Errorf("Deal proposals did not match") + } + return dealID, marketDeal, err } diff --git a/markets/storageadapter/getcurrentdealinfo_test.go b/markets/storageadapter/getcurrentdealinfo_test.go index 3bb117038..efbc11250 100644 --- a/markets/storageadapter/getcurrentdealinfo_test.go +++ b/markets/storageadapter/getcurrentdealinfo_test.go @@ -30,7 +30,25 @@ func TestGetCurrentDealInfo(t *testing.T) { twoValuesReturn := makePublishDealsReturnBytes(t, []abi.DealID{abi.DealID(rand.Uint64()), abi.DealID(rand.Uint64())}) sameValueReturn := makePublishDealsReturnBytes(t, []abi.DealID{startDealID}) newValueReturn := makePublishDealsReturnBytes(t, []abi.DealID{newDealID}) + proposal := market.DealProposal{ + PieceCID: dummyCid, + PieceSize: abi.PaddedPieceSize(rand.Uint64()), + Label: "success", + } + otherProposal := market.DealProposal{ + PieceCID: dummyCid, + PieceSize: abi.PaddedPieceSize(rand.Uint64()), + Label: "other", + } successDeal := &api.MarketDeal{ + Proposal: proposal, + State: market.DealState{ + SectorStartEpoch: 1, + LastUpdatedEpoch: 2, + }, + } + otherDeal := &api.MarketDeal{ + Proposal: otherProposal, State: market.DealState{ SectorStartEpoch: 1, LastUpdatedEpoch: 2, @@ -56,6 +74,13 @@ func TestGetCurrentDealInfo(t *testing.T) { expectedDealID: startDealID, expectedError: errNotFound, }, + "publish CID = nil, other deal on lookup": { + marketDeals: map[abi.DealID]*api.MarketDeal{ + startDealID: otherDeal, + }, + expectedDealID: startDealID, + expectedError: xerrors.Errorf("Deal proposals did not match"), + }, "search message fails": { publishCid: &dummyCid, searchMessageErr: errors.New("something went wrong"), @@ -119,6 +144,21 @@ func TestGetCurrentDealInfo(t *testing.T) { expectedDealID: newDealID, expectedMarketDeal: successDeal, }, + "new deal id after other deal found": { + publishCid: &dummyCid, + searchMessageLookup: &api.MsgLookup{ + Receipt: types.MessageReceipt{ + ExitCode: exitcode.Ok, + Return: newValueReturn, + }, + }, + marketDeals: map[abi.DealID]*api.MarketDeal{ + startDealID: otherDeal, + newDealID: successDeal, + }, + expectedDealID: newDealID, + expectedMarketDeal: successDeal, + }, "new deal id failure": { publishCid: &dummyCid, searchMessageLookup: &api.MsgLookup{ @@ -130,6 +170,20 @@ func TestGetCurrentDealInfo(t *testing.T) { expectedDealID: newDealID, expectedError: errNotFound, }, + "new deal id, failure due to other deal present": { + publishCid: &dummyCid, + searchMessageLookup: &api.MsgLookup{ + Receipt: types.MessageReceipt{ + ExitCode: exitcode.Ok, + Return: newValueReturn, + }, + }, + marketDeals: map[abi.DealID]*api.MarketDeal{ + newDealID: otherDeal, + }, + expectedDealID: newDealID, + expectedError: xerrors.Errorf("Deal proposals did not match"), + }, } for testCase, data := range testCases { t.Run(testCase, func(t *testing.T) { @@ -147,7 +201,7 @@ func TestGetCurrentDealInfo(t *testing.T) { MarketDeals: marketDeals, } - dealID, marketDeal, err := GetCurrentDealInfo(ctx, ts, api, startDealID, data.publishCid) + dealID, marketDeal, err := GetCurrentDealInfo(ctx, ts, api, startDealID, proposal, data.publishCid) require.Equal(t, data.expectedDealID, dealID) require.Equal(t, data.expectedMarketDeal, marketDeal) if data.expectedError == nil { diff --git a/markets/storageadapter/ondealsectorcommitted.go b/markets/storageadapter/ondealsectorcommitted.go index 72569cf7e..59e649147 100644 --- a/markets/storageadapter/ondealsectorcommitted.go +++ b/markets/storageadapter/ondealsectorcommitted.go @@ -8,6 +8,7 @@ import ( "github.com/filecoin-project/go-fil-markets/storagemarket" "github.com/filecoin-project/go-state-types/abi" "github.com/filecoin-project/lotus/build" + "github.com/filecoin-project/lotus/chain/actors/builtin/market" "github.com/filecoin-project/lotus/chain/actors/builtin/miner" "github.com/filecoin-project/lotus/chain/events" "github.com/filecoin-project/lotus/chain/types" @@ -19,9 +20,9 @@ type sectorCommittedEventsAPI interface { Called(check events.CheckFunc, msgHnd events.MsgHandler, rev events.RevertHandler, confidence int, timeout abi.ChainEpoch, mf events.MsgMatchFunc) error } -func OnDealSectorCommitted(ctx context.Context, api getCurrentDealInfoAPI, eventsApi sectorCommittedEventsAPI, provider address.Address, dealID abi.DealID, publishCid *cid.Cid, cb storagemarket.DealSectorCommittedCallback) error { +func OnDealSectorCommitted(ctx context.Context, api getCurrentDealInfoAPI, eventsApi sectorCommittedEventsAPI, provider address.Address, dealID abi.DealID, proposal market.DealProposal, publishCid *cid.Cid, cb storagemarket.DealSectorCommittedCallback) error { checkFunc := func(ts *types.TipSet) (done bool, more bool, err error) { - newDealID, sd, err := GetCurrentDealInfo(ctx, ts, api, dealID, publishCid) + newDealID, sd, err := GetCurrentDealInfo(ctx, ts, api, dealID, proposal, publishCid) if err != nil { // TODO: This may be fine for some errors return false, false, xerrors.Errorf("failed to look up deal on chain: %w", err) @@ -47,16 +48,16 @@ func OnDealSectorCommitted(ctx context.Context, api getCurrentDealInfoAPI, event }() switch msg.Method { case miner.Methods.PreCommitSector: - dealID, _, err = GetCurrentDealInfo(ctx, ts, api, dealID, publishCid) - if err != nil { - return false, err - } - var params miner.SectorPreCommitInfo if err := params.UnmarshalCBOR(bytes.NewReader(msg.Params)); err != nil { return false, xerrors.Errorf("unmarshal pre commit: %w", err) } + dealID, _, err = GetCurrentDealInfo(ctx, ts, api, dealID, proposal, publishCid) + if err != nil { + return false, err + } + for _, did := range params.DealIDs { if did == dealID { sectorNumber = params.SectorNumber @@ -71,7 +72,7 @@ func OnDealSectorCommitted(ctx context.Context, api getCurrentDealInfoAPI, event return false, nil } - _, sd, err := GetCurrentDealInfo(ctx, ts, api, dealID, publishCid) + _, sd, err := GetCurrentDealInfo(ctx, ts, api, dealID, proposal, publishCid) if err != nil { return false, xerrors.Errorf("failed to look up deal on chain: %w", err) } @@ -105,15 +106,15 @@ func OnDealSectorCommitted(ctx context.Context, api getCurrentDealInfoAPI, event case miner.Methods.PreCommitSector: return !sectorFound, nil case miner.Methods.ProveCommitSector: + if !sectorFound { + return false, nil + } + var params miner.ProveCommitSectorParams if err := params.UnmarshalCBOR(bytes.NewReader(msg.Params)); err != nil { return false, xerrors.Errorf("failed to unmarshal prove commit sector params: %w", err) } - if !sectorFound { - return false, nil - } - if params.SectorNumber != sectorNumber { return false, nil } diff --git a/markets/storageadapter/ondealsectorcommitted_test.go b/markets/storageadapter/ondealsectorcommitted_test.go index f2dafc137..10175db79 100644 --- a/markets/storageadapter/ondealsectorcommitted_test.go +++ b/markets/storageadapter/ondealsectorcommitted_test.go @@ -29,17 +29,25 @@ func TestOnDealSectorCommitted(t *testing.T) { ctx := context.Background() publishCid := generateCids(1)[0] sealedCid := generateCids(1)[0] + pieceCid := generateCids(1)[0] startDealID := abi.DealID(rand.Uint64()) newDealID := abi.DealID(rand.Uint64()) newValueReturn := makePublishDealsReturnBytes(t, []abi.DealID{newDealID}) sectorNumber := abi.SectorNumber(rand.Uint64()) + proposal := market.DealProposal{ + PieceCID: pieceCid, + PieceSize: abi.PaddedPieceSize(rand.Uint64()), + Label: "success", + } unfinishedDeal := &api.MarketDeal{ + Proposal: proposal, State: market.DealState{ SectorStartEpoch: -1, LastUpdatedEpoch: 2, }, } successDeal := &api.MarketDeal{ + Proposal: proposal, State: market.DealState{ SectorStartEpoch: 1, LastUpdatedEpoch: 2, @@ -263,7 +271,7 @@ func TestOnDealSectorCommitted(t *testing.T) { cbCallCount++ cbError = err } - err = OnDealSectorCommitted(ctx, api, eventsAPI, provider, startDealID, &publishCid, cb) + err = OnDealSectorCommitted(ctx, api, eventsAPI, provider, startDealID, proposal, &publishCid, cb) if data.expectedError == nil { require.NoError(t, err) } else { diff --git a/markets/storageadapter/provider.go b/markets/storageadapter/provider.go index 98a7d16ed..6e96c1147 100644 --- a/markets/storageadapter/provider.go +++ b/markets/storageadapter/provider.go @@ -253,8 +253,8 @@ func (n *ProviderNodeAdapter) DealProviderCollateralBounds(ctx context.Context, return bounds.Min, bounds.Max, nil } -func (n *ProviderNodeAdapter) OnDealSectorCommitted(ctx context.Context, provider address.Address, dealID abi.DealID, publishCid *cid.Cid, cb storagemarket.DealSectorCommittedCallback) error { - return OnDealSectorCommitted(ctx, n, n.ev, provider, dealID, publishCid, cb) +func (n *ProviderNodeAdapter) OnDealSectorCommitted(ctx context.Context, provider address.Address, dealID abi.DealID, proposal market2.DealProposal, publishCid *cid.Cid, cb storagemarket.DealSectorCommittedCallback) error { + return OnDealSectorCommitted(ctx, n, n.ev, provider, dealID, market.DealProposal(proposal), publishCid, cb) } func (n *ProviderNodeAdapter) GetChainHead(ctx context.Context) (shared.TipSetToken, abi.ChainEpoch, error) {