Skip to content
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: support Stork oracle #13

Open
wants to merge 20 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Empty file added .chain_cookie
Empty file.
6 changes: 6 additions & 0 deletions cmd/injective-price-oracle/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ func initExternalFeedsOptions(
cmd *cli.Cmd,
binanceBaseURL **string,
dynamicFeedsDir **string,
storkFeedsDir **string,
) {
*binanceBaseURL = cmd.String(cli.StringOpt{
Name: "binance-url",
Expand All @@ -148,6 +149,11 @@ func initExternalFeedsOptions(
Desc: "Path to dynamic feeds configuration files in TOML format",
EnvVar: "ORACLE_DYNAMIC_FEEDS_DIR",
})
*storkFeedsDir = cmd.String(cli.StringOpt{
Name: "stork-feeds",
Desc: "Path to stork feeds configuration files in TOML format",
EnvVar: "ORACLE_STORK_FEEDS_DIR",
})
}

// initStatsdOptions sets options for StatsD metrics.
Expand Down
42 changes: 42 additions & 0 deletions cmd/injective-price-oracle/oracle.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ func oracleCmd(cmd *cli.Cmd) {
// External Feeds params
dynamicFeedsDir *string
binanceBaseURL *string
storkFeedsDir *string

// Metrics
statsdPrefix *string
Expand Down Expand Up @@ -82,6 +83,7 @@ func oracleCmd(cmd *cli.Cmd) {
cmd,
&binanceBaseURL,
&dynamicFeedsDir,
&storkFeedsDir,
)

initStatsdOptions(
Expand Down Expand Up @@ -201,12 +203,52 @@ func oracleCmd(cmd *cli.Cmd) {
log.Infof("found %d dynamic feed configs", len(dynamicFeedConfigs))
}

storkFeedConfigs := make([]*oracle.StorkFeedConfig, 0, 10)
if len(*storkFeedsDir) > 0 {
err := filepath.WalkDir(*storkFeedsDir, func(path string, d fs.DirEntry, err error) error {
if err != nil {
return err
} else if d.IsDir() {
return nil
} else if filepath.Ext(path) != ".toml" {
return nil
}

cfgBody, err := ioutil.ReadFile(path)
if err != nil {
err = errors.Wrapf(err, "failed to read stork feed config")
return err
}

feedCfg, err := oracle.ParseStorkFeedConfig(cfgBody)
Copy link
Member

Choose a reason for hiding this comment

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

we should support price updates for multiple tickers

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated flow, can you plz recheck?

if err != nil {
log.WithError(err).WithFields(log.Fields{
"filename": d.Name(),
}).Errorln("failed to parse stork feed config")
return nil
}

storkFeedConfigs = append(storkFeedConfigs, feedCfg)

return nil
})

if err != nil {
err = errors.Wrapf(err, "stork feeds dir is specified, but failed to read from it: %s", *dynamicFeedsDir)
log.WithError(err).Fatalln("failed to load stork feeds")
Copy link

@coderabbitai coderabbitai bot Aug 19, 2024

Choose a reason for hiding this comment

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

Fix copy-paste error in error handling message.

The error message incorrectly references *dynamicFeedsDir instead of *storkFeedsDir. Correct this to avoid confusion.

- err = errors.Wrapf(err, "stork feeds dir is specified, but failed to read from it: %s", *dynamicFeedsDir)
+ err = errors.Wrapf(err, "stork feeds dir is specified, but failed to read from it: %s", *storkFeedsDir)
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.

Suggested change
err = errors.Wrapf(err, "stork feeds dir is specified, but failed to read from it: %s", *dynamicFeedsDir)
log.WithError(err).Fatalln("failed to load stork feeds")
err = errors.Wrapf(err, "stork feeds dir is specified, but failed to read from it: %s", *storkFeedsDir)
log.WithError(err).Fatalln("failed to load stork feeds")

Copy link
Contributor

Choose a reason for hiding this comment

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

@ThanhNhann can you accept this please 😉

Copy link

Choose a reason for hiding this comment

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

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

return
}

log.Infof("found %d stork feed configs", len(storkFeedConfigs))
}
Copy link

Choose a reason for hiding this comment

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

Review of Stork feed configuration handling in oracleCmd function.

The handling of Stork feed configurations is well-integrated into the existing system. The use of filepath.WalkDir to load configurations from a directory is efficient and appropriate.

However, there's a copy-paste error in the error handling (line 251). The error message incorrectly references *dynamicFeedsDir instead of *storkFeedsDir. This needs correction to avoid confusion during troubleshooting.

- err = errors.Wrapf(err, "stork feeds dir is specified, but failed to read from it: %s", *dynamicFeedsDir)
+ err = errors.Wrapf(err, "stork feeds dir is specified, but failed to read from it: %s", *storkFeedsDir)
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.

Suggested change
storkFeedConfigs := make([]*oracle.StorkFeedConfig, 0, 10)
if len(*storkFeedsDir) > 0 {
err := filepath.WalkDir(*storkFeedsDir, func(path string, d fs.DirEntry, err error) error {
if err != nil {
return err
} else if d.IsDir() {
return nil
} else if filepath.Ext(path) != ".toml" {
return nil
}
cfgBody, err := ioutil.ReadFile(path)
if err != nil {
err = errors.Wrapf(err, "failed to read stork feed config")
return err
}
feedCfg, err := oracle.ParseStorkFeedConfig(cfgBody)
if err != nil {
log.WithError(err).WithFields(log.Fields{
"filename": d.Name(),
}).Errorln("failed to parse stork feed config")
return nil
}
storkFeedConfigs = append(storkFeedConfigs, feedCfg)
return nil
})
if err != nil {
err = errors.Wrapf(err, "stork feeds dir is specified, but failed to read from it: %s", *dynamicFeedsDir)
log.WithError(err).Fatalln("failed to load stork feeds")
return
}
log.Infof("found %d stork feed configs", len(storkFeedConfigs))
}
err = errors.Wrapf(err, "stork feeds dir is specified, but failed to read from it: %s", *storkFeedsDir)


svc, err := oracle.NewService(
cosmosClient,
exchangetypes.NewQueryClient(daemonConn),
oracletypes.NewQueryClient(daemonConn),
feedProviderConfigs,
dynamicFeedConfigs,
storkFeedConfigs,
)
if err != nil {
log.Fatalln(err)
Expand Down
7 changes: 7 additions & 0 deletions examples/stork.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
provider = "Stork"
Copy link
Contributor

@hmoragrega hmoragrega Aug 22, 2024

Choose a reason for hiding this comment

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

This file should be committed in a different folder, with the code as it is, we cannot have price feed and stork configs in the same folder, because they are parsed twice, as DynamicFeedConfig and then as StorkFeedConfig too

Ideally, it'll be better if both file types can coexist in the same folder, it'll require less DevOps overhead and it's much clearer to know all the enabled feeds, the oracleType should be used to discriminate which type of config and the puller to get the prices

Copy link
Contributor Author

Choose a reason for hiding this comment

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

First we should separate them into different folders then process your idea in another pr to make this pr can be merged soon

ticker = "BTCUSD"
pullInterval = "1m"
oracleType = "Stork"
url = "wss://dev.api.stork-oracle.network/evm/subscribe"
header = ""
message = "{\"type\":\"subscribe\",\"trace_id\":\"123\",\"data\":[\"BTCUSD\"]}"
Loading