-
Notifications
You must be signed in to change notification settings - Fork 318
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: override timeout commit via --timeout-commit
#4103
Changes from all commits
0ab3f1e
eeeab6c
4b0aad0
c950c48
9a9a828
a3534af
c85e318
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -202,7 +202,8 @@ func Run(ctx context.Context, cfg BuilderConfig, dir string) error { | |
nil, | ||
0, | ||
encCfg, | ||
0, | ||
0, // upgrade height v2 | ||
0, // timeout commit | ||
Comment on lines
+205
to
+206
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Add support for configurable timeout commit via command line flag The hardcoded timeout commit value of 0 contradicts the PR objectives which aim to support custom timeout settings for testing environments. Consider these changes:
func main() {
rootCmd.Flags().Int("num-blocks", 100, "Number of blocks to generate")
+ rootCmd.Flags().Duration("timeout-commit", 0, "Block timeout commit duration for testing. Only applicable for private testnets")
type BuilderConfig struct {
NumBlocks int
BlockSize int
BlockInterval time.Duration
+ TimeoutCommit time.Duration
ExistingDir string
Namespace share.Namespace
ChainID string
AppVersion uint64
UpToTime bool
}
- 0, // timeout commit
+ cfg.TimeoutCommit,
func Run(ctx context.Context, cfg BuilderConfig, dir string) error {
+ // Validate timeout commit usage
+ if cfg.TimeoutCommit > 0 && !strings.HasPrefix(cfg.ChainID, "private-") {
+ return fmt.Errorf("timeout commit can only be set for private testnets (chain IDs prefixed with 'private-')")
+ }
|
||
util.EmptyAppOptions{}, | ||
baseapp.SetMinGasPrices(fmt.Sprintf("%f%s", appconsts.DefaultMinGasPrice, appconsts.BondDenom)), | ||
) | ||
|
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 suggestion
Add validation for public network protection.
Consider adding validation in the constructor to prevent setting custom timeout commits on public networks (Arabica, Mocha, or Mainnet Beta) as mentioned in the documentation.
📝 Committable suggestion