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.

Sign up for GitHub

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

feat: client flag to not announce deals #1051

Merged
merged 12 commits into from
Jan 10, 2023
Merged

feat: client flag to not announce deals #1051

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
3b42b8f
client flag to announce deals
Jan 4, 2023
adb7529
fix itest
Jan 4, 2023
c385d9d
apply suggestions
Jan 5, 2023
be5958e
test: fix announce tests
dirkmc Jan 6, 2023
3b6f7e2
refactor: remove dead code
dirkmc Jan 6, 2023
388b263
fix: TestCancelTransferForTransferredDealFails
dirkmc Jan 6, 2023
c101d66
fix var names
Jan 6, 2023
f2ae3be
fix typo
Jan 6, 2023
3f75923
refactor test
Jan 6, 2023
22fb267
fix double negative
Jan 6, 2023
bbcb206
change flag names
Jan 10, 2023
98737ad
change defaults value to false
Jan 10, 2023
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
Binary file modified build/openrpc/boost.json.gz
View file
Open in desktop
Binary file not shown.
14 changes: 10 additions & 4 deletions cmd/boost/deal_cmd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,14 +78,19 @@ var dealFlags = []cli.Flag{
Value: true,
},
&cli.BoolFlag{
Name: "fast-retrieval",
Usage: "indicates that data should be available for fast retrieval",
Value: true,
Name: "remove-unsealed-copy",
Usage: "indicates that an unsealed copy of the sector in not required for fast retrieval",
dirkmc marked this conversation as resolved.
Show resolved Hide resolved
Value: false,
},
&cli.StringFlag{
Name: "wallet",
Usage: "wallet address to be used to initiate the deal",
},
&cli.BoolFlag{
Name: "skip-ipni-announce",
Usage: "indicates that deal index should not be announced to the IPNI(Network Indexer)",
dirkmc marked this conversation as resolved.
Show resolved Hide resolved
Value: false,
},
}

