-
Notifications
You must be signed in to change notification settings - Fork 234
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
Problem: sig verification result is not cached between incarnations #1515
Conversation
WalkthroughThe recent changes focus on enhancing the performance of the application by caching signature verification results between transaction incarnations. Additionally, several dependencies in the project have been updated to their latest versions, reflecting improvements and fixes from their respective upstream projects. These updates involve changes to Changes
Poem
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 (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1515 +/- ##
===========================================
+ Coverage 17.54% 35.87% +18.33%
===========================================
Files 87 136 +49
Lines 5273 9809 +4536
===========================================
+ Hits 925 3519 +2594
- Misses 4238 5894 +1656
- Partials 110 396 +286 |
Signed-off-by: yihuang <huang@crypto.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
go.sum
is excluded by!**/*.sum
Files selected for processing (3)
- CHANGELOG.md (1 hunks)
- go.mod (2 hunks)
- gomod2nix.toml (2 hunks)
Additional comments not posted (5)
go.mod (2)
260-260
: Verify dependency change for cosmos-sdk.The dependency
github.com/cosmos/cosmos-sdk
has been changed to a forkgithub.com/mmsqe/cosmos-sdk
. This is a significant change as it switches the source of the SDK.Verification successful
Verify dependency change for cosmos-sdk.
The dependency
github.com/cosmos/cosmos-sdk
has been changed to a forkgithub.com/mmsqe/cosmos-sdk
. This is a significant change as it switches the source of the SDK.Let's correct the script to fetch recent commits and check the repository's state.
Verify dependency change for cosmos-sdk.
The dependency
github.com/cosmos/cosmos-sdk
has been changed to a forkgithub.com/mmsqe/cosmos-sdk
. This is a significant change as it switches the source of the SDK.Let's adjust our approach to fetch the commit history correctly.
Verified dependency change for cosmos-sdk.
The dependency
github.com/cosmos/cosmos-sdk
has been changed to a forkgithub.com/mmsqe/cosmos-sdk
. The recent commits indicate active development and maintenance, suggesting that the fork is being actively managed and is likely stable.
- Recent commits include various improvements and refactoring, which indicate ongoing maintenance.
- The repository details show it is public and licensed under Apache License 2.0, which is consistent with open-source standards.
Commit Details:
3781c462a9442a4142d8a38b77e587984d9a4c08
: chore: minor bech32 global removal27210f0e62ae60b346c5d65e2ce051b7daf62ac4
: docs: Improve docs for vote query222d28e1798b845afed622669be572a49b626444
: refactor(x/staking): migrateRedelegationByValSrcIndexKey
key to collections393dcc1f2a60d13488e9d3ea8ff34bd12d5b9026
: refactor: remove header type from NewContextdc62bd7ade471f462f4799a535fb88be768556c9
: build(deps): Bump cosmossdk.io/math from 1.0.1 to 1.1.2Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the new fork's compatibility and check for any open issues or recent commits that might affect stability. # Check recent commits gh repo view mmsqe/cosmos-sdk --json commits --jq '.[0:5]' # Check open issues gh issue list -R mmsqe/cosmos-sdkLength of output: 1275
Script:
#!/bin/bash # Description: Verify the new fork's compatibility and check for any recent commits that might affect stability. # Check recent commits gh repo view mmsqe/cosmos-sdk --json defaultBranchRef --jq '.defaultBranchRef.target.history.nodes[0:5] | {commits: .}' # Check repository details gh repo view mmsqe/cosmos-sdk --json name,description,createdAt,updatedAt,pushedAt,defaultBranchRef,licenseInfo,visibility,stargazerCount,forkCountLength of output: 672
Script:
#!/bin/bash # Description: Verify the new fork's compatibility and check for any recent commits that might affect stability. # Check recent commits using the correct GraphQL query gh api graphql -f query=' { repository(owner: "mmsqe", name: "cosmos-sdk") { defaultBranchRef { target { ... on Commit { history(first: 5) { edges { node { message committedDate oid } } } } } } } }'Length of output: 1625
280-280
: Updated dependency forgithub.com/evmos/ethermint
.The version of
github.com/evmos/ethermint
has been updated to a newer commit. Ensure this update is compatible with other dependencies and does not introduce any breaking changes.gomod2nix.toml (2)
173-175
: Updated dependency forgithub.com/cosmos/cosmos-sdk
.The version has been updated to
v0.46.0-beta2.0.20240715072401-d2de62e1575e
and the source has been changed fromgithub.com/crypto-org-chain/cosmos-sdk
togithub.com/mmsqe/cosmos-sdk
. This change should be verified for compatibility and integration with other system components.Ensure that the new version integrates well with existing system components and does not introduce breaking changes.
265-266
: Updated dependency forgithub.com/evmos/ethermint
.The version has been updated to
v0.6.1-0.20240715061533-9c959a26e04f
and the source has been changed fromgithub.com/crypto-org-chain/ethermint
togithub.com/evmos/ethermint
. It is important to verify that this new version does not introduce any incompatibilities or regressions.Assess the impact of this version change on the overall system performance and stability.
CHANGELOG.md (1)
26-26
: Changelog Entry ReviewThe changelog entry correctly summarizes the improvements made by PR #1515, which involves caching the signature verification results to enhance performance. This aligns well with the PR's objectives and the description provided.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (1)
go.mod (1)
259-259
: Issue Found: Invalid or Missing Version forgithub.com/cosmos/cosmos-sdk
The specified version
v0.50.6-0.20240716063309-c47504d189d4
for thegithub.com/cosmos/cosmos-sdk
dependency does not exist in the repository. Please verify the version and ensure it is correctly specified or pushed to the remote repository.
- File:
go.mod
- Line: 259
Analysis chain
Updated dependency:
github.com/cosmos/cosmos-sdk
.The dependency has been updated to a specific commit. Verify the stability and appropriateness of this version for the project's requirements.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check the commit log for the specific version of `cosmos-sdk` to ensure it includes necessary fixes or features. # Test: Fetch the commit log for the specified version. Expect: Relevant fixes or features. git log --oneline github.com/crypto-org-chain/cosmos-sdk@v0.50.6-0.20240716063309-c47504d189d4Length of output: 353
Script:
#!/bin/bash # Description: List the tags and recent commits of the `github.com/crypto-org-chain/cosmos-sdk` repository to verify the specified version. # Test: List tags in the repository. git ls-remote --tags https://github.com/crypto-org-chain/cosmos-sdk.git # Test: List recent commits in the repository. git ls-remote --heads https://github.com/crypto-org-chain/cosmos-sdk.gitLength of output: 96470
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
go.sum
is excluded by!**/*.sum
Files selected for processing (2)
- go.mod (3 hunks)
- gomod2nix.toml (3 hunks)
Additional comments not posted (3)
go.mod (1)
279-279
: Updated dependency:github.com/evmos/ethermint
.The dependency has been updated to a specific commit. Verify the stability and appropriateness of this version for the project's requirements.
gomod2nix.toml (2)
173-174
: Updated dependency:github.com/cosmos/cosmos-sdk
The version and hash of
github.com/cosmos/cosmos-sdk
have been updated. Ensure that this new version is compatible with other project dependencies and that it includes all necessary security patches.Verification successful
Updated dependency:
github.com/cosmos/cosmos-sdk
The version and hash of
github.com/cosmos/cosmos-sdk
have been updated to a forked version hosted atgithub.com/crypto-org-chain/cosmos-sdk
. Ensure that this new version is compatible with other project dependencies and that it includes all necessary security patches.
go.mod
confirms the presence and replacement ofgithub.com/cosmos/cosmos-sdk
withgithub.com/crypto-org-chain/cosmos-sdk
versionv0.50.6-0.20240716063309-c47504d189d4
.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the new version of cosmos-sdk does not introduce breaking changes or dependency conflicts ast-grep --lang go --pattern $'import("github.com/cosmos/cosmos-sdk")'Length of output: 1278
Script:
#!/bin/bash # Search for `github.com/cosmos/cosmos-sdk` in the `go.mod` file to verify its presence and version rg 'github.com/cosmos/cosmos-sdk' go.modLength of output: 329
265-266
: Updated dependency:github.com/evmos/ethermint
The version and hash of
github.com/evmos/ethermint
have been updated. It's important to verify that this update does not conflict with other dependencies, especially given the significant role ofethermint
in the project.
Benchmark Results
100 accounts on x86 mac (run by @mmsqe)
1 unique account (Worst Case)
100 unique accounts
10 unique accounts
👮🏻👮🏻👮🏻 !!!! REFERENCE THE PROBLEM YOUR ARE SOLVING IN THE PR TITLE AND DESCRIBE YOUR SOLUTION HERE !!!! DO NOT FORGET !!!! 👮🏻👮🏻👮🏻
PR Checklist:
make
)make test
)go fmt
)golangci-lint run
)go list -json -m all | nancy sleuth
)Thank you for your code, it's appreciated! :)
Summary by CodeRabbit
Performance Improvements
Dependency Updates
cosmos-sdk
to versionv0.50.6
.ethermint
to versionv0.6.1
.