-
Notifications
You must be signed in to change notification settings - Fork 33
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
Omnirpc: add module for receipt request backups #2678
Conversation
WalkthroughThe recent changes introduce a new Changes
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 (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2678 +/- ##
===================================================
+ Coverage 39.86313% 40.42884% +0.56571%
===================================================
Files 180 197 +17
Lines 14758 16090 +1332
Branches 80 80
===================================================
+ Hits 5883 6505 +622
- Misses 8111 8748 +637
- Partials 764 837 +73
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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: 2
Outside diff range and nitpick comments (2)
services/omnirpc/modules/receiptsbackup/receiptsbackup.go (2)
1-1
: Add a package comment to explain the purpose of thereceiptsbackup
package.Tools
GitHub Check: Lint (services/omnirpc)
[warning] 1-1:
package-comments: should have a package comment (revive)
52-64
: Consider using a larger integer type forport
if port numbers outside theuint16
range might be needed.
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (3)
- services/omnirpc/cmd/commands.go (2 hunks)
- services/omnirpc/cmd/flags.go (2 hunks)
- services/omnirpc/modules/receiptsbackup/receiptsbackup.go (1 hunks)
Additional context used
GitHub Check: Lint (services/omnirpc)
services/omnirpc/cmd/flags.go
[failure] 58-58:
varbackupRPCFlag
is unused (unused)
[failure] 63-63:
varrecieptsTimeoutFlag
is unused (unused)services/omnirpc/modules/receiptsbackup/receiptsbackup.go
[failure] 140-140:
singleCaseSwitch: should rewrite switch statement to if statement (gocritic)
[warning] 1-1:
package-comments: should have a package comment (revive)services/omnirpc/cmd/commands.go
[failure] 200-200:
receiptsProxy
is unused (deadcode)
Additional comments not posted (4)
services/omnirpc/cmd/flags.go (2)
63-66
: Verify the usage ofrecieptsTimeoutFlag
across the codebase.Verification successful
The
recieptsTimeoutFlag
is indeed used in the codebase, specifically inservices/omnirpc/cmd/commands.go
. Here are the relevant locations:
services/omnirpc/cmd/commands.go: recieptsTimeoutFlag
services/omnirpc/cmd/commands.go: simpleProxy := receiptsbackup.NewProxy(c.String(rpcFlag.Name), c.String(backupRPCFlag.Name), c.Duration(recieptsTimeoutFlag.Name), metrics.Get(), c.Int(portFlag.Name), c.Int(chainIDFlag.Name))
This confirms that the flag is utilized and not redundant.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of `recieptsTimeoutFlag` across the codebase. # Test: Search for the flag usage. Expect: Occurrences in other files. rg --type go $'recieptsTimeoutFlag'Length of output: 393
Tools
GitHub Check: Lint (services/omnirpc)
[failure] 63-63:
varrecieptsTimeoutFlag
is unused (unused)
58-61
: Verify the usage ofbackupRPCFlag
across the codebase.Verification successful
Verify the context of
backupRPCFlag
usage inservices/omnirpc/cmd/commands.go
.
The
backupRPCFlag
is correctly defined and used within thereceiptsProxy
command inservices/omnirpc/cmd/commands.go
.
services/omnirpc/cmd/commands.go
:backupRPCFlag
is used in thereceiptsProxy
command.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of `backupRPCFlag` across the codebase. # Test: Search for the flag usage. Expect: Occurrences in other files. rg --type go $'backupRPCFlag'Length of output: 373
Script:
#!/bin/bash # Description: Verify the context of `backupRPCFlag` usage in `services/omnirpc/cmd/commands.go`. # Test: Display relevant lines in `commands.go` to ensure proper usage. rg --type go --context 5 $'backupRPCFlag' services/omnirpc/cmd/commands.goLength of output: 646
Tools
GitHub Check: Lint (services/omnirpc)
[failure] 58-58:
varbackupRPCFlag
is unused (unused)services/omnirpc/modules/receiptsbackup/receiptsbackup.go (2)
26-50
: TheReceiptsProxy
interface andreceiptsProxyImpl
struct are well-defined and documented.
66-95
: TheRun
method is well-implemented with robust logging and error handling.
func (r *receiptsProxyImpl) processRequest(ctx context.Context, rpcRequest rpc.Request, requestID []byte) (resp omniHTTP.Response, err error) { | ||
//nolint: exhaustive | ||
switch client.RPCMethod(rpcRequest.Method) { | ||
case client.TransactionReceiptByHashMethod: | ||
req := r.client.NewRequest() | ||
body, err := json.Marshal(rpcRequest) | ||
if err != nil { | ||
return nil, errors.New("could not marshal request") | ||
} | ||
|
||
ctxWithTimeout, cancel := context.WithTimeout(ctx, r.receiptTimeout) | ||
defer cancel() | ||
|
||
resp, err := req. | ||
SetContext(ctxWithTimeout). | ||
SetRequestURI(r.proxyURL). | ||
SetBody(body). | ||
SetHeaderBytes(omniHTTP.XRequestID, requestID). | ||
SetHeaderBytes(omniHTTP.XForwardedFor, []byte(r.proxyURL)). | ||
SetHeaderBytes(omniHTTP.ContentType, omniHTTP.JSONType). | ||
SetHeaderBytes(omniHTTP.Accept, omniHTTP.JSONType). | ||
Do() | ||
if err != nil { | ||
// do backup request | ||
resp, err = req. | ||
SetContext(ctxWithTimeout). | ||
SetRequestURI(r.backupURL). | ||
SetBody(body). | ||
SetHeaderBytes(omniHTTP.XRequestID, requestID). | ||
SetHeaderBytes(omniHTTP.XForwardedFor, []byte(r.proxyURL)). | ||
SetHeaderBytes(omniHTTP.ContentType, omniHTTP.JSONType). | ||
SetHeaderBytes(omniHTTP.Accept, omniHTTP.JSONType). | ||
Do() | ||
if err != nil { | ||
return nil, fmt.Errorf("could not get response from backup RPC %s: %w", r.proxyURL, err) | ||
} | ||
} | ||
return resp, nil | ||
} | ||
return nil, nil | ||
} |
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.
Refactor the switch statement to an if statement since it only contains one case.
- switch client.RPCMethod(rpcRequest.Method) {
- case client.TransactionReceiptByHashMethod:
+ if client.RPCMethod(rpcRequest.Method) == client.TransactionReceiptByHashMethod {
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.
func (r *receiptsProxyImpl) processRequest(ctx context.Context, rpcRequest rpc.Request, requestID []byte) (resp omniHTTP.Response, err error) { | |
//nolint: exhaustive | |
switch client.RPCMethod(rpcRequest.Method) { | |
case client.TransactionReceiptByHashMethod: | |
req := r.client.NewRequest() | |
body, err := json.Marshal(rpcRequest) | |
if err != nil { | |
return nil, errors.New("could not marshal request") | |
} | |
ctxWithTimeout, cancel := context.WithTimeout(ctx, r.receiptTimeout) | |
defer cancel() | |
resp, err := req. | |
SetContext(ctxWithTimeout). | |
SetRequestURI(r.proxyURL). | |
SetBody(body). | |
SetHeaderBytes(omniHTTP.XRequestID, requestID). | |
SetHeaderBytes(omniHTTP.XForwardedFor, []byte(r.proxyURL)). | |
SetHeaderBytes(omniHTTP.ContentType, omniHTTP.JSONType). | |
SetHeaderBytes(omniHTTP.Accept, omniHTTP.JSONType). | |
Do() | |
if err != nil { | |
// do backup request | |
resp, err = req. | |
SetContext(ctxWithTimeout). | |
SetRequestURI(r.backupURL). | |
SetBody(body). | |
SetHeaderBytes(omniHTTP.XRequestID, requestID). | |
SetHeaderBytes(omniHTTP.XForwardedFor, []byte(r.proxyURL)). | |
SetHeaderBytes(omniHTTP.ContentType, omniHTTP.JSONType). | |
SetHeaderBytes(omniHTTP.Accept, omniHTTP.JSONType). | |
Do() | |
if err != nil { | |
return nil, fmt.Errorf("could not get response from backup RPC %s: %w", r.proxyURL, err) | |
} | |
} | |
return resp, nil | |
} | |
return nil, nil | |
} | |
func (r *receiptsProxyImpl) processRequest(ctx context.Context, rpcRequest rpc.Request, requestID []byte) (resp omniHTTP.Response, err error) { | |
//nolint: exhaustive | |
if client.RPCMethod(rpcRequest.Method) == client.TransactionReceiptByHashMethod { | |
req := r.client.NewRequest() | |
body, err := json.Marshal(rpcRequest) | |
if err != nil { | |
return nil, errors.New("could not marshal request") | |
} | |
ctxWithTimeout, cancel := context.WithTimeout(ctx, r.receiptTimeout) | |
defer cancel() | |
resp, err := req. | |
SetContext(ctxWithTimeout). | |
SetRequestURI(r.proxyURL). | |
SetBody(body). | |
SetHeaderBytes(omniHTTP.XRequestID, requestID). | |
SetHeaderBytes(omniHTTP.XForwardedFor, []byte(r.proxyURL)). | |
SetHeaderBytes(omniHTTP.ContentType, omniHTTP.JSONType). | |
SetHeaderBytes(omniHTTP.Accept, omniHTTP.JSONType). | |
Do() | |
if err != nil { | |
// do backup request | |
resp, err = req. | |
SetContext(ctxWithTimeout). | |
SetRequestURI(r.backupURL). | |
SetBody(body). | |
SetHeaderBytes(omniHTTP.XRequestID, requestID). | |
SetHeaderBytes(omniHTTP.XForwardedFor, []byte(r.proxyURL)). | |
SetHeaderBytes(omniHTTP.ContentType, omniHTTP.JSONType). | |
SetHeaderBytes(omniHTTP.Accept, omniHTTP.JSONType). | |
Do() | |
if err != nil { | |
return nil, fmt.Errorf("could not get response from backup RPC %s: %w", r.proxyURL, err) | |
} | |
} | |
return resp, nil | |
} | |
return nil, nil | |
} |
Tools
GitHub Check: Lint (services/omnirpc)
[failure] 140-140:
singleCaseSwitch: should rewrite switch statement to if statement (gocritic)
var receiptsProxy = &cli.Command{ | ||
Name: "receipts-backup", | ||
Usage: "A rpc proxy for using backup RPC with hanging receipt requests", | ||
Flags: []cli.Flag{ | ||
rpcFlag, | ||
backupRPCFlag, | ||
portFlag, | ||
chainIDFlag, | ||
recieptsTimeoutFlag, | ||
}, | ||
Action: func(c *cli.Context) error { | ||
simpleProxy := receiptsbackup.NewProxy(c.String(rpcFlag.Name), c.String(backupRPCFlag.Name), c.Duration(recieptsTimeoutFlag.Name), metrics.Get(), c.Int(portFlag.Name), c.Int(chainIDFlag.Name)) | ||
|
||
err := simpleProxy.Run(c.Context) | ||
if err != nil { | ||
return fmt.Errorf("return err: %w", err) | ||
} | ||
return nil | ||
}, |
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.
Tip
Codebase Verification
The receiptsProxy
command is defined in services/omnirpc/cmd/commands.go
but is not registered or used elsewhere in the application. This indicates that it might not be integrated correctly into the main application.
- Ensure that the
receiptsProxy
command is registered in the main application entry point or any relevant command registration file.
Analysis chain
Verify the usage of receiptsProxy
in the application to ensure it is integrated correctly.
Verify if the receiptsProxy
command is integrated into the main application entry point or if it is intended to be used in a different context.
Verify if the receiptsProxy
command is registered in any other potential files where commands might be registered.
Verify if the receiptsProxy
command is registered in any other potential methods or files where commands might be registered.
Verify the specific registration of receiptsProxy
in the identified files.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the usage of `receiptsProxy` in the application.
# Test: Search for the command usage. Expect: Occurrences in other files or entry points.
rg --type go $'receiptsProxy'
Length of output: 842
Script:
#!/bin/bash
# Description: Verify the integration of `receiptsProxy` in the main application entry point.
# Test: Search for the main application entry point and check for command integration. Expect: Occurrences of command integration.
rg --type go 'cli.App' --files-with-matches | xargs rg 'commands ='
Length of output: 67
Script:
#!/bin/bash
# Description: Verify the registration of `receiptsProxy` in other potential files.
# Test: Search for command registration in other files. Expect: Occurrences of command registration.
rg --type go 'commands =' --files-with-matches
Length of output: 48
Script:
#!/bin/bash
# Description: Verify the registration of `receiptsProxy` in other potential methods or files.
# Test: Search for command registration in other methods or files. Expect: Occurrences of command registration.
rg --type go 'cli.Command' --files-with-matches
Length of output: 1087
Script:
#!/bin/bash
# Description: Verify the specific registration of `receiptsProxy` in the identified files.
# Test: Search for the specific registration of `receiptsProxy`. Expect: Occurrences of `receiptsProxy` registration.
rg --type go 'receiptsProxy' --files-with-matches
Length of output: 140
Tools
GitHub Check: Lint (services/omnirpc)
[failure] 200-200:
receiptsProxy
is unused (deadcode)
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: 2
Outside diff range and nitpick comments (1)
services/omnirpc/modules/receiptsbackup/receiptsbackup.go (1)
1-1
: Add a package comment to describe thereceiptsbackup
package.Tools
GitHub Check: Lint (services/omnirpc)
[warning] 1-1:
package-comments: should have a package comment (revive)
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (4)
- ethergo/parser/rpc/rpc.go (2 hunks)
- ethergo/parser/rpc/rpc_test.go (1 hunks)
- services/omnirpc/cmd/cmd.go (2 hunks)
- services/omnirpc/modules/receiptsbackup/receiptsbackup.go (1 hunks)
Additional context used
GitHub Check: Lint (ethergo)
ethergo/parser/rpc/rpc.go
[failure] 88-88:
Comment should end in a period (godot)
GitHub Check: Lint (services/omnirpc)
services/omnirpc/modules/receiptsbackup/receiptsbackup.go
[failure] 140-140:
singleCaseSwitch: should rewrite switch statement to if statement (gocritic)
[warning] 1-1:
package-comments: should have a package comment (revive)
Additional comments not posted (2)
ethergo/parser/rpc/rpc_test.go (1)
15-20
: The tests forIsNullResponse
are well-implemented and provide good coverage for both null and non-null scenarios.services/omnirpc/cmd/cmd.go (1)
26-26
: The integration of thereceiptsProxy
command into the application's command list is correctly implemented.
@@ -83,3 +84,19 @@ | |||
} | |||
return []Request{rpcRequest}, nil | |||
} | |||
|
|||
// IsNullResponse checks if the result field in the JSON-RPC response is null |
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.
Add a period at the end of the comment for IsNullResponse
to adhere to Go documentation standards.
- // IsNullResponse checks if the result field in the JSON-RPC response is null
+ // IsNullResponse checks if the result field in the JSON-RPC response is null.
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.
// IsNullResponse checks if the result field in the JSON-RPC response is null | |
// IsNullResponse checks if the result field in the JSON-RPC response is null. |
Tools
GitHub Check: Lint (ethergo)
[failure] 88-88:
Comment should end in a period (godot)
|
||
func (r *receiptsProxyImpl) processRequest(ctx context.Context, rpcRequest rpc.Request, requestID []byte) (resp omniHTTP.Response, err error) { | ||
//nolint: exhaustive | ||
switch client.RPCMethod(rpcRequest.Method) { |
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.
Refactor the switch statement to an if statement since it only contains one case.
- switch client.RPCMethod(rpcRequest.Method) {
- case client.TransactionReceiptByHashMethod:
+ if client.RPCMethod(rpcRequest.Method) == client.TransactionReceiptByHashMethod {
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.
switch client.RPCMethod(rpcRequest.Method) { | |
if client.RPCMethod(rpcRequest.Method) == client.TransactionReceiptByHashMethod { |
Tools
GitHub Check: Lint (services/omnirpc)
[failure] 140-140:
singleCaseSwitch: should rewrite switch statement to if statement (gocritic)
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
Outside diff range and nitpick comments (10)
README.md (10)
Line range hint
24-24
: Correct article usage for better readability.- is a interchain messaging protocol + is an interchain messaging protocol
Line range hint
24-24
: Consider simplifying "a variety of" to enhance clarity.- This repo contains a variety of tools, interfaces and services + This repo contains multiple tools, interfaces and services - There are a variety of different packages in this repo + There are several different packages in this repoAlso applies to: 34-34
Line range hint
93-104
: Standardize the spelling of "Node.js" for consistency.- install nodejs packages + install Node.js packages - Install the Correct Version of NodeJS + Install the Correct Version of Node.js - install the correct version of NodeJS. + install the correct version of Node.js.
Line range hint
112-112
: Consider removing "of" for conciseness.- To build all of the [TypeScript packages](./packages), run: + To build all [TypeScript packages](./packages), run:
Line range hint
133-133
: Add periods after abbreviations for consistency in American English.- pull requests (new features, bug fixes, etc) should be opened against this branch. + pull requests (new features, bug fixes, etc.) should be opened against this branch. - This branch is used for production front-end releases and is the one that gets deployed to production. + This branch is used for production front-end releases and is the one that gets deployed to production.Also applies to: 135-135
Line range hint
138-138
: Clarify the subject in the sentence for better understanding.- `master` should never be behind `fe-release`! The only exception is when a hotfix is needed on the production front-end. + It should be noted that `master` should never be behind `fe-release`! The only exception is when a hotfix is needed on the production front-end.
Line range hint
150-150
: Add a comma for clarity in complex sentences.- In this scenario you are implementing a new feature that doesn't automatically trigger a front-end release. + In this scenario, you are implementing a new feature that doesn't automatically trigger a front-end release. - In this scenario you are releasing a front-end update using the latest changes from `master`. + In this scenario, you are releasing a front-end update using the latest changes from `master`.Also applies to: 195-195
Line range hint
190-190
: Use a more formal verb for professional documentation.- If any of the checks fail, you can fix the issues in your feature branch and push again. + If any of the checks fail, you can resolve the issues in your feature branch and push again.
Line range hint
243-243
: Add a comma before 'or' in compound sentences for correct punctuation.- you might want to merge the PR as soon or before it passes all checks. + you might want to merge the PR as soon, or before it passes all checks.
Line range hint
272-272
: Simplify language to avoid wordiness.- In order to minimize risks coming from extraneous dependencies or supply chain attacks in a production like environment + To minimize risks from extraneous dependencies or supply chain attacks in a production-like environment
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files ignored due to path filters (7)
committee/go.mod
is excluded by!**/*.mod
committee/go.sum
is excluded by!**/*.sum
go.work
is excluded by!**/*.work
go.work.sum
is excluded by!**/*.sum
services/explorer/go.mod
is excluded by!**/*.mod
sin-executor/go.mod
is excluded by!**/*.mod
sin-executor/go.sum
is excluded by!**/*.sum
Files selected for processing (3)
- .codecov.yml (1 hunks)
- .github/workflows/go.yml (1 hunks)
- README.md (2 hunks)
Files skipped from review due to trivial changes (2)
- .codecov.yml
- .github/workflows/go.yml
Additional context used
LanguageTool
README.md
[misspelling] ~24-~24: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’. (EN_A_VS_AN)
Context: ...napse](https://synapseprotocol.com/) is a interchain messaging protocol & bridge ...
[style] ~24-~24: The phrase “a variety of” may be wordy. To make your writing clearer, consider replacing it. (A_VARIETY_OF)
Context: ...oss multiple chains. This repo contains a variety of tools, interfaces and services that for...
[style] ~34-~34: The phrase “a variety of” may be wordy. To make your writing clearer, consider replacing it. (A_VARIETY_OF)
Context: ...fun to work on! ## Projects There are a variety of different packages in this repo, covere...
[uncategorized] ~93-~93: The official spelling of this programming framework is “Node.js”. (NODE_JS)
Context: ...ne the repository, open it, and install nodejs packages withyarn
: ```bash git clon...
[uncategorized] ~102-~102: The official spelling of this programming framework is “Node.js”. (NODE_JS)
Context: ...`` ### Install the Correct Version of NodeJS Usingnvm
, install the correct versi...
[uncategorized] ~104-~104: The official spelling of this programming framework is “Node.js”. (NODE_JS)
Context: ...gnvm
, install the correct version of NodeJS.nvm use
### Building the Typ...
[style] ~112-~112: Consider removing “of” to be more concise (ALL_OF_THE)
Context: ...lding the TypeScript packages To build all of the TypeScript packages, run:...
[uncategorized] ~133-~133: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION)
Context: ...ch strategy for development: -master
: This is the primary development branch ...
[style] ~133-~133: In American English, abbreviations like “etc.” require a period. (ETC_PERIOD)
Context: ...pull requests (new features, bug fixes, etc) should be opened against this branch. ...
[uncategorized] ~135-~135: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION)
Context: ...eature) for more details. -fe-release
: This branch is used for production fron...
[style] ~138-~138: To form a complete sentence, be sure to include a subject. (MISSING_IT_THERE)
Context: ...d-update) for more details. >master
should never be behindfe-release
! The only ...
[typographical] ~150-~150: It appears that a comma is missing. (DURING_THAT_TIME_COMMA)
Context: ... 1: Implementing a New Feature In this scenario you are implementing a new feature that...
[style] ~190-~190: Consider using a different verb for a more formal wording. (FIX_RESOLVE)
Context: ...lly. If any of the checks fail, you can fix the issues in your feature branch and p...
[typographical] ~195-~195: It appears that a comma is missing. (DURING_THAT_TIME_COMMA)
Context: ...: Releasing a Front-end Update In this scenario you are releasing a front-end update us...
[uncategorized] ~243-~243: Use a comma before ‘or’ if it connects two independent clauses (unless they are closely connected and short). (COMMA_COMPOUND_SENTENCE)
Context: ..., you might want to merge the PR as soon or before it passes all checks. However, *...
[style] ~272-~272: Consider a shorter alternative to avoid wordiness. (IN_ORDER_TO_PREMIUM)
Context: ...the docs or both. Maybe a symlink? --> In order to minimize risks coming from extraneous d...
Markdownlint
README.md
16-16: Expected: 1; Actual: 2 (MD012, no-multiple-blanks)
Multiple consecutive blank lines
22-22: Expected: 1; Actual: 2 (MD012, no-multiple-blanks)
Multiple consecutive blank lines
23-23: Expected: 1; Actual: 3 (MD012, no-multiple-blanks)
Multiple consecutive blank lines
101-101: Expected: 1; Actual: 2 (MD012, no-multiple-blanks)
Multiple consecutive blank lines
28-28: Expected: 1; Actual: 0; Below (MD022, blanks-around-headings)
Headings should be surrounded by blank lines
106-106: null (MD040, fenced-code-language)
Fenced code blocks should have a language specified
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
Outside diff range and nitpick comments (7)
README.md (7)
Line range hint
24-24
: Correct article usage before a vowel sound.- is a interchain messaging protocol + is an interchain messaging protocol
Line range hint
92-92
: Standardize the spelling of "Node.js".- install nodejs packages + install Node.js packages - Install the Correct Version of NodeJS + Install the Correct Version of Node.js - install the correct version of NodeJS. + install the correct version of Node.js.Also applies to: 101-101, 103-103
Line range hint
111-111
: Simplify language for clarity.- To build all of the [TypeScript packages](./packages), run: + To build all [TypeScript packages](./packages), run:
Line range hint
132-132
: Add missing punctuation for clarity and correctness.- pull requests (new features, bug fixes, etc) should be opened against this branch. + pull requests (new features, bug fixes, etc.) should be opened against this branch. - `fe-release`: This branch is used for production front-end releases and is the one that gets deployed to production. + `fe-release`: This branch is used for production front-end releases and is the one that gets deployed to production.Also applies to: 134-134
Line range hint
137-137
: Clarify the sentence for better understanding.- > `master` should never be behind `fe-release`! The only exception is when a hotfix is needed on the production front-end. + > `master` should never be behind `fe-release`! The only exception occurs when a hotfix is needed on the production front-end.
Line range hint
242-242
: Clarify the conditional statement with proper punctuation.- you might want to merge the PR as soon or before it passes all checks. + you might want to merge the PR as soon as, or before, it passes all checks.
Line range hint
271-271
: Simplify language to enhance readability.- In order to minimize risks coming from extraneous dependencies or supply chain attacks in a production like environment, all distributed images are built as [scratch](https://hub.docker.com/_/scratch) or [distroless](https://github.com/GoogleContainerTools/distroless#distroless-container-images) images. + To minimize risks from extraneous dependencies or supply chain attacks in a production-like environment, all distributed images are built as [scratch](https://hub.docker.com/_/scratch) or [distroless](https://github.com/GoogleContainerTools/distroless#distroless-container-images) images.
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (2)
- .github/workflows/solidity.yml (3 hunks)
- README.md (3 hunks)
Files skipped from review due to trivial changes (1)
- .github/workflows/solidity.yml
Additional context used
LanguageTool
README.md
[misspelling] ~24-~24: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’. (EN_A_VS_AN)
Context: ...napse](https://synapseprotocol.com/) is a interchain messaging protocol & bridge ...
[style] ~24-~24: The phrase “a variety of” may be wordy. To make your writing clearer, consider replacing it. (A_VARIETY_OF)
Context: ...oss multiple chains. This repo contains a variety of tools, interfaces and services that for...
[style] ~34-~34: The phrase “a variety of” may be wordy. To make your writing clearer, consider replacing it. (A_VARIETY_OF)
Context: ...fun to work on! ## Projects There are a variety of different packages in this repo, covere...
[uncategorized] ~92-~92: The official spelling of this programming framework is “Node.js”. (NODE_JS)
Context: ...ne the repository, open it, and install nodejs packages withyarn
: ```bash git clon...
[uncategorized] ~101-~101: The official spelling of this programming framework is “Node.js”. (NODE_JS)
Context: ...`` ### Install the Correct Version of NodeJS Usingnvm
, install the correct versi...
[uncategorized] ~103-~103: The official spelling of this programming framework is “Node.js”. (NODE_JS)
Context: ...gnvm
, install the correct version of NodeJS.nvm use
### Building the Typ...
[style] ~111-~111: Consider removing “of” to be more concise (ALL_OF_THE)
Context: ...lding the TypeScript packages To build all of the TypeScript packages, run:...
[uncategorized] ~132-~132: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION)
Context: ...ch strategy for development: -master
: This is the primary development branch ...
[style] ~132-~132: In American English, abbreviations like “etc.” require a period. (ETC_PERIOD)
Context: ...pull requests (new features, bug fixes, etc) should be opened against this branch. ...
[uncategorized] ~134-~134: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION)
Context: ...eature) for more details. -fe-release
: This branch is used for production fron...
[style] ~137-~137: To form a complete sentence, be sure to include a subject. (MISSING_IT_THERE)
Context: ...d-update) for more details. >master
should never be behindfe-release
! The only ...
[typographical] ~149-~149: It appears that a comma is missing. (DURING_THAT_TIME_COMMA)
Context: ... 1: Implementing a New Feature In this scenario you are implementing a new feature that...
[style] ~189-~189: Consider using a different verb for a more formal wording. (FIX_RESOLVE)
Context: ...lly. If any of the checks fail, you can fix the issues in your feature branch and p...
[typographical] ~194-~194: It appears that a comma is missing. (DURING_THAT_TIME_COMMA)
Context: ...: Releasing a Front-end Update In this scenario you are releasing a front-end update us...
[uncategorized] ~242-~242: Use a comma before ‘or’ if it connects two independent clauses (unless they are closely connected and short). (COMMA_COMPOUND_SENTENCE)
Context: ..., you might want to merge the PR as soon or before it passes all checks. However, *...
[style] ~271-~271: Consider a shorter alternative to avoid wordiness. (IN_ORDER_TO_PREMIUM)
Context: ...the docs or both. Maybe a symlink? --> In order to minimize risks coming from extraneous d...
Markdownlint
README.md
16-16: Expected: 1; Actual: 2 (MD012, no-multiple-blanks)
Multiple consecutive blank lines
22-22: Expected: 1; Actual: 2 (MD012, no-multiple-blanks)
Multiple consecutive blank lines
23-23: Expected: 1; Actual: 3 (MD012, no-multiple-blanks)
Multiple consecutive blank lines
100-100: Expected: 1; Actual: 2 (MD012, no-multiple-blanks)
Multiple consecutive blank lines
28-28: Expected: 1; Actual: 0; Below (MD022, blanks-around-headings)
Headings should be surrounded by blank lines
105-105: null (MD040, fenced-code-language)
Fenced code blocks should have a language specified
Summary by CodeRabbit
New Features
receipts-backup
to set up an RPC proxy for handling receipt requests with backup RPC.Bug Fixes
IsNullResponse
function to check for null responses in JSON-RPC payloads.Chores
.codecov.yml
and GitHub workflows to remove deprecated paths and packages.committe
andsin-executor
packages.Tests
IsNullResponse
function.