From b47f6867540f65ebbe655085a50361cdc35cb429 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Thu, 20 Aug 2020 13:14:44 +0100 Subject: [PATCH 1/5] consensus: Add assertions for Params::HalvingHeight parameters - Height must be non-negative. - Halving index must be positive. --- src/consensus/params.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/consensus/params.cpp b/src/consensus/params.cpp index 48ff3909a..a6525f4b7 100644 --- a/src/consensus/params.cpp +++ b/src/consensus/params.cpp @@ -47,6 +47,9 @@ namespace Consensus { * first halving. */ int Params::HalvingHeight(int nHeight, int halvingIndex) const { + assert(nHeight >= 0); + assert(halvingIndex > 0); + // zip208 // HalvingHeight(i) := max({ height ⦂ N | Halving(height) < i }) + 1 // From bfeaa0e4c09f411694e12477a48a2eb349fd628f Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Thu, 20 Aug 2020 13:19:25 +0100 Subject: [PATCH 2/5] consensus: Document the empty conditional branch in ContextualCheckBlock It exists to implement a ZIP 207 consensus rule that turns off the Founders' Reward once Canopy activates. --- src/main.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/main.cpp b/src/main.cpp index 21800b51e..247f8f056 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -4181,6 +4181,10 @@ bool ContextualCheckBlock( if (consensusParams.NetworkUpgradeActive(nHeight, Consensus::UPGRADE_CANOPY)) { // Funding streams are checked inside ContextualCheckTransaction. + // This empty conditional branch exists to enforce this ZIP 207 consensus rule: + // + // Once the Canopy network upgrade activates, the existing consensus rule for + // payment of the Founders' Reward is no longer active. } else if ((nHeight > 0) && (nHeight <= consensusParams.GetLastFoundersRewardBlockHeight(nHeight))) { // Coinbase transaction must include an output sending 20% of // the block subsidy to a Founders' Reward script, until the last Founders' From 768534a8b9a9d64ebfa0c44b7d6dcf96566ac3bf Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Thu, 20 Aug 2020 14:48:01 +0100 Subject: [PATCH 3/5] consensus: Statically check funding stream numerators and denominators --- src/consensus/funding.cpp | 12 +++++++++++- src/consensus/funding.h | 4 ++-- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/consensus/funding.cpp b/src/consensus/funding.cpp index 79dffb631..7f622f843 100644 --- a/src/consensus/funding.cpp +++ b/src/consensus/funding.cpp @@ -10,7 +10,7 @@ namespace Consensus * General information about each funding stream. * Ordered by Consensus::FundingStreamIndex. */ -const struct FSInfo FundingStreamInfo[Consensus::MAX_FUNDING_STREAMS] = { +constexpr struct FSInfo FundingStreamInfo[Consensus::MAX_FUNDING_STREAMS] = { { .recipient = "Electric Coin Company", .specification = "https://zips.z.cash/zip-0214", @@ -31,6 +31,16 @@ const struct FSInfo FundingStreamInfo[Consensus::MAX_FUNDING_STREAMS] = { } }; +static constexpr bool validateFundingStreamInfo(uint32_t idx) { + return (idx >= Consensus::MAX_FUNDING_STREAMS || ( + FundingStreamInfo[idx].valueNumerator < FundingStreamInfo[idx].valueDenominator && + FundingStreamInfo[idx].valueNumerator < (INT64_MAX / MAX_MONEY) && + validateFundingStreamInfo(idx + 1))); +} +static_assert( + validateFundingStreamInfo(Consensus::FIRST_FUNDING_STREAM), + "Invalid FundingStreamInfo"); + CAmount FSInfo::Value(CAmount blockSubsidy) const { // Integer division is floor division for nonnegative integers in C++ diff --git a/src/consensus/funding.h b/src/consensus/funding.h index 5f7b96acc..2a6f7b512 100644 --- a/src/consensus/funding.h +++ b/src/consensus/funding.h @@ -14,8 +14,8 @@ namespace Consensus { struct FSInfo { - std::string recipient; - std::string specification; + const char* recipient; + const char* specification; uint64_t valueNumerator; uint64_t valueDenominator; From c59505688373ba876b3797327d3b27be22fef45c Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Thu, 20 Aug 2020 17:46:57 +0100 Subject: [PATCH 4/5] consensus: Clearly gate active funding stream elements on Canopy We only use the output of GetActiveFundingStreamElements and GetActiveFundingStreams within Canopy contexts, but this makes it explicit that funding streams are disabled before Canopy activation. --- src/consensus/funding.cpp | 36 +++++++++++++++++++++++------------- src/consensus/funding.h | 6 ++++++ 2 files changed, 29 insertions(+), 13 deletions(-) diff --git a/src/consensus/funding.cpp b/src/consensus/funding.cpp index 7f622f843..5efaf919b 100644 --- a/src/consensus/funding.cpp +++ b/src/consensus/funding.cpp @@ -53,17 +53,22 @@ std::set GetActiveFundingStreamElements( const Consensus::Params& params) { std::set> requiredElements; - for (uint32_t idx = Consensus::FIRST_FUNDING_STREAM; idx < Consensus::MAX_FUNDING_STREAMS; idx++) { - // The following indexed access is safe as Consensus::MAX_FUNDING_STREAMS is used - // in the definition of vFundingStreams. - auto fs = params.vFundingStreams[idx]; - // Funding period is [startHeight, endHeight) - if (fs && nHeight >= fs.get().GetStartHeight() && nHeight < fs.get().GetEndHeight()) { - requiredElements.insert(std::make_pair( - fs.get().RecipientAddress(params, nHeight), - FundingStreamInfo[idx].Value(blockSubsidy))); + + // Funding streams are disabled if height < CanopyActivationHeight + if (params.NetworkUpgradeActive(nHeight, Consensus::UPGRADE_CANOPY)) { + for (uint32_t idx = Consensus::FIRST_FUNDING_STREAM; idx < Consensus::MAX_FUNDING_STREAMS; idx++) { + // The following indexed access is safe as Consensus::MAX_FUNDING_STREAMS is used + // in the definition of vFundingStreams. + auto fs = params.vFundingStreams[idx]; + // Funding period is [startHeight, endHeight) + if (fs && nHeight >= fs.get().GetStartHeight() && nHeight < fs.get().GetEndHeight()) { + requiredElements.insert(std::make_pair( + fs.get().RecipientAddress(params, nHeight), + FundingStreamInfo[idx].Value(blockSubsidy))); + } } } + return requiredElements; }; @@ -72,12 +77,17 @@ std::vector GetActiveFundingStreams( const Consensus::Params& params) { std::vector activeStreams; - for (uint32_t idx = Consensus::FIRST_FUNDING_STREAM; idx < Consensus::MAX_FUNDING_STREAMS; idx++) { - auto fs = params.vFundingStreams[idx]; - if (fs && nHeight >= fs.get().GetStartHeight() && nHeight < fs.get().GetEndHeight()) { - activeStreams.push_back(FundingStreamInfo[idx]); + + // Funding streams are disabled if height < CanopyActivationHeight + if (params.NetworkUpgradeActive(nHeight, Consensus::UPGRADE_CANOPY)) { + for (uint32_t idx = Consensus::FIRST_FUNDING_STREAM; idx < Consensus::MAX_FUNDING_STREAMS; idx++) { + auto fs = params.vFundingStreams[idx]; + if (fs && nHeight >= fs.get().GetStartHeight() && nHeight < fs.get().GetEndHeight()) { + activeStreams.push_back(FundingStreamInfo[idx]); + } } } + return activeStreams; }; diff --git a/src/consensus/funding.h b/src/consensus/funding.h index 2a6f7b512..b2dc1cd6c 100644 --- a/src/consensus/funding.h +++ b/src/consensus/funding.h @@ -19,6 +19,12 @@ struct FSInfo { uint64_t valueNumerator; uint64_t valueDenominator; + /** + * Returns the inherent value of this funding stream. + * + * For the active funding streams at a given height, use + * GetActiveFundingStreams() or GetActiveFundingStreamElements(). + */ CAmount Value(CAmount blockSubsidy) const; }; From 732f1a76e4d12c39b794b9b5b53ec248dd8185ab Mon Sep 17 00:00:00 2001 From: str4d Date: Sat, 22 Aug 2020 03:47:12 +1200 Subject: [PATCH 5/5] Adjust GetActiveFundingStream* comments Co-authored-by: Daira Hopwood --- src/consensus/funding.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/consensus/funding.cpp b/src/consensus/funding.cpp index 5efaf919b..918deae8d 100644 --- a/src/consensus/funding.cpp +++ b/src/consensus/funding.cpp @@ -54,7 +54,7 @@ std::set GetActiveFundingStreamElements( { std::set> requiredElements; - // Funding streams are disabled if height < CanopyActivationHeight + // Funding streams are disabled if Canopy is not active. if (params.NetworkUpgradeActive(nHeight, Consensus::UPGRADE_CANOPY)) { for (uint32_t idx = Consensus::FIRST_FUNDING_STREAM; idx < Consensus::MAX_FUNDING_STREAMS; idx++) { // The following indexed access is safe as Consensus::MAX_FUNDING_STREAMS is used @@ -78,7 +78,7 @@ std::vector GetActiveFundingStreams( { std::vector activeStreams; - // Funding streams are disabled if height < CanopyActivationHeight + // Funding streams are disabled if Canopy is not active. if (params.NetworkUpgradeActive(nHeight, Consensus::UPGRADE_CANOPY)) { for (uint32_t idx = Consensus::FIRST_FUNDING_STREAM; idx < Consensus::MAX_FUNDING_STREAMS; idx++) { auto fs = params.vFundingStreams[idx];