feat: implement multi-send transaction command by julienrbrt · Pull Request #11738 · cosmos/cosmos-sdk

julienrbrt · 2022-04-24T00:07:23Z

Description

Closes: #7809

Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.

I have...

confirmed the correct type prefix in the PR title
confirmed ! in the type prefix if API or client breaking change
confirmed all author checklist items have been addressed
reviewed state machine logic
reviewed API design and naming
reviewed documentation is accurate
reviewed tests and test coverage
manually tested (if applicable)

julienrbrt · 2022-04-24T00:36:26Z

types/coin.go

+// {4A} / {8A} = {0A}
+// {2A} / 0 = panics
+// Note, if IsValid was true on Coins, IsValid stays true,
+// unless the `divisor` is greater than the smallest coin amount.


I am uncertain if we actually want this behavior, but I created these helpers function for avoiding having to do for every coin:

sendCoins := coins if split && !coins.IsZero() { sendCoins[0].Amount = coins[0].Amount.Quo(totalAddr) }

If you find this better, then I can modify it and remove the changes in coins.

alexanderbez

👏

types/coin_test.go

x/bank/client/cli/tx.go

x/bank/types/msgs_test.go

x/bank/client/cli/tx.go

types/coin.go

x/bank/client/cli/tx.go

alexanderbez

Nice work! What happens when --split is provided/true, but doesn't have a fully divisible amount? One of the outputs is rounded, correct?

x/bank/client/cli/tx.go

julienrbrt · 2022-04-25T22:02:19Z

Nice work! What happens when --split is provided/true, but doesn't have a fully divisible amount? One of the outputs is rounded, correct?

Thanks for all the feedback :)

Yes, that's why we need to multiply from sendCoins to get the actual amount: #11738 (comment)

alexanderbez

@julienrbrt I completely forgot. We need to add a test to x/bank/client/testutil 👍

types/coin.go

x/bank/client/cli/tx.go

likhita-809

lgtm

codecov · 2022-04-29T07:25:58Z

Codecov Report

Merging #11738 (f573ec9) into main (0c0b4da) will increase coverage by 0.07%.
The diff coverage is 98.27%.

@@            Coverage Diff             @@
##             main   #11738      +/-   ##
==========================================
+ Coverage   65.94%   66.02%   +0.07%     
==========================================
  Files         669      669              
  Lines       70427    70599     +172     
==========================================
+ Hits        46444    46613     +169     
- Misses      21312    21316       +4     
+ Partials     2671     2670       -1

Impacted Files	Coverage Δ
x/auth/client/cli/tx_multisign.go	`0.00% <0.00%> (ø)`
types/coin.go	`93.56% <100.00%> (+0.44%)`	⬆️
x/bank/client/testutil/cli_helpers.go	`73.68% <100.00%> (+23.68%)`	⬆️
x/bank/client/testutil/suite.go	`99.47% <100.00%> (+0.15%)`	⬆️
crypto/keys/internal/ecdsa/privkey.go	`82.45% <0.00%> (-1.76%)`	⬇️
x/group/keeper/keeper.go	`53.36% <0.00%> (-0.43%)`	⬇️

alexanderbez · 2022-04-29T13:33:52Z

@Mergifyio backport release/v0.46.x

Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com> Co-authored-by: Anil Kumar Kammari <anil@vitwit.com> (cherry picked from commit 6a9b824) # Conflicts: # x/bank/client/cli/tx.go

mergify · 2022-04-29T13:34:37Z

backport release/v0.46.x

✅ Backports have been created

Details

#11830 feat: implement multi-send transaction command (backport #11738) has been created for branch release/v0.46.x

)

julienrbrt requested review from aaronc and alexanderbez as code owners April 24, 2022 00:07

julienrbrt linked an issue Apr 24, 2022 that may be closed by this pull request

gaiad tx bank multi-send was not implemented for cli #7809

Closed

4 tasks

github-actions bot added C:CLI C:x/bank labels Apr 24, 2022

julienrbrt changed the title ~~feat: implement multi-send transaction cli~~ feat: implement multi-send transaction command Apr 24, 2022

julienrbrt commented Apr 24, 2022

View reviewed changes

alexanderbez reviewed Apr 25, 2022

View reviewed changes

