Skip to content

Commit

Permalink
fix(docs): Fix NU6-related documentation (#8949)
Browse files Browse the repository at this point in the history
* replaces some potential panics with error in block verifier checks

* fixes outstanding documentation issues listed as examples in audit report

* fixes remaining TODOs in `parameters::network::subsidy` module.

* Addresses other TODOs added during NU6 implementation, fixes a TODO in `subsidy_is_valid()` about using the network slow start interval parameters when PoW is disabled.

* updates snapshot
  • Loading branch information
arya2 authored Oct 22, 2024
1 parent b1ffc89 commit 46c6b6e
Show file tree
Hide file tree
Showing 9 changed files with 47 additions and 157 deletions.
33 changes: 13 additions & 20 deletions zebra-chain/src/parameters/network/subsidy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,18 +65,18 @@ pub enum FundingStreamReceiver {

/// The Major Grants (Zcash Community Grants) funding stream.
MajorGrants,
/// The deferred pool contribution.
// TODO: Add link to lockbox stream ZIP

/// The deferred pool contribution, see [ZIP-1015](https://zips.z.cash/zip-1015) for more details.
Deferred,
}

impl FundingStreamReceiver {
/// Returns a human-readable name and a specification URL for the receiver, as described in
/// [ZIP-1014] and [`zcashd`] before NU6. After NU6, the specification is in the [ZIP-lockbox].
/// [ZIP-1014] and [`zcashd`] before NU6. After NU6, the specification is in the [ZIP-1015].
///
/// [ZIP-1014]: https://zips.z.cash/zip-1014#abstract
/// [`zcashd`]: https://github.com/zcash/zcash/blob/3f09cfa00a3c90336580a127e0096d99e25a38d6/src/consensus/funding.cpp#L13-L32
/// [ZIP-lockbox]: https://zips.z.cash/draft-nuttycom-funding-allocation#alternative-2-hybrid-deferred-dev-fund-transitioning-to-a-non-direct-funding-model
/// [ZIP-1015]: https://zips.z.cash/zip-1015
pub fn info(&self, is_nu6: bool) -> (&'static str, &'static str) {
if is_nu6 {
(
Expand Down Expand Up @@ -112,10 +112,10 @@ pub const FUNDING_STREAM_RECEIVER_DENOMINATOR: u64 = 100;
/// [ZIP-214]: https://zips.z.cash/zip-0214
pub const FUNDING_STREAM_SPECIFICATION: &str = "https://zips.z.cash/zip-0214";

/// The specification for post-NU6 funding stream and lockbox receivers, a URL that links to [ZIP-lockbox].
/// The specification for post-NU6 funding stream and lockbox receivers, a URL that links to [ZIP-1015].
///
/// [ZIP-lockbox]: https://zips.z.cash/draft-nuttycom-funding-allocation#alternative-2-hybrid-deferred-dev-fund-transitioning-to-a-non-direct-funding-model
pub const LOCKBOX_SPECIFICATION: &str = "https://zips.z.cash/draft-nuttycom-funding-allocation#alternative-2-hybrid-deferred-dev-fund-transitioning-to-a-non-direct-funding-model";
/// [ZIP-1015]: https://zips.z.cash/zip-1015
pub const LOCKBOX_SPECIFICATION: &str = "https://zips.z.cash/zip-1015";

/// Funding stream recipients and height ranges.
#[derive(Deserialize, Clone, Debug, Eq, PartialEq)]
Expand Down Expand Up @@ -225,10 +225,8 @@ lazy_static! {
.collect(),
};

/// The post-NU6 funding streams for Mainnet
// TODO: Add a reference to lockbox stream ZIP, this is currently based on https://zips.z.cash/draft-nuttycom-funding-allocation
/// The post-NU6 funding streams for Mainnet as described in [ZIP-1015](https://zips.z.cash/zip-1015).
pub static ref POST_NU6_FUNDING_STREAMS_MAINNET: FundingStreams = FundingStreams {
// TODO: Adjust this height range and recipient list once a proposal is selected
height_range: POST_NU6_FUNDING_STREAM_START_RANGE_MAINNET,
recipients: [
(
Expand Down Expand Up @@ -266,11 +264,8 @@ lazy_static! {
.collect(),
};

/// The post-NU6 funding streams for Testnet
// TODO: Add a reference to lockbox stream ZIP, this is currently based on the number of blocks between the
// start and end heights for Mainnet in https://zips.z.cash/draft-nuttycom-funding-allocation
/// The post-NU6 funding streams for Testnet as described in [ZIP-1015](https://zips.z.cash/zip-1015).
pub static ref POST_NU6_FUNDING_STREAMS_TESTNET: FundingStreams = FundingStreams {
// TODO: Adjust this height range and recipient list once a proposal is selected
height_range: POST_NU6_FUNDING_STREAM_START_RANGE_TESTNET,
recipients: [
(
Expand All @@ -279,7 +274,6 @@ lazy_static! {
),
(
FundingStreamReceiver::MajorGrants,
// TODO: Update these addresses
FundingStreamRecipient::new(8, POST_NU6_FUNDING_STREAM_FPF_ADDRESSES_TESTNET),
),
]
Expand All @@ -288,15 +282,14 @@ lazy_static! {
};
}

/// The start height of post-NU6 funding streams on Mainnet
// TODO: Add a reference to lockbox stream ZIP, this is currently based on https://zips.z.cash/draft-nuttycom-funding-allocation
/// The start height of post-NU6 funding streams on Mainnet as described in [ZIP-1015](https://zips.z.cash/zip-1015).
const POST_NU6_FUNDING_STREAM_START_HEIGHT_MAINNET: u32 = 2_726_400;

/// The start height of post-NU6 funding streams on Testnet
// TODO: Add a reference to lockbox stream ZIP, this is currently based on https://zips.z.cash/draft-nuttycom-funding-allocation
/// The start height of post-NU6 funding streams on Testnet as described in [ZIP-1015](https://zips.z.cash/zip-1015).
const POST_NU6_FUNDING_STREAM_START_HEIGHT_TESTNET: u32 = 2_976_000;

/// The number of blocks contained in the post-NU6 funding streams height ranges on Mainnet or Testnet.
/// The number of blocks contained in the post-NU6 funding streams height ranges on Mainnet or Testnet, as specified
/// in [ZIP-1015](https://zips.z.cash/zip-1015).
const POST_NU6_FUNDING_STREAM_NUM_BLOCKS: u32 = 420_000;

/// The post-NU6 funding stream height range on Mainnet
Expand Down
2 changes: 1 addition & 1 deletion zebra-consensus/src/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ where
})?;
}

// TODO: Add link to lockbox stream ZIP
// See [ZIP-1015](https://zips.z.cash/zip-1015).
let expected_deferred_amount = subsidy::funding_streams::funding_stream_values(
height,
&network,
Expand Down
49 changes: 23 additions & 26 deletions zebra-consensus/src/block/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,13 +162,7 @@ pub fn subsidy_is_valid(
.activation_height(network)
.expect("Canopy activation height is known");

// TODO: Add this as a field on `testnet::Parameters` instead of checking `disable_pow()`, this is 0 for Regtest in zcashd,
// see <https://github.com/zcash/zcash/blob/master/src/chainparams.cpp#L640>
let slow_start_interval = if network.disable_pow() {
Height(0)
} else {
network.slow_start_interval()
};
let slow_start_interval = network.slow_start_interval();

if height < slow_start_interval {
unreachable!(
Expand All @@ -191,7 +185,8 @@ pub fn subsidy_is_valid(
network,
expected_block_subsidy,
)
.expect("We always expect a funding stream hashmap response even if empty");
// we always expect a funding stream hashmap response even if empty
.map_err(|err| BlockError::Other(err.to_string()))?;

// # Consensus
//
Expand All @@ -204,14 +199,18 @@ pub fn subsidy_is_valid(
for (receiver, expected_amount) in funding_streams {
if receiver == FundingStreamReceiver::Deferred {
// The deferred pool contribution is checked in `miner_fees_are_valid()`
// TODO: Add link to lockbox stream ZIP
// See [ZIP-1015](https://zips.z.cash/zip-1015) for more details.
continue;
}

let address = subsidy::funding_streams::funding_stream_address(
height, network, receiver,
)
.expect("funding stream receivers other than the deferred pool must have an address");
let address =
subsidy::funding_streams::funding_stream_address(height, network, receiver)
// funding stream receivers other than the deferred pool must have an address
.ok_or_else(|| {
BlockError::Other(format!(
"missing funding stream address at height {height:?}"
))
})?;

let has_expected_output =
subsidy::funding_streams::filter_outputs_by_address(coinbase, address)
Expand Down Expand Up @@ -250,35 +249,33 @@ pub fn miner_fees_are_valid(
let sapling_value_balance = coinbase_tx.sapling_value_balance().sapling_amount();
let orchard_value_balance = coinbase_tx.orchard_value_balance().orchard_amount();

// TODO: Update the quote below once its been updated for NU6.
//
// # Consensus
//
// > The total value in zatoshi of transparent outputs from a coinbase transaction,
// > minus vbalanceSapling, minus vbalanceOrchard, MUST NOT be greater than the value
// > in zatoshi of block subsidy plus the transaction fees paid by transactions in this block.
// > - define the total output value of its coinbase transaction to be the total value in zatoshi of its transparent
// > outputs, minus vbalanceSapling, minus vbalanceOrchard, plus totalDeferredOutput(height);
// > – define the total input value of its coinbase transaction to be the value in zatoshi of the block subsidy,
// > plus the transaction fees paid by transactions in the block.
//
// https://zips.z.cash/protocol/protocol.pdf#txnconsensus
//
// The expected lockbox funding stream output of the coinbase transaction is also subtracted
// from the block subsidy value plus the transaction fees paid by transactions in this block.
let left = (transparent_value_balance - sapling_value_balance - orchard_value_balance)
.map_err(|_| SubsidyError::SumOverflow)?;
let right = (expected_block_subsidy + block_miner_fees - expected_deferred_amount)
.map_err(|_| SubsidyError::SumOverflow)?;
let total_output_value = (transparent_value_balance - sapling_value_balance - orchard_value_balance
+ expected_deferred_amount.constrain().expect("valid Amount with NonNegative constraint should be valid with NegativeAllowed constraint"))
.map_err(|_| SubsidyError::SumOverflow)?;
let total_input_value =
(expected_block_subsidy + block_miner_fees).map_err(|_| SubsidyError::SumOverflow)?;

// TODO: Updadte the quotes below if the final phrasing changes in the spec for NU6.
//
// # Consensus
//
// > [Pre-NU6] The total output of a coinbase transaction MUST NOT be greater than its total
// input.
//
// > [NU6 onward] The total output of a coinbase transaction MUST be equal to its total input.
if if NetworkUpgrade::current(network, height) < NetworkUpgrade::Nu6 {
left > right
total_output_value > total_input_value
} else {
left != right
total_output_value != total_input_value
} {
Err(SubsidyError::InvalidMinerFees)?
};
Expand Down
1 change: 1 addition & 0 deletions zebra-consensus/src/block/subsidy/funding_streams.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ pub fn funding_stream_values(
}
}
}

Ok(results)
}

Expand Down
104 changes: 0 additions & 104 deletions zebra-consensus/src/block/subsidy/general.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,47 +120,10 @@ pub fn output_amounts(transaction: &Transaction) -> HashSet<Amount<NonNegative>>
.collect()
}

/// Lockbox funding stream total input value for a block height.
///
/// Assumes a constant funding stream amount per block.
// TODO: Cache the lockbox value balance in zebra-state (will be required for tracking lockbox
// value balance after the Zcash Sustainability Fund ZIPs or after a ZIP for spending from the deferred pool)
#[allow(dead_code)]
fn lockbox_input_value(network: &Network, height: Height) -> Amount<NonNegative> {
let Some(nu6_activation_height) = Nu6.activation_height(network) else {
return Amount::zero();
};

let expected_block_subsidy = block_subsidy(nu6_activation_height, network)
.expect("block at NU6 activation height must have valid expected subsidy");
let &deferred_amount_per_block =
funding_stream_values(nu6_activation_height, network, expected_block_subsidy)
.expect("we always expect a funding stream hashmap response even if empty")
.get(&FundingStreamReceiver::Deferred)
.expect("we expect a lockbox funding stream after NU5");

let post_nu6_funding_stream_height_range = network.post_nu6_funding_streams().height_range();

// `min(height, last_height_with_deferred_pool_contribution) - (nu6_activation_height - 1)`,
// We decrement NU6 activation height since it's an inclusive lower bound.
// Funding stream height range end bound is not incremented since it's an exclusive end bound
let num_blocks_with_lockbox_output = (height.0 + 1)
.min(post_nu6_funding_stream_height_range.end.0)
.checked_sub(post_nu6_funding_stream_height_range.start.0)
.unwrap_or_default();

(deferred_amount_per_block * num_blocks_with_lockbox_output.into())
.expect("lockbox input value should fit in Amount")
}

#[cfg(test)]
mod test {
use super::*;
use color_eyre::Report;
use zebra_chain::parameters::testnet::{
self, ConfiguredActivationHeights, ConfiguredFundingStreamRecipient,
ConfiguredFundingStreams,
};

#[test]
fn halving_test() -> Result<(), Report> {
Expand Down Expand Up @@ -436,73 +399,6 @@ mod test {
Ok(())
}

#[test]
fn check_lockbox_input_value() -> Result<(), Report> {
let _init_guard = zebra_test::init();

let network = testnet::Parameters::build()
.with_activation_heights(ConfiguredActivationHeights {
blossom: Some(Blossom.activation_height(&Network::Mainnet).unwrap().0),
nu6: Some(POST_NU6_FUNDING_STREAMS_MAINNET.height_range().start.0),
..Default::default()
})
.with_post_nu6_funding_streams(ConfiguredFundingStreams {
// Start checking funding streams from block height 1
height_range: Some(POST_NU6_FUNDING_STREAMS_MAINNET.height_range().clone()),
// Use default post-NU6 recipients
recipients: Some(
POST_NU6_FUNDING_STREAMS_TESTNET
.recipients()
.iter()
.map(|(&receiver, recipient)| ConfiguredFundingStreamRecipient {
receiver,
numerator: recipient.numerator(),
addresses: Some(
recipient
.addresses()
.iter()
.map(|addr| addr.to_string())
.collect(),
),
})
.collect(),
),
})
.to_network();

let nu6_height = Nu6.activation_height(&network).unwrap();
let post_nu6_funding_streams = network.post_nu6_funding_streams();
let height_range = post_nu6_funding_streams.height_range();

let last_funding_stream_height = post_nu6_funding_streams
.height_range()
.end
.previous()
.expect("the previous height should be valid");

assert_eq!(
Amount::<NonNegative>::zero(),
lockbox_input_value(&network, Height::MIN)
);

let expected_lockbox_value: Amount<NonNegative> = Amount::try_from(18_750_000)?;
assert_eq!(
expected_lockbox_value,
lockbox_input_value(&network, nu6_height)
);

let num_blocks_total = height_range.end.0 - height_range.start.0;
let expected_input_per_block: Amount<NonNegative> = Amount::try_from(18_750_000)?;
let expected_lockbox_value = (expected_input_per_block * num_blocks_total.into())?;

assert_eq!(
expected_lockbox_value,
lockbox_input_value(&network, last_funding_stream_height)
);

Ok(())
}

#[test]
fn check_height_for_num_halvings() {
for network in Network::iter() {
Expand Down
6 changes: 3 additions & 3 deletions zebra-consensus/src/block/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -515,7 +515,7 @@ fn miner_fees_validation_for_network(network: Network) -> Result<(), Report> {

let expected_block_subsidy = block_subsidy(height, &network)?;

// TODO: Add link to lockbox stream ZIP
// See [ZIP-1015](https://zips.z.cash/zip-1015).
let expected_deferred_amount = subsidy::funding_streams::funding_stream_values(
height,
&network,
Expand Down Expand Up @@ -549,8 +549,8 @@ fn miner_fees_validation_failure() -> Result<(), Report> {
.expect("block should deserialize");
let height = block.coinbase_height().expect("valid coinbase height");
let expected_block_subsidy = block_subsidy(height, &network)?;
// TODO: Add link to lockbox stream ZIP
let expected_deferred_amount =
// See [ZIP-1015](https://zips.z.cash/zip-1015).
let expected_deferred_amount: Amount<zebra_chain::amount::NonNegative> =
subsidy::funding_streams::funding_stream_values(height, &network, expected_block_subsidy)
.expect("we always expect a funding stream hashmap response even if empty")
.remove(&FundingStreamReceiver::Deferred)
Expand Down
2 changes: 1 addition & 1 deletion zebra-consensus/src/checkpoint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -611,7 +611,7 @@ where
// We can't get the block subsidy for blocks with heights in the slow start interval, so we
// omit the calculation of the expected deferred amount.
let expected_deferred_amount = if height > self.network.slow_start_interval() {
// TODO: Add link to lockbox stream ZIP
// See [ZIP-1015](https://zips.z.cash/zip-1015).
funding_stream_values(height, &self.network, block_subsidy(height, &self.network)?)?
.remove(&FundingStreamReceiver::Deferred)
} else {
Expand Down
3 changes: 3 additions & 0 deletions zebra-consensus/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,9 @@ pub enum BlockError {
hash: zebra_chain::block::Hash,
source: amount::Error,
},

#[error("unexpected error occurred: {0}")]
Other(String),
}

impl From<SubsidyError> for BlockError {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ expression: get_block_subsidy
"fundingstreams": [
{
"recipient": "Zcash Community Grants NU6",
"specification": "https://zips.z.cash/draft-nuttycom-funding-allocation#alternative-2-hybrid-deferred-dev-fund-transitioning-to-a-non-direct-funding-model",
"specification": "https://zips.z.cash/zip-1015",
"value": 0.125,
"valueZat": 12500000,
"address": "t2HifwjUj9uyxr9bknR8LFuQbc98c3vkXtu"
Expand All @@ -15,7 +15,7 @@ expression: get_block_subsidy
"lockboxstreams": [
{
"recipient": "Lockbox NU6",
"specification": "https://zips.z.cash/draft-nuttycom-funding-allocation#alternative-2-hybrid-deferred-dev-fund-transitioning-to-a-non-direct-funding-model",
"specification": "https://zips.z.cash/zip-1015",
"value": 0.1875,
"valueZat": 18750000
}
Expand Down

0 comments on commit 46c6b6e

Please sign in to comment.