perf: reduce the number of ValidateDenom calls in bank.SendCoins #20354

tropicaldog · 2024-05-11T15:09:30Z

Description

Partially addresses: #19529

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...

included the correct type prefix in the PR title
confirmed ! in the type prefix if API or client breaking change
targeted the correct branch (see PR Targeting)
provided a link to the relevant issue or specification
reviewed "Files changed" and left comments if necessary
included the necessary unit and integration tests
added a changelog entry to CHANGELOG.md
updated the relevant documentation or specification, including comments for documenting Go code
confirmed all CI checks have passed

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 all author checklist items have been addressed
reviewed state machine logic, API design and naming, documentation is accurate, tests and test coverage

Summary by CodeRabbit

Improvements
- Enhanced error handling for coin transfers and balance adjustments.
- Added validation for coin amounts before minting or burning, ensuring accurate transactions.
Performance Enhancements
- Reduced the number of ValidateDenom calls in coin transfers to improve efficiency.

…tion call

coderabbitai · 2024-05-11T15:09:37Z

Walkthrough

The recent updates to the Cosmos SDK involve refining error handling and validation in the bank module, specifically within the SendCoins, MintCoins, and BurnCoins functions. Validation logic for coin amounts has been centralized to improve efficiency and clarity. Additionally, the number of ValidateDenom calls in SendCoins has been reduced to enhance performance.

Changes

File	Summary
`x/bank/keeper/send.go`	Adjusted error handling by centralizing coin amount validation in `SendCoins`. Updated comments for `addCoins`.
`x/bank/keeper/keeper.go`	Added validation for coin amounts in `MintCoins` and `BurnCoins` functions.
`CHANGELOG.md`	Documented the reduction of `ValidateDenom` calls in `bank.SendCoins` for efficiency.

Sequence Diagram(s) (Beta)

sequenceDiagram
    participant User
    participant BankModule
    participant BaseSendKeeper
    participant BaseKeeper

    User->>BankModule: Request to SendCoins
    BankModule->>BaseSendKeeper: Validate and SendCoins
    BaseSendKeeper->>BaseSendKeeper: Validate coin amounts
    BaseSendKeeper->>BaseKeeper: subUnlockedCoins
    BaseKeeper->>BaseKeeper: Adjust balance
    BaseKeeper-->>BaseSendKeeper: Balance adjusted
    BaseSendKeeper->>BaseKeeper: addCoins
    BaseKeeper->>BaseKeeper: Increase balance
    BaseKeeper-->>BaseSendKeeper: Balance increased
    BaseSendKeeper-->>BankModule: Coins sent
    BankModule-->>User: SendCoins successful

    User->>BankModule: Request to MintCoins
    BankModule->>BaseKeeper: Validate and MintCoins
    BaseKeeper->>BaseKeeper: Validate coin amounts
    BaseKeeper->>BaseKeeper: Mint coins
    BaseKeeper-->>BankModule: Coins minted
    BankModule-->>User: MintCoins successful

    User->>BankModule: Request to BurnCoins
    BankModule->>BaseKeeper: Validate and BurnCoins
    BaseKeeper->>BaseKeeper: Validate coin amounts
    BaseKeeper->>BaseKeeper: Burn coins
    BaseKeeper-->>BankModule: Coins burned
    BankModule-->>User: BurnCoins successful

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?

Share

Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>.
- Generate unit testing code for this file.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai generate unit testing code for this file.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai generate interesting stats about this repository and render them as a table.
- @coderabbitai show all the console.log statements in this repository.
- @coderabbitai read src/utils.ts and generate unit testing code.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
- @coderabbitai help me debug CodeRabbit configuration file.

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 as PR comments)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai full review to full the review from scratch and review all the files again.
@coderabbitai summary to regenerate the summary of the PR.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai configuration to show the current CodeRabbit configuration for the repository.
@coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
Please see the configuration documentation for more information.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

coderabbitai

Actionable comments posted: 0

Review Details

Configuration used: .coderabbit.yml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 0c91044 and 49e146c.

Files selected for processing (2)

types/coin.go (1 hunks)
x/bank/keeper/send.go (3 hunks)

Files skipped from review due to trivial changes (1)

types/coin.go

Additional Context Used

Path-based Instructions (1)

x/bank/keeper/send.go (1)

Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.

Additional comments not posted (3)

x/bank/keeper/send.go (3)

187-189: Validation of amt coins added at the start of SendCoins function enhances early error detection and reduces redundant validation deeper in the call stack.

225-225: Removal of coin validation from subUnlockedCoins function aligns with the goal of reducing redundant validations and clarifies the function's responsibility.

270-270: Updated comment in addCoins function clarifies that coin amounts should be validated prior to being passed, ensuring clear communication of responsibilities.

tac0turtle

I dont think this is safe, as the caller of subunlocked and addcoins does not call validate. I would leave as is and optimise in v2

tropicaldog · 2024-05-14T13:16:02Z

I dont think this is safe, as the caller of subunlocked and addcoins does not call validate. I would leave as is and optimise in v2

