-
Notifications
You must be signed in to change notification settings - Fork 124
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
fix(tx_search): Set cometbft-db
to v0.12
to fix tx_search issue (Long-term fix needed)
#2699
Conversation
WalkthroughThe pull request focuses on updating and replacing dependencies in the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (9)
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
CodeRabbit Configuration File (
|
d86a362
to
4845491
Compare
Can you update the comment to be more specific that all tx search resolves to a single entry? And reference cometbft/cometbft#4295 |
@Mergifyio backport release/protocol/v8.x |
🟠 Waiting for conditions to match
|
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
🧹 Nitpick comments (2)
protocol/go.mod (2)
476-476
: Track the TODO for proper fix.The comment indicates this is a temporary solution. Consider creating a tracking issue if one doesn't exist.
Would you like me to help create a tracking issue for implementing the proper fix by backporting the upstream changes?
474-475
: Document the specific interface change.Consider adding more details about which interface change in v0.13.0+ caused the tx_search issue. This will help with implementing the proper fix later.
Add a comment explaining the specific interface change that caused the issue:
// Fixes the issue that `tx_search` resolves to a single entry, due to an cometbft-db interface -// change in v0.13.0+. +// change in v0.13.0+. Specifically, the change in <interface_name> that affects <specific_component>.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
protocol/go.sum
is excluded by!**/*.sum
📒 Files selected for processing (1)
protocol/go.mod
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: test-sim-import-export
- GitHub Check: test-sim-after-import
- GitHub Check: test-sim-multi-seed-short
- GitHub Check: test-coverage-upload
- GitHub Check: container-tests
- GitHub Check: Summary
🔇 Additional comments (1)
protocol/go.mod (1)
474-477
: Verify the impact of downgrading cometbft-db.While this fixes the tx_search issue, downgrading a dependency could potentially introduce other compatibility issues or miss security fixes from newer versions.
Run the following script to check for any security advisories or breaking changes between v0.12.0 and v0.13.0:
✅ Verification successful
Downgrade to cometbft-db v0.12.0 is appropriate and safe.
The downgrade is a valid temporary fix for the tx_search issue caused by breaking iterator API changes in v0.13.0+. No security vulnerabilities were found in v0.12.0, and the issue is being tracked for a proper fix via backporting (CT-1343).
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for security advisories and breaking changes in cometbft-db # Check for security advisories gh api graphql -f query=' { securityVulnerabilities(first: 5, ecosystem: GO, package: "github.com/cometbft/cometbft-db") { nodes { advisory { summary severity publishedAt } vulnerableVersionRange firstPatchedVersion { identifier } } } }' # Check for breaking changes in commit history between v0.12.0 and v0.13.0 rg -A 5 "BREAKING CHANGE"Length of output: 390
Script:
#!/bin/bash # Check for changes between v0.12.0 and v0.13.0 in cometbft-db # Clone the repository to a temporary directory git clone https://github.com/cometbft/cometbft-db /tmp/cometbft-db cd /tmp/cometbft-db # Check changelog entries git log --oneline v0.12.0..v0.13.0 # Search for tx_search related changes git log -p v0.12.0..v0.13.0 | grep -B 5 -A 5 "tx_search" # Check for issues mentioning tx_search gh search issues "tx_search repo:cometbft/cometbft-db" --json title,url,state,body # Check the diff between versions git diff v0.12.0..v0.13.0Length of output: 51396
Script:
#!/bin/bash # Search for tx_search usage and iterator patterns # Search for tx_search usage rg -A 5 -B 5 "tx_search" # Search for Iterator usage patterns ast-grep --pattern 'iter.$$$Key()' # Search for issues mentioning tx_search gh search issues "tx_search repo:cometbft/cometbft" --json title,url,state,body # Check commit messages mentioning tx_search git log --grep="tx_search"Length of output: 42777
Script:
#!/bin/bash # Check for related fixes or PRs # Search for CT-1343 related commits git log --all --grep="CT-1343" # Search for tx_search related fixes in recent commits git log --since="2024-01-01" --grep="tx_search" # Check open PRs related to tx_search gh search prs "tx_search repo:cometbft/cometbft is:open" --json title,url,body # Check open PRs related to cometbft-db gh search prs "cometbft-db repo:cometbft/cometbft is:open" --json title,url,bodyLength of output: 597
@Mergifyio backport release/protocol/v8.x |
✅ Backports have been created
|
…Long-term fix needed) (backport #2699) (#2701) Co-authored-by: Teddy Ding <[email protected]>
Changelist
Context
Test Plan
[Describe how this PR was tested (if applicable)]
Author/Reviewer Checklist
state-breaking
label.indexer-postgres-breaking
label.PrepareProposal
orProcessProposal
, manually add the labelproposal-breaking
.feature:[feature-name]
.backport/[branch-name]
.refactor
,chore
,bug
.Summary by CodeRabbit
cometbft-db
to versionv0.12.0