fix(x/auth): simulation multisig signature verification#19148
Conversation
WalkthroughWalkthroughThe changes focus on fixing a bug related to gas estimation for multisig.LegacyAmino accounts during simulation in the Cosmos SDK. A conditional check for a nil Changes
Assessment against linked issues
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
This comment has been minimized.
This comment has been minimized.
|
@likhita-809 can you help us with this review please? |
Added one test |
Seems like the test is not covering the newly added fix |
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Configuration used: .coderabbit.yml
Files selected for processing (2)
- x/auth/go.mod (4 hunks)
- x/auth/go.sum (6 hunks)
Additional comments: 4
x/auth/go.mod (3)
- 42-42: The update of
github.com/99designs/keyringto version1.2.2is noted. Ensure compatibility with this version as it might introduce breaking changes or new features that could affect the project.- 64-64: The update of
github.com/danieljoos/wincredto version1.2.0is observed. Verify that this version is compatible with the project's requirements, especially if it's used for credential storage or retrieval in Windows environments.- 77-77: The update of
github.com/go-kit/kitto version0.13.0is noted. Given thatgo-kitis a foundational library for microservices, ensure that this update does not introduce any regressions or incompatibilities with existing service implementations.x/auth/go.sum (1)
- 4-9: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1-1]
The
go.sumfile has been modified to remove specific versions of modulescosmossdk.io/coreandcosmossdk.io/x/tx. Ensure that these removals do not affect the build or any dependencies that require these specific versions.
| cosmossdk.io/core => ../../core | ||
| cosmossdk.io/depinject => ../../depinject | ||
| cosmossdk.io/x/bank => ../bank | ||
| cosmossdk.io/x/staking => ../staking | ||
| cosmossdk.io/x/tx => ../tx |
There was a problem hiding this comment.
The module replacements for cosmossdk.io/core, cosmossdk.io/depinject, cosmossdk.io/x/bank, cosmossdk.io/x/staking, and cosmossdk.io/x/tx point to local directories. This setup is typically used for local development or testing. Confirm that these replacements are intended for inclusion in the PR and not accidentally committed. If these are temporary for development, they should be removed before merging to maintain module resolution integrity for other developers and in production environments.


Description
Closes:
#18441
Adds signature verification gas for multisig simulation tx
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...
!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.
I have...
Summary by CodeRabbit