Data Transfer V2 Integration, Rebased #757

hannahhoward · 2022-10-12T04:47:25Z

Goals

Update to go data transfer v2!

Implementation

This is a rebase and update for #718, now using go-data-transfer v2 on master, not quite yet tagged.

@rvagg several of these changes are yours.

Others are my own. None are really "new" code since #718

This is dependent on #755, and also should go in after #752.

Also, we shouldn't merge until we get data transfer tagged. I'd like to deploy this in our autoretrieve instance and also maybe sofia miner first.

rvagg · 2022-10-12T06:34:39Z

retrievalmarket/impl/dtutils/dtutils.go

-		if !ok {
+		voucher := channelState.Voucher()
+		if voucher.Voucher == nil {
+			log.Errorf("received empty voucher")


if we get this in the logs, what is the likelihood that it's going to be in close-enough proximity to other messages that tell us what transfer this is for? do we need to start putting context-specific information in these messages to help with debugging?

a good question. ultimately, our pressing priority is going to be to improve error messages returned over the wire, since the data we collect is primarily from the client side. We often already have plenty of information on this side, but minimal ability to access it (cause it's per miner logs).

Let's address this as we move forward.

rvagg · 2022-10-12T08:31:33Z

retrievalmarket/impl/providerstates/provider_fsm.go

@@ -47,52 +45,46 @@ var ProviderEvents = fsm.Events{
 	// receiving blocks
 	fsm.Event(rm.ProviderEventBlockSent).
 		FromMany(rm.DealStatusOngoing).ToNoChange().
-		From(rm.DealStatusUnsealed).To(rm.DealStatusOngoing).
-		Action(func(deal *rm.ProviderDealState, totalSent uint64) error {


a ProviderEventBlockSent is fired in retrievalmarket/impl/requestvalidation/revalidator.go with an argument; is there any harm in leaving that in, should we remove it just to tidy up?

this file no longer exists

rvagg · 2022-10-12T08:33:08Z

retrievalmarket/impl/providerstates/provider_fsm.go


 	// request payment
 	fsm.Event(rm.ProviderEventPaymentRequested).
 		FromMany(rm.DealStatusOngoing, rm.DealStatusUnsealed).To(rm.DealStatusFundsNeeded).
 		From(rm.DealStatusFundsNeeded).ToJustRecord().
-		From(rm.DealStatusBlocksComplete).To(rm.DealStatusFundsNeededLastPayment).
-		From(rm.DealStatusNew).To(rm.DealStatusFundsNeededUnseal).
-		Action(func(deal *rm.ProviderDealState, totalSent uint64) error {


there's two ProviderEventPaymentRequested events in retrievalmarket/impl/requestvalidation/revalidator.go with arguments

this file no longer exists

rvagg · 2022-10-13T01:39:03Z

should the ffi have been updated with this too?

rvagg · 2022-10-13T08:06:26Z

retrievalmarket/migrations/migrations.go

-		LegacyProtocol:       true,
-	}, nil
+// NoOpClientDealState0To1 does nothing (old type removed)
+func NoOpClientDealState0To1(oldDs *maptypes.ClientDealState1) (*maptypes.ClientDealState1, error) {


What are these noop 0To1 migrations for?

This is an annoying part of the migration system -- it has to start from 0... even though I'm intentionally dropping the support here for making these changes. Reason # 243388 to not role your own database and database migration system.

retrievalmarket/types.go

rvagg · 2022-10-13T08:39:22Z

retrievalmarket/types.go

+	}
+	currentInterval := uint64(0)
+	bytesPaid := fundsReceived
+	if !p.UnsealPrice.NilOrZero() {


if these can be nil then do we need checks in IntervalLowerBound as well? we just proceed as if they're set

storagemarket/types.go

rvagg

Went over it all, hard to remember which bits are mine and which aren't. I'll admit to not following much of the provider states tests although they are nice coverage for that stuff.

codecov-commenter · 2022-10-14T04:58:07Z

Codecov Report

Merging #757 (76a7087) into master (0fccfc9) will increase coverage by 0.51%.
The diff coverage is 84.10%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #757      +/-   ##
==========================================
+ Coverage   57.66%   58.17%   +0.51%     
==========================================
  Files          66       64       -2     
  Lines        5564     5373     -191     
==========================================
- Hits         3208     3125      -83     
+ Misses       2013     1918      -95     
+ Partials      343      330      -13

Impacted Files	Coverage Δ
retrievalmarket/events.go	`0.00% <ø> (ø)`
retrievalmarket/impl/clientstates/client_fsm.go	`66.93% <ø> (ø)`
...etrievalmarket/impl/providerstates/provider_fsm.go	`47.83% <ø> (+39.01%)`	⬆️
retrievalmarket/network/query_stream.go	`60.00% <ø> (ø)`
shared/retrystream.go	`92.31% <ø> (ø)`
shared/selectors.go	`0.00% <0.00%> (ø)`
storagemarket/impl/client_environments.go	`0.00% <ø> (ø)`
storagemarket/impl/clientstates/client_fsm.go	`79.63% <ø> (ø)`
storagemarket/impl/provider_environments.go	`10.38% <ø> (ø)`
storagemarket/impl/providerstates/provider_fsm.go	`74.60% <ø> (ø)`
... and 34 more

Voucher refactor integration (#707) * refactor(storagemarket): update storagemarket * refactor(retrievalmarket): use new voucher system refactor for predictability and correctness using new voucher system * chore(deps): update go-data-transfer v2 * style(lint): prep for pr * docs(retrievalmarket): add comments * chore(deps): update go-statemachine * style(imports): fix imports chore(retrievalmarket): remove old types (#712) feat(ipld): bindnode support for all voucher types (#713) * feat(ipld): new data-transfer ipld vouchers + bindnode * feat(ipld): simplify ipldutils API * feat(ipld): use new bindnode registry in go-ipld-prime Ref: ipld/go-ipld-prime#437 feat(deps): update data transfer 61f0756c feat(deps): update data transfer and other deps update to master data transfer with libp2p v0.22.0 plus associated other deps Update retrievalmarket/types.go Update storagemarket/types.go chore(deps): update to rc candidate chore(dtutils): update for new data transfer configuration interface chore(deps): update to rc4

hannahhoward requested a review from rvagg October 12, 2022 04:47

hannahhoward force-pushed the feat/update-integration-final branch 2 times, most recently from 319fef7 to cd7796b Compare October 12, 2022 04:49

rvagg reviewed Oct 12, 2022

View reviewed changes

rvagg reviewed Oct 13, 2022

View reviewed changes

retrievalmarket/types.go Outdated Show resolved Hide resolved

rvagg reviewed Oct 13, 2022

View reviewed changes

storagemarket/types.go Outdated Show resolved Hide resolved

rvagg approved these changes Oct 13, 2022

View reviewed changes

hannahhoward force-pushed the feat/update-integration-final branch from 03c4f97 to 5bc3411 Compare October 14, 2022 04:53

hannahhoward changed the base branch from feat/revert-actors-v9 to master October 14, 2022 04:53

hannahhoward force-pushed the feat/update-integration-final branch 2 times, most recently from 0ec8e36 to 68279a8 Compare October 14, 2022 04:55

hannahhoward marked this pull request as ready for review November 15, 2022 19:36

hannahhoward force-pushed the feat/update-integration-final branch from 68279a8 to e6a7ac9 Compare November 17, 2022 18:30

hannahhoward force-pushed the feat/update-integration-final branch from e6a7ac9 to 143abad Compare January 7, 2023 01:05

hannahhoward force-pushed the feat/update-integration-final branch 2 times, most recently from 2217ef5 to ca5e771 Compare February 11, 2023 02:47

hannahhoward force-pushed the feat/update-integration-final branch from ca5e771 to 523aa39 Compare February 18, 2023 02:09

Merge branch 'master' into feat/update-integration-final

76a7087

hannahhoward merged commit 955fd43 into master Feb 18, 2023

dirkmc mentioned this pull request May 15, 2023

release: 1.28.0 #791

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Data Transfer V2 Integration, Rebased #757

Data Transfer V2 Integration, Rebased #757

hannahhoward commented Oct 12, 2022

rvagg Oct 12, 2022

hannahhoward Oct 14, 2022

rvagg Oct 12, 2022

hannahhoward Oct 14, 2022

rvagg Oct 12, 2022

hannahhoward Oct 14, 2022

rvagg commented Oct 13, 2022

rvagg Oct 13, 2022

hannahhoward Oct 14, 2022

rvagg Oct 13, 2022

rvagg left a comment

codecov-commenter commented Oct 14, 2022 •

edited

Loading

Data Transfer V2 Integration, Rebased #757

Data Transfer V2 Integration, Rebased #757

Conversation

hannahhoward commented Oct 12, 2022

Goals

Implementation

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rvagg commented Oct 13, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rvagg left a comment

Choose a reason for hiding this comment

codecov-commenter commented Oct 14, 2022 • edited Loading

Codecov Report

codecov-commenter commented Oct 14, 2022 •

edited

Loading