From 24db3297df203ca34877440632ea3d233a7f6717 Mon Sep 17 00:00:00 2001 From: Sean Bowe Date: Wed, 13 Mar 2019 00:43:54 -0600 Subject: [PATCH 01/12] (testnet) Fall back to hardcoded shielded pool balance to avoid reorgs. --- src/chainparams.cpp | 6 ++++++ src/chainparams.h | 6 ++++++ src/main.cpp | 41 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 53 insertions(+) diff --git a/src/chainparams.cpp b/src/chainparams.cpp index 1303e6681..7b8a5a7c3 100644 --- a/src/chainparams.cpp +++ b/src/chainparams.cpp @@ -353,6 +353,12 @@ public: 715 // total number of tx / (checkpoint block height / (24 * 24)) }; + // Hardcoded fallback value for the Sprout shielded value pool balance + // for nodes that have not reindexed since the introduction of monitoring + // in #2795. + nSproutValuePoolCheckpointHeight = 440329; + nSproutValuePoolCheckpointBalance = 40000029096803; + // Founders reward script expects a vector of 2-of-3 multisig addresses vFoundersRewardAddress = { "t2UNzUUx8mWBCRYPRezvA363EYXyEpHokyi", "t2N9PH9Wk9xjqYg9iin1Ua3aekJqfAtE543", "t2NGQjYMQhFndDHguvUw4wZdNdsssA6K7x2", "t2ENg7hHVqqs9JwU5cgjvSbxnT2a9USNfhy", diff --git a/src/chainparams.h b/src/chainparams.h index 2649fa9be..90a0712ae 100644 --- a/src/chainparams.h +++ b/src/chainparams.h @@ -70,6 +70,9 @@ public: const std::vector& AlertKey() const { return vAlertPubKey; } int GetDefaultPort() const { return nDefaultPort; } + CAmount SproutValuePoolCheckpointHeight() const { return nSproutValuePoolCheckpointHeight; } + CAmount SproutValuePoolCheckpointBalance() const { return nSproutValuePoolCheckpointBalance; } + const CBlock& GenesisBlock() const { return genesis; } /** Make miner wait to have peers to avoid wasting work */ bool MiningRequiresPeers() const { return fMiningRequiresPeers; } @@ -125,6 +128,9 @@ protected: bool fTestnetToBeDeprecatedFieldRPC = false; CCheckpointData checkpointData; std::vector vFoundersRewardAddress; + + CAmount nSproutValuePoolCheckpointHeight = 0; + CAmount nSproutValuePoolCheckpointBalance = 0; }; /** diff --git a/src/main.cpp b/src/main.cpp index e11dcbc3c..2aeb9fcfc 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -3296,9 +3296,34 @@ CBlockIndex* AddToBlockIndex(const CBlockHeader& block) return pindexNew; } +void FallbackSproutValuePoolBalance( + CBlockIndex *pindex, + const CChainParams& chainparams +) +{ + // Check if the height of this block matches the checkpoint + if (pindex->nHeight == chainparams.SproutValuePoolCheckpointHeight()) { + // Are we monitoring the Sprout pool? + if (!pindex->nChainSproutValue) { + // Apparently not. Introduce the hardcoded value. + pindex->nChainSproutValue = chainparams.SproutValuePoolCheckpointBalance(); + } else { + // Apparently we have been. So, we should expect the current + // value to match the hardcoded one. + assert(*pindex->nChainSproutValue == chainparams.SproutValuePoolCheckpointBalance()); + // And we should expect non-none for the delta stored in the block index here, + // or the checkpoint is too early. + assert(pindex->nSproutValue != boost::none); + } + } +} + /** Mark a block as having its data received and checked (up to BLOCK_VALID_TRANSACTIONS). */ bool ReceivedBlockTransactions(const CBlock &block, CValidationState& state, CBlockIndex *pindexNew, const CDiskBlockPos& pos) { + const CChainParams& chainparams = Params(); + bool this_is_testnet = chainparams.NetworkIDString() == "test"; + pindexNew->nTx = block.vtx.size(); pindexNew->nChainTx = 0; CAmount sproutValue = 0; @@ -3351,6 +3376,13 @@ bool ReceivedBlockTransactions(const CBlock &block, CValidationState& state, CBl pindex->nChainSproutValue = pindex->nSproutValue; pindex->nChainSaplingValue = pindex->nSaplingValue; } + + // Fall back to hardcoded Sprout value pool balance if we're on + // testnet. + if (this_is_testnet) { + FallbackSproutValuePoolBalance(pindex, chainparams); + } + { LOCK(cs_nBlockSequenceId); pindex->nSequenceId = nBlockSequenceId++; @@ -4003,6 +4035,8 @@ CBlockIndex * InsertBlockIndex(uint256 hash) bool static LoadBlockIndexDB() { const CChainParams& chainparams = Params(); + bool this_is_testnet = chainparams.NetworkIDString() == "test"; + if (!pblocktree->LoadBlockIndexGuts()) return false; @@ -4032,6 +4066,7 @@ bool static LoadBlockIndexDB() } else { pindex->nChainSproutValue = boost::none; } + if (pindex->pprev->nChainSaplingValue) { pindex->nChainSaplingValue = *pindex->pprev->nChainSaplingValue + pindex->nSaplingValue; } else { @@ -4048,6 +4083,12 @@ bool static LoadBlockIndexDB() pindex->nChainSproutValue = pindex->nSproutValue; pindex->nChainSaplingValue = pindex->nSaplingValue; } + + // Fall back to hardcoded Sprout value pool balance if we're on + // testnet. + if (this_is_testnet) { + FallbackSproutValuePoolBalance(pindex, chainparams); + } } // Construct in-memory chain of branch IDs. // Relies on invariant: a block that does not activate a network upgrade From 8a990a7d6441dcfa30933cf0cd5f10d5ca368441 Mon Sep 17 00:00:00 2001 From: Sean Bowe Date: Wed, 13 Mar 2019 00:54:29 -0600 Subject: [PATCH 02/12] (testnet) Reject blocks that result in turnstile violations --- src/main.cpp | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/src/main.cpp b/src/main.cpp index 2aeb9fcfc..26a8517ec 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -2451,6 +2451,32 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin return true; } + // If we're on testnet, reject a block that results in a negative + // shielded value pool balance. + if (Params().NetworkIDString() == "test") { + // Sprout + // + // We can expect nChainSproutValue to be valid after the hardcoded + // height, and this will be enforced on all descendant blocks. If + // the node was reindexed then this will be enforced for all blocks. + if (pindex->nChainSproutValue) { + if (*pindex->nChainSproutValue < 0) { + return state.DoS(100, error("ConnectBlock(): turnstile violation in Sprout shielded value pool"), + REJECT_INVALID, "turnstile-violation-sprout-shielded-pool"); + } + } + + // Sapling + // + // If we've reached ConnectBlock, we have all transactions of + // parents and can expect nChainSaplingValue not to be boost::none. + assert(pindex->nChainSaplingValue != boost::none); + if (*pindex->nChainSaplingValue < 0) { + return state.DoS(100, error("ConnectBlock(): turnstile violation in Sapling shielded value pool"), + REJECT_INVALID, "turnstile-violation-sapling-shielded-pool"); + } + } + // Do not allow blocks that contain transactions which 'overwrite' older transactions, // unless those are already completely spent. BOOST_FOREACH(const CTransaction& tx, block.vtx) { From bf4de896e7415fa3951faedfb0b1d3ec6203b2a8 Mon Sep 17 00:00:00 2001 From: Sean Bowe Date: Wed, 13 Mar 2019 01:13:59 -0600 Subject: [PATCH 03/12] (testnet/regtest) Avoid mining transactions that would violate the turnstile. --- src/miner.cpp | 46 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/src/miner.cpp b/src/miner.cpp index 5699e4ba5..dc211ebec 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -109,6 +109,10 @@ void UpdateTime(CBlockHeader* pblock, const Consensus::Params& consensusParams, CBlockTemplate* CreateNewBlock(const CScript& scriptPubKeyIn) { const CChainParams& chainparams = Params(); + bool this_is_testnet_or_regtest = + (chainparams.NetworkIDString() == "test") || + (chainparams.NetworkIDString() == "regtest"); + // Create new block std::unique_ptr pblocktemplate(new CBlockTemplate()); if(!pblocktemplate.get()) @@ -250,6 +254,28 @@ CBlockTemplate* CreateNewBlock(const CScript& scriptPubKeyIn) TxPriorityCompare comparer(fSortedByFee); std::make_heap(vecPriority.begin(), vecPriority.end(), comparer); + // We want to track the value pool, but if the miner gets + // invoked on an old block before the hardcoded fallback + // is active we don't want to trip up any assertions. So, + // we only adhere to the turnstile (as a miner) if we + // actually have all of the information necessary to do + // so. + CAmount sproutValue = 0; + CAmount saplingValue = 0; + bool monitoring_pool_balances = true; + if (this_is_testnet_or_regtest) { + if (pindexPrev->nChainSproutValue) { + sproutValue = *pindexPrev->nChainSproutValue; + } else { + monitoring_pool_balances = false; + } + if (pindexPrev->nChainSaplingValue) { + saplingValue = *pindexPrev->nChainSaplingValue; + } else { + monitoring_pool_balances = false; + } + } + while (!vecPriority.empty()) { // Take highest priority transaction off the priority queue: @@ -305,6 +331,26 @@ CBlockTemplate* CreateNewBlock(const CScript& scriptPubKeyIn) if (!ContextualCheckInputs(tx, state, view, true, MANDATORY_SCRIPT_VERIFY_FLAGS, true, txdata, Params().GetConsensus(), consensusBranchId)) continue; + if (this_is_testnet_or_regtest && monitoring_pool_balances) { + // Does this transaction lead to a turnstile violation? + + CAmount sproutValueDummy = sproutValue; + CAmount saplingValueDummy = saplingValue; + + sproutValueDummy += -tx.valueBalance; + + for (auto js : tx.vjoinsplit) { + sproutValueDummy += js.vpub_old; + sproutValueDummy -= js.vpub_new; + } + + if (sproutValueDummy < 0) continue; + if (saplingValueDummy < 0) continue; + + sproutValue = sproutValueDummy; + saplingValue = saplingValueDummy; + } + UpdateCoins(tx, view, nHeight); BOOST_FOREACH(const OutputDescription &outDescription, tx.vShieldedOutput) { From cb6df4b0ccb131ebcd88f1d8aea9e670dea07ba7 Mon Sep 17 00:00:00 2001 From: Sean Bowe Date: Thu, 14 Mar 2019 14:08:12 -0600 Subject: [PATCH 04/12] Fix tallying for Sprout/Sapling value pools. --- src/main.cpp | 1 - src/miner.cpp | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index 26a8517ec..3516deff6 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -4092,7 +4092,6 @@ bool static LoadBlockIndexDB() } else { pindex->nChainSproutValue = boost::none; } - if (pindex->pprev->nChainSaplingValue) { pindex->nChainSaplingValue = *pindex->pprev->nChainSaplingValue + pindex->nSaplingValue; } else { diff --git a/src/miner.cpp b/src/miner.cpp index dc211ebec..f45582ba2 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -337,7 +337,7 @@ CBlockTemplate* CreateNewBlock(const CScript& scriptPubKeyIn) CAmount sproutValueDummy = sproutValue; CAmount saplingValueDummy = saplingValue; - sproutValueDummy += -tx.valueBalance; + saplingValueDummy += -tx.valueBalance; for (auto js : tx.vjoinsplit) { sproutValueDummy += js.vpub_old; From 2b1252af8046350cbeda213564d600c3b6786896 Mon Sep 17 00:00:00 2001 From: Sean Bowe Date: Thu, 14 Mar 2019 14:20:51 -0600 Subject: [PATCH 05/12] Consolidate logic to enable turnstile auditing for testnet/regtest/mainnet. --- src/chainparams.cpp | 5 +++++ src/chainparams.h | 2 ++ src/main.cpp | 27 ++++++++++++--------------- src/miner.cpp | 17 ++++++++++------- 4 files changed, 29 insertions(+), 22 deletions(-) diff --git a/src/chainparams.cpp b/src/chainparams.cpp index 7b8a5a7c3..582c7b6f9 100644 --- a/src/chainparams.cpp +++ b/src/chainparams.cpp @@ -358,6 +358,7 @@ public: // in #2795. nSproutValuePoolCheckpointHeight = 440329; nSproutValuePoolCheckpointBalance = 40000029096803; + fSproutValuePoolCheckpointEnabled = true; // Founders reward script expects a vector of 2-of-3 multisig addresses vFoundersRewardAddress = { @@ -472,6 +473,10 @@ public: // Founders reward script expects a vector of 2-of-3 multisig addresses vFoundersRewardAddress = { "t2FwcEhFdNXuFMv1tcYwaBJtYVtMj8b1uTg" }; assert(vFoundersRewardAddress.size() <= consensus.GetLastFoundersRewardBlockHeight()); + + // Enable Sprout shielded value pool checkpointing on + // regtest. + fSproutValuePoolCheckpointEnabled = true; } void UpdateNetworkUpgradeParameters(Consensus::UpgradeIndex idx, int nActivationHeight) diff --git a/src/chainparams.h b/src/chainparams.h index 90a0712ae..c9c1284d3 100644 --- a/src/chainparams.h +++ b/src/chainparams.h @@ -72,6 +72,7 @@ public: CAmount SproutValuePoolCheckpointHeight() const { return nSproutValuePoolCheckpointHeight; } CAmount SproutValuePoolCheckpointBalance() const { return nSproutValuePoolCheckpointBalance; } + bool SproutValuePoolCheckpointEnabled() const { return fSproutValuePoolCheckpointEnabled; } const CBlock& GenesisBlock() const { return genesis; } /** Make miner wait to have peers to avoid wasting work */ @@ -131,6 +132,7 @@ protected: CAmount nSproutValuePoolCheckpointHeight = 0; CAmount nSproutValuePoolCheckpointBalance = 0; + bool fSproutValuePoolCheckpointEnabled = false; }; /** diff --git a/src/main.cpp b/src/main.cpp index 3516deff6..40291aab1 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -2451,9 +2451,8 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin return true; } - // If we're on testnet, reject a block that results in a negative - // shielded value pool balance. - if (Params().NetworkIDString() == "test") { + // Reject a block that results in a negative shielded value pool balance. + if (Params().SproutValuePoolCheckpointEnabled()) { // Sprout // // We can expect nChainSproutValue to be valid after the hardcoded @@ -3327,6 +3326,12 @@ void FallbackSproutValuePoolBalance( const CChainParams& chainparams ) { + // We might not want to enable the checkpointing for mainnet + // yet. + if (!chainparams.SproutValuePoolCheckpointEnabled()) { + return; + } + // Check if the height of this block matches the checkpoint if (pindex->nHeight == chainparams.SproutValuePoolCheckpointHeight()) { // Are we monitoring the Sprout pool? @@ -3348,7 +3353,6 @@ void FallbackSproutValuePoolBalance( bool ReceivedBlockTransactions(const CBlock &block, CValidationState& state, CBlockIndex *pindexNew, const CDiskBlockPos& pos) { const CChainParams& chainparams = Params(); - bool this_is_testnet = chainparams.NetworkIDString() == "test"; pindexNew->nTx = block.vtx.size(); pindexNew->nChainTx = 0; @@ -3403,11 +3407,8 @@ bool ReceivedBlockTransactions(const CBlock &block, CValidationState& state, CBl pindex->nChainSaplingValue = pindex->nSaplingValue; } - // Fall back to hardcoded Sprout value pool balance if we're on - // testnet. - if (this_is_testnet) { - FallbackSproutValuePoolBalance(pindex, chainparams); - } + // Fall back to hardcoded Sprout value pool balance + FallbackSproutValuePoolBalance(pindex, chainparams); { LOCK(cs_nBlockSequenceId); @@ -4061,7 +4062,6 @@ CBlockIndex * InsertBlockIndex(uint256 hash) bool static LoadBlockIndexDB() { const CChainParams& chainparams = Params(); - bool this_is_testnet = chainparams.NetworkIDString() == "test"; if (!pblocktree->LoadBlockIndexGuts()) return false; @@ -4109,11 +4109,8 @@ bool static LoadBlockIndexDB() pindex->nChainSaplingValue = pindex->nSaplingValue; } - // Fall back to hardcoded Sprout value pool balance if we're on - // testnet. - if (this_is_testnet) { - FallbackSproutValuePoolBalance(pindex, chainparams); - } + // Fall back to hardcoded Sprout value pool balance + FallbackSproutValuePoolBalance(pindex, chainparams); } // Construct in-memory chain of branch IDs. // Relies on invariant: a block that does not activate a network upgrade diff --git a/src/miner.cpp b/src/miner.cpp index f45582ba2..ec5a198ce 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -109,9 +109,6 @@ void UpdateTime(CBlockHeader* pblock, const Consensus::Params& consensusParams, CBlockTemplate* CreateNewBlock(const CScript& scriptPubKeyIn) { const CChainParams& chainparams = Params(); - bool this_is_testnet_or_regtest = - (chainparams.NetworkIDString() == "test") || - (chainparams.NetworkIDString() == "regtest"); // Create new block std::unique_ptr pblocktemplate(new CBlockTemplate()); @@ -263,7 +260,7 @@ CBlockTemplate* CreateNewBlock(const CScript& scriptPubKeyIn) CAmount sproutValue = 0; CAmount saplingValue = 0; bool monitoring_pool_balances = true; - if (this_is_testnet_or_regtest) { + if (chainparams.SproutValuePoolCheckpointEnabled()) { if (pindexPrev->nChainSproutValue) { sproutValue = *pindexPrev->nChainSproutValue; } else { @@ -331,7 +328,7 @@ CBlockTemplate* CreateNewBlock(const CScript& scriptPubKeyIn) if (!ContextualCheckInputs(tx, state, view, true, MANDATORY_SCRIPT_VERIFY_FLAGS, true, txdata, Params().GetConsensus(), consensusBranchId)) continue; - if (this_is_testnet_or_regtest && monitoring_pool_balances) { + if (chainparams.SproutValuePoolCheckpointEnabled() && monitoring_pool_balances) { // Does this transaction lead to a turnstile violation? CAmount sproutValueDummy = sproutValue; @@ -344,8 +341,14 @@ CBlockTemplate* CreateNewBlock(const CScript& scriptPubKeyIn) sproutValueDummy -= js.vpub_new; } - if (sproutValueDummy < 0) continue; - if (saplingValueDummy < 0) continue; + if (sproutValueDummy < 0) { + LogPrintf("CreateNewBlock(): tx %s appears to violate Sprout turnstile", tx.GetHash().ToString()); + continue; + } + if (saplingValueDummy < 0) { + LogPrintf("CreateNewBlock(): tx %s appears to violate Sapling turnstile", tx.GetHash().ToString()); + continue; + } sproutValue = sproutValueDummy; saplingValue = saplingValueDummy; From 831725a6711b6fb950c80bd6b4e3e4b90e5b42ab Mon Sep 17 00:00:00 2001 From: Sean Bowe Date: Thu, 14 Mar 2019 15:25:10 -0600 Subject: [PATCH 06/12] Use existing chainparams variable --- src/main.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main.cpp b/src/main.cpp index 40291aab1..2005a74ea 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -2452,7 +2452,7 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin } // Reject a block that results in a negative shielded value pool balance. - if (Params().SproutValuePoolCheckpointEnabled()) { + if (chainparams.SproutValuePoolCheckpointEnabled()) { // Sprout // // We can expect nChainSproutValue to be valid after the hardcoded From ebe2edce9a72450b9b889871c085bf12d8591258 Mon Sep 17 00:00:00 2001 From: Sean Bowe Date: Thu, 14 Mar 2019 15:25:31 -0600 Subject: [PATCH 07/12] Add newlines to turntile log messages for miner --- src/miner.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/miner.cpp b/src/miner.cpp index ec5a198ce..53e169c61 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -342,11 +342,11 @@ CBlockTemplate* CreateNewBlock(const CScript& scriptPubKeyIn) } if (sproutValueDummy < 0) { - LogPrintf("CreateNewBlock(): tx %s appears to violate Sprout turnstile", tx.GetHash().ToString()); + LogPrintf("CreateNewBlock(): tx %s appears to violate Sprout turnstile\n", tx.GetHash().ToString()); continue; } if (saplingValueDummy < 0) { - LogPrintf("CreateNewBlock(): tx %s appears to violate Sapling turnstile", tx.GetHash().ToString()); + LogPrintf("CreateNewBlock(): tx %s appears to violate Sapling turnstile\n", tx.GetHash().ToString()); continue; } From 6482b661abe58b287c548debda33a923826867fd Mon Sep 17 00:00:00 2001 From: Sean Bowe Date: Thu, 14 Mar 2019 15:39:11 -0600 Subject: [PATCH 08/12] Check blockhash of fallback block for Sprout value pool balance --- src/chainparams.cpp | 4 ++++ src/chainparams.h | 2 ++ src/main.cpp | 28 ++++++++++++++++++---------- 3 files changed, 24 insertions(+), 10 deletions(-) diff --git a/src/chainparams.cpp b/src/chainparams.cpp index 582c7b6f9..5a02f87be 100644 --- a/src/chainparams.cpp +++ b/src/chainparams.cpp @@ -359,6 +359,7 @@ public: nSproutValuePoolCheckpointHeight = 440329; nSproutValuePoolCheckpointBalance = 40000029096803; fSproutValuePoolCheckpointEnabled = true; + hashSproutValuePoolCheckpointBlock = uint256S("000a95d08ba5dcbabe881fc6471d11807bcca7df5f1795c99f3ec4580db4279b"); // Founders reward script expects a vector of 2-of-3 multisig addresses vFoundersRewardAddress = { @@ -476,7 +477,10 @@ public: // Enable Sprout shielded value pool checkpointing on // regtest. + nSproutValuePoolCheckpointHeight = 0; + nSproutValuePoolCheckpointBalance = 0; fSproutValuePoolCheckpointEnabled = true; + hashSproutValuePoolCheckpointBlock = consensus.hashGenesisBlock; } void UpdateNetworkUpgradeParameters(Consensus::UpgradeIndex idx, int nActivationHeight) diff --git a/src/chainparams.h b/src/chainparams.h index c9c1284d3..13e374536 100644 --- a/src/chainparams.h +++ b/src/chainparams.h @@ -72,6 +72,7 @@ public: CAmount SproutValuePoolCheckpointHeight() const { return nSproutValuePoolCheckpointHeight; } CAmount SproutValuePoolCheckpointBalance() const { return nSproutValuePoolCheckpointBalance; } + uint256 SproutValuePoolCheckpointBlockHash() const { return hashSproutValuePoolCheckpointBlock; } bool SproutValuePoolCheckpointEnabled() const { return fSproutValuePoolCheckpointEnabled; } const CBlock& GenesisBlock() const { return genesis; } @@ -132,6 +133,7 @@ protected: CAmount nSproutValuePoolCheckpointHeight = 0; CAmount nSproutValuePoolCheckpointBalance = 0; + uint256 hashSproutValuePoolCheckpointBlock; bool fSproutValuePoolCheckpointEnabled = false; }; diff --git a/src/main.cpp b/src/main.cpp index 2005a74ea..ae9e83536 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -3334,17 +3334,25 @@ void FallbackSproutValuePoolBalance( // Check if the height of this block matches the checkpoint if (pindex->nHeight == chainparams.SproutValuePoolCheckpointHeight()) { - // Are we monitoring the Sprout pool? - if (!pindex->nChainSproutValue) { - // Apparently not. Introduce the hardcoded value. - pindex->nChainSproutValue = chainparams.SproutValuePoolCheckpointBalance(); + if (pindex->GetBlockHash() == chainparams.SproutValuePoolCheckpointBlockHash()) { + // Are we monitoring the Sprout pool? + if (!pindex->nChainSproutValue) { + // Apparently not. Introduce the hardcoded value so we monitor for + // this point onwards (assuming the checkpoint is late enough) + pindex->nChainSproutValue = chainparams.SproutValuePoolCheckpointBalance(); + } else { + // Apparently we have been. So, we should expect the current + // value to match the hardcoded one. + assert(*pindex->nChainSproutValue == chainparams.SproutValuePoolCheckpointBalance()); + // And we should expect non-none for the delta stored in the block index here, + // or the checkpoint is too early. + assert(pindex->nSproutValue != boost::none); + } } else { - // Apparently we have been. So, we should expect the current - // value to match the hardcoded one. - assert(*pindex->nChainSproutValue == chainparams.SproutValuePoolCheckpointBalance()); - // And we should expect non-none for the delta stored in the block index here, - // or the checkpoint is too early. - assert(pindex->nSproutValue != boost::none); + LogPrintf( + "FallbackSproutValuePoolBalance(): fallback block hash is incorrect, we got %s\n", + pindex->GetBlockHash().ToString() + ); } } } From b5c7e63bcdab317bc36370e1ec3acf699c263fc0 Mon Sep 17 00:00:00 2001 From: Sean Bowe Date: Mon, 18 Mar 2019 11:32:26 -0600 Subject: [PATCH 09/12] Change SproutValuePoolCheckpointEnabled to ZIP209Activated --- src/chainparams.cpp | 4 ++-- src/chainparams.h | 4 ++-- src/main.cpp | 4 ++-- src/miner.cpp | 5 ++--- 4 files changed, 8 insertions(+), 9 deletions(-) diff --git a/src/chainparams.cpp b/src/chainparams.cpp index 5a02f87be..8a45521b2 100644 --- a/src/chainparams.cpp +++ b/src/chainparams.cpp @@ -358,7 +358,7 @@ public: // in #2795. nSproutValuePoolCheckpointHeight = 440329; nSproutValuePoolCheckpointBalance = 40000029096803; - fSproutValuePoolCheckpointEnabled = true; + fZIP209Enabled = true; hashSproutValuePoolCheckpointBlock = uint256S("000a95d08ba5dcbabe881fc6471d11807bcca7df5f1795c99f3ec4580db4279b"); // Founders reward script expects a vector of 2-of-3 multisig addresses @@ -479,7 +479,7 @@ public: // regtest. nSproutValuePoolCheckpointHeight = 0; nSproutValuePoolCheckpointBalance = 0; - fSproutValuePoolCheckpointEnabled = true; + fZIP209Enabled = true; hashSproutValuePoolCheckpointBlock = consensus.hashGenesisBlock; } diff --git a/src/chainparams.h b/src/chainparams.h index 13e374536..ce5c1d6a1 100644 --- a/src/chainparams.h +++ b/src/chainparams.h @@ -73,7 +73,7 @@ public: CAmount SproutValuePoolCheckpointHeight() const { return nSproutValuePoolCheckpointHeight; } CAmount SproutValuePoolCheckpointBalance() const { return nSproutValuePoolCheckpointBalance; } uint256 SproutValuePoolCheckpointBlockHash() const { return hashSproutValuePoolCheckpointBlock; } - bool SproutValuePoolCheckpointEnabled() const { return fSproutValuePoolCheckpointEnabled; } + bool ZIP209Enabled() const { return fZIP209Enabled; } const CBlock& GenesisBlock() const { return genesis; } /** Make miner wait to have peers to avoid wasting work */ @@ -134,7 +134,7 @@ protected: CAmount nSproutValuePoolCheckpointHeight = 0; CAmount nSproutValuePoolCheckpointBalance = 0; uint256 hashSproutValuePoolCheckpointBlock; - bool fSproutValuePoolCheckpointEnabled = false; + bool fZIP209Enabled = false; }; /** diff --git a/src/main.cpp b/src/main.cpp index ae9e83536..50687866c 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -2452,7 +2452,7 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin } // Reject a block that results in a negative shielded value pool balance. - if (chainparams.SproutValuePoolCheckpointEnabled()) { + if (chainparams.ZIP209Enabled()) { // Sprout // // We can expect nChainSproutValue to be valid after the hardcoded @@ -3328,7 +3328,7 @@ void FallbackSproutValuePoolBalance( { // We might not want to enable the checkpointing for mainnet // yet. - if (!chainparams.SproutValuePoolCheckpointEnabled()) { + if (!chainparams.ZIP209Enabled()) { return; } diff --git a/src/miner.cpp b/src/miner.cpp index 53e169c61..f83acfe49 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -109,7 +109,6 @@ void UpdateTime(CBlockHeader* pblock, const Consensus::Params& consensusParams, CBlockTemplate* CreateNewBlock(const CScript& scriptPubKeyIn) { const CChainParams& chainparams = Params(); - // Create new block std::unique_ptr pblocktemplate(new CBlockTemplate()); if(!pblocktemplate.get()) @@ -260,7 +259,7 @@ CBlockTemplate* CreateNewBlock(const CScript& scriptPubKeyIn) CAmount sproutValue = 0; CAmount saplingValue = 0; bool monitoring_pool_balances = true; - if (chainparams.SproutValuePoolCheckpointEnabled()) { + if (chainparams.ZIP209Enabled()) { if (pindexPrev->nChainSproutValue) { sproutValue = *pindexPrev->nChainSproutValue; } else { @@ -328,7 +327,7 @@ CBlockTemplate* CreateNewBlock(const CScript& scriptPubKeyIn) if (!ContextualCheckInputs(tx, state, view, true, MANDATORY_SCRIPT_VERIFY_FLAGS, true, txdata, Params().GetConsensus(), consensusBranchId)) continue; - if (chainparams.SproutValuePoolCheckpointEnabled() && monitoring_pool_balances) { + if (chainparams.ZIP209Enabled() && monitoring_pool_balances) { // Does this transaction lead to a turnstile violation? CAmount sproutValueDummy = sproutValue; From 30a5d6f520c9661d3b1da9901b71bf235cc9b25e Mon Sep 17 00:00:00 2001 From: Sean Bowe Date: Mon, 18 Mar 2019 14:09:32 -0600 Subject: [PATCH 10/12] Only enforce Sapling turnstile if balance values have been populated. --- src/main.cpp | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index 50687866c..be82258d2 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -2469,10 +2469,14 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin // // If we've reached ConnectBlock, we have all transactions of // parents and can expect nChainSaplingValue not to be boost::none. - assert(pindex->nChainSaplingValue != boost::none); - if (*pindex->nChainSaplingValue < 0) { - return state.DoS(100, error("ConnectBlock(): turnstile violation in Sapling shielded value pool"), - REJECT_INVALID, "turnstile-violation-sapling-shielded-pool"); + // However, the miner and mining RPCs may not have populated this + // value and will call `TestBlockValidity`. So, we act + // conditionally. + if (pindex->nChainSaplingValue) { + if (*pindex->nChainSaplingValue < 0) { + return state.DoS(100, error("ConnectBlock(): turnstile violation in Sapling shielded value pool"), + REJECT_INVALID, "turnstile-violation-sapling-shielded-pool"); + } } } From 4e3dca978bb4cdcfffc0e3ad7fa2f187cab00b79 Mon Sep 17 00:00:00 2001 From: Sean Bowe Date: Tue, 19 Mar 2019 12:44:53 -0600 Subject: [PATCH 11/12] Do not enable ZIP209 on regtest right now. --- src/chainparams.cpp | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/chainparams.cpp b/src/chainparams.cpp index 8a45521b2..4b1e391c2 100644 --- a/src/chainparams.cpp +++ b/src/chainparams.cpp @@ -474,13 +474,6 @@ public: // Founders reward script expects a vector of 2-of-3 multisig addresses vFoundersRewardAddress = { "t2FwcEhFdNXuFMv1tcYwaBJtYVtMj8b1uTg" }; assert(vFoundersRewardAddress.size() <= consensus.GetLastFoundersRewardBlockHeight()); - - // Enable Sprout shielded value pool checkpointing on - // regtest. - nSproutValuePoolCheckpointHeight = 0; - nSproutValuePoolCheckpointBalance = 0; - fZIP209Enabled = true; - hashSproutValuePoolCheckpointBlock = consensus.hashGenesisBlock; } void UpdateNetworkUpgradeParameters(Consensus::UpgradeIndex idx, int nActivationHeight) From 8d0e2befe712754b32b0f0eef54707916b0fa557 Mon Sep 17 00:00:00 2001 From: Sean Bowe Date: Tue, 19 Mar 2019 12:45:21 -0600 Subject: [PATCH 12/12] (minor) Remove added newline. --- src/main.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main.cpp b/src/main.cpp index be82258d2..d0d8db976 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -3365,7 +3365,6 @@ void FallbackSproutValuePoolBalance( bool ReceivedBlockTransactions(const CBlock &block, CValidationState& state, CBlockIndex *pindexNew, const CDiskBlockPos& pos) { const CChainParams& chainparams = Params(); - pindexNew->nTx = block.vtx.size(); pindexNew->nChainTx = 0; CAmount sproutValue = 0;