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

Jump to bottom

chore: refactor allocate command #1912

Merged
merged 2 commits into from
Apr 24, 2024
Merged

chore: refactor allocate command #1912

merged 2 commits into from
Apr 24, 2024

Conversation

LexLuthr
Copy link
Collaborator

@LexLuthr LexLuthr commented Apr 15, 2024 •
edited
Loading

Allows using a file to allocate in CSV format. The delimiter is ","

Fixes #1897

root@03d705eacb40:/app# boost allocate --pf ./text.txt 
about to send message with the following gas costs
max fee:      0.00000000340735296 FIL (absolute maximum amount you are willing to pay to get your transaction confirmed)
gas fee cap:  0.00000000000000012 FIL
gas limit:    28394608
gas premium:  0.000000000000000035 FIL
basefee:      0.0000000000000001 FIL

Proceed? Yes [Y/y] / No [N/n], Ctrl+C (^C) to exit y
2024-04-17T10:06:31.060Z        INFO    boost   boost/direct_deal.go:280        submitted data cap allocation message[s]
2024-04-17T10:06:31.060Z        INFO    boost   boost/direct_deal.go:281        waiting for message to be included in a block
AllocationID  Client  Miner  PieceCid                                                          PieceSize  TermMin  TermMax  
13            1007    1000   baga6ea4seaqpfonfro6j3gcw4uxkxbivlqn2fghx5dpulc6sbi5noz7bcvzmuiq  2048       518400   5256000  
  Expiration: 213775
16            1007    1000   baga6ea4seaqpfonfro6j3gcw4uxkxbivlqn2fghx5dpulc6sbi5noz7bcvzmuiq  2048       518400   5256000  
  Expiration: 213775
18            1007    1006   baga6ea4seaqpfonfro6j3gcw4uxkxbivlqn2fghx5dpulc6sbi5noz7bcvzmuiq  2048       518400   5256000  
  Expiration: 213775
21            1007    1006   baga6ea4seaqpfonfro6j3gcw4uxkxbivlqn2fghx5dpulc6sbi5noz7bcvzmuiq  2048       518400   5256000  
  Expiration: 213775
22            1007    1006   baga6ea4seaqpfonfro6j3gcw4uxkxbivlqn2fghx5dpulc6sbi5noz7bcvzmuiq  2048       518400   5256000  
  Expiration: 213775
24            1007    1006   baga6ea4seaqpfonfro6j3gcw4uxkxbivlqn2fghx5dpulc6sbi5noz7bcvzmuiq  2048       518400   5256000  
  Expiration: 213775
4             1007    1000   baga6ea4seaqpfonfro6j3gcw4uxkxbivlqn2fghx5dpulc6sbi5noz7bcvzmuiq  2048       518400   5256000  
  Expiration: 213775
6             1007    1000   baga6ea4seaqpfonfro6j3gcw4uxkxbivlqn2fghx5dpulc6sbi5noz7bcvzmuiq  2048       518400   5256000  
  Expiration: 213775
7             1007    1000   baga6ea4seaqpfonfro6j3gcw4uxkxbivlqn2fghx5dpulc6sbi5noz7bcvzmuiq  2048       518400   5256000  
  Expiration: 213775
9             1007    1000   baga6ea4seaqpfonfro6j3gcw4uxkxbivlqn2fghx5dpulc6sbi5noz7bcvzmuiq  2048       518400   5256000  
  Expiration: 213775
15            1007    1000   baga6ea4seaqpfonfro6j3gcw4uxkxbivlqn2fghx5dpulc6sbi5noz7bcvzmuiq  2048       518400   5256000  
  Expiration: 213775
20            1007    1006   baga6ea4seaqpfonfro6j3gcw4uxkxbivlqn2fghx5dpulc6sbi5noz7bcvzmuiq  2048       518400   5256000  
  Expiration: 213775
8             1007    1000   baga6ea4seaqpfonfro6j3gcw4uxkxbivlqn2fghx5dpulc6sbi5noz7bcvzmuiq  2048       518400   5256000  
  Expiration: 213775
10            1007    1000   baga6ea4seaqpfonfro6j3gcw4uxkxbivlqn2fghx5dpulc6sbi5noz7bcvzmuiq  2048       518400   5256000  
  Expiration: 213775
11            1007    1000   baga6ea4seaqpfonfro6j3gcw4uxkxbivlqn2fghx5dpulc6sbi5noz7bcvzmuiq  2048       518400   5256000  
  Expiration: 213775
12            1007    1000   baga6ea4seaqpfonfro6j3gcw4uxkxbivlqn2fghx5dpulc6sbi5noz7bcvzmuiq  2048       518400   5256000  
  Expiration: 213775
14            1007    1000   baga6ea4seaqpfonfro6j3gcw4uxkxbivlqn2fghx5dpulc6sbi5noz7bcvzmuiq  2048       518400   5256000  
  Expiration: 213775