blushi assigned amaury1093 and likhita-809 Apr 25, 2022

alexanderbez reviewed Apr 25, 2022

View reviewed changes

alexanderbez approved these changes Apr 25, 2022

View reviewed changes

x/bank/client/cli/tx.go Outdated Show resolved Hide resolved

alexanderbez requested a review from tac0turtle April 25, 2022 21:05

julienrbrt and others added 8 commits April 26, 2022 00:02

feat: Implement multi-send transaction

cc3df15

add changelog

0a46ca2

improve quo

9d3d8f5

implement renaming feedback

398e92f

implement feedback

53cd623

implement part of feedback

34e8d8b

create new coin in loop

7236694

Update x/bank/client/cli/tx.go

51ed808

julienrbrt force-pushed the julien/7809-gaiad-tx-bank-multi-send-cli branch from 26e5d22 to 51ed808 Compare April 25, 2022 22:04

Merge branch 'main' into julien/7809-gaiad-tx-bank-multi-send-cli

da99dde

julienrbrt changed the title ~~feat: implement multi-send transaction command~~ feat: implement multisend transaction command Apr 26, 2022

alexanderbez suggested changes Apr 26, 2022

View reviewed changes

julienrbrt changed the title ~~feat: implement multisend transaction command~~ feat: implement multi-send transaction command Apr 26, 2022

likhita-809 reviewed Apr 27, 2022

View reviewed changes

types/coin.go Outdated Show resolved Hide resolved

types/coin.go Outdated Show resolved Hide resolved

x/bank/client/cli/tx.go Outdated Show resolved Hide resolved

x/bank/client/cli/tx.go Show resolved Hide resolved

julienrbrt added 2 commits April 27, 2022 14:18

implement feedback

16bb41e

improve description

e55c7e8

Merge branch 'main' into julien/7809-gaiad-tx-bank-multi-send-cli

4c5511d

alexanderbez approved these changes Apr 27, 2022

View reviewed changes

alexanderbez and others added 4 commits April 27, 2022 13:56

Merge branch 'main' into julien/7809-gaiad-tx-bank-multi-send-cli

6e91344

alias for consistency

2431edb

add missing test

1f571da

Merge branch 'main' into julien/7809-gaiad-tx-bank-multi-send-cli

f29c132

github-actions bot added the C:x/auth label Apr 28, 2022

julienrbrt and others added 4 commits April 28, 2022 14:52

rebase fix

a9cbf7b

remove sequence

22b2b07

fix sequence issue

8818b86

Merge branch 'main' into julien/7809-gaiad-tx-bank-multi-send-cli

0d4a978

alexanderbez approved these changes Apr 28, 2022

View reviewed changes

update changelog

5e5260f

likhita-809 approved these changes Apr 29, 2022

View reviewed changes

Merge branch 'main' into julien/7809-gaiad-tx-bank-multi-send-cli

f573ec9

julienrbrt merged commit 6a9b824 into main Apr 29, 2022

julienrbrt deleted the julien/7809-gaiad-tx-bank-multi-send-cli branch April 29, 2022 09:27

mergify bot mentioned this pull request Apr 29, 2022

feat: implement multi-send transaction command (backport #11738) #11830

Merged

alexanderbez pushed a commit that referenced this pull request Apr 29, 2022

feat: implement multi-send transaction command (backport #11738) (#11830

7e4d5d7

)

amaury1093 mentioned this pull request May 20, 2022

Manual Testing for v0.46 #11880

Closed

72 tasks

LampardNguyen234 mentioned this pull request Aug 15, 2023

deps: bump cosmos-sdk v0.46 and evmos v12 AstraProtocol/astra#209

Merged

Conversation

julienrbrt commented Apr 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Author Checklist

Reviewers Checklist

Uh oh!

julienrbrt Apr 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alexanderbez left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alexanderbez left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

julienrbrt commented Apr 25, 2022

Uh oh!

alexanderbez left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

likhita-809 left a comment

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Apr 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

alexanderbez commented Apr 29, 2022

Uh oh!

mergify bot commented Apr 29, 2022

✅ Backports have been created

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

julienrbrt commented Apr 24, 2022 •

edited

Loading

julienrbrt Apr 24, 2022 •

edited

Loading

codecov bot commented Apr 29, 2022 •

edited

Loading