Yeah but addCoins and subUnlockedCoins are only use internally in a few methods, so I guess adding the amount validation to these methods seems good?

tac0turtle · 2024-05-14T13:17:43Z

I dont think this is safe, as the caller of subunlocked and addcoins does not call validate. I would leave as is and optimise in v2

Yeah but addCoins and subUnlockedCoins are only use internally in a few methods, so I guess adding the amount validation to these methods seems good?

yea we can look at doing this. Want to give it a stab

coderabbitai

Actionable comments posted: 0

Review Details

Configuration used: .coderabbit.yml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 49e146c and e2923de.

Files selected for processing (1)

x/bank/keeper/keeper.go (2 hunks)

Additional Context Used

Path-based Instructions (1)

x/bank/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 (2)

x/bank/keeper/keeper.go (2)

369-372: Validation check for amounts ensures only valid coins are minted. This is a good safety measure.

412-414: Validation check for amounts ensures only valid coins are burned. This is a good safety measure.

alpe

The changes look good. 🏌️
I check all the callers if the amount is validated to fail with the correct error. This is given (at the moment). There is also a safety net in setBalance that ensures only valid denoms are stored.

☝️ Nevertheless this PR only solves the problem to a certain degree.
The subUnlockedCoins method uses locked := sdk.NewCoin(coin.Denom, lockedCoins.AmountOf(coin.Denom)) which comes with 2 denom checks which could be avoided by locked, ok := lockedCoins.Find(coin.Denom).

Please also see #19529 (comment) before closing the Issue

alpe · 2024-05-21T08:50:18Z

x/bank/keeper/send.go

@@ -218,13 +222,9 @@ func (k BaseSendKeeper) SendCoins(ctx context.Context, fromAddr, toAddr sdk.AccA
 }

 // subUnlockedCoins removes the unlocked amt coins of the given account. An error is
-// returned if the resulting balance is negative or the initial amount is invalid.
+// returned if the resulting balance is negative.


nit: here and SendCoins: some godoc comment about the amount assumptions would be helpful

coderabbitai

Actionable comments posted: 0

Review Details

Configuration used: .coderabbit.yml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between e2923de and 9f839f7.

Files selected for processing (1)

x/bank/keeper/send.go (4 hunks)

Files skipped from review as they are similar to previous changes (1)

x/bank/keeper/send.go

coderabbitai

Actionable comments posted: 0

Review Details

Configuration used: .coderabbit.yml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 9f839f7 and 7f995ab.

Files selected for processing (1)

x/bank/keeper/send.go (4 hunks)

Files skipped from review as they are similar to previous changes (1)

x/bank/keeper/send.go

coderabbitai

Actionable comments posted: 0

Review Details

Configuration used: .coderabbit.yml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 7f995ab and 105a2a7.

Files selected for processing (1)

x/bank/keeper/keeper.go (2 hunks)

Files skipped from review as they are similar to previous changes (1)

x/bank/keeper/keeper.go

facundomedica

LGTM, thank you! Please add a changelog and we'll merge this :)

coderabbitai

Actionable comments posted: 3

Review Details

Configuration used: .coderabbit.yml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 105a2a7 and 046e7cb.

Files selected for processing (1)

CHANGELOG.md (1 hunks)

Additional Context Used

Markdownlint (386)

CHANGELOG.md (386)

70: Expected: 2; Actual: 4
Unordered list indentation

71: Expected: 2; Actual: 4
Unordered list indentation

75: Expected: 2; Actual: 4
Unordered list indentation

76: Expected: 2; Actual: 4
Unordered list indentation

77: Expected: 2; Actual: 4
Unordered list indentation

78: Expected: 2; Actual: 4
Unordered list indentation

83: Expected: 2; Actual: 4
Unordered list indentation

126: Expected: 2; Actual: 4
Unordered list indentation

127: Expected: 2; Actual: 4
Unordered list indentation

128: Expected: 2; Actual: 4
Unordered list indentation

132: Expected: 2; Actual: 4
Unordered list indentation

135: Expected: 2; Actual: 4
Unordered list indentation

136: Expected: 2; Actual: 4
Unordered list indentation

137: Expected: 2; Actual: 4
Unordered list indentation

144: Expected: 2; Actual: 4
Unordered list indentation

154: Expected: 2; Actual: 4
Unordered list indentation

156: Expected: 2; Actual: 4
Unordered list indentation

159: Expected: 2; Actual: 4
Unordered list indentation

178: Expected: 2; Actual: 4
Unordered list indentation

179: Expected: 2; Actual: 4
Unordered list indentation

181: Expected: 2; Actual: 4
Unordered list indentation

182: Expected: 2; Actual: 4
Unordered list indentation

214: Expected: 2; Actual: 4
Unordered list indentation

215: Expected: 2; Actual: 4
Unordered list indentation

216: Expected: 2; Actual: 4
Unordered list indentation

380: Expected: 2; Actual: 4
Unordered list indentation

383: Expected: 2; Actual: 4
Unordered list indentation

