From 672120f9094e3833849649055fc530fcb6064fb6 Mon Sep 17 00:00:00 2001 From: shannonwells Date: Thu, 9 Apr 2020 17:54:57 -0700 Subject: [PATCH 1/9] WIP test + new states and events and stuff --- .../impl/clientstates/client_fsm.go | 64 +++++++++++++------ .../impl/clientstates/client_states.go | 14 ++-- .../impl/clientstates/client_states_test.go | 16 ++--- .../testnodes/test_retrieval_client_node.go | 7 +- retrievalmarket/types.go | 33 ++++++++-- 5 files changed, 92 insertions(+), 42 deletions(-) diff --git a/retrievalmarket/impl/clientstates/client_fsm.go b/retrievalmarket/impl/clientstates/client_fsm.go index 11e9e3b9..3d1db7f6 100644 --- a/retrievalmarket/impl/clientstates/client_fsm.go +++ b/retrievalmarket/impl/clientstates/client_fsm.go @@ -7,6 +7,7 @@ import ( "github.com/filecoin-project/go-statemachine/fsm" "github.com/filecoin-project/specs-actors/actors/abi" "github.com/filecoin-project/specs-actors/actors/abi/big" + "github.com/ipfs/go-cid" "golang.org/x/xerrors" rm "github.com/filecoin-project/go-fil-markets/retrievalmarket" @@ -33,14 +34,21 @@ var ClientEvents = fsm.Events{ deal.Message = xerrors.Errorf("getting payment channel: %w", err).Error() return nil }), - fsm.Event(rm.ClientEventAllocateLaneErrored). - From(rm.DealStatusAccepted).To(rm.DealStatusFailed). - Action(func(deal *rm.ClientDealState, err error) error { - deal.Message = xerrors.Errorf("allocating payment lane: %w", err).Error() + fsm.Event(rm.ClientEventPaymentChannelCreateInitiated). + From(rm.DealStatusAccepted).To(rm.DealStatusPaymentChannelCreating). + Action(func(deal *rm.ClientDealState, msgCID cid.Cid) error { + // WaitForPaychCreate, triggers ClientEventPaymentChannelCreated return nil - }), + }), fsm.Event(rm.ClientEventPaymentChannelCreated). - From(rm.DealStatusAccepted).To(rm.DealStatusPaymentChannelCreated). + From(rm.DealStatusPaymentChannelCreating).ToNoChange(). + Action(func(deal *rm.ClientDealState, paych address.Address) error { + // AllocateLane, triggers ClientEventPaymentChannelReady + return nil + }), + fsm.Event(rm.ClientEventPaymentChannelReady). + FromMany(rm.DealStatusPaymentChannelCreating, rm.ClientEventPaymentChannelFundsAdded). + To(rm.DealStatusPaymentChannelReady). Action(func(deal *rm.ClientDealState, payCh address.Address, lane uint64) error { deal.PaymentInfo = &rm.PaymentInfo{ PayCh: payCh, @@ -48,6 +56,18 @@ var ClientEvents = fsm.Events{ } return nil }), + fsm.Event(rm.ClientEventPaymentChannelAddingFunds). + From(rm.DealStatusPaymentChannelReady).To(rm.DealStatusPaymentChannelAddingFunds). + Action(func(deal *rm.ClientDealState, paych address.Address, msgCID cid.Cid) error { + // WaitForPaychAddFunds, triggers ClientEventPaychFundsAdded + return nil + }), + fsm.Event(rm.ClientEventAllocateLaneErrored). + From(rm.DealStatusAccepted).To(rm.DealStatusFailed). + Action(func(deal *rm.ClientDealState, err error) error { + deal.Message = xerrors.Errorf("allocating payment lane: %w", err).Error() + return nil + }), fsm.Event(rm.ClientEventWriteDealProposalErrored). FromAny().To(rm.DealStatusErrored). Action(func(deal *rm.ClientDealState, err error) error { @@ -123,50 +143,52 @@ var ClientEvents = fsm.Events{ return nil }), fsm.Event(rm.ClientEventConsumeBlockFailed). - FromMany(rm.DealStatusPaymentChannelCreated, rm.DealStatusOngoing).To(rm.DealStatusFailed). + FromMany(rm.DealStatusPaymentChannelReady, rm.DealStatusOngoing).To(rm.DealStatusFailed). Action(func(deal *rm.ClientDealState, err error) error { deal.Message = xerrors.Errorf("consuming block: %w", err).Error() return nil }), fsm.Event(rm.ClientEventLastPaymentRequested). - FromMany(rm.DealStatusPaymentChannelCreated, + FromMany(rm.DealStatusPaymentChannelReady, rm.DealStatusOngoing, rm.DealStatusBlocksComplete).To(rm.DealStatusFundsNeededLastPayment). Action(recordPaymentOwed), fsm.Event(rm.ClientEventAllBlocksReceived). - FromMany(rm.DealStatusPaymentChannelCreated, + FromMany(rm.DealStatusPaymentChannelReady, rm.DealStatusOngoing, rm.DealStatusBlocksComplete).To(rm.DealStatusBlocksComplete). Action(recordProcessed), fsm.Event(rm.ClientEventComplete). - FromMany(rm.DealStatusPaymentChannelCreated, + FromMany(rm.DealStatusPaymentChannelReady, rm.DealStatusOngoing, rm.DealStatusBlocksComplete, rm.DealStatusFinalizing).To(rm.DealStatusCompleted). Action(recordProcessed), fsm.Event(rm.ClientEventEarlyTermination). - FromMany(rm.DealStatusPaymentChannelCreated, rm.DealStatusOngoing).To(rm.DealStatusFailed). + FromMany(rm.DealStatusPaymentChannelReady, rm.DealStatusOngoing).To(rm.DealStatusFailed). Action(func(deal *rm.ClientDealState) error { deal.Message = "received complete status before all blocks received" return nil }), fsm.Event(rm.ClientEventPaymentRequested). - FromMany(rm.DealStatusPaymentChannelCreated, rm.DealStatusOngoing).To(rm.DealStatusFundsNeeded). + FromMany(rm.DealStatusPaymentChannelReady, rm.DealStatusOngoing).To(rm.DealStatusFundsNeeded). Action(recordPaymentOwed), fsm.Event(rm.ClientEventBlocksReceived). - From(rm.DealStatusPaymentChannelCreated).To(rm.DealStatusOngoing). + From(rm.DealStatusPaymentChannelReady).To(rm.DealStatusOngoing). From(rm.DealStatusOngoing).ToNoChange(). Action(recordProcessed), } // ClientStateEntryFuncs are the handlers for different states in a retrieval client var ClientStateEntryFuncs = fsm.StateEntryFuncs{ - rm.DealStatusNew: ProposeDeal, - rm.DealStatusAccepted: SetupPaymentChannel, - rm.DealStatusPaymentChannelCreated: ProcessNextResponse, - rm.DealStatusOngoing: ProcessNextResponse, - rm.DealStatusBlocksComplete: ProcessNextResponse, - rm.DealStatusFundsNeeded: ProcessPaymentRequested, - rm.DealStatusFundsNeededLastPayment: ProcessPaymentRequested, - rm.DealStatusFinalizing: Finalize, + rm.DealStatusNew: ProposeDeal, + rm.DealStatusAccepted: SetupPaymentChannel, + rm.DealStatusPaymentChannelCreating: ProcessNextResponse, + rm.DealStatusPaymentChannelAddingFunds: ProcessNextResponse, + rm.DealStatusPaymentChannelReady: ProcessNextResponse, + rm.DealStatusOngoing: ProcessNextResponse, + rm.DealStatusBlocksComplete: ProcessNextResponse, + rm.DealStatusFundsNeeded: ProcessPaymentRequested, + rm.DealStatusFundsNeededLastPayment: ProcessPaymentRequested, + rm.DealStatusFinalizing: Finalize, } diff --git a/retrievalmarket/impl/clientstates/client_states.go b/retrievalmarket/impl/clientstates/client_states.go index 1b46f356..80ffa357 100644 --- a/retrievalmarket/impl/clientstates/client_states.go +++ b/retrievalmarket/impl/clientstates/client_states.go @@ -3,6 +3,7 @@ package clientstates import ( "context" + "github.com/filecoin-project/go-address" "github.com/filecoin-project/go-statemachine/fsm" "github.com/filecoin-project/specs-actors/actors/abi" "github.com/filecoin-project/specs-actors/actors/abi/big" @@ -25,15 +26,18 @@ func SetupPaymentChannel(ctx fsm.Context, environment ClientDealEnvironment, dea return ctx.Trigger(rm.ClientEventPaymentChannelErrored, err) } - paych, err := environment.Node().GetOrCreatePaymentChannel(ctx.Context(), deal.ClientWallet, deal.MinerWallet, deal.TotalFunds, tok) + paych, msgCID, err := environment.Node().GetOrCreatePaymentChannel(ctx.Context(), deal.ClientWallet, deal.MinerWallet, deal.TotalFunds, tok) if err != nil { return ctx.Trigger(rm.ClientEventPaymentChannelErrored, err) } - lane, err := environment.Node().AllocateLane(paych) - if err != nil { - return ctx.Trigger(rm.ClientEventAllocateLaneErrored, err) + if paych == address.Undef { + return ctx.Trigger(rm.ClientEventPaymentChannelCreateInitiated) } - return ctx.Trigger(rm.ClientEventPaymentChannelCreated, paych, lane) + //lane, err := environment.Node().AllocateLane(paych) + //if err != nil { + // return ctx.Trigger(rm.ClientEventAllocateLaneErrored, err) + //} + return ctx.Trigger(rm.ClientEventPaymentChannelAddingFunds, paych, msgCID) } // ProposeDeal sends the proposal to the other party diff --git a/retrievalmarket/impl/clientstates/client_states_test.go b/retrievalmarket/impl/clientstates/client_states_test.go index ec7578f8..ae2e91b2 100644 --- a/retrievalmarket/impl/clientstates/client_states_test.go +++ b/retrievalmarket/impl/clientstates/client_states_test.go @@ -17,7 +17,7 @@ import ( "github.com/stretchr/testify/require" "github.com/filecoin-project/go-fil-markets/retrievalmarket" - clientstates "github.com/filecoin-project/go-fil-markets/retrievalmarket/impl/clientstates" + "github.com/filecoin-project/go-fil-markets/retrievalmarket/impl/clientstates" "github.com/filecoin-project/go-fil-markets/retrievalmarket/impl/testnodes" rmnet "github.com/filecoin-project/go-fil-markets/retrievalmarket/network" testnet "github.com/filecoin-project/go-fil-markets/shared_testutil" @@ -71,17 +71,15 @@ func TestSetupPaymentChannel(t *testing.T) { fsmCtx.ReplayEvents(t, dealState) } - t.Run("it works", func(t *testing.T) { - dealState := makeDealState(retrievalmarket.DealStatusAccepted) + t.Run("payment channel create initiated", func(t *testing.T) { envParams := testnodes.TestRetrievalClientNodeParams{ - PayCh: expectedPayCh, - Lane: expectedLane, + PayCh: address.Undef, + CreatePaychCID: testnet.GenerateCids(1)[0], } + dealState := makeDealState(retrievalmarket.DealStatusAccepted) runSetupPaymentChannel(t, envParams, dealState) - require.Empty(t, dealState.Message) - require.Equal(t, dealState.Status, retrievalmarket.DealStatusPaymentChannelCreated) - require.Equal(t, dealState.PaymentInfo.PayCh, expectedPayCh) - require.Equal(t, dealState.PaymentInfo.Lane, expectedLane) + require.NotEmpty(t, dealState.Message) + require.Equal(t, dealState.Status, retrievalmarket.DealStatusPaymentChannelCreating) }) t.Run("when create payment channel fails", func(t *testing.T) { diff --git a/retrievalmarket/impl/testnodes/test_retrieval_client_node.go b/retrievalmarket/impl/testnodes/test_retrieval_client_node.go index 675284fb..0fc1b2f2 100644 --- a/retrievalmarket/impl/testnodes/test_retrieval_client_node.go +++ b/retrievalmarket/impl/testnodes/test_retrieval_client_node.go @@ -6,6 +6,7 @@ import ( "github.com/filecoin-project/go-address" "github.com/filecoin-project/specs-actors/actors/abi" "github.com/filecoin-project/specs-actors/actors/builtin/paych" + "github.com/ipfs/go-cid" "github.com/filecoin-project/go-fil-markets/retrievalmarket" "github.com/filecoin-project/go-fil-markets/shared" @@ -16,6 +17,7 @@ import ( type TestRetrievalClientNode struct { payCh address.Address payChErr error + createPaychMsgCID, addFundsMsgCID cid.Cid lane uint64 laneError error voucher *paych.SignedVoucher @@ -30,6 +32,7 @@ type TestRetrievalClientNode struct { type TestRetrievalClientNodeParams struct { PayCh address.Address PayChErr error + CreatePaychCID, AddFundsCID cid.Cid Lane uint64 LaneError error Voucher *paych.SignedVoucher @@ -57,11 +60,11 @@ func NewTestRetrievalClientNode(params TestRetrievalClientNodeParams) *TestRetri } // GetOrCreatePaymentChannel returns a mocked payment channel -func (trcn *TestRetrievalClientNode) GetOrCreatePaymentChannel(ctx context.Context, clientAddress address.Address, minerAddress address.Address, clientFundsAvailable abi.TokenAmount, tok shared.TipSetToken) (address.Address, error) { +func (trcn *TestRetrievalClientNode) GetOrCreatePaymentChannel(ctx context.Context, clientAddress address.Address, minerAddress address.Address, clientFundsAvailable abi.TokenAmount, tok shared.TipSetToken) (address.Address, cid.Cid, error) { if trcn.getCreatePaymentChannelRecorder != nil { trcn.getCreatePaymentChannelRecorder(clientAddress, minerAddress, clientFundsAvailable) } - return trcn.payCh, trcn.payChErr + return trcn.payCh, trcn.createPaychMsgCID, trcn.payChErr } // AllocateLane creates a mock lane on a payment channel diff --git a/retrievalmarket/types.go b/retrievalmarket/types.go index 349f7719..6216e5c5 100644 --- a/retrievalmarket/types.go +++ b/retrievalmarket/types.go @@ -70,9 +70,24 @@ const ( // ClientEventAllocateLaneErrored means there was a failure creating a lane in a payment channel ClientEventAllocateLaneErrored - // ClientEventPaymentChannelCreated means a payment channel has successfully been created + // ClientEventPaymentChannelCreateInitiated means the request to create a payment channel was + // successful and we are waiting for it to appear on chain + ClientEventPaymentChannelCreateInitiated + + // ClientEventPaymentChannelCreated means a payment channel has been created ClientEventPaymentChannelCreated + // ClientEventPaymentChannelReady means the newly created payment channel is ready for the + // deal to begin + ClientEventPaymentChannelReady + + // ClientEventPaymentChannelAddingFunds means the request to add funds to a payment channel was + // successful and we are waiting for the funds to be sent + ClientEventPaymentChannelAddingFunds + + // ClientEventPaymentChannelFundsAdded means funds have been added to the payment channel + ClientEventPaymentChannelFundsAdded + // ClientEventWriteDealProposalErrored means a network error writing a deal proposal ClientEventWriteDealProposalErrored @@ -183,7 +198,7 @@ type RetrievalClientNode interface { // GetOrCreatePaymentChannel sets up a new payment channel if one does not exist // between a client and a miner and insures the client has the given amount of funds available in the channel - GetOrCreatePaymentChannel(ctx context.Context, clientAddress address.Address, minerAddress address.Address, clientFundsAvailable abi.TokenAmount, tok shared.TipSetToken) (address.Address, error) + GetOrCreatePaymentChannel(ctx context.Context, clientAddress address.Address, minerAddress address.Address, clientFundsAvailable abi.TokenAmount, tok shared.TipSetToken) (address.Address, cid.Cid, error) // Allocate late creates a lane within a payment channel so that calls to // CreatePaymentVoucher will automatically make vouchers only for the difference @@ -437,9 +452,17 @@ const ( // DealStatusNew is a deal that nothing has happened with yet DealStatusNew DealStatus = iota - // DealStatusPaymentChannelCreated is a deal status that has a payment channel + // DealStatusPaymentChannelCreating is the status set while waiting for the + // payment channel creation to complete + DealStatusPaymentChannelCreating + + // DealStatusPaymentChannelAddingFunds is the status when we are waiting for funds + // to finish being sent to the payment channel + DealStatusPaymentChannelAddingFunds + + // DealStatusPaymentChannelReady is a deal status that has a payment channel // & lane setup - DealStatusPaymentChannelCreated + DealStatusPaymentChannelReady // DealStatusAccepted means a deal has been accepted by a provider // and its is ready to proceed with retrieval @@ -487,7 +510,7 @@ const ( // DealStatuses maps deal status to a human readable representation var DealStatuses = map[DealStatus]string{ DealStatusNew: "DealStatusNew", - DealStatusPaymentChannelCreated: "DealStatusPaymentChannelCreated", + DealStatusPaymentChannelCreating: "DealStatusPaymentChannelCreating", DealStatusAccepted: "DealStatusAccepted", DealStatusFailed: "DealStatusFailed", DealStatusRejected: "DealStatusRejected", From 841b4850fbba216c11ab20ced66ef0b03773cf21 Mon Sep 17 00:00:00 2001 From: shannonwells Date: Fri, 10 Apr 2020 16:53:14 -0700 Subject: [PATCH 2/9] passing tests! 'and there was much rejoicing:' yaaaaay. --- .../impl/clientstates/client_fsm.go | 26 ++++------ .../impl/clientstates/client_states.go | 46 ++++++++++++++---- .../impl/clientstates/client_states_test.go | 43 +++++++++++------ retrievalmarket/impl/integration_test.go | 10 +++- .../testnodes/test_retrieval_client_node.go | 48 ++++++++++++------- retrievalmarket/types.go | 14 +++++- retrievalmarket/types_cbor_gen.go | 41 +++++++++++++++- 7 files changed, 165 insertions(+), 63 deletions(-) diff --git a/retrievalmarket/impl/clientstates/client_fsm.go b/retrievalmarket/impl/clientstates/client_fsm.go index 3d1db7f6..36fe923e 100644 --- a/retrievalmarket/impl/clientstates/client_fsm.go +++ b/retrievalmarket/impl/clientstates/client_fsm.go @@ -37,17 +37,13 @@ var ClientEvents = fsm.Events{ fsm.Event(rm.ClientEventPaymentChannelCreateInitiated). From(rm.DealStatusAccepted).To(rm.DealStatusPaymentChannelCreating). Action(func(deal *rm.ClientDealState, msgCID cid.Cid) error { - // WaitForPaychCreate, triggers ClientEventPaymentChannelCreated + deal.WaitMsgCID = &msgCID return nil - }), - fsm.Event(rm.ClientEventPaymentChannelCreated). - From(rm.DealStatusPaymentChannelCreating).ToNoChange(). - Action(func(deal *rm.ClientDealState, paych address.Address) error { - // AllocateLane, triggers ClientEventPaymentChannelReady - return nil - }), + }), + fsm.Event(rm.ClientEventPaymentChannelAddingFunds). + From(rm.DealStatusPaymentChannelReady).To(rm.DealStatusPaymentChannelAddingFunds), fsm.Event(rm.ClientEventPaymentChannelReady). - FromMany(rm.DealStatusPaymentChannelCreating, rm.ClientEventPaymentChannelFundsAdded). + FromMany(rm.DealStatusPaymentChannelCreating, rm.DealStatusPaymentChannelAddingFunds). To(rm.DealStatusPaymentChannelReady). Action(func(deal *rm.ClientDealState, payCh address.Address, lane uint64) error { deal.PaymentInfo = &rm.PaymentInfo{ @@ -56,12 +52,6 @@ var ClientEvents = fsm.Events{ } return nil }), - fsm.Event(rm.ClientEventPaymentChannelAddingFunds). - From(rm.DealStatusPaymentChannelReady).To(rm.DealStatusPaymentChannelAddingFunds). - Action(func(deal *rm.ClientDealState, paych address.Address, msgCID cid.Cid) error { - // WaitForPaychAddFunds, triggers ClientEventPaychFundsAdded - return nil - }), fsm.Event(rm.ClientEventAllocateLaneErrored). From(rm.DealStatusAccepted).To(rm.DealStatusFailed). Action(func(deal *rm.ClientDealState, err error) error { @@ -182,9 +172,9 @@ var ClientEvents = fsm.Events{ // ClientStateEntryFuncs are the handlers for different states in a retrieval client var ClientStateEntryFuncs = fsm.StateEntryFuncs{ rm.DealStatusNew: ProposeDeal, - rm.DealStatusAccepted: SetupPaymentChannel, - rm.DealStatusPaymentChannelCreating: ProcessNextResponse, - rm.DealStatusPaymentChannelAddingFunds: ProcessNextResponse, + rm.DealStatusAccepted: SetupPaymentChannelStart, + rm.DealStatusPaymentChannelCreating: WaitForPaymentChannelCreate, + rm.DealStatusPaymentChannelAddingFunds: WaitForPaymentChannelAddFunds, rm.DealStatusPaymentChannelReady: ProcessNextResponse, rm.DealStatusOngoing: ProcessNextResponse, rm.DealStatusBlocksComplete: ProcessNextResponse, diff --git a/retrievalmarket/impl/clientstates/client_states.go b/retrievalmarket/impl/clientstates/client_states.go index 80ffa357..15e18d90 100644 --- a/retrievalmarket/impl/clientstates/client_states.go +++ b/retrievalmarket/impl/clientstates/client_states.go @@ -19,8 +19,8 @@ type ClientDealEnvironment interface { ConsumeBlock(context.Context, rm.DealID, rm.Block) (uint64, bool, error) } -// SetupPaymentChannel sets up a payment channel for a deal -func SetupPaymentChannel(ctx fsm.Context, environment ClientDealEnvironment, deal rm.ClientDealState) error { +// SetupPaymentChannelStart sets up a payment channel for a deal +func SetupPaymentChannelStart(ctx fsm.Context, environment ClientDealEnvironment, deal rm.ClientDealState) error { tok, _, err := environment.Node().GetChainHead(ctx.Context()) if err != nil { return ctx.Trigger(rm.ClientEventPaymentChannelErrored, err) @@ -30,14 +30,40 @@ func SetupPaymentChannel(ctx fsm.Context, environment ClientDealEnvironment, dea if err != nil { return ctx.Trigger(rm.ClientEventPaymentChannelErrored, err) } - if paych == address.Undef { - return ctx.Trigger(rm.ClientEventPaymentChannelCreateInitiated) + + if paych == address.Undef { + return ctx.Trigger(rm.ClientEventPaymentChannelCreateInitiated, msgCID) + } + + return ctx.Trigger(rm.ClientEventPaymentChannelAddingFunds) +} + +// WaitForPaymentChannelCreate waits for payment channel creation to be posted on chain, +// allocates a lane for vouchers, then signals that the payment channel is ready +func WaitForPaymentChannelCreate(ctx fsm.Context, environment ClientDealEnvironment, deal rm.ClientDealState) error { + paych, err := environment.Node().WaitForPaymentChannelCreation(*deal.WaitMsgCID) + if err != nil { + return ctx.Trigger(rm.ClientEventPaymentChannelErrored, err) + } + // if successful call allocate lane and trigger ClientEventPaymentChannelReady evt + + lane, err := environment.Node().AllocateLane(paych) + if err != nil { + return ctx.Trigger(rm.ClientEventAllocateLaneErrored, err) + } + return ctx.Trigger(rm.ClientEventPaymentChannelReady, paych, lane) +} + +// WaitForPaymentChannelAddFunds waits for funds to be added to a payment channel, then +// signals that payment channel is ready again +func WaitForPaymentChannelAddFunds(ctx fsm.Context, environment ClientDealEnvironment, deal rm.ClientDealState) error { + // call wait for funds to be added func, + err := environment.Node().WaitForPaymentChannelAddFunds(*deal.WaitMsgCID) + if err != nil { + return ctx.Trigger(rm.ClientEventPaymentChannelErrored, err) } - //lane, err := environment.Node().AllocateLane(paych) - //if err != nil { - // return ctx.Trigger(rm.ClientEventAllocateLaneErrored, err) - //} - return ctx.Trigger(rm.ClientEventPaymentChannelAddingFunds, paych, msgCID) + // then trigger ClientEventPaymentChannelReady evt + return ctx.Trigger(rm.ClientEventPaymentChannelReady, deal.PaymentInfo.PayCh, deal.PaymentInfo.Lane) } // ProposeDeal sends the proposal to the other party @@ -151,7 +177,7 @@ func ProcessNextResponse(ctx fsm.Context, environment ClientDealEnvironment, dea return ctx.Trigger(rm.ClientEventEarlyTermination) case rm.DealStatusFundsNeeded: return ctx.Trigger(rm.ClientEventPaymentRequested, totalProcessed, response.PaymentOwed) - case rm.DealStatusOngoing: + case rm.DealStatusOngoing, rm.DealStatusPaymentChannelReady: return ctx.Trigger(rm.ClientEventBlocksReceived, totalProcessed) default: return ctx.Trigger(rm.ClientEventUnknownResponseReceived) diff --git a/retrievalmarket/impl/clientstates/client_states_test.go b/retrievalmarket/impl/clientstates/client_states_test.go index ae2e91b2..b7c8f60d 100644 --- a/retrievalmarket/impl/clientstates/client_states_test.go +++ b/retrievalmarket/impl/clientstates/client_states_test.go @@ -66,45 +66,58 @@ func TestSetupPaymentChannel(t *testing.T) { node := testnodes.NewTestRetrievalClientNode(params) environment := &fakeEnvironment{node, ds, 0, nil} fsmCtx := fsmtest.NewTestContext(ctx, eventMachine) - err := clientstates.SetupPaymentChannel(fsmCtx, environment, *dealState) + err := clientstates.SetupPaymentChannelStart(fsmCtx, environment, *dealState) require.NoError(t, err) fsmCtx.ReplayEvents(t, dealState) } t.Run("payment channel create initiated", func(t *testing.T) { envParams := testnodes.TestRetrievalClientNodeParams{ - PayCh: address.Undef, - CreatePaychCID: testnet.GenerateCids(1)[0], + PayCh: address.Undef, + CreatePaychCID: testnet.GenerateCids(1)[0], } dealState := makeDealState(retrievalmarket.DealStatusAccepted) runSetupPaymentChannel(t, envParams, dealState) - require.NotEmpty(t, dealState.Message) + require.Empty(t, dealState.Message) require.Equal(t, dealState.Status, retrievalmarket.DealStatusPaymentChannelCreating) }) - t.Run("when create payment channel fails", func(t *testing.T) { - dealState := makeDealState(retrievalmarket.DealStatusAccepted) + t.Run("payment channel needs funds added", func(t *testing.T) { envParams := testnodes.TestRetrievalClientNodeParams{ - PayCh: address.Undef, - PayChErr: errors.New("Something went wrong"), - Lane: expectedLane, + AddFundsOnly: true, + PayCh: expectedPayCh, + CreatePaychCID: testnet.GenerateCids(1)[0], } + dealState := makeDealState(retrievalmarket.DealStatusPaymentChannelReady) runSetupPaymentChannel(t, envParams, dealState) - require.NotEmpty(t, dealState.Message) - require.Equal(t, dealState.Status, retrievalmarket.DealStatusFailed) + require.Empty(t, dealState.Message) + require.Equal(t, dealState.Status, retrievalmarket.DealStatusPaymentChannelAddingFunds) }) - t.Run("when allocate lane fails", func(t *testing.T) { + t.Run("when create payment channel fails", func(t *testing.T) { dealState := makeDealState(retrievalmarket.DealStatusAccepted) envParams := testnodes.TestRetrievalClientNodeParams{ - PayCh: expectedPayCh, - Lane: expectedLane, - LaneError: errors.New("Something went wrong"), + PayCh: address.Undef, + PayChErr: errors.New("Something went wrong"), + Lane: expectedLane, } runSetupPaymentChannel(t, envParams, dealState) require.NotEmpty(t, dealState.Message) require.Equal(t, dealState.Status, retrievalmarket.DealStatusFailed) }) + + // TODO: find the right place for this test + //t.Run("when allocate lane fails", func(t *testing.T) { + // dealState := makeDealState(retrievalmarket.ClientEventPaymentChannelAddingFunds) + // envParams := testnodes.TestRetrievalClientNodeParams{ + // PayCh: expectedPayCh, + // Lane: expectedLane, + // LaneError: errors.New("Something went wrong"), + // } + // runSetupPaymentChannel(t, envParams, dealState) + // require.NotEmpty(t, dealState.Message) + // require.Equal(t, dealState.Status, retrievalmarket.DealStatusFailed) + //}) } func TestProposeDeal(t *testing.T) { diff --git a/retrievalmarket/impl/integration_test.go b/retrievalmarket/impl/integration_test.go index f903b599..d62adfed 100644 --- a/retrievalmarket/impl/integration_test.go +++ b/retrievalmarket/impl/integration_test.go @@ -79,7 +79,12 @@ func requireSetupTestClientAndProvider(bgCtx context.Context, t *testing.T, payC retrievalmarket.RetrievalProvider) { testData := tut.NewLibp2pTestData(bgCtx, t) nw1 := rmnet.NewFromLibp2pHost(testData.Host1) - rcNode1 := testnodes.NewTestRetrievalClientNode(testnodes.TestRetrievalClientNodeParams{PayCh: payChAddr}) + cids := tut.GenerateCids(2) + rcNode1 := testnodes.NewTestRetrievalClientNode(testnodes.TestRetrievalClientNodeParams{ + PayCh: payChAddr, + CreatePaychCID: cids[0], + AddFundsCID: cids[1], + }) client, err := retrievalimpl.NewClient(nw1, testData.Bs1, rcNode1, &testPeerResolver{}, testData.Ds1, testData.StoredCounter1) require.NoError(t, err) nw2 := rmnet.NewFromLibp2pHost(testData.Host2) @@ -389,6 +394,7 @@ func setupClient( paymentVoucherRecorder := func(v *paych.SignedVoucher) { createdVoucher = *v } + cids := tut.GenerateCids(2) clientNode := testnodes.NewTestRetrievalClientNode(testnodes.TestRetrievalClientNodeParams{ PayCh: clientPaymentChannel, Lane: expectedVoucher.Lane, @@ -396,6 +402,8 @@ func setupClient( PaymentChannelRecorder: paymentChannelRecorder, AllocateLaneRecorder: laneRecorder, PaymentVoucherRecorder: paymentVoucherRecorder, + CreatePaychCID: cids[0], + AddFundsCID: cids[1], }) client, err := retrievalimpl.NewClient(nw1, testData.Bs1, clientNode, &testPeerResolver{}, testData.Ds1, testData.StoredCounter1) return &createdChan, &newLaneAddr, &createdVoucher, client, err diff --git a/retrievalmarket/impl/testnodes/test_retrieval_client_node.go b/retrievalmarket/impl/testnodes/test_retrieval_client_node.go index 0fc1b2f2..069aefed 100644 --- a/retrievalmarket/impl/testnodes/test_retrieval_client_node.go +++ b/retrievalmarket/impl/testnodes/test_retrieval_client_node.go @@ -15,13 +15,14 @@ import ( // TestRetrievalClientNode is a node adapter for a retrieval client whose responses // are stubbed type TestRetrievalClientNode struct { - payCh address.Address - payChErr error + addFundsOnly bool // set this to true if the payment channel is expected to have been created already + payCh address.Address + payChErr error createPaychMsgCID, addFundsMsgCID cid.Cid - lane uint64 - laneError error - voucher *paych.SignedVoucher - voucherError error + lane uint64 + laneError error + voucher *paych.SignedVoucher + voucherError error allocateLaneRecorder func(address.Address) createPaymentVoucherRecorder func(voucher *paych.SignedVoucher) @@ -30,16 +31,17 @@ type TestRetrievalClientNode struct { // TestRetrievalClientNodeParams are parameters for initializing a TestRetrievalClientNode type TestRetrievalClientNodeParams struct { - PayCh address.Address - PayChErr error + PayCh address.Address + PayChErr error CreatePaychCID, AddFundsCID cid.Cid - Lane uint64 - LaneError error - Voucher *paych.SignedVoucher - VoucherError error - AllocateLaneRecorder func(address.Address) - PaymentVoucherRecorder func(voucher *paych.SignedVoucher) - PaymentChannelRecorder func(address.Address, address.Address, abi.TokenAmount) + Lane uint64 + LaneError error + Voucher *paych.SignedVoucher + VoucherError error + AllocateLaneRecorder func(address.Address) + PaymentVoucherRecorder func(voucher *paych.SignedVoucher) + PaymentChannelRecorder func(address.Address, address.Address, abi.TokenAmount) + AddFundsOnly bool } var _ retrievalmarket.RetrievalClientNode = &TestRetrievalClientNode{} @@ -47,6 +49,7 @@ var _ retrievalmarket.RetrievalClientNode = &TestRetrievalClientNode{} // NewTestRetrievalClientNode instantiates a new TestRetrievalClientNode based ont he given params func NewTestRetrievalClientNode(params TestRetrievalClientNodeParams) *TestRetrievalClientNode { return &TestRetrievalClientNode{ + addFundsOnly: params.AddFundsOnly, payCh: params.PayCh, payChErr: params.PayChErr, lane: params.Lane, @@ -56,6 +59,8 @@ func NewTestRetrievalClientNode(params TestRetrievalClientNodeParams) *TestRetri allocateLaneRecorder: params.AllocateLaneRecorder, createPaymentVoucherRecorder: params.PaymentVoucherRecorder, getCreatePaymentChannelRecorder: params.PaymentChannelRecorder, + createPaychMsgCID: params.CreatePaychCID, + addFundsMsgCID: params.AddFundsCID, } } @@ -64,7 +69,11 @@ func (trcn *TestRetrievalClientNode) GetOrCreatePaymentChannel(ctx context.Conte if trcn.getCreatePaymentChannelRecorder != nil { trcn.getCreatePaymentChannelRecorder(clientAddress, minerAddress, clientFundsAvailable) } - return trcn.payCh, trcn.createPaychMsgCID, trcn.payChErr + var payCh address.Address + if trcn.addFundsOnly { + payCh = trcn.payCh + } + return payCh, trcn.createPaychMsgCID, trcn.payChErr } // AllocateLane creates a mock lane on a payment channel @@ -86,3 +95,10 @@ func (trcn *TestRetrievalClientNode) CreatePaymentVoucher(ctx context.Context, p func (trcn *TestRetrievalClientNode) GetChainHead(ctx context.Context) (shared.TipSetToken, abi.ChainEpoch, error) { return shared.TipSetToken{}, 0, nil } +func (trcn *TestRetrievalClientNode) WaitForPaymentChannelAddFunds(messageCID cid.Cid) error { + return nil +} + +func (trcn *TestRetrievalClientNode) WaitForPaymentChannelCreation(messageCID cid.Cid) (address.Address, error) { + return trcn.payCh, nil +} diff --git a/retrievalmarket/types.go b/retrievalmarket/types.go index 6216e5c5..e8352c12 100644 --- a/retrievalmarket/types.go +++ b/retrievalmarket/types.go @@ -55,6 +55,7 @@ type ClientDealState struct { CurrentInterval uint64 PaymentRequested abi.TokenAmount FundsSpent abi.TokenAmount + WaitMsgCID *cid.Cid // the CID of any message the client deal is waiting for } // ClientEvent is an event that occurs in a deal lifecycle on the client @@ -209,6 +210,14 @@ type RetrievalClientNode interface { // given payment channel so that all the payment vouchers in the lane add up // to the given amount (so the payment voucher will be for the difference) CreatePaymentVoucher(ctx context.Context, paymentChannel address.Address, amount abi.TokenAmount, lane uint64, tok shared.TipSetToken) (*paych.SignedVoucher, error) + + // WaitForPaymentChannelAddFunds waits for a message on chain that funds have + // been sent to a payment channel + WaitForPaymentChannelAddFunds(messageCID cid.Cid) error + + // WaitForPaymentChannelCreation waits for a message on chain that a + // payment channel has been created + WaitForPaymentChannelCreation(messageCID cid.Cid) (address.Address, error) } // ProviderDealState is the current state of a deal from the point of view @@ -460,6 +469,9 @@ const ( // to finish being sent to the payment channel DealStatusPaymentChannelAddingFunds + // DealStatusPaymentChannelAllocatingLane is the status during lane allocation + DealStatusPaymentChannelAllocatingLane + // DealStatusPaymentChannelReady is a deal status that has a payment channel // & lane setup DealStatusPaymentChannelReady @@ -510,7 +522,7 @@ const ( // DealStatuses maps deal status to a human readable representation var DealStatuses = map[DealStatus]string{ DealStatusNew: "DealStatusNew", - DealStatusPaymentChannelCreating: "DealStatusPaymentChannelCreating", + DealStatusPaymentChannelCreating: "DealStatusPaymentChannelCreating", DealStatusAccepted: "DealStatusAccepted", DealStatusFailed: "DealStatusFailed", DealStatusRejected: "DealStatusRejected", diff --git a/retrievalmarket/types_cbor_gen.go b/retrievalmarket/types_cbor_gen.go index e867c96c..b6b6f7e2 100644 --- a/retrievalmarket/types_cbor_gen.go +++ b/retrievalmarket/types_cbor_gen.go @@ -874,7 +874,7 @@ func (t *ClientDealState) MarshalCBOR(w io.Writer) error { _, err := w.Write(cbg.CborNull) return err } - if _, err := w.Write([]byte{141}); err != nil { + if _, err := w.Write([]byte{142}); err != nil { return err } @@ -960,6 +960,19 @@ func (t *ClientDealState) MarshalCBOR(w io.Writer) error { if err := t.FundsSpent.MarshalCBOR(w); err != nil { return err } + + // t.WaitMsgCID (cid.Cid) (struct) + + if t.WaitMsgCID == nil { + if _, err := w.Write(cbg.CborNull); err != nil { + return err + } + } else { + if err := cbg.WriteCid(w, *t.WaitMsgCID); err != nil { + return xerrors.Errorf("failed to write cid field t.WaitMsgCID: %w", err) + } + } + return nil } @@ -974,7 +987,7 @@ func (t *ClientDealState) UnmarshalCBOR(r io.Reader) error { return fmt.Errorf("cbor input should be of type array") } - if extra != 13 { + if extra != 14 { return fmt.Errorf("cbor input had wrong number of fields") } @@ -1128,6 +1141,30 @@ func (t *ClientDealState) UnmarshalCBOR(r io.Reader) error { return xerrors.Errorf("unmarshaling t.FundsSpent: %w", err) } + } + // t.WaitMsgCID (cid.Cid) (struct) + + { + + pb, err := br.PeekByte() + if err != nil { + return err + } + if pb == cbg.CborNull[0] { + var nbuf [1]byte + if _, err := br.Read(nbuf[:]); err != nil { + return err + } + } else { + + c, err := cbg.ReadCid(br) + if err != nil { + return xerrors.Errorf("failed to read cid field t.WaitMsgCID: %w", err) + } + + t.WaitMsgCID = &c + } + } return nil } From 0c862dbe190126d473b49ebc7df271d3036c46cc Mon Sep 17 00:00:00 2001 From: shannonwells Date: Mon, 13 Apr 2020 16:42:38 -0700 Subject: [PATCH 3/9] tests for adding funds and wait for payment channel creation, some cleanup --- .../impl/clientstates/client_fsm.go | 12 +- .../impl/clientstates/client_states.go | 9 +- .../impl/clientstates/client_states_test.go | 117 ++++++++++++++++-- .../testnodes/test_retrieval_client_node.go | 54 ++++---- retrievalmarket/types.go | 10 +- 5 files changed, 153 insertions(+), 49 deletions(-) diff --git a/retrievalmarket/impl/clientstates/client_fsm.go b/retrievalmarket/impl/clientstates/client_fsm.go index 36fe923e..fa21534b 100644 --- a/retrievalmarket/impl/clientstates/client_fsm.go +++ b/retrievalmarket/impl/clientstates/client_fsm.go @@ -29,7 +29,7 @@ var ClientEvents = fsm.Events{ fsm.Event(rm.ClientEventOpen). From(rm.DealStatusNew).ToNoChange(), fsm.Event(rm.ClientEventPaymentChannelErrored). - From(rm.DealStatusAccepted).To(rm.DealStatusFailed). + From(rm.DealStatusPaymentChannelCreating).To(rm.DealStatusFailed). Action(func(deal *rm.ClientDealState, err error) error { deal.Message = xerrors.Errorf("getting payment channel: %w", err).Error() return nil @@ -41,7 +41,7 @@ var ClientEvents = fsm.Events{ return nil }), fsm.Event(rm.ClientEventPaymentChannelAddingFunds). - From(rm.DealStatusPaymentChannelReady).To(rm.DealStatusPaymentChannelAddingFunds), + FromMany(rm.DealStatusOngoing, rm.DealStatusPaymentChannelReady).To(rm.DealStatusPaymentChannelAddingFunds), fsm.Event(rm.ClientEventPaymentChannelReady). FromMany(rm.DealStatusPaymentChannelCreating, rm.DealStatusPaymentChannelAddingFunds). To(rm.DealStatusPaymentChannelReady). @@ -53,11 +53,17 @@ var ClientEvents = fsm.Events{ return nil }), fsm.Event(rm.ClientEventAllocateLaneErrored). - From(rm.DealStatusAccepted).To(rm.DealStatusFailed). + From(rm.DealStatusPaymentChannelCreating).To(rm.DealStatusFailed). Action(func(deal *rm.ClientDealState, err error) error { deal.Message = xerrors.Errorf("allocating payment lane: %w", err).Error() return nil }), + fsm.Event(rm.ClientEventPaymentChannelAddFundsErrored). + From(rm.DealStatusPaymentChannelAddingFunds).To(rm.DealStatusFailed). + Action(func(deal *rm.ClientDealState, err error) error { + deal.Message = xerrors.Errorf("wait for add funds: %w", err).Error() + return nil + }), fsm.Event(rm.ClientEventWriteDealProposalErrored). FromAny().To(rm.DealStatusErrored). Action(func(deal *rm.ClientDealState, err error) error { diff --git a/retrievalmarket/impl/clientstates/client_states.go b/retrievalmarket/impl/clientstates/client_states.go index 15e18d90..1e627b7e 100644 --- a/retrievalmarket/impl/clientstates/client_states.go +++ b/retrievalmarket/impl/clientstates/client_states.go @@ -19,7 +19,7 @@ type ClientDealEnvironment interface { ConsumeBlock(context.Context, rm.DealID, rm.Block) (uint64, bool, error) } -// SetupPaymentChannelStart sets up a payment channel for a deal +// SetupPaymentChannelStart initiates setting up a payment channel for a deal func SetupPaymentChannelStart(ctx fsm.Context, environment ClientDealEnvironment, deal rm.ClientDealState) error { tok, _, err := environment.Node().GetChainHead(ctx.Context()) if err != nil { @@ -45,7 +45,6 @@ func WaitForPaymentChannelCreate(ctx fsm.Context, environment ClientDealEnvironm if err != nil { return ctx.Trigger(rm.ClientEventPaymentChannelErrored, err) } - // if successful call allocate lane and trigger ClientEventPaymentChannelReady evt lane, err := environment.Node().AllocateLane(paych) if err != nil { @@ -54,15 +53,13 @@ func WaitForPaymentChannelCreate(ctx fsm.Context, environment ClientDealEnvironm return ctx.Trigger(rm.ClientEventPaymentChannelReady, paych, lane) } -// WaitForPaymentChannelAddFunds waits for funds to be added to a payment channel, then +// WaitForPaymentChannelAddFunds waits for funds to be added to an existing payment channel, then // signals that payment channel is ready again func WaitForPaymentChannelAddFunds(ctx fsm.Context, environment ClientDealEnvironment, deal rm.ClientDealState) error { - // call wait for funds to be added func, err := environment.Node().WaitForPaymentChannelAddFunds(*deal.WaitMsgCID) if err != nil { - return ctx.Trigger(rm.ClientEventPaymentChannelErrored, err) + return ctx.Trigger(rm.ClientEventPaymentChannelAddFundsErrored, err) } - // then trigger ClientEventPaymentChannelReady evt return ctx.Trigger(rm.ClientEventPaymentChannelReady, deal.PaymentInfo.PayCh, deal.PaymentInfo.Lane) } diff --git a/retrievalmarket/impl/clientstates/client_states_test.go b/retrievalmarket/impl/clientstates/client_states_test.go index b7c8f60d..53182e01 100644 --- a/retrievalmarket/impl/clientstates/client_states_test.go +++ b/retrievalmarket/impl/clientstates/client_states_test.go @@ -14,6 +14,7 @@ import ( "github.com/filecoin-project/specs-actors/actors/builtin/paych" "github.com/ipfs/go-cid" mh "github.com/multiformats/go-multihash" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/filecoin-project/go-fil-markets/retrievalmarket" @@ -106,18 +107,110 @@ func TestSetupPaymentChannel(t *testing.T) { require.Equal(t, dealState.Status, retrievalmarket.DealStatusFailed) }) - // TODO: find the right place for this test - //t.Run("when allocate lane fails", func(t *testing.T) { - // dealState := makeDealState(retrievalmarket.ClientEventPaymentChannelAddingFunds) - // envParams := testnodes.TestRetrievalClientNodeParams{ - // PayCh: expectedPayCh, - // Lane: expectedLane, - // LaneError: errors.New("Something went wrong"), - // } - // runSetupPaymentChannel(t, envParams, dealState) - // require.NotEmpty(t, dealState.Message) - // require.Equal(t, dealState.Status, retrievalmarket.DealStatusFailed) - //}) +} + +func TestWaitForPaymentChannelCreate(t *testing.T) { + ctx := context.Background() + ds := testnet.NewTestRetrievalDealStream(testnet.TestDealStreamParams{}) + expectedPayCh := address.TestAddress2 + expectedLane := uint64(10) + eventMachine, err := fsm.NewEventProcessor(retrievalmarket.ClientDealState{}, "Status", clientstates.ClientEvents) + require.NoError(t, err) + runWaitForPaychCreate := func(t *testing.T, + params testnodes.TestRetrievalClientNodeParams, + dealState *retrievalmarket.ClientDealState) { + node := testnodes.NewTestRetrievalClientNode(params) + environment := &fakeEnvironment{node, ds, 0, nil} + fsmCtx := fsmtest.NewTestContext(ctx, eventMachine) + err := clientstates.WaitForPaymentChannelCreate(fsmCtx, environment, *dealState) + require.NoError(t, err) + fsmCtx.ReplayEvents(t, dealState) + } + msgCID := testnet.GenerateCids(1)[0] + + t.Run("it works", func(t *testing.T) { + dealState := makeDealState(retrievalmarket.DealStatusPaymentChannelCreating) + dealState.WaitMsgCID = &msgCID + params := testnodes.TestRetrievalClientNodeParams{ + PayCh: expectedPayCh, + CreatePaychCID: msgCID, + Lane: expectedLane, + } + runWaitForPaychCreate(t, params, dealState) + require.Empty(t, dealState.Message) + assert.Equal(t, dealState.Status, retrievalmarket.DealStatusPaymentChannelReady) + assert.Equal(t, expectedLane, dealState.PaymentInfo.Lane) + }) + t.Run("if Wait fails", func(t *testing.T) { + dealState := makeDealState(retrievalmarket.DealStatusPaymentChannelCreating) + dealState.WaitMsgCID = &msgCID + params := testnodes.TestRetrievalClientNodeParams{ + PayCh: expectedPayCh, + CreatePaychCID: msgCID, + WaitForChCreateErr: errors.New("boom"), + } + runWaitForPaychCreate(t, params, dealState) + assert.Contains(t, dealState.Message, "boom") + assert.Equal(t, dealState.Status, retrievalmarket.DealStatusFailed) + }) + + t.Run("if AllocateLane fails", func(t *testing.T) { + dealState := makeDealState(retrievalmarket.DealStatusPaymentChannelCreating) + dealState.WaitMsgCID = &msgCID + params := testnodes.TestRetrievalClientNodeParams{ + PayCh: expectedPayCh, + CreatePaychCID: msgCID, + LaneError: errors.New("boom"), + } + runWaitForPaychCreate(t, params, dealState) + assert.Contains(t, dealState.Message, "boom") + assert.Equal(t, dealState.Status, retrievalmarket.DealStatusFailed) + }) +} + +func TestWaitForPaymentChannelAddFunds(t *testing.T) { + ctx := context.Background() + ds := testnet.NewTestRetrievalDealStream(testnet.TestDealStreamParams{}) + expectedPayCh := address.TestAddress2 + eventMachine, err := fsm.NewEventProcessor(retrievalmarket.ClientDealState{}, "Status", clientstates.ClientEvents) + require.NoError(t, err) + runWaitForPaychAddFunds := func(t *testing.T, + params testnodes.TestRetrievalClientNodeParams, + dealState *retrievalmarket.ClientDealState) { + node := testnodes.NewTestRetrievalClientNode(params) + environment := &fakeEnvironment{node, ds, 0, nil} + fsmCtx := fsmtest.NewTestContext(ctx, eventMachine) + err := clientstates.WaitForPaymentChannelAddFunds(fsmCtx, environment, *dealState) + require.NoError(t, err) + fsmCtx.ReplayEvents(t, dealState) + } + msgCID := testnet.GenerateCids(1)[0] + + t.Run("it works", func(t *testing.T) { + dealState := makeDealState(retrievalmarket.DealStatusPaymentChannelAddingFunds) + dealState.WaitMsgCID = &msgCID + params := testnodes.TestRetrievalClientNodeParams{ + AddFundsOnly: true, + PayCh: expectedPayCh, + AddFundsCID: msgCID, + } + runWaitForPaychAddFunds(t, params, dealState) + require.Empty(t, dealState.Message) + assert.Equal(t, dealState.Status, retrievalmarket.DealStatusPaymentChannelReady) + }) + t.Run("if Wait fails", func(t *testing.T) { + dealState := makeDealState(retrievalmarket.DealStatusPaymentChannelAddingFunds) + dealState.WaitMsgCID = &msgCID + params := testnodes.TestRetrievalClientNodeParams{ + AddFundsOnly: true, + PayCh: expectedPayCh, + AddFundsCID: msgCID, + WaitForAddFundsErr: errors.New("boom"), + } + runWaitForPaychAddFunds(t, params, dealState) + assert.Contains(t, dealState.Message, "boom") + assert.Equal(t, dealState.Status, retrievalmarket.DealStatusFailed) + }) } func TestProposeDeal(t *testing.T) { diff --git a/retrievalmarket/impl/testnodes/test_retrieval_client_node.go b/retrievalmarket/impl/testnodes/test_retrieval_client_node.go index 069aefed..06f5032f 100644 --- a/retrievalmarket/impl/testnodes/test_retrieval_client_node.go +++ b/retrievalmarket/impl/testnodes/test_retrieval_client_node.go @@ -2,6 +2,7 @@ package testnodes import ( "context" + "fmt" "github.com/filecoin-project/go-address" "github.com/filecoin-project/specs-actors/actors/abi" @@ -15,14 +16,14 @@ import ( // TestRetrievalClientNode is a node adapter for a retrieval client whose responses // are stubbed type TestRetrievalClientNode struct { - addFundsOnly bool // set this to true if the payment channel is expected to have been created already - payCh address.Address - payChErr error - createPaychMsgCID, addFundsMsgCID cid.Cid - lane uint64 - laneError error - voucher *paych.SignedVoucher - voucherError error + addFundsOnly bool // set this to true to test adding funds to an existing payment channel + payCh address.Address + payChErr error + createPaychMsgCID, addFundsMsgCID cid.Cid + lane uint64 + laneError error + voucher *paych.SignedVoucher + voucherError, waitCreateErr, waitAddErr error allocateLaneRecorder func(address.Address) createPaymentVoucherRecorder func(voucher *paych.SignedVoucher) @@ -31,27 +32,30 @@ type TestRetrievalClientNode struct { // TestRetrievalClientNodeParams are parameters for initializing a TestRetrievalClientNode type TestRetrievalClientNodeParams struct { - PayCh address.Address - PayChErr error - CreatePaychCID, AddFundsCID cid.Cid - Lane uint64 - LaneError error - Voucher *paych.SignedVoucher - VoucherError error - AllocateLaneRecorder func(address.Address) - PaymentVoucherRecorder func(voucher *paych.SignedVoucher) - PaymentChannelRecorder func(address.Address, address.Address, abi.TokenAmount) - AddFundsOnly bool + PayCh address.Address + PayChErr error + CreatePaychCID, AddFundsCID cid.Cid + Lane uint64 + LaneError error + Voucher *paych.SignedVoucher + VoucherError error + AllocateLaneRecorder func(address.Address) + PaymentVoucherRecorder func(voucher *paych.SignedVoucher) + PaymentChannelRecorder func(address.Address, address.Address, abi.TokenAmount) + AddFundsOnly bool + WaitForAddFundsErr, WaitForChCreateErr error } var _ retrievalmarket.RetrievalClientNode = &TestRetrievalClientNode{} -// NewTestRetrievalClientNode instantiates a new TestRetrievalClientNode based ont he given params +// NewTestRetrievalClientNode initializes a new TestRetrievalClientNode based on the given params func NewTestRetrievalClientNode(params TestRetrievalClientNodeParams) *TestRetrievalClientNode { return &TestRetrievalClientNode{ addFundsOnly: params.AddFundsOnly, payCh: params.PayCh, payChErr: params.PayChErr, + waitCreateErr: params.WaitForChCreateErr, + waitAddErr: params.WaitForAddFundsErr, lane: params.Lane, laneError: params.LaneError, voucher: params.Voucher, @@ -96,9 +100,15 @@ func (trcn *TestRetrievalClientNode) GetChainHead(ctx context.Context) (shared.T return shared.TipSetToken{}, 0, nil } func (trcn *TestRetrievalClientNode) WaitForPaymentChannelAddFunds(messageCID cid.Cid) error { - return nil + if messageCID != trcn.addFundsMsgCID { + return fmt.Errorf("expected messageCID: %s does not match actual: %s", trcn.addFundsMsgCID, messageCID) + } + return trcn.waitAddErr } func (trcn *TestRetrievalClientNode) WaitForPaymentChannelCreation(messageCID cid.Cid) (address.Address, error) { - return trcn.payCh, nil + if messageCID != trcn.createPaychMsgCID { + return address.Undef, fmt.Errorf("expected messageCID: %s does not match actual: %s", trcn.createPaychMsgCID, messageCID) + } + return trcn.payCh, trcn.waitCreateErr } diff --git a/retrievalmarket/types.go b/retrievalmarket/types.go index e8352c12..ddf1c93e 100644 --- a/retrievalmarket/types.go +++ b/retrievalmarket/types.go @@ -75,19 +75,17 @@ const ( // successful and we are waiting for it to appear on chain ClientEventPaymentChannelCreateInitiated - // ClientEventPaymentChannelCreated means a payment channel has been created - ClientEventPaymentChannelCreated - // ClientEventPaymentChannelReady means the newly created payment channel is ready for the // deal to begin ClientEventPaymentChannelReady // ClientEventPaymentChannelAddingFunds means the request to add funds to a payment channel was - // successful and we are waiting for the funds to be sent + // sent and we are waiting for the funds to be added to the payment channel ClientEventPaymentChannelAddingFunds - // ClientEventPaymentChannelFundsAdded means funds have been added to the payment channel - ClientEventPaymentChannelFundsAdded + // ClientEventPaymentChannelAddingFunds means that adding funds to the payment channel + // failed + ClientEventPaymentChannelAddFundsErrored // ClientEventWriteDealProposalErrored means a network error writing a deal proposal ClientEventWriteDealProposalErrored From 3b4659dfb8a4da98f8625f289244b4bae803d562 Mon Sep 17 00:00:00 2001 From: shannonwells Date: Mon, 13 Apr 2020 16:49:40 -0700 Subject: [PATCH 4/9] fix introduced fsm bug --- retrievalmarket/impl/clientstates/client_fsm.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/retrievalmarket/impl/clientstates/client_fsm.go b/retrievalmarket/impl/clientstates/client_fsm.go index fa21534b..9e928dbc 100644 --- a/retrievalmarket/impl/clientstates/client_fsm.go +++ b/retrievalmarket/impl/clientstates/client_fsm.go @@ -29,9 +29,9 @@ var ClientEvents = fsm.Events{ fsm.Event(rm.ClientEventOpen). From(rm.DealStatusNew).ToNoChange(), fsm.Event(rm.ClientEventPaymentChannelErrored). - From(rm.DealStatusPaymentChannelCreating).To(rm.DealStatusFailed). + FromMany(rm.DealStatusAccepted, rm.DealStatusPaymentChannelCreating).To(rm.DealStatusFailed). Action(func(deal *rm.ClientDealState, err error) error { - deal.Message = xerrors.Errorf("getting payment channel: %w", err).Error() + deal.Message = xerrors.Errorf("creating/getting payment channel: %w", err).Error() return nil }), fsm.Event(rm.ClientEventPaymentChannelCreateInitiated). From 11d9f47a9e77ae4d7984169e01c67165fa07938e Mon Sep 17 00:00:00 2001 From: shannonwells Date: Tue, 14 Apr 2020 09:58:03 -0700 Subject: [PATCH 5/9] cleanup for PR --- go.sum | 1 + .../impl/clientstates/client_fsm.go | 2 +- retrievalmarket/types.go | 40 ++++++++++--------- 3 files changed, 23 insertions(+), 20 deletions(-) diff --git a/go.sum b/go.sum index 05586549..b7ccd428 100644 --- a/go.sum +++ b/go.sum @@ -134,6 +134,7 @@ github.com/filecoin-project/sector-storage v0.0.0-20200411000242-61616264b16d h1 github.com/filecoin-project/sector-storage v0.0.0-20200411000242-61616264b16d/go.mod h1:/yueJueMh0Yc+0G1adS0lhnedcSnjY86EjKsA20+DVY= github.com/filecoin-project/specs-actors v0.0.0-20200210130641-2d1fbd8672cf h1:fbxBG12yrxilPFV1EG2lYqpUyAlRZWkvtqjk2svSeXY= github.com/filecoin-project/specs-actors v0.0.0-20200210130641-2d1fbd8672cf/go.mod h1:xtDZUB6pe4Pksa/bAJbJ693OilaC5Wbot9jMhLm3cZA= +github.com/filecoin-project/specs-actors v0.0.0-20200226200336-94c9b92b2775/go.mod h1:0HAWYrvajFHDgRaKbF0rl+IybVLZL5z4gQ8koCMPhoU= github.com/filecoin-project/specs-actors v0.0.0-20200409043918-e569f4a2f504 h1:mwuAaqxKThl70+7FkGdFKVLdwaQZQ8XmscKdhSBBtnc= github.com/filecoin-project/specs-actors v0.0.0-20200409043918-e569f4a2f504/go.mod h1:mdJraXq5vMy0+/FqVQIrnNlpQ/Em6zeu06G/ltQ0/lA= github.com/filecoin-project/specs-storage v0.0.0-20200410185809-9fbaaa08f275 h1:6OTcpsTQBQM0f/A67oEi4E4YtYd6fzkMqbU8cPIWMMs= diff --git a/retrievalmarket/impl/clientstates/client_fsm.go b/retrievalmarket/impl/clientstates/client_fsm.go index 9e928dbc..9c8764a5 100644 --- a/retrievalmarket/impl/clientstates/client_fsm.go +++ b/retrievalmarket/impl/clientstates/client_fsm.go @@ -31,7 +31,7 @@ var ClientEvents = fsm.Events{ fsm.Event(rm.ClientEventPaymentChannelErrored). FromMany(rm.DealStatusAccepted, rm.DealStatusPaymentChannelCreating).To(rm.DealStatusFailed). Action(func(deal *rm.ClientDealState, err error) error { - deal.Message = xerrors.Errorf("creating/getting payment channel: %w", err).Error() + deal.Message = xerrors.Errorf("get or create payment channel: %w", err).Error() return nil }), fsm.Event(rm.ClientEventPaymentChannelCreateInitiated). diff --git a/retrievalmarket/types.go b/retrievalmarket/types.go index ddf1c93e..34530a9c 100644 --- a/retrievalmarket/types.go +++ b/retrievalmarket/types.go @@ -71,16 +71,16 @@ const ( // ClientEventAllocateLaneErrored means there was a failure creating a lane in a payment channel ClientEventAllocateLaneErrored - // ClientEventPaymentChannelCreateInitiated means the request to create a payment channel was - // successful and we are waiting for it to appear on chain + // ClientEventPaymentChannelCreateInitiated means we are waiting for a message to + // create a payment channel to appear on chain ClientEventPaymentChannelCreateInitiated // ClientEventPaymentChannelReady means the newly created payment channel is ready for the - // deal to begin + // deal to resume ClientEventPaymentChannelReady - // ClientEventPaymentChannelAddingFunds means the request to add funds to a payment channel was - // sent and we are waiting for the funds to be added to the payment channel + // ClientEventPaymentChannelAddingFunds mean we are waiting for funds to be + // added to a payment channel ClientEventPaymentChannelAddingFunds // ClientEventPaymentChannelAddingFunds means that adding funds to the payment channel @@ -519,20 +519,22 @@ const ( // DealStatuses maps deal status to a human readable representation var DealStatuses = map[DealStatus]string{ - DealStatusNew: "DealStatusNew", - DealStatusPaymentChannelCreating: "DealStatusPaymentChannelCreating", - DealStatusAccepted: "DealStatusAccepted", - DealStatusFailed: "DealStatusFailed", - DealStatusRejected: "DealStatusRejected", - DealStatusFundsNeeded: "DealStatusFundsNeeded", - DealStatusOngoing: "DealStatusOngoing", - DealStatusFundsNeededLastPayment: "DealStatusFundsNeededLastPayment", - DealStatusCompleted: "DealStatusCompleted", - DealStatusDealNotFound: "DealStatusDealNotFound", - DealStatusVerified: "DealStatusVerified", - DealStatusErrored: "DealStatusErrored", - DealStatusBlocksComplete: "DealStatusBlocksComplete", - DealStatusFinalizing: "DealStatusFinalizing", + DealStatusNew: "DealStatusNew", + DealStatusPaymentChannelCreating: "DealStatusPaymentChannelCreating", + DealStatusPaymentChannelAddingFunds: "DealStatusPaymentChannelAddingFunds", + DealStatusPaymentChannelReady: "DealStatusPaymentChannelReady", + DealStatusAccepted: "DealStatusAccepted", + DealStatusFailed: "DealStatusFailed", + DealStatusRejected: "DealStatusRejected", + DealStatusFundsNeeded: "DealStatusFundsNeeded", + DealStatusOngoing: "DealStatusOngoing", + DealStatusFundsNeededLastPayment: "DealStatusFundsNeededLastPayment", + DealStatusCompleted: "DealStatusCompleted", + DealStatusDealNotFound: "DealStatusDealNotFound", + DealStatusVerified: "DealStatusVerified", + DealStatusErrored: "DealStatusErrored", + DealStatusBlocksComplete: "DealStatusBlocksComplete", + DealStatusFinalizing: "DealStatusFinalizing", } // IsTerminalError returns true if this status indicates processing of this deal From 969682bbe7de0238352344243e6abe3d3b4c5ff3 Mon Sep 17 00:00:00 2001 From: shannonwells Date: Tue, 14 Apr 2020 10:31:09 -0700 Subject: [PATCH 6/9] mod tidy --- go.sum | 1 - 1 file changed, 1 deletion(-) diff --git a/go.sum b/go.sum index b7ccd428..05586549 100644 --- a/go.sum +++ b/go.sum @@ -134,7 +134,6 @@ github.com/filecoin-project/sector-storage v0.0.0-20200411000242-61616264b16d h1 github.com/filecoin-project/sector-storage v0.0.0-20200411000242-61616264b16d/go.mod h1:/yueJueMh0Yc+0G1adS0lhnedcSnjY86EjKsA20+DVY= github.com/filecoin-project/specs-actors v0.0.0-20200210130641-2d1fbd8672cf h1:fbxBG12yrxilPFV1EG2lYqpUyAlRZWkvtqjk2svSeXY= github.com/filecoin-project/specs-actors v0.0.0-20200210130641-2d1fbd8672cf/go.mod h1:xtDZUB6pe4Pksa/bAJbJ693OilaC5Wbot9jMhLm3cZA= -github.com/filecoin-project/specs-actors v0.0.0-20200226200336-94c9b92b2775/go.mod h1:0HAWYrvajFHDgRaKbF0rl+IybVLZL5z4gQ8koCMPhoU= github.com/filecoin-project/specs-actors v0.0.0-20200409043918-e569f4a2f504 h1:mwuAaqxKThl70+7FkGdFKVLdwaQZQ8XmscKdhSBBtnc= github.com/filecoin-project/specs-actors v0.0.0-20200409043918-e569f4a2f504/go.mod h1:mdJraXq5vMy0+/FqVQIrnNlpQ/Em6zeu06G/ltQ0/lA= github.com/filecoin-project/specs-storage v0.0.0-20200410185809-9fbaaa08f275 h1:6OTcpsTQBQM0f/A67oEi4E4YtYd6fzkMqbU8cPIWMMs= From c974bb5fa2efc4d734a09d8c3ff2dc1865f363e1 Mon Sep 17 00:00:00 2001 From: shannonwells Date: Tue, 14 Apr 2020 12:38:43 -0700 Subject: [PATCH 7/9] correct AddFunds behavior, add AddFunds integration test --- .../impl/clientstates/client_fsm.go | 9 +- .../impl/clientstates/client_states.go | 10 +- .../impl/clientstates/client_states_test.go | 2 +- retrievalmarket/impl/integration_test.go | 111 ++++++++++-------- .../testnodes/test_retrieval_client_node.go | 4 +- 5 files changed, 78 insertions(+), 58 deletions(-) diff --git a/retrievalmarket/impl/clientstates/client_fsm.go b/retrievalmarket/impl/clientstates/client_fsm.go index 9c8764a5..36fa410b 100644 --- a/retrievalmarket/impl/clientstates/client_fsm.go +++ b/retrievalmarket/impl/clientstates/client_fsm.go @@ -41,7 +41,14 @@ var ClientEvents = fsm.Events{ return nil }), fsm.Event(rm.ClientEventPaymentChannelAddingFunds). - FromMany(rm.DealStatusOngoing, rm.DealStatusPaymentChannelReady).To(rm.DealStatusPaymentChannelAddingFunds), + FromMany(rm.DealStatusAccepted).To(rm.DealStatusPaymentChannelAddingFunds). + Action(func(deal *rm.ClientDealState, msgCID cid.Cid, payCh address.Address) error { + deal.WaitMsgCID = &msgCID + deal.PaymentInfo = &rm.PaymentInfo{ + PayCh: payCh, + } + return nil + }), fsm.Event(rm.ClientEventPaymentChannelReady). FromMany(rm.DealStatusPaymentChannelCreating, rm.DealStatusPaymentChannelAddingFunds). To(rm.DealStatusPaymentChannelReady). diff --git a/retrievalmarket/impl/clientstates/client_states.go b/retrievalmarket/impl/clientstates/client_states.go index 1e627b7e..e05fbe56 100644 --- a/retrievalmarket/impl/clientstates/client_states.go +++ b/retrievalmarket/impl/clientstates/client_states.go @@ -35,7 +35,7 @@ func SetupPaymentChannelStart(ctx fsm.Context, environment ClientDealEnvironment return ctx.Trigger(rm.ClientEventPaymentChannelCreateInitiated, msgCID) } - return ctx.Trigger(rm.ClientEventPaymentChannelAddingFunds) + return ctx.Trigger(rm.ClientEventPaymentChannelAddingFunds, msgCID, paych) } // WaitForPaymentChannelCreate waits for payment channel creation to be posted on chain, @@ -60,7 +60,11 @@ func WaitForPaymentChannelAddFunds(ctx fsm.Context, environment ClientDealEnviro if err != nil { return ctx.Trigger(rm.ClientEventPaymentChannelAddFundsErrored, err) } - return ctx.Trigger(rm.ClientEventPaymentChannelReady, deal.PaymentInfo.PayCh, deal.PaymentInfo.Lane) + lane, err := environment.Node().AllocateLane(deal.PaymentInfo.PayCh) + if err != nil { + return ctx.Trigger(rm.ClientEventAllocateLaneErrored, err) + } + return ctx.Trigger(rm.ClientEventPaymentChannelReady, deal.PaymentInfo.PayCh, lane) } // ProposeDeal sends the proposal to the other party @@ -174,7 +178,7 @@ func ProcessNextResponse(ctx fsm.Context, environment ClientDealEnvironment, dea return ctx.Trigger(rm.ClientEventEarlyTermination) case rm.DealStatusFundsNeeded: return ctx.Trigger(rm.ClientEventPaymentRequested, totalProcessed, response.PaymentOwed) - case rm.DealStatusOngoing, rm.DealStatusPaymentChannelReady: + case rm.DealStatusOngoing: return ctx.Trigger(rm.ClientEventBlocksReceived, totalProcessed) default: return ctx.Trigger(rm.ClientEventUnknownResponseReceived) diff --git a/retrievalmarket/impl/clientstates/client_states_test.go b/retrievalmarket/impl/clientstates/client_states_test.go index 53182e01..830d3377 100644 --- a/retrievalmarket/impl/clientstates/client_states_test.go +++ b/retrievalmarket/impl/clientstates/client_states_test.go @@ -89,7 +89,7 @@ func TestSetupPaymentChannel(t *testing.T) { PayCh: expectedPayCh, CreatePaychCID: testnet.GenerateCids(1)[0], } - dealState := makeDealState(retrievalmarket.DealStatusPaymentChannelReady) + dealState := makeDealState(retrievalmarket.DealStatusAccepted) runSetupPaymentChannel(t, envParams, dealState) require.Empty(t, dealState.Message) require.Equal(t, dealState.Status, retrievalmarket.DealStatusPaymentChannelAddingFunds) diff --git a/retrievalmarket/impl/integration_test.go b/retrievalmarket/impl/integration_test.go index d62adfed..62b7f060 100644 --- a/retrievalmarket/impl/integration_test.go +++ b/retrievalmarket/impl/integration_test.go @@ -12,10 +12,7 @@ import ( "github.com/filecoin-project/specs-actors/actors/builtin/paych" "github.com/ipfs/go-cid" "github.com/ipld/go-ipld-prime" - ipldfree "github.com/ipld/go-ipld-prime/impl/free" cidlink "github.com/ipld/go-ipld-prime/linking/cid" - "github.com/ipld/go-ipld-prime/traversal/selector" - "github.com/ipld/go-ipld-prime/traversal/selector/builder" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -133,59 +130,64 @@ func requireSetupTestClientAndProvider(bgCtx context.Context, t *testing.T, payC func TestClientCanMakeDealWithProvider(t *testing.T) { // -------- SET UP PROVIDER - ssb := builder.NewSelectorSpecBuilder(ipldfree.NodeBuilder()) + //ssb := builder.NewSelectorSpecBuilder(ipldfree.NodeBuilder()) - allSelector := ssb.ExploreRecursive(selector.RecursionLimitNone(), - ssb.ExploreAll(ssb.ExploreRecursiveEdge())).Node() - - partialSelector := ssb.ExploreFields(func(specBuilder builder.ExploreFieldsSpecBuilder) { - specBuilder.Insert("Links", ssb.ExploreIndex(0, ssb.ExploreFields(func(specBuilder builder.ExploreFieldsSpecBuilder) { - specBuilder.Insert("Hash", ssb.Matcher()) - }))) - }).Node() + //allSelector := ssb.ExploreRecursive(selector.RecursionLimitNone(), + // ssb.ExploreAll(ssb.ExploreRecursiveEdge())).Node() + // + //partialSelector := ssb.ExploreFields(func(specBuilder builder.ExploreFieldsSpecBuilder) { + // specBuilder.Insert("Links", ssb.ExploreIndex(0, ssb.ExploreFields(func(specBuilder builder.ExploreFieldsSpecBuilder) { + // specBuilder.Insert("Hash", ssb.Matcher()) + // }))) + //}).Node() testCases := []struct { - name string - filename string - filesize uint64 - voucherAmts []abi.TokenAmount - selector ipld.Node - paramsV1, unsealing bool + name string + filename string + filesize uint64 + voucherAmts []abi.TokenAmount + selector ipld.Node + paramsV1, unsealing, addFunds bool }{ - {name: "1 block file retrieval succeeds", - filename: "lorem_under_1_block.txt", - filesize: 410, - voucherAmts: []abi.TokenAmount{abi.NewTokenAmount(410000)}, - unsealing: false}, - {name: "1 block file retrieval succeeds with unsealing", + //{name: "1 block file retrieval succeeds", + // filename: "lorem_under_1_block.txt", + // filesize: 410, + // voucherAmts: []abi.TokenAmount{abi.NewTokenAmount(410000)}, + // unsealing: false}, + {name: "1 block file retrieval succeeds with existing payment channel", filename: "lorem_under_1_block.txt", filesize: 410, voucherAmts: []abi.TokenAmount{abi.NewTokenAmount(410000)}, - unsealing: true}, - {name: "multi-block file retrieval succeeds", - filename: "lorem.txt", - filesize: 19000, - voucherAmts: []abi.TokenAmount{abi.NewTokenAmount(10136000), abi.NewTokenAmount(9784000)}, - unsealing: false}, - {name: "multi-block file retrieval succeeds with unsealing", - filename: "lorem.txt", - filesize: 19000, - voucherAmts: []abi.TokenAmount{abi.NewTokenAmount(10136000), abi.NewTokenAmount(9784000)}, - unsealing: true}, - {name: "multi-block file retrieval succeeds with V1 params and allSelector", - filename: "lorem.txt", - filesize: 19000, - voucherAmts: []abi.TokenAmount{abi.NewTokenAmount(10136000), abi.NewTokenAmount(9784000)}, - paramsV1: true, - selector: allSelector, - unsealing: false}, - {name: "partial file retrieval succeeds with V1 params and selector recursion depth 1", - filename: "lorem.txt", - filesize: 1024, - voucherAmts: []abi.TokenAmount{abi.NewTokenAmount(1944000)}, - paramsV1: true, - selector: partialSelector, - unsealing: false}, + unsealing: false, addFunds: true}, + //{name: "1 block file retrieval succeeds with unsealing", + // filename: "lorem_under_1_block.txt", + // filesize: 410, + // voucherAmts: []abi.TokenAmount{abi.NewTokenAmount(410000)}, + // unsealing: true}, + //{name: "multi-block file retrieval succeeds", + // filename: "lorem.txt", + // filesize: 19000, + // voucherAmts: []abi.TokenAmount{abi.NewTokenAmount(10136000), abi.NewTokenAmount(9784000)}, + // unsealing: false}, + //{name: "multi-block file retrieval succeeds with unsealing", + // filename: "lorem.txt", + // filesize: 19000, + // voucherAmts: []abi.TokenAmount{abi.NewTokenAmount(10136000), abi.NewTokenAmount(9784000)}, + // unsealing: true}, + //{name: "multi-block file retrieval succeeds with V1 params and allSelector", + // filename: "lorem.txt", + // filesize: 19000, + // voucherAmts: []abi.TokenAmount{abi.NewTokenAmount(10136000), abi.NewTokenAmount(9784000)}, + // paramsV1: true, + // selector: allSelector, + // unsealing: false}, + //{name: "partial file retrieval succeeds with V1 params and selector recursion depth 1", + // filename: "lorem.txt", + // filesize: 1024, + // voucherAmts: []abi.TokenAmount{abi.NewTokenAmount(1944000)}, + // paramsV1: true, + // selector: partialSelector, + // unsealing: false}, } for i, testCase := range testCases { @@ -272,7 +274,7 @@ func TestClientCanMakeDealWithProvider(t *testing.T) { // ------- SET UP CLIENT nw1 := rmnet.NewFromLibp2pHost(testData.Host1) - createdChan, newLaneAddr, createdVoucher, client, err := setupClient(clientPaymentChannel, expectedVoucher, nw1, testData) + createdChan, newLaneAddr, createdVoucher, client, err := setupClient(clientPaymentChannel, expectedVoucher, nw1, testData, testCase.addFunds) require.NoError(t, err) clientDealStateChan := make(chan retrievalmarket.ClientDealState) @@ -376,10 +378,14 @@ func setupClient( clientPaymentChannel address.Address, expectedVoucher *paych.SignedVoucher, nw1 rmnet.RetrievalMarketNetwork, - testData *tut.Libp2pTestData) (*pmtChan, + testData *tut.Libp2pTestData, + addFunds bool, +) ( + *pmtChan, *address.Address, *paych.SignedVoucher, - retrievalmarket.RetrievalClient, error) { + retrievalmarket.RetrievalClient, + error) { var createdChan pmtChan paymentChannelRecorder := func(client, miner address.Address, amt abi.TokenAmount) { createdChan = pmtChan{client, miner, amt} @@ -396,6 +402,7 @@ func setupClient( } cids := tut.GenerateCids(2) clientNode := testnodes.NewTestRetrievalClientNode(testnodes.TestRetrievalClientNodeParams{ + AddFundsOnly: addFunds, PayCh: clientPaymentChannel, Lane: expectedVoucher.Lane, Voucher: expectedVoucher, diff --git a/retrievalmarket/impl/testnodes/test_retrieval_client_node.go b/retrievalmarket/impl/testnodes/test_retrieval_client_node.go index 06f5032f..34f02ace 100644 --- a/retrievalmarket/impl/testnodes/test_retrieval_client_node.go +++ b/retrievalmarket/impl/testnodes/test_retrieval_client_node.go @@ -74,10 +74,12 @@ func (trcn *TestRetrievalClientNode) GetOrCreatePaymentChannel(ctx context.Conte trcn.getCreatePaymentChannelRecorder(clientAddress, minerAddress, clientFundsAvailable) } var payCh address.Address + msgCID := trcn.createPaychMsgCID if trcn.addFundsOnly { payCh = trcn.payCh + msgCID = trcn.addFundsMsgCID } - return payCh, trcn.createPaychMsgCID, trcn.payChErr + return payCh, msgCID, trcn.payChErr } // AllocateLane creates a mock lane on a payment channel From 61fb8407737d91e9877c64082ba095475903b974 Mon Sep 17 00:00:00 2001 From: shannonwells Date: Tue, 14 Apr 2020 13:04:37 -0700 Subject: [PATCH 8/9] more PR comment addressing --- .../impl/clientstates/client_states_test.go | 43 +++++---- retrievalmarket/impl/integration_test.go | 89 ++++++++++--------- 2 files changed, 70 insertions(+), 62 deletions(-) diff --git a/retrievalmarket/impl/clientstates/client_states_test.go b/retrievalmarket/impl/clientstates/client_states_test.go index 830d3377..213ed8f2 100644 --- a/retrievalmarket/impl/clientstates/client_states_test.go +++ b/retrievalmarket/impl/clientstates/client_states_test.go @@ -58,7 +58,6 @@ func TestSetupPaymentChannel(t *testing.T) { ctx := context.Background() ds := testnet.NewTestRetrievalDealStream(testnet.TestDealStreamParams{}) expectedPayCh := address.TestAddress2 - expectedLane := uint64(10) eventMachine, err := fsm.NewEventProcessor(retrievalmarket.ClientDealState{}, "Status", clientstates.ClientEvents) require.NoError(t, err) runSetupPaymentChannel := func(t *testing.T, @@ -79,8 +78,8 @@ func TestSetupPaymentChannel(t *testing.T) { } dealState := makeDealState(retrievalmarket.DealStatusAccepted) runSetupPaymentChannel(t, envParams, dealState) - require.Empty(t, dealState.Message) - require.Equal(t, dealState.Status, retrievalmarket.DealStatusPaymentChannelCreating) + assert.Empty(t, dealState.Message) + assert.Equal(t, dealState.Status, retrievalmarket.DealStatusPaymentChannelCreating) }) t.Run("payment channel needs funds added", func(t *testing.T) { @@ -92,7 +91,8 @@ func TestSetupPaymentChannel(t *testing.T) { dealState := makeDealState(retrievalmarket.DealStatusAccepted) runSetupPaymentChannel(t, envParams, dealState) require.Empty(t, dealState.Message) - require.Equal(t, dealState.Status, retrievalmarket.DealStatusPaymentChannelAddingFunds) + require.Equal(t, retrievalmarket.DealStatusPaymentChannelAddingFunds, dealState.Status) + require.Equal(t, expectedPayCh, dealState.PaymentInfo.PayCh) }) t.Run("when create payment channel fails", func(t *testing.T) { @@ -100,7 +100,6 @@ func TestSetupPaymentChannel(t *testing.T) { envParams := testnodes.TestRetrievalClientNodeParams{ PayCh: address.Undef, PayChErr: errors.New("Something went wrong"), - Lane: expectedLane, } runSetupPaymentChannel(t, envParams, dealState) require.NotEmpty(t, dealState.Message) @@ -138,8 +137,9 @@ func TestWaitForPaymentChannelCreate(t *testing.T) { } runWaitForPaychCreate(t, params, dealState) require.Empty(t, dealState.Message) - assert.Equal(t, dealState.Status, retrievalmarket.DealStatusPaymentChannelReady) - assert.Equal(t, expectedLane, dealState.PaymentInfo.Lane) + require.Equal(t, dealState.Status, retrievalmarket.DealStatusPaymentChannelReady) + require.Equal(t, expectedLane, dealState.PaymentInfo.Lane) + require.Equal(t, expectedPayCh, dealState.PaymentInfo.PayCh) }) t.Run("if Wait fails", func(t *testing.T) { dealState := makeDealState(retrievalmarket.DealStatusPaymentChannelCreating) @@ -150,8 +150,8 @@ func TestWaitForPaymentChannelCreate(t *testing.T) { WaitForChCreateErr: errors.New("boom"), } runWaitForPaychCreate(t, params, dealState) - assert.Contains(t, dealState.Message, "boom") - assert.Equal(t, dealState.Status, retrievalmarket.DealStatusFailed) + require.Contains(t, dealState.Message, "boom") + require.Equal(t, dealState.Status, retrievalmarket.DealStatusFailed) }) t.Run("if AllocateLane fails", func(t *testing.T) { @@ -163,8 +163,8 @@ func TestWaitForPaymentChannelCreate(t *testing.T) { LaneError: errors.New("boom"), } runWaitForPaychCreate(t, params, dealState) - assert.Contains(t, dealState.Message, "boom") - assert.Equal(t, dealState.Status, retrievalmarket.DealStatusFailed) + require.Contains(t, dealState.Message, "boom") + require.Equal(t, dealState.Status, retrievalmarket.DealStatusFailed) }) } @@ -172,6 +172,7 @@ func TestWaitForPaymentChannelAddFunds(t *testing.T) { ctx := context.Background() ds := testnet.NewTestRetrievalDealStream(testnet.TestDealStreamParams{}) expectedPayCh := address.TestAddress2 + expectedLane := uint64(99) eventMachine, err := fsm.NewEventProcessor(retrievalmarket.ClientDealState{}, "Status", clientstates.ClientEvents) require.NoError(t, err) runWaitForPaychAddFunds := func(t *testing.T, @@ -188,15 +189,20 @@ func TestWaitForPaymentChannelAddFunds(t *testing.T) { t.Run("it works", func(t *testing.T) { dealState := makeDealState(retrievalmarket.DealStatusPaymentChannelAddingFunds) + dealState.PaymentInfo.PayCh = expectedPayCh dealState.WaitMsgCID = &msgCID + params := testnodes.TestRetrievalClientNodeParams{ AddFundsOnly: true, PayCh: expectedPayCh, AddFundsCID: msgCID, + Lane: expectedLane, } runWaitForPaychAddFunds(t, params, dealState) require.Empty(t, dealState.Message) - assert.Equal(t, dealState.Status, retrievalmarket.DealStatusPaymentChannelReady) + assert.Equal(t, retrievalmarket.DealStatusPaymentChannelReady, dealState.Status) + assert.Equal(t, expectedLane, dealState.PaymentInfo.Lane) + assert.Equal(t, expectedPayCh, dealState.PaymentInfo.PayCh) }) t.Run("if Wait fails", func(t *testing.T) { dealState := makeDealState(retrievalmarket.DealStatusPaymentChannelAddingFunds) @@ -206,10 +212,12 @@ func TestWaitForPaymentChannelAddFunds(t *testing.T) { PayCh: expectedPayCh, AddFundsCID: msgCID, WaitForAddFundsErr: errors.New("boom"), + Lane: expectedLane, } runWaitForPaychAddFunds(t, params, dealState) assert.Contains(t, dealState.Message, "boom") assert.Equal(t, dealState.Status, retrievalmarket.DealStatusFailed) + assert.Equal(t, uint64(0), dealState.PaymentInfo.Lane) }) } @@ -591,13 +599,10 @@ var defaultPaymentRequested = abi.NewTokenAmount(500000) func makeDealState(status retrievalmarket.DealStatus) *retrievalmarket.ClientDealState { return &retrievalmarket.ClientDealState{ - TotalFunds: defaultTotalFunds, - MinerWallet: address.TestAddress, - ClientWallet: address.TestAddress2, - PaymentInfo: &retrievalmarket.PaymentInfo{ - PayCh: address.TestAddress2, - Lane: uint64(10), - }, + TotalFunds: defaultTotalFunds, + MinerWallet: address.TestAddress, + ClientWallet: address.TestAddress2, + PaymentInfo: &retrievalmarket.PaymentInfo{}, Status: status, BytesPaidFor: defaultBytesPaidFor, TotalReceived: defaultTotalReceived, diff --git a/retrievalmarket/impl/integration_test.go b/retrievalmarket/impl/integration_test.go index 62b7f060..95fc5d6f 100644 --- a/retrievalmarket/impl/integration_test.go +++ b/retrievalmarket/impl/integration_test.go @@ -12,7 +12,10 @@ import ( "github.com/filecoin-project/specs-actors/actors/builtin/paych" "github.com/ipfs/go-cid" "github.com/ipld/go-ipld-prime" + ipldfree "github.com/ipld/go-ipld-prime/impl/free" cidlink "github.com/ipld/go-ipld-prime/linking/cid" + "github.com/ipld/go-ipld-prime/traversal/selector" + "github.com/ipld/go-ipld-prime/traversal/selector/builder" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -130,16 +133,16 @@ func requireSetupTestClientAndProvider(bgCtx context.Context, t *testing.T, payC func TestClientCanMakeDealWithProvider(t *testing.T) { // -------- SET UP PROVIDER - //ssb := builder.NewSelectorSpecBuilder(ipldfree.NodeBuilder()) + ssb := builder.NewSelectorSpecBuilder(ipldfree.NodeBuilder()) - //allSelector := ssb.ExploreRecursive(selector.RecursionLimitNone(), - // ssb.ExploreAll(ssb.ExploreRecursiveEdge())).Node() - // - //partialSelector := ssb.ExploreFields(func(specBuilder builder.ExploreFieldsSpecBuilder) { - // specBuilder.Insert("Links", ssb.ExploreIndex(0, ssb.ExploreFields(func(specBuilder builder.ExploreFieldsSpecBuilder) { - // specBuilder.Insert("Hash", ssb.Matcher()) - // }))) - //}).Node() + allSelector := ssb.ExploreRecursive(selector.RecursionLimitNone(), + ssb.ExploreAll(ssb.ExploreRecursiveEdge())).Node() + + partialSelector := ssb.ExploreFields(func(specBuilder builder.ExploreFieldsSpecBuilder) { + specBuilder.Insert("Links", ssb.ExploreIndex(0, ssb.ExploreFields(func(specBuilder builder.ExploreFieldsSpecBuilder) { + specBuilder.Insert("Hash", ssb.Matcher()) + }))) + }).Node() testCases := []struct { name string @@ -149,45 +152,45 @@ func TestClientCanMakeDealWithProvider(t *testing.T) { selector ipld.Node paramsV1, unsealing, addFunds bool }{ - //{name: "1 block file retrieval succeeds", - // filename: "lorem_under_1_block.txt", - // filesize: 410, - // voucherAmts: []abi.TokenAmount{abi.NewTokenAmount(410000)}, - // unsealing: false}, + {name: "1 block file retrieval succeeds", + filename: "lorem_under_1_block.txt", + filesize: 410, + voucherAmts: []abi.TokenAmount{abi.NewTokenAmount(410000)}, + unsealing: false}, {name: "1 block file retrieval succeeds with existing payment channel", filename: "lorem_under_1_block.txt", filesize: 410, voucherAmts: []abi.TokenAmount{abi.NewTokenAmount(410000)}, unsealing: false, addFunds: true}, - //{name: "1 block file retrieval succeeds with unsealing", - // filename: "lorem_under_1_block.txt", - // filesize: 410, - // voucherAmts: []abi.TokenAmount{abi.NewTokenAmount(410000)}, - // unsealing: true}, - //{name: "multi-block file retrieval succeeds", - // filename: "lorem.txt", - // filesize: 19000, - // voucherAmts: []abi.TokenAmount{abi.NewTokenAmount(10136000), abi.NewTokenAmount(9784000)}, - // unsealing: false}, - //{name: "multi-block file retrieval succeeds with unsealing", - // filename: "lorem.txt", - // filesize: 19000, - // voucherAmts: []abi.TokenAmount{abi.NewTokenAmount(10136000), abi.NewTokenAmount(9784000)}, - // unsealing: true}, - //{name: "multi-block file retrieval succeeds with V1 params and allSelector", - // filename: "lorem.txt", - // filesize: 19000, - // voucherAmts: []abi.TokenAmount{abi.NewTokenAmount(10136000), abi.NewTokenAmount(9784000)}, - // paramsV1: true, - // selector: allSelector, - // unsealing: false}, - //{name: "partial file retrieval succeeds with V1 params and selector recursion depth 1", - // filename: "lorem.txt", - // filesize: 1024, - // voucherAmts: []abi.TokenAmount{abi.NewTokenAmount(1944000)}, - // paramsV1: true, - // selector: partialSelector, - // unsealing: false}, + {name: "1 block file retrieval succeeds with unsealing", + filename: "lorem_under_1_block.txt", + filesize: 410, + voucherAmts: []abi.TokenAmount{abi.NewTokenAmount(410000)}, + unsealing: true}, + {name: "multi-block file retrieval succeeds", + filename: "lorem.txt", + filesize: 19000, + voucherAmts: []abi.TokenAmount{abi.NewTokenAmount(10136000), abi.NewTokenAmount(9784000)}, + unsealing: false}, + {name: "multi-block file retrieval succeeds with unsealing", + filename: "lorem.txt", + filesize: 19000, + voucherAmts: []abi.TokenAmount{abi.NewTokenAmount(10136000), abi.NewTokenAmount(9784000)}, + unsealing: true}, + {name: "multi-block file retrieval succeeds with V1 params and allSelector", + filename: "lorem.txt", + filesize: 19000, + voucherAmts: []abi.TokenAmount{abi.NewTokenAmount(10136000), abi.NewTokenAmount(9784000)}, + paramsV1: true, + selector: allSelector, + unsealing: false}, + {name: "partial file retrieval succeeds with V1 params and selector recursion depth 1", + filename: "lorem.txt", + filesize: 1024, + voucherAmts: []abi.TokenAmount{abi.NewTokenAmount(1944000)}, + paramsV1: true, + selector: partialSelector, + unsealing: false}, } for i, testCase := range testCases { From 84823dccbc37c7eab4a92ad740942467a01f0f64 Mon Sep 17 00:00:00 2001 From: shannonwells Date: Tue, 14 Apr 2020 13:44:07 -0700 Subject: [PATCH 9/9] allocate lane failure test, add state to from list in allocate lane errored --- retrievalmarket/impl/clientstates/client_fsm.go | 2 +- .../impl/clientstates/client_states_test.go | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/retrievalmarket/impl/clientstates/client_fsm.go b/retrievalmarket/impl/clientstates/client_fsm.go index 36fa410b..7eeac92b 100644 --- a/retrievalmarket/impl/clientstates/client_fsm.go +++ b/retrievalmarket/impl/clientstates/client_fsm.go @@ -60,7 +60,7 @@ var ClientEvents = fsm.Events{ return nil }), fsm.Event(rm.ClientEventAllocateLaneErrored). - From(rm.DealStatusPaymentChannelCreating).To(rm.DealStatusFailed). + FromMany(rm.DealStatusPaymentChannelCreating, rm.DealStatusPaymentChannelAddingFunds).To(rm.DealStatusFailed). Action(func(deal *rm.ClientDealState, err error) error { deal.Message = xerrors.Errorf("allocating payment lane: %w", err).Error() return nil diff --git a/retrievalmarket/impl/clientstates/client_states_test.go b/retrievalmarket/impl/clientstates/client_states_test.go index 213ed8f2..6e99160d 100644 --- a/retrievalmarket/impl/clientstates/client_states_test.go +++ b/retrievalmarket/impl/clientstates/client_states_test.go @@ -219,6 +219,21 @@ func TestWaitForPaymentChannelAddFunds(t *testing.T) { assert.Equal(t, dealState.Status, retrievalmarket.DealStatusFailed) assert.Equal(t, uint64(0), dealState.PaymentInfo.Lane) }) + t.Run("if AllocateLane fails", func(t *testing.T) { + dealState := makeDealState(retrievalmarket.DealStatusPaymentChannelAddingFunds) + dealState.WaitMsgCID = &msgCID + params := testnodes.TestRetrievalClientNodeParams{ + AddFundsOnly: true, + PayCh: expectedPayCh, + AddFundsCID: msgCID, + LaneError: errors.New("boom"), + Lane: expectedLane, + } + runWaitForPaychAddFunds(t, params, dealState) + assert.Contains(t, dealState.Message, "boom") + assert.Equal(t, dealState.Status, retrievalmarket.DealStatusFailed) + assert.Equal(t, uint64(0), dealState.PaymentInfo.Lane) + }) } func TestProposeDeal(t *testing.T) {