diff --git a/.github/workflows/tests-unit.yml b/.github/workflows/tests-unit.yml index bf3db7b8f7..bd81a45086 100644 --- a/.github/workflows/tests-unit.yml +++ b/.github/workflows/tests-unit.yml @@ -393,6 +393,36 @@ jobs: cargo test --locked --features zip233 \ -p zakura-consensus -p zakura-rpc --lib + zip235: + name: ZIP 235 feature tests + needs: changes + # Compiles its own binaries, because the shared nextest archive is built + # without this feature, and because the feature needs its own RUSTFLAGS. + if: needs.changes.outputs.rust == 'true' || github.event_name != 'pull_request' + permissions: + contents: read + id-token: write + statuses: write + runs-on: ubuntu-latest + timeout-minutes: 60 + env: + # ZIP 235 enables ZIP 233, which needs this cfg to reach the matching + # `zakura-primitives` code. + RUSTFLAGS: --cfg zcash_unstable="nu7" + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 #v7.0.1 + with: + persist-credentials: false + - uses: actions-rust-lang/setup-rust-toolchain@166cdcfd11aee3cb47222f9ddb555ce30ddb9659 #v1.17.0 + with: + toolchain: stable + - uses: ./.github/actions/setup-zakura-build + - name: Test the crates that ZIP 235 changes + run: | + cargo test --locked --features zakura-chain/zip235 -p zakura-chain --lib + cargo test --locked --features zip235 \ + -p zakura-consensus -p zakura-rpc --lib + network-dependent: name: network-dependent acceptance tests needs: changes @@ -523,6 +553,7 @@ jobs: - zakura-config - zip218 - zip233 + - zip235 - network-dependent - pruned-storage - check-no-git-dependencies @@ -540,7 +571,7 @@ jobs: # check-no-git-dependencies is A-release-label only, publish-graph # skips on PRs that do not touch Cargo manifests or this check. # Failed, non-skipped jobs still fail this aggregate status. - allowed-skips: check-no-git-dependencies, pruned-storage, zakura-config, build-tests, unit-tests, network-dependent, zip218, zip233, publish-graph + allowed-skips: check-no-git-dependencies, pruned-storage, zakura-config, build-tests, unit-tests, network-dependent, zip218, zip233, zip235, publish-graph report-nightly-result: name: report nightly release test result diff --git a/Cargo.lock b/Cargo.lock index 477f0b6ba1..5d40925d3c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8165,7 +8165,7 @@ dependencies = [ [[package]] name = "zakura-rpc" -version = "8.1.0" +version = "9.0.0" dependencies = [ "anyhow", "base64 0.22.1", diff --git a/crates/zakura-chain/Cargo.toml b/crates/zakura-chain/Cargo.toml index cf61cea019..83e45cecea 100644 --- a/crates/zakura-chain/Cargo.toml +++ b/crates/zakura-chain/Cargo.toml @@ -39,6 +39,13 @@ zip218 = [] # ZIP 233. zip233 = ["zcash_primitives/zip-233"] +# ZIP 235 "Remove 60% of Transaction Fees From Circulation": the coinbase +# must remove at least 60% of the block's transaction fees from circulation. +# +# ZIP 235 must deploy at the same time as or after ZIP 233, because the burn is carried +# by ZIP 233's `zip233Amount` field. +zip235 = ["zip233"] + # Consensus-critical conversion from JSON to Zcash types json-conversion = [ "serde_json", diff --git a/crates/zakura-chain/src/parameters/network/subsidy.rs b/crates/zakura-chain/src/parameters/network/subsidy.rs index 276532608c..684ad1d708 100644 --- a/crates/zakura-chain/src/parameters/network/subsidy.rs +++ b/crates/zakura-chain/src/parameters/network/subsidy.rs @@ -385,6 +385,9 @@ pub enum SubsidyError { #[error("miner fees are invalid")] InvalidMinerFees, + #[error("coinbase removes less than 60% of the block's transaction fees from circulation")] + InsufficientFeeBurn, + #[error("addition of amounts overflowed")] Overflow, @@ -451,6 +454,34 @@ pub fn halving(height: Height, network: &Network) -> u32 { .expect("halving index is non-negative and fits in u32") } +/// The share of a block's transaction fees that [ZIP 235] removes from circulation, as a +/// numerator over [`FEE_BURN_DENOMINATOR`]. +/// +/// [ZIP 235]: https://zips.z.cash/zip-0235 +pub const FEE_BURN_NUMERATOR: u64 = 6; + +/// The denominator of [`FEE_BURN_NUMERATOR`]. +pub const FEE_BURN_DENOMINATOR: u64 = 10; + +/// Returns the smallest amount a block's coinbase may remove from circulation under +/// [ZIP 235], given the block's total transaction fees. +/// +/// # Consensus +/// +/// > For each block, at least 60% (rounded down) of the total fees are to be removed from +/// > circulation. +/// +/// The rounding favours the miner, so this is `floor(block_miner_fees * 6 / 10)`. +/// +/// [ZIP 235]: https://zips.z.cash/zip-0235 +pub fn minimum_fee_burn( + block_miner_fees: Amount, +) -> Result, SubsidyError> { + let scaled = (block_miner_fees * FEE_BURN_NUMERATOR).map_err(|_| SubsidyError::Overflow)?; + + (scaled / FEE_BURN_DENOMINATOR).map_err(|_| SubsidyError::Overflow) +} + /// `BlockSubsidy(height)` as described in [protocol specification ยง7.8][7.8] /// /// [7.8]: https://zips.z.cash/protocol/protocol.pdf#subsidies diff --git a/crates/zakura-chain/src/parameters/network_upgrade.rs b/crates/zakura-chain/src/parameters/network_upgrade.rs index d57ddc0e41..7e35af17b5 100644 --- a/crates/zakura-chain/src/parameters/network_upgrade.rs +++ b/crates/zakura-chain/src/parameters/network_upgrade.rs @@ -287,6 +287,15 @@ pub const ZIP218_ENABLED: bool = cfg!(feature = "zip218"); /// The rules are still dormant until NU7 activates on the configured network. pub const ZIP233_ENABLED: bool = cfg!(feature = "zip233") && cfg!(zcash_unstable = "nu7"); +/// Whether the ZIP 235 rules are compiled into this build. +/// +/// ZIP 235 requires a block's coinbase transaction to remove at least 60% of the block's +/// transaction fees from circulation, through ZIP 233's `zip233Amount` field, so it needs +/// [`ZIP233_ENABLED`] as well as its own feature. +/// +/// The rules are still dormant until NU7 activates on the configured network. +pub const ZIP235_ENABLED: bool = cfg!(feature = "zip235") && ZIP233_ENABLED; + /// The target block spacing after NU7 activation, in seconds. /// /// `PostNU7PoWTargetSpacing` in ZIP 218. @@ -656,6 +665,14 @@ impl NetworkUpgrade { ZIP233_ENABLED && Self::is_nu7_active(network, height) } + /// Returns `true` if the ZIP 235 rules are compiled in and active for + /// `network` at `height`. + /// + /// See [`ZIP235_ENABLED`] and [`NetworkUpgrade::is_nu7_active`]. + pub fn is_zip235_active(network: &Network, height: block::Height) -> bool { + ZIP235_ENABLED && Self::is_nu7_active(network, height) + } + /// Returns `true` if the ZIP 218 rules are compiled in and active for /// `network` at `height`. /// diff --git a/crates/zakura-consensus/Cargo.toml b/crates/zakura-consensus/Cargo.toml index 480d66f1fb..58166b51a5 100644 --- a/crates/zakura-consensus/Cargo.toml +++ b/crates/zakura-consensus/Cargo.toml @@ -38,6 +38,16 @@ zip233 = [ "zakura-state/zip233", ] +# ZIP 235 "Remove 60% of Transaction Fees From Circulation": the coinbase +# must remove at least 60% of the block's transaction fees from circulation. +# +# Enables `zip233`, which carries the burn, and so needs the same RUSTFLAGS. +zip235 = [ + "zakura-chain/zip235", + "zakura-state/zip235", + "zip233", +] + # Production features that activate extra dependencies, or extra features in dependencies progress-bar = [ diff --git a/crates/zakura-consensus/src/block/check.rs b/crates/zakura-consensus/src/block/check.rs index 48aa1d451c..db2809d7dc 100644 --- a/crates/zakura-consensus/src/block/check.rs +++ b/crates/zakura-consensus/src/block/check.rs @@ -11,8 +11,8 @@ use zakura_chain::{ block::{Block, Hash, Header, Height}, parameters::{ subsidy::{ - founders_reward, founders_reward_address, funding_stream_values, FundingStreamReceiver, - ParameterSubsidy, SubsidyError, + founders_reward, founders_reward_address, funding_stream_values, minimum_fee_burn, + FundingStreamReceiver, ParameterSubsidy, SubsidyError, }, Network, NetworkUpgrade, GLOBAL_SHIELDED_BUDGET, ORCHARD_BLOCK_ACTION_LIMIT, SAPLING_BLOCK_IO_LIMIT, SPROUT_BLOCK_JOINSPLIT_LIMIT, @@ -341,6 +341,28 @@ pub fn miner_fees_are_valid( let orchard_value_balance = coinbase_tx.orchard_value_balance().orchard_amount(); let ironwood_value_balance = coinbase_tx.ironwood_value_balance().ironwood_amount(); + // A coinbase transaction can remove value from circulation too, under ZIP 233. That + // value comes out of the same block subsidy and fees as its outputs, so it counts + // towards the coinbase's total output value below. + let zip233_amount: Amount = coinbase_tx + .zip233_amount() + .constrain() + .map_err(SubsidyError::InvalidAmount)?; + + // # Consensus + // + // > For each block, at least 60% (rounded down) of the total fees are to be removed + // > from circulation. + // + // https://zips.z.cash/zip-0235 + if NetworkUpgrade::is_zip235_active(network, height) { + let minimum_fee_burn = minimum_fee_burn(block_miner_fees)?; + + if coinbase_tx.zip233_amount() < minimum_fee_burn { + Err(SubsidyError::InsufficientFeeBurn)? + } + } + // # Consensus // // > - define the total output value of its coinbase transaction to be the total value in zatoshi of its transparent @@ -357,8 +379,9 @@ pub fn miner_fees_are_valid( - sapling_value_balance - orchard_value_balance - ironwood_value_balance - + expected_deferred_pool_balance_change.value()) - .map_err(|_| SubsidyError::Overflow)?; + + expected_deferred_pool_balance_change.value() + + zip233_amount) + .map_err(|_| SubsidyError::Overflow)?; let total_input_value = (expected_block_subsidy + block_miner_fees).map_err(|_| SubsidyError::Overflow)?; diff --git a/crates/zakura-consensus/src/block/tests.rs b/crates/zakura-consensus/src/block/tests.rs index 06caeaa365..bfbbc07b3b 100644 --- a/crates/zakura-consensus/src/block/tests.rs +++ b/crates/zakura-consensus/src/block/tests.rs @@ -615,6 +615,103 @@ fn miner_fees_validation_for_network(network: Network) -> Result<(), Report> { Ok(()) } +/// Checks the ZIP 235 fee burn: the coinbase must remove at least 60% of the block's +/// transaction fees from circulation, and the burn counts towards its total output value. +#[test] +fn zip235_coinbase_must_burn_sixty_percent_of_fees() -> Result<(), Report> { + use zakura_chain::{ + amount::NonNegative, + parameters::{ + subsidy::minimum_fee_burn, + testnet::{ConfiguredActivationHeights, Parameters}, + ZIP235_ENABLED, + }, + }; + + let _init_guard = zakura_test::init(); + + let nu7 = Height(1); + let network = Parameters::build() + .with_slow_start_interval(Height::MIN) + .with_activation_heights(ConfiguredActivationHeights { + nu7: Some(nu7.0), + ..Default::default() + })? + .clear_funding_streams() + .to_network() + .expect("configured testnet is valid"); + + let block_miner_fees = Amount::::try_from(1_000)?; + let expected_block_subsidy = block_subsidy(nu7, &network)?; + let expected_deferred_pool_balance_change = DeferredPoolBalanceChange::zero(); + + // 60% of 1000, rounded down in the miner's favour. + let required_burn = minimum_fee_burn(block_miner_fees)?; + assert_eq!(required_burn, Amount::::try_from(600)?); + + // A coinbase pays out the subsidy plus the fees it did not burn, and burns the rest, + // so its total output value is unchanged. + let coinbase_with_burn = |zip233_amount: Amount| -> Result { + let payout = ((expected_block_subsidy + block_miner_fees) - zip233_amount)?; + + Ok(Transaction::V6 { + network_upgrade: NetworkUpgrade::Nu7, + lock_time: LockTime::unlocked(), + expiry_height: nu7, + zip233_amount, + inputs: vec![transparent::Input::Coinbase { + height: nu7, + data: Vec::new(), + sequence: u32::MAX, + }], + outputs: vec![transparent::Output { + value: payout, + lock_script: transparent::Script::new(&[]), + }], + sapling_shielded_data: None, + orchard_shielded_data: None, + ironwood_shielded_data: None, + }) + }; + + let check = |coinbase: &Transaction| { + check::miner_fees_are_valid( + coinbase, + nu7, + block_miner_fees, + expected_block_subsidy, + expected_deferred_pool_balance_change, + &network, + ) + }; + + // Burning exactly the minimum is valid, and so is burning more. + assert!(check(&coinbase_with_burn(required_burn)?).is_ok()); + assert!(check(&coinbase_with_burn(Amount::::try_from(700)?)?).is_ok()); + + let too_little = coinbase_with_burn(Amount::::try_from(599)?)?; + let none = coinbase_with_burn(Amount::zero())?; + + if ZIP235_ENABLED { + for coinbase in [&too_little, &none] { + assert_eq!( + check(coinbase), + Err(BlockError::Transaction(TransactionError::Subsidy( + SubsidyError::InsufficientFeeBurn + ))), + "a coinbase that burns less than 60% of the fees must be invalid", + ); + } + } else { + // Without the feature there is no minimum, and the burn still counts towards the + // coinbase's total output value, so these coinbases balance. + assert!(check(&too_little).is_ok()); + assert!(check(&none).is_ok()); + } + + Ok(()) +} + #[test] fn miner_fees_validation_failure() -> Result<(), Report> { let _init_guard = zakura_test::init(); diff --git a/crates/zakura-rpc/Cargo.toml b/crates/zakura-rpc/Cargo.toml index 807c846e61..f5f7b0f5fb 100644 --- a/crates/zakura-rpc/Cargo.toml +++ b/crates/zakura-rpc/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "zakura-rpc" -version = "8.1.0" +version = "9.0.0" authors.workspace = true description = "The Zakura node's JSON Remote Procedure Call (JSON-RPC) interface. Internal crate, published to support cargo install zakura" license.workspace = true @@ -42,6 +42,17 @@ zip233 = [ "zakura-state/zip233", ] +# ZIP 235 "Remove 60% of Transaction Fees From Circulation": the coinbase +# must remove at least 60% of the block's transaction fees from circulation. +# +# Enables `zip233`, which carries the burn, and so needs the same RUSTFLAGS. +zip235 = [ + "zakura-chain/zip235", + "zakura-consensus/zip235", + "zakura-state/zip235", + "zip233", +] + # Production features that activate extra dependencies, or extra features in # dependencies diff --git a/crates/zakura-rpc/src/methods/types/transaction.rs b/crates/zakura-rpc/src/methods/types/transaction.rs index 69ef65fcbe..5715b6ddf1 100644 --- a/crates/zakura-rpc/src/methods/types/transaction.rs +++ b/crates/zakura-rpc/src/methods/types/transaction.rs @@ -14,7 +14,7 @@ use zakura_chain::{ block::{self, merkle::AUTH_DIGEST_PLACEHOLDER, Height}, orchard, parameters::{ - subsidy::{block_subsidy, funding_stream_values, miner_subsidy}, + subsidy::{block_subsidy, funding_stream_values, miner_subsidy, minimum_fee_burn}, Network, NetworkUpgrade, }, primitives::ed25519, @@ -135,7 +135,25 @@ impl TransactionTemplate { txs_fee: Amount, ) -> Result { let block_subsidy = block_subsidy(height, net)?; - let miner_reward = miner_subsidy(height, net, block_subsidy)? + txs_fee; + + // # Consensus + // + // > For each block, at least 60% (rounded down) of the total fees are to be + // > removed from circulation. + // + // https://zips.z.cash/zip-0235 + // + // The burn comes out of the fees, so the miner keeps the block subsidy plus the + // remaining 40%. Templates burn exactly the minimum: any more is the miner's to + // give away, not this node's. + let fee_burn = if NetworkUpgrade::is_zip235_active(net, height) { + minimum_fee_burn(txs_fee)? + } else { + Amount::zero() + }; + let miner_fees = (txs_fee - fee_burn)?; + + let miner_reward = miner_subsidy(height, net, block_subsidy)? + miner_fees; let miner_reward = Zatoshis::try_from(miner_reward?)?; let mut builder = Builder::new( @@ -278,6 +296,17 @@ impl TransactionTemplate { builder.add_transparent_output(&fs_addr, fs_amount)?; } + // The ZIP 233 field carries the ZIP 235 burn. It is set after the outputs, so the + // builder's value balance already accounts for the reduced miner reward. + // + // The setter only exists where `zakura-primitives` compiles its ZIP 233 code, + // which needs the `zcash_unstable = "nu7"` cfg as well as the feature. That is + // exactly when `ZIP235_ENABLED` is true, so `fee_burn` is zero otherwise. + #[cfg(all(feature = "zip235", zcash_unstable = "nu7"))] + if fee_burn > Amount::::zero() { + builder.set_zip233_amount(Zatoshis::try_from(fee_burn)?); + } + // Reuse the process-wide Sapling prover instead of re-parsing the bundled parameters on // every coinbase build. let sapling_prover = zakura_consensus::sapling_prover(); diff --git a/crates/zakura-state/Cargo.toml b/crates/zakura-state/Cargo.toml index 5d6df25f4c..5027a9d926 100644 --- a/crates/zakura-state/Cargo.toml +++ b/crates/zakura-state/Cargo.toml @@ -31,6 +31,15 @@ zip233 = [ "zakura-chain/zip233", ] +# ZIP 235 "Remove 60% of Transaction Fees From Circulation": the coinbase +# must remove at least 60% of the block's transaction fees from circulation. +# +# Enables `zip233`, which carries the burn, and so needs the same RUSTFLAGS. +zip235 = [ + "zakura-chain/zip235", + "zip233", +] + # Production features that activate extra dependencies, or extra features in dependencies # Exposes narrow internal hooks used by benchmarks. diff --git a/crates/zakurad/Cargo.toml b/crates/zakurad/Cargo.toml index cd1b42eb8a..d53adb1119 100644 --- a/crates/zakurad/Cargo.toml +++ b/crates/zakurad/Cargo.toml @@ -95,6 +95,18 @@ zip233 = [ "zakura-rpc/zip233", ] +# ZIP 235 "Remove 60% of Transaction Fees From Circulation": the coinbase +# must remove at least 60% of the block's transaction fees from circulation. +# +# Enables `zip233`, which carries the burn, and so needs the same RUSTFLAGS. +zip235 = [ + "zakura-chain/zip235", + "zakura-consensus/zip235", + "zakura-state/zip235", + "zakura-rpc/zip235", + "zip233", +] + # In release builds, don't compile debug logging code, to improve performance. default-release-binaries = ["release_max_level_info", "progress-bar", "prometheus", "sentry", "opentelemetry"] @@ -195,7 +207,7 @@ zakura-header-chain = { path = "../zakura-header-chain", version = "1.2.0" } zakura-jsonl-trace = { path = "../zakura-jsonl-trace", version = "1.2.0" } zakura-network = { path = "../zakura-network", version = "7.0.1" } zakura-node-services = { path = "../zakura-node-services", version = "3.2.2", features = ["rpc-client"] } -zakura-rpc = { path = "../zakura-rpc", version = "8.1.0" } +zakura-rpc = { path = "../zakura-rpc", version = "9.0.0" } zakura-state = { path = "../zakura-state", version = "7.2.0" } # zakura-script is not used directly, but we list it here to enable the # "comparison-interpreter" feature. (Feature unification will take care of diff --git a/docs/changelog/unreleased/858.md b/docs/changelog/unreleased/858.md new file mode 100644 index 0000000000..99d5984b50 --- /dev/null +++ b/docs/changelog/unreleased/858.md @@ -0,0 +1,15 @@ +## Added + +- ZIP 235 "Remove 60% of Transaction Fees From Circulation", behind the + off-by-default `zip235` build feature: from NU7 activation onwards, a block's + coinbase must remove at least 60% of the block's transaction fees from + circulation through the ZIP 233 `zip233Amount` field. Block templates burn exactly + that minimum and pay the miner the remaining 40%. The feature enables `zip233`, + and so needs the same `RUSTFLAGS='--cfg zcash_unstable="nu7"'` + ([#858](https://github.com/zakura-core/zakura/pull/858)). + +## Changed + +- `TransactionTemplate::new_coinbase` takes the money reserve, so `zakura-rpc` + takes a major version bump + ([#858](https://github.com/zakura-core/zakura/pull/858)).