-
Couldn't load subscription status.
- Fork 252
feat: bump ledger dependencies for latest fw support #2010
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
Conversation
WalkthroughUpdates to Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Potential focus areas:
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.5.0)Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions Comment |
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
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (1)
go.mod(7 hunks)
🧰 Additional context used
🪛 OSV Scanner (2.2.3)
go.mod
[HIGH] 39-39: google.golang.org/grpc 1.33.2: Denial of service from HTTP/2 Rapid Reset in google.golang.org/grpc
(GO-2023-2153)
[HIGH] 39-39: google.golang.org/grpc 1.33.2: gRPC-Go HTTP/2 Rapid Reset vulnerability
|
Marked as stale; will be closed in five days. |
Signed-off-by: Artur Troian <[email protected]>
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
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (1)
go.mod(7 hunks)
🧰 Additional context used
🪛 OSV Scanner (2.2.3)
go.mod
[HIGH] 39-39: google.golang.org/grpc 1.33.2: Denial of service from HTTP/2 Rapid Reset in google.golang.org/grpc
(GO-2023-2153)
[HIGH] 39-39: google.golang.org/grpc 1.33.2: gRPC-Go HTTP/2 Rapid Reset vulnerability
⏰ 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). (7)
- GitHub Check: tests
- GitHub Check: sims
- GitHub Check: release-dry-run
- GitHub Check: coverage
- GitHub Check: lint
- GitHub Check: build-bins
- GitHub Check: build-macos
🔇 Additional comments (5)
go.mod (5)
55-58: Ledger ecosystem forks updated; verify Cosmos SDK and ledger API compatibility.The PR bumps ledger support by updating forks:
- Line 55:
cosmos-sdk→akash-network/cosmos-sdk v0.45.16-akash.7(patch bump)- Line 58:
cosmos/ledger-cosmos-go→akash-network/ledger-go/cosmos v0.15.0(new fork)Verify that:
- The Cosmos SDK fork is compatible with the rest of the dependency tree (especially gRPC, once unblocked).
- The new ledger-cosmos-go fork location is the correct canonical source for this project.
- All ledger API calls in the codebase are compatible with the v0.15.0 API surface.
73-74: Clarify the purpose of the zondax/hid fork to a personal account.Line 73 replaces
zondax/hidwithtroian/hid v0.13.2(a personal fork), while line 74 replaceszondax/ledger-gowith the officialakash-network/ledger-go v0.15.1fork. The inconsistency—routing one dependency to a personal fork and another to an organizational fork—raises concerns:
- Is the personal
troian/hidfork maintained and secure?- Why not use the zondax official version or an akash-network fork instead?
- Does this fork carry any customizations or patches required for ledger hardware support?
Please clarify the rationale and confirm this fork is production-ready and security-audited.
36-39: Direct dependency upgrades are substantial; confirm test coverage.Lines 36–39 bump several golang.org/x and gRPC dependencies:
golang.org/x/mod v0.26.0(significant jump)golang.org/x/oauth2 v0.30.0(significant jump)golang.org/x/sync v0.16.0(significant jump)google.golang.org/grpc v1.75.0(major upgrade once blocker is resolved)These are foundational packages. Ensure:
- The codebase is tested against these new versions (especially gRPC API changes).
- No breaking changes in
mod,oauth2, orsyncare silently breaking internal usage.Please confirm that integration tests have run successfully with these upgraded versions.
24-32: Prometheus and spf13 suite upgrades are substantial; review for breaking changes.Lines 24–32 bump observability and CLI tooling:
prometheus/client_golang v1.23.0(from v1.18.0, per comment: avoid certain upstream changes)spf13/cobra v1.9.1,spf13/viper v1.20.1,spf13/pflag v1.0.6,spf13/cast v1.9.2(multi-package update)stretchr/testify v1.11.1(test framework)The inline comment on line 23 warns against upgrading Prometheus past v1.16.0 due to SDK compatibility. Confirm v1.23.0 aligns with that constraint or that the constraint has been relaxed.
For spf13, verify that the CLI configuration (Cobra/Viper) and flag parsing changes are compatible with current command definitions and configuration loading logic.
202-205: Protobuf and genproto tooling bumped significantly; verify compatibility with modified proto compiler.Lines 202–205 bump gRPC code generation and protocol buffer dependencies:
google.golang.org/genproto v0.0.0-20250603155806-513f23925822(new timestamp)google.golang.org/genproto/googleapis/apiand/rpc(new timestamps, rpc updated to later date)google.golang.org/protobuf v1.36.8(significant bump)The comment on lines 75–76 mentions that "latest grpc doesn't work with cosmos-sdk modified proto compiler." Confirm that the protobuf v1.36.8 tooling is compatible with any custom compiler modifications the project uses.
| github.com/zondax/ledger-go => github.com/akash-network/ledger-go v0.15.1 | ||
| // latest grpc doesn't work with with cosmos-sdk modified proto compiler, so we need to enforce | ||
| // the following version across all dependencies. | ||
| google.golang.org/grpc => google.golang.org/grpc v1.33.2 |
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.
🔴 BLOCKER: Unresolved gRPC DoS vulnerability still pinned in replace directive.
Line 77 still forces google.golang.org/grpc v1.33.2, which is affected by the HTTP/2 Rapid Reset DoS (GO-2023-2153 / GHSA-m425-mq94-257g). This was flagged in the previous review and remains unresolved. The direct dependency already specifies v1.75.0 (line 39), which contains the fix—the replace override defeats this upgrade.
To unblock this PR:
- Remove or update the gRPC replace directive to allow
v1.75.0or confirm that the Cosmos SDK modified proto compiler works with a safe version (≥ v1.56.3). - Run
go get google.golang.org/[email protected] && go mod tidy(and update vendor if used). - If the replace is genuinely required for proto compatibility, document why and pin a safe version instead.
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...
!to the type prefix if API or client breaking changeCHANGELOG.md