[consensus] Pin P2SH activation to block 173805 on mainnet
ClosedPublic
Actions

Authored by markblundeberg on Jun 19 2019, 03:54.

Details

Reviewers

jasonbcox
Fabien
deadalnix

Group Reviewers

Restricted Project

Commits

rSTAGING288031bc0197: [consensus] Pin P2SH activation to block 173805 on mainnet
rABC288031bc0197: [consensus] Pin P2SH activation to block 173805 on mainnet

Summary

Partial backport of PR11389:
https://github.com/bitcoin/bitcoin/pull/11389/commits/18e071841e83044b47aa45c3e98c0796a407d445

Reviewers please also confirm that the rest of the PR is not worth
backporting (it has to do with segwit activation / versionbits), so we
can mark it 'done'.

This is not a totally faithful backport, due to divergence (D1262).

Depends on D3365 due to how the miner_tests build on a weird chain.

Test Plan

make check
test_runner.py --extended

IBD with -checkpoints=0 and -assumevalid=0 on mainnet: OK up to at least height 216000 (2013 January)
IBD with -checkpoints=0 and -assumevalid=0 on testnet: OK up to at least height 230000 (2014 May)

Diff Detail

Repository

rABC Bitcoin ABC

Branch

patch_bip16height

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 6398
Build 10843: Bitcoin ABC Buildbot (legacy)
Build 10842: arc lint + arc unit

Event Timeline

markblundeberg created this revision.Jun 19 2019, 03:54

Herald added a reviewer: Restricted Project. · View Herald TranscriptJun 19 2019, 03:54

Herald added a reviewer: deadalnix. · View Herald Transcript

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

markblundeberg added inline comments.Jun 19 2019, 04:02

src/test/miner_tests.cpp
508	This was causing a segfault (null pointer deref) without parent D3365, due to the call path TestBlockValidity / ConnectBlock / CheckInputs / GetSpendHeight / LookupBlockIndex. Basically, the way the 210000-long chain is built above is very funky, and doesn't populate mapBlockIndex. So, LookupBlockIndex on that tip returns a nullptr, and GetSpendHeight barfs.

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing passed.

Harbormaster completed remote builds in B6398: Diff 9528.Jun 19 2019, 04:04

markblundeberg edited the test plan for this revision. (Show Details)Jun 19 2019, 04:54

markblundeberg edited the test plan for this revision. (Show Details)Jun 19 2019, 05:08

markblundeberg added inline comments.Jun 19 2019, 05:37

src/consensus/consensus.h
36	Note that this is the MTP for block 173804. So, for both the old and new code the first block with P2SH flag is 173805.

markblundeberg edited the summary of this revision. (Show Details)Jun 19 2019, 05:44

markblundeberg added inline comments.

src/test/miner_tests.cpp
448	^^^ This bit removed (came from D1262).
470	vvv ditto for the bit below this line; otherwise this test is just copied down

Fabien accepted this revision.Jun 19 2019, 08:20

Fabien added inline comments.

src/test/miner_tests.cpp
508	Nice ! This is the segfault I was after, good to see you found the missing backport !
src/validation.cpp
1565	Nit: remove extra parenthesis: `(pindex->nHeight + 1)` => `pindex->nHeight + 1`