-
Notifications
You must be signed in to change notification settings - Fork 17
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
chore: bump cometbft and use latest iavl #82
Conversation
WalkthroughThe pull request involves updates to the Changes
Poem
🪧 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
CodeRabbit Configuration File (
|
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: 1
🧹 Nitpick comments (1)
go.mod (1)
279-279
: Consider documenting GRPC version constraintThe GRPC version is pinned to v1.65.0 due to an issue with cosmos/relayer. Consider adding this constraint to your documentation to help future maintainers.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sum
is excluded by!**/*.sum
📒 Files selected for processing (1)
go.mod
(1 hunks)
🔇 Additional comments (1)
go.mod (1)
279-279
: Verify CometBFT version and IAVL changes
The changes align with the PR objectives:
- CometBFT has been updated to use the initia-labs fork with latest commit
- IAVL replacement directive has been removed as intended
Let's verify these changes:
✅ Verification successful
CometBFT fork update verified, IAVL dependency simplified
The verification confirms:
- The CometBFT commit
a905db74416a
exists in the initia-labs fork, confirming the replacement directive is valid - CometBFT is properly imported across multiple files in the codebase
- IAVL import pattern search shows no direct imports, suggesting the dependency is now handled through transitive dependencies as intended
Let's verify one more aspect to ensure the IAVL removal is safe:
CometBFT and IAVL changes verified successfully
The verification confirms:
- CometBFT fork update is valid:
- Commit
a905db74416a
exists in initia-labs/cometbft - The package is properly imported in multiple core files
- Commit
- IAVL dependency is correctly handled:
- No direct imports found in Go files
- Still present as an indirect dependency in go.mod, which is expected as it's likely required by other cosmos-sdk packages
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify CometBFT version and IAVL usage
# Check if IAVL is now using the default version
rg -l "github.com/cosmos/iavl" --type go
# Check CometBFT import usage
rg -l "github.com/cometbft/cometbft" --type go
# Verify the commit exists in the fork
gh api repos/initia-labs/cometbft/commits/a905db74416a --jq '.sha'
Length of output: 332
Script:
#!/bin/bash
# Check go.mod for any remaining IAVL references
rg "github.com/cosmos/iavl" go.mod
# Check if IAVL is a transitive dependency
go list -m all | grep iavl
Length of output: 199
@@ -279,7 +276,7 @@ replace ( | |||
|
|||
// initia custom | |||
replace ( | |||
github.com/cometbft/cometbft => github.com/initia-labs/cometbft v0.0.0-20241217032826-9bad4b012b88 | |||
github.com/cometbft/cometbft => github.com/initia-labs/cometbft v0.0.0-20241224101634-a905db74416a |
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.
💡 Codebase verification
Update dependency: upstream PR is merged but fork is still in use
The upstream PR (strangelove-ventures/cometbft-client#10) is now closed/merged, but the codebase is still using a custom fork. Consider updating to use the official release v0.1.0
which is already listed as a requirement, by removing these lines from go.mod
:
- The replace directive:
github.com/strangelove-ventures/cometbft-client => github.com/initia-labs/cometbft-client v0.0.0-20240924071428-ef115cefa07e
- The comment:
// - https://github.com/strangelove-ventures/cometbft-client/pull/10
🔗 Analysis chain
Check status of pending upstream PR
There's a dependency on a custom fork of cometbft-client due to a pending upstream PR: strangelove-ventures/cometbft-client#10
Let's check if the PR is still pending:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the upstream PR is still open
# Check PR status
gh api repos/strangelove-ventures/cometbft-client/pulls/10 --jq '.state'
# If closed, we should update the comment to remove the custom fork
Length of output: 79
Script:
#!/bin/bash
# Since the PR is closed, let's check if the dependency is still using the fork in go.mod
rg "github.com/strangelove-ventures/cometbft-client" go.mod
Length of output: 323
Description
Closes: #XXXX
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 changeReviewers 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