17            1007    1006   baga6ea4seaqpfonfro6j3gcw4uxkxbivlqn2fghx5dpulc6sbi5noz7bcvzmuiq  2048       518400   5256000  
  Expiration: 213775
19            1007    1006   baga6ea4seaqpfonfro6j3gcw4uxkxbivlqn2fghx5dpulc6sbi5noz7bcvzmuiq  2048       518400   5256000  
  Expiration: 213775
2             1007    1000   baga6ea4seaqpfonfro6j3gcw4uxkxbivlqn2fghx5dpulc6sbi5noz7bcvzmuiq  2048       518400   5256000  
  Expiration: 213775
23            1007    1006   baga6ea4seaqpfonfro6j3gcw4uxkxbivlqn2fghx5dpulc6sbi5noz7bcvzmuiq  2048       518400   5256000  
  Expiration: 213775
3             1007    1000   baga6ea4seaqpfonfro6j3gcw4uxkxbivlqn2fghx5dpulc6sbi5noz7bcvzmuiq  2048       518400   5256000  
  Expiration: 213775
5             1007    1000   baga6ea4seaqpfonfro6j3gcw4uxkxbivlqn2fghx5dpulc6sbi5noz7bcvzmuiq  2048       518400   5256000  
  Expiration: 213775
root@03d705eacb40:/app# cat text.txt | wc -l
23

@LexLuthr
Copy link
Collaborator Author

@beck-8 @rvagg Please review and let me know if we need to change file format.

@beck-8
Copy link
Contributor

beck-8 commented Apr 15, 2024

looks great

beck-8
beck-8 reviewed Apr 15, 2024
View reviewed changes
Copy link
Contributor

@beck-8 beck-8 left a comment

Choose a reason for hiding this comment

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

default parameters Required need to be changed

cmd/boost/direct_deal.go Outdated
&cli.StringFlag{
Name: "piece-file",
Usage: "file containing piece-info[s] to create the allocation. Each line in the file should be in the format 'pieceCid,pieceSize,miner,tmin,tmax,expiration'",
Required: true,
Copy link
Contributor

Choose a reason for hiding this comment

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

Required shouble true ->false
AND

boost/cmd/boost/direct_deal.go

Line 35 in b363965

Required: true,

boost/cmd/boost/direct_deal.go

Line 41 in b363965

Required: true,

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed

@LexLuthr LexLuthr force-pushed the update-ddo-commands branch from b363965 to 52b46f9 Compare April 15, 2024 13:09
@LexLuthr LexLuthr force-pushed the update-ddo-commands branch from 52b46f9 to 017d69c Compare April 15, 2024 13:10
@LexLuthr
Copy link
Collaborator Author

@beck-8 Can you please also test this as a user? A real world test would reveal any missing features which might make life easier for users.

@beck-8
Copy link
Contributor

beck-8 commented Apr 16, 2024

@beck-8 Can you please also test this as a user? A real world test would reveal any missing features which might make life easier for users.

Sorry, I don't have datacap credit, so I can't do the test.

@LexLuthr LexLuthr requested a review from snadrus April 17, 2024 10:08
snadrus
snadrus requested changes Apr 22, 2024
View reviewed changes
cmd/boost/direct_deal.go
for _, msg := range msgs {
mcid, sent, err := lib.SignAndPushToMpool(cctx, ctx, gapi, n, msg)
if err != nil {
return err
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the user-retry plan if things fail here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Message failure is a little tricky. One way would be to simply print the CIDs we already have sent successfully and users can get the param from those and decode into a list of piece details. Other way would be to print the allocation list and a diff can be used to remove the already published allocation details from the file and then user can retry.
I could print the success list in case of failure but that could be huge depending upon the input file and not a pretty output.

Let me know your thoughts.

Copy link
Contributor

Choose a reason for hiding this comment

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

does this relate to the CSV input thing so that the file drops out and the format could get re-absorbed with a manual run?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I did not understand that comment. Can you clarify?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, error retries are difficult to handle. You can only succeed once, otherwise it will take a lot of time to filter out the wrong entries. This is the cost of using on-chain submissions.

cmd/boost/direct_deal.go Outdated Show resolved Hide resolved
cmd/boost/direct_deal.go Outdated Show resolved Hide resolved
@snadrus snadrus merged commit 49b2de4 into main Apr 24, 2024
21 checks passed
@snadrus snadrus deleted the update-ddo-commands branch April 24, 2024 19:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@beck-8 beck-8 beck-8 left review comments

@snadrus snadrus snadrus approved these changes

Assignees
No one assigned
Labels
None yet
Projects
Boost
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

What is the Params limit of TransferExported method?
3 participants
@LexLuthr @beck-8 @snadrus