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

Shed: Add a util to send a batch of messages #7667

Merged
merged 4 commits into from
Jan 4, 2022
Merged

Conversation

arajasek
Copy link
Contributor

@arajasek arajasek commented Nov 23, 2021

eg (not all messages are valid)

Recipient, FIL, Method, Params
t15sk4gsiavj4wxwgdiilzmigddwwded2vo6tt6wq, 1, 0, a1
t1ax46m5gxt47makivrzgnpkxc25nftkd5phpkujy, 1, 0, b1
t1dtvhlbzgqiebfnu4lcejlmldxqm4hq5gquf2nda, 1, 1, nil
t1ezofyps6t4kcoerh2ljemmsl4pyjrryijixvc5a, 1, 0, b1
t1grul3jzzzr5722fmn3macizidayqgswd3qd64hq, 1, 1, b1
t1j7i7qwzvmwrgadslfz5sl26jpym7q72tuc2pipq, 10000, 0, b1
t1kbq37ptnbgmgxscvuaqyq26cpyh2wyqhzkge3xy, 1, 0, b1
t1osv6wuq57c6qf2flbxzzym3hyyj4qdoh2dmbhyq, 1, 0, b1
t1pwgfrr7duqtrkdcnuigccd72p7izqj3dubidgya, 1, 7, nil
t1q7vonc7x47g3addcs2iuestou7uptplfxdg3dqy, 1, 2, b1
t1tn7wuhqyv3xq7as3z4da3pmzwc4myj6tffkj3gq, 1, 0, nil
t1xyx2zgde33piwasw7r4t6tvweda7l4lsc2clr4q, 1, 1, b1
t1yeko3rfkkagtj5pyomkfk7jxthivbaxn7rca5xi, 1, 1, b1

@arajasek arajasek requested a review from a team as a code owner November 23, 2021 00:51

fmt.Printf("sending %s to %s in msg %s\n", value.String(), addr, smsg.Cid())

if i > 0 && i%100 == 0 {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe remove? I'm just vaguely worried about bombarding mempool.

Copy link
Contributor

Choose a reason for hiding this comment

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

Probably ok to keep, can change into a flag if needed

@codecov
Copy link

codecov bot commented Nov 23, 2021

Codecov Report

Merging #7667 (63d7ca3) into master (b72b12a) will decrease coverage by 0.04%.
The diff coverage is 3.79%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #7667      +/-   ##
==========================================
- Coverage   39.44%   39.40%   -0.05%     
==========================================
  Files         654      655       +1     
  Lines       70171    70247      +76     
==========================================
  Hits        27679    27679              
- Misses      37741    37803      +62     
- Partials     4751     4765      +14     
Impacted Files Coverage Δ
cmd/lotus-shed/main.go 0.00% <0.00%> (ø)
cmd/lotus-shed/send-csv.go 0.00% <0.00%> (ø)
chain/messagepool/messagepool.go 58.04% <100.00%> (-0.38%) ⬇️
chain/stmgr/execute.go 86.95% <0.00%> (-4.35%) ⬇️
chain/exchange/peer_tracker.go 66.66% <0.00%> (-4.31%) ⬇️
extern/sector-storage/sched.go 83.95% <0.00%> (-1.65%) ⬇️
chain/vm/vm.go 61.76% <0.00%> (-1.07%) ⬇️
chain/messagepool/selection.go 83.52% <0.00%> (-0.78%) ⬇️
miner/miner.go 56.95% <0.00%> (-0.67%) ⬇️
chain/consensus/filcns/filecoin.go 51.21% <0.00%> (-0.45%) ⬇️
... and 13 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b72b12a...63d7ca3. Read the comment docs.

Copy link
Contributor

@magik6k magik6k left a comment

Choose a reason for hiding this comment

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

It would be really cool if this also optionally supported method/params

cmd/lotus-shed/send-csv.go Outdated Show resolved Hide resolved
cmd/lotus-shed/send-csv.go Show resolved Hide resolved
return xerrors.Errorf("failed to parse value balance: %w", err)
}

