feat(provider): add provider bid pre-check support types #218
Conversation
WalkthroughThis change introduces a new bid pre-check feature to the provider service, adding new protobuf messages and an RPC method for bid eligibility checks. The Go client interface and implementation are updated to use the new request and response types. Test utilities are extended with helper functions for generating group specs. Additionally, new methods for attribute comparison and string formatting are added with corresponding tests. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant ProviderRPC
Client->>ProviderRPC: BidPreCheck(BidPreCheckRequest)
ProviderRPC-->>Client: BidPreCheckResponse (group bid checks, total price)
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (3)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
go/testutil/v1beta3/deployment.go (1)
53-63: Avoid code duplication – reuse existingGroupSpechelper
GroupSpecsduplicates the literal construction already encapsulated byGroupSpec.
Consider building on the single-item helper to keep one source of truth and prevent drift.-func GroupSpecs(t testing.TB) dtypes.GroupSpecs { - t.Helper() - return dtypes.GroupSpecs{ - &dtypes.GroupSpec{ - Name: testutil.Name(t, "dgroup"), - Requirements: PlacementRequirements(t), - Resources: ResourcesList(t, 1), - }, - } +func GroupSpecs(t testing.TB) dtypes.GroupSpecs { + t.Helper() + gs := GroupSpec(t) // reuse existing generator + return dtypes.GroupSpecs{&gs} }go/testutil/deployment.go (1)
52-62: DRY: build slice fromGroupSpecinstead of repeating struct literalSame duplication comment as in
v1beta3package; leveraging the existing generator reduces maintenance overhead and keeps behaviour identical.proto/provider/akash/provider/v1/service.proto (1)
78-90: Typo in comment & lingering TODO
- Comment says
PreBidCheckResponsewhile the message isBidPreCheckResponse.// TODO: Support different coins?should be turned into a tracked issue or removed before merging.go/provider/client/client.go (1)
383-394: Validation loop fine, but accept empty slice?
gspecsof length 0 passes the loop silently and leads to an empty request – does the provider accept that?
Consider returningErrInvalidRequestwhenlen(gspecs)==0to surface accidental misuse.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (10)
go/provider/v1/service.pb.gois excluded by!**/*.pb.gogo/provider/v1/service.pb.gw.gois excluded by!**/*.pb.gw.gots/src/generated/akash/inventory/v1/resourcepair.tsis excluded by!**/generated/**ts/src/generated/akash/provider/v1/service.grpc-js.tsis excluded by!**/generated/**ts/src/generated/akash/provider/v1/service.tsis excluded by!**/generated/**ts/src/generated/cosmos/base/v1beta1/coin.original.tsis excluded by!**/generated/**ts/src/generated/cosmos/base/v1beta1/coin.tsis excluded by!**/generated/**ts/src/generated/index.akash.base.v1beta3.tsis excluded by!**/generated/**ts/src/generated/index.akash.tsis excluded by!**/generated/**ts/src/generated/index.tsis excluded by!**/generated/**
📒 Files selected for processing (6)
docs/proto/provider.md(3 hunks)go/provider/client/client.go(6 hunks)go/provider/client/types.go(0 hunks)go/testutil/deployment.go(1 hunks)go/testutil/v1beta3/deployment.go(1 hunks)proto/provider/akash/provider/v1/service.proto(2 hunks)
💤 Files with no reviewable changes (1)
- go/provider/client/types.go
🧰 Additional context used
🧬 Code Graph Analysis (1)
go/testutil/deployment.go (4)
go/testutil/v1beta3/deployment.go (2)
GroupSpecs(54-63)GroupSpec(44-51)go/node/deployment/v1beta3/groupspec.go (1)
GroupSpecs(19-19)go/testutil/v1beta2/deployment.go (1)
GroupSpec(44-51)go/testutil/base.go (3)
Name(25-27)PlacementRequirements(57-61)Resources(89-120)
🪛 markdownlint-cli2 (0.17.2)
docs/proto/provider.md
49-49: Unordered list indentation
Expected: 2; Actual: 5
(MD007, ul-indent)
50-50: Unordered list indentation
Expected: 2; Actual: 5
(MD007, ul-indent)
51-51: Unordered list indentation
Expected: 2; Actual: 5
(MD007, ul-indent)
656-656: Headings must start at the beginning of the line
null
(MD023, heading-start-left)
662-662: Link fragments should be valid
null
(MD051, link-fragments)
671-671: Headings must start at the beginning of the line
null
(MD023, heading-start-left)
678-678: Link fragments should be valid
null
(MD051, link-fragments)
687-687: Headings must start at the beginning of the line
null
(MD023, heading-start-left)
694-694: Link fragments should be valid
null
(MD051, link-fragments)
🪛 LanguageTool
docs/proto/provider.md
[duplication] ~656-~656: Possible typo: you repeated a word.
Context: ...vider.v1.BidPreCheckRequest"> ### BidPreCheckRequest BidPreCheckRequest is request type for the BidPreCheck RPC...
(ENGLISH_WORD_REPEAT_RULE)
[duplication] ~687-~687: Possible typo: you repeated a word.
Context: ...rovider.v1.GroupBidPreCheck"> ### GroupBidPreCheck GroupBidPreCheck contains bid information for a specific...
(ENGLISH_WORD_REPEAT_RULE)
🪛 Buf (1.54.0)
proto/provider/akash/provider/v1/service.proto
4-4: import "gogoproto/gogo.proto": file does not exist
(COMPILE)
🔇 Additional comments (2)
proto/provider/akash/provider/v1/service.proto (1)
31-40: HTTP verb mismatch with client implementationRPC is exposed as
POST /v1/bidprecheckbut the REST client builds a GET request.
Either change the option here toget:or update the client tohttp.MethodPost; both sides must agree.docs/proto/provider.md (1)
48-52: Auto-generated file – lint warnings acceptableMarkdown-lint flags indentation and heading issues, but this file is generated; fixing manually will be overwritten.
No action required.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
48-48: Unordered list indentation
Expected: 0; Actual: 1(MD007, ul-indent)
49-49: Unordered list indentation
Expected: 2; Actual: 5(MD007, ul-indent)
50-50: Unordered list indentation
Expected: 2; Actual: 5(MD007, ul-indent)
51-51: Unordered list indentation
Expected: 2; Actual: 5(MD007, ul-indent)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
go/node/types/v1beta3/attribute.go (1)
403-416: Rename shadowed loop variable to avoid confusionThe inner
forre-uses the identifierattr, shadowing the receiver variable.
While it doesn’t break functionality, it hampers readability and can lead to subtle bugs when the code is later modified.- for _, attr := range b { - if req.SubsetOf(attr) { + for _, candidate := range b { + if req.SubsetOf(candidate) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
go/node/types/v1beta3/attribute.go(2 hunks)go/node/types/v1beta3/attribute_test.go(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: test
- GitHub Check: coverage
- GitHub Check: go
🔇 Additional comments (2)
go/node/types/v1beta3/attribute_test.go (2)
184-214: 👍 Good coverage forGetNonMatchingAttributesThe added test cases exercise the typical, empty, and no-match scenarios and will catch regressions in the matching logic.
No changes needed.
216-259: Test expectations correctly reflect the desiredString()outputOnce the production fix (remove the extra space) is applied, these test cases will pass and will guard against format regressions.
No action required on the test side.
This PR adds types and services for bid pre-checks and related test utilities.
Summary by CodeRabbit
New Features
Documentation