fix: move the key parameters to env #137
Conversation
WalkthroughThe changes add two new configuration parameters to control the loop duration and block search range. New environment variables and corresponding command-line options are introduced, and the configuration structs in both CLI and orchestrator packages are extended to include these parameters. Hardcoded defaults are replaced with dynamic values parsed from the configuration, and related logging and looping logic is updated across several orchestrator components. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User/CLI
participant E as Environment Loader
participant C as Config Setup
participant O as Orchestrator
participant BC as BatchCreator
participant OR as Oracle
participant SG as Signer
U->>E: Read .env and CLI parameters
E->>C: Provide config values (loopDuration, BlocksToSearch)
C->>O: Initialize orchestrator configuration
O->>BC: Start batch creation using LoopDuration
O->>OR: Run oracle loop with BlocksToSearch and LoopDuration
O->>SG: Run signer loop using LoopDuration
Poem
✨ Finishing Touches
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
orchestrator/orchestrator.go (1)
33-34: Add documentation for LoopDuration field.Similar to NumberOfBlocksToSearch, add a comment explaining the purpose and usage of LoopDuration.
+// Duration between each loop iteration for various orchestrator processes LoopDuration time.Durationcmd/peggo/orchestrator.go (1)
142-145: Improve error handling for loop duration parsing.Consider:
- Define the default duration as a constant
- Log a warning when falling back to default
+const defaultLoopDuration = 60 * time.Second loopDur, err = time.ParseDuration(*cfg.loopDuration) if err != nil { - loopDur = 60 * time.Second + log.WithError(err).Warnf("invalid loop duration %q, using default %v", *cfg.loopDuration, defaultLoopDuration) + loopDur = defaultLoopDuration }orchestrator/oracle.go (1)
90-93: Update the comment to reflect the new configuration.The comment still references the old constant name
defaultBlocksToSearch.-// ensure the block range is within defaultBlocksToSearch +// ensure the block range is within configured NumberOfBlocksToSearchcmd/peggo/options.go (1)
489-490: Fix typo in comment.The comment has a typo in "serach".
-/** Default block to serach **/ +/** Default blocks to search **/README.md (1)
92-93: Use consistent naming convention for command-line options.The new options use underscores (
--loop_duration,--blocks_to_search) while other options use hyphens (e.g.,--eth-chain-id,--cosmos-chain-id). Consider using hyphens for consistency.- --loop_duration Specify the main the loop duration (env $PEGGO_LOOP_DURATION) (default "60s") - --blocks_to_search Specify maximum block range for Ethereum event query (env $PEGGO_BLOCKS_TO_SEARCH) (default 2000) + --loop-duration Specify the main the loop duration (env $PEGGO_LOOP_DURATION) (default "60s") + --blocks-to-search Specify maximum block range for Ethereum event query (env $PEGGO_BLOCKS_TO_SEARCH) (default 2000)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
.env.example(1 hunks)README.md(3 hunks)cmd/peggo/options.go(3 hunks)cmd/peggo/orchestrator.go(2 hunks)orchestrator/batch_creator.go(1 hunks)orchestrator/oracle.go(2 hunks)orchestrator/orchestrator.go(1 hunks)orchestrator/signer.go(1 hunks)
🔇 Additional comments (4)
orchestrator/signer.go (1)
23-25: LGTM! Good use of configured loop duration.The changes correctly use the configured loop duration for both logging and execution.
orchestrator/batch_creator.go (1)
17-19: LGTM! Good use of configured loop duration.The changes correctly use the configured loop duration for both logging and execution.
orchestrator/oracle.go (1)
31-33: LGTM! Good addition of debug logging.The changes improve observability by logging the loop duration and make the duration configurable.
cmd/peggo/options.go (1)
276-277: LGTM! Good addition of configuration fields.The new fields follow the project's conventions and are properly typed.
| // Maximum block range for Ethereum event query. If the orchestrator has been offline for a long time, | ||
| // the oracle loop can potentially run longer than defaultLoopDur due to a surge of events. This usually happens | ||
| // when there are more than ~50 events to claim in a single run. | ||
| NumberOfBlocksToSearch uint64 | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Consider using uint64 consistently.
The NumberOfBlocksToSearch field uses uint64, which is appropriate for block numbers. However, ensure this type is used consistently across the codebase for block-related values.
🏁 Script executed:
#!/bin/bash
# Check for inconsistent block number types
rg -A 1 "block.*uint|block.*int" --type goLength of output: 301
Ensure Consistent Block Number Types Across Files
Please update block number types for consistency. While the organizer/orchestrator.go file correctly uses uint64 for NumberOfBlocksToSearch, our search revealed that in orchestrator/relayer.go block numbers are converted to int64 (e.g., using int64(vs.Height) and int64(batch.Block)). Aligning these types across the codebase would reduce potential confusion or type-mismatch issues, unless using int64 is required by external API constraints. Please either refactor the relayer code to use uint64 where applicable or add documentation explaining the need for the current discrepancy.
| PEGGO_LOOP_DURATION=60s | ||
| PEGGO_BLOCKS_TO_SEARCH=2000 |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add validation for the new configuration values.
The code should validate:
PEGGO_LOOP_DURATIONis a valid duration string (e.g., "60s", "1m").PEGGO_BLOCKS_TO_SEARCHis a positive number.
Would you like me to generate the validation code for these configuration values?
This PR will fix the issue #58
Changes:
defaultLoopDurto configdefaultBlocksToSearchto configSummary by CodeRabbit
New Features
Documentation