chore: remove Miner JSON-RPC methods #1847
Conversation
randy-cro
requested review from
calvinaco and
songgaoye
and removed request for
a team
|
Warning Rate limit exceeded@randy-cro has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 10 minutes and 58 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
WalkthroughUpdates ethermint dependency pins in go.mod and gomod2nix, initializes an AnteCache when constructing the sim app's AnteHandler, and adds a build-tagged placeholder GetObjKVStore method that panics. Changes
Sequence Diagram(s)sequenceDiagram
participant Sim as NewSimApp (sim_test)
participant Cache as AnteCache
participant Ante as AnteHandler
Sim->>Cache: cache.NewAnteCache(0)
Cache-->>Sim: AnteCache instance
Sim->>Ante: construct HandlerOptions{..., AnteCache: instance}
Ante-->>Sim: AnteHandler configured with AnteCache
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1847 +/- ##
===========================================
+ Coverage 16.87% 35.79% +18.91%
===========================================
Files 72 127 +55
Lines 6163 11812 +5649
===========================================
+ Hits 1040 4228 +3188
- Misses 5000 7161 +2161
- Partials 123 423 +300 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
app/sim_test.go (1)
70-81: AnteCache initialization with 0: confirm intended semantics or documentPassing 0 to NewAnteCache likely means “disabled” or “no limit.” If the intent is deterministic sims without caching, add a clarifying comment; if it means “unbounded,” consider a small bounded value to avoid masking cache-related issues.
Apply this small doc diff for clarity:
MaxTxGasWanted: 0, - AnteCache: cache.NewAnteCache(0), + AnteCache: cache.NewAnteCache(0), // 0 disables ante cache in simulations for determinismIf 0 does not disable caching in this version, please pick the appropriate constant/value exposed by Ethermint (and we can update this accordingly).
store/rootmulti/objstore_placeholder.go (1)
13-15: Make panic message actionable for !objstore buildsThe panic is fine for the placeholder, but making it explicit about the build tag helps operators diagnose issues faster. Also add a short doc comment.
+// GetObjKVStore is a stub in !objstore builds. +// Build with '-tags objstore' to enable object-store backed KV stores. func (rs *Store) GetObjKVStore(key types.StoreKey) types.ObjKVStore { - panic("placeholder does not support GetObjKVStore") + panic("GetObjKVStore is unavailable: build with -tags objstore to enable object store support") }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (4)
app/sim_test.go(2 hunks)go.mod(1 hunks)gomod2nix.toml(1 hunks)store/rootmulti/objstore_placeholder.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- gomod2nix.toml
🧰 Additional context used
🧬 Code Graph Analysis (1)
store/rootmulti/objstore_placeholder.go (1)
store/rootmulti/store.go (1)
Store(35-54)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (21)
- GitHub Check: Run golangci-lint
- GitHub Check: build (macos-14)
- GitHub Check: gomod2nix
- GitHub Check: unittest
- GitHub Check: build (ubuntu-latest)
- GitHub Check: build (macos-latest)
- GitHub Check: integration_tests (ibc)
- GitHub Check: integration_tests (ibc_update_client)
- GitHub Check: integration_tests (ibc_rly_gas)
- GitHub Check: integration_tests (upgrade)
- GitHub Check: integration_tests (ica)
- GitHub Check: integration_tests (gas)
- GitHub Check: integration_tests (slow)
- GitHub Check: integration_tests (unmarked)
- GitHub Check: integration_tests (ibc_rly_evm)
- GitHub Check: testground-test
- GitHub Check: integration_tests (gov)
- GitHub Check: Analyze (go)
- GitHub Check: Socket Security: Pull Request Alerts
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (2)
app/sim_test.go (1)
18-18: Import of ante/cache is aligned with updated EthermintLooks good assuming the replaced Ethermint exposes ante/cache in this version.
store/rootmulti/objstore_placeholder.go (1)
17-19: Consistent behavior for unsupported extra storesThis placeholder continues to panic on unrecognized store types, which is consistent with the non-objstore build behavior. No action needed.
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
go.mod (2)
308-308: Regenerate and commit lockfiles after bumping Ethermint.After adjusting the replace, ensure go.sum and gomod2nix.toml are regenerated and committed. This avoids CI drift and fetch failures.
- go: run “go mod tidy && go mod download”
- gomod2nix: regenerate the TOML to capture the new pseudo-version and sums.
You can use:
#!/bin/bash set -euo pipefail echo "Tidying go modules..." go mod tidy go mod download if command -v gomod2nix >/dev/null 2>&1; then echo "Regenerating gomod2nix.toml..." gomod2nix else echo "gomod2nix not installed; please regenerate gomod2nix.toml with your standard workflow." fi echo echo "Diff overview of files likely changed:" git status --porcelain=1 | rg -n 'go\.sum|gomod2nix\.toml|go\.mod' || true
308-308: Document the removal of Miner JSON-RPC methods as a breaking change and scrub docs.Given the Ethermint bump is intended to remove miner_* RPCs, please:
- Add a Breaking Changes entry to CHANGELOG/RELEASE_NOTES.
- Remove or mark deprecated in docs any occurrences of:
- miner_setGasPrice, miner_setExtra, miner_start, miner_stop, miner_setEtherbase.
Run this to find lingering references and check for a changelog note:
#!/bin/bash set -euo pipefail echo "Scanning repo for miner_* RPC references (code, tests, docs):" rg -n -C2 -g '!**/vendor/**' -P '"?miner_(setGasPrice|setExtra|start|stop|setEtherbase)"?' || true echo echo "Looking for Breaking Changes mention:" rg -n -C2 -i 'breaking change|miner_' CHANGELOG* RELEASE* docs || trueIf helpful, I can draft the CHANGELOG snippet and a docs diff for the endpoints page—say the word.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
go.mod(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (21)
- GitHub Check: gomod2nix
- GitHub Check: build (macos-latest)
- GitHub Check: build (ubuntu-latest)
- GitHub Check: build (macos-14)
- GitHub Check: unittest
- GitHub Check: Run golangci-lint
- GitHub Check: integration_tests (ibc_update_client)
- GitHub Check: integration_tests (gov)
- GitHub Check: integration_tests (gas)
- GitHub Check: integration_tests (ibc_timeout)
- GitHub Check: integration_tests (upgrade)
- GitHub Check: integration_tests (unmarked)
- GitHub Check: integration_tests (slow)
- GitHub Check: integration_tests (ica)
- GitHub Check: integration_tests (ibc_rly_evm)
- GitHub Check: integration_tests (ibc)
- GitHub Check: integration_tests (ibc_rly_gas)
- GitHub Check: Analyze (go)
- GitHub Check: Socket Security: Pull Request Alerts
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
randy-cro
force-pushed
the
feat/remove-non-geth-json-rpc
branch
from
6e47825 to
0d0cc12
Compare
2 participants
Description
Details in ethermint PR: crypto-org-chain/ethermint#694 and crypto-org-chain/ethermint#695
👮🏻👮🏻👮🏻 !!!! 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