smsg, err := api.MpoolPushMessage(ctx, &types.Message{
Copy link
Member

Choose a reason for hiding this comment

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

this should use the batch APIs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the batch api isn't gonna work well, cuz its inputs need to be signed. that means you're gonna either use the same gas params for all msgs (which probably won't work correctly), or individually estimate (tracking nonce yourself), and then batch, which seems ew.

the batch api isn't doing anything that different than just calling MpoolPushMessage individually in a loop, i don't think

Usage: "Utility for sending a batch of balance transfers",
ArgsUsage: "[sender] [csvfile]",
Action: func(cctx *cli.Context) error {
if cctx.NArg() != 2 {
Copy link
Member

Choose a reason for hiding this comment

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

I would personally use the --from flag instead of a positional argument, its standard across a lot of other things in our tooling

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fine, but are you okay with it being mandatory? i hate "default wallet" more with each passing day, especially for something like this.

@arajasek
Copy link
Contributor Author

It would be really cool if this also optionally supported method/params

Done

@ychiaoli18
Copy link
Contributor

ychiaoli18 commented Dec 1, 2021

It would be really cool if this also optionally supported method/params

Done

@arajasek could we provide different params for each row? e.g.

Recipient, FIL, Param
t15sk4gsiavj4wxwgdiilzmigddwwded2vo6tt6wq, 1, Param1
t1ax46m5gxt47makivrzgnpkxc25nftkd5phpkujy, 1, Param2
t1dtvhlbzgqiebfnu4lcejlmldxqm4hq5gquf2nda, 1, Param3
t1ezofyps6t4kcoerh2ljemmsl4pyjrryijixvc5a, 1, Param4
t1grul3jzzzr5722fmn3macizidayqgswd3qd64hq, 1, Param5

The current payment tool I'm building generates a batch of lotus send commands (or a CSV after this PR merges) for the billing team to copy and execute in the terminal, but the system needs a way to identify if the transaction has been paid, so each transaction needs to have a unique identifier in the param so the tool can search it on the Filecoin chain to know if it has been paid.

@arajasek
Copy link
Contributor Author

arajasek commented Dec 5, 2021

It would be really cool if this also optionally supported method/params

Done

@arajasek could we provide different params for each row? e.g.

Recipient, FIL, Param t15sk4gsiavj4wxwgdiilzmigddwwded2vo6tt6wq, 1, Param1 t1ax46m5gxt47makivrzgnpkxc25nftkd5phpkujy, 1, Param2 t1dtvhlbzgqiebfnu4lcejlmldxqm4hq5gquf2nda, 1, Param3 t1ezofyps6t4kcoerh2ljemmsl4pyjrryijixvc5a, 1, Param4 t1grul3jzzzr5722fmn3macizidayqgswd3qd64hq, 1, Param5

The current payment tool I'm building generates a batch of lotus send commands (or a CSV after this PR merges) for the billing team to copy and execute in the terminal, but the system needs a way to identify if the transaction has been paid, so each transaction needs to have a unique identifier in the param so the tool can search it on the Filecoin chain to know if it has been paid.

Sure thing, done -- using hex encoding for params, lemme know if you'd prefer something else / want it to be configable.

@ychiaoli18
Copy link
Contributor

ychiaoli18 commented Dec 7, 2021

@arajasek thank you! Would you mind also printing the message ids? I may write a short script to look at the result and send the message ids to my system, so the ideal usage may look like the command below.

lotus-shed send-csv data.csv | forward-messageid

@arajasek
Copy link
Contributor Author

arajasek commented Dec 7, 2021

@ychiaoli18 The message IDs are being printed out, but in this format

fmt.Printf("sending %s to %s in msg %s\n", msg.Value.String(), msg.To, smsg.Cid())

I think it's important to do that, so that you know which msg id corresponds to which row of the CSV in case some messages get skipped. I can lower the output to be more script-friendly, though if you'd like (something like just row no, msgid).

@ychiaoli18
Copy link
Contributor

@ychiaoli18 The message IDs are being printed out, but in this format

fmt.Printf("sending %s to %s in msg %s\n", msg.Value.String(), msg.To, smsg.Cid())

I think it's important to do that, so that you know which msg id corresponds to which row of the CSV in case some messages get skipped. I can lower the output to be more script-friendly, though if you'd like (something like just row no, msgid).

row no, msgid would be awesome, thanks!

@arajasek
Copy link
Contributor Author

arajasek commented Dec 7, 2021

@ychiaoli18 done!


fmt.Printf("sending %s to %s in msg %s\n", value.String(), addr, smsg.Cid())

if i > 0 && i%100 == 0 {
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably ok to keep, can change into a flag if needed

@magik6k magik6k enabled auto-merge January 4, 2022 15:58
@magik6k magik6k merged commit 100e198 into master Jan 4, 2022
@magik6k magik6k deleted the asr/shed-balances branch January 4, 2022 16:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants