Skip to content

Conversation

@fridrik01
Copy link
Contributor

@fridrik01 fridrik01 commented Jun 26, 2025

Context

We spend hours today debugging why we had long blocktimes in our devnet. After a lot of debugging we found out that the chain spec file used in our devnet had the wrong name for max block delay. This caused the node to start with MaxBlockDelay of 0seconds.

Fix
With the introduction of stable blocktime, we added an embedded struct to our chain spec file and current validation logic did not support embedded structs causing this issue to be missed on node startup. This PR addresses this.

Test plan

Rename max-block-delay to max-delay in our testing/files/spec.toml file. Then startup the node:

make start-custom testing/files/spec.toml

You should see an error like:

Error: failed to create chain spec: missing required configuration for key: block-delay-configuration.max-block-delay

@codecov
Copy link

codecov bot commented Jun 26, 2025

Codecov Report

Attention: Patch coverage is 61.53846% with 15 lines in your changes missing coverage. Please review.

Project coverage is 60.79%. Comparing base (9d4ec3a) to head (ea96fcc).

Files with missing lines Patch % Lines
config/spec/creator.go 63.15% 11 Missing and 3 partials ⚠️
cli/commands/initialize/initialize.go 0.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@                    Coverage Diff                    @@
##           stable-block-time-pt2    #2830      +/-   ##
=========================================================
+ Coverage                  60.74%   60.79%   +0.04%     
=========================================================
  Files                        351      351              
  Lines                      16295    16328      +33     
=========================================================
+ Hits                        9898     9926      +28     
- Misses                      5631     5636       +5     
  Partials                     766      766              
Files with missing lines Coverage Δ
cli/commands/initialize/initialize.go 54.81% <0.00%> (ø)
config/spec/creator.go 70.45% <63.15%> (+8.63%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@fridrik01 fridrik01 requested review from abi87, calbera and rezbera June 26, 2025 16:39
@fridrik01 fridrik01 marked this pull request as ready for review June 26, 2025 17:42
@fridrik01 fridrik01 requested a review from a team as a code owner June 26, 2025 17:42
consensus-enable-height = 2
`,
wantErr: "missing required configuration for key: max-effective-balance",
},
Copy link
Contributor

@calbera calbera Jul 1, 2025

Choose a reason for hiding this comment

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

can we also add a test case where err is nil? i.e. the valid way to load block-delay-configuration, as reference

Copy link
Contributor

@calbera calbera left a comment

Choose a reason for hiding this comment

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

utACK

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants