fix!: address Ironwood review findings across mempool, consensus, rpc, and state (#10884)

* fix(consensus): enforce coinbase output-decryptability for the Ironwood pool

NU6.3+ routes shielded coinbase value into the Ironwood pool, but
`decrypts_successfully` only iterated the Sapling and Orchard bundles, so
the all-zero-OVK coinbase decryptability rule (ZIP-213) was never applied to
Ironwood coinbase outputs. Add the Ironwood arm using `IronwoodDomain`.

* fix(mempool): track Ironwood nullifiers for spend-conflict detection

`VerifiedSet` tracked sprout/sapling/orchard nullifiers but not Ironwood, so
two mempool txs revealing the same Ironwood nullifier were both admitted, and
`reject_and_remove_same_effects` never evicted a mempool tx double-spending a
just-mined Ironwood nullifier. Either lets `getblocktemplate` build a block
template that every node then rejects. Mirror the Orchard handling for the
Ironwood pool.

* fix(consensus): fail closed instead of panicking on unexpected halo2 batch error

The `RestrictionUnsupportedByKey` arm called `unreachable!` inside the shared,
long-lived halo2 verifier service, so a future routing change or new
`#[non_exhaustive]` `BatchError` variant would crash the node instead of
rejecting one proof. Collapse it into the existing graceful per-item arm; the
routing invariant is kept as a comment.

* fix(state): include the Ironwood pool in supply metrics and spend indexing

`value_pool_metrics`/`chain_supply.total` omitted the Ironwood pool, so total
supply under-reported it every block post-NU6.3; and the `indexer` back-fill's
spend-sampling chain skipped Ironwood-only v6 txs. Add the Ironwood gauge, the
supply sum term, and Ironwood to the sampled spends.

* feat(rpc): expose Ironwood bundles in verbose transaction output

`getrawtransaction <txid> 1` / `getblock <hash> 2` serialized an `orchard`
section but no `ironwood` one, so v6 Ironwood bundles (including Zebra's own
Ironwood coinbase) were invisible to explorers and wallets. Add an `ironwood`
field reusing the Orchard-shaped object, and factor the shared construction
into `orchard_shaped_object` (also dropping an incidental O(n^2) action scan).

* refactor: deduplicate Ironwood/Orchard tree-update and bundle-verify paths

Ironwood reuses the Orchard tree and bundle types, so
`update_ironwood_note_commitment_tree` now delegates to the Orchard version
(re-tagging only the error variant), and `verify_orchard_v6_bundle` /
`verify_orchard_bundle` share a `queue_orchard_bundle` helper. No behavior
change.

* chore: remove the dead ZIP-235 cfg and error variant

The ZIP-235 burn validation and its tests were removed, but the `zip235`
`cfg` value and `SubsidyError::InvalidZip233Amount` (gated on it) remained, so
a `--cfg zcash_unstable="zip235"` build compiled yet enforced nothing. Drop
both; they can return with the NU7 NSM implementation.

* fix(state): fall back to the empty Ironwood tree before the genesis backfill

`ironwood_tree_by_height` panicked when the Ironwood column family was still
empty, which happens in the brief window after a v27->v28 upgrade before the
background `add_ironwood_tree` migration backfills genesis. That window is
always pre-NU6.3, when the Ironwood tree at every height is in fact empty, so
return the empty tree instead of panicking.

* refactor(state): share the Orchard/Ironwood tree-and-anchor add/remove logic

`add_/remove_{orchard,ironwood}_tree_and_anchor` were verbatim copies over
identically-typed field triples, including the reorg `RevertPosition::Root`
invariant-restoration. Extract shared `add_/remove_note_commitment_tree_and_anchor`
helpers parameterized on the target fields and a pool label. No behavior change
(non-finalized-state prop + reorg tests pass).

* refactor(chain): pass block commitment tree roots in a named struct

The history-tree functions took `sapling_root`, `orchard_root`, and
`ironwood_root` as adjacent positional arguments; the Orchard and Ironwood
roots reuse the same `orchard::tree::Root` type, so a swapped pair compiled
cleanly and would silently corrupt the ZIP-221 chain-history commitment.
Group them in a named `BlockCommitmentTreeRoots` struct so a swap can't happen.

* refactor(state): share the Orchard/Ironwood note-commitment DB accessors

The Ironwood tree-range, subtree-list, subtree-for-tip, tree-create, and
subtree-insert accessors were verbatim copies of their Orchard counterparts,
differing only in the column-family name (both pools reuse the `orchard::tree`
types). Route them through CF-parameterized private helpers; the public
signatures are unchanged.

Author: upbqdn

Cargo.toml

@@ -330,7 +330,7 @@ debug = false
 # The linter should ignore these expected config flags/values
 unexpected_cfgs = { level = "warn", check-cfg = [
     'cfg(tokio_unstable)', # Used by tokio-console
-    'cfg(zcash_unstable, values("zfuture", "nu6.1", "nu7", "zip235"))' # Used in Zebra and librustzcash
+    'cfg(zcash_unstable, values("zfuture", "nu6.1", "nu7"))' # Used in Zebra and librustzcash
 ] }
 
 # High-risk code

zebra-chain/CHANGELOG.md

@@ -36,11 +36,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - `parallel::tree::NoteCommitmentTreeError::Ironwood`
 - `primitives::zcash_history::V3` (the ZIP-221 Ironwood history node).
 - `impl Version for zcash_history::version::V3`
+- `primitives::zcash_history::BlockCommitmentTreeRoots`, grouping a block's Sapling,
+  Orchard, and Ironwood note commitment tree roots.
 
 ### Changed
 
-- The following history-tree functions now take an additional
-  `ironwood_root: &orchard::tree::Root` parameter:
+- The following history-tree functions now take a
+  `primitives::zcash_history::BlockCommitmentTreeRoots` struct grouping the Sapling,
+  Orchard, and Ironwood roots by name, instead of separate positional root parameters:
   - `history_tree::HistoryTree::{from_block, push}`
   - `history_tree::NonEmptyHistoryTree::{from_block, push, try_extend}`
   - `primitives::zcash_history::Tree::{append_leaf, new_from_block}`

zebra-chain/src/block/arbitrary.rs

@@ -8,6 +8,7 @@ use crate::{
     fmt::{HexDebug, SummaryDebug},
     history_tree::HistoryTree,
     parameters::{NetworkUpgrade::*, GENESIS_PREVIOUS_BLOCK_HASH},
+    primitives::zcash_history::BlockCommitmentTreeRoots,
     serialization::{self, BytesInDisplayOrder},
     transaction::arbitrary::MAX_ARBITRARY_ITEMS,
     transparent::{
@@ -537,19 +538,23 @@ impl Block {
                             .push(
                                 &current.network,
                                 Arc::new(block.clone()),
-                                &sapling_tree.root(),
-                                &orchard_tree.root(),
-                                &ironwood_tree.root(),
+                                BlockCommitmentTreeRoots {
+                                    sapling: &sapling_tree.root(),
+                                    orchard: &orchard_tree.root(),
+                                    ironwood: &ironwood_tree.root(),
+                                },
                             )
                             .unwrap();
                     } else {
                         history_tree = Some(
                             HistoryTree::from_block(
                                 &current.network,
                                 Arc::new(block.clone()),
-                                &sapling_tree.root(),
-                                &orchard_tree.root(),
-                                &ironwood_tree.root(),
+                                BlockCommitmentTreeRoots {
+                                    sapling: &sapling_tree.root(),
+                                    orchard: &orchard_tree.root(),
+                                    ironwood: &ironwood_tree.root(),
+                                },
                             )
                             .unwrap(),
                         );

zebra-chain/src/history_tree.rs

@@ -15,12 +15,11 @@ use thiserror::Error;
 use crate::{
     block::{Block, ChainHistoryMmrRootHash, Height},
     fmt::SummaryDebug,
-    orchard,
     parameters::{Network, NetworkUpgrade},
     primitives::zcash_history::{
-        Entry, Tree, V1 as PreOrchard, V2 as OrchardOnward, V3 as IronwoodOnward,
+        BlockCommitmentTreeRoots, Entry, Tree, V1 as PreOrchard, V2 as OrchardOnward,
+        V3 as IronwoodOnward,
     },
-    sapling,
 };
 
 /// An error describing why a history tree operation failed.
@@ -154,16 +153,13 @@ impl NonEmptyHistoryTree {
 
     /// Create a new history tree with a single block.
     ///
-    /// `sapling_root` is the root of the Sapling note commitment tree of the block.
-    /// `orchard_root` is the root of the Orchard note commitment tree of the block;
-    ///  (ignored for pre-Orchard blocks).
+    /// The Orchard root in `roots` is ignored for pre-Orchard blocks, and the Ironwood root for
+    /// pre-NU6.3 blocks; each inner tree version only reads the roots it commits to.
     #[allow(clippy::unwrap_in_result)]
     pub fn from_block(
         network: &Network,
         block: Arc<Block>,
-        sapling_root: &sapling::tree::Root,
-        orchard_root: &orchard::tree::Root,
-        ironwood_root: &orchard::tree::Root,
+        roots: BlockCommitmentTreeRoots,
     ) -> Result<Self, HistoryTreeError> {
         let height = block
             .coinbase_height()
@@ -178,48 +174,24 @@ impl NonEmptyHistoryTree {
                 panic!("HistoryTree does not exist for pre-Heartwood upgrades")
             }
             NetworkUpgrade::Heartwood | NetworkUpgrade::Canopy => {
-                let (tree, entry) = Tree::<PreOrchard>::new_from_block(
-                    network,
-                    block,
-                    sapling_root,
-                    &Default::default(),
-                    &Default::default(),
-                )?;
+                let (tree, entry) = Tree::<PreOrchard>::new_from_block(network, block, roots)?;
                 (InnerHistoryTree::PreOrchard(tree), entry)
             }
             NetworkUpgrade::Nu5
             | NetworkUpgrade::Nu6
             | NetworkUpgrade::Nu6_1
             | NetworkUpgrade::Nu6_2 => {
-                let (tree, entry) = Tree::<OrchardOnward>::new_from_block(
-                    network,
-                    block,
-                    sapling_root,
-                    orchard_root,
-                    &Default::default(),
-                )?;
+                let (tree, entry) = Tree::<OrchardOnward>::new_from_block(network, block, roots)?;
                 (InnerHistoryTree::OrchardOnward(tree), entry)
             }
             NetworkUpgrade::Nu6_3 | NetworkUpgrade::Nu7 => {
-                let (tree, entry) = Tree::<IronwoodOnward>::new_from_block(
-                    network,
-                    block,
-                    sapling_root,
-                    orchard_root,
-                    ironwood_root,
-                )?;
+                let (tree, entry) = Tree::<IronwoodOnward>::new_from_block(network, block, roots)?;
                 (InnerHistoryTree::IronwoodOnward(tree), entry)
             }
 
             #[cfg(zcash_unstable = "zfuture")]
             NetworkUpgrade::ZFuture => {
-                let (tree, entry) = Tree::<IronwoodOnward>::new_from_block(
-                    network,
-                    block,
-                    sapling_root,
-                    orchard_root,
-                    ironwood_root,
-                )?;
+                let (tree, entry) = Tree::<IronwoodOnward>::new_from_block(network, block, roots)?;
                 (InnerHistoryTree::IronwoodOnward(tree), entry)
             }
         };
@@ -237,9 +209,8 @@ impl NonEmptyHistoryTree {
 
     /// Add block data to the tree.
     ///
-    /// `sapling_root` is the root of the Sapling note commitment tree of the block.
-    /// `orchard_root` is the root of the Orchard note commitment tree of the block;
-    ///  (ignored for pre-Orchard blocks).
+    /// The Orchard root in `roots` is ignored for pre-Orchard blocks, and the Ironwood root for
+    /// pre-NU6.3 blocks.
     ///
     /// # Panics
     ///
@@ -248,9 +219,7 @@ impl NonEmptyHistoryTree {
     pub fn push(
         &mut self,
         block: Arc<Block>,
-        sapling_root: &sapling::tree::Root,
-        orchard_root: &orchard::tree::Root,
-        ironwood_root: &orchard::tree::Root,
+        roots: BlockCommitmentTreeRoots,
     ) -> Result<(), HistoryTreeError> {
         // Check if the block has the expected height.
         // librustzcash assumes the heights are correct and corrupts the tree if they are wrong,
@@ -270,13 +239,7 @@ impl NonEmptyHistoryTree {
         if network_upgrade != self.network_upgrade {
             // This is the activation block of a network upgrade.
             // Create a new tree.
-            let new_tree = Self::from_block(
-                &self.network,
-                block,
-                sapling_root,
-                orchard_root,
-                ironwood_root,
-            )?;
+            let new_tree = Self::from_block(&self.network, block, roots)?;
             // Replaces self with the new tree
             *self = new_tree;
             assert_eq!(self.network_upgrade, network_upgrade);
@@ -285,13 +248,13 @@ impl NonEmptyHistoryTree {
 
         let new_entries = match &mut self.inner {
             InnerHistoryTree::PreOrchard(tree) => tree
-                .append_leaf(block, sapling_root, orchard_root, ironwood_root)
+                .append_leaf(block, roots)
                 .map_err(|e| HistoryTreeError::InnerError { inner: e })?,
             InnerHistoryTree::OrchardOnward(tree) => tree
-                .append_leaf(block, sapling_root, orchard_root, ironwood_root)
+                .append_leaf(block, roots)
                 .map_err(|e| HistoryTreeError::InnerError { inner: e })?,
             InnerHistoryTree::IronwoodOnward(tree) => tree
-                .append_leaf(block, sapling_root, orchard_root, ironwood_root)
+                .append_leaf(block, roots)
                 .map_err(|e| HistoryTreeError::InnerError { inner: e })?,
         };
         for entry in new_entries {
@@ -305,22 +268,12 @@ impl NonEmptyHistoryTree {
     }
 
     /// Extend the history tree with the given blocks.
-    pub fn try_extend<
-        'a,
-        T: IntoIterator<
-            Item = (
-                Arc<Block>,
-                &'a sapling::tree::Root,
-                &'a orchard::tree::Root,
-                &'a orchard::tree::Root,
-            ),
-        >,
-    >(
+    pub fn try_extend<'a, T: IntoIterator<Item = (Arc<Block>, BlockCommitmentTreeRoots<'a>)>>(
         &mut self,
         iter: T,
     ) -> Result<(), HistoryTreeError> {
-        for (block, sapling_root, orchard_root, ironwood_root) in iter {
-            self.push(block, sapling_root, orchard_root, ironwood_root)?;
+        for (block, roots) in iter {
+            self.push(block, roots)?;
         }
         Ok(())
     }
@@ -519,9 +472,7 @@ impl HistoryTree {
     pub fn from_block(
         network: &Network,
         block: Arc<Block>,
-        sapling_root: &sapling::tree::Root,
-        orchard_root: &orchard::tree::Root,
-        ironwood_root: &orchard::tree::Root,
+        roots: BlockCommitmentTreeRoots,
     ) -> Result<Self, HistoryTreeError> {
         let Some(heartwood_height) = NetworkUpgrade::Heartwood.activation_height(network) else {
             // Return early if there is no Heartwood activation height.
@@ -534,14 +485,7 @@ impl HistoryTree {
             .cmp(&heartwood_height)
         {
             std::cmp::Ordering::Less => Ok(HistoryTree(None)),
-            _ => Ok(NonEmptyHistoryTree::from_block(
-                network,
-                block,
-                sapling_root,
-                orchard_root,
-                ironwood_root,
-            )?
-            .into()),
+            _ => Ok(NonEmptyHistoryTree::from_block(network, block, roots)?.into()),
         }
     }
 
@@ -554,9 +498,7 @@ impl HistoryTree {
         &mut self,
         network: &Network,
         block: Arc<Block>,
-        sapling_root: &sapling::tree::Root,
-        orchard_root: &orchard::tree::Root,
-        ironwood_root: &orchard::tree::Root,
+        roots: BlockCommitmentTreeRoots,
     ) -> Result<(), HistoryTreeError> {
         let Some(heartwood_height) = NetworkUpgrade::Heartwood.activation_height(network) else {
             assert!(
@@ -579,21 +521,15 @@ impl HistoryTree {
                 );
             }
             std::cmp::Ordering::Equal => {
-                let tree = Some(NonEmptyHistoryTree::from_block(
-                    network,
-                    block,
-                    sapling_root,
-                    orchard_root,
-                    ironwood_root,
-                )?);
+                let tree = Some(NonEmptyHistoryTree::from_block(network, block, roots)?);
                 // Replace the current object with the new tree
                 *self = HistoryTree(tree);
             }
             std::cmp::Ordering::Greater => {
                 self.0
                     .as_mut()
                     .expect("history tree must exist Heartwood-onward")
-                    .push(block.clone(), sapling_root, orchard_root, ironwood_root)?;
+                    .push(block.clone(), roots)?;
             }
         };
         Ok(())

zebra-chain/src/history_tree/tests/vectors.rs

@@ -7,6 +7,7 @@ use crate::{
     },
     history_tree::NonEmptyHistoryTree,
     parameters::{Network, NetworkUpgrade},
+    primitives::zcash_history::BlockCommitmentTreeRoots,
     sapling,
     serialization::ZcashDeserializeInto,
 };
@@ -61,9 +62,11 @@ fn push_and_prune_for_network_upgrade(
     let mut tree = NonEmptyHistoryTree::from_block(
         &network,
         first_block,
-        &first_sapling_root,
-        &Default::default(),
-        &Default::default(),
+        BlockCommitmentTreeRoots {
+            sapling: &first_sapling_root,
+            orchard: &Default::default(),
+            ironwood: &Default::default(),
+        },
     )?;
 
     assert_eq!(tree.size(), 1);
@@ -94,9 +97,11 @@ fn push_and_prune_for_network_upgrade(
     )?;
     tree.push(
         second_block,
-        &second_sapling_root,
-        &Default::default(),
-        &Default::default(),
+        BlockCommitmentTreeRoots {
+            sapling: &second_sapling_root,
+            orchard: &Default::default(),
+            ironwood: &Default::default(),
+        },
     )
     .unwrap();
 
@@ -143,9 +148,11 @@ fn upgrade_for_network_upgrade(network: Network, network_upgrade: NetworkUpgrade
     let mut tree = NonEmptyHistoryTree::from_block(
         &network,
         block_prev,
-        &sapling_root_prev,
-        &Default::default(),
-        &Default::default(),
+        BlockCommitmentTreeRoots {
+            sapling: &sapling_root_prev,
+            orchard: &Default::default(),
+            ironwood: &Default::default(),
+        },
     )?;
 
     assert_eq!(tree.size(), 1);
@@ -170,9 +177,11 @@ fn upgrade_for_network_upgrade(network: Network, network_upgrade: NetworkUpgrade
     )?;
     tree.push(
         activation_block,
-        &activation_sapling_root,
-        &Default::default(),
-        &Default::default(),
+        BlockCommitmentTreeRoots {
+            sapling: &activation_sapling_root,
+            orchard: &Default::default(),
+            ironwood: &Default::default(),
+        },
     )
     .unwrap();