Summary
In src/chainparams.cpp, CRegTestParams::UpdateActivationParametersFromArgs(), line 409 (verified on v0.21.5.5 and master), the local variables nStartHeight and nTimeoutHeight are declared without initialization:
int64_t nStartTime, nTimeout, nStartHeight, nTimeoutHeight;
When -vbparams is invoked with the 3-argument form (deployment:start:end), both height fields are skipped by the size() > 3 / size() > 4 guards on lines 416 and 419. With the 4-argument form, only nTimeoutHeight is skipped. In either case, uninitialized stack values are passed to UpdateVersionBitsParameters() on line 425.
Reading uninitialized automatic-storage variables is undefined behavior under the C++ standard ([basic.indet]).
Affected code
src/chainparams.cpp (v0.21.5.5, lines 405–430):
boost::split(vDeploymentParams, strDeployment, boost::is_any_of(":"));
if (vDeploymentParams.size() < 3 || 5 < vDeploymentParams.size()) {
throw std::runtime_error("Version bits parameters malformed, expecting deployment:start:end[:heightstart:heightend]");
}
int64_t nStartTime, nTimeout, nStartHeight, nTimeoutHeight; // ← uninitialized
if (!ParseInt64(vDeploymentParams[1], &nStartTime)) { ... }
if (!ParseInt64(vDeploymentParams[2], &nTimeout)) { ... }
if (vDeploymentParams.size() > 3 && !ParseInt64(vDeploymentParams[3], &nStartHeight)) { ... }
if (vDeploymentParams.size() > 4 && !ParseInt64(vDeploymentParams[4], &nTimeoutHeight)) { ... }
bool found = false;
for (int j=0; j < (int)Consensus::MAX_VERSION_BITS_DEPLOYMENTS; ++j) {
if (vDeploymentParams[0] == VersionBitsDeploymentInfo[j].name) {
UpdateVersionBitsParameters(Consensus::DeploymentPos(j), nStartTime, nTimeout, nStartHeight, nTimeoutHeight);
found = true;
LogPrintf("Setting version bits activation parameters for %s to start=%ld, timeout=%ld, start_height=%d, timeout_height=%d\n", vDeploymentParams[0], nStartTime, nTimeout, nStartHeight, nTimeoutHeight);
break;
}
}
If vDeploymentParams.size() is 3 or 4, one or both height fields are never assigned. The range guard at line 406 prevents fewer than 3 or more than 5 arguments, but does not address the in-range case where the optional height arguments are simply omitted — which is the common BIP9-compatible form.
This appears to be the only such pattern in chainparams.cpp; a grep for multi-variable int64_t declarations in this file returns this line alone. The issue is therefore localized to the change that introduced the height-based fields (MWEB integration), and not a systemic style problem.
Reproduction
litecoind -regtest -vbparams=testdummy:1199145601:1230767999
The resulting log line contains a garbage timeout_height:
Setting version bits activation parameters for testdummy to
start=1199145601, timeout=1230767999, start_height=0, timeout_height=6183037403340677632
0x55ce8fc25b132200-class values vary between runs and builds. (Observed on a Litecoin-derived codebase running this parser unchanged; cited in "Provenance" below.)
Note: in this particular log, start_height reads as 0, but this is not evidence of correct initialization — it is simply the indeterminate value that nStartHeight happened to hold on this run. Under the 3-arg form both fields are uninitialized; either can read as nonzero on another build, as timeout_height does here.
Comparison with Bitcoin Core
Bitcoin Core's equivalent parser, ReadRegTestArgs() in src/chainparams.cpp (verified on v31.0), avoids this class of bug through two complementary mechanisms:
// Layer 1: value-initialize the parameter struct
CChainParams::VersionBitsParameters vbparams{};
// ...
if (vDeploymentParams.size() >= 4) {
// ... parse min_activation_height ...
vbparams.min_activation_height = *min_activation_height;
} else {
// Layer 2: explicit fallback in the else-branch
vbparams.min_activation_height = 0;
}
Note that Bitcoin Core's min_activation_height is not the same concept as Litecoin's height-based nStartHeight/nTimeoutHeight (the former is a BIP9 minimum-activation height; the latter are BIP8 activation bounds).
The point of comparison is not the field semantics but the initialization discipline: Bitcoin Core consistently gives optional, possibly-unparsed fields an explicit default, whereas the Litecoin parser leaves the optional height fields with indeterminate values. The size-range check at line 406 does not cover the in-range omitted-argument case.
Operational impact
-vbparams is exposed only on regtest (it is parsed inside CRegTestParams::UpdateActivationParametersFromArgs()), so this does not affect mainnet or testnet consensus. The bug:
- corrupts regtest consensus parameters: the 3-arg form leaves both height fields uninitialized, the 4-arg form leaves nTimeoutHeight uninitialized
- produces nondeterministic results in
versionbits_computeblockversion unit tests
- propagates silently to downstream projects that inherited this parser
Proposed fix (minimal, one-line)
- int64_t nStartTime, nTimeout, nStartHeight, nTimeoutHeight;
+ int64_t nStartTime, nTimeout;
+ int64_t nStartHeight = 0, nTimeoutHeight = 0;
This matches the implicit contract that the height-based fields default to 0 when not specified on the command line, and aligns with Bitcoin Core's practice of giving optional command-line fields an explicit default.
Provenance
Identified while investigating a versionbits_computeblockversion unit-test failure on a Litecoin-derived codebase. The same parser pattern is present unchanged in that codebase; the one-line fix above was applied there for reference:
github.com/Aevust/rincoin-sim/commit/247dd40f57ccb3109bb559b9593dadf75c7fad9e
Filed as an issue rather than a PR; the fix is trivial enough to apply directly.
Summary
In
src/chainparams.cpp,CRegTestParams::UpdateActivationParametersFromArgs(), line 409 (verified onv0.21.5.5andmaster), the local variablesnStartHeightandnTimeoutHeightare declared without initialization:int64_t nStartTime, nTimeout, nStartHeight, nTimeoutHeight;When
-vbparamsis invoked with the 3-argument form (deployment:start:end), both height fields are skipped by thesize() > 3/size() > 4guards on lines 416 and 419. With the 4-argument form, onlynTimeoutHeightis skipped. In either case, uninitialized stack values are passed toUpdateVersionBitsParameters()on line 425.Reading uninitialized automatic-storage variables is undefined behavior under the C++ standard ([basic.indet]).
Affected code
src/chainparams.cpp(v0.21.5.5, lines 405–430):If
vDeploymentParams.size()is 3 or 4, one or both height fields are never assigned. The range guard at line 406 prevents fewer than 3 or more than 5 arguments, but does not address the in-range case where the optional height arguments are simply omitted — which is the common BIP9-compatible form.This appears to be the only such pattern in
chainparams.cpp; agrepfor multi-variableint64_tdeclarations in this file returns this line alone. The issue is therefore localized to the change that introduced the height-based fields (MWEB integration), and not a systemic style problem.Reproduction
The resulting log line contains a garbage
timeout_height:0x55ce8fc25b132200-class values vary between runs and builds. (Observed on a Litecoin-derived codebase running this parser unchanged; cited in "Provenance" below.)Note: in this particular log,
start_heightreads as0, but this is not evidence of correct initialization — it is simply the indeterminate value thatnStartHeighthappened to hold on this run. Under the 3-arg form both fields are uninitialized; either can read as nonzero on another build, astimeout_heightdoes here.Comparison with Bitcoin Core
Bitcoin Core's equivalent parser,
ReadRegTestArgs()insrc/chainparams.cpp(verified on v31.0), avoids this class of bug through two complementary mechanisms:Note that Bitcoin Core's
min_activation_heightis not the same concept as Litecoin's height-basednStartHeight/nTimeoutHeight(the former is a BIP9 minimum-activation height; the latter are BIP8 activation bounds).The point of comparison is not the field semantics but the initialization discipline: Bitcoin Core consistently gives optional, possibly-unparsed fields an explicit default, whereas the Litecoin parser leaves the optional height fields with indeterminate values. The size-range check at line 406 does not cover the in-range omitted-argument case.
Operational impact
-vbparamsis exposed only on regtest (it is parsed insideCRegTestParams::UpdateActivationParametersFromArgs()), so this does not affect mainnet or testnet consensus. The bug:versionbits_computeblockversionunit testsProposed fix (minimal, one-line)
This matches the implicit contract that the height-based fields default to
0when not specified on the command line, and aligns with Bitcoin Core's practice of giving optional command-line fields an explicit default.Provenance
Identified while investigating a
versionbits_computeblockversionunit-test failure on a Litecoin-derived codebase. The same parser pattern is present unchanged in that codebase; the one-line fix above was applied there for reference:github.com/Aevust/rincoin-sim/commit/247dd40f57ccb3109bb559b9593dadf75c7fad9e
Filed as an issue rather than a PR; the fix is trivial enough to apply directly.