add `inspect-deal` command to `lotus client` #5833

nonsense · 2021-03-18T12:55:08Z

Requires: Add DealStages to track and log Deal status updates go-fil-markets#502
Requires: Add ChannelStages to keep track of history of lifecycle of a DataTransfer go-data-transfer#163

This PR is adding a inspect-deal subcommand to lotus client. Its purpose is to provide more information about a specific deal, and filter deals by deal-id or by proposal-cid.

At the moment the output looks like:

Deal ID: 10
Proposal CID: bafyreigvio7johbflmwvcqsqvov26gz62mscy3f5yb4ymjdcbscd2q6d3m

Stage: ReserveClientFunds: Reserving client funds (a few minutes)
  Mar 18 12:18:56 funds reserved, amount <402397026845688>
  Mar 18 12:18:56 adding funds with message <bafy2bzaceax3kesknjfft25kq3m3nnf4zfkuxdebseosessntkbhrmv3sgzgk>
Stage: FundsReserved: FundsReserved (a few minutes)
  Mar 18 12:19:25 initiate data transfer
Stage: StartDataTransfer: Starting data transfer (a few minutes)
  Mar 18 12:19:26 data transfer initiated on channel id <12D3KooWPUTfSmaU5RMKF2oQZfWdDsNGP6ooGoi4wNzKNeGTaDiC-12D3KooWFXYNSdc5z74bjHp1AhVAZR9bijayy62oRRgwnR6fGw3e-0>
              Data transfer stage: Requested
              Data transfer stage: Ongoing
              Mar 18 12:19:26 queued data
              Mar 18 12:19:26 sent data
              Data transfer stage: ResponderCompleted
              Data transfer stage: Completing
Stage: CheckForAcceptance: Checking for deal acceptance (a few minutes)
  Mar 18 12:19:27 provider is verifying the data
  Mar 18 12:19:28 waiting for provider to publish the deal on-chain
  Mar 18 12:21:33 provider has submitted the deal on-chain
  Mar 18 12:22:22 deal has been accepted
Stage: ProposalAccepted: Proposal accepted (a few minutes)
  Mar 18 12:22:22 funds released, amount <402397026845688>
...

dirkmc

Once the debugging code is removed, LGTM 👍

cli/client.go

build/params_2k.go

cli/deal/showdeals.go

raulk · 2021-03-18T17:20:47Z

cli/deal/showdeals.go

+	color.Blue("Proposal CID: %s\n\n", di.ProposalCid.String())
+
+	for _, stg := range di.DealStages.Stages {
+		msg := fmt.Sprintf("%s %s: %s (%s)", color.BlueString("Stage:"), color.BlueString(strings.TrimPrefix(stg.Name, "StorageDeal")), stg.Description, color.GreenString(stg.ExpectedDuration))


Ideally we'd do a fmt.Fprintf in the cli.Context.App.Writer, but if all other commands are writing outputs directly to stdout, then we can keep it like it is.

cli/deal/showdeals.go

go.mod

node/impl/client/client.go

raulk · 2021-03-18T17:28:20Z

node/impl/client/client.go

@@ -289,6 +292,7 @@ func (a *API) newDealInfoWithTransfer(transferCh *api.DataTransferChannel, v sto
 		Verified:          v.Proposal.VerifiedDeal,
 		TransferChannelID: v.TransferChannelID,
 		DataTransfer:      transferCh,
+		DealStages:        v.DealStages,


This data can be quite heavy to transfer over the wire (JSON-RPC) if it's not needed.

Since the CLI only allows to inspect a single deal, why not create an ClientDealInspect JSON-RPC operation? It's wasteful to send over all the DealStages detail, when we know the CLI will only use a single one.

How big can this get?

If the deal transfer stages are unbounded (i.e. data transfer keeps crashing, and deal is restarted indefinitely), this will be unbounded. In general for successful deals (or those that have a few failures), the struct looks like the following (see attached file).
output.txt

So this is ok for a single deal, but I think that we shouldn't set this in ClientListDeals (which we do now)

I am now populating the Stages and DealStages fields only when calling ClientGetDealInfo and not on ClientListDeals - not sure if we want omitempty JSON tags so that they don't appear at all when making JSON RPC calls...

willscott · 2021-03-24T22:23:35Z

Noting that this is likely in critical path for work next week, so we ideally can get some eyes on it and get it into a version of lotus that a can be run at least on the testnet sooner rather than later.

node/impl/client/client.go

raulk

I was trying to take this over the finish line, but I believe it's missing commits. I know we removed the interactive menu, we renamed the command to inspect-deal, and it takes proposal-cid or deal-id flags. But I see none of this in this branch.

nonsense · 2021-03-29T16:57:38Z

@raulk this is ready for merging, once we get go-fil-markets and go-data-transfer PRs merged and we update go.mod here. The inspect-deal has been tested with deals created before/after this version, so it's backward compatible.

nonsense · 2021-03-30T14:01:40Z

I think this is ready for review and merge - all deps have been updated given the latest go-fil-markets release.

@dirkmc could you confirm if we should merge this before/after the changes in the minerx branch end on master?

cli/deal/inspect.go

magik6k · 2021-04-01T11:58:43Z

node/impl/client/client.go

@@ -289,6 +292,7 @@ func (a *API) newDealInfoWithTransfer(transferCh *api.DataTransferChannel, v sto
 		Verified:          v.Proposal.VerifiedDeal,
 		TransferChannelID: v.TransferChannelID,
 		DataTransfer:      transferCh,
+		DealStages:        v.DealStages,


How big can this get?

…stDeals

dirkmc and others added 2 commits March 4, 2021 16:56

feat: show deals CLI command

84803f8

remove interactive UX; add inspect-deal cmd

9d6c77d

nonsense requested review from dirkmc and raulk March 18, 2021 12:55

fixup

af81048

dirkmc approved these changes Mar 18, 2021

View reviewed changes

cli/client.go Outdated Show resolved Hide resolved

raulk reviewed Mar 18, 2021

View reviewed changes

nonsense added 5 commits March 19, 2021 13:53

use new channelmonitor.Config

86772a0

amend restart config

ac576c7

Merge branch 'master' into nonsense/cli-show-deals

22217b7

fix linter and merge master

bbd0cdd

use go-data-transfer from personal fork

b4d4766

raulk self-assigned this Mar 25, 2021

raulk force-pushed the nonsense/cli-show-deals branch from b4d4766 to 17ba23e Compare March 25, 2021 20:26

raulk reviewed Mar 25, 2021

View reviewed changes

node/impl/client/client.go Outdated Show resolved Hide resolved

raulk reviewed Mar 25, 2021

View reviewed changes

Merge branch 'master' into nonsense/cli-show-deals-backup

8ba0bcb

nonsense force-pushed the nonsense/cli-show-deals branch from 17ba23e to 8ba0bcb Compare March 29, 2021 15:20

nonsense added 2 commits March 29, 2021 18:31

use latest go-fil-markets and go-data-transfer

14e6ce8

commit full.json.gz and miner.json.gz docs

fd6d8b3

nonsense force-pushed the nonsense/cli-show-deals branch from 06379b3 to fd6d8b3 Compare March 29, 2021 15:32

nonsense added 3 commits March 29, 2021 18:39

remove storedCounters

77bc8d6

go mod tidy

f7eff6c

short circuit deals with no stages

82fb81a

nonsense force-pushed the nonsense/cli-show-deals branch from 91f3bb9 to 82fb81a Compare March 29, 2021 16:11

upgrade go-data-transfer

a43f4b3

use go-fil-markets v1.2.4

e79927a

nonsense added 2 commits March 30, 2021 16:15

resolved conflicts

139ff4a

rebuild docs with make docsgen

5014b17

nonsense marked this pull request as ready for review March 30, 2021 14:00

nonsense requested review from Kubuxu, magik6k and whyrusleeping as code owners March 30, 2021 14:00

nonsense added 5 commits March 30, 2021 17:08

better description for inspect-deal

b661f51

remove found boolean and use a pointer

adaf3a2

resolve conflicts

967fbf5

remove commented out code

03f4334

remove overwrite for go-data-transfer in testplans

1626dff

raulk added the docs-needed label Apr 1, 2021

magik6k reviewed Apr 1, 2021

View reviewed changes

nonsense added 2 commits April 1, 2021 15:07

move inspectDeal to cli package

686c719

include Stages and DealStages only on ClientDealInfo, not on ClientLi…

df003d4

…stDeals

nonsense force-pushed the nonsense/cli-show-deals branch from fc5ff8a to df003d4 Compare April 1, 2021 14:08

magik6k approved these changes Apr 1, 2021

View reviewed changes

magik6k merged commit 49be1f3 into master Apr 1, 2021

magik6k deleted the nonsense/cli-show-deals branch April 1, 2021 14:33

jennijuju mentioned this pull request Apr 6, 2021

[Deal Making Issue] Retrieval Deals Error: deal data transfer failed: incomplete response #5954

Closed

magik6k mentioned this pull request Apr 13, 2021

lotus v1.9.0 #5977

Closed

69 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add `inspect-deal` command to `lotus client` #5833

add `inspect-deal` command to `lotus client` #5833

nonsense commented Mar 18, 2021 •

edited

Loading

dirkmc left a comment

raulk Mar 18, 2021

raulk Mar 18, 2021

magik6k Apr 1, 2021

nonsense Apr 1, 2021

magik6k Apr 1, 2021

nonsense Apr 1, 2021 •

edited

Loading

willscott commented Mar 24, 2021

raulk left a comment

nonsense commented Mar 29, 2021

nonsense commented Mar 30, 2021

magik6k Apr 1, 2021

add inspect-deal command to lotus client #5833

add inspect-deal command to lotus client #5833

Conversation

nonsense commented Mar 18, 2021 • edited Loading

dirkmc left a comment

Choose a reason for hiding this comment

raulk Mar 18, 2021

Choose a reason for hiding this comment

raulk Mar 18, 2021

Choose a reason for hiding this comment

magik6k Apr 1, 2021

Choose a reason for hiding this comment

nonsense Apr 1, 2021

Choose a reason for hiding this comment

magik6k Apr 1, 2021

Choose a reason for hiding this comment

nonsense Apr 1, 2021 • edited Loading

Choose a reason for hiding this comment

willscott commented Mar 24, 2021

raulk left a comment

Choose a reason for hiding this comment

nonsense commented Mar 29, 2021

nonsense commented Mar 30, 2021

magik6k Apr 1, 2021

Choose a reason for hiding this comment

add `inspect-deal` command to `lotus client` #5833

add `inspect-deal` command to `lotus client` #5833

nonsense commented Mar 18, 2021 •

edited

Loading

nonsense Apr 1, 2021 •

edited

Loading