-
Notifications
You must be signed in to change notification settings - Fork 10
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
feat: add flag to enable price-feeder on node start #157
feat: add flag to enable price-feeder on node start #157
Conversation
WalkthroughThe recent changes enhance the Exocore node's command-line interface by introducing flags for oracle feeding and consensus key mnemonics, enabling the price feeder to operate asynchronously during startup. Additionally, the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ExocoreNode
participant PriceFeeder
User->>ExocoreNode: Start with --oracle and --mnemonic flags
ExocoreNode->>PriceFeeder: Launch price feeder in a goroutine
PriceFeeder-->>ExocoreNode: Price feeder running
ExocoreNode->>User: Node started successfully
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
go func() { | ||
defer func() { | ||
if err := recover(); err != nil { | ||
fmt.Println("price-feeder failed", err) | ||
} | ||
}() | ||
mnemonic, _ := cmd.Flags().GetString(flagMnemonic) | ||
pricefeeder.StartPriceFeeder(path.Join(clientCtx.HomeDir, confPath, confOracle), mnemonic, path.Join(clientCtx.HomeDir, confPath)) | ||
}() |
Check notice
Code scanning / CodeQL
Spawning a Go routine Note
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: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
go.sum
is excluded by!**/*.sum
Files selected for processing (2)
- cmd/exocored/root.go (4 hunks)
- go.mod (11 hunks)
Additional context used
GitHub Check: CodeQL
cmd/exocored/root.go
[notice] 149-157: Spawning a Go routine
Spawning a Go routine may be a possible source of non-determinism
Additional comments not posted (20)
cmd/exocored/root.go (2)
58-63
: LGTM!The new constants are well-defined and improve code readability.
141-143
: LGTM!The new flags
flagOracle
andflagMnemonic
enhance the functionality ofstartCmd
.go.mod (18)
10-10
: LGTM!The new dependency
github.com/ExocoreNetwork/price-feeder
is necessary for the new price feeder functionality.
21-21
: LGTM!The update to
github.com/ethereum/go-ethereum
from1.13.5-0.20231027145059-2d7dba024d76
to1.13.15
is likely to include important improvements.
33-33
: LGTM!The update to
github.com/spf13/cast
from1.5.1
to1.6.0
is likely to include important improvements.
34-34
: LGTM!The update to
github.com/spf13/cobra
from1.7.0
to1.8.0
is likely to include important improvements.
36-36
: LGTM!The update to
github.com/spf13/viper
from1.16.0
to1.18.2
is likely to include important improvements.
54-54
: LGTM!The update to
github.com/davecgh/go-spew
from1.1.1
to1.1.2-0.20180830191138-d8f796af33cc
is likely to include important improvements.
71-71
: LGTM!The update to
cloud.google.com/go/storage
from1.30.1
to1.35.1
is likely to include important improvements.
124-124
: LGTM!The update to
github.com/go-ole/go-ole
from1.2.6
to1.3.0
is likely to include important improvements.
156-156
: LGTM!The update to
github.com/holiman/uint256
from1.2.3
to1.2.4
is likely to include important improvements.
186-186
: LGTM!The update to
github.com/pelletier/go-toml/v2
from2.0.9
to2.1.0
is likely to include important improvements.
188-188
: LGTM!The update to
github.com/pmezard/go-difflib
from1.0.0
to1.0.1-0.20181226105442-5d4384ee4fb2
is likely to include important improvements.
231-231
: LGTM!The update to
golang.org/x/oauth2
from0.13.0
to0.15.0
is likely to include important improvements.
238-238
: LGTM!The update to
google.golang.org/api
from0.149.0
to0.153.0
is likely to include important improvements.
201-201
: LGTM!The new dependency
github.com/sagikazarmark/locafero
is likely for new functionality or improvements.
202-202
: LGTM!The new dependency
github.com/sagikazarmark/slog-shim
is likely for new functionality or improvements.
207-207
: LGTM!The new dependency
github.com/sourcegraph/conc
is likely for new functionality or improvements.
210-210
: LGTM!The update to
github.com/subosito/gotenv
from1.4.2
to1.6.0
is likely to include important improvements.
229-229
: LGTM!The update to
go.uber.org/atomic
from1.10.0
to1.11.0
is likely to include important improvements.
preRunE := startCmd.PreRunE | ||
// add preRun to run price-feeder first before starting the node | ||
startCmd.PreRunE = func(cmd *cobra.Command, args []string) error { | ||
if enableFeeder, _ := cmd.Flags().GetBool(flagOracle); enableFeeder { | ||
clientCtx := cmd.Context().Value(client.ClientContextKey).(*client.Context) | ||
go func() { | ||
defer func() { | ||
if err := recover(); err != nil { | ||
fmt.Println("price-feeder failed", err) | ||
} | ||
}() | ||
mnemonic, _ := cmd.Flags().GetString(flagMnemonic) | ||
pricefeeder.StartPriceFeeder(path.Join(clientCtx.HomeDir, confPath, confOracle), mnemonic, path.Join(clientCtx.HomeDir, confPath)) | ||
}() | ||
} | ||
return preRunE(cmd, args) | ||
} |
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.
Improve error handling within the goroutine.
Spawning a goroutine can introduce non-determinism. Ensure that any panic is logged appropriately to aid in debugging.
go func() {
defer func() {
if err := recover(); err != nil {
fmt.Println("price-feeder failed", err)
+ // Log the error with more context
+ log.Printf("price-feeder panic: %v", err)
}
}()
mnemonic, _ := cmd.Flags().GetString(flagMnemonic)
pricefeeder.StartPriceFeeder(path.Join(clientCtx.HomeDir, confPath, confOracle), mnemonic, path.Join(clientCtx.HomeDir, confPath))
}()
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. Thoroughly test & benchmark the code to ensure it meets the requirements.
preRunE := startCmd.PreRunE | |
// add preRun to run price-feeder first before starting the node | |
startCmd.PreRunE = func(cmd *cobra.Command, args []string) error { | |
if enableFeeder, _ := cmd.Flags().GetBool(flagOracle); enableFeeder { | |
clientCtx := cmd.Context().Value(client.ClientContextKey).(*client.Context) | |
go func() { | |
defer func() { | |
if err := recover(); err != nil { | |
fmt.Println("price-feeder failed", err) | |
} | |
}() | |
mnemonic, _ := cmd.Flags().GetString(flagMnemonic) | |
pricefeeder.StartPriceFeeder(path.Join(clientCtx.HomeDir, confPath, confOracle), mnemonic, path.Join(clientCtx.HomeDir, confPath)) | |
}() | |
} | |
return preRunE(cmd, args) | |
} | |
preRunE := startCmd.PreRunE | |
// add preRun to run price-feeder first before starting the node | |
startCmd.PreRunE = func(cmd *cobra.Command, args []string) error { | |
if enableFeeder, _ := cmd.Flags().GetBool(flagOracle); enableFeeder { | |
clientCtx := cmd.Context().Value(client.ClientContextKey).(*client.Context) | |
go func() { | |
defer func() { | |
if err := recover(); err != nil { | |
fmt.Println("price-feeder failed", err) | |
// Log the error with more context | |
log.Printf("price-feeder panic: %v", err) | |
} | |
}() | |
mnemonic, _ := cmd.Flags().GetString(flagMnemonic) | |
pricefeeder.StartPriceFeeder(path.Join(clientCtx.HomeDir, confPath, confOracle), mnemonic, path.Join(clientCtx.HomeDir, confPath)) | |
}() | |
} | |
return preRunE(cmd, args) | |
} |
Tools
GitHub Check: CodeQL
[notice] 149-157: Spawning a Go routine
Spawning a Go routine may be a possible source of non-determinism
using oracle config file like:
|
Test passed, but the downside of this solution is whenever whitelist a new token/asset, it needs restart exocore node to enable new token price feeding. |
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 ignored due to path filters (1)
go.sum
is excluded by!**/*.sum
Files selected for processing (1)
- go.mod (10 hunks)
Additional comments not posted (13)
go.mod (13)
10-10
: New dependency added:github.com/ExocoreNetwork/price-feeder v0.1.4
.This addition aligns with the PR objective of enhancing the node's capabilities with price-feeder functionality.
33-33
: Dependency update:github.com/spf13/cast
tov1.6.0
.Ensure compatibility with the rest of the codebase and verify any new features or bug fixes.
34-34
: Dependency update:github.com/spf13/cobra
tov1.8.0
.Check for any improvements or bug fixes relevant to the command-line interface.
36-36
: Dependency update:github.com/spf13/viper
tov1.18.2
.Ensure that configuration handling remains compatible with the updated version.
54-54
: Dependency update:github.com/davecgh/go-spew
to a specific commit version.Check the impact of this specific commit on the project for any bug fixes or performance improvements.
71-71
: Dependency update:cloud.google.com/go/storage
tov1.35.1
.Verify compatibility with storage-related functionality and check for any new features or bug fixes.
115-115
: Dependency update:github.com/fsnotify/fsnotify
tov1.7.0
.Ensure that file system notifications remain compatible with the updated version.
124-124
: Dependency update:github.com/go-ole/go-ole
tov1.3.0
.Verify compatibility with OLE-related functionality and check for any new features or bug fixes.
156-156
: Dependency update:github.com/holiman/uint256
tov1.2.4
.Ensure compatibility with uint256-related operations and verify any bug fixes or performance improvements.
186-186
: Dependency update:github.com/pelletier/go-toml/v2
tov2.1.0
.Ensure compatibility with TOML parsing and serialization, and check for any new features or bug fixes.
188-188
: Dependency update:github.com/pmezard/go-difflib
to a specific commit version.Check the impact of this specific commit on the project for any bug fixes or performance improvements.
231-231
: Dependency update:golang.org/x/oauth2
tov0.15.0
.Ensure compatibility with OAuth2-related functionality and verify any security patches or new features.
238-238
: Dependency update:google.golang.org/api
tov0.153.0
.Ensure compatibility with Google API-related functionality and check for any new features or bug fixes.
use the pr179 to replace this 157 |
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 ignored due to path filters (1)
go.sum
is excluded by!**/*.sum
Files selected for processing (1)
- go.mod (10 hunks)
Additional comments not posted (13)
go.mod (13)
10-10
: New dependency added:github.com/ExocoreNetwork/price-feeder
The addition of
github.com/ExocoreNetwork/price-feeder
at versionv0.1.5
aligns with the PR's objective to enhance the Exocore node's functionality by enabling the price-feeder feature. Ensure that this version is stable and has been security vetted.
124-124
: ```shell
#!/bin/bashSearch for import statements of github.com/go-ole/go-ole in the codebase
rg 'import.github.com/go-ole/go-ole' --glob '.go'
Search for usages of go-ole in the codebase
rg 'ole.' --glob '*.go'
--- `115-115`: ```shell #!/bin/bash # Search for instances where fsnotify is used in the codebase to understand its usage. rg 'fsnotify' -A 3 # Check for any breaking changes or deprecations in the fsnotify changelog or release notes. # Assuming the changelog or release notes might be present in the repository. rg 'breaking changes|deprecations' -A 5
186-186
: ```shell
#!/bin/bashSearch for import statements of github.com/pelletier/go-toml/v2 to identify usage in the codebase.
ast-grep --lang go --pattern 'import "github.com/pelletier/go-toml/v2"'
Search for test cases related to TOML parsing.
rg 'TOML' --type go -A 5
--- `238-238`: ```shell #!/bin/bash # Search for usages of google.golang.org/api in the codebase rg 'google\.golang\.org/api' -A 3 # Check for any breaking changes or deprecations in the v0.153.0 release notes or documentation # Note: This assumes that release notes or changelogs are present in markdown or text files. rg 'v0\.153\.0' -A 5 --type markdown --type text
54-54
: ```shell
#!/bin/bashCheck if the specific commit exists in the upstream repository of github.com/davecgh/go-spew
git ls-remote https://github.com/davecgh/go-spew.git d8f796af33cc
If the commit exists, fetch the commit message and details
if git ls-remote https://github.com/davecgh/go-spew.git | grep -q d8f796af33cc; then
git clone https://github.com/davecgh/go-spew.git temp-go-spew-repo
cd temp-go-spew-repo
git show d8f796af33cc
cd ..
rm -rf temp-go-spew-repo
fi--- `231-231`: ```shell #!/bin/bash # Search for files that might contain release notes or changelog information. fd --type f --name '*CHANGELOG*' --name '*RELEASE*' --name '*NOTES*' # If any files are found, search for version v0.15.0 in those files. for file in $(fd --type f --name '*CHANGELOG*' --name '*RELEASE*' --name '*NOTES*'); do echo "Searching in $file" rg --type markdown -A 5 'v0.15.0' "$file" done
156-156
: ```shell
#!/bin/bashSearch for a changelog or release notes for github.com/holiman/uint256 to identify changes in version v1.2.4.
rg --files-with-matches "github.com/holiman/uint256" | xargs -I {} rg "v1.2.4" {}
Search the codebase for usage of github.com/holiman/uint256 to determine if any arithmetic operations are affected by the update.
ast-grep --lang go --pattern 'uint256.$($)'
--- `33-33`: **Dependency updated: `github.com/spf13/cast`** Upgraded from `v1.5.1` to `v1.6.0`. Review the release notes of this version to ensure there are no breaking changes that could affect the existing codebase. --- `34-34`: **Dependency updated: `github.com/spf13/cobra`** Updated from `v1.7.0` to `v1.8.0`. This is a commonly used library for CLI applications; ensure compatibility with other CLI-related dependencies. --- `188-188`: **Dependency updated: `github.com/pmezard/go-difflib`** Updated to a specific commit `v1.0.1-0.20181226105442-5d4384ee4fb2`. Similar to `go-spew`, this points to a specific commit rather than a stable release. Verify the stability and necessity of this commit. --- `71-71`: ```shell #!/bin/bash # Corrected command to check if CHANGELOG.md exists in the repository fd --type f --glob 'CHANGELOG.md' # If CHANGELOG.md exists, search for entries related to cloud.google.com/go/storage if [ -f "CHANGELOG.md" ]; then rg 'cloud.google.com/go/storage' CHANGELOG.md fi
36-36
: ```shell
#!/bin/bashReview the CHANGELOG.md file for any deprecation notices or significant changes related to viper.
rg 'viper' CHANGELOG.md -A 5
Check if the functions used in the codebase are affected by any changes in the updated version.
rg 'viper.New|viper.SetConfigFile|viper.ReadInConfig|viper.BindPFlag' CHANGELOG.md -A 5
</blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
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 ignored due to path filters (1)
go.sum
is excluded by!**/*.sum
Files selected for processing (1)
- go.mod (10 hunks)
Files skipped from review as they are similar to previous changes (1)
- go.mod
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 ignored due to path filters (1)
go.sum
is excluded by!**/*.sum
Files selected for processing (1)
- go.mod (10 hunks)
Files skipped from review as they are similar to previous changes (1)
- go.mod
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 ignored due to path filters (1)
go.sum
is excluded by!**/*.sum
Files selected for processing (1)
- go.mod (10 hunks)
Files skipped from review as they are similar to previous changes (1)
- go.mod
Description
add flag `--oracle` to start price-feeder on exocored node start.
----Closes #XXX
Summary by CodeRabbit
New Features
Improvements
These changes will provide users with more flexibility in managing their nodes and ensure a better overall experience.