405: Expected: 2; Actual: 4
Unordered list indentation

406: Expected: 2; Actual: 4
Unordered list indentation

419: Expected: 2; Actual: 4
Unordered list indentation

451: Expected: 2; Actual: 4
Unordered list indentation

452: Expected: 2; Actual: 4
Unordered list indentation

453: Expected: 2; Actual: 4
Unordered list indentation

454: Expected: 2; Actual: 4
Unordered list indentation

456: Expected: 2; Actual: 4
Unordered list indentation

457: Expected: 2; Actual: 4
Unordered list indentation

458: Expected: 2; Actual: 4
Unordered list indentation

459: Expected: 2; Actual: 4
Unordered list indentation

473: Expected: 2; Actual: 4
Unordered list indentation

475: Expected: 2; Actual: 4
Unordered list indentation

477: Expected: 2; Actual: 4
Unordered list indentation

479: Expected: 2; Actual: 4
Unordered list indentation

482: Expected: 2; Actual: 4
Unordered list indentation

483: Expected: 2; Actual: 4
Unordered list indentation

484: Expected: 2; Actual: 4
Unordered list indentation

492: Expected: 2; Actual: 4
Unordered list indentation

493: Expected: 2; Actual: 4
Unordered list indentation

495: Expected: 2; Actual: 4
Unordered list indentation

496: Expected: 2; Actual: 4
Unordered list indentation

498: Expected: 2; Actual: 4
Unordered list indentation

499: Expected: 2; Actual: 4
Unordered list indentation

500: Expected: 2; Actual: 4
Unordered list indentation

502: Expected: 2; Actual: 4
Unordered list indentation

503: Expected: 2; Actual: 4
Unordered list indentation

511: Expected: 2; Actual: 4
Unordered list indentation

522: Expected: 2; Actual: 4
Unordered list indentation

523: Expected: 2; Actual: 4
Unordered list indentation

524: Expected: 2; Actual: 4
Unordered list indentation

530: Expected: 2; Actual: 4
Unordered list indentation

531: Expected: 2; Actual: 4
Unordered list indentation

532: Expected: 2; Actual: 4
Unordered list indentation

538: Expected: 2; Actual: 4
Unordered list indentation

554: Expected: 2; Actual: 4
Unordered list indentation

555: Expected: 2; Actual: 4
Unordered list indentation

556: Expected: 2; Actual: 4
Unordered list indentation

557: Expected: 2; Actual: 4
Unordered list indentation

558: Expected: 2; Actual: 4
Unordered list indentation

559: Expected: 2; Actual: 4
Unordered list indentation

564: Expected: 2; Actual: 4
Unordered list indentation

565: Expected: 2; Actual: 4
Unordered list indentation

566: Expected: 2; Actual: 4
Unordered list indentation

567: Expected: 2; Actual: 4
Unordered list indentation

574: Expected: 2; Actual: 4
Unordered list indentation

575: Expected: 2; Actual: 4
Unordered list indentation

576: Expected: 2; Actual: 4
Unordered list indentation

610: Expected: 2; Actual: 4
Unordered list indentation

611: Expected: 2; Actual: 4
Unordered list indentation

612: Expected: 2; Actual: 4
Unordered list indentation

613: Expected: 2; Actual: 4
Unordered list indentation

618: Expected: 2; Actual: 4
Unordered list indentation

619: Expected: 2; Actual: 4
Unordered list indentation

756: Expected: 2; Actual: 4
Unordered list indentation

899: Expected: 2; Actual: 4
Unordered list indentation

920: Expected: 2; Actual: 4
Unordered list indentation

923: Expected: 2; Actual: 4
Unordered list indentation

1005: Expected: 2; Actual: 4
Unordered list indentation

1006: Expected: 2; Actual: 4
Unordered list indentation

1007: Expected: 2; Actual: 4
Unordered list indentation

1008: Expected: 2; Actual: 4
Unordered list indentation

1009: Expected: 2; Actual: 4
Unordered list indentation

1010: Expected: 2; Actual: 4
Unordered list indentation

1107: Expected: 2; Actual: 4
Unordered list indentation

1193: Expected: 2; Actual: 4
Unordered list indentation

1239: Expected: 2; Actual: 4
Unordered list indentation

1245: Expected: 2; Actual: 4
Unordered list indentation

1246: Expected: 2; Actual: 4
Unordered list indentation

1247: Expected: 2; Actual: 4
Unordered list indentation

1248: Expected: 2; Actual: 4
Unordered list indentation

1249: Expected: 2; Actual: 4
Unordered list indentation

1250: Expected: 2; Actual: 4
Unordered list indentation

1350: Expected: 2; Actual: 4
Unordered list indentation

1475: Expected: 2; Actual: 4
Unordered list indentation

1476: Expected: 4; Actual: 8
Unordered list indentation

1477: Expected: 4; Actual: 8
Unordered list indentation

1478: Expected: 2; Actual: 4
Unordered list indentation

1479: Expected: 4; Actual: 8
Unordered list indentation

