Clarify BIP9 behaviour when nTimeout <= 0 #11494

pull ajtowns wants to merge 1 commits into bitcoin:master from ajtowns:vb_0_timeout changing 3 files +8 −5

ajtowns commented at 7:11 AM on October 13, 2017: member

Adds a comment in the BIP9Deployment class declaration for when to set nTimeout=0, and changes the checks for (nTimeout!=0) to (nTimeout>0) for consistency.

The remaining comparison for nTimeout is in rpc/blockchain.cpp:BIP9SoftForkDescPushBack which already checks for >0 rather than !=0.

Compatible with #11389 which adds another special case behaviour (negative nStartTime) for defining "BIP9" deployments.

Clarify BIP9 behaviour when nTimeout <= 0

Adds a comment in the BIP9Deployment class declaration for when to set
nTimeout=0, and changes the checks for (nTimeout!=0) to (nTimeout>0)
for consistency.

313c7a4d9b

in src/consensus/params.h:33 in 313c7a4d9b

  28 | @@ -29,7 +29,12 @@ struct BIP9Deployment {
  29 |      int bit;
  30 |      /** Start MedianTime for version bits miner confirmation. Can be a date in the past */
  31 |      int64_t nStartTime;
  32 | -    /** Timeout/expiry MedianTime for the deployment attempt. */
  33 | +    /** Timeout/expiry MedianTime for the deployment attempt.
  34 | +     * Setting to 0 will prevent advertising presence of softfork,

promag commented at 1:33 PM on October 18, 2017:

Either change to uint64_t nTimeout or update description to something like:

If greater than 0 then it prevents ...

since below you have changed to > operator and nTimeout is not unsigned.

fanquake added the label Consensus on Jan 22, 2018
DrahtBot commented at 8:30 PM on July 20, 2018: member

The last travis run for this pull request was 280 days ago and is thus outdated. To trigger a fresh travis build, this pull request should be closed and re-opened.
DrahtBot closed this on Jul 20, 2018
DrahtBot reopened this on Jul 20, 2018
fanquake added this to the "Mergeable" column in a project

fanquake moved this from the "Mergeable" to the "Concept ACK / Early Discussion" column in a project

fanquake removed this from the "Concept ACK / Early Discussion" column in a project

laanwj commented at 12:34 PM on August 31, 2018: member

Not sure what to do here? It has been open since 2017 has hardly any discussion.

I'm going to close it, let me know if it needs reopen.
laanwj closed this on Aug 31, 2018
laanwj added the label Up for grabs on Aug 31, 2018
MarcoFalke locked this on Sep 8, 2021
MarcoFalke removed the label Up for grabs on Mar 22, 2022

Contributors

Labels

Linked (view graph)

#11389 Support having SegWit always active in regtest (sipa, ajtowns, jnewbery)