Conversation
|
The latest Buf updates on your PR. Results from workflow Protobuf / buf-build (pull_request).
|
WalkthroughThis PR removes two Buf-related GitHub Actions workflows, introduces a consolidated Protobuf workflow, updates the Buf version to 0.16.0, reorganizes proto configuration and imports, and extends marketmap and oracle proto messages with new fields, RPC methods, and standardized formatting conventions. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Comment Tip You can disable the changed files summary in the walkthrough.Disable the |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
.github/workflows/proto.yml (2)
13-16: Add explicitpull_requestevent types if usingbuf skip breakinglabel for re-runs.According to Buf's official documentation, the workflow must include
labeledandunlabeledevent types so toggling thebuf skip breakinglabel re-runs checks. Without these types, the workflow will not respond to label changes.🔧 Suggested trigger update
pull_request: + types: [opened, synchronize, reopened, labeled, unlabeled] paths: - "proto/**" - ".github/workflows/proto.yml"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/proto.yml around lines 13 - 16, The pull_request trigger must include explicit event types so label changes (like toggling the "buf skip breaking" label) re-run the workflow; update the pull_request trigger in the workflow to add types including at minimum "labeled" and "unlabeled" (you can also keep other types you need such as "opened", "synchronize", "reopened"); ensure the YAML block containing pull_request (the existing pull_request: paths: ...) is modified to include a types: [labeled, unlabeled, ...] entry so label toggles correctly retrigger the Buf check.
28-29: Pin GitHub Actions to immutable commit SHAs.Using moving major tags (
@v6,@v1) weakens workflow reproducibility and supply-chain hardening.🔧 Suggested hardening
- - uses: actions/checkout@v6 - - uses: bufbuild/buf-action@v1 + - uses: actions/checkout@<full-length-commit-sha> + - uses: bufbuild/buf-action@<full-length-commit-sha>To find the commit SHAs, check the release page for each action or use:
git ls-remote https://github.com/actions/checkout refs/tags/v6 | awk '{print $1}' git ls-remote https://github.com/bufbuild/buf-action refs/tags/v1 | awk '{print $1}'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/proto.yml around lines 28 - 29, Replace the floating action tags with immutable commit SHAs: locate the uses of actions/checkout@v6 and bufbuild/buf-action@v1 in the workflow and replace each tag with the corresponding full commit SHA for the specific released tag (e.g., actions/checkout@<FULL_SHA> and bufbuild/buf-action@<FULL_SHA>); verify the SHAs against each repo's release/tag to ensure accuracy and update the workflow file so the two identified lines reference the immutable SHAs instead of v6 and v1.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/proto.yml:
- Around line 13-16: The pull_request trigger must include explicit event types
so label changes (like toggling the "buf skip breaking" label) re-run the
workflow; update the pull_request trigger in the workflow to add types including
at minimum "labeled" and "unlabeled" (you can also keep other types you need
such as "opened", "synchronize", "reopened"); ensure the YAML block containing
pull_request (the existing pull_request: paths: ...) is modified to include a
types: [labeled, unlabeled, ...] entry so label toggles correctly retrigger the
Buf check.
- Around line 28-29: Replace the floating action tags with immutable commit
SHAs: locate the uses of actions/checkout@v6 and bufbuild/buf-action@v1 in the
workflow and replace each tag with the corresponding full commit SHA for the
specific released tag (e.g., actions/checkout@<FULL_SHA> and
bufbuild/buf-action@<FULL_SHA>); verify the SHAs against each repo's release/tag
to ensure accuracy and update the workflow file so the two identified lines
reference the immutable SHAs instead of v6 and v1.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2639df45-b636-447f-8176-1656987385f4
⛔ Files ignored due to path filters (10)
api/connect/marketmap/v2/query_grpc.pb.gois excluded by!**/*.pb.goapi/connect/marketmap/v2/tx_grpc.pb.gois excluded by!**/*.pb.goapi/connect/oracle/v2/query_grpc.pb.gois excluded by!**/*.pb.goapi/connect/oracle/v2/tx_grpc.pb.gois excluded by!**/*.pb.goservice/servers/oracle/types/oracle.pb.gois excluded by!**/*.pb.gox/marketmap/types/market.pb.gois excluded by!**/*.pb.gox/marketmap/types/query.pb.gois excluded by!**/*.pb.gox/marketmap/types/tx.pb.gois excluded by!**/*.pb.gox/oracle/types/query.pb.gois excluded by!**/*.pb.gox/oracle/types/tx.pb.gois excluded by!**/*.pb.go
📒 Files selected for processing (17)
.github/workflows/buf_breaking.yml.github/workflows/buf_push.yml.github/workflows/proto.ymlMakefileapi/connect/marketmap/v2/market.pulsar.goapi/connect/marketmap/v2/query.pulsar.goapi/connect/marketmap/v2/tx.pulsar.goapi/connect/oracle/v2/genesis.pulsar.goapi/connect/oracle/v2/query.pulsar.goapi/connect/oracle/v2/tx.pulsar.goproto/buf.yamlproto/connect/marketmap/v2/market.protoproto/connect/marketmap/v2/query.protoproto/connect/marketmap/v2/tx.protoproto/connect/oracle/v2/query.protoproto/connect/oracle/v2/tx.protoproto/connect/service/v2/oracle.proto
💤 Files with no reviewable changes (3)
- .github/workflows/buf_push.yml
- .github/workflows/buf_breaking.yml
- proto/connect/service/v2/oracle.proto
9617c4c to
937f475
Compare
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)
proto/connect/oracle/v2/tx.proto (1)
54-57:⚠️ Potential issue | 🟡 MinorAmino name mismatch with message name.
The
amino.nameoption is set to"connect/x/oracle/MsgSetCurrencyPairs"but the message isMsgRemoveCurrencyPairs. This is a copy-paste error. The amino name should be"connect/x/oracle/MsgRemoveCurrencyPairs"to match the message name, consistent with the pattern used inMsgAddCurrencyPairsabove.🔧 Proposed fix
message MsgRemoveCurrencyPairs { option (cosmos.msg.v1.signer) = "authority"; - option (amino.name) = "connect/x/oracle/MsgSetCurrencyPairs"; + option (amino.name) = "connect/x/oracle/MsgRemoveCurrencyPairs";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@proto/connect/oracle/v2/tx.proto` around lines 54 - 57, The proto message MsgRemoveCurrencyPairs has an incorrect amino.name option (copy-pasted as "connect/x/oracle/MsgSetCurrencyPairs"); update the option in the MsgRemoveCurrencyPairs message to option (amino.name) = "connect/x/oracle/MsgRemoveCurrencyPairs" so the amino name matches the message symbol MsgRemoveCurrencyPairs (follow the same pattern used by MsgAddCurrencyPairs).
🧹 Nitpick comments (1)
proto/connect/oracle/v2/query.proto (1)
32-37: Emptyadditional_bindingsarrays appear unnecessary.The
additional_bindings: []declarations on lines 35-36 and 45-46 define empty arrays that don't add any HTTP bindings. Unless there's a specific reason (e.g., placeholder for future bindings or a tooling requirement), these could be removed for cleanliness.♻️ Proposed cleanup
rpc GetCurrencyPairMapping(GetCurrencyPairMappingRequest) returns (GetCurrencyPairMappingResponse) { - option (google.api.http) = { - get: "/connect/oracle/v2/get_currency_pair_mapping" - additional_bindings: [] - }; + option (google.api.http) = {get: "/connect/oracle/v2/get_currency_pair_mapping"}; } rpc GetCurrencyPairMappingList(GetCurrencyPairMappingListRequest) returns (GetCurrencyPairMappingListResponse) { - option (google.api.http) = { - get: "/connect/oracle/v2/get_currency_pair_mapping_list" - additional_bindings: [] - }; + option (google.api.http) = {get: "/connect/oracle/v2/get_currency_pair_mapping_list"}; }Also applies to: 42-47
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@proto/connect/oracle/v2/query.proto` around lines 32 - 37, Remove the empty additional_bindings: [] entries in the gRPC HTTP options for the RPCs (e.g., rpc GetCurrencyPairMapping(GetCurrencyPairMappingRequest) and the other affected RPC shown around lines 42-47) — these empty arrays are no-ops; edit the option blocks for GetCurrencyPairMapping and the other RPC to omit the additional_bindings field entirely so the (google.api.http) option only contains the relevant get: "/connect/oracle/v2/get_currency_pair_mapping" (and the corresponding path for the other RPC).
🤖 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 `@proto/connect/oracle/v2/tx.proto`:
- Around line 54-57: The proto message MsgRemoveCurrencyPairs has an incorrect
amino.name option (copy-pasted as "connect/x/oracle/MsgSetCurrencyPairs");
update the option in the MsgRemoveCurrencyPairs message to option (amino.name) =
"connect/x/oracle/MsgRemoveCurrencyPairs" so the amino name matches the message
symbol MsgRemoveCurrencyPairs (follow the same pattern used by
MsgAddCurrencyPairs).
---
Nitpick comments:
In `@proto/connect/oracle/v2/query.proto`:
- Around line 32-37: Remove the empty additional_bindings: [] entries in the
gRPC HTTP options for the RPCs (e.g., rpc
GetCurrencyPairMapping(GetCurrencyPairMappingRequest) and the other affected RPC
shown around lines 42-47) — these empty arrays are no-ops; edit the option
blocks for GetCurrencyPairMapping and the other RPC to omit the
additional_bindings field entirely so the (google.api.http) option only contains
the relevant get: "/connect/oracle/v2/get_currency_pair_mapping" (and the
corresponding path for the other RPC).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2b93765d-374e-48dd-9092-54582e335767
📒 Files selected for processing (15)
.github/workflows/buf_breaking.yml.github/workflows/buf_push.yml.github/workflows/proto.ymlMakefileproto/buf.yamlproto/connect/marketmap/module/v2/module.protoproto/connect/marketmap/v2/genesis.protoproto/connect/marketmap/v2/market.protoproto/connect/marketmap/v2/query.protoproto/connect/marketmap/v2/tx.protoproto/connect/oracle/module/v2/module.protoproto/connect/oracle/v2/genesis.protoproto/connect/oracle/v2/query.protoproto/connect/oracle/v2/tx.protoproto/connect/service/v2/oracle.proto
💤 Files with no reviewable changes (2)
- .github/workflows/buf_push.yml
- .github/workflows/buf_breaking.yml
✅ Files skipped from review due to trivial changes (2)
- proto/connect/marketmap/v2/genesis.proto
- proto/connect/oracle/module/v2/module.proto
🚧 Files skipped from review as they are similar to previous changes (5)
- .github/workflows/proto.yml
- proto/buf.yaml
- Makefile
- proto/connect/marketmap/v2/tx.proto
- proto/connect/service/v2/oracle.proto
Summary by CodeRabbit
New Features
Chores