Page MenuHomePhabricator
Differential D4743 Diff 14898 src/consensus/activation.cpp

Changeset View

Standalone View

View Options

src/consensus/activation.cpp

Show First 20 Lines • Show All 59 Lines • ▼ Show 20 Lines
} }
bool IsPhononEnabled(const Consensus::Params &params, bool IsPhononEnabled(const Consensus::Params &params,
const CBlockIndex *pindexPrev) { const CBlockIndex *pindexPrev) {
if (pindexPrev == nullptr) { if (pindexPrev == nullptr) {
return false; return false;
} }
return pindexPrev->GetMedianTimePast() >= return (!gArgs.IsArgSet("-phononactivationtime") &&
gArgs.GetArg("-phononactivationtime", params.phononActivationTime); gArgs.GetBoolArg("-activatenextupgrade", false)) ||
jasonbcoxUnsubmitted
Not Done
Inline Actions

I think making this checking logic more complex as well as complicating the associated test isn't the right way to do this. Setting -activatenextupgrade should be a proxy for setting -phononactivationtime (or whatever the next upgrade activation argument is) to current time - 1 second. This way, the -activatenextupgrade check is out of the code path for the actual upgrade (which IMO should be as simple as possible).

jasonbcox: I think making this checking logic more complex as well as complicating the associated test…
FabienAuthorUnsubmitted
Done
Inline Actions

There are a few problems with this approach:

  • Setting -activatenextupgrade implies setting -phononactivationtime, so this should be done during init, making it difficult to test (it requires a functional test that starts the node with the parameter, and a new RPC or similar to check the activation status, which defeats the maintenance reduction purpose of this diff).
  • Related to the issue above, this does not work for the unit tests (since there is no node startup).
  • Related to @markblundeberg comments, using MTP can lead to untested code on tests that do not build enough blocks.
Fabien: There are a few problems with this approach: - Setting `-activatenextupgrade` implies setting…
(pindexPrev->GetMedianTimePast() >=
gArgs.GetArg("-phononactivationtime", params.phononActivationTime));
} }