Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fatally fail contract deals on publish errors #1294

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion storagemarket/deal_execution.go
Original file line number Diff line number Diff line change
Expand Up @@ -451,11 +451,24 @@ func (p *Provider) publishDeal(ctx context.Context, pub event.Emitter, deal *typ
}
}

// Check if the client is an f4 address, ie an FVM contract
clientAddr := deal.ClientDealProposal.Proposal.Client.String()
isContractClient := len(clientAddr) >= 2 && (clientAddr[:2] == "t4" || clientAddr[:2] == "f4")

if isContractClient {
// For contract deal publish errors the deal fails fatally: we don't
// want to retry because the deal is probably taken by another SP
return &dealMakingError{
retry: types.DealRetryFatal,
error: fmt.Errorf("fatal error to publish deal %s: %w", deal.DealUuid, err),
}
}

// For any other publish error the user must manually retry: we don't
// want to automatically retry because deal publishing costs money
return &dealMakingError{
retry: types.DealRetryManual,
error: fmt.Errorf("failed to publish deal %s: %w", deal.DealUuid, err),
error: fmt.Errorf("recoverable error to publish deal %s: %w", deal.DealUuid, err),
}
}

Expand Down
37 changes: 36 additions & 1 deletion storagemarket/provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -707,6 +707,33 @@ func TestDealRestartAfterManualRecoverableErrors(t *testing.T) {
}
}

// Tests scenario that a contract deal fails fatally when PublishStorageDeal fails.
func TestContractDealFatalFailAfterPublishError(t *testing.T) {
ctx := context.Background()

// setup the provider test harness
harness := NewHarness(t)
// start the provider test harness
harness.Start(t, ctx)
defer harness.Stop()

// generate random f4 client contract address
f4addr, err := address.NewFromString("f410fnqocy7tkrlw4l5mdhgjlc4gsiafr7e76yopvo2y")
require.NoError(t, err)

// simulate publish deal failure
td := harness.newDealBuilder(t, 1, withClientAddr(f4addr)).withCommpNonBlocking().withPublishFailing(errors.New("puberr")).withNormalHttpServer().build()

// execute deal
err = td.executeAndSubscribe()
require.NoError(t, err)

// expect fatal error (types.DealRetryFatal) as this is a contract deal
// note that we return recoverable errors for regular client addresses
err = td.waitForError("puberr", types.DealRetryFatal)
require.NoError(t, err)
}

// Tests scenarios where a deal is paused with an error that the user must
// resolve by retrying manually, and the user fails the deal
func TestDealFailAfterManualRecoverableErrors(t *testing.T) {
Expand Down Expand Up @@ -1604,6 +1631,7 @@ type dealProposalConfig struct {
offlineDeal bool
verifiedDeal bool
providerCollateral abi.TokenAmount
clientAddr address.Address
minerAddr address.Address
pieceCid cid.Cid
pieceSize abi.PaddedPieceSize
Expand Down Expand Up @@ -1661,6 +1689,12 @@ func withMinerAddr(addr address.Address) dealProposalOpt {
}
}

func withClientAddr(addr address.Address) dealProposalOpt {
return func(dc *dealProposalConfig) {
dc.clientAddr = addr
}
}

func withPieceCid(c cid.Cid) dealProposalOpt {
return func(dc *dealProposalConfig) {
dc.pieceCid = c
Expand Down Expand Up @@ -1694,6 +1728,7 @@ func (ph *ProviderHarness) newDealBuilder(t *testing.T, seed int, opts ...dealPr
verifiedDeal: false,
providerCollateral: abi.NewTokenAmount(1),
minerAddr: tbuilder.ph.MinerAddr,
clientAddr: tbuilder.ph.ClientAddr,
pieceCid: cid.Undef,
undefinedPieceCid: false,
startEpoch: 50000,
Expand Down Expand Up @@ -1750,7 +1785,7 @@ func (ph *ProviderHarness) newDealBuilder(t *testing.T, seed int, opts ...dealPr
PieceCID: pieceCid,
PieceSize: pieceSize,
VerifiedDeal: dc.verifiedDeal,
Client: tbuilder.ph.ClientAddr,
Client: dc.clientAddr,
Provider: dc.minerAddr,
Label: dc.label,
StartEpoch: dc.startEpoch,
Expand Down