1480: Expected: 4; Actual: 8
Unordered list indentation

1481: Expected: 4; Actual: 8
Unordered list indentation

1482: Expected: 2; Actual: 4
Unordered list indentation

1485: Expected: 2; Actual: 4
Unordered list indentation

1486: Expected: 4; Actual: 8
Unordered list indentation

1487: Expected: 2; Actual: 4
Unordered list indentation

1488: Expected: 4; Actual: 8
Unordered list indentation

1489: Expected: 4; Actual: 8
Unordered list indentation

1490: Expected: 4; Actual: 8
Unordered list indentation

1739: Expected: 2; Actual: 4
Unordered list indentation

1740: Expected: 2; Actual: 4
Unordered list indentation

1741: Expected: 2; Actual: 4
Unordered list indentation

1742: Expected: 2; Actual: 4
Unordered list indentation

1743: Expected: 2; Actual: 4
Unordered list indentation

1744: Expected: 2; Actual: 4
Unordered list indentation

1854: Expected: 2; Actual: 4
Unordered list indentation

2191: Expected: 2; Actual: 4
Unordered list indentation

2192: Expected: 2; Actual: 4
Unordered list indentation

2193: Expected: 2; Actual: 4
Unordered list indentation

2196: Expected: 2; Actual: 4
Unordered list indentation

2197: Expected: 2; Actual: 4
Unordered list indentation

2198: Expected: 2; Actual: 4
Unordered list indentation

2220: Expected: 2; Actual: 4
Unordered list indentation

2221: Expected: 2; Actual: 4
Unordered list indentation

2222: Expected: 2; Actual: 4
Unordered list indentation

2223: Expected: 2; Actual: 4
Unordered list indentation

2224: Expected: 2; Actual: 4
Unordered list indentation

2232: Expected: 2; Actual: 4
Unordered list indentation

2233: Expected: 2; Actual: 4
Unordered list indentation

2234: Expected: 2; Actual: 4
Unordered list indentation

2235: Expected: 2; Actual: 4
Unordered list indentation

2236: Expected: 2; Actual: 4
Unordered list indentation

2238: Expected: 2; Actual: 4
Unordered list indentation

2239: Expected: 2; Actual: 4
Unordered list indentation

2240: Expected: 2; Actual: 4
Unordered list indentation

2567: Expected: 2; Actual: 4
Unordered list indentation

2568: Expected: 2; Actual: 4
Unordered list indentation

2569: Expected: 2; Actual: 4
Unordered list indentation

2570: Expected: 2; Actual: 4
Unordered list indentation

2571: Expected: 2; Actual: 4
Unordered list indentation

2573: Expected: 2; Actual: 4
Unordered list indentation

2575: Expected: 2; Actual: 4
Unordered list indentation

2576: Expected: 2; Actual: 4
Unordered list indentation

2577: Expected: 2; Actual: 4
Unordered list indentation

2578: Expected: 2; Actual: 4
Unordered list indentation

2579: Expected: 2; Actual: 4
Unordered list indentation

2580: Expected: 2; Actual: 4
Unordered list indentation

2582: Expected: 2; Actual: 4
Unordered list indentation

2583: Expected: 2; Actual: 4
Unordered list indentation

2584: Expected: 2; Actual: 4
Unordered list indentation

2587: Expected: 2; Actual: 4
Unordered list indentation

2588: Expected: 2; Actual: 4
Unordered list indentation

2589: Expected: 2; Actual: 4
Unordered list indentation

2590: Expected: 2; Actual: 4
Unordered list indentation

2591: Expected: 2; Actual: 4
Unordered list indentation

2594: Expected: 2; Actual: 4
Unordered list indentation

2597: Expected: 2; Actual: 4
Unordered list indentation

2600: Expected: 2; Actual: 4
Unordered list indentation

2601: Expected: 2; Actual: 4
Unordered list indentation

2604: Expected: 2; Actual: 4
Unordered list indentation

2611: Expected: 2; Actual: 4
Unordered list indentation

2612: Expected: 2; Actual: 4
Unordered list indentation

2613: Expected: 2; Actual: 4
Unordered list indentation

2614: Expected: 2; Actual: 4
Unordered list indentation

2615: Expected: 2; Actual: 4
Unordered list indentation

2617: Expected: 2; Actual: 4
Unordered list indentation

2618: Expected: 2; Actual: 4
Unordered list indentation

2619: Expected: 2; Actual: 4
Unordered list indentation

2620: Expected: 2; Actual: 4
Unordered list indentation

2621: Expected: 2; Actual: 4
Unordered list indentation

2622: Expected: 2; Actual: 4
Unordered list indentation

2623: Expected: 2; Actual: 4
Unordered list indentation

2624: Expected: 2; Actual: 4
Unordered list indentation

2625: Expected: 4; Actual: 8
Unordered list indentation

2628: Expected: 2; Actual: 4
Unordered list indentation

2629: Expected: 2; Actual: 4
Unordered list indentation

2630: Expected: 2; Actual: 4
Unordered list indentation

