feat(consensus): implement ZIP 235 behind a build feature

.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

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",

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<NonNegative>,
+) -> Result<Amount<NonNegative>, 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

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`.
     ///

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 = [

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<NegativeAllowed> = 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)?;

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::<NonNegative>::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::<NonNegative>::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<NonNegative>| -> Result<Transaction, Report> {
+        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::<NonNegative>::try_from(700)?)?).is_ok());
+
+    let too_little = coinbase_with_burn(Amount::<NonNegative>::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();

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