feat(bank/v2): Send function #21606
feat(bank/v2): Send function #21606
Conversation
WalkthroughWalkthroughThe pull request enhances the Cosmos SDK's bank module by integrating an Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant BankKeeper
participant AuthKeeper
User->>BankKeeper: MintCoins(amounts)
BankKeeper->>AuthKeeper: GetModuleAccount(moduleName)
AuthKeeper-->>BankKeeper: Return module account
BankKeeper->>BankKeeper: Validate and Mint Coins
BankKeeper-->>User: Return success
sequenceDiagram
participant User
participant BankKeeper
participant AuthKeeper
User->>BankKeeper: SendCoins(from, to, amounts)
BankKeeper->>AuthKeeper: GetAccount(from)
AuthKeeper-->>BankKeeper: Return sender account
BankKeeper->>AuthKeeper: GetAccount(to)
AuthKeeper-->>BankKeeper: Return receiver account
BankKeeper->>BankKeeper: Validate and Transfer Coins
BankKeeper-->>User: Return success
Possibly related PRs
Suggested labels
Recent review detailsConfiguration used: .coderabbit.yml Files selected for processing (1)
Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
|
@hieuvubk your pull request is missing a changelog! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range, codebase verification and nitpick comments (1)
x/bank/v2/keeper/keeper.go (1)
36-46: Constructor modifications are appropriate for the new functionality.The addition of
AuthKeeperis essential for integrating authorization checks. Consider addressing the TODO comment about whether to addaddressCodecto the environment for clarity.
Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files selected for processing (9)
- x/bank/v2/depinject.go (2 hunks)
- x/bank/v2/keeper/keeper.go (2 hunks)
- x/bank/v2/keeper/keeper_test.go (1 hunks)
- x/bank/v2/testutil/expected_keepers_mocks.go (1 hunks)
- x/bank/v2/testutil/helpers.go (1 hunks)
- x/bank/v2/types/events.go (1 hunks)
- x/bank/v2/types/expected_keepers.go (1 hunks)
- x/bank/v2/types/keys.go (1 hunks)
- x/bank/v2/types/vesting.go (1 hunks)
Files skipped from review due to trivial changes (1)
- x/bank/v2/testutil/expected_keepers_mocks.go
Additional context used
Path-based instructions (8)
x/bank/v2/types/expected_keepers.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/bank/v2/testutil/helpers.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/bank/v2/types/events.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/bank/v2/types/vesting.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/bank/v2/depinject.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/bank/v2/types/keys.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/bank/v2/keeper/keeper_test.go (2)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern
**/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"x/bank/v2/keeper/keeper.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Additional comments not posted (10)
x/bank/v2/types/expected_keepers.go (1)
9-15: Interface AuthKeeper is well-defined and adheres to Go conventions.The
AuthKeeperinterface is clearly defined, with method signatures that are appropriate for the operations they are intended to perform. The use ofcontext.Contextand specific types from the Cosmos SDK (sdk.AccAddress,sdk.AccountI,sdk.ModuleAccountI) enhances the clarity and effectiveness of the interface.x/bank/v2/types/events.go (1)
7-24: Event type and attribute constants are well-defined and appropriately documented.The constants defined in this file are clear and follow Go naming conventions. The note regarding the choice of
EventTypeCoinMintto avoid clashes with the mint module is particularly insightful, demonstrating careful consideration of the broader module ecosystem.x/bank/v2/types/vesting.go (1)
9-32: Well-defined interface for vesting accounts.The
VestingAccountinterface is clearly defined, with methods that cover essential functionalities required for managing vesting accounts. The use ofsdk.Coinsandtime.Timeis appropriate, ensuring that the vesting logic can be accurately implemented based on the blockchain's time.x/bank/v2/depinject.go (2)
35-35: Appropriate addition ofAuthKeepertoModuleInputs.The inclusion of
AuthKeeperin theModuleInputsstruct is a well-considered enhancement that aligns with the module's need to handle authentication-related functionalities. The field is correctly placed and follows good practices for dependency injection.
57-57: Correct integration ofAuthKeeperinProvideModule.The modification to include
in.AuthKeeperin thekeeper.NewKeeperfunction call withinProvideModuleis correctly implemented. This change effectively integrates the authentication keeper into the module's operations, enhancing its capabilities to handle authentication-related functionalities.x/bank/v2/types/keys.go (1)
15-49: Well-implemented constants and variable for enhanced data handling.The introduction of constants such as
StoreKey,MintModuleName, and various prefixes, along with the variableBalanceValueCodec, is well-executed. These additions significantly enhance the module's capability to manage data efficiently and maintain compatibility with previous implementations. The use ofcollections.NewPrefixandcollcodec.NewAltValueCodecis appropriate and aligns with the module's objectives.x/bank/v2/keeper/keeper_test.go (2)
84-122: Test setup is well-structured and appropriate for the scope of the tests.The initialization of components and the use of mocking are well-implemented, ensuring that the tests are isolated and focused on the functionality being tested.
184-213: Module-to-module transfer tests are correctly implemented and comprehensive.The test case effectively checks both error conditions and successful scenarios, ensuring that the functionality behaves as expected.
x/bank/v2/keeper/keeper.go (2)
58-100:MintCoinsmethod implementation is robust and meets functional requirements.The method includes comprehensive checks and validations, ensuring that coins are minted securely and appropriately. The error handling and event emission are well-implemented.
102-160:SendCoinsmethod is correctly implemented and aligns with module requirements.The method effectively handles different scenarios for coin transfers, including validation checks and error handling. The event logging is a crucial aspect for tracking transactions.
x/bank/v2/testutil/helpers.go
Outdated
| // FundAccount is a utility function that funds an account by minting and | ||
| // sending the coins to the address. This should be used for testing purposes | ||
| // only! | ||
| func FundAccount(ctx context.Context, bankKeeper bankkeeper.Keeper, addr string, amounts sdk.Coins) error { | ||
| if err := bankKeeper.MintCoins(ctx, types.MintModuleName, amounts); err != nil { | ||
| return err | ||
| } | ||
|
|
||
| return bankKeeper.SendCoins(ctx, types.MintModuleName, addr, amounts) | ||
| } |
There was a problem hiding this comment.
Function FundAccount is well-implemented for testing purposes.
The FundAccount function is appropriately documented and implemented for testing purposes. It handles errors effectively and uses the context.Context properly. Consider adding more detailed error messages or logging within the function to aid in debugging during test failures.
Consider enhancing error handling and logging:
- if err := bankKeeper.MintCoins(ctx, types.MintModuleName, amounts); err != nil {
- return err
+ if err := bankKeeper.MintCoins(ctx, types.MintModuleName, amounts); err != nil {
+ return fmt.Errorf("failed to mint coins: %w", err)
+ }
+
+ if err := bankKeeper.SendCoins(ctx, types.MintModuleName, addr, amounts); err != nil {
+ return fmt.Errorf("failed to send coins: %w", err)Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // FundAccount is a utility function that funds an account by minting and | |
| // sending the coins to the address. This should be used for testing purposes | |
| // only! | |
| func FundAccount(ctx context.Context, bankKeeper bankkeeper.Keeper, addr string, amounts sdk.Coins) error { | |
| if err := bankKeeper.MintCoins(ctx, types.MintModuleName, amounts); err != nil { | |
| return err | |
| } | |
| return bankKeeper.SendCoins(ctx, types.MintModuleName, addr, amounts) | |
| } | |
| // FundAccount is a utility function that funds an account by minting and | |
| // sending the coins to the address. This should be used for testing purposes | |
| // only! | |
| func FundAccount(ctx context.Context, bankKeeper bankkeeper.Keeper, addr string, amounts sdk.Coins) error { | |
| if err := bankKeeper.MintCoins(ctx, types.MintModuleName, amounts); err != nil { | |
| return fmt.Errorf("failed to mint coins: %w", err) | |
| } | |
| if err := bankKeeper.SendCoins(ctx, types.MintModuleName, addr, amounts); err != nil { | |
| return fmt.Errorf("failed to send coins: %w", err) | |
| } | |
| return nil | |
| } |
x/bank/v2/keeper/keeper_test.go
Outdated
| func (suite *KeeperTestSuite) TestSendCoins_Acount_To_Account() { | ||
| ctx := suite.ctx | ||
| require := suite.Require() | ||
| balances := sdk.NewCoins(newFooCoin(100), newBarCoin(50)) | ||
| sendAmt := sdk.NewCoins(newFooCoin(10), newBarCoin(10)) | ||
|
|
||
| acc0Str, _ := suite.addressCodec.BytesToString(accAddrs[0]) | ||
| acc1Str, _ := suite.addressCodec.BytesToString(accAddrs[1]) | ||
|
|
||
| // Try send with empty balances | ||
| err := suite.bankKeeper.SendCoins(ctx, acc0Str, acc1Str, sendAmt) | ||
| require.Error(err) | ||
|
|
||
| // Set balances for acc0 and then try send to acc1 | ||
| suite.mockFundAccount(accAddrs[0]) | ||
| require.NoError(banktestutil.FundAccount(ctx, suite.bankKeeper, acc0Str, balances)) | ||
| require.NoError(suite.bankKeeper.SendCoins(ctx, acc0Str, acc1Str, sendAmt)) | ||
|
|
||
| // Check balances | ||
| acc0FooBalance := suite.bankKeeper.GetBalance(ctx, accAddrs[0], fooDenom) | ||
| require.Equal(acc0FooBalance.Amount, math.NewInt(90)) | ||
| acc0BarBalance := suite.bankKeeper.GetBalance(ctx, accAddrs[0], barDenom) | ||
| require.Equal(acc0BarBalance.Amount, math.NewInt(40)) | ||
| acc1FooBalance := suite.bankKeeper.GetBalance(ctx, accAddrs[1], fooDenom) | ||
| require.Equal(acc1FooBalance.Amount, math.NewInt(10)) | ||
| acc1BarBalance := suite.bankKeeper.GetBalance(ctx, accAddrs[1], barDenom) | ||
| require.Equal(acc1BarBalance.Amount, math.NewInt(10)) | ||
| } |
There was a problem hiding this comment.
Consider expanding test coverage for edge cases in coin transfers.
While the existing tests cover basic scenarios, adding tests for edge cases such as sending coins to the same account or sending zero amounts would enhance the robustness of the test suite.
x/bank/v2/keeper/keeper.go
Outdated
| // Function take sender & receipient as string. | ||
| // They can be sdk address or module name. | ||
| // An error is returned upon failure. | ||
| func (k Keeper) SendCoins(ctx context.Context, from, to string, amt sdk.Coins) error { |
x/bank/v2/keeper/keeper.go
Outdated
| var err error | ||
|
|
||
| // Detect from & to is address format or module name | ||
| fromAddr, err = sdk.AccAddressFromBech32(from) |
x/bank/v2/keeper/keeper.go
Outdated
| } | ||
| } | ||
|
|
||
| toAddr, err = sdk.AccAddressFromBech32(to) |
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (2)
x/bank/v2/keeper/keeper.go (2)
59-99: LGTM with a minor suggestion!The
MintCoinsfunction is well-implemented with proper checks and error handling. It ensures that only authorized module accounts can mint coins and that the amounts are valid. The function correctly updates the supply and emits an event for tracking the minting operation.Consider adding a log statement at the debug level to log the module account address along with the amount minted. This can be helpful for debugging purposes.
+ k.Logger.Debug("minted coins from module account", "amount", amounts.String(), "from", moduleName, "address", addrStr)
105-151: LGTM with a minor suggestion!The
SendCoinsfunction is well-implemented with proper checks and error handling. It correctly handles both regular addresses and module account addresses. The function ensures that the transfer amounts are valid and that the sender has sufficient funds. It emits an event for tracking the transfer operation.Consider adding a log statement at the debug level to log the sender and recipient addresses along with the transferred amount. This can be helpful for debugging purposes.
+ k.Logger.Debug("transferred coins", "amount", amt.String(), "from", fromAddrString, "to", toAddrString)
Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files selected for processing (3)
- x/bank/v2/keeper/keeper.go (2 hunks)
- x/bank/v2/keeper/keeper_test.go (1 hunks)
- x/bank/v2/testutil/helpers.go (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- x/bank/v2/keeper/keeper_test.go
- x/bank/v2/testutil/helpers.go
Additional context used
Path-based instructions (1)
x/bank/v2/keeper/keeper.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Additional comments not posted (6)
x/bank/v2/keeper/keeper.go (6)
35-35: LGTM!The changes to the
NewKeeperfunction are approved. The addition of theakparameter and its initialization in theKeeperstruct are necessary for the keeper to interact with module accounts for authorization checks.
154-160: LGTM!The
GetSupplyfunction is straightforward and correctly retrieves the supply for a given denomination from the store. It handles the case when the supply is not found by returning a zero coin.
164-170: LGTM!The
GetBalancefunction is straightforward and correctly retrieves the balance of a specific denomination for a given account address. It handles the case when the balance is not found by returning a zero coin.
178-212: LGTM!The
subUnlockedCoinsfunction is well-implemented with proper checks and error handling. It correctly retrieves the balance, checks for sufficient funds, and updates the balance. It emits an event for tracking the coin spent operation.
219-240: LGTM!The
addCoinsfunction is straightforward and correctly retrieves the balance, adds the specified amount, and updates the balance. It emits an event for tracking the coin received operation.
243-250: LGTM!The
setSupplyfunction is straightforward and correctly sets the supply for the given coin. It handles the case of zero coins by removing them from the store to maintain bank invariants and IBC requirements.
x/bank/v2/keeper/keeper.go
Outdated
| } | ||
|
|
||
| // SendCoins transfers amt coins from a sending account to a receiving account. | ||
| // Function take sender & receipient as string. |
There was a problem hiding this comment.
| // Function take sender & receipient as string. | |
| // Function take sender & receipient as []byte. |
x/bank/v2/keeper/keeper.go
Outdated
| func (k Keeper) MintCoins(ctx context.Context, moduleName string, amounts sdk.Coins) error { | ||
|
|
||
| // TODO: Mint restriction | ||
| acc := k.ak.GetModuleAccount(ctx, moduleName) |
There was a problem hiding this comment.
we are trying to remove auth from being a dependency. Im not sure we want to introduce it again here. The module account system is a legacy design we should not bring into0 bank v2 imo.
There was a problem hiding this comment.
then Mint will take input as []byte. We should check if this address have Minter permission. Marked as TODO.
if !acc.HasPermission(authtypes.Minter) {
x/bank/v2/types/expected_keepers.go
Outdated
| sdk "github.com/cosmos/cosmos-sdk/types" | ||
| ) | ||
|
|
||
| // AuthKeeper defines the account contract that must be fulfilled when |
There was a problem hiding this comment.
Let's indeed not require auth at all
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range and nitpick comments (2)
x/bank/v2/keeper/keeper_test.go (2)
87-110: Add more test cases for edge scenarios.The current test covers the basic scenarios of sending coins between accounts, which looks good! However, as mentioned in the previous review, consider adding more test cases to cover edge scenarios such as:
- Sending coins to the same account
- Sending zero amounts
This will further improve the robustness of the test suite.
1-187: Code follows Uber Golang style guide, but test coverage can be improved.I've reviewed the entire test file and can confirm that the code follows the Uber Golang style guide. No deviations were found.
The existing tests provide good coverage for the
SendCoinsfunction, testing various scenarios of sending coins between regular accounts and module accounts. However, I noticed that there are no tests for theMintCoinsfunction, which is also part of the changes in this pull request.To improve the test coverage, please consider adding tests for the
MintCoinsfunction, covering scenarios such as:
- Minting coins to a regular account
- Minting coins to a module account
- Minting zero amount of coins
- Minting invalid denominations of coins
This will ensure that all the changes in the pull request have adequate test coverage.
Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files selected for processing (4)
- x/bank/v2/keeper/keeper.go (4 hunks)
- x/bank/v2/keeper/keeper_test.go (1 hunks)
- x/bank/v2/testutil/helpers.go (1 hunks)
- x/bank/v2/types/expected_keepers.go (1 hunks)
Files skipped from review due to trivial changes (1)
- x/bank/v2/types/expected_keepers.go
Additional context used
Path-based instructions (3)
x/bank/v2/testutil/helpers.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/bank/v2/keeper/keeper_test.go (2)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern
**/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"x/bank/v2/keeper/keeper.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Additional comments not posted (11)
x/bank/v2/testutil/helpers.go (1)
14-16: The past review comment is still valid:Function FundAccount is well-implemented for testing purposes.
The
FundAccountfunction is appropriately documented and implemented for testing purposes. It handles errors effectively and uses thecontext.Contextproperly.Consider enhancing error handling and logging as suggested in the past review comment:
- return bankKeeper.MintCoins(ctx, addr, amounts) + if err := bankKeeper.MintCoins(ctx, types.MintModuleName, amounts); err != nil { + return fmt.Errorf("failed to mint coins: %w", err) + } + + if err := bankKeeper.SendCoins(ctx, types.MintModuleName, addr, amounts); err != nil { + return fmt.Errorf("failed to send coins: %w", err) + } + + return nilx/bank/v2/keeper/keeper_test.go (3)
112-135: LGTM!The test covers the necessary scenarios for sending coins from an account to a module account. The assertions look good.
137-161: LGTM!The test covers the necessary scenarios for sending coins from a module account to a regular account. It correctly checks the failure case when sending from an account with insufficient balance and the success case when sending from an account with sufficient balance. The assertions look good.
163-187: LGTM!The test covers the necessary scenarios for sending coins from one module account to another. It correctly checks the failure case when sending from a module account with insufficient balance and the success case when sending from a module account with sufficient balance. The assertions look good.
x/bank/v2/keeper/keeper.go (7)
Line range hint
33-52: LGTM!The
NewKeeperfunction is correctly initializing the new fieldsbalancesandsupplyin theKeeperstruct. The usage of[]bytefor theauthorityparameter aligns with the past review comment.
126-133: LGTM!The
GetSupplyfunction is correctly retrieving the supply from the store and returning a zero coin if the supply doesn't exist.
135-143: LGTM!The
GetBalancefunction is correctly retrieving the balance from the store and returning a zero coin if the balance doesn't exist.
145-185: LGTM!The
subUnlockedCoinsfunction is correctly retrieving the balance, checking if the resulting balance is negative, updating the balance, emitting a coin spent event, and returning an error if the resulting balance is negative.
187-213: LGTM!The
addCoinsfunction is correctly retrieving the balance, adding the specified amount to it, updating the balance, and emitting a coin received event.
215-223: LGTM!The
setSupplyfunction is correctly removing the supply if the coin is zero and setting the supply if the coin is non-zero.
225-240: LGTM!The
setBalancefunction is correctly validating thebalanceparameter, removing the balance if the coin is zero, setting the balance if the coin is non-zero, and returning an error if thebalanceparameter is invalid.
| // MintCoins creates new coins from thin air and adds it to the module account. | ||
| // An error is returned if the module account does not exist or is unauthorized. | ||
| func (k Keeper) MintCoins(ctx context.Context, addr []byte, amounts sdk.Coins) error { | ||
| // TODO: Mint restriction & permission | ||
|
|
||
| if !amounts.IsValid() { | ||
| return errorsmod.Wrap(sdkerrors.ErrInvalidCoins, amounts.String()) | ||
| } | ||
|
|
||
| err := k.addCoins(ctx, addr, amounts) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| for _, amount := range amounts { | ||
| supply := k.GetSupply(ctx, amount.GetDenom()) | ||
| supply = supply.Add(amount) | ||
| k.setSupply(ctx, supply) | ||
| } | ||
|
|
||
| addrStr, err := k.addressCodec.BytesToString(addr) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| // emit mint event | ||
| return k.EventService.EventManager(ctx).EmitKV( | ||
| types.EventTypeCoinMint, | ||
| event.NewAttribute(types.AttributeKeyMinter, addrStr), | ||
| event.NewAttribute(sdk.AttributeKeyAmount, amounts.String()), | ||
| ) | ||
| } |
There was a problem hiding this comment.
Address the TODO comment.
The MintCoins function looks good overall. It correctly validates the amounts parameter, updates the supply, and emits a minting event. However, I wanted to remind you about the TODO comment regarding the missing mint restriction and permission logic.
Do you want me to implement the mint restriction and permission logic or open a GitHub issue to track this task?
| // SendCoins transfers amt coins from a sending account to a receiving account. | ||
| // Function take sender & receipient as []byte. | ||
| // They can be sdk address or module name. | ||
| // An error is returned upon failure. | ||
| func (k Keeper) SendCoins(ctx context.Context, from, to []byte, amt sdk.Coins) error { | ||
| if !amt.IsValid() { | ||
| return errorsmod.Wrap(sdkerrors.ErrInvalidCoins, amt.String()) | ||
| } | ||
|
|
||
| var err error | ||
| // TODO: Send restriction | ||
|
|
||
| err = k.subUnlockedCoins(ctx, from, amt) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| err = k.addCoins(ctx, to, amt) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| fromAddrString, err := k.addressCodec.BytesToString(from) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| toAddrString, err := k.addressCodec.BytesToString(to) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| return k.EventService.EventManager(ctx).EmitKV( | ||
| types.EventTypeTransfer, | ||
| event.NewAttribute(types.AttributeKeyRecipient, toAddrString), | ||
| event.NewAttribute(types.AttributeKeySender, fromAddrString), | ||
| event.NewAttribute(sdk.AttributeKeyAmount, amt.String()), | ||
| ) | ||
| } |
There was a problem hiding this comment.
Address the TODO comment.
The SendCoins function looks good overall. It correctly validates the amt parameter, updates the balances of the sender and recipient accounts, and emits a transfer event. However, I wanted to remind you about the TODO comment regarding the missing send restriction logic.
Do you want me to implement the send restriction logic or open a GitHub issue to track this task?
… into hieu/bankv2/send
Description
Closes: #21596
SendCoins&MintCoinsto v2Author 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...
!in the type prefix if API or client breaking changeCHANGELOG.mdReviewers 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.
Please see Pull Request Reviewer section in the contributing guide for more information on how to review a pull request.
I have...
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Documentation