2631: Expected: 2; Actual: 4
Unordered list indentation

2632: Expected: 2; Actual: 4
Unordered list indentation

2633: Expected: 2; Actual: 4
Unordered list indentation

2640: Expected: 2; Actual: 4
Unordered list indentation

2641: Expected: 2; Actual: 4
Unordered list indentation

2642: Expected: 2; Actual: 4
Unordered list indentation

2643: Expected: 2; Actual: 4
Unordered list indentation

2650: Expected: 2; Actual: 4
Unordered list indentation

2652: Expected: 2; Actual: 4
Unordered list indentation

2654: Expected: 2; Actual: 4
Unordered list indentation

2655: Expected: 2; Actual: 4
Unordered list indentation

2656: Expected: 2; Actual: 4
Unordered list indentation

2657: Expected: 2; Actual: 4
Unordered list indentation

2658: Expected: 2; Actual: 4
Unordered list indentation

2659: Expected: 2; Actual: 4
Unordered list indentation

2660: Expected: 2; Actual: 4
Unordered list indentation

2661: Expected: 2; Actual: 4
Unordered list indentation

2662: Expected: 2; Actual: 4
Unordered list indentation

2663: Expected: 2; Actual: 4
Unordered list indentation

2664: Expected: 2; Actual: 4
Unordered list indentation

2665: Expected: 2; Actual: 4
Unordered list indentation

2666: Expected: 2; Actual: 4
Unordered list indentation

2667: Expected: 4; Actual: 8
Unordered list indentation

2668: Expected: 4; Actual: 8
Unordered list indentation

2669: Expected: 4; Actual: 8
Unordered list indentation

2670: Expected: 2; Actual: 4
Unordered list indentation

2671: Expected: 2; Actual: 4
Unordered list indentation

2672: Expected: 2; Actual: 4
Unordered list indentation

2673: Expected: 2; Actual: 4
Unordered list indentation

2674: Expected: 2; Actual: 4
Unordered list indentation

2675: Expected: 2; Actual: 4
Unordered list indentation

2676: Expected: 2; Actual: 4
Unordered list indentation

2677: Expected: 4; Actual: 8
Unordered list indentation

2678: Expected: 4; Actual: 8
Unordered list indentation

2679: Expected: 4; Actual: 8
Unordered list indentation

2680: Expected: 2; Actual: 4
Unordered list indentation

2682: Expected: 4; Actual: 8
Unordered list indentation

2683: Expected: 4; Actual: 8
Unordered list indentation

2685: Expected: 2; Actual: 4
Unordered list indentation

2686: Expected: 2; Actual: 4
Unordered list indentation

2687: Expected: 4; Actual: 8
Unordered list indentation

2688: Expected: 4; Actual: 8
Unordered list indentation

2689: Expected: 2; Actual: 4
Unordered list indentation

2690: Expected: 2; Actual: 4
Unordered list indentation

2691: Expected: 2; Actual: 4
Unordered list indentation

2694: Expected: 2; Actual: 4
Unordered list indentation

2695: Expected: 2; Actual: 4
Unordered list indentation

2697: Expected: 2; Actual: 4
Unordered list indentation

2698: Expected: 2; Actual: 4
Unordered list indentation

2701: Expected: 2; Actual: 4
Unordered list indentation

2702: Expected: 4; Actual: 8
Unordered list indentation

2703: Expected: 4; Actual: 8
Unordered list indentation

2704: Expected: 4; Actual: 8
Unordered list indentation

2705: Expected: 4; Actual: 8
Unordered list indentation

2706: Expected: 2; Actual: 4
Unordered list indentation

2707: Expected: 2; Actual: 4
Unordered list indentation

2708: Expected: 2; Actual: 4
Unordered list indentation

2710: Expected: 2; Actual: 4
Unordered list indentation

2711: Expected: 2; Actual: 4
Unordered list indentation

2712: Expected: 2; Actual: 4
Unordered list indentation

2718: Expected: 2; Actual: 4
Unordered list indentation

2721: Expected: 2; Actual: 4
Unordered list indentation

2727: Expected: 2; Actual: 4
Unordered list indentation

2735: Expected: 2; Actual: 4
Unordered list indentation

2736: Expected: 4; Actual: 8
Unordered list indentation

2737: Expected: 4; Actual: 8
Unordered list indentation

2738: Expected: 2; Actual: 4
Unordered list indentation

2746: Expected: 2; Actual: 4
Unordered list indentation

2753: Expected: 2; Actual: 4
Unordered list indentation

2754: Expected: 2; Actual: 4
Unordered list indentation

2761: Expected: 2; Actual: 4
Unordered list indentation

2763: Expected: 2; Actual: 4
Unordered list indentation

2767: Expected: 2; Actual: 4
Unordered list indentation

2768: Expected: 2; Actual: 4
Unordered list indentation

2770: Expected: 2; Actual: 4
Unordered list indentation

2778: Expected: 2; Actual: 4
Unordered list indentation

2780: Expected: 2; Actual: 4
Unordered list indentation

