-
Notifications
You must be signed in to change notification settings - Fork 245
fix(rpc): optional params #1769
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThe changes improve the RPC JSON handling by adding support for additional pointer types and ensuring safer handling of query parameters. This involves checking for nil values before assignment in various functions, thus enhancing robustness. Additionally, some struct fields have been converted to pointers, allowing for nullable values. These updates ensure better compatibility with CometBFT RPC methods. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant RPCHandler
participant Service
Client->>+RPCHandler: Request with pointer type parameters
RPCHandler->>+Service: Forward request with checked pointers
Service-->>-RPCHandler: Processed response
RPCHandler-->>-Client: Response with nullable fields
Assessment against linked issues
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
086bccb to
7ab4fa1
Compare
7ab4fa1 to
cdaf328
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- rpc/json/handler.go (2 hunks)
- rpc/json/service.go (7 hunks)
- rpc/json/service_test.go (2 hunks)
- rpc/json/types.go (3 hunks)
Additional comments not posted (30)
rpc/json/types.go (13)
14-14: LGTM!The change to make
Querya pointer to string aligns with the objective of allowing optional parameters.
18-18: LGTM!The change to make
Querya pointer to string aligns with the objective of allowing optional parameters.
31-32: LGTM!The change to make
MinHeightandMaxHeightpointers toStrInt64aligns with the objective of allowing optional parameters.
43-43: LGTM!The change to make
Heighta pointer toStrInt64aligns with the objective of allowing optional parameters.
50-50: LGTM!The change to make
Heighta pointer toStrInt64aligns with the objective of allowing optional parameters.
54-54: LGTM!The change to make
Heighta pointer toStrInt64aligns with the objective of allowing optional parameters.
58-58: LGTM!The change to make
Heighta pointer toStrInt64aligns with the objective of allowing optional parameters.
77-79: LGTM!The change to make
Page,PerPage, andOrderBypointers aligns with the objective of allowing optional parameters.
84-86: LGTM!The change to make
Page,PerPage, andOrderBypointers aligns with the objective of allowing optional parameters.
90-92: LGTM!The change to make
Height,Page, andPerPagepointers aligns with the objective of allowing optional parameters.
100-100: LGTM!The change to make
Heighta pointer toStrInt64aligns with the objective of allowing optional parameters.
104-104: LGTM!The change to make
Limita pointer toStrIntaligns with the objective of allowing optional parameters.
126-127: LGTM!The change to make
HeightandProvepointers aligns with the objective of allowing optional parameters.rpc/json/handler.go (4)
217-218: Add support forStrIntpointer type.This change allows the function to handle
StrIntpointer types, aligning with the objective of supporting additional pointer types.
232-233: Add support forstringpointer type.This change allows the function to handle
stringpointer types, aligning with the objective of supporting additional pointer types.
234-235: Add support forboolpointer type.This change allows the function to handle
boolpointer types, aligning with the objective of supporting additional pointer types.
240-241: Add support forbytes.HexBytespointer type.This change allows the function to handle
bytes.HexBytespointer types, aligning with the objective of supporting additional pointer types.rpc/json/service.go (12)
99-102: LGTM! Handling optional parameter forquery.The changes correctly handle the optional
queryparameter.
144-147: LGTM! Handling optional parameter forquery.The changes correctly handle the optional
queryparameter.
179-187: LGTM! Handling optional parameters forminHeightandmaxHeight.The changes correctly handle the optional
minHeightandmaxHeightparameters.
213-217: LGTM! Handling optional parameter forheight.The changes correctly handle the optional
heightparameter.
222-226: LGTM! Handling optional parameter forheight.The changes correctly handle the optional
heightparameter.
231-235: LGTM! Handling optional parameter forheight.The changes correctly handle the optional
heightparameter.
252-265: LGTM! Handling optional parameters forpage,perPage, andorderBy.The changes correctly handle the optional
page,perPage, andorderByparameters.
271-284: LGTM! Handling optional parameters forpage,perPage, andorderBy.The changes correctly handle the optional
page,perPage, andorderByparameters.
290-303: LGTM! Handling optional parameters forheight,page, andperPage.The changes correctly handle the optional
height,page, andperPageparameters.
318-322: LGTM! Handling optional parameter forheight.The changes correctly handle the optional
heightparameter.
327-330: LGTM! Handling optional parameter forlimit.The changes correctly handle the optional
limitparameter.
354-361: LGTM! Handling optional parameters foroptions.Heightandoptions.Prove.The changes correctly handle the optional
options.Heightandoptions.Proveparameters.rpc/json/service_test.go (1)
151-175: LGTM! Testing optional parameters forsubscribeandunsubscribe.The changes correctly test the handling of optional parameters for
subscribeandunsubscribe.
Manav-Aggarwal
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Overview
This PR restores cometbft compat by allowing optional params for all endpoints, following
/blockin #1760Fixes #1761
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Refactor