Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds lease reclamation across market and deployment modules (handlers, keeper APIs, and tests); integrates Akash node version discovery into gRPC/REST and app/test startup; registers market v2.1.0 upgrade and default reclamation params; adds agent skill docs; updates dependencies and infra. Changes
Sequence Diagram(s)sequenceDiagram
participant Tenant
participant DeploymentHandler as Deployment\nHandler
participant MarketKeeper as Market\nKeeper
participant LeaseStore as Lease\nStorage
participant Registry as Akash\nRegistry
Tenant->>DeploymentHandler: CreateDeployment(msg with Reclamation)
DeploymentHandler->>MarketKeeper: GetParams(ctx)
MarketKeeper-->>DeploymentHandler: params (min/max)
DeploymentHandler->>DeploymentHandler: Validate reclamation vs params
DeploymentHandler->>MarketKeeper: CreateOrder(..., reclamation)
MarketKeeper->>LeaseStore: Persist order with Reclamation
Tenant->>MarketKeeper: CreateBid(..., reclamationWindow)
MarketKeeper->>LeaseStore: Persist bid with ReclamationWindow
Tenant->>MarketKeeper: LeaseStartReclaim(leaseID)
MarketKeeper->>LeaseStore: Load lease, set StartedAt/Deadline/Reason
MarketKeeper->>LeaseStore: Persist lease state LeaseReclaiming
MarketKeeper->>Tenant: Emit EventLeaseReclaimStarted
Note over Registry,DeploymentHandler: Registry initialized at app/test startup
DeploymentHandler->>Registry: SetRegistry(chainID, nodeVersion)
Registry-->>DeploymentHandler: Provides node discovery info
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (5)
.agents/skills/upgrade-changelog/SKILL.md (1)
79-80: Prefer repo-local constant resolution before module cache/go doc.Relying on module cache or
go doccan make runs environment-dependent. Recommend explicitly prioritizing repository-local lookup first, then fallback to cache/go doc.Suggested wording update
-- When resolving Go constants from external packages, search under the Go module cache or use `go doc` if needed. The packages typically follow the pattern `pkg.akt.dev/go/node/<module>/<version>`. +- When resolving Go constants, first search repository-local sources (including vendored/copied module code if present). If not found, search the Go module cache or use `go doc` as fallback. The packages typically follow the pattern `pkg.akt.dev/go/node/<module>/<version>`.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/skills/upgrade-changelog/SKILL.md around lines 79 - 80, Update the constant-resolution logic to prefer repository-local lookup before falling back to the Go module cache or invoking `go doc`: first search the repo source tree for the package and constant (e.g., resolve `epochstypes.StoreKey` within local packages), then if not found search the module cache pattern `pkg.akt.dev/go/node/<module>/<version>` or call `go doc`; if still unresolved, keep using the raw Go expression placeholder (e.g., `` `epochstypes.StoreKey` ``) and emit a user warning.x/oracle/keeper/abci.go (1)
163-165: Align error log field naming with the rest of the file.At Line 164, prefer the same structured field convention used elsewhere (
"error", err) for consistent log querying.Suggested refactor
err = k.aggregatedPrices.Set(sctx, did, aggregatedPrice) if err != nil { - sctx.Logger().Error("set aggregated price", "reason", err.Error()) + sctx.Logger().Error("set aggregated price", "error", err) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@x/oracle/keeper/abci.go` around lines 163 - 165, Replace the structured log field "reason" with the consistent key "error" in the sctx.Logger().Error call and pass the error value (err) itself rather than err.Error(); locate the call to sctx.Logger().Error that logs "set aggregated price" and change the field tuple from ("reason", err.Error()) to ("error", err) to match the rest of the file's logging convention.tests/upgrade/docker-compose.hermes.yaml (2)
46-47: Remove redundant port mapping with host networking.Port mappings are ignored when using
network_mode: "host"because the container shares the host's network namespace directly. The service will automatically bind to port 3000 on the host without explicit mapping.♻️ Proposed cleanup
env_file: - ${AKASH_RUN_DIR}/hermes.env - ports: - - "3000:3000" entrypoint: ["/bin/sh", "-c"]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/upgrade/docker-compose.hermes.yaml` around lines 46 - 47, The docker-compose service currently declares a host network mode and also a redundant ports mapping; remove the "ports:" entry (specifically the "- \"3000:3000\"" mapping) from the service configuration so it relies on network_mode: "host" instead, ensuring no leftover ports: block remains; look for the service block containing network_mode: "host" and the ports: key to update.
29-29: Consider pinning the Hermes image version.Using the
latesttag can lead to unpredictable behavior and non-reproducible builds. Consider pinning to a specific version or commit SHA for consistency across test environments.📌 Example with version pinning
- image: ghcr.io/akash-network/hermes:latest + image: ghcr.io/akash-network/hermes:v1.2.3 # Replace with actual version🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/upgrade/docker-compose.hermes.yaml` at line 29, The docker-compose entry uses an unstable tag "image: ghcr.io/akash-network/hermes:latest"; change that to a pinned version or digest (for example replace with "image: ghcr.io/akash-network/hermes:<semver-or-sha>" or "image: ghcr.io/akash-network/hermes@sha256:<digest>") to ensure reproducible test runs, or make the tag configurable via an environment variable (e.g., use "${HERMES_IMAGE}") and document/update the .env/CI variables accordingly.go.mod (1)
62-62: Align Sonicrequireandreplaceversions to eliminate dependency-report drift.Line 62 replaces all
github.com/bytedance/sonicversions withv1.15.0, but line 121 still requiresv1.14.2indirect. While the replace directive correctly overrides at resolution time, this mismatch creates confusion in SBOM and security scanning outputs.Update line 121 to
v1.15.0and rungo mod tidyto keepgo.mod/go.sumconsistent:Proposed cleanup
- github.com/bytedance/sonic v1.14.2 // indirect + github.com/bytedance/sonic v1.15.0 // indirectThen refresh module metadata (
go mod tidy) sogo.mod/go.sumreflect actual resolved versions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@go.mod` at line 62, Update the mismatched Sonic version in go.mod by changing the require entry for github.com/bytedance/sonic (currently v1.14.2) to v1.15.0 to match the replace directive, then run `go mod tidy` to refresh module metadata and regenerate go.sum so SBOM/security scans reflect the resolved v1.15.0 version.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.agents/skills/guidelines/SKILL.md:
- Around line 59-63: The fenced code block that currently starts with ``` (the
three-backtick block showing the numbered verification steps) lacks a language
tag and triggers markdownlint MD040; fix it by adding a language identifier
(e.g., "text") immediately after the opening backticks so the block becomes
```text, ensuring the code fence is tagged without changing the block contents.
In @.agents/skills/upgrade-changelog/SKILL.md:
- Around line 43-45: The fenced code block containing the text "Add new upgrades
after this line based on the template above" is missing a language tag which
triggers markdownlint MD040; update the opening fence in SKILL.md for that block
(the triple-backtick block shown in the diff) to include a language tag such as
```text so the block becomes ```text and keep the closing ``` intact.
In `@make/test-upgrade.mk`:
- Line 63: The Makefile `test` target currently comments out its `init`
prerequisite (`test: `#init``) so `make test` runs without initializing required
state; change the target declaration to include the `init` prerequisite (e.g.,
`test: init`) so `init` runs before tests and ensures dependencies like
`$(COSMOVISOR)`, `$(AKASH_INIT)` and the `$(AP_RUN_DIR)/validators` state are
created; update any related comments if necessary to reflect that `init` is a
real prerequisite and not commented out.
In `@tests/upgrade/docker-compose.hermes.yaml`:
- Around line 38-39: The comment above UPDATE_INTERVAL_MS is inconsistent with
its value; update either the comment or the value: either change the comment to
state "10 seconds for testing" to match UPDATE_INTERVAL_MS=10000, or set
UPDATE_INTERVAL_MS=60000 to match "1 minute for testing"; locate the
UPDATE_INTERVAL_MS entry in the docker-compose.hermes.yaml and make the comment
and the numeric value consistent.
- Around line 32-35: The container is using network_mode: "host" while
RPC_ENDPOINT is set to host.docker.internal, which is incompatible; update the
configuration so the RPC endpoint matches host networking by replacing the
RPC_ENDPOINT value (RPC_ENDPOINT=http://host.docker.internal:26657) with the
loopback host used with host networking (e.g.,
RPC_ENDPOINT=http://127.0.0.1:26657), or alternatively remove/disable
network_mode: "host" and keep RPC_ENDPOINT as host.docker.internal to run the
container on the bridge network.
In `@testutil/network/rpc.go`:
- Around line 24-28: Tests call aclient.DiscoverClient which relies on
aclient.GetRegistry(), but the registry isn't initialized in the test app path;
update the test app construction to call aclient.SetRegistry(...) after creating
the app so GetRegistry() returns a populated registry. Specifically, add
aclient.SetRegistry(...) in the test setup flow (for example in
testutil/types.go immediately after app.NewApp() returns, or in startInProcess()
before LocalRPCClient.Akash() / DiscoverClient() are used) so
LocalRPCClient.Akash(), aclient.GetRegistry(), and DiscoverClient() see a proper
registry.
In `@x/market/handler/handler_reclamation_test.go`:
- Around line 110-126: The helper createLeaseWithReclamation currently writes
lease.Reclamation directly into the store after calling
MarketKeeper().CreateLease, which prevents tests from verifying that
msgServer.CreateLease actually copies the bid's ReclamationWindow onto the
lease; to fix, remove the direct store mutation in createLeaseWithReclamation so
it only creates the bid (via createBidWithReclamation) and calls
MarketKeeper().CreateLease, then return the bid/order/lease id for callers to
assert propagation; if tests need a helper that force-injects reclamation for
other scenarios, create a separate helper (e.g., injectReclamationOnLease) that
uses GetLease and SaveLease to set lease.Reclamation, leaving
TestCreateLease_StoresReclamation to call CreateLease and then read the lease
from MarketKeeper().GetLease to assert reclamation was set.
In `@x/market/module.go`:
- Around line 182-184: ConsensusVersion for the market module was bumped to 9
(AppModule.ConsensusVersion()) but no migration handler was registered, so
register the market module migration for v8→v9 by calling
utypes.RegisterMigration(...) during the upgrade initialization (same place
other modules register theirs in the upgrade init flow) and point it to the
migration handler implemented in upgrades software v2.1.0 (the function that
sets reclamation params); ensure the registered migration key/version matches
the market module and new consensus version so the handler is invoked during the
node upgrade.
In `@x/oracle/keeper/abci.go`:
- Around line 155-157: The error log inside the aggregation failure branch
currently drops the error object; update the error handling in the block where
sctx.Logger().Error("calculate aggregated price") is called (after the
aggregated price calculation that sets err) to include the actual err value and
relevant context (e.g., denom or request identifiers) in the log entry so the
logger records the error details for debugging.
---
Nitpick comments:
In @.agents/skills/upgrade-changelog/SKILL.md:
- Around line 79-80: Update the constant-resolution logic to prefer
repository-local lookup before falling back to the Go module cache or invoking
`go doc`: first search the repo source tree for the package and constant (e.g.,
resolve `epochstypes.StoreKey` within local packages), then if not found search
the module cache pattern `pkg.akt.dev/go/node/<module>/<version>` or call `go
doc`; if still unresolved, keep using the raw Go expression placeholder (e.g.,
`` `epochstypes.StoreKey` ``) and emit a user warning.
In `@go.mod`:
- Line 62: Update the mismatched Sonic version in go.mod by changing the require
entry for github.com/bytedance/sonic (currently v1.14.2) to v1.15.0 to match the
replace directive, then run `go mod tidy` to refresh module metadata and
regenerate go.sum so SBOM/security scans reflect the resolved v1.15.0 version.
In `@tests/upgrade/docker-compose.hermes.yaml`:
- Around line 46-47: The docker-compose service currently declares a host
network mode and also a redundant ports mapping; remove the "ports:" entry
(specifically the "- \"3000:3000\"" mapping) from the service configuration so
it relies on network_mode: "host" instead, ensuring no leftover ports: block
remains; look for the service block containing network_mode: "host" and the
ports: key to update.
- Line 29: The docker-compose entry uses an unstable tag "image:
ghcr.io/akash-network/hermes:latest"; change that to a pinned version or digest
(for example replace with "image: ghcr.io/akash-network/hermes:<semver-or-sha>"
or "image: ghcr.io/akash-network/hermes@sha256:<digest>") to ensure reproducible
test runs, or make the tag configurable via an environment variable (e.g., use
"${HERMES_IMAGE}") and document/update the .env/CI variables accordingly.
In `@x/oracle/keeper/abci.go`:
- Around line 163-165: Replace the structured log field "reason" with the
consistent key "error" in the sctx.Logger().Error call and pass the error value
(err) itself rather than err.Error(); locate the call to sctx.Logger().Error
that logs "set aggregated price" and change the field tuple from ("reason",
err.Error()) to ("error", err) to match the rest of the file's logging
convention.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 68dfe338-e947-4264-88fc-ebcc07eeb316
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (22)
.agents/skills/guidelines/SKILL.md.agents/skills/upgrade-changelog/SKILL.mdapp/app.gocmd/akash/cmd/app_creator.gogo.modmake/test-upgrade.mktests/upgrade/docker-compose.hermes.yamltestutil/network/rpc.goupgrades/software/v2.1.0/upgrade.gox/deployment/handler/server.gox/deployment/imports/keepers.gox/market/handler/handler.gox/market/handler/handler_reclamation_test.gox/market/handler/handler_test.gox/market/handler/server.gox/market/keeper/grpc_query.gox/market/keeper/keeper.gox/market/keeper/keeper_reclamation_test.gox/market/keeper/keeper_test.gox/market/module.gox/market/simulation/genesis.gox/oracle/keeper/abci.go
Comment on lines
+59
to
+63
| ``` | ||
| 1. [Step] → verify: [check] | ||
| 2. [Step] → verify: [check] | ||
| 3. [Step] → verify: [check] | ||
| ``` |
There was a problem hiding this comment.
Add a language tag to the fenced code block (markdownlint MD040).
Line 59 opens a fenced block without a language identifier, which will fail common markdown linting rules.
Suggested fix
-```
+```text
1. [Step] → verify: [check]
2. [Step] → verify: [check]
3. [Step] → verify: [check]</details>
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.22.1)</summary>
[warning] 59-59: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against the current code and only fix it if needed.
In @.agents/skills/guidelines/SKILL.md around lines 59 - 63, The fenced code
block that currently starts with (the three-backtick block showing the numbered verification steps) lacks a language tag and triggers markdownlint MD040; fix it by adding a language identifier (e.g., "text") immediately after the opening backticks so the block becomestext, ensuring the code fence is
tagged without changing the block contents.
</details>
<!-- fingerprinting:phantom:triton:hawk:753d3d17-a651-49d5-85d3-9cb9da9cac21 -->
<!-- d98c2f50 -->
<!-- This is an auto-generated comment by CodeRabbit -->
Comment on lines
+38
to
+39
| # Update interval (1 minute for testing, use 5+ minutes in production) | ||
| - UPDATE_INTERVAL_MS=10000 |
There was a problem hiding this comment.
Align comment with actual interval value.
The comment states "1 minute for testing" but the actual value is 10000 ms (10 seconds). Either update the comment to reflect the 10-second interval or change the value to 60000 ms for 1 minute.
📝 Proposed fix to match comment
- # Update interval (1 minute for testing, use 5+ minutes in production)
- - UPDATE_INTERVAL_MS=10000
+ # Update interval (10 seconds for testing, use 5+ minutes in production)
+ - UPDATE_INTERVAL_MS=10000Or to match the comment:
# Update interval (1 minute for testing, use 5+ minutes in production)
- - UPDATE_INTERVAL_MS=10000
+ - UPDATE_INTERVAL_MS=60000📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| # Update interval (1 minute for testing, use 5+ minutes in production) | |
| - UPDATE_INTERVAL_MS=10000 | |
| # Update interval (10 seconds for testing, use 5+ minutes in production) | |
| - UPDATE_INTERVAL_MS=10000 |
Suggested change
| # Update interval (1 minute for testing, use 5+ minutes in production) | |
| - UPDATE_INTERVAL_MS=10000 | |
| # Update interval (1 minute for testing, use 5+ minutes in production) | |
| - UPDATE_INTERVAL_MS=60000 |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/upgrade/docker-compose.hermes.yaml` around lines 38 - 39, The comment
above UPDATE_INTERVAL_MS is inconsistent with its value; update either the
comment or the value: either change the comment to state "10 seconds for
testing" to match UPDATE_INTERVAL_MS=10000, or set UPDATE_INTERVAL_MS=60000 to
match "1 minute for testing"; locate the UPDATE_INTERVAL_MS entry in the
docker-compose.hermes.yaml and make the comment and the numeric value
consistent.
Signed-off-by: Artur Troian <troian@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
.agents/skills/upgrade-changelog/SKILL.md (1)
43-45:⚠️ Potential issue | 🟡 MinorTag the fenced block language to pass MD040.
At Line 43, add a language identifier (e.g.,
text) to the opening fence.Suggested fix
- ``` + ```text Add new upgrades after this line based on the template above ```🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/skills/upgrade-changelog/SKILL.md around lines 43 - 45, The fenced code block opener currently has no language tag; update the opening fence (the line with ```) so it reads as a language-tagged fence (e.g., add "text" after the backticks) so the block becomes ```text to satisfy MD040; leave the closing ``` unchanged..agents/skills/guidelines/SKILL.md (1)
59-63:⚠️ Potential issue | 🟡 MinorAdd a fence language to satisfy markdownlint MD040.
At Line 59, the fenced block is still untyped. Please annotate it (e.g.,
text) to avoid lint failures.Suggested fix
-``` +```text 1. [Step] → verify: [check] 2. [Step] → verify: [check] 3. [Step] → verify: [check]</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In @.agents/skills/guidelines/SKILL.md around lines 59 - 63, The fenced code
block containing the three checklist lines is untyped and triggers markdownlint
MD040; update that backtick block in SKILL.md (the block starting with "1.
[Step] → verify: [check]") by adding a fence language (for example "text") after
the openingso it becomestext, ensuring the block remains otherwise
unchanged.</details> </blockquote></details> </blockquote></details> <details> <summary>🧹 Nitpick comments (1)</summary><blockquote> <details> <summary>upgrades/software/v2.1.0/market.go (1)</summary><blockquote> `22-22`: **Fix misleading comment.** The comment says "marketMigrations deployment" which appears to be a copy-paste artifact. It should describe this as a market migration. <details> <summary>📝 Proposed fix</summary> ```diff -// handler marketMigrations deployment from version 7 to 8. +// handler migrates market module from version 7 to 8. ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@upgrades/software/v2.1.0/market.go` at line 22, Update the misleading top-line comment in market.go that currently reads "handler marketMigrations deployment from version 7 to 8." to accurately describe the code as a market migration handler (e.g., "handler for market migration from version 7 to 8") so readers know this is a migration handler; locate the comment near the marketMigrations function and replace the copy-paste wording with the corrected phrase. ``` </details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against the current code and only fix it if needed.
Inline comments:
In@testutil/network/util.go:
- Around line 51-53: The test mutates global discovery state via
aclient.SetRegistry(...) which can cause cross-test interference with
LocalRPCClient.Akash; instead, stop mutating global state by creating a local
registry instance with aclient.DefaultRegistry(aclient.WithChainID(cfg.ChainID))
and use that registry for the client(s) you construct (e.g., pass the registry
into the client constructor or aclient.New* variant that accepts a registry) or,
if the client API lacks that overload, capture the current registry, call
SetRegistry only locally during client creation and immediately restore the
original registry; update code references to
aclient.SetRegistry/aclient.DefaultRegistry and LocalRPCClient.Akash
accordingly.- Around line 31-32: The test registry initialization omits the node version,
making discovery/negotiation differ from production; update the registry
creation call in testutil/network/util.go (the code that currently initializes
the registry with just the chain ID) to include the node version option by
passing aclient.WithNodeVersion(version.Version) into the same registry
constructor call, and add the necessary import for the version package so the
tests mirror production's use of aclient.WithNodeVersion(version.Version).In
@upgrades/software/v2.1.0/market.go:
- Around line 14-16: newMarketMigration currently constructs and returns
deploymentMigrations{Migrator: m} which causes
deploymentMigrations.handler()/GetHandler() to be used instead of the intended
marketMigrations; fix by returning marketMigrations{Migrator: m} from
newMarketMigration so the marketMigrations type (and its GetHandler/handler
methods) are actually invoked; update the constructor in the newMarketMigration
function to instantiate marketMigrations with the provided Migrator.
Duplicate comments:
In @.agents/skills/guidelines/SKILL.md:
- Around line 59-63: The fenced code block containing the three checklist lines
is untyped and triggers markdownlint MD040; update that backtick block in
SKILL.md (the block starting with "1. [Step] → verify: [check]") by adding a
fence language (for example "text") after the openingso it becomestext,
ensuring the block remains otherwise unchanged.In @.agents/skills/upgrade-changelog/SKILL.md:
- Around line 43-45: The fenced code block opener currently has no language tag;
update the opening fence (the line with) so it reads as a language-tagged fence (e.g., add "text" after the backticks) so the block becomestext to
satisfy MD040; leave the closing ``` unchanged.
Nitpick comments:
In@upgrades/software/v2.1.0/market.go:
- Line 22: Update the misleading top-line comment in market.go that currently
reads "handler marketMigrations deployment from version 7 to 8." to accurately
describe the code as a market migration handler (e.g., "handler for market
migration from version 7 to 8") so readers know this is a migration handler;
locate the comment near the marketMigrations function and replace the copy-paste
wording with the corrected phrase.</details> <details> <summary>🪄 Autofix (Beta)</summary> Fix all unresolved CodeRabbit comments on this PR: - [ ] <!-- {"checkboxId": "4b0d0e0a-96d7-4f10-b296-3a18ea78f0b9"} --> Push a commit to this branch (recommended) - [ ] <!-- {"checkboxId": "ff5b1114-7d8c-49e6-8ac1-43f82af23a33"} --> Create a new PR with the fixes </details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: Path: .coderabbit.yaml **Review profile**: CHILL **Plan**: Pro **Run ID**: `1a61674f-59fa-42c7-bf5a-733fdc7b5694` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 27e6142433a9af2708a7a083c9161508aa120279 and 05de3d2f8bb9a640a618f05bbe89db15883faa62. </details> <details> <summary>⛔ Files ignored due to path filters (1)</summary> * `go.sum` is excluded by `!**/*.sum` </details> <details> <summary>📒 Files selected for processing (24)</summary> * `.agents/skills/guidelines/SKILL.md` * `.agents/skills/upgrade-changelog/SKILL.md` * `app/app.go` * `cmd/akash/cmd/app_creator.go` * `go.mod` * `tests/upgrade/docker-compose.hermes.yaml` * `testutil/network/rpc.go` * `testutil/network/util.go` * `upgrades/software/v2.1.0/init.go` * `upgrades/software/v2.1.0/market.go` * `upgrades/software/v2.1.0/upgrade.go` * `x/deployment/handler/server.go` * `x/deployment/imports/keepers.go` * `x/market/handler/handler.go` * `x/market/handler/handler_reclamation_test.go` * `x/market/handler/handler_test.go` * `x/market/handler/server.go` * `x/market/keeper/grpc_query.go` * `x/market/keeper/keeper.go` * `x/market/keeper/keeper_reclamation_test.go` * `x/market/keeper/keeper_test.go` * `x/market/module.go` * `x/market/simulation/genesis.go` * `x/oracle/keeper/abci.go` </details> <details> <summary>✅ Files skipped from review due to trivial changes (5)</summary> * cmd/akash/cmd/app_creator.go * testutil/network/rpc.go * tests/upgrade/docker-compose.hermes.yaml * x/market/handler/handler_test.go * go.mod </details> <details> <summary>🚧 Files skipped from review as they are similar to previous changes (7)</summary> * x/market/module.go * x/market/keeper/grpc_query.go * x/market/simulation/genesis.go * x/market/handler/handler.go * x/oracle/keeper/abci.go * x/market/keeper/keeper_reclamation_test.go * x/market/handler/server.go </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
Signed-off-by: Artur Troian <troian@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@go.mod`:
- Line 272: The go.mod entry for module go.opentelemetry.io/otel/sdk (currently
v1.37.0) is vulnerable; update that module to at least v1.43.0 to address both
CVEs (prefer v1.43.0+), ensure the go.mod line for go.opentelemetry.io/otel/sdk
is changed to v1.43.0 (or newer), then run dependency resolution (e.g., go get
./... or go get go.opentelemetry.io/otel/sdk@v1.43.0 and go mod tidy) and
rebuild/tests to confirm compatibility with other OpenTelemetry modules (note
other entries use v1.41.0).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 988159c7-e586-4fc6-9ffe-5f74ecff978e
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (3)
go.modmake/setup-cache.mktests/upgrade/test-cases.json
✅ Files skipped from review due to trivial changes (1)
- tests/upgrade/test-cases.json
There was a problem hiding this comment.
♻️ Duplicate comments (1)
go.mod (1)
269-273:⚠️ Potential issue | 🔴 CriticalUpgrade
go.opentelemetry.io/otel/sdkandsdk/metric; current versions remain vulnerable.At Lines 271-272,
go.opentelemetry.io/otel/sdkandgo.opentelemetry.io/otel/sdk/metricare stillv1.37.0while other OTEL modules were bumped. This keeps known HIGH-severity advisories in the graph and leaves OTEL modules version-skewed.Proposed minimal fix
- go.opentelemetry.io/otel/sdk v1.37.0 // indirect - go.opentelemetry.io/otel/sdk/metric v1.37.0 // indirect + go.opentelemetry.io/otel/sdk v1.43.0 // indirect + go.opentelemetry.io/otel/sdk/metric v1.43.0 // indirect#!/bin/bash # Validate current OTEL versions in go.mod and check OSV advisories for sdk v1.37.0. # Read-only verification only. set -euo pipefail echo "== OTEL entries in go.mod ==" rg -n '^\s*go\.opentelemetry\.io/otel(/sdk|/sdk/metric|/metric|/trace)?\s+v' go.mod echo echo "== OSV query: go.opentelemetry.io/otel/sdk@v1.37.0 ==" python - <<'PY' import json, urllib.request payload = { "package": {"name": "go.opentelemetry.io/otel/sdk", "ecosystem": "Go"}, "version": "1.37.0" } req = urllib.request.Request( "https://api.osv.dev/v1/query", data=json.dumps(payload).encode(), headers={"Content-Type": "application/json"}, ) with urllib.request.urlopen(req) as r: data = json.load(r) for v in data.get("vulns", []): print(v.get("id"), "-", v.get("summary", "")) PY🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@go.mod` around lines 269 - 273, go.opentelemetry.io/otel/sdk and go.opentelemetry.io/otel/sdk/metric are pinned at v1.37.0 and must be bumped to match the other OTEL modules to eliminate known vulnerabilities; update the go.mod entries for go.opentelemetry.io/otel/sdk and go.opentelemetry.io/otel/sdk/metric to the same safe version used elsewhere (e.g., v1.41.0), then run module resolution/cleanup (go module get / tidy) to apply the change and verify there are no remaining OSV advisories for those packages.
🧹 Nitpick comments (1)
go.mod (1)
3-3: Go 1.26.2 version is properly propagated through CI—no blocker detected.The CI workflow dynamically detects the Go version from
go.modviascript/tools.sh gotoolchain, which extracts and passes1.26.2toactions/setup-go@v5withcheck-latest: true, ensuring version consistency across builds. However,_build/test.Dockerfileuses a genericFROM golang:alpinewithout an explicit version pin; consider pinning it togolang:1.26.2-alpinefor reproducibility and to match the toolchain specified ingo.mod.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@go.mod` at line 3, The Dockerfile base image is unpinned causing potential divergence from go.mod; update the FROM line in _build/test.Dockerfile to explicitly pin the Go version to match go.mod (go 1.26.2) by replacing the generic "FROM golang:alpine" with "FROM golang:1.26.2-alpine" so the build image and the go.mod toolchain remain consistent and reproducible.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@go.mod`:
- Around line 269-273: go.opentelemetry.io/otel/sdk and
go.opentelemetry.io/otel/sdk/metric are pinned at v1.37.0 and must be bumped to
match the other OTEL modules to eliminate known vulnerabilities; update the
go.mod entries for go.opentelemetry.io/otel/sdk and
go.opentelemetry.io/otel/sdk/metric to the same safe version used elsewhere
(e.g., v1.41.0), then run module resolution/cleanup (go module get / tidy) to
apply the change and verify there are no remaining OSV advisories for those
packages.
---
Nitpick comments:
In `@go.mod`:
- Line 3: The Dockerfile base image is unpinned causing potential divergence
from go.mod; update the FROM line in _build/test.Dockerfile to explicitly pin
the Go version to match go.mod (go 1.26.2) by replacing the generic "FROM
golang:alpine" with "FROM golang:1.26.2-alpine" so the build image and the
go.mod toolchain remain consistent and reproducible.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3f132b0e-b78f-4a83-b416-66fa0334a345
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (2)
go.modupgrades/software/v2.1.0/market.go
🚧 Files skipped from review as they are similar to previous changes (1)
- upgrades/software/v2.1.0/market.go
Signed-off-by: Artur Troian <troian@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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