2781: Expected: 2; Actual: 4
Unordered list indentation

2787: Expected: 2; Actual: 4
Unordered list indentation

2795: Expected: 2; Actual: 4
Unordered list indentation

2796: Expected: 2; Actual: 4
Unordered list indentation

2797: Expected: 2; Actual: 4
Unordered list indentation

2798: Expected: 2; Actual: 4
Unordered list indentation

2799: Expected: 2; Actual: 4
Unordered list indentation

2800: Expected: 2; Actual: 4
Unordered list indentation

2801: Expected: 2; Actual: 4
Unordered list indentation

2802: Expected: 2; Actual: 4
Unordered list indentation

2803: Expected: 2; Actual: 4
Unordered list indentation

2804: Expected: 2; Actual: 4
Unordered list indentation

2805: Expected: 2; Actual: 4
Unordered list indentation

2806: Expected: 2; Actual: 4
Unordered list indentation

2807: Expected: 2; Actual: 4
Unordered list indentation

2808: Expected: 2; Actual: 4
Unordered list indentation

2810: Expected: 2; Actual: 4
Unordered list indentation

2811: Expected: 2; Actual: 4
Unordered list indentation

2813: Expected: 2; Actual: 4
Unordered list indentation

2814: Expected: 2; Actual: 4
Unordered list indentation

2815: Expected: 4; Actual: 8
Unordered list indentation

2816: Expected: 4; Actual: 8
Unordered list indentation

2817: Expected: 2; Actual: 4
Unordered list indentation

2818: Expected: 2; Actual: 4
Unordered list indentation

2819: Expected: 2; Actual: 4
Unordered list indentation

2820: Expected: 2; Actual: 4
Unordered list indentation

2821: Expected: 4; Actual: 8
Unordered list indentation

2822: Expected: 4; Actual: 8
Unordered list indentation

2824: Expected: 2; Actual: 4
Unordered list indentation

2825: Expected: 2; Actual: 4
Unordered list indentation

2828: Expected: 2; Actual: 4
Unordered list indentation

2829: Expected: 2; Actual: 4
Unordered list indentation

2830: Expected: 2; Actual: 4
Unordered list indentation

2831: Expected: 2; Actual: 4
Unordered list indentation

2832: Expected: 4; Actual: 8
Unordered list indentation

2833: Expected: 4; Actual: 8
Unordered list indentation

2834: Expected: 4; Actual: 8
Unordered list indentation

2835: Expected: 4; Actual: 8
Unordered list indentation

2836: Expected: 4; Actual: 8
Unordered list indentation

2837: Expected: 4; Actual: 8
Unordered list indentation

2838: Expected: 4; Actual: 8
Unordered list indentation

2839: Expected: 4; Actual: 8
Unordered list indentation

2840: Expected: 4; Actual: 8
Unordered list indentation

2841: Expected: 4; Actual: 8
Unordered list indentation

2842: Expected: 2; Actual: 4
Unordered list indentation

2843: Expected: 2; Actual: 4
Unordered list indentation

2848: Expected: 2; Actual: 4
Unordered list indentation

2849: Expected: 2; Actual: 4
Unordered list indentation

2850: Expected: 2; Actual: 4
Unordered list indentation

2851: Expected: 2; Actual: 4
Unordered list indentation

2852: Expected: 2; Actual: 4
Unordered list indentation

2853: Expected: 2; Actual: 4
Unordered list indentation

2855: Expected: 2; Actual: 4
Unordered list indentation

2857: Expected: 2; Actual: 4
Unordered list indentation

2871: Expected: 2; Actual: 4
Unordered list indentation

2872: Expected: 2; Actual: 4
Unordered list indentation

2873: Expected: 2; Actual: 4
Unordered list indentation

2878: Expected: 2; Actual: 4
Unordered list indentation

2879: Expected: 2; Actual: 4
Unordered list indentation

2880: Expected: 2; Actual: 4
Unordered list indentation

2884: Expected: 2; Actual: 4
Unordered list indentation

2885: Expected: 2; Actual: 4
Unordered list indentation

2886: Expected: 2; Actual: 4
Unordered list indentation

2887: Expected: 2; Actual: 4
Unordered list indentation

2888: Expected: 2; Actual: 4
Unordered list indentation

2889: Expected: 2; Actual: 4
Unordered list indentation

2892: Expected: 2; Actual: 4
Unordered list indentation

2893: Expected: 2; Actual: 4
Unordered list indentation

2894: Expected: 2; Actual: 4
Unordered list indentation

2895: Expected: 2; Actual: 4
Unordered list indentation

2896: Expected: 2; Actual: 4
Unordered list indentation

2897: Expected: 2; Actual: 4
Unordered list indentation

2898: Expected: 2; Actual: 4
Unordered list indentation

2899: Expected: 2; Actual: 4
Unordered list indentation

2900: Expected: 2; Actual: 4
Unordered list indentation

2902: Expected: 2; Actual: 4
Unordered list indentation

2904: Expected: 2; Actual: 4
Unordered list indentation

2906: Expected: 2; Actual: 4
Unordered list indentation

2911: Expected: 2; Actual: 4
Unordered list indentation

2912: Expected: 2; Actual: 4
Unordered list indentation

2913: Expected: 2; Actual: 4
Unordered list indentation

2914: Expected: 2; Actual: 4
Unordered list indentation

2915: Expected: 2; Actual: 4
Unordered list indentation

2916: Expected: 4; Actual: 8
Unordered list indentation

2917: Expected: 4; Actual: 8
Unordered list indentation

2918: Expected: 2; Actual: 4
Unordered list indentation

2919: Expected: 4; Actual: 8
Unordered list indentation

2920: Expected: 4; Actual: 8
Unordered list indentation

2921: Expected: 4; Actual: 8
Unordered list indentation

2922: Expected: 2; Actual: 4
Unordered list indentation

2923: Expected: 2; Actual: 4
Unordered list indentation

2924: Expected: 2; Actual: 4
Unordered list indentation

2925: Expected: 4; Actual: 8
Unordered list indentation

2926: Expected: 4; Actual: 8
Unordered list indentation

2927: Expected: 2; Actual: 4
Unordered list indentation

2928: Expected: 2; Actual: 4
Unordered list indentation

2929: Expected: 2; Actual: 4
Unordered list indentation

2930: Expected: 2; Actual: 4
Unordered list indentation

2931: Expected: 2; Actual: 4
Unordered list indentation

188: Expected: 0 or 2; Actual: 1
Trailing spaces

211: Expected: 0 or 2; Actual: 1
Trailing spaces

66: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines

67: null
Lists should be surrounded by blank lines

1657: null
Bare URL used

1687: null
Bare URL used

2634: null
Spaces inside emphasis markers

2722: null
Spaces inside emphasis markers

2724: null
Spaces inside emphasis markers

2729: null
Spaces inside emphasis markers

2731: null
Spaces inside emphasis markers

2741: null
Spaces inside emphasis markers

2743: null
Spaces inside emphasis markers

2748: null
Spaces inside emphasis markers

2756: null
Spaces inside emphasis markers

2773: null
Spaces inside emphasis markers

2775: null
Spaces inside emphasis markers

2783: null
Spaces inside emphasis markers

2789: null
Spaces inside emphasis markers

2859: null
Spaces inside emphasis markers

2862: null
Spaces inside emphasis markers

2865: null
Spaces inside emphasis markers

2867: null
Spaces inside emphasis markers

2907: null
Spaces inside emphasis markers

1028: null
Spaces inside code span elements

2675: null
Spaces inside code span elements

2675: null
Spaces inside code span elements

Path-based Instructions (1)

CHANGELOG.md (1)

Pattern **/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"

coderabbitai · 2024-05-28T12:38:03Z

CHANGELOG.md

@@ -64,7 +64,7 @@ Every module contains its own CHANGELOG.md. Please refer to the module you are i
 * (debug) [#20328](https://github.com/cosmos/cosmos-sdk/pull/20328) Add consensus address for debug cmd.

 ### Improvements
-
+* (bank) [#20354](https://github.com/cosmos/cosmos-sdk/pull/20354) Reduce the number of `ValidateDenom` calls in `bank.SendCoins`.


Ensure lists are surrounded by blank lines for proper markdown formatting.

66a67 > 68a70 >

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.

Suggested change

* (bank) [#20354](https://github.com/cosmos/cosmos-sdk/pull/20354) Reduce the number of `ValidateDenom` calls in `bank.SendCoins`.

* (bank) [#20354](https://github.com/cosmos/cosmos-sdk/pull/20354) Reduce the number of `ValidateDenom` calls in `bank.SendCoins`.

Adjust the unordered list indentation to maintain consistency.

- * (bank) [#20354](https://github.com/cosmos/cosmos-sdk/pull/20354) Reduce the number of `ValidateDenom` calls in `bank.SendCoins`. + * (bank) [#20354](https://github.com/cosmos/cosmos-sdk/pull/20354) Reduce the number of `ValidateDenom` calls in `bank.SendCoins`.

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.

Suggested change

* (bank) [#20354](https://github.com/cosmos/cosmos-sdk/pull/20354) Reduce the number of `ValidateDenom` calls in `bank.SendCoins`.

* (bank) [#20354](https://github.com/cosmos/cosmos-sdk/pull/20354) Reduce the number of `ValidateDenom` calls in `bank.SendCoins`.

coderabbitai · 2024-05-28T12:38:03Z

CHANGELOG.md

@@ -64,7 +64,7 @@
 * (debug) [#20328](https://github.com/cosmos/cosmos-sdk/pull/20328) Add consensus address for debug cmd.

 ### Improvements


Add a blank line above the "Improvements" heading to adhere to markdown best practices.

65a66 >

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.

Suggested change

### Improvements

### Improvements

move ValidateDenom to top level function, to reduce duplicated valida…

49e146c

…tion call

tropicaldog requested a review from a team as a code owner May 11, 2024 15:09

github-actions bot added the C:x/bank label May 11, 2024

tropicaldog changed the title ~~move ValidateDenom to top level function to reduce duplicated validation call~~ perf: reduce the number of ValidateDenom calls in bank.SendCoins May 11, 2024

coderabbitai bot reviewed May 11, 2024

View reviewed changes

Merge branch 'main' into tuan/refactor-bank-send

d401ac5

tac0turtle reviewed May 14, 2024

View reviewed changes

tac0turtle assigned alpe, facundomedica and tac0turtle May 14, 2024

tropicaldog added 2 commits May 14, 2024 20:51

Merge branch 'main' into tuan/refactor-bank-send

0a8ddbb

add amount validation to Mintcoins

e2923de

coderabbitai bot reviewed May 14, 2024

View reviewed changes

Merge branch 'main' into tuan/refactor-bank-send

51ba23b

tropicaldog requested a review from tac0turtle May 16, 2024 04:08

alpe approved these changes May 21, 2024

View reviewed changes

tropicaldog added 2 commits May 21, 2024 19:14

further remove redundant ValidateDenom call in subUnlockedCoins

9f839f7

add comment

7f995ab

coderabbitai bot reviewed May 21, 2024

View reviewed changes

Merge branch 'main' into tuan/refactor-bank-send

105a2a7

coderabbitai bot reviewed May 24, 2024

View reviewed changes

facundomedica approved these changes May 28, 2024

View reviewed changes

add changelog

046e7cb

coderabbitai bot reviewed May 28, 2024

View reviewed changes

tac0turtle added this pull request to the merge queue May 28, 2024

Merged via the queue into cosmos:main with commit 6a07568 May 28, 2024
64 of 65 checks passed

tac0turtle deleted the tuan/refactor-bank-send branch May 28, 2024 20:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

perf: reduce the number of ValidateDenom calls in bank.SendCoins #20354

perf: reduce the number of ValidateDenom calls in bank.SendCoins #20354

tropicaldog commented May 11, 2024 •

edited by coderabbitai bot

coderabbitai bot commented May 11, 2024 •

edited

Walkthrough

Changes

Sequence Diagram(s) (Beta)

Chat

CodeRabbit Commands (invoked as PR comments)

CodeRabbit Configration File (`.coderabbit.yaml`)

Documentation and Community

coderabbitai bot left a comment

tac0turtle left a comment

tropicaldog commented May 14, 2024

tac0turtle commented May 14, 2024

coderabbitai bot left a comment

alpe left a comment

alpe May 21, 2024

coderabbitai bot left a comment

coderabbitai bot left a comment

coderabbitai bot left a comment

facundomedica left a comment •

edited

coderabbitai bot left a comment

coderabbitai bot May 28, 2024

coderabbitai bot May 28, 2024

	* (bank) [#20354](https://github.com/cosmos/cosmos-sdk/pull/20354) Reduce the number of `ValidateDenom` calls in `bank.SendCoins`.
	* (bank) [#20354](https://github.com/cosmos/cosmos-sdk/pull/20354) Reduce the number of `ValidateDenom` calls in `bank.SendCoins`.

		@@ -64,7 +64,7 @@
		* (debug) [#20328](https://github.com/cosmos/cosmos-sdk/pull/20328) Add consensus address for debug cmd.

		### Improvements

perf: reduce the number of ValidateDenom calls in bank.SendCoins #20354

perf: reduce the number of ValidateDenom calls in bank.SendCoins #20354

Conversation

tropicaldog commented May 11, 2024 • edited by coderabbitai bot

Description

Author Checklist

Reviewers Checklist

Summary by CodeRabbit

coderabbitai bot commented May 11, 2024 • edited

Walkthrough

Changes

Sequence Diagram(s) (Beta)

Chat

CodeRabbit Commands (invoked as PR comments)

CodeRabbit Configration File (.coderabbit.yaml)

Documentation and Community

coderabbitai bot left a comment

Choose a reason for hiding this comment

tac0turtle left a comment

Choose a reason for hiding this comment

tropicaldog commented May 14, 2024

tac0turtle commented May 14, 2024

coderabbitai bot left a comment

Choose a reason for hiding this comment

alpe left a comment

Choose a reason for hiding this comment

alpe May 21, 2024

Choose a reason for hiding this comment

coderabbitai bot left a comment

Choose a reason for hiding this comment

coderabbitai bot left a comment

Choose a reason for hiding this comment

coderabbitai bot left a comment

Choose a reason for hiding this comment

facundomedica left a comment • edited

Choose a reason for hiding this comment

coderabbitai bot left a comment

Choose a reason for hiding this comment

coderabbitai bot May 28, 2024

Choose a reason for hiding this comment

coderabbitai bot May 28, 2024

Choose a reason for hiding this comment

tropicaldog commented May 11, 2024 •

edited by coderabbitai bot

coderabbitai bot commented May 11, 2024 •

edited

CodeRabbit Configration File (`.coderabbit.yaml`)

facundomedica left a comment •

edited