Skip to content

fix: sync proto with indexer - remove status from RFQRequestInputType#3

Open
tajik-ninja wants to merge 1 commit intomainfrom
fix/sync-proto-remove-status
Open

fix: sync proto with indexer - remove status from RFQRequestInputType#3
tajik-ninja wants to merge 1 commit intomainfrom
fix/sync-proto-remove-status

Conversation

@tajik-ninja
Copy link
Contributor

@tajik-ninja tajik-ninja commented Mar 9, 2026

Syncs with indexer commit cd282f4 (C-871) which fixed client-controlled status field.

Summary by CodeRabbit

  • Refactor
    • Removed status field from RFQ requests.
    • Updated RFQ request structure organization.

@coderabbitai
Copy link

coderabbitai bot commented Mar 9, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: ecf33fb9-350b-4c5f-9562-7fb5201f968f

📥 Commits

Reviewing files that changed from the base of the PR and between b3f299d and f6ff72b.

📒 Files selected for processing (1)
  • src/rfq_test/proto/injective_rfq_rpc.proto

📝 Walkthrough

Walkthrough

A protobuf schema modification that removes the status field from the RFQRequestInputType message and reassigns the transaction_time field number from 10 to 9 in the injective RFQ RPC definition.

Changes

Cohort / File(s) Summary
Protobuf Schema Update
src/rfq_test/proto/injective_rfq_rpc.proto
Removed string status field from RFQRequestInputType message and renumbered uint64 transaction_time from field tag 10 to tag 9.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 A field disappears with a hop and a skip,
The status takes flight on a protobuf trip,
Transaction_time finds a new numbered home,
Through schemas we shuffle, through protocols roam! ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly and clearly describes the main change: removing the status field from RFQRequestInputType to sync with the indexer, which matches the file modifications shown in the raw summary.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/sync-proto-remove-status

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Syncs the RFQ gRPC proto definitions with the referenced indexer change to prevent a client-controlled RFQ request status field.

Changes:

  • Removed status from RFQRequestInputType.
  • Renumbered transaction_time within RFQRequestInputType.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 59 to +61
uint64 expiry = 8;
// Status (open, cancelled, completed)
string status = 9;
// Transaction time timestamp
uint64 transaction_time = 10;
uint64 transaction_time = 9;
Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

transaction_time was renumbered from field 10 to 9 after removing status. In protobuf, field numbers are part of the wire contract and should not be reused; this change will break backward/forward compatibility (old clients/servers will misinterpret or drop the field due to tag/type mismatch). Keep transaction_time = 10 and reserve field number 9 (and optionally the name status) to prevent accidental reuse.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants