From 8dcb533226557041c0b745566057131a1178959b Mon Sep 17 00:00:00 2001 From: Sean Bowe Date: Tue, 5 May 2020 09:28:39 -0600 Subject: [PATCH 1/3] Add the intended testnet activation block of Heartwood to our intended rewind logic. --- src/chainparams.cpp | 2 ++ src/main.cpp | 4 +++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/chainparams.cpp b/src/chainparams.cpp index 31a9673ff..d8a3c0f60 100644 --- a/src/chainparams.cpp +++ b/src/chainparams.cpp @@ -322,6 +322,8 @@ public: uint256S("00367515ef2e781b8c9358b443b6329572599edd02c59e8af67db9785122f298"); consensus.vUpgrades[Consensus::UPGRADE_HEARTWOOD].nProtocolVersion = 170010; consensus.vUpgrades[Consensus::UPGRADE_HEARTWOOD].nActivationHeight = 903800; + consensus.vUpgrades[Consensus::UPGRADE_HEARTWOOD].hashActivationBlock = + uint256S("05688d8a0e9ff7c04f6f05e6d695dc5ab43b9c4803342d77ae360b2b27d2468e"); // On testnet we activate this rule 6 blocks after Blossom activation. From block 299188 and // prior to Blossom activation, the testnet minimum-difficulty threshold was 15 minutes (i.e. diff --git a/src/main.cpp b/src/main.cpp index 85914e54e..6bc809a72 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -4770,7 +4770,9 @@ bool RewindBlockIndex(const CChainParams& chainparams, bool& clearWitnessCaches) (networkID == "test" && nHeight == 252500 && *phashFirstInsufValidated == uint256S("0018bd16a9c6f15795a754c498d2b2083ab78f14dae44a66a8d0e90ba8464d9c")) || (networkID == "test" && nHeight == 584000 && *phashFirstInsufValidated == - uint256S("002e1d6daf4ab7b296e7df839dc1bee9d615583bb4bc34b1926ce78307532852")); + uint256S("002e1d6daf4ab7b296e7df839dc1bee9d615583bb4bc34b1926ce78307532852")) || + (networkID == "test" && nHeight == 903800 && *phashFirstInsufValidated == + uint256S("0044f3c696a242220ed608c70beba5aa804f1cfb8a20e32186727d1bec5dfa1e")); clearWitnessCaches = (rewindLength > MAX_REORG_LENGTH && intendedRewind); From d946b69cf824669bfde72fcd8add77619000d063 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Tue, 5 May 2020 14:59:22 +1200 Subject: [PATCH 2/3] txdb/chain: Restrict Heartwood chain consistency check to block index objects that were created by Heartwood-unaware clients. In the vicinity of a network upgrade, a zcashd node may receive headers for a non-upgrading chain from its non-upgraded peers (e.g. if the block at the NU activation height is found more quickly by the non-upgrading chain). In this situation, the node will end up with two headers at the NU activation height (and possibly for subsequent block heights). In the case of Heartwood, the block headers from the non-upgrading chain do not satisfy the Heartwood header consistency check in CBlockTreeDB::LoadBlockIndexGuts. In this commit, we restrict the Heartwood consistency checks to block index objects that were created by clients that are CHAIN_HISTORY_ROOT_VERSION or better. --- src/chain.h | 7 +++++++ src/txdb.cpp | 10 +++++++--- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/src/chain.h b/src/chain.h index 1a6c198f7..06052986b 100644 --- a/src/chain.h +++ b/src/chain.h @@ -435,6 +435,12 @@ class CDiskBlockIndex : public CBlockIndex public: uint256 hashPrev; + // This is the serialized `nVersion` of the block index, which is only set + // after the (de)serialization routine is called. This should only be used + // in LoadBlockIndexGuts (which is the only place where we read block index + // objects from disk anyway). + int nClientVersion = 0; + CDiskBlockIndex() { hashPrev = uint256(); } @@ -450,6 +456,7 @@ public: int nVersion = s.GetVersion(); if (!(s.GetType() & SER_GETHASH)) READWRITE(VARINT(nVersion)); + nClientVersion = nVersion; READWRITE(VARINT(nHeight)); READWRITE(VARINT(nStatus)); diff --git a/src/txdb.cpp b/src/txdb.cpp index 3a11ba123..de372b826 100644 --- a/src/txdb.cpp +++ b/src/txdb.cpp @@ -566,11 +566,15 @@ bool CBlockTreeDB::LoadBlockIndexGuts( return error("LoadBlockIndex(): CheckProofOfWork failed: %s", pindexNew->ToString()); // ZIP 221 consistency checks - if (chainParams.GetConsensus().NetworkUpgradeActive(pindexNew->nHeight, Consensus::UPGRADE_HEARTWOOD)) { + // We assume block index objects on disk that are not at least + // CHAIN_HISTORY_ROOT_VERSION were created by nodes that were + // not Heartwood aware. + if (diskindex.nClientVersion >= CHAIN_HISTORY_ROOT_VERSION && + chainParams.GetConsensus().NetworkUpgradeActive(pindexNew->nHeight, Consensus::UPGRADE_HEARTWOOD)) { if (pindexNew->hashLightClientRoot != pindexNew->hashChainHistoryRoot) { return error( - "LoadBlockIndex(): block index inconsistency detected (hashLightClientRoot != hashChainHistoryRoot): %s", - pindexNew->ToString()); + "LoadBlockIndex(): block index inconsistency detected (hashLightClientRoot %s != hashChainHistoryRoot %s): %s", + pindexNew->hashLightClientRoot.ToString(), pindexNew->hashChainHistoryRoot.ToString(), pindexNew->ToString()); } } else { if (pindexNew->hashLightClientRoot != pindexNew->hashFinalSaplingRoot) { From c3018904c31864a87459305bae4079c964f441aa Mon Sep 17 00:00:00 2001 From: Sean Bowe Date: Tue, 5 May 2020 17:24:27 -0600 Subject: [PATCH 3/3] Don't throw exception in PopHistoryNode when popping from empty tree. If we are doing an expected rollback that changes the consensus branch ID for some upgrade (or introduces one that wasn't present at the equivalent height) this will occur because `SelectHistoryCache` selects the tree for the new consensus branch ID, not the one that existed on the chain being rolled back. --- src/coins.cpp | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/coins.cpp b/src/coins.cpp index 9672118eb..2446b3c0b 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -506,9 +506,17 @@ void CCoinsViewCache::PopHistoryNode(uint32_t epochId) { switch (historyCache.length) { case 0: - // Caller is not expected to pop from empty tree! Caller should - // switch to previous epoch and pop history from there. - throw std::runtime_error("popping history node from empty history"); + // Caller is generally not expected to pop from empty tree! Caller + // should switch to previous epoch and pop history from there. + + // If we are doing an expected rollback that changes the consensus + // branch ID for some upgrade (or introduces one that wasn't present + // at the equivalent height) this will occur because + // `SelectHistoryCache` selects the tree for the new consensus + // branch ID, not the one that existed on the chain being rolled + // back. + + // Sensible action is to truncate the history cache: case 1: // Just resetting tree to empty historyCache.Truncate(0);