send tipset identifier to node when interacting with chain #172

laser · 2020-03-25T22:06:19Z

Why does this PR exist?

This is the last (hopefully) PR in a chain of PRs which pass chain state identifiers to methods on the Node* interfaces.

shannonwells · 2020-03-25T22:56:46Z

storagemarket/testnodes/testnodes.go

@@ -180,8 +180,8 @@ func (n *FakeClientNode) OnDealSectorCommitted(ctx context.Context, provider add
 }

 // ValidateAskSignature returns the stubbed validation error


fix comment

shannonwells

LGTM, updating comment is optional

hannahhoward

For the forward facing APIs on Client and Provider, I would prefer not to augment them with a tipset token, and instead have them internally call GetMoreRecentStateID internally — these are intended to be used at the command line level in a user facing way, and I think it’s unlikely that a user will want or know how to pass a TipSet token in directly. Open to a discussion about this -- it might be useful to hear @ingar 's feedback cause he's worked with StorageMarket a fair amount. One other alternative is to let it be a variadic argument, but I'd be inclined to lean away from that.

Also, should LocatePieceForDealWithinSector take a tipset token? My understanding is this doesn't really talk to the chain so much as it uses the storage miner code directly. This is non-blocking if you can confirm my understanding is wrong.

storagemarket/impl/client.go

hannahhoward · 2020-03-25T23:57:49Z

storagemarket/impl/client.go

@@ -242,8 +248,8 @@ func (c *Client) ProposeStorageDeal(
 		})
 }

-func (c *Client) GetPaymentEscrow(ctx context.Context, addr address.Address) (storagemarket.Balance, error) {
-	return c.node.GetBalance(ctx, addr)
+func (c *Client) GetPaymentEscrow(ctx context.Context, addr address.Address, tok shared.TipSetToken) (storagemarket.Balance, error) {


hannahhoward · 2020-03-25T23:58:00Z

storagemarket/impl/client.go

@@ -137,7 +138,7 @@ func (c *Client) GetInProgressDeal(ctx context.Context, cid cid.Cid) (storagemar
 	return out, nil
 }

-func (c *Client) GetAsk(ctx context.Context, info storagemarket.StorageProviderInfo) (*storagemarket.SignedStorageAsk, error) {
+func (c *Client) GetAsk(ctx context.Context, info storagemarket.StorageProviderInfo, tok shared.TipSetToken) (*storagemarket.SignedStorageAsk, error) {
 	s, err := c.net.NewAskStream(info.PeerID)


hannahhoward · 2020-03-25T23:58:25Z

storagemarket/impl/client.go

@@ -117,8 +118,8 @@ func (c *Client) ListProviders(ctx context.Context) (<-chan storagemarket.Storag
 	return out, nil
 }

-func (c *Client) ListDeals(ctx context.Context, addr address.Address) ([]storagemarket.StorageDeal, error) {
-	return c.node.ListClientDeals(ctx, addr)
+func (c *Client) ListDeals(ctx context.Context, addr address.Address, tok shared.TipSetToken) ([]storagemarket.StorageDeal, error) {


hannahhoward · 2020-03-25T23:59:48Z

storagemarket/impl/provider.go

@@ -223,16 +225,16 @@ func (p *Provider) ListAsks(addr address.Address) []*storagemarket.SignedStorage
 	return nil
 }

-func (p *Provider) ListDeals(ctx context.Context) ([]storagemarket.StorageDeal, error) {
-	return p.spn.ListProviderDeals(ctx, p.actor)
+func (p *Provider) ListDeals(ctx context.Context, tok shared.TipSetToken) ([]storagemarket.StorageDeal, error) {


Please call MostRecentStateID internally and leave signature unchanged

hannahhoward · 2020-03-25T23:59:55Z

storagemarket/impl/provider.go

 }

 func (p *Provider) AddStorageCollateral(ctx context.Context, amount abi.TokenAmount) error {
 	return p.spn.AddFunds(ctx, p.actor, amount)
 }

-func (p *Provider) GetStorageCollateral(ctx context.Context) (storagemarket.Balance, error) {
-	return p.spn.GetBalance(ctx, p.actor)
+func (p *Provider) GetStorageCollateral(ctx context.Context, tok shared.TipSetToken) (storagemarket.Balance, error) {


hannahhoward · 2020-03-26T00:02:15Z

storagemarket/types.go

@@ -321,7 +321,7 @@ type StorageProviderNode interface {

 	OnDealSectorCommitted(ctx context.Context, provider address.Address, dealID abi.DealID, cb DealSectorCommittedCallback) error

-	LocatePieceForDealWithinSector(ctx context.Context, dealID abi.DealID) (sectorID uint64, offset uint64, length uint64, err error)
+	LocatePieceForDealWithinSector(ctx context.Context, dealID abi.DealID, tok shared.TipSetToken) (sectorID uint64, offset uint64, length uint64, err error)


Is this method actually talking to the chain? I believe it's not.. though I could be wrong.

Yep. The go-filecoin implementation of this method requires both storage miner actor and storage miner actor states.

laser · 2020-03-26T02:54:20Z

@hannahhoward

For the forward facing APIs on Client and Provider, I would prefer not to augment them with a tipset token, and instead have them internally call GetMoreRecentStateID internally

That makes sense to me. I'll make that update tomorrow.

laser · 2020-03-26T02:58:09Z

Also, should LocatePieceForDealWithinSector take a tipset token? My understanding is this doesn't really talk to the chain so much as it uses the storage miner code directly. This is non-blocking if you can confirm my understanding is wrong.

Yep, indeed it does. At least in go-filecoin.

laser added 2 commits March 25, 2020 15:09

plumb TipSetToken to various other iface methods

a303116

provide tipset identifier to all ops which interact with chain

fb66677

laser force-pushed the feat/send-state-identifier-beta branch from 79dc1c5 to fb66677 Compare March 25, 2020 22:09

laser requested review from hannahhoward and shannonwells March 25, 2020 22:11

shannonwells reviewed Mar 25, 2020

View reviewed changes

shannonwells approved these changes Mar 25, 2020

View reviewed changes

hannahhoward requested changes Mar 26, 2020

View reviewed changes

Client and Provider call GetHead themselves, as per @hannahhoward

3bf436c

laser requested a review from hannahhoward March 26, 2020 20:07

use version of filecoin-ffi which authenticates with GitHub releases

11da847

laser force-pushed the feat/send-state-identifier-beta branch from 87066d3 to 11da847 Compare March 26, 2020 20:08

hannahhoward approved these changes Mar 26, 2020

View reviewed changes

fix comment as per PR feedback

b8ce9e6

laser merged commit 16ce89e into master Mar 26, 2020

laser deleted the feat/send-state-identifier-beta branch March 26, 2020 20:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

send tipset identifier to node when interacting with chain #172

send tipset identifier to node when interacting with chain #172

laser commented Mar 25, 2020

shannonwells Mar 25, 2020

laser Mar 26, 2020

shannonwells left a comment •

edited

Loading

hannahhoward left a comment

hannahhoward Mar 25, 2020

hannahhoward Mar 25, 2020

hannahhoward Mar 25, 2020

hannahhoward Mar 25, 2020

hannahhoward Mar 25, 2020

hannahhoward Mar 26, 2020

laser Mar 26, 2020

laser commented Mar 26, 2020

laser commented Mar 26, 2020

		@@ -180,8 +180,8 @@ func (n *FakeClientNode) OnDealSectorCommitted(ctx context.Context, provider add
		}

		// ValidateAskSignature returns the stubbed validation error

send tipset identifier to node when interacting with chain #172

send tipset identifier to node when interacting with chain #172

Conversation

laser commented Mar 25, 2020

Why does this PR exist?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shannonwells left a comment • edited Loading

Choose a reason for hiding this comment

hannahhoward left a 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

laser commented Mar 26, 2020

laser commented Mar 26, 2020

shannonwells left a comment •

edited

Loading