feat(market): add OfferPrices and BidMinDeposits to market proto#242
Merged
feat(market): add OfferPrices and BidMinDeposits to market proto#242
Conversation
Contributor
WalkthroughParams now accept multiple bid-denom deposits via a new repeated Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
proto/node/akash/market/v1beta5/params.proto (1)
12-17:⚠️ Potential issue | 🟠 MajorAmbiguous coexistence of
bid_min_depositandbid_min_deposits— mark deprecated or document the relationship.Both the singular
bid_min_deposit(tag 1) and the new pluralbid_min_deposits(tag 3) are present with no indication of which takes precedence, whether one supersedes the other, or whether both must be populated. Consumers of thisParamsmessage cannot determine the intended semantics without external context.If
bid_min_depositsis meant to supersedebid_min_deposit, the older field should carry an explicitdeprecated = trueoption and an updated comment. If both are intentionally in use (e.g., single-denom vs. multi-denom paths), the comments must clearly state that.📝 Suggested deprecation annotation for the singular field if it is being superseded
// BidMinDeposit is a parameter for the minimum deposit on a Bid. + // Deprecated: use BidMinDeposits instead. cosmos.base.v1beta1.Coin bid_min_deposit = 1 [ (gogoproto.customname) = "BidMinDeposit", (gogoproto.nullable) = false, (gogoproto.jsontag) = "bid_min_deposit", - (gogoproto.moretags) = "yaml:\"bid_min_deposit\"" + (gogoproto.moretags) = "yaml:\"bid_min_deposit\"", + deprecated = true ];Also applies to: 26-33
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@proto/node/akash/market/v1beta5/params.proto` around lines 12 - 17, The Params message currently contains both fields bid_min_deposit and bid_min_deposits with no guidance; update the proto to disambiguate by either marking the singular field (bid_min_deposit) as deprecated when the plural supersedes it—add the gogoproto.deprecated = true option and an inline comment like "Deprecated: use bid_min_deposits"—or, if both are required, add clear comments to both bid_min_deposit and bid_min_deposits describing their intended coexistence, precedence, and migration behavior so consumers know which to use; reference the Params message and the fields bid_min_deposit and bid_min_deposits when making the change.
🧹 Nitpick comments (1)
proto/node/akash/market/v1beta5/resourcesoffer.proto (1)
98-101: No uniqueness guarantee on the keying fields of repeated offer-price entries.
storageis keyed byname(free-form string) andendpointsis keyed bykind(enum). The proto schema does not prevent duplicate entries (twoStorageOfferPriceentries withname = "ssd", or twoEndpointOfferPriceentries withkind = LEASED_IP). Consumers will need to decide themselves how to handle duplicates — first wins, last wins, or error.Consider at minimum documenting the expected uniqueness contract in the field comments, and enforce it in validation logic on the application layer. A
map<string, DecCoin>/map<int32, DecCoin>would encode the uniqueness constraint in the schema itself, though the gogoproto ecosystem handles maps less ergonomically.Also applies to: 115-118
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@proto/node/akash/market/v1beta5/resourcesoffer.proto` around lines 98 - 101, The repeated fields storage (repeated StorageOfferPrice) and endpoints (repeated EndpointOfferPrice) allow duplicate keys because their keying fields (StorageOfferPrice.name and EndpointOfferPrice.kind) are not enforced unique; update the proto and application validation: either change the schema to use maps (e.g., map<string, DecCoin> for storage and map<int32, DecCoin> for endpoints) to encode uniqueness, or (if keeping repeated messages) add clear field comments documenting the uniqueness contract and implement validation logic in the relevant constructor/validator (e.g., where StorageOfferPrice/EndpointOfferPrice lists are parsed) to detect duplicates by StorageOfferPrice.name and EndpointOfferPrice.kind and return an error; choose one approach and apply the same change to the analogous fields at lines 115-118 as well.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@proto/node/akash/market/v1beta5/params.proto`:
- Line 26: Update the stale doc comment for the plural field bid_min_deposits in
the Params message to reflect its semantics (a list/array of minimum deposit
entries) rather than duplicating the singular description; locate the comment
above the bid_min_deposits field in proto/node/akash/market/v1beta5/params.proto
and replace the sentence "BidMinDeposit is a parameter for the minimum deposit
on a Bid." with a concise description such as "BidMinDeposits specifies a
repeated list of minimum deposit amounts applicable to bids" (or equivalent) so
it accurately documents the plural nature of bid_min_deposits.
In `@proto/node/akash/market/v1beta5/resourcesoffer.proto`:
- Around line 74-77: The new message OfferPrices currently contains `reserved
1;` with an unclear comment; update the proto so the reservation is explicit or
removed: either keep `reserved 1;` but change its comment to name the removed
field (e.g. `reserved 1; // formerly "FIELD_NAME"`) and include a brief reason,
or remove the `reserved 1;` line entirely if no prior serialized field existed;
make this change in the OfferPrices message declaration so future readers
understand why field number 1 is unavailable.
---
Outside diff comments:
In `@proto/node/akash/market/v1beta5/params.proto`:
- Around line 12-17: The Params message currently contains both fields
bid_min_deposit and bid_min_deposits with no guidance; update the proto to
disambiguate by either marking the singular field (bid_min_deposit) as
deprecated when the plural supersedes it—add the gogoproto.deprecated = true
option and an inline comment like "Deprecated: use bid_min_deposits"—or, if both
are required, add clear comments to both bid_min_deposit and bid_min_deposits
describing their intended coexistence, precedence, and migration behavior so
consumers know which to use; reference the Params message and the fields
bid_min_deposit and bid_min_deposits when making the change.
---
Nitpick comments:
In `@proto/node/akash/market/v1beta5/resourcesoffer.proto`:
- Around line 98-101: The repeated fields storage (repeated StorageOfferPrice)
and endpoints (repeated EndpointOfferPrice) allow duplicate keys because their
keying fields (StorageOfferPrice.name and EndpointOfferPrice.kind) are not
enforced unique; update the proto and application validation: either change the
schema to use maps (e.g., map<string, DecCoin> for storage and map<int32,
DecCoin> for endpoints) to encode uniqueness, or (if keeping repeated messages)
add clear field comments documenting the uniqueness contract and implement
validation logic in the relevant constructor/validator (e.g., where
StorageOfferPrice/EndpointOfferPrice lists are parsed) to detect duplicates by
StorageOfferPrice.name and EndpointOfferPrice.kind and return an error; choose
one approach and apply the same change to the analogous fields at lines 115-118
as well.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go/node/market/v1beta5/resourcesoffer.pb.gois excluded by!**/*.pb.go
📒 Files selected for processing (2)
proto/node/akash/market/v1beta5/params.protoproto/node/akash/market/v1beta5/resourcesoffer.proto
troian
force-pushed
the
feat/market-offer-prices-proto
branch
from
016912c to
4a08072
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
go/node/market/v1beta5/params.go (1)
51-60:⚠️ Potential issue | 🟠 Major
Validate()does not validate the newBidMinDepositsfield.Every other
Paramsfield is checked here, but the newly addedBidMinDepositsis skipped. This means invalid values (e.g., zero-amount coins, duplicate denoms) will pass validation whenValidate()is called directly—such as during genesis validation or governance parameter-change proposals.🐛 Proposed fix
func (p Params) Validate() error { if err := validateCoin(p.BidMinDeposit); err != nil { return err } + if err := validateCoins(p.BidMinDeposits); err != nil { + return err + } + if err := validateOrderMaxBids(p.OrderMaxBids); err != nil { return err } return nil }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@go/node/market/v1beta5/params.go` around lines 51 - 60, Params.Validate currently checks BidMinDeposit and OrderMaxBids but omits the newly added BidMinDeposits slice; add a validation call for p.BidMinDeposits (e.g., call the existing slice validator such as validateCoins or the appropriate helper that checks zero amounts and duplicate denoms) inside Params.Validate alongside the other checks and return any error, referencing the Params.Validate method and the BidMinDeposits field to locate where to insert the call.
♻️ Duplicate comments (1)
proto/node/akash/market/v1beta5/resourcesoffer.proto (1)
64-78: Clarify why field number 1 is reserved inOfferPrices.This matches a prior review note; please either name the removed field in the comment or drop the reservation if it never existed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@proto/node/akash/market/v1beta5/resourcesoffer.proto` around lines 64 - 78, The comment and the proto message OfferPrices reserve field number 1 but give no reason; update the file so the reservation is explicit: either add a brief comment naming the previously removed field (e.g., "reserved 1 for <RemovedFieldName> removed in commit XYZ") to document why reserved 1 exists, or if no such field ever existed remove the "reserved 1;" line. Edit the OfferPrices message to include the chosen fix (reference: message OfferPrices and the reserved 1 declaration).
🧹 Nitpick comments (5)
go/node/market/v1beta5/params.go (1)
71-78:validateCoinsshould validate coin values, not just the type.While the existing
validateCoinfollows the same type-check-only pattern,sdk.Coinshas a larger invalid-state surface (zero/negative amounts, duplicate denoms, unsorted, invalid denoms). TheValidate()method catches all of these and should be called here to ensure malformed parameters are rejected at the param-store boundary.♻️ Proposed improvement
func validateCoins(i interface{}) error { - _, ok := i.(sdk.Coins) + val, ok := i.(sdk.Coins) if !ok { return fmt.Errorf("%w: invalid type %T", v1.ErrInvalidParam, i) } + if err := val.Validate(); err != nil { + return fmt.Errorf("%w: %w", v1.ErrInvalidParam, err) + } + return nil }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@go/node/market/v1beta5/params.go` around lines 71 - 78, The validateCoins function currently only checks the type of the input; update it to assert i is sdk.Coins and then call its Validate() method to reject zero/negative amounts, duplicates, unsorted lists, and invalid denoms; specifically modify validateCoins (and mirror validateCoin if present) to perform a type assertion to sdk.Coins, return fmt.Errorf wrapping v1.ErrInvalidParam when the assertion fails, then call coins.Validate() and return any validation error (wrap with v1.ErrInvalidParam or appropriate context) so malformed coin parameters are rejected at the param-store boundary.docs/swagger-ui/swagger.yaml (4)
51336-51338:bid_min_depositdeprecation signalling is non-standardThe deprecation is communicated via plain text in the
titlefield. In Swagger 2.0, there is no built-indeprecatedproperty for schema fields, but the widely-supported vendor extensionx-deprecated: trueis available and understood by most tooling (Swagger UI, openapi-generator, etc.).Additionally, the two occurrences of this
titleuse different YAML scalar styles (>-folded in hunk 3 / line 6223 vs|-literal in hunk 6 / line 51337), which is a minor inconsistency.🛠️ Suggested addition (apply to all three Params occurrences)
bid_min_deposit: title: |- BidMinDeposit is a parameter for the minimum deposit on a Bid. Deprecated: use BidMinDeposits + x-deprecated: true type: object🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/swagger-ui/swagger.yaml` around lines 51336 - 51338, Add the standard vendor extension to mark the parameter/schema as deprecated instead of only describing it in the title: locate the schema/parameter named BidMinDeposit (and any occurrences of bid_min_deposit/ BidMinDeposits in the Params sections) and add x-deprecated: true next to its definition; also normalize the YAML scalar style for the title across all occurrences (choose one style, e.g., folded >- ) so the three Param entries are consistent.
6248-6267:bid_min_depositsshould declareuniqueItems: trueSince each
Coinentry inbid_min_depositsis keyed bydenom, duplicate denoms are semantically invalid. The schema currently permits a repeated denom, leaving validation entirely to application logic.
uniqueItems: trueprevents duplicate entries — Swagger 2.0 checks for value equality. Adding it here makes the constraint explicit to validators and SDK generators.🛠️ Proposed addition
bid_min_deposits: type: array + uniqueItems: true items: type: objectApply the same to the parallel occurrences at lines 51356–51372 and 54062–54081.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/swagger-ui/swagger.yaml` around lines 6248 - 6267, The bid_min_deposits array schema currently allows duplicate Coin entries; update the bid_min_deposits array definition to include uniqueItems: true directly under the array node to prevent duplicate denom entries (the array whose items are Coin objects with denom/amount), and apply the same uniqueItems: true change to the other bid_min_deposits occurrences in this swagger.yaml so all equivalent schemas enforce unique items.
50689-50895: Top-levelOfferPricesdefinition still inlinesStorageOfferPrice/EndpointOfferPriceitems rather than$ref'ing their own definitions
akash.market.v1beta5.OfferPrices.storage.itemsandakash.market.v1beta5.OfferPrices.endpoints.itemsrepeat the full object schema inline, even thoughakash.market.v1beta5.StorageOfferPrice(hunk 10, line 54464) andakash.market.v1beta5.EndpointOfferPrice(hunk 5, line 50689) are defined at the top level. Inconsistency between the canonical definition and its children reduces discoverability and causes SDK generators to emit unnamed anonymous types.If the file is auto-generated, address this at the proto annotation/generation layer. If hand-maintained, the
items:blocks should be:- items: - type: object - properties: - name: ... - price: ... - description: StorageOfferPrice represents ... + items: + $ref: '#/definitions/akash.market.v1beta5.StorageOfferPrice'and similarly for
endpoints.items→$ref: '#/definitions/akash.market.v1beta5.EndpointOfferPrice'.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/swagger-ui/swagger.yaml` around lines 50689 - 50895, The OfferPrices schema in akash.market.v1beta5 inlines StorageOfferPrice and EndpointOfferPrice under OfferPrices.storage.items and OfferPrices.endpoints.items; replace those inline object definitions with JSON Schema $ref pointers to the top-level definitions (use '#/definitions/akash.market.v1beta5.StorageOfferPrice' for storage.items and '#/definitions/akash.market.v1beta5.EndpointOfferPrice' for endpoints.items) so SDK generators produce named types; if this file is generated, update the proto->OpenAPI generation/templates to emit $ref for repeated message types instead of inlining.
3080-3255: Use$refforOfferPrices,StorageOfferPrice, andEndpointOfferPricedefinitionsThe
OfferPricesstructure is inlined at 6 locations (lines 3080, 3860, 50435, 51694, 52355, 54294) instead of referencing the top-level definition via$ref: '#/definitions/akash.market.v1beta5.OfferPrices'. Additionally, within the canonicalakash.market.v1beta5.OfferPricesdefinition itself,storage.itemsandendpoints.itemsare fully inlined rather than using$reftoakash.market.v1beta5.StorageOfferPriceandakash.market.v1beta5.EndpointOfferPrice.Inline schemas repeated across multiple paths cause SDK code generators to create numbered duplicate classes (e.g., OfferPrices1, OfferPrices2), degrading the client developer experience. The canonical top-level definitions should consistently use
$refpointers.If this file is auto-generated (e.g., via
protoc-gen-openapiv2), configure the generation tool to emit$refinstead of inlining (e.g., via--openapiv2_optflags or proto annotations). If hand-maintained, replace all inline expansions with$refpointers.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/swagger-ui/swagger.yaml` around lines 3080 - 3255, The OfferPrices schema (and its nested StorageOfferPrice and EndpointOfferPrice types) has been inlined in multiple places causing duplicated generated classes; replace each inline schema with a $ref to the canonical definitions (use $ref: '#/definitions/akash.market.v1beta5.OfferPrices' for the six inlined OfferPrices occurrences and inside the canonical OfferPrices replace storage.items and endpoints.items with $ref: '#/definitions/akash.market.v1beta5.StorageOfferPrice' and $ref: '#/definitions/akash.market.v1beta5.EndpointOfferPrice' respectively), and if this YAML is generated adjust protoc-gen-openapiv2 generation options (or proto annotations / --openapiv2_opt flags) to emit $ref instead of inlining so future regenerations keep the references.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/proto/node.md`:
- Line 4683: The generated docs show an empty description for the ResourceOffer
field `prices`; add a proto comment to the `prices` field in the ResourceOffer
message (the field typed as OfferPrices) in the .proto source so the generator
emits the description—locate the ResourceOffer message, find the `prices` field,
and add a clear one-line comment describing what `prices` represents (e.g.,
pricing details for the resource offer, currency, and units) so the docs include
it.
In `@docs/proto/provider.md`:
- Around line 620-623: The markdown heading "### ServicePermissions" in the
generated docs has a leading space triggering MD023; update the
generator/template that emits headings so headings are emitted flush-left (no
leading spaces) — locate the template or generator code that writes "###
ServicePermissions" (or the logic that prefixes headings) and remove or trim the
leading space when rendering headings, ensure the change is applied consistently
for all heading levels, and run markdownlint/MD023 to verify the fix.
---
Outside diff comments:
In `@go/node/market/v1beta5/params.go`:
- Around line 51-60: Params.Validate currently checks BidMinDeposit and
OrderMaxBids but omits the newly added BidMinDeposits slice; add a validation
call for p.BidMinDeposits (e.g., call the existing slice validator such as
validateCoins or the appropriate helper that checks zero amounts and duplicate
denoms) inside Params.Validate alongside the other checks and return any error,
referencing the Params.Validate method and the BidMinDeposits field to locate
where to insert the call.
---
Duplicate comments:
In `@proto/node/akash/market/v1beta5/resourcesoffer.proto`:
- Around line 64-78: The comment and the proto message OfferPrices reserve field
number 1 but give no reason; update the file so the reservation is explicit:
either add a brief comment naming the previously removed field (e.g., "reserved
1 for <RemovedFieldName> removed in commit XYZ") to document why reserved 1
exists, or if no such field ever existed remove the "reserved 1;" line. Edit the
OfferPrices message to include the chosen fix (reference: message OfferPrices
and the reserved 1 declaration).
---
Nitpick comments:
In `@docs/swagger-ui/swagger.yaml`:
- Around line 51336-51338: Add the standard vendor extension to mark the
parameter/schema as deprecated instead of only describing it in the title:
locate the schema/parameter named BidMinDeposit (and any occurrences of
bid_min_deposit/ BidMinDeposits in the Params sections) and add x-deprecated:
true next to its definition; also normalize the YAML scalar style for the title
across all occurrences (choose one style, e.g., folded >- ) so the three Param
entries are consistent.
- Around line 6248-6267: The bid_min_deposits array schema currently allows
duplicate Coin entries; update the bid_min_deposits array definition to include
uniqueItems: true directly under the array node to prevent duplicate denom
entries (the array whose items are Coin objects with denom/amount), and apply
the same uniqueItems: true change to the other bid_min_deposits occurrences in
this swagger.yaml so all equivalent schemas enforce unique items.
- Around line 50689-50895: The OfferPrices schema in akash.market.v1beta5
inlines StorageOfferPrice and EndpointOfferPrice under OfferPrices.storage.items
and OfferPrices.endpoints.items; replace those inline object definitions with
JSON Schema $ref pointers to the top-level definitions (use
'#/definitions/akash.market.v1beta5.StorageOfferPrice' for storage.items and
'#/definitions/akash.market.v1beta5.EndpointOfferPrice' for endpoints.items) so
SDK generators produce named types; if this file is generated, update the
proto->OpenAPI generation/templates to emit $ref for repeated message types
instead of inlining.
- Around line 3080-3255: The OfferPrices schema (and its nested
StorageOfferPrice and EndpointOfferPrice types) has been inlined in multiple
places causing duplicated generated classes; replace each inline schema with a
$ref to the canonical definitions (use $ref:
'#/definitions/akash.market.v1beta5.OfferPrices' for the six inlined OfferPrices
occurrences and inside the canonical OfferPrices replace storage.items and
endpoints.items with $ref:
'#/definitions/akash.market.v1beta5.StorageOfferPrice' and $ref:
'#/definitions/akash.market.v1beta5.EndpointOfferPrice' respectively), and if
this YAML is generated adjust protoc-gen-openapiv2 generation options (or proto
annotations / --openapiv2_opt flags) to emit $ref instead of inlining so future
regenerations keep the references.
In `@go/node/market/v1beta5/params.go`:
- Around line 71-78: The validateCoins function currently only checks the type
of the input; update it to assert i is sdk.Coins and then call its Validate()
method to reject zero/negative amounts, duplicates, unsorted lists, and invalid
denoms; specifically modify validateCoins (and mirror validateCoin if present)
to perform a type assertion to sdk.Coins, return fmt.Errorf wrapping
v1.ErrInvalidParam when the assertion fails, then call coins.Validate() and
return any validation error (wrap with v1.ErrInvalidParam or appropriate
context) so malformed coin parameters are rejected at the param-store boundary.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (13)
go/manifest/v2beta3/service.pb.gois excluded by!**/*.pb.gogo/node/market/v1beta5/params.pb.gois excluded by!**/*.pb.gogo/node/market/v1beta5/resourcesoffer.pb.gois excluded by!**/*.pb.gots/src/generated/createNodeSDK.tsis excluded by!**/generated/**ts/src/generated/protos/akash/bme/v1/filters.tsis excluded by!**/generated/**ts/src/generated/protos/akash/bme/v1/query.tsis excluded by!**/generated/**ts/src/generated/protos/akash/bme/v1/query_akash.tsis excluded by!**/generated/**ts/src/generated/protos/akash/market/v1beta5/params.tsis excluded by!**/generated/**ts/src/generated/protos/akash/market/v1beta5/resourcesoffer.tsis excluded by!**/generated/**ts/src/generated/protos/akash/oracle/v1/params.tsis excluded by!**/generated/**ts/src/generated/protos/google/protobuf/descriptor.tsis excluded by!**/generated/**ts/src/generated/protos/index.akash.v1.tsis excluded by!**/generated/**ts/src/generated/protos/index.akash.v1beta5.tsis excluded by!**/generated/**
📒 Files selected for processing (6)
docs/proto/node.mddocs/proto/provider.mddocs/swagger-ui/swagger.yamlgo/node/market/v1beta5/params.goproto/node/akash/market/v1beta5/params.protoproto/node/akash/market/v1beta5/resourcesoffer.proto
🚧 Files skipped from review as they are similar to previous changes (1)
- proto/node/akash/market/v1beta5/params.proto
docs/proto/node.md
Outdated
| | ----- | ---- | ----- | ----------- | | ||
| | `resources` | [akash.base.resources.v1beta4.Resources](#akash.base.resources.v1beta4.Resources) | | Resources holds information about bid resources. | | ||
| | `count` | [uint32](#uint32) | | Count is the number of resources. | | ||
| | `prices` | [OfferPrices](#akash.market.v1beta5.OfferPrices) | | | |
Contributor
There was a problem hiding this comment.
Missing field description for prices on ResourceOffer.
The prices field row has an empty description column while all surrounding fields (including the new OfferPrices message itself) carry meaningful descriptions. Since this is auto-generated output, the fix should be applied to the proto source comment for this field.
📝 Expected description (to be added as a proto comment)
- | `prices` | [OfferPrices](`#akash.market.v1beta5.OfferPrices`) | | |
+ | `prices` | [OfferPrices](`#akash.market.v1beta5.OfferPrices`) | | Prices is the optional per-resource pricing breakdown for this offer. When nil, no per-resource pricing is specified. |🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/proto/node.md` at line 4683, The generated docs show an empty
description for the ResourceOffer field `prices`; add a proto comment to the
`prices` field in the ResourceOffer message (the field typed as OfferPrices) in
the .proto source so the generator emits the description—locate the
ResourceOffer message, find the `prices` field, and add a clear one-line comment
describing what `prices` represents (e.g., pricing details for the resource
offer, currency, and units) so the docs include it.
4a08072 to
094badf
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
go/node/market/v1beta5/params.go (2)
51-60:⚠️ Potential issue | 🟠 Major
Validate()does not validateBidMinDeposits— invalid state can slip through genesis and CLI paths.
ParamSetPairsregistersvalidateCoinsforBidMinDeposits, but that validator is only invoked by the param-store onSet. Direct callers ofValidate()(e.g.,InitGenesis,ValidateBasic, CLI param-change proposals) will silently skip the new field.🐛 Proposed fix
func (p Params) Validate() error { if err := validateCoin(p.BidMinDeposit); err != nil { return err } + if err := validateCoins(p.BidMinDeposits); err != nil { + return err + } + if err := validateOrderMaxBids(p.OrderMaxBids); err != nil { return err } return nil }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@go/node/market/v1beta5/params.go` around lines 51 - 60, The Params.Validate() method is missing validation for the BidMinDeposits field so callers like InitGenesis and CLI paths can accept invalid state; add a call to the existing validateCoins validator inside Params.Validate() (e.g., if err := validateCoins(p.BidMinDeposits); err != nil { return err }) alongside the existing validateCoin and validateOrderMaxBids calls, referencing the BidMinDeposits field and validateCoins function to ensure the same checks enforced by ParamSetPairs are applied here and return the error directly.
51-60:⚠️ Potential issue | 🟠 Major
Validate()silently skipsBidMinDeposits— invalid state can escape genesis and CLI validation paths.
ParamSetPairswiresvalidateCoinsforBidMinDeposits, but that hook is only invoked by the param-store onSet. Any direct call toValidate()—InitGenesis,ValidateGenesis, CLI param-change proposals, unit tests — will pass regardless of the content ofBidMinDeposits.🐛 Proposed fix
func (p Params) Validate() error { if err := validateCoin(p.BidMinDeposit); err != nil { return err } + if err := validateCoins(p.BidMinDeposits); err != nil { + return err + } + if err := validateOrderMaxBids(p.OrderMaxBids); err != nil { return err } return nil }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@go/node/market/v1beta5/params.go` around lines 51 - 60, Params.Validate currently omits validating BidMinDeposits; update the Params.Validate method to invoke the same validator used in ParamSetPairs (validateCoins) on p.BidMinDeposits and return any error, so direct calls like InitGenesis/ValidateGenesis/CLI checks can't bypass coin-list validation; modify the Validate function (Params.Validate) to call validateCoins(p.BidMinDeposits) (or the project’s coin-list validator) and propagate the error.docs/proto/provider.md (1)
609-614:⚠️ Potential issue | 🟡 MinorAdd a description for
ServiceParams.permissions.
Line 613 has an empty description column; add a proto comment on thepermissionsfield so the generated docs populate.✏️ Expected doc output
- | `permissions` | [ServicePermissions](`#akash.manifest.v2beta3.ServicePermissions`) | | | + | `permissions` | [ServicePermissions](`#akash.manifest.v2beta3.ServicePermissions`) | | Permissions defines resource access permissions for the service. |🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/proto/provider.md` around lines 609 - 614, The docs show an empty description for ServiceParams.permissions; add a proto comment on the permissions field in the ServiceParams message so the generated docs populate. Edit the proto definition containing message ServiceParams and add a concise comment above the field named permissions (or the field declaration like "ServicePermissions permissions = <n>;") describing what permissions control (e.g., which service actions or access the provider allows) so the generated docs under ServiceParams.permissions include that description.
♻️ Duplicate comments (1)
docs/proto/provider.md (1)
620-623:⚠️ Potential issue | 🟡 MinorRemove leading space before the heading (MD023).
Line 622 starts with a space; markdownlint flags MD023. Since this file is generated, update the generator to emit headings flush-left.🔧 Example output
- ### ServicePermissions +### ServicePermissions🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/proto/provider.md` around lines 620 - 623, The generated Markdown contains a leading space before the heading for "ServicePermissions" (see the anchor "<a name=\"akash.manifest.v2beta3.ServicePermissions\"></a>" followed by " ### ServicePermissions"); update the generator/template that emits this block so headings are written flush-left (e.g., remove the extra space before "###") so the output no longer triggers MD023.
🤖 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/node/market/v1beta5/params.go`:
- Around line 71-78: The validateCoins function currently only checks the static
type; after asserting i to sdk.Coins (in validateCoins) call the
Coins.Validate() method and return a wrapped v1.ErrInvalidParam if validation
fails. Concretely: keep the type assertion to sdk.Coins, then call c.Validate()
(or coins.Validate()), and if that returns an error return fmt.Errorf("%w: %v",
v1.ErrInvalidParam, err); otherwise return nil. This ensures semantic checks
(positive amounts, no duplicates, sorted, valid denoms) are enforced.
- Around line 71-78: The validateCoins function only checks the static type and
misses semantic validation; after asserting the value to sdk.Coins (in
validateCoins) call its Validate() method and return any validation error
(wrapped or annotated with v1.ErrInvalidParam) so malformed coins (zero amounts,
duplicates, unsorted, invalid denoms) are rejected; keep the original type-check
error behavior but when the assertion succeeds run coins.Validate() and
propagate that error if non-nil.
---
Outside diff comments:
In `@docs/proto/provider.md`:
- Around line 609-614: The docs show an empty description for
ServiceParams.permissions; add a proto comment on the permissions field in the
ServiceParams message so the generated docs populate. Edit the proto definition
containing message ServiceParams and add a concise comment above the field named
permissions (or the field declaration like "ServicePermissions permissions =
<n>;") describing what permissions control (e.g., which service actions or
access the provider allows) so the generated docs under
ServiceParams.permissions include that description.
In `@go/node/market/v1beta5/params.go`:
- Around line 51-60: The Params.Validate() method is missing validation for the
BidMinDeposits field so callers like InitGenesis and CLI paths can accept
invalid state; add a call to the existing validateCoins validator inside
Params.Validate() (e.g., if err := validateCoins(p.BidMinDeposits); err != nil {
return err }) alongside the existing validateCoin and validateOrderMaxBids
calls, referencing the BidMinDeposits field and validateCoins function to ensure
the same checks enforced by ParamSetPairs are applied here and return the error
directly.
- Around line 51-60: Params.Validate currently omits validating BidMinDeposits;
update the Params.Validate method to invoke the same validator used in
ParamSetPairs (validateCoins) on p.BidMinDeposits and return any error, so
direct calls like InitGenesis/ValidateGenesis/CLI checks can't bypass coin-list
validation; modify the Validate function (Params.Validate) to call
validateCoins(p.BidMinDeposits) (or the project’s coin-list validator) and
propagate the error.
---
Duplicate comments:
In `@docs/proto/provider.md`:
- Around line 620-623: The generated Markdown contains a leading space before
the heading for "ServicePermissions" (see the anchor "<a
name=\"akash.manifest.v2beta3.ServicePermissions\"></a>" followed by " ###
ServicePermissions"); update the generator/template that emits this block so
headings are written flush-left (e.g., remove the extra space before "###") so
the output no longer triggers MD023.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (13)
go/manifest/v2beta3/service.pb.gois excluded by!**/*.pb.gogo/node/market/v1beta5/params.pb.gois excluded by!**/*.pb.gogo/node/market/v1beta5/resourcesoffer.pb.gois excluded by!**/*.pb.gots/src/generated/createNodeSDK.tsis excluded by!**/generated/**ts/src/generated/protos/akash/bme/v1/filters.tsis excluded by!**/generated/**ts/src/generated/protos/akash/bme/v1/query.tsis excluded by!**/generated/**ts/src/generated/protos/akash/bme/v1/query_akash.tsis excluded by!**/generated/**ts/src/generated/protos/akash/market/v1beta5/params.tsis excluded by!**/generated/**ts/src/generated/protos/akash/market/v1beta5/resourcesoffer.tsis excluded by!**/generated/**ts/src/generated/protos/akash/oracle/v1/params.tsis excluded by!**/generated/**ts/src/generated/protos/google/protobuf/descriptor.tsis excluded by!**/generated/**ts/src/generated/protos/index.akash.v1.tsis excluded by!**/generated/**ts/src/generated/protos/index.akash.v1beta5.tsis excluded by!**/generated/**
📒 Files selected for processing (6)
docs/proto/node.mddocs/proto/provider.mddocs/swagger-ui/swagger.yamlgo/node/market/v1beta5/params.goproto/node/akash/market/v1beta5/params.protoproto/node/akash/market/v1beta5/resourcesoffer.proto
🚧 Files skipped from review as they are similar to previous changes (1)
- proto/node/akash/market/v1beta5/params.proto
Comment on lines
+1606
to
+1612
| ## akash/bme/v1/filters.proto | ||
|
|
||
|
|
||
|
|
||
| <a name="akash.bme.v1.LedgerRecordFilters"></a> | ||
|
|
||
| ### LedgerRecordFilters |
Contributor
There was a problem hiding this comment.
Trim leading spaces on the new headings (MD023).
Lines 1606 and 1612 start with a space; markdownlint MD023. Since this is generated output, update the generator to emit headings flush-left (this will also fix other newly added headings in this doc).
🔧 Example output
- ## akash/bme/v1/filters.proto
+## akash/bme/v1/filters.proto
- ### LedgerRecordFilters
+### LedgerRecordFilters📝 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
| ## akash/bme/v1/filters.proto | |
| <a name="akash.bme.v1.LedgerRecordFilters"></a> | |
| ### LedgerRecordFilters | |
| ## akash/bme/v1/filters.proto | |
| <a name="akash.bme.v1.LedgerRecordFilters"></a> | |
| ### LedgerRecordFilters |
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 1606-1606: Headings must start at the beginning of the line
(MD023, heading-start-left)
[warning] 1612-1612: Headings must start at the beginning of the line
(MD023, heading-start-left)
Signed-off-by: Artur Troian <troian@users.noreply.github.com>
troian
force-pushed
the
feat/market-offer-prices-proto
branch
from
094badf to
ab882d9
Compare
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
go/node/market/v1beta5/params.go (1)
51-60:⚠️ Potential issue | 🟠 Major
Params.Validate()does not validate the newBidMinDepositsfield.
validateCoinsis registered inParamSetPairs(storage-level), butParams.Validate()— called during genesis and direct struct validation — never checksBidMinDeposits. This means malformed multi-coin deposits can bypass validation when the struct is validated directly.🛡️ Proposed fix
func (p Params) Validate() error { if err := validateCoin(p.BidMinDeposit); err != nil { return err } + if err := validateCoins(p.BidMinDeposits); err != nil { + return err + } + if err := validateOrderMaxBids(p.OrderMaxBids); err != nil { return err } return nil }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@go/node/market/v1beta5/params.go` around lines 51 - 60, Params.Validate currently validates BidMinDeposit and OrderMaxBids but ignores the new BidMinDeposits field; update Params.Validate to also validate the multi-coin field (BidMinDeposits) by calling the existing validator (validateCoins) and returning any error, similarly to how BidMinDeposit uses validateCoin, so the function (Params.Validate) checks validateCoins(p.BidMinDeposits) before returning nil.
♻️ Duplicate comments (8)
docs/proto/provider.md (1)
620-630:⚠️ Potential issue | 🟡 MinorLeading space on heading still triggers MD023.
Line 622's
### ServicePermissionsheading still has a leading space per the markdownlint warning. Since the file is auto-generated, the generator template needs to be fixed to emit headings flush-left.Additionally, lines 623–625 show a multi-line description block that may not render as intended in Markdown — the continuation lines (
Resources map to Kubernetes RBAC permissions:and the indented- logs) lack a blank-line separator from the preceding sentence, which can cause them to collapse into a single paragraph rather than rendering the bullet list.Since this file is auto-generated, both issues should be addressed in the doc generator.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/proto/provider.md` around lines 620 - 630, The generated "ServicePermissions" section has a leading space before the "### ServicePermissions" heading and a missing blank line before the "Resources map to Kubernetes RBAC permissions:" paragraph and its bullet list; update the doc generator template that emits this block so headings are emitted flush-left (no leading space) and ensure a blank line is inserted between the preceding sentence and the subsequent paragraph/list so the "- logs" bullet renders properly; locate the template that formats the ServicePermissions block (search for the "ServicePermissions" heading emitter) and change it to trim leading whitespace on headings and to print a blank line before multi-line descriptions and lists.docs/swagger-ui/swagger.yaml (6)
51626-51776: Same as earlier: prefer$reftoOfferPricesto avoid schema drift.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/swagger-ui/swagger.yaml` around lines 51626 - 51776, The inline schema under the "prices" property should be replaced with a $ref to the existing OfferPrices definition to avoid schema drift; update the "prices" property (and the repeated item schemas for storage and endpoints) to reference the canonical components/schemas OfferPrices, StorageOfferPrice and EndpointOfferPrice instead of duplicating object/property definitions so the file uses $ref for OfferPrices (and nested storage/endpoints types) throughout.
50390-50540: Same as earlier: prefer$reftoOfferPricesto avoid schema drift.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/swagger-ui/swagger.yaml` around lines 50390 - 50540, The prices object in the Offer/OpenAPI schema is defined inline (properties cpu, memory, storage, gpu, endpoints) which risks schema drift; replace the inline definition under the prices property with a $ref to the existing OfferPrices schema (use "$ref": "#/components/schemas/OfferPrices" or the project's equivalent) so the Offer's prices field reuses the single source of truth (look for the prices property in the Offer schema and the OfferPrices component).
3837-3992: Same as earlier: prefer$reftoOfferPricesto avoid schema drift.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/swagger-ui/swagger.yaml` around lines 3837 - 3992, The "prices" schema is duplicated inline causing schema drift; replace the inline object under the "prices" property with a $ref to the canonical OfferPrices schema (e.g., use a $ref to the existing OfferPrices component/schema) and remove the expanded nested definitions (cpu, memory, storage, gpu, endpoints) from this location; ensure consumers rely on OfferPrices (and update any references to nested types if OfferPrices encapsulates them) so there's a single source-of-truth for pricing.
52264-52414: Same as earlier: prefer$reftoOfferPricesto avoid schema drift.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/swagger-ui/swagger.yaml` around lines 52264 - 52414, The inline "prices" schema and its nested types (cpu, memory, storage, gpu, endpoints) should be replaced with a $ref to the canonical OfferPrices schema to avoid schema drift; update the "prices" property to use $ref: '#/components/schemas/OfferPrices' and replace storage.items and endpoints.items inline objects with $ref references to the corresponding canonical schemas (e.g., StorageOfferPrice and EndpointOfferPrice) so the spec reuses the existing component definitions (ensure those component schemas exist and remove the duplicated inline definitions).
51267-51304: Same as earlier: makebid_min_depositdeprecation machine‑readable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/swagger-ui/swagger.yaml` around lines 51267 - 51304, The bid_min_deposit schema is only marked as deprecated in its title string; make this machine-readable by adding a deprecation flag (e.g., add deprecated: true and/or x-deprecated: true) to the bid_min_deposit schema object, and optionally remove the "Deprecated:" prefix from the title to avoid duplication; update the schema named bid_min_deposit (and not bid_min_deposits) so tooling can detect the deprecation automatically.
53925-53967: Same as earlier: makebid_min_depositdeprecation machine‑readable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/swagger-ui/swagger.yaml` around lines 53925 - 53967, The schema currently marks "BidMinDeposit" as deprecated only in the title text; make the deprecation machine-readable by adding a deprecated: true flag on the deprecated property (the schema that defines BidMinDeposit / property name bid_min_deposit) and include a clear replacement pointer (e.g., add an x-deprecated-note or x-deprecation-message saying "use bid_min_deposits") in that property's schema; update the object that currently has title "BidMinDeposit is a parameter..." to include deprecated: true and the x-deprecation message alongside the description so tools can detect and surface the deprecation automatically.docs/proto/node.md (1)
1606-1612:⚠️ Potential issue | 🟡 MinorTrim leading spaces on generated headings to satisfy MD023.
These headings still start with a leading space (e.g., Line 1606, Line 1612, Line 1960, Line 1978, Line 1994, Line 4626, Line 4646, Line 4692, Line 5756). Since this file is generated, please adjust the doc generator to emit headings flush-left.
Also applies to: 1958-1995, 4624-4692, 5754-5757
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/proto/node.md` around lines 1606 - 1612, The generated Markdown contains headings with a leading space (for example the "## akash/bme/v1/filters.proto" and "### LedgerRecordFilters" blocks and the anchor "akash.bme.v1.LedgerRecordFilters"); fix the doc generator so it emits headings flush-left by trimming any leading whitespace before writing heading lines (or update the template to remove indentation), ensuring all generated headings (e.g., those around lines with LedgerRecordFilters and akash/bme/v1/filters.proto) are written without a leading space.
🧹 Nitpick comments (2)
docs/swagger-ui/swagger.yaml (2)
6177-6222: Makebid_min_depositdeprecation machine‑readable.The deprecation is only in text. Consider adding the appropriate deprecation flag (e.g.,
x-deprecated: truefor OAS2 ordeprecated: truefor OAS3) so generators can surface it.🧾 Example (if OAS2)
bid_min_deposit: + x-deprecated: true title: >- BidMinDeposit is a parameter for the minimum deposit on a Bid.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/swagger-ui/swagger.yaml` around lines 6177 - 6222, The bid_min_deposit schema is only marked deprecated in its description text; make the deprecation machine-readable by adding the appropriate deprecation flag under the bid_min_deposit schema (e.g., add deprecated: true if this OpenAPI is v3, or x-deprecated: true for v2/OAS2) so generators can detect it; update the bid_min_deposit object (the schema labeled bid_min_deposit / title "BidMinDeposit") accordingly and keep the human-readable deprecation text if desired to preserve context.
3080-3231: Consider referencing the sharedOfferPricesschema instead of inlining it.This block is repeated in several places; using
$refto the new definitions reduces drift risk and eases maintenance.♻️ Example refactor
- prices: - description: >- - Prices contains per-resource pricing details (CPU, - memory, storage, GPU, endpoints) for this offer. - type: object - properties: - cpu: - ... + prices: + $ref: '#/definitions/akash.market.v1beta5.OfferPrices'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/swagger-ui/swagger.yaml` around lines 3080 - 3231, The inline "prices" object and its nested types (cpu, memory, storage/StorageOfferPrice, gpu, endpoints/EndpointOfferPrice) should be extracted into a reusable schema named OfferPrices (including StorageOfferPrice and EndpointOfferPrice as component schemas that preserve the enum, descriptions and properties such as denom/amount, name, price, kind, etc.), then replace this entire inline object under the "prices" property with a $ref to the new OfferPrices schema (e.g., $ref: '#/components/schemas/OfferPrices'); also find and replace other repeated blocks in the spec that currently duplicate these structures to reference the same OfferPrices/StorageOfferPrice/EndpointOfferPrice components.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@go/node/market/v1beta5/params.go`:
- Around line 51-60: Params.Validate currently validates BidMinDeposit and
OrderMaxBids but ignores the new BidMinDeposits field; update Params.Validate to
also validate the multi-coin field (BidMinDeposits) by calling the existing
validator (validateCoins) and returning any error, similarly to how
BidMinDeposit uses validateCoin, so the function (Params.Validate) checks
validateCoins(p.BidMinDeposits) before returning nil.
---
Duplicate comments:
In `@docs/proto/node.md`:
- Around line 1606-1612: The generated Markdown contains headings with a leading
space (for example the "## akash/bme/v1/filters.proto" and "###
LedgerRecordFilters" blocks and the anchor "akash.bme.v1.LedgerRecordFilters");
fix the doc generator so it emits headings flush-left by trimming any leading
whitespace before writing heading lines (or update the template to remove
indentation), ensuring all generated headings (e.g., those around lines with
LedgerRecordFilters and akash/bme/v1/filters.proto) are written without a
leading space.
In `@docs/proto/provider.md`:
- Around line 620-630: The generated "ServicePermissions" section has a leading
space before the "### ServicePermissions" heading and a missing blank line
before the "Resources map to Kubernetes RBAC permissions:" paragraph and its
bullet list; update the doc generator template that emits this block so headings
are emitted flush-left (no leading space) and ensure a blank line is inserted
between the preceding sentence and the subsequent paragraph/list so the "- logs"
bullet renders properly; locate the template that formats the ServicePermissions
block (search for the "ServicePermissions" heading emitter) and change it to
trim leading whitespace on headings and to print a blank line before multi-line
descriptions and lists.
In `@docs/swagger-ui/swagger.yaml`:
- Around line 51626-51776: The inline schema under the "prices" property should
be replaced with a $ref to the existing OfferPrices definition to avoid schema
drift; update the "prices" property (and the repeated item schemas for storage
and endpoints) to reference the canonical components/schemas OfferPrices,
StorageOfferPrice and EndpointOfferPrice instead of duplicating object/property
definitions so the file uses $ref for OfferPrices (and nested storage/endpoints
types) throughout.
- Around line 50390-50540: The prices object in the Offer/OpenAPI schema is
defined inline (properties cpu, memory, storage, gpu, endpoints) which risks
schema drift; replace the inline definition under the prices property with a
$ref to the existing OfferPrices schema (use "$ref":
"#/components/schemas/OfferPrices" or the project's equivalent) so the Offer's
prices field reuses the single source of truth (look for the prices property in
the Offer schema and the OfferPrices component).
- Around line 3837-3992: The "prices" schema is duplicated inline causing schema
drift; replace the inline object under the "prices" property with a $ref to the
canonical OfferPrices schema (e.g., use a $ref to the existing OfferPrices
component/schema) and remove the expanded nested definitions (cpu, memory,
storage, gpu, endpoints) from this location; ensure consumers rely on
OfferPrices (and update any references to nested types if OfferPrices
encapsulates them) so there's a single source-of-truth for pricing.
- Around line 52264-52414: The inline "prices" schema and its nested types (cpu,
memory, storage, gpu, endpoints) should be replaced with a $ref to the canonical
OfferPrices schema to avoid schema drift; update the "prices" property to use
$ref: '#/components/schemas/OfferPrices' and replace storage.items and
endpoints.items inline objects with $ref references to the corresponding
canonical schemas (e.g., StorageOfferPrice and EndpointOfferPrice) so the spec
reuses the existing component definitions (ensure those component schemas exist
and remove the duplicated inline definitions).
- Around line 51267-51304: The bid_min_deposit schema is only marked as
deprecated in its title string; make this machine-readable by adding a
deprecation flag (e.g., add deprecated: true and/or x-deprecated: true) to the
bid_min_deposit schema object, and optionally remove the "Deprecated:" prefix
from the title to avoid duplication; update the schema named bid_min_deposit
(and not bid_min_deposits) so tooling can detect the deprecation automatically.
- Around line 53925-53967: The schema currently marks "BidMinDeposit" as
deprecated only in the title text; make the deprecation machine-readable by
adding a deprecated: true flag on the deprecated property (the schema that
defines BidMinDeposit / property name bid_min_deposit) and include a clear
replacement pointer (e.g., add an x-deprecated-note or x-deprecation-message
saying "use bid_min_deposits") in that property's schema; update the object that
currently has title "BidMinDeposit is a parameter..." to include deprecated:
true and the x-deprecation message alongside the description so tools can detect
and surface the deprecation automatically.
---
Nitpick comments:
In `@docs/swagger-ui/swagger.yaml`:
- Around line 6177-6222: The bid_min_deposit schema is only marked deprecated in
its description text; make the deprecation machine-readable by adding the
appropriate deprecation flag under the bid_min_deposit schema (e.g., add
deprecated: true if this OpenAPI is v3, or x-deprecated: true for v2/OAS2) so
generators can detect it; update the bid_min_deposit object (the schema labeled
bid_min_deposit / title "BidMinDeposit") accordingly and keep the human-readable
deprecation text if desired to preserve context.
- Around line 3080-3231: The inline "prices" object and its nested types (cpu,
memory, storage/StorageOfferPrice, gpu, endpoints/EndpointOfferPrice) should be
extracted into a reusable schema named OfferPrices (including StorageOfferPrice
and EndpointOfferPrice as component schemas that preserve the enum, descriptions
and properties such as denom/amount, name, price, kind, etc.), then replace this
entire inline object under the "prices" property with a $ref to the new
OfferPrices schema (e.g., $ref: '#/components/schemas/OfferPrices'); also find
and replace other repeated blocks in the spec that currently duplicate these
structures to reference the same
OfferPrices/StorageOfferPrice/EndpointOfferPrice components.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (13)
go/manifest/v2beta3/service.pb.gois excluded by!**/*.pb.gogo/node/market/v1beta5/params.pb.gois excluded by!**/*.pb.gogo/node/market/v1beta5/resourcesoffer.pb.gois excluded by!**/*.pb.gots/src/generated/createNodeSDK.tsis excluded by!**/generated/**ts/src/generated/protos/akash/bme/v1/filters.tsis excluded by!**/generated/**ts/src/generated/protos/akash/bme/v1/query.tsis excluded by!**/generated/**ts/src/generated/protos/akash/bme/v1/query_akash.tsis excluded by!**/generated/**ts/src/generated/protos/akash/market/v1beta5/params.tsis excluded by!**/generated/**ts/src/generated/protos/akash/market/v1beta5/resourcesoffer.tsis excluded by!**/generated/**ts/src/generated/protos/akash/oracle/v1/params.tsis excluded by!**/generated/**ts/src/generated/protos/google/protobuf/descriptor.tsis excluded by!**/generated/**ts/src/generated/protos/index.akash.v1.tsis excluded by!**/generated/**ts/src/generated/protos/index.akash.v1beta5.tsis excluded by!**/generated/**
📒 Files selected for processing (6)
docs/proto/node.mddocs/proto/provider.mddocs/swagger-ui/swagger.yamlgo/node/market/v1beta5/params.goproto/node/akash/market/v1beta5/params.protoproto/node/akash/market/v1beta5/resourcesoffer.proto
🚧 Files skipped from review as they are similar to previous changes (1)
- proto/node/akash/market/v1beta5/params.proto
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
[Explain what this PR does in 2-3 sentences. Include context about the feature or problem being solved]
🔧 Purpose of the Change
📌 Related Issues
✅ Checklist
📎 Notes for Reviewers
[Include any additional context, architectural decisions, or specific areas to focus on]