fix: add funds to payment channel before allocating lane (#495)

docs/retrievalclient.mmd

@@ -25,6 +25,7 @@ stateDiagram-v2
 	state "DealStatusRetryLegacy" as 27
 	state "DealStatusWaitForAcceptanceLegacy" as 28
 	state "DealStatusWaitingForLastBlocks" as 29
+	state "DealStatusPaymentChannelAddingInitialFunds" as 30
 	0 : On entry runs ProposeDeal
 	4 : On entry runs WaitPaymentChannelReady
 	5 : On entry runs WaitPaymentChannelReady
@@ -40,6 +41,7 @@ stateDiagram-v2
 	24 : On entry runs AllocateLane
 	25 : On entry runs CancelDeal
 	27 : On entry runs ProposeDeal
+	30 : On entry runs WaitPaymentChannelReady
 	[*] --> 0
 	note right of 0
 		The following events are not shown cause they can trigger from any state.
@@ -65,11 +67,12 @@ stateDiagram-v2
 	6 --> 8 : ClientEventPaymentChannelErrored
 	6 --> 13 : ClientEventPaymentChannelSkip
 	6 --> 4 : ClientEventPaymentChannelCreateInitiated
-	6 --> 24 : ClientEventPaymentChannelAddingFunds
+	6 --> 30 : ClientEventPaymentChannelAddingFunds
 	22 --> 5 : ClientEventPaymentChannelAddingFunds
 	4 --> 24 : ClientEventPaymentChannelReady
 	5 --> 13 : ClientEventPaymentChannelReady
 	22 --> 13 : ClientEventPaymentChannelReady
+	30 --> 24 : ClientEventPaymentChannelReady
 	24 --> 8 : ClientEventAllocateLaneErrored
 	24 --> 13 : ClientEventLaneAllocated
 	10 --> 14 : ClientEventLastPaymentRequested

retrievalmarket/dealstatus.go

@@ -113,38 +113,44 @@ const (
 	// the deal price per byte is zero (if it's not zero the provider asks
 	// for final payment after sending the last blocks).
 	DealStatusClientWaitingForLastBlocks
+
+	// DealStatusPaymentChannelAddingInitialFunds means that a payment channel
+	// exists from an earlier deal between client and provider, but we need
+	// to add funds to the channel for this particular deal
+	DealStatusPaymentChannelAddingInitialFunds
 )
 
 // DealStatuses maps deal status to a human readable representation
 var DealStatuses = map[DealStatus]string{
-	DealStatusNew:                          "DealStatusNew",
-	DealStatusUnsealing:                    "DealStatusUnsealing",
-	DealStatusUnsealed:                     "DealStatusUnsealed",
-	DealStatusWaitForAcceptance:            "DealStatusWaitForAcceptance",
-	DealStatusPaymentChannelCreating:       "DealStatusPaymentChannelCreating",
-	DealStatusPaymentChannelAddingFunds:    "DealStatusPaymentChannelAddingFunds",
-	DealStatusAccepted:                     "DealStatusAccepted",
-	DealStatusFundsNeededUnseal:            "DealStatusFundsNeededUnseal",
-	DealStatusFailing:                      "DealStatusFailing",
-	DealStatusRejected:                     "DealStatusRejected",
-	DealStatusFundsNeeded:                  "DealStatusFundsNeeded",
-	DealStatusSendFunds:                    "DealStatusSendFunds",
-	DealStatusSendFundsLastPayment:         "DealStatusSendFundsLastPayment",
-	DealStatusOngoing:                      "DealStatusOngoing",
-	DealStatusFundsNeededLastPayment:       "DealStatusFundsNeededLastPayment",
-	DealStatusCompleted:                    "DealStatusCompleted",
-	DealStatusDealNotFound:                 "DealStatusDealNotFound",
-	DealStatusErrored:                      "DealStatusErrored",
-	DealStatusBlocksComplete:               "DealStatusBlocksComplete",
-	DealStatusFinalizing:                   "DealStatusFinalizing",
-	DealStatusCompleting:                   "DealStatusCompleting",
-	DealStatusCheckComplete:                "DealStatusCheckComplete",
-	DealStatusCheckFunds:                   "DealStatusCheckFunds",
-	DealStatusInsufficientFunds:            "DealStatusInsufficientFunds",
-	DealStatusPaymentChannelAllocatingLane: "DealStatusPaymentChannelAllocatingLane",
-	DealStatusCancelling:                   "DealStatusCancelling",
-	DealStatusCancelled:                    "DealStatusCancelled",
-	DealStatusRetryLegacy:                  "DealStatusRetryLegacy",
-	DealStatusWaitForAcceptanceLegacy:      "DealStatusWaitForAcceptanceLegacy",
-	DealStatusClientWaitingForLastBlocks:   "DealStatusWaitingForLastBlocks",
+	DealStatusNew:                              "DealStatusNew",
+	DealStatusUnsealing:                        "DealStatusUnsealing",
+	DealStatusUnsealed:                         "DealStatusUnsealed",
+	DealStatusWaitForAcceptance:                "DealStatusWaitForAcceptance",
+	DealStatusPaymentChannelCreating:           "DealStatusPaymentChannelCreating",
+	DealStatusPaymentChannelAddingFunds:        "DealStatusPaymentChannelAddingFunds",
+	DealStatusAccepted:                         "DealStatusAccepted",
+	DealStatusFundsNeededUnseal:                "DealStatusFundsNeededUnseal",
+	DealStatusFailing:                          "DealStatusFailing",
+	DealStatusRejected:                         "DealStatusRejected",
+	DealStatusFundsNeeded:                      "DealStatusFundsNeeded",
+	DealStatusSendFunds:                        "DealStatusSendFunds",
+	DealStatusSendFundsLastPayment:             "DealStatusSendFundsLastPayment",
+	DealStatusOngoing:                          "DealStatusOngoing",
+	DealStatusFundsNeededLastPayment:           "DealStatusFundsNeededLastPayment",
+	DealStatusCompleted:                        "DealStatusCompleted",
+	DealStatusDealNotFound:                     "DealStatusDealNotFound",
+	DealStatusErrored:                          "DealStatusErrored",
+	DealStatusBlocksComplete:                   "DealStatusBlocksComplete",
+	DealStatusFinalizing:                       "DealStatusFinalizing",
+	DealStatusCompleting:                       "DealStatusCompleting",
+	DealStatusCheckComplete:                    "DealStatusCheckComplete",
+	DealStatusCheckFunds:                       "DealStatusCheckFunds",
+	DealStatusInsufficientFunds:                "DealStatusInsufficientFunds",
+	DealStatusPaymentChannelAllocatingLane:     "DealStatusPaymentChannelAllocatingLane",
+	DealStatusCancelling:                       "DealStatusCancelling",
+	DealStatusCancelled:                        "DealStatusCancelled",
+	DealStatusRetryLegacy:                      "DealStatusRetryLegacy",
+	DealStatusWaitForAcceptanceLegacy:          "DealStatusWaitForAcceptanceLegacy",
+	DealStatusClientWaitingForLastBlocks:       "DealStatusWaitingForLastBlocks",
+	DealStatusPaymentChannelAddingInitialFunds: "DealStatusPaymentChannelAddingInitialFunds",
 }

retrievalmarket/impl/clientstates/client_fsm.go

@@ -92,8 +92,13 @@ var ClientEvents = fsm.Events{
 			return nil
 		}),
 
+	// Client is adding funds to payment channel
 	fsm.Event(rm.ClientEventPaymentChannelAddingFunds).
-		FromMany(rm.DealStatusAccepted).To(rm.DealStatusPaymentChannelAllocatingLane).
+		// If the deal has just been accepted, we are adding the initial funds
+		// to the payment channel
+		FromMany(rm.DealStatusAccepted).To(rm.DealStatusPaymentChannelAddingInitialFunds).
+		// If the deal was already ongoing, and ran out of funds, we are
+		// topping up funds in the payment channel
 		FromMany(rm.DealStatusCheckFunds).To(rm.DealStatusPaymentChannelAddingFunds).
 		Action(func(deal *rm.ClientDealState, msgCID cid.Cid, payCh address.Address) error {
 			deal.WaitMsgCID = &msgCID
@@ -105,8 +110,18 @@ var ClientEvents = fsm.Events{
 			return nil
 		}),
 
+	// The payment channel add funds message has landed on chain
 	fsm.Event(rm.ClientEventPaymentChannelReady).
+		// If the payment channel between client and provider was being created
+		// for the first time, or if the payment channel had already been
+		// created for an earlier deal but the initial funding for this deal
+		// was being added, then we still need to allocate a payment channel
+		// lane
 		From(rm.DealStatusPaymentChannelCreating).To(rm.DealStatusPaymentChannelAllocatingLane).
+		From(rm.DealStatusPaymentChannelAddingInitialFunds).To(rm.DealStatusPaymentChannelAllocatingLane).
+		// If the payment channel ran out of funds and needed to be topped up,
+		// then the payment channel lane already exists so just move straight
+		// to the ongoing state
 		From(rm.DealStatusPaymentChannelAddingFunds).To(rm.DealStatusOngoing).
 		From(rm.DealStatusCheckFunds).To(rm.DealStatusOngoing).
 		Action(func(deal *rm.ClientDealState, payCh address.Address) error {
@@ -318,19 +333,20 @@ var ClientFinalityStates = []fsm.StateKey{
 
 // ClientStateEntryFuncs are the handlers for different states in a retrieval client
 var ClientStateEntryFuncs = fsm.StateEntryFuncs{
-	rm.DealStatusNew:                          ProposeDeal,
-	rm.DealStatusRetryLegacy:                  ProposeDeal,
-	rm.DealStatusAccepted:                     SetupPaymentChannelStart,
-	rm.DealStatusPaymentChannelCreating:       WaitPaymentChannelReady,
-	rm.DealStatusPaymentChannelAllocatingLane: AllocateLane,
-	rm.DealStatusOngoing:                      Ongoing,
-	rm.DealStatusFundsNeeded:                  ProcessPaymentRequested,
-	rm.DealStatusFundsNeededLastPayment:       ProcessPaymentRequested,
-	rm.DealStatusSendFunds:                    SendFunds,
-	rm.DealStatusSendFundsLastPayment:         SendFunds,
-	rm.DealStatusCheckFunds:                   CheckFunds,
-	rm.DealStatusPaymentChannelAddingFunds:    WaitPaymentChannelReady,
-	rm.DealStatusFailing:                      CancelDeal,
-	rm.DealStatusCancelling:                   CancelDeal,
-	rm.DealStatusCheckComplete:                CheckComplete,
+	rm.DealStatusNew:                              ProposeDeal,
+	rm.DealStatusRetryLegacy:                      ProposeDeal,
+	rm.DealStatusAccepted:                         SetupPaymentChannelStart,
+	rm.DealStatusPaymentChannelCreating:           WaitPaymentChannelReady,
+	rm.DealStatusPaymentChannelAddingInitialFunds: WaitPaymentChannelReady,
+	rm.DealStatusPaymentChannelAllocatingLane:     AllocateLane,
+	rm.DealStatusOngoing:                          Ongoing,
+	rm.DealStatusFundsNeeded:                      ProcessPaymentRequested,
+	rm.DealStatusFundsNeededLastPayment:           ProcessPaymentRequested,
+	rm.DealStatusSendFunds:                        SendFunds,
+	rm.DealStatusSendFundsLastPayment:             SendFunds,
+	rm.DealStatusCheckFunds:                       CheckFunds,
+	rm.DealStatusPaymentChannelAddingFunds:        WaitPaymentChannelReady,
+	rm.DealStatusFailing:                          CancelDeal,
+	rm.DealStatusCancelling:                       CancelDeal,
+	rm.DealStatusCheckComplete:                    CheckComplete,
 }

retrievalmarket/impl/clientstates/client_states_test.go

@@ -93,6 +93,7 @@ func TestProposeDeal(t *testing.T) {
 		require.Equal(t, dealState.Status, retrievalmarket.DealStatusErrored)
 	})
 }
+
 func TestSetupPaymentChannel(t *testing.T) {
 	ctx := context.Background()
 	expectedPayCh := address.TestAddress2
@@ -131,7 +132,7 @@ func TestSetupPaymentChannel(t *testing.T) {
 		runSetupPaymentChannel(t, envParams, dealState)
 		require.Empty(t, dealState.Message)
 		require.Equal(t, envParams.AddFundsCID, *dealState.WaitMsgCID)
-		require.Equal(t, retrievalmarket.DealStatusPaymentChannelAllocatingLane, dealState.Status)
+		require.Equal(t, retrievalmarket.DealStatusPaymentChannelAddingInitialFunds, dealState.Status)
 		require.Equal(t, expectedPayCh, dealState.PaymentInfo.PayCh)
 	})
 

retrievalmarket/storage_retrieval_integration_test.go

@@ -54,7 +54,7 @@ func TestStorageRetrieval(t *testing.T) {
 	_ = sh.Client.SubscribeToEvents(clientSubscriber)
 
 	// set ask price where we'll accept any price
-	err := sh.Provider.SetAsk(big.NewInt(0), big.NewInt(0), 50_000)
+	err := sh.Provider.SetAsk(big.NewInt(0), big.NewInt(0), 50000)
 	assert.NoError(t, err)
 
 	result := sh.ProposeStorageDeal(t, &storagemarket.DataRef{TransferType: storagemarket.TTGraphsync, Root: sh.PayloadCid}, false, false)