Refactor: break down read config from app opts parser #1453
Conversation
WalkthroughThe refactoring of the 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 (
|
kienn6034
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (1)
- cosmos/config/config.go (3 hunks)
Additional comments: 7
cosmos/config/config.go (7)
- 80-109: The refactoring of
ReadConfigFromAppOptsintoreadConfigFromAppOptsParserand subsequent parsing functions appears to be well-structured and modular. Each configuration aspect is handled by a dedicated function, which should improve readability and maintainability.- 112-128: The
readPolarisCoreSettingsfunction is well-defined and encapsulates the parsing of Polaris Core settings. It correctly handles errors and returns early if any occur.- 151-561: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [131-173]
The
readPolarisMinterSettingsfunction is well-defined and encapsulates the parsing of Polaris Minter settings. The use ofversion.NewInfo().GitCommitto set defaultExtraDatais a good fallback mechanism. Ensure that the git commit length check is consistent with the project's versioning strategy.
- 177-277: The
readPolarisChainSettingsfunction is well-defined and encapsulates the parsing of Polaris Chain settings. It correctly handles errors and returns early if any occur. The function is quite long; consider if breaking it down further would improve readability.- 280-320: The
readPolarisGPOSettingsfunction is well-defined and encapsulates the parsing of Polaris GPO settings. The conversion ofint64values tobig.Intis done correctly.- 393-536: The
readNodeSettingsfunction is well-defined and encapsulates the parsing of Node settings. It correctly handles errors and returns early if any occur. The function is quite long and complex; consider if breaking it down further would improve readability.- 539-560: The
readNodeHTTPTimeoutsSettingsfunction is well-defined and encapsulates the parsing of Node HTTP timeouts settings. It correctly handles errors and returns early if any occur.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 5
Configuration used: CodeRabbit UI
Files selected for processing (1)
- cosmos/config/config.go (3 hunks)
Additional comments: 4
cosmos/config/config.go (4)
- 78-148: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [70-109]
The refactoring of
ReadConfigFromAppOptsto delegate toreadConfigFromAppOptsParseris a good approach to simplify the original function. However, ensure that all calls toReadConfigFromAppOptshave been updated accordingly if its behavior has changed.
- 81-107: The breakdown of configuration parsing into separate functions within
readConfigFromAppOptsParseris a good practice for modularity and readability. Ensure that the error handling is consistent and that the configuration is not partially applied in case of an error.- 112-129:
readPolarisCoreSettingsfollows the established pattern of parsing and error checking. Ensure that the magic number comment//nolint:gomnd // its okay.is justified and that ignoring the magic number linting rule here is indeed okay.Verification successful
Verifying the Magic Number Linting Rule
The script has identified several files where the magic number linting rule is ignored. To ensure consistency, I'll review each of these files to verify if the
//nolint:gomndcomments are justified.I'll start by examining the occurrences of the
//nolint:gomndcomment in each file to determine if it's consistently used for legitimate cases where the magic number linting rule should be ignored.First, I'll review the occurrences of the
//nolint:gomndcomment in thecosmos/config/config.gofile to understand the context in which it's used.This will provide insight into the specific instances where the magic number linting rule is ignored in the
cosmos/config/config.gofile.After reviewing the occurrences in the
cosmos/config/config.gofile, I'll proceed to review the other files identified by the script to ensure consistency in the use of the//nolint:gomndcomment.Let's start by examining the occurrences in the
cosmos/config/config.gofile.
The search for occurrences of the
//nolint:gomndcomment in thecosmos/config/config.gofile returned one result. The comment is associated with a comparison of the length of a commit, and it's justified with the explanation// its okay.Given this context, it seems that the use of the
//nolint:gomndcomment is justified in thecosmos/config/config.gofile. The comment is used to suppress the magic number linting rule for a specific comparison, and the justification provided supports its use.Next, I'll proceed to review the occurrences of the
//nolint:gomndcomment in the other files identified by the initial script to ensure consistency in the use of this comment.This will provide a comprehensive view of the occurrences of the
//nolint:gomndcomment in the identified files, allowing for a thorough review of its consistency across the codebase.
The search for occurrences of the
//nolint:gomndcomment in the identified files has provided valuable insights into the consistency of its usage across the codebase. Here are the findings:
cosmos/config/default.go: The comment//nolint:gomnd // default.justifies the suppression of the magic number linting rule for a specific assignment related to a default value.
cosmos/precompile/governance/testutil.go: The comment//nolint:gomnd // its okay.explains the justification for ignoring the magic number linting rule for a specific case.
cosmos/crypto/hd/hd.go: The comment//nolint:gomnd // ignore magic numbersprovides a general justification for suppressing the magic number linting rule in the file.
cosmos/runtime/logging.go: The comment//nolint:gomnd // 2 times.explains the reason for ignoring the magic number linting rule for a specific multiplication.
cosmos/runtime/runtime.go: The comment//nolint:gomnd // as explained above.justifies the suppression of the magic number linting rule for a specifictime.Sleepduration.
e2e/localnet/utils/utils.go: The comments//nolint:gomnd // success.,//nolint:gomnd // fail., and//nolint:gomnd // temporary.provide explanations for ignoring the magic number linting rule in different contexts.
e2e/testapp/polard/cmd/commands.go: The comments//nolint:gomnd // from sdk.justify the suppression of the magic number linting rule for specific values.
eth/version/version.go: The comment//nolint:gomnd // its okay.provides a justification for ignoring the magic number linting rule for a specific comparison.
eth/core/genesis.go: The comments//nolint:gomnd // its okay.and//nolint:gomnd // its okay.justify the suppression of the magic number linting rule for specific values.
eth/polar/config.go: The comment//nolint:gomnd // default.explains the justification for ignoring the magic number linting rule for a specific value.Based on these findings, it appears that the
//nolint:gomndcomments are consistently used with justifications across the identified files, aligning with legitimate cases where the magic number linting rule should be ignored.Therefore, the usage of the
//nolint:gomndcomments seems to be consistent and justified across the codebase.The verification is successful, and the usage of the
//nolint:gomndcomments appears to be consistent and justified across the identified files.Result
* 539-560: `readNodeHTTPTimeoutsSettings` sets various HTTP timeout settings. The function is consistent with the rest of the refactoring and follows the established pattern.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for other instances where the magic number linting rule is ignored and ensure consistency. rg --type go '//nolint:gomnd' --files-with-matchesLength of output: 341
Script:
#!/bin/bash # Search for occurrences of the `//nolint:gomnd` comment in the `cosmos/config/config.go` file. rg --type go '//nolint:gomnd' cosmos/config/config.goLength of output: 106
Script:
#!/bin/bash # Search for occurrences of the `//nolint:gomnd` comment in the other identified files. rg --type go '//nolint:gomnd' eth/core/genesis.go eth/version/version.go eth/polar/config.go e2e/testapp/polard/cmd/commands.go e2e/localnet/utils/utils.go cosmos/runtime/runtime.go cosmos/runtime/logging.go cosmos/crypto/hd/hd.go cosmos/precompile/governance/testutil.go cosmos/config/default.goLength of output: 1628
| return err | ||
| } | ||
| if conf.Node.WSHost, err = | ||
| parser.GetString(flags.WsHost); err != nil { | ||
| return nil, err | ||
| return err | ||
| } | ||
| if conf.Node.WSPort, err = | ||
| parser.GetInt(flags.WsPort); err != nil { | ||
| return nil, err | ||
| return err | ||
| } | ||
| if conf.Node.WSPathPrefix, err = | ||
| parser.GetString(flags.WsPathPrefix); err != nil { | ||
| return nil, err | ||
| return err | ||
| } | ||
| if conf.Node.WSOrigins, err = | ||
| parser.GetStringSlice(flags.WsOrigins); err != nil { | ||
| return nil, err | ||
| return err | ||
| } | ||
| if conf.Node.WSModules, err = | ||
| parser.GetStringSlice(flags.WsModules); err != nil { | ||
| return nil, err | ||
| return err | ||
| } | ||
| if conf.Node.WSExposeAll, err = | ||
| parser.GetBool(flags.WsExposeAll); err != nil { | ||
| return nil, err | ||
| return err | ||
| } | ||
| if conf.Node.GraphQLCors, err = | ||
| parser.GetStringSlice(flags.GraphqlCors); err != nil { | ||
| return nil, err | ||
| return err | ||
| } | ||
| if conf.Node.GraphQLVirtualHosts, err = | ||
| parser.GetStringSlice(flags.GraphqlVirtualHosts); err != nil { | ||
| return nil, err | ||
| return err | ||
| } | ||
| if conf.Node.AllowUnprotectedTxs, err = | ||
| parser.GetBool(flags.AllowUnprotectedTxs); err != nil { | ||
| return nil, err | ||
| return err | ||
| } | ||
| if conf.Node.BatchRequestLimit, err = | ||
| parser.GetInt(flags.BatchRequestLimit); err != nil { | ||
| return nil, err | ||
| return err | ||
| } | ||
| if conf.Node.BatchResponseMaxSize, err = | ||
| parser.GetInt(flags.BatchResponseMaxSize); err != nil { | ||
| return nil, err | ||
| return err | ||
| } | ||
| if conf.Node.JWTSecret, err = | ||
| parser.GetString(flags.JwtSecret); err != nil { | ||
| return nil, err | ||
| return err | ||
| } | ||
| if conf.Node.DBEngine, err = | ||
| parser.GetString(flags.DBEngine); err != nil { | ||
| return nil, err | ||
| return err | ||
| } | ||
|
|
||
| // Node.HTTPTimeouts settings | ||
| return nil | ||
| } | ||
|
|
||
| func readNodeHTTPTimeoutsSettings(parser AppOptionsParser, conf *Config) error { | ||
| var err error | ||
|
|
||
| if conf.Node.HTTPTimeouts.ReadTimeout, err = | ||
| parser.GetTimeDuration(flags.ReadTimeout); err != nil { | ||
| return nil, err | ||
| return err | ||
| } | ||
| if conf.Node.HTTPTimeouts.ReadHeaderTimeout, err = | ||
| parser.GetTimeDuration( | ||
| flags.ReadHeaderTimeout); err != nil { | ||
| return nil, err | ||
| return err | ||
| } | ||
| if conf.Node.HTTPTimeouts.WriteTimeout, err = | ||
| parser.GetTimeDuration(flags.WriteTimeout); err != nil { | ||
| return nil, err | ||
| return err | ||
| } | ||
| if conf.Node.HTTPTimeouts.IdleTimeout, err = | ||
| parser.GetTimeDuration(flags.IdleTimeout); err != nil { | ||
| return nil, err | ||
| return err | ||
| } | ||
|
|
||
| return conf, nil | ||
| return nil | ||
| } |
There was a problem hiding this comment.
📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [131-174]
In readPolarisMinterSettings, the check for the git commit length is hardcoded to 40. This is a magic number and should be defined as a constant to avoid magic numbers in the code.
- if len(commit) != 40 { //nolint:gomnd // its okay.
+ const expectedGitCommitLength = 40
+ if len(commit) != expectedGitCommitLength {| } | ||
| if conf.Polar.Chain.EIP155Block, err = | ||
| parser.GetBigInt(flags.EIP155Block); err != nil { | ||
| return nil, err | ||
| return err | ||
| } | ||
| if conf.Polar.Chain.EIP158Block, err = | ||
| parser.GetBigInt(flags.EIP158Block); err != nil { | ||
| return nil, err | ||
| return err | ||
| } | ||
| if conf.Polar.Chain.ByzantiumBlock, err = | ||
| parser.GetBigInt(flags.ByzantiumBlock); err != nil { | ||
| return nil, err | ||
| return err | ||
| } | ||
| if conf.Polar.Chain.ConstantinopleBlock, err = | ||
| parser.GetBigInt(flags.ConstantinopleBlock); err != nil { | ||
| return nil, err | ||
| return err | ||
| } | ||
| if conf.Polar.Chain.PetersburgBlock, err = | ||
| parser.GetBigInt(flags.PetersburgBlock); err != nil { | ||
| return nil, err | ||
| return err | ||
| } | ||
| if conf.Polar.Chain.IstanbulBlock, err = | ||
| parser.GetBigInt(flags.IstanbulBlock); err != nil { | ||
| return nil, err | ||
| return err | ||
| } | ||
| if conf.Polar.Chain.MuirGlacierBlock, err = | ||
| parser.GetBigInt(flags.MuirGlacierBlock); err != nil { | ||
| return nil, err | ||
| return err | ||
| } | ||
| if conf.Polar.Chain.BerlinBlock, err = | ||
| parser.GetBigInt(flags.BerlinBlock); err != nil { | ||
| return nil, err | ||
| return err | ||
| } | ||
| if conf.Polar.Chain.LondonBlock, err = | ||
| parser.GetBigInt(flags.LondonBlock); err != nil { | ||
| return nil, err | ||
| return err | ||
| } | ||
| if conf.Polar.Chain.ArrowGlacierBlock, err = | ||
| parser.GetBigInt(flags.ArrowGlacierBlock); err != nil { | ||
| return nil, err | ||
| return err | ||
| } | ||
| if conf.Polar.Chain.GrayGlacierBlock, err = | ||
| parser.GetBigInt(flags.GrayGlacierBlock); err != nil { | ||
| return nil, err | ||
| return err | ||
| } | ||
| if conf.Polar.Chain.MergeNetsplitBlock, err = | ||
| parser.GetBigInt(flags.MergeNetsplitBlock); err != nil { | ||
| return nil, err | ||
| return err | ||
| } | ||
| if conf.Polar.Chain.ShanghaiTime, err = | ||
| parser.GetUint64Ptr(flags.ShanghaiTime); err != nil { | ||
| return nil, err | ||
| return err | ||
| } | ||
|
|
||
| if conf.Polar.Chain.CancunTime, err = | ||
| parser.GetUint64Ptr(flags.CancunTime); err != nil { | ||
| return nil, err | ||
| return err | ||
| } | ||
| if conf.Polar.Chain.PragueTime, err = | ||
| parser.GetUint64Ptr(flags.PragueTime); err != nil { | ||
| return nil, err | ||
| return err | ||
| } | ||
| if conf.Polar.Chain.VerkleTime, err = | ||
| parser.GetUint64Ptr(flags.VerkleTime); err != nil { | ||
| return nil, err | ||
| return err | ||
| } | ||
|
|
||
| if conf.Polar.Chain.TerminalTotalDifficulty, err = | ||
| parser.GetBigInt( | ||
| flags.TerminalTotalDifficulty); err != nil { | ||
| return nil, err | ||
| return err | ||
| } | ||
|
|
||
| if conf.Polar.Chain.TerminalTotalDifficultyPassed, err = | ||
| parser.GetBool( | ||
| flags.TerminalTotalDifficultyPassed); err != nil { | ||
| return nil, err | ||
| return err | ||
| } | ||
|
|
||
| // Polar.GPO settings | ||
| return nil | ||
| } |
There was a problem hiding this comment.
readPolarisChainSettings is setting a large number of configuration options. Consider if this function could be further broken down for better readability and maintainability.
| func readPolarisGPOSettings(parser AppOptionsParser, conf *Config) error { | ||
| var ( | ||
| err error | ||
| val int64 | ||
| ) | ||
|
|
||
| if conf.Polar.GPO.Blocks, err = | ||
| parser.GetInt(flags.Blocks); err != nil { | ||
| return nil, err | ||
| return err | ||
| } | ||
| if conf.Polar.GPO.Percentile, err = | ||
| parser.GetInt(flags.Percentile); err != nil { | ||
| return nil, err | ||
| return err | ||
| } | ||
| if conf.Polar.GPO.MaxHeaderHistory, err = | ||
| parser.GetUint64(flags.MaxHeaderHistory); err != nil { | ||
| return nil, err | ||
| return err | ||
| } | ||
| if conf.Polar.GPO.MaxBlockHistory, err = | ||
| parser.GetUint64(flags.MaxBlockHistory); err != nil { | ||
| return nil, err | ||
| return err | ||
| } | ||
| if val, err = | ||
| parser.GetInt64(flags.Default); err != nil { | ||
| return nil, err | ||
| return err | ||
| } | ||
| conf.Polar.GPO.Default = big.NewInt(val) | ||
|
|
||
| if val, err = | ||
| parser.GetInt64(flags.MaxPrice); err != nil { | ||
| return nil, err | ||
| return err | ||
| } | ||
| conf.Polar.GPO.MaxPrice = big.NewInt(val) | ||
|
|
||
| if val, err = | ||
| parser.GetInt64(flags.IgnorePrice); err != nil { | ||
| return nil, err | ||
| return err | ||
| } | ||
| conf.Polar.GPO.IgnorePrice = big.NewInt(val) | ||
|
|
||
| // LegacyPool | ||
| return nil | ||
| } |
There was a problem hiding this comment.
The conversion from int64 to big.Int in readPolarisGPOSettings is done correctly. However, consider creating a helper function for this repeated pattern to adhere to the DRY principle.
+ func newBigIntFromInt64(val int64) *big.Int {
+ return big.NewInt(val)
+ }
- conf.Polar.GPO.Default = big.NewInt(val)
+ conf.Polar.GPO.Default = newBigIntFromInt64(val)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 readPolarisGPOSettings(parser AppOptionsParser, conf *Config) error { | |
| var ( | |
| err error | |
| val int64 | |
| ) | |
| if conf.Polar.GPO.Blocks, err = | |
| parser.GetInt(flags.Blocks); err != nil { | |
| return nil, err | |
| return err | |
| } | |
| if conf.Polar.GPO.Percentile, err = | |
| parser.GetInt(flags.Percentile); err != nil { | |
| return nil, err | |
| return err | |
| } | |
| if conf.Polar.GPO.MaxHeaderHistory, err = | |
| parser.GetUint64(flags.MaxHeaderHistory); err != nil { | |
| return nil, err | |
| return err | |
| } | |
| if conf.Polar.GPO.MaxBlockHistory, err = | |
| parser.GetUint64(flags.MaxBlockHistory); err != nil { | |
| return nil, err | |
| return err | |
| } | |
| if val, err = | |
| parser.GetInt64(flags.Default); err != nil { | |
| return nil, err | |
| return err | |
| } | |
| conf.Polar.GPO.Default = big.NewInt(val) | |
| if val, err = | |
| parser.GetInt64(flags.MaxPrice); err != nil { | |
| return nil, err | |
| return err | |
| } | |
| conf.Polar.GPO.MaxPrice = big.NewInt(val) | |
| if val, err = | |
| parser.GetInt64(flags.IgnorePrice); err != nil { | |
| return nil, err | |
| return err | |
| } | |
| conf.Polar.GPO.IgnorePrice = big.NewInt(val) | |
| // LegacyPool | |
| return nil | |
| } | |
| func readPolarisGPOSettings(parser AppOptionsParser, conf *Config) error { | |
| var ( | |
| err error | |
| val int64 | |
| ) | |
| if conf.Polar.GPO.Blocks, err = | |
| parser.GetInt(flags.Blocks); err != nil { | |
| return err | |
| } | |
| if conf.Polar.GPO.Percentile, err = | |
| parser.GetInt(flags.Percentile); err != nil { | |
| return err | |
| } | |
| if conf.Polar.GPO.MaxHeaderHistory, err = | |
| parser.GetUint64(flags.MaxHeaderHistory); err != nil { | |
| return err | |
| } | |
| if conf.Polar.GPO.MaxBlockHistory, err = | |
| parser.GetUint64(flags.MaxBlockHistory); err != nil { | |
| return err | |
| } | |
| if val, err = | |
| parser.GetInt64(flags.Default); err != nil { | |
| return err | |
| } | |
| conf.Polar.GPO.Default = newBigIntFromInt64(val) | |
| if val, err = | |
| parser.GetInt64(flags.MaxPrice); err != nil { | |
| return err | |
| } | |
| conf.Polar.GPO.MaxPrice = big.NewInt(val) | |
| if val, err = | |
| parser.GetInt64(flags.IgnorePrice); err != nil { | |
| return err | |
| } | |
| conf.Polar.GPO.IgnorePrice = big.NewInt(val) | |
| return nil | |
| } | |
| func newBigIntFromInt64(val int64) *big.Int { | |
| return big.NewInt(val) | |
| } |
| func readPolarisLegacyPool(parser AppOptionsParser, conf *Config) error { | ||
| var err error | ||
|
|
||
| if conf.Polar.LegacyTxPool.Locals, err = | ||
| parser.GetCommonAddressList(flags.Locals); err != nil { | ||
| return nil, err | ||
| return err | ||
| } | ||
| if conf.Polar.LegacyTxPool.NoLocals, err = | ||
| parser.GetBool(flags.NoLocals); err != nil { | ||
| return nil, err | ||
| return err | ||
| } | ||
|
|
||
| if conf.Polar.LegacyTxPool.Journal, err = | ||
| parser.GetString(flags.Journal); err != nil { | ||
| return nil, err | ||
| return err | ||
| } | ||
|
|
||
| if conf.Polar.LegacyTxPool.Journal == "" { | ||
| conf.Polar.LegacyTxPool.Journal, err = | ||
| parser.GetString(sdkflags.FlagHome) | ||
| if err != nil { | ||
| return nil, err | ||
| return err | ||
| } | ||
| conf.Polar.LegacyTxPool.Journal += "/data/transactions.rlp" | ||
| } | ||
|
|
||
| if conf.Polar.LegacyTxPool.Rejournal, err = | ||
| parser.GetTimeDuration(flags.ReJournal); err != nil { | ||
| return nil, err | ||
| return err | ||
| } | ||
|
|
||
| if conf.Polar.LegacyTxPool.PriceLimit, err = | ||
| parser.GetUint64(flags.PriceLimit); err != nil { | ||
| return nil, err | ||
| return err | ||
| } | ||
|
|
||
| if conf.Polar.LegacyTxPool.PriceBump, err = | ||
| parser.GetUint64(flags.PriceBump); err != nil { | ||
| return nil, err | ||
| return err | ||
| } | ||
|
|
||
| if conf.Polar.LegacyTxPool.AccountSlots, err = | ||
| parser.GetUint64(flags.AccountSlots); err != nil { | ||
| return nil, err | ||
| return err | ||
| } | ||
|
|
||
| if conf.Polar.LegacyTxPool.GlobalSlots, err = | ||
| parser.GetUint64(flags.GlobalSlots); err != nil { | ||
| return nil, err | ||
| return err | ||
| } | ||
|
|
||
| if conf.Polar.LegacyTxPool.AccountQueue, err = | ||
| parser.GetUint64(flags.AccountQueue); err != nil { | ||
| return nil, err | ||
| return err | ||
| } | ||
|
|
||
| if conf.Polar.LegacyTxPool.GlobalQueue, err = | ||
| parser.GetUint64(flags.GlobalQueue); err != nil { | ||
| return nil, err | ||
| return err | ||
| } | ||
|
|
||
| if conf.Polar.LegacyTxPool.Lifetime, err = | ||
| parser.GetTimeDuration(flags.Lifetime); err != nil { | ||
| return nil, err | ||
| return err | ||
| } | ||
|
|
||
| // Node settings | ||
| return nil | ||
| } |
There was a problem hiding this comment.
readPolarisLegacyPool sets various transaction pool settings. Ensure that the default journal path concatenation is done correctly and consider using filepath.Join for path construction to avoid issues with path separators on different operating systems.
- conf.Polar.LegacyTxPool.Journal += "/data/transactions.rlp"
+ conf.Polar.LegacyTxPool.Journal = filepath.Join(conf.Polar.LegacyTxPool.Journal, "data", "transactions.rlp")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 readPolarisLegacyPool(parser AppOptionsParser, conf *Config) error { | |
| var err error | |
| if conf.Polar.LegacyTxPool.Locals, err = | |
| parser.GetCommonAddressList(flags.Locals); err != nil { | |
| return nil, err | |
| return err | |
| } | |
| if conf.Polar.LegacyTxPool.NoLocals, err = | |
| parser.GetBool(flags.NoLocals); err != nil { | |
| return nil, err | |
| return err | |
| } | |
| if conf.Polar.LegacyTxPool.Journal, err = | |
| parser.GetString(flags.Journal); err != nil { | |
| return nil, err | |
| return err | |
| } | |
| if conf.Polar.LegacyTxPool.Journal == "" { | |
| conf.Polar.LegacyTxPool.Journal, err = | |
| parser.GetString(sdkflags.FlagHome) | |
| if err != nil { | |
| return nil, err | |
| return err | |
| } | |
| conf.Polar.LegacyTxPool.Journal += "/data/transactions.rlp" | |
| } | |
| if conf.Polar.LegacyTxPool.Rejournal, err = | |
| parser.GetTimeDuration(flags.ReJournal); err != nil { | |
| return nil, err | |
| return err | |
| } | |
| if conf.Polar.LegacyTxPool.PriceLimit, err = | |
| parser.GetUint64(flags.PriceLimit); err != nil { | |
| return nil, err | |
| return err | |
| } | |
| if conf.Polar.LegacyTxPool.PriceBump, err = | |
| parser.GetUint64(flags.PriceBump); err != nil { | |
| return nil, err | |
| return err | |
| } | |
| if conf.Polar.LegacyTxPool.AccountSlots, err = | |
| parser.GetUint64(flags.AccountSlots); err != nil { | |
| return nil, err | |
| return err | |
| } | |
| if conf.Polar.LegacyTxPool.GlobalSlots, err = | |
| parser.GetUint64(flags.GlobalSlots); err != nil { | |
| return nil, err | |
| return err | |
| } | |
| if conf.Polar.LegacyTxPool.AccountQueue, err = | |
| parser.GetUint64(flags.AccountQueue); err != nil { | |
| return nil, err | |
| return err | |
| } | |
| if conf.Polar.LegacyTxPool.GlobalQueue, err = | |
| parser.GetUint64(flags.GlobalQueue); err != nil { | |
| return nil, err | |
| return err | |
| } | |
| if conf.Polar.LegacyTxPool.Lifetime, err = | |
| parser.GetTimeDuration(flags.Lifetime); err != nil { | |
| return nil, err | |
| return err | |
| } | |
| // Node settings | |
| return nil | |
| } | |
| func readPolarisLegacyPool(parser AppOptionsParser, conf *Config) error { | |
| var err error | |
| if conf.Polar.LegacyTxPool.Locals, err = | |
| parser.GetCommonAddressList(flags.Locals); err != nil { | |
| return err | |
| } | |
| if conf.Polar.LegacyTxPool.NoLocals, err = | |
| parser.GetBool(flags.NoLocals); err != nil { | |
| return err | |
| } | |
| if conf.Polar.LegacyTxPool.Journal, err = | |
| parser.GetString(flags.Journal); err != nil { | |
| return err | |
| } | |
| if conf.Polar.LegacyTxPool.Journal == "" { | |
| conf.Polar.LegacyTxPool.Journal, err = | |
| parser.GetString(sdkflags.FlagHome) | |
| if err != nil { | |
| return err | |
| } | |
| conf.Polar.LegacyTxPool.Journal = filepath.Join(conf.Polar.LegacyTxPool.Journal, "data", "transactions.rlp") | |
| } | |
| if conf.Polar.LegacyTxPool.Rejournal, err = | |
| parser.GetTimeDuration(flags.ReJournal); err != nil { | |
| return err | |
| } | |
| if conf.Polar.LegacyTxPool.PriceLimit, err = | |
| parser.GetUint64(flags.PriceLimit); err != nil { | |
| return err | |
| } | |
| if conf.Polar.LegacyTxPool.PriceBump, err = | |
| parser.GetUint64(flags.PriceBump); err != nil { | |
| return err | |
| } | |
| if conf.Polar.LegacyTxPool.AccountSlots, err = | |
| parser.GetUint64(flags.AccountSlots); err != nil { | |
| return err | |
| } | |
| if conf.Polar.LegacyTxPool.GlobalSlots, err = | |
| parser.GetUint64(flags.GlobalSlots); err != nil { | |
| return err | |
| } | |
| if conf.Polar.LegacyTxPool.AccountQueue, err = | |
| parser.GetUint64(flags.AccountQueue); err != nil { | |
| return err | |
| } | |
| if conf.Polar.LegacyTxPool.GlobalQueue, err = | |
| parser.GetUint64(flags.GlobalQueue); err != nil { | |
| return err | |
| } | |
| if conf.Polar.LegacyTxPool.Lifetime, err = | |
| parser.GetTimeDuration(flags.Lifetime); err != nil { | |
| return err | |
| } | |
| return nil | |
| } |
| } | ||
| if conf.Node.HTTPVirtualHosts, err = | ||
| parser.GetStringSlice(flags.HTTPVirtualHosts); err != nil { | ||
| return nil, err | ||
| return err | ||
| } | ||
| if conf.Node.HTTPModules, err = | ||
| parser.GetStringSlice(flags.HTTPModules); err != nil { | ||
| return nil, err | ||
| return err | ||
| } | ||
| if conf.Node.HTTPPathPrefix, err = | ||
| parser.GetString(flags.HTTPPathPrefix); err != nil { | ||
| return nil, err | ||
| return err | ||
| } | ||
| if conf.Node.AuthAddr, err = | ||
| parser.GetString(flags.AuthAddr); err != nil { | ||
| return nil, err | ||
| return err | ||
| } | ||
| if conf.Node.AuthPort, err = | ||
| parser.GetInt(flags.AuthPort); err != nil { | ||
| return nil, err | ||
| return err | ||
| } | ||
| if conf.Node.AuthVirtualHosts, err = | ||
| parser.GetStringSlice(flags.AuthVirtualHosts); err != nil { | ||
| return nil, err | ||
| return err | ||
| } | ||
| if conf.Node.WSHost, err = | ||
| parser.GetString(flags.WsHost); err != nil { | ||
| return nil, err | ||
| return err | ||
| } | ||
| if conf.Node.WSPort, err = | ||
| parser.GetInt(flags.WsPort); err != nil { | ||
| return nil, err | ||
| return err | ||
| } | ||
| if conf.Node.WSPathPrefix, err = | ||
| parser.GetString(flags.WsPathPrefix); err != nil { | ||
| return nil, err | ||
| return err | ||
| } | ||
| if conf.Node.WSOrigins, err = | ||
| parser.GetStringSlice(flags.WsOrigins); err != nil { | ||
| return nil, err | ||
| return err | ||
| } | ||
| if conf.Node.WSModules, err = | ||
| parser.GetStringSlice(flags.WsModules); err != nil { | ||
| return nil, err | ||
| return err | ||
| } | ||
| if conf.Node.WSExposeAll, err = | ||
| parser.GetBool(flags.WsExposeAll); err != nil { | ||
| return nil, err | ||
| return err | ||
| } | ||
| if conf.Node.GraphQLCors, err = | ||
| parser.GetStringSlice(flags.GraphqlCors); err != nil { | ||
| return nil, err | ||
| return err | ||
| } | ||
| if conf.Node.GraphQLVirtualHosts, err = | ||
| parser.GetStringSlice(flags.GraphqlVirtualHosts); err != nil { | ||
| return nil, err | ||
| return err | ||
| } | ||
| if conf.Node.AllowUnprotectedTxs, err = | ||
| parser.GetBool(flags.AllowUnprotectedTxs); err != nil { | ||
| return nil, err | ||
| return err | ||
| } | ||
| if conf.Node.BatchRequestLimit, err = | ||
| parser.GetInt(flags.BatchRequestLimit); err != nil { | ||
| return nil, err | ||
| return err | ||
| } | ||
| if conf.Node.BatchResponseMaxSize, err = | ||
| parser.GetInt(flags.BatchResponseMaxSize); err != nil { | ||
| return nil, err | ||
| return err | ||
| } | ||
| if conf.Node.JWTSecret, err = | ||
| parser.GetString(flags.JwtSecret); err != nil { | ||
| return nil, err | ||
| return err | ||
| } | ||
| if conf.Node.DBEngine, err = | ||
| parser.GetString(flags.DBEngine); err != nil { | ||
| return nil, err | ||
| return err | ||
| } | ||
|
|
||
| // Node.HTTPTimeouts settings | ||
| return nil | ||
| } |
There was a problem hiding this comment.
readNodeSettings is marked with nolint comments for complexity and function length. This is a red flag for maintainability. Consider refactoring to reduce complexity and function length, possibly by breaking it down into smaller functions.
Refactor
Breakdown function
readConfigFromAppOptsParser()into smaller functionsSummary by CodeRabbit