Conversation
The hardcoded 120-second block wait timeout in Chain.Start() is too short for state sync nodes that need extra time to sync before producing blocks. Add a configurable WithBlockWaitTimeout option on ChainBuilder so callers can set a longer timeout when needed. Closes celestiaorg/celestia-app#7017 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe changes introduce a configurable Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
framework/docker/cosmos/chain_builder.go (1)
348-354: Keep timeout when cloning from an existing chain.
WithBlockWaitTimeoutis added, butNewChainBuilderFromChain(Line 186 onward) does not copychain.blockWaitTimeout, so this config can be dropped when rebuilding from an existing chain.♻️ Proposed fix
func NewChainBuilderFromChain(chain *Chain) *ChainBuilder { cfg := &chain.Config return NewChainBuilder(chain.t). WithLogger(chain.log). WithEncodingConfig(cfg.EncodingConfig). WithName(cfg.Name). WithChainID(cfg.ChainID). WithBinaryName(cfg.Bin). WithCoinType(cfg.CoinType). WithGasPrices(cfg.GasPrices). WithGasAdjustment(cfg.GasAdjustment). WithBech32Prefix(cfg.Bech32Prefix). WithDenom(cfg.Denom). WithGenesis(cfg.GenesisFileBz). WithImage(cfg.Image). WithDockerClient(chain.Config.DockerClient). WithDockerNetworkID(chain.Config.DockerNetworkID). WithAdditionalStartArgs(cfg.AdditionalStartArgs...). - WithEnv(cfg.Env...) + WithEnv(cfg.Env...). + WithBlockWaitTimeout(chain.blockWaitTimeout) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@framework/docker/cosmos/chain_builder.go` around lines 348 - 354, NewChainBuilderFromChain currently doesn't copy the ChainBuilder.blockWaitTimeout set via WithBlockWaitTimeout, so when cloning an existing chain that timeout is lost; update NewChainBuilderFromChain to copy the blockWaitTimeout field from the source chain into the new ChainBuilder (preserve same behavior as other copied fields), referencing ChainBuilder.blockWaitTimeout, the WithBlockWaitTimeout method, and the NewChainBuilderFromChain constructor to locate where to add the assignment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@framework/docker/cosmos/chain.go`:
- Around line 337-341: The block wait uses context.Background() which ignores
cancellations from the caller; in Start(ctx context.Context) replace
context.Background() with the incoming ctx when creating blockWaitCtx so the
block wait respects the caller's cancellation/deadline (adjust creation of
blockWaitCtx and cancel where blockWaitTimeout and blockWaitCtx are defined to
use ctx instead of context.Background()); ensure you still apply the timeout
derived from blockWaitTimeout via context.WithTimeout(ctx, blockWaitTimeout) and
propagate the returned cancel as before.
---
Nitpick comments:
In `@framework/docker/cosmos/chain_builder.go`:
- Around line 348-354: NewChainBuilderFromChain currently doesn't copy the
ChainBuilder.blockWaitTimeout set via WithBlockWaitTimeout, so when cloning an
existing chain that timeout is lost; update NewChainBuilderFromChain to copy the
blockWaitTimeout field from the source chain into the new ChainBuilder (preserve
same behavior as other copied fields), referencing
ChainBuilder.blockWaitTimeout, the WithBlockWaitTimeout method, and the
NewChainBuilderFromChain constructor to locate where to add the assignment.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c63073d1-7aee-418e-ba3e-ed964cd0cb61
📒 Files selected for processing (2)
framework/docker/cosmos/chain.goframework/docker/cosmos/chain_builder.go
Replace context.Background() with the caller's ctx so the block wait respects the caller's cancellation and deadline instead of always running for the full timeout duration. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
WithBlockWaitTimeoutoption toChainBuilderso callers can override the hardcoded 120-second block wait timeout inChain.Start()WithBlockWaitTimeoutis not calledcontext.Background()) for the block wait timeout so it respects the caller's cancellation and deadlineCloses celestiaorg/celestia-app#7017
Test plan
WithBlockWaitTimeoutis usable from celestia-app's sync-to-tip test🤖 Generated with Claude Code