var dealCmd = &cli.Command{
Expand Down Expand Up @@ -256,7 +261,8 @@ func dealCmdAction(cctx *cli.Context, isOnline bool) error {
DealDataRoot: rootCid,
IsOffline: !isOnline,
Transfer: transfer,
FastRetrieval: cctx.Bool("fast-retrieval"),
RemoveUnsealedCopy: cctx.Bool("remove-unsealed-copy"),
SkipIPNIAnnounce: cctx.Bool("skip-ipni-announce"),
}

log.Debugw("about to submit deal proposal", "uuid", dealUuid.String())
Expand Down
1 change: 1 addition & 0 deletions db/deals.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ func newDealAccessor(db *sql.DB, deal *types.ProviderDealState) *dealAccessor {
"Error": &fielddef.FieldDef{F: &deal.Err},
"Retry": &fielddef.FieldDef{F: &deal.Retry},
"FastRetrieval": &fielddef.FieldDef{F: &deal.FastRetrieval},
"AnnounceToIPNI": &fielddef.FieldDef{F: &deal.AnnounceToIPNI},

// Needed so the deal can be looked up by signed proposal cid
"SignedProposalCID": &fielddef.SignedPropFieldDef{Prop: deal.ClientDealProposal},
Expand Down
19 changes: 10 additions & 9 deletions db/fixtures.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,15 +84,16 @@ func GenerateNDeals(count int) ([]types.ProviderDealState, error) {
Params: []byte(fmt.Sprintf(`{"url":"http://files.org/file%d.car"}`, rand.Intn(1000))),
Size: uint64(rand.Intn(10000)),
},
ChainDealID: abi.DealID(rand.Intn(10000)),
PublishCID: &publishCid,
SectorID: abi.SectorNumber(rand.Intn(10000)),
Offset: abi.PaddedPieceSize(rand.Intn(1000000)),
Length: abi.PaddedPieceSize(rand.Intn(1000000)),
Checkpoint: dealcheckpoints.Accepted,
Retry: types.DealRetryAuto,
Err: dealErr,
FastRetrieval: false,
ChainDealID: abi.DealID(rand.Intn(10000)),
PublishCID: &publishCid,
SectorID: abi.SectorNumber(rand.Intn(10000)),
Offset: abi.PaddedPieceSize(rand.Intn(1000000)),
Length: abi.PaddedPieceSize(rand.Intn(1000000)),
Checkpoint: dealcheckpoints.Accepted,
Retry: types.DealRetryAuto,
Err: dealErr,
FastRetrieval: false,
AnnounceToIPNI: false,
}

deals = append(deals, deal)
Expand Down
10 changes: 10 additions & 0 deletions db/migrations/20230104230242_deal_announce_to_ipni.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
-- +goose Up
-- +goose StatementBegin
ALTER TABLE Deals
ADD AnnounceToIPNI BOOL;
-- +goose StatementEnd

-- +goose Down
-- +goose StatementBegin
SELECT 'down SQL query';
-- +goose StatementEnd
24 changes: 24 additions & 0 deletions db/migrations/20230104230411_deal_announce_to_ipni.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package migrations

import (
"database/sql"

"github.com/pressly/goose/v3"
)

func init() {
goose.AddMigration(upSetdealsAnnounceToIPNI, downSetdealsAnnounceToIPNI)
}

func upSetdealsAnnounceToIPNI(tx *sql.Tx) error {
_, err := tx.Exec("UPDATE Deals SET AnnounceToIPNI=?;", true)
if err != nil {
return err
}
return nil
}

func downSetdealsAnnounceToIPNI(tx *sql.Tx) error {
// This code is executed when the migration is rolled back.
return nil
}
46 changes: 46 additions & 0 deletions db/migrations_tests/deals_announce_to_ipni_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
package migrations_tests

import (
"context"
"testing"

"github.com/filecoin-project/boost/db"
"github.com/filecoin-project/boost/db/migrations"
"github.com/pressly/goose/v3"
"github.com/stretchr/testify/require"
)

func TestDealAnnounceToIPNI(t *testing.T) {
req := require.New(t)
ctx := context.Background()

sqldb := db.CreateTestTmpDB(t)
req.NoError(db.CreateAllBoostTables(ctx, sqldb, sqldb))

// Run migrations up to the one that adds the AnnounceToIPNI field to Deals
goose.SetBaseFS(migrations.EmbedMigrations)
req.NoError(goose.SetDialect("sqlite3"))
req.NoError(goose.UpTo(sqldb, ".", 20230104230242))

// Generate 1 deal
dealsDB := db.NewDealsDB(sqldb)
deals, err := db.GenerateNDeals(1)
req.NoError(err)

// Insert the deals in DB
err = dealsDB.Insert(ctx, &deals[0])
require.NoError(t, err)

// Get deal state
dealState, err := dealsDB.ByID(ctx, deals[0].DealUuid)
require.NoError(t, err)
require.False(t, dealState.AnnounceToIPNI)

//Run migration
req.NoError(goose.UpByOne(sqldb, "."))

// Check the deal state again
dealState, err = dealsDB.ByID(ctx, deals[0].DealUuid)
require.NoError(t, err)
require.True(t, dealState.AnnounceToIPNI)
}
38 changes: 27 additions & 11 deletions db/migrations_tests/deals_fast_retrieval_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,25 +22,41 @@ func TestDealFastRetrieval(t *testing.T) {
req.NoError(goose.SetDialect("sqlite3"))
req.NoError(goose.UpTo(sqldb, ".", 20221124191002))

// Generate 2 deals
dealsDB := db.NewDealsDB(sqldb)
// Generate 1 deal
deals, err := db.GenerateNDeals(1)
req.NoError(err)

// Insert the deals in DB
err = dealsDB.Insert(ctx, &deals[0])
require.NoError(t, err)
deal := deals[0]

_, err = sqldb.Exec(`INSERT INTO Deals ("ID", "CreatedAt", "DealProposalSignature", "PieceCID", "PieceSize",
"VerifiedDeal", "IsOffline", "ClientAddress", "ProviderAddress","Label", "StartEpoch", "EndEpoch",
"StoragePricePerEpoch", "ProviderCollateral", "ClientCollateral", "ClientPeerID", "DealDataRoot",
"InboundFilePath", "TransferType", "TransferParams", "TransferSize", "ChainDealID", "PublishCID",
"SectorID", "Offset", "Length", "Checkpoint", "CheckpointAt", "Error", "Retry", "SignedProposalCID")
VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)`,
deal.DealUuid, deal.CreatedAt, []byte("test"), deal.ClientDealProposal.Proposal.PieceCID.String(),
deal.ClientDealProposal.Proposal.PieceSize, deal.ClientDealProposal.Proposal.VerifiedDeal, deal.IsOffline,
deal.ClientDealProposal.Proposal.Client.String(), deal.ClientDealProposal.Proposal.Provider.String(), "test",
deal.ClientDealProposal.Proposal.StartEpoch, deal.ClientDealProposal.Proposal.EndEpoch, deal.ClientDealProposal.Proposal.StoragePricePerEpoch.Uint64(),
deal.ClientDealProposal.Proposal.ProviderCollateral.Int64(), deal.ClientDealProposal.Proposal.ClientCollateral.Uint64(), deal.ClientPeerID.String(),
deal.DealDataRoot.String(), deal.InboundFilePath, deal.Transfer.Type, deal.Transfer.Params, deal.Transfer.Size, deal.ChainDealID,
deal.PublishCID.String(), deal.SectorID, deal.Offset, deal.Length, deal.Checkpoint, deal.CheckpointAt, deal.Err, deal.Retry, []byte("test"))

// Get deal state
dealState, err := dealsDB.ByID(ctx, deals[0].DealUuid)
require.NoError(t, err)
require.False(t, dealState.FastRetrieval)

//Run migration
req.NoError(goose.UpByOne(sqldb, "."))

// Check the deal state again
dealState, err = dealsDB.ByID(ctx, deals[0].DealUuid)
rows, err := sqldb.Query("SELECT FastRetrieval FROM Deals WHERE ID =?", deal.DealUuid)
require.NoError(t, err)
require.True(t, dealState.FastRetrieval)
defer rows.Close()

for rows.Next() {

var ft bool

err = rows.Scan(&ft)
require.NoError(t, err)
require.True(t, ft)
}
}
24 changes: 15 additions & 9 deletions db/migrations_tests/storagetagged_set_host_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,6 @@ func TestStorageTaggedSetHost(t *testing.T) {
req.NoError(goose.SetDialect("sqlite3"))
req.NoError(goose.UpTo(sqldb, ".", 20220908122510))

// Generate 2 deals
dealsDB := db.NewDealsDB(sqldb)

// Add FastRetrieval to allow tests to works
_, err := sqldb.Exec(`ALTER TABLE Deals ADD FastRetrieval BOOL`)
require.NoError(t, err)

deals, err := db.GenerateNDeals(2)
req.NoError(err)

Expand All @@ -44,8 +37,21 @@ func TestStorageTaggedSetHost(t *testing.T) {
Params: []byte(fmt.Sprintf(`{"url":"http://%s/file.car"}`, getHost(i))),
Size: uint64(1024),
}
err = dealsDB.Insert(ctx, &deal)
req.NoError(err)
_, err = sqldb.Exec(`INSERT INTO Deals ("ID", "CreatedAt", "DealProposalSignature", "PieceCID", "PieceSize",
"VerifiedDeal", "IsOffline", "ClientAddress", "ProviderAddress","Label", "StartEpoch", "EndEpoch",
"StoragePricePerEpoch", "ProviderCollateral", "ClientCollateral", "ClientPeerID", "DealDataRoot",
"InboundFilePath", "TransferType", "TransferParams", "TransferSize", "ChainDealID", "PublishCID",
"SectorID", "Offset", "Length", "Checkpoint", "CheckpointAt", "Error", "Retry", "SignedProposalCID")
VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)`,
deal.DealUuid, deal.CreatedAt, []byte("test"), deal.ClientDealProposal.Proposal.PieceCID.String(),
deal.ClientDealProposal.Proposal.PieceSize, deal.ClientDealProposal.Proposal.VerifiedDeal, deal.IsOffline,
deal.ClientDealProposal.Proposal.Client.String(), deal.ClientDealProposal.Proposal.Provider.String(), "test",
deal.ClientDealProposal.Proposal.StartEpoch, deal.ClientDealProposal.Proposal.EndEpoch, deal.ClientDealProposal.Proposal.StoragePricePerEpoch.Uint64(),
deal.ClientDealProposal.Proposal.ProviderCollateral.Int64(), deal.ClientDealProposal.Proposal.ClientCollateral.Uint64(), deal.ClientPeerID.String(),
deal.DealDataRoot.String(), deal.InboundFilePath, deal.Transfer.Type, deal.Transfer.Params, deal.Transfer.Size, deal.ChainDealID,
deal.PublishCID.String(), deal.SectorID, deal.Offset, deal.Length, deal.Checkpoint, deal.CheckpointAt, deal.Err, deal.Retry, []byte("test"))

require.NoError(t, err)
}

// Simulate tagging a deal
Expand Down
9 changes: 6 additions & 3 deletions documentation/en/api-v1-methods.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,8 @@ Response:
"Err": "string value",
"Retry": "auto",
"NBytesReceived": 9,
"FastRetrieval": true
"FastRetrieval": true,
"AnnounceToIPNI": true
}
```

Expand Down Expand Up @@ -464,7 +465,8 @@ Response:
"Err": "string value",
"Retry": "auto",
"NBytesReceived": 9,
"FastRetrieval": true
"FastRetrieval": true,
"AnnounceToIPNI": true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These seem wrong...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are auto-generated by the docs tool

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should update the FastRetrieval naming to be aligned with the KeepUnsealedCopy name we are using. @dirkmc - if there's somewhere we are still using FastRetrieval, let's remove it to reduce confusion...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok we can do that - note though that it will be a breaking change, so we would need to change the Propose Storage Deal Protocol major version (rather than just the minor version). This will require a little more work on our end.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry my mistake - these variable names are API methods that are called by the boost client.
I don't believe anyone outside our team is using these API methods so it should be fine to change them. But the naming is unrelated to this PR so I'd suggest we address it in a follow-up PR so that we can merge this one.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that works for me, thank you!

}
```

Expand Down Expand Up @@ -509,7 +511,8 @@ Inputs:
"Params": "Ynl0ZSBhcnJheQ==",
"Size": 42
},
"FastRetrieval": true
"RemoveUnsealedCopy": true,
"SkipIPNIAnnounce": true
}
]
```
Expand Down
8 changes: 8 additions & 0 deletions gql/resolver.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,14 @@ func (dr *dealResolver) IsVerified() bool {
return dr.ProviderDealState.ClientDealProposal.Proposal.VerifiedDeal
}

func (dr *dealResolver) KeepUnsealedCopy() bool {
LexLuthr marked this conversation as resolved.
Show resolved Hide resolved
return dr.ProviderDealState.FastRetrieval
}

func (dr *dealResolver) AnnounceToIPNI() bool {
return dr.ProviderDealState.AnnounceToIPNI
}

func (dr *dealResolver) ProposalLabel() (string, error) {
l := dr.ProviderDealState.ClientDealProposal.Proposal.Label
if l.IsString() {
Expand Down
2 changes: 2 additions & 0 deletions gql/schema.graphql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ type Deal {
PieceCid: String!
PieceSize: Uint64!
IsVerified: Boolean!
AnnounceToIPNI: Boolean!
KeepUnsealedCopy: Boolean!
Comment on lines +48 to +49
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't the API match the protocol?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just for display on the front end, and again I think it's clearer to use a positive name for booleans

ProposalLabel: String!
ProviderCollateral: Uint64!
ClientCollateral: Uint64!
Expand Down
3 changes: 2 additions & 1 deletion itests/framework/framework.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -541,7 +541,8 @@ func (f *TestFramework) MakeDummyDeal(dealUuid uuid.UUID, carFilepath string, ro
Params: transferParamsJSON,
Size: uint64(carFileinfo.Size()),
},
FastRetrieval: true,
RemoveUnsealedCopy: false,
SkipIPNIAnnounce: false,
}

return f.Client.StorageDeal(f.ctx, dealParams, peerID)
Expand Down
8 changes: 8 additions & 0 deletions react/src/DealDetail.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,14 @@ export function DealDetail(props) {
<th>Verified</th>
<td>{deal.IsVerified ? 'Yes' : 'No'}</td>
</tr>
<tr>
<th>Keep Unsealed Copy</th>
<td>{deal.KeepUnsealedCopy ? 'Yes' : 'No'}</td>
</tr>
<tr>
<th>Announce To IPNI</th>
<td>{deal.AnnounceToIPNI ? 'Yes' : 'No'}</td>
</tr>
<tr>
<th>Piece CID</th>
<td><Link to={'/inspect/'+deal.PieceCid}>{deal.PieceCid}</Link></td>
Expand Down
6 changes: 6 additions & 0 deletions react/src/gql.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@ const DealsListQuery = gql`
ClientAddress
Checkpoint
CheckpointAt
AnnounceToIPNI
KeepUnsealedCopy
IsOffline
Err
Retry
Expand Down Expand Up @@ -166,6 +168,8 @@ const DealSubscription = gql`
IsOffline
Checkpoint
CheckpointAt
AnnounceToIPNI
KeepUnsealedCopy
Retry
Err
Message
Expand Down Expand Up @@ -314,6 +318,8 @@ const NewDealsSubscription = gql`
CreatedAt
PieceCid
PieceSize
AnnounceToIPNI
KeepUnsealedCopy
ClientAddress
StartEpoch
EndEpoch
Expand Down
25 changes: 17 additions & 8 deletions storagemarket/deal_execution.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,11 @@ func (p *Provider) execDealUptoAddPiece(ctx context.Context, deal *types.Provide
err.error = fmt.Errorf("failed to add index and announce deal: %w", err.error)
return err
}
p.dealLogger.Infow(deal.DealUuid, "deal successfully indexed and announced")
if deal.AnnounceToIPNI {
p.dealLogger.Infow(deal.DealUuid, "deal successfully indexed and announced")
} else {
p.dealLogger.Infow(deal.DealUuid, "deal successfully indexed")
}
} else {
p.dealLogger.Infow(deal.DealUuid, "deal has already been indexed and announced")
}
Expand Down Expand Up @@ -596,15 +600,20 @@ func (p *Provider) indexAndAnnounce(ctx context.Context, pub event.Emitter, deal

// if the index provider is enabled
if p.ip.Enabled() {
// announce to the network indexer but do not fail the deal if the announcement fails
annCid, err := p.ip.AnnounceBoostDeal(ctx, deal)
if err != nil {
return &dealMakingError{
retry: types.DealRetryAuto,
error: fmt.Errorf("failed to announce deal to network indexer: %w", err),
if deal.AnnounceToIPNI {
// announce to the network indexer but do not fail the deal if the announcement fails,
// just retry the next time boost restarts
annCid, err := p.ip.AnnounceBoostDeal(ctx, deal)
if err != nil {
return &dealMakingError{
retry: types.DealRetryAuto,
error: fmt.Errorf("failed to announce deal to network indexer: %w", err),
}
}
p.dealLogger.Infow(deal.DealUuid, "announced deal to network indexer", "announcement-cid", annCid)
} else {
p.dealLogger.Infow(deal.DealUuid, "didn't announce deal as requested in the deal proposal")
}
p.dealLogger.Infow(deal.DealUuid, "announced deal to network indexer", "announcement-cid", annCid)
} else {
p.dealLogger.Infow(deal.DealUuid, "didn't announce deal because network indexer is disabled")
}
Expand Down
Loading