fix!: address more Ironwood review findings across chain, consensus, and state (#10886)

* fix: correct Ironwood v6 flag, history-tree, and empty-tree rules

- chain: reject the enableCrossAddress flag (bit 2) on the v6 Orchard
  bundle at deserialization, matching orchard::Flags::from_byte, which
  reserves it for the Orchard pool in every tx version. Only the Ironwood
  bundle uses the FlagsV6 codec. Without this a crafted v6 tx parsed but
  then aborted the node in the txid-path expect() when to_librustzcash
  rejected the flag.
- state: fall back to the legacy history-tree entry width on any
  current-width parse error, not just UnexpectedEof. A multi-peak legacy
  record misaligns at the wider width and bincode fails with a varint
  error, which the EOF-only gate turned into an upgrade crash-loop.
- state: serve the empty Ironwood tree only before NU6.3 activation; from
  activation onward a missing tree is a real invariant violation, so fail
  loudly like the Orchard accessor instead of masking corruption.

* refactor(state): thread Ironwood data and trees through named types

- Yield ironwood_shielded_data from the per-transaction version match in
  both update_chain_tip_with_block and revert_chain_with, replacing the
  two hand-synced `if let V6` blocks. A future tx version now cannot
  silently skip Ironwood nullifier tracking in one direction only.
- Pass the note commitment trees to Chain::new and Treestate::new in a
  NoteCommitmentTrees struct instead of adjacent positional Arc<orchard::
  tree::NoteCommitmentTree> arguments (orchard and ironwood share the
  type), so an orchard/ironwood swap is a compile error rather than
  silent tree corruption. Drops two too_many_arguments allows.

* perf(consensus): cut per-transaction network-upgrade and bundle work

- Compute NetworkUpgrade::current once in check_structure_and_network_rules
  and pass it into orchard_value_balance_non_negative and
  coinbase_orchard_component_empty, instead of each rebuilding the
  activation-height map per transaction.
- Store the halo2 Item bundle in an Arc so the eager clone tower-fallback
  makes for every request shares the bundle instead of deep-copying its
  actions and multi-KB proof; add_bundle only needs a reference.

* refactor(chain): deduplicate history-tree rebuild and Ironwood checks

- Share the per-variant InnerHistoryTree rebuild between prune() and the
  Clone impl via rebuilt_inner(), so a new history-tree version adds its
  arm once.
- Test Ironwood-bundle presence with has_ironwood_shielded_data() instead
  of ironwood_actions().count(), which walked the AtLeastOne action list.
- Delegate insert_fake_orchard_shielded_data to fake_v6_orchard_shielded_data.

* fix(state): keep the indexer Spend enum's nullifier imports

The refactor(state) commit dropped sprout/sapling/orchard from the shared
zebra_chain import block (unused without the indexer feature), but the
indexer-only Spend enum and its From impls still reference them, breaking
`cargo check --features indexer` (9 x E0433). Re-add them alongside the
ironwood import, cfg-gated to the indexer feature.

* docs(chain): fix stale v6 Orchard flag comment

The call-site comment still said the v6 Orchard bundle permits
enableCrossAddress; the flag-mask fix reserves it for the Orchard pool
(only Ironwood permits it).

* refactor(chain): imply the v6 Orchard flag codec from the bundle type

Address @arya2 review: tie the flagsOrchard codec to each v6 bundle newtype
via a `V6FlagCodec` associated type (orchard::ShieldedDataV6 -> Flags,
ironwood::ShieldedData -> FlagsV6) so the deserializers imply it instead of
naming it explicitly. No behavior change.

Author: upbqdn

zebra-chain/src/history_tree.rs

@@ -352,7 +352,16 @@ impl NonEmptyHistoryTree {
         // Remove all non-peak entries
         self.peaks.retain(|k, _| peak_pos_set.contains(k));
         // Rebuild tree
-        self.inner = match self.inner {
+        self.inner = self.rebuilt_inner()?;
+        Ok(())
+    }
+
+    /// Rebuilds the inner tree from the cached `network`, `network_upgrade`, `size`, and `peaks`.
+    ///
+    /// Shared by [`Self::prune`] and the [`Clone`] impl, which reconstruct the inner tree
+    /// identically and differ only in how they handle the (practically impossible) rebuild error.
+    fn rebuilt_inner(&self) -> Result<InnerHistoryTree, io::Error> {
+        Ok(match &self.inner {
             InnerHistoryTree::PreOrchard(_) => {
                 InnerHistoryTree::PreOrchard(Tree::<PreOrchard>::new_from_cache(
                     &self.network,
@@ -380,8 +389,7 @@ impl NonEmptyHistoryTree {
                     &Default::default(),
                 )?)
             }
-        };
-        Ok(())
+        })
     }
 
     /// Return the hash of the tree root.
@@ -416,38 +424,9 @@ impl NonEmptyHistoryTree {
 
 impl Clone for NonEmptyHistoryTree {
     fn clone(&self) -> Self {
-        let tree = match self.inner {
-            InnerHistoryTree::PreOrchard(_) => InnerHistoryTree::PreOrchard(
-                Tree::<PreOrchard>::new_from_cache(
-                    &self.network,
-                    self.network_upgrade,
-                    self.size,
-                    &self.peaks,
-                    &Default::default(),
-                )
-                .expect("rebuilding an existing tree should always work"),
-            ),
-            InnerHistoryTree::OrchardOnward(_) => InnerHistoryTree::OrchardOnward(
-                Tree::<OrchardOnward>::new_from_cache(
-                    &self.network,
-                    self.network_upgrade,
-                    self.size,
-                    &self.peaks,
-                    &Default::default(),
-                )
-                .expect("rebuilding an existing tree should always work"),
-            ),
-            InnerHistoryTree::IronwoodOnward(_) => InnerHistoryTree::IronwoodOnward(
-                Tree::<IronwoodOnward>::new_from_cache(
-                    &self.network,
-                    self.network_upgrade,
-                    self.size,
-                    &self.peaks,
-                    &Default::default(),
-                )
-                .expect("rebuilding an existing tree should always work"),
-            ),
-        };
+        let tree = self
+            .rebuilt_inner()
+            .expect("rebuilding an existing tree should always work");
         NonEmptyHistoryTree {
             network: self.network.clone(),
             network_upgrade: self.network_upgrade,

zebra-chain/src/ironwood.rs

@@ -38,11 +38,11 @@ impl From<orchard::Nullifier> for Nullifier {
 
 /// Ironwood shielded data: a v6 Orchard-protocol bundle committed to the Ironwood pool.
 ///
-/// Wraps [`orchard::ShieldedDataV6`] (which itself carries the NU6.3 flag-byte format that permits
-/// the `enableCrossAddress` flag). The Ironwood bundle shares the exact wire format of the v6
-/// Orchard bundle; this newtype keeps the two type-distinct so they cannot be accidentally
-/// interchanged, and so the Ironwood bundle can commit into a separate note commitment tree and
-/// nullifier set.
+/// Wraps [`orchard::ShieldedDataV6`] (the v6 Orchard bundle shape). The Ironwood bundle shares the
+/// exact wire format of the v6 Orchard bundle, but is the only pool that permits the
+/// `enableCrossAddress` flag (bit 2), which the Orchard pool reserves regardless of tx version. This
+/// newtype keeps the two type-distinct so they cannot be accidentally interchanged, and so the
+/// Ironwood bundle can commit into a separate note commitment tree and nullifier set.
 #[derive(Clone, Debug, PartialEq, Eq, Deserialize, Serialize)]
 pub struct ShieldedData(orchard::ShieldedDataV6);
 

zebra-chain/src/orchard/shielded_data.rs

@@ -296,19 +296,19 @@ bitflags! {
         /// `enableCrossAddress` (NU6.3, bit 2): allow output notes to use a different
         /// protocol-level address than the spending key.
         ///
-        /// Reserved (MUST be 0) before NU6.3. Valid only in the NU6.3 flag-byte format (v6
-        /// Orchard and Ironwood bundles); parsed via the `FlagsV6` newtype.
+        /// Reserved (MUST be 0) for the Orchard pool in every tx version. Valid only for the
+        /// Ironwood pool (v6), parsed via the `FlagsV6` newtype.
         const ENABLE_CROSS_ADDRESS = 0b00000100;
     }
 }
 
-/// The Orchard flags of a v6 (NU6.3) Orchard or Ironwood bundle.
+/// The Orchard flags of an Ironwood (v6) bundle.
 ///
-/// Newtype over [`Flags`] whose [`ZcashDeserialize`] impl uses the NU6.3 flag-byte format: bit 2
-/// (`enableCrossAddress`) is valid and only bits 3..7 are reserved. The bare [`Flags`] codec is the
-/// pre-NU6.3 (v5 Orchard) format, where bits 2..7 are all reserved. Encoding the format in the type
-/// keeps the v5 and v6 flag-parsing paths from being confused (parallels
-/// [`ShieldedDataV6`]).
+/// Newtype over [`Flags`] whose [`ZcashDeserialize`] impl uses the NU6.3 Ironwood flag-byte format:
+/// bit 2 (`enableCrossAddress`) is valid and only bits 3..7 are reserved. The bare [`Flags`] codec
+/// is the format for every Orchard-pool bundle (v5 *and* v6), where bits 2..7 are all reserved —
+/// `enableCrossAddress` is permitted only for the Ironwood pool. Encoding the format in the type
+/// keeps the two flag-parsing paths from being confused (parallels [`ShieldedDataV6`]).
 #[derive(Copy, Clone, Debug, PartialEq, Eq)]
 pub struct FlagsV6(Flags);
 
@@ -380,28 +380,29 @@ impl ZcashDeserialize for Flags {
     /// > [NU5 onward] In a version 5 transaction, the reserved bits 2..7 of the flagsOrchard
     /// > field MUST be zero.
     ///
-    /// From NU6.3, the v6 Orchard and Ironwood flag bytes use bit 2 as `enableCrossAddress`, so
-    /// only bits 3..7 are reserved (see [`FlagsV6`]).
+    /// From NU6.3, the Ironwood flag byte uses bit 2 as `enableCrossAddress`, so only bits 3..7 are
+    /// reserved (see [`FlagsV6`]); the Orchard pool keeps bit 2 reserved in every tx version.
     ///
     /// <https://zips.z.cash/protocol/protocol.pdf#txnconsensus>
     fn zcash_deserialize<R: io::Read>(mut reader: R) -> Result<Self, SerializationError> {
-        // The default codec is the pre-NU6.3 format, used by v5 Orchard bundles, where bits 2..7
-        // (including `enableCrossAddress`) are reserved and MUST be zero. v6 Orchard and Ironwood
-        // bundles deserialize via the `FlagsV6` newtype, which permits bit 2.
+        // The default codec is the pre-NU6.3 format, used by v5 *and* v6 Orchard bundles, where
+        // bits 2..7 (including `enableCrossAddress`) are reserved and MUST be zero. Only the Ironwood
+        // bundle deserializes via the `FlagsV6` newtype, which permits bit 2.
         Flags::from_byte(reader.read_u8()?, Flags::PRE_NU6_3_RESERVED)
     }
 }
 
 impl ZcashDeserialize for FlagsV6 {
     /// # Consensus
     ///
-    /// From NU6.3, the v6 Orchard and Ironwood flag bytes use bit 2 as `enableCrossAddress`, so
-    /// only bits 3..7 are reserved and MUST be zero (cf. the v5 rule on [`Flags`]).
+    /// From NU6.3, the Ironwood flag byte uses bit 2 as `enableCrossAddress`, so only bits 3..7 are
+    /// reserved and MUST be zero (cf. the Orchard-pool rule on [`Flags`], which keeps bit 2
+    /// reserved).
     ///
     /// <https://zips.z.cash/protocol/protocol.pdf#txnconsensus>
     fn zcash_deserialize<R: io::Read>(mut reader: R) -> Result<Self, SerializationError> {
-        // The NU6.3 format, used by v6 Orchard and Ironwood bundles: bit 2 (`enableCrossAddress`)
-        // is valid and only bits 3..7 are reserved.
+        // The NU6.3 Ironwood format: bit 2 (`enableCrossAddress`) is valid and only bits 3..7 are
+        // reserved.
         Ok(FlagsV6(Flags::from_byte(
             reader.read_u8()?,
             Flags::NU6_3_RESERVED,

zebra-chain/src/transaction.rs

@@ -324,7 +324,7 @@ impl Transaction {
                     .orchard_flags()
                     .unwrap_or_else(orchard::Flags::empty)
                     .contains(orchard::Flags::ENABLE_SPENDS))
-            || (self.ironwood_actions().count() > 0
+            || (self.has_ironwood_shielded_data()
                 && self
                     .ironwood_flags()
                     .unwrap_or_else(orchard::Flags::empty)
@@ -342,7 +342,7 @@ impl Transaction {
                     .orchard_flags()
                     .unwrap_or_else(orchard::Flags::empty)
                     .contains(orchard::Flags::ENABLE_OUTPUTS))
-            || (self.ironwood_actions().count() > 0
+            || (self.has_ironwood_shielded_data()
                 && self
                     .ironwood_flags()
                     .unwrap_or_else(orchard::Flags::empty)
@@ -369,7 +369,7 @@ impl Transaction {
     ///
     /// Mirrors [`Self::has_enough_orchard_flags`] for the Ironwood pool.
     pub fn has_enough_ironwood_flags(&self) -> bool {
-        if self.ironwood_actions().count() == 0 {
+        if !self.has_ironwood_shielded_data() {
             return true;
         }
         self.ironwood_flags()

zebra-chain/src/transaction/arbitrary.rs

@@ -8,7 +8,6 @@ use reddsa::{orchard::Binding, Signature};
 
 use crate::{
     amount::{self, Amount, NegativeAllowed, NonNegative},
-    at_least_one,
     block::{self, arbitrary::MAX_PARTIAL_CHAIN_BLOCKS},
     orchard,
     parameters::{Network, NetworkUpgrade},
@@ -1078,28 +1077,12 @@ pub fn transactions_from_blocks<'a>(
 pub fn insert_fake_orchard_shielded_data(
     transaction: &mut Transaction,
 ) -> &mut orchard::ShieldedData {
-    // Create a dummy action
-    let mut runner = TestRunner::default();
-    let dummy_action = orchard::Action::arbitrary()
-        .new_tree(&mut runner)
-        .unwrap()
-        .current();
-
-    // Pair the dummy action with a fake signature
-    let dummy_authorized_action = orchard::AuthorizedAction {
-        action: dummy_action,
-        spend_auth_sig: Signature::from([0u8; 64]),
-    };
-
-    // Place the dummy action inside the Orchard shielded data
-    let dummy_shielded_data = orchard::ShieldedData {
-        flags: orchard::Flags::empty(),
-        value_balance: Amount::try_from(0).expect("invalid transaction amount"),
-        shared_anchor: orchard::tree::Root::default(),
-        proof: Halo2Proof(vec![]),
-        actions: at_least_one![dummy_authorized_action],
-        binding_sig: Signature::from([0u8; 64]),
-    };
+    // A single-action dummy bundle with no flags and a zero value balance.
+    let dummy_shielded_data = fake_v6_orchard_shielded_data(
+        orchard::Flags::empty(),
+        Amount::try_from(0).expect("invalid transaction amount"),
+        1,
+    );
 
     // Replace the shielded data in the transaction
     match transaction {

zebra-chain/src/transaction/serialize.rs

@@ -419,15 +419,45 @@ impl ZcashSerialize for orchard::ShieldedData {
     }
 }
 
-// A v6 (NU6.3) Orchard or Ironwood bundle differs from a v5 Orchard bundle only in its flag-byte
-// format on *deserialization* (the NU6.3 format permits `enableCrossAddress`). It encodes
-// identically on the wire (the flag byte is written as-is), so the v6 Orchard and Ironwood
-// (de)serializers below delegate to the v5 Orchard bundle codec, only wrapping/unwrapping their
-// newtypes (`orchard::ShieldedDataV6` and `ironwood::ShieldedData`).
+/// The `flagsOrchard` codec a v6 Orchard-protocol bundle newtype uses on deserialization.
+///
+/// A v6 Orchard or Ironwood bundle encodes identically on the wire to a v5 Orchard bundle (the flag
+/// byte is written as-is); the pools differ only in the reserved-bit rule applied to `flagsOrchard`.
+/// The `enableCrossAddress` bit (bit 2) is permitted only for the Ironwood pool, and is reserved
+/// (MUST be 0) for the Orchard pool regardless of tx version — matching
+/// `orchard::bundle::Flags::from_byte`, which rejects bit 2 for `ValuePool::Orchard`. Tying the
+/// codec to the bundle type lets the (de)serializers below imply it instead of naming it explicitly.
+trait V6FlagCodec {
+    /// The flag codec: `orchard::Flags` reserves bit 2, `orchard::FlagsV6` permits it.
+    type Codec: ZcashDeserialize + Into<orchard::Flags>;
+}
+
+impl V6FlagCodec for orchard::ShieldedDataV6 {
+    // The v6 Orchard bundle parses with the pre-NU6.3 codec, exactly like v5.
+    type Codec = orchard::Flags;
+}
+
+impl V6FlagCodec for ironwood::ShieldedData {
+    // Only the Ironwood bundle permits `enableCrossAddress`.
+    type Codec = orchard::FlagsV6;
+}
+
+/// Deserializes the shared Orchard-protocol bundle body of a v6 bundle newtype `T`, using the flag
+/// codec [implied by `T`](V6FlagCodec) rather than one named at the call site.
+fn deserialize_v6_orchard_shielded_data<R, T>(
+    reader: R,
+) -> Result<Option<orchard::ShieldedData>, SerializationError>
+where
+    R: io::Read,
+    T: V6FlagCodec,
+{
+    deserialize_orchard_shielded_data::<R, T::Codec>(reader)
+}
+
 impl ZcashDeserialize for Option<orchard::ShieldedDataV6> {
     fn zcash_deserialize<R: io::Read>(reader: R) -> Result<Self, SerializationError> {
         Ok(
-            deserialize_orchard_shielded_data::<R, orchard::FlagsV6>(reader)?
+            deserialize_v6_orchard_shielded_data::<R, orchard::ShieldedDataV6>(reader)?
                 .map(orchard::ShieldedDataV6::new),
         )
     }
@@ -442,7 +472,8 @@ impl ZcashSerialize for Option<orchard::ShieldedDataV6> {
 impl ZcashDeserialize for Option<ironwood::ShieldedData> {
     fn zcash_deserialize<R: io::Read>(reader: R) -> Result<Self, SerializationError> {
         Ok(
-            Option::<orchard::ShieldedDataV6>::zcash_deserialize(reader)?
+            deserialize_v6_orchard_shielded_data::<R, ironwood::ShieldedData>(reader)?
+                .map(orchard::ShieldedDataV6::new)
                 .map(ironwood::ShieldedData::new),
         )
     }
@@ -1155,9 +1186,9 @@ impl ZcashDeserialize for Transaction {
 
                 // A bundle of fields denoted in the spec as `nActionsOrchard`, `vActionsOrchard`,
                 // `flagsOrchard`,`valueBalanceOrchard`, `anchorOrchard`, `sizeProofsOrchard`,
-                // `proofsOrchard`, `vSpendAuthSigsOrchard`, and `bindingSigOrchard`. The
-                // `ShieldedDataV6` codec uses the NU6.3 flag-byte format (`enableCrossAddress`
-                // permitted).
+                // `proofsOrchard`, `vSpendAuthSigsOrchard`, and `bindingSigOrchard`. The v6 Orchard
+                // bundle reserves the `enableCrossAddress` bit (like v5); only the Ironwood bundle
+                // below permits it.
                 let orchard_shielded_data = (&mut limited_reader)
                     .zcash_deserialize_into::<Option<orchard::ShieldedDataV6>>()?;
 
@@ -1166,6 +1197,12 @@ impl ZcashDeserialize for Transaction {
                 let ironwood_shielded_data = (&mut limited_reader)
                     .zcash_deserialize_into::<Option<ironwood::ShieldedData>>()?;
 
+                // Unlike the v5 arm, the v6 arm does not round-trip through `to_librustzcash` here:
+                // that would drive librustzcash's proof parser (which rejects the structurally-fake
+                // proofs the test helpers produce), coupling wire deserialization to proof parsing.
+                // The `enableCrossAddress` divergence that would otherwise reach the txid-path
+                // `expect(...)` is already rejected above, inside `orchard::Flags::from_byte`, before
+                // this transaction is constructed.
                 Ok(Transaction::V6 {
                     network_upgrade,
                     lock_time,

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

@@ -1122,6 +1122,56 @@ fn v6_transaction_with_bundles_round_trips() {
     assert_eq!(tx, tx2);
 }
 
+/// The `enableCrossAddress` flag (bit 2) is permitted only for the Ironwood pool: a v6 Orchard
+/// bundle carrying it MUST be rejected at deserialization (matching `orchard::Flags::from_byte`,
+/// which reserves bit 2 for `ValuePool::Orchard` in every tx version), while the same flag on the
+/// Ironwood bundle round-trips.
+///
+/// The Orchard case is the wire-layer guard: without it, a crafted bundle deserializes and then
+/// aborts the node in the txid-path `expect(...)` when `to_librustzcash` rejects the flag.
+#[test]
+fn v6_orchard_bundle_rejects_cross_address_flag_on_the_wire() {
+    use crate::ironwood;
+    use crate::orchard::{Flags, ShieldedDataV6};
+
+    let _init_guard = zebra_test::init();
+    let zero = Amount::try_from(0).expect("zero is a valid amount");
+
+    // A v6 Orchard bundle with `enableCrossAddress` serializes (the flag byte is written as-is) but
+    // MUST NOT deserialize.
+    let orchard = ShieldedDataV6::new(arbitrary::fake_v6_orchard_shielded_data(
+        Flags::ENABLE_SPENDS | Flags::ENABLE_CROSS_ADDRESS,
+        zero,
+        1,
+    ));
+    let tx = arbitrary::fake_v6_transaction(NetworkUpgrade::Nu6_3, Some(orchard), None);
+    let bytes = tx
+        .zcash_serialize_to_vec()
+        .expect("v6 transaction serializes");
+    let result: Result<Transaction, _> = bytes.zcash_deserialize_into();
+    assert!(
+        result.is_err(),
+        "a v6 Orchard bundle with enableCrossAddress must be rejected on the wire",
+    );
+
+    // The same flag on the Ironwood bundle is valid and round-trips.
+    let ironwood = ironwood::ShieldedData::new(ShieldedDataV6::new(
+        arbitrary::fake_v6_orchard_shielded_data(
+            Flags::ENABLE_SPENDS | Flags::ENABLE_CROSS_ADDRESS,
+            zero,
+            1,
+        ),
+    ));
+    let tx = arbitrary::fake_v6_transaction(NetworkUpgrade::Nu6_3, None, Some(ironwood));
+    let bytes = tx
+        .zcash_serialize_to_vec()
+        .expect("v6 transaction serializes");
+    let tx2: Transaction = bytes
+        .zcash_deserialize_into()
+        .expect("a v6 Ironwood bundle with enableCrossAddress round-trips");
+    assert_eq!(tx, tx2);
+}
+
 #[test]
 fn test_coinbase_script() -> Result<()> {
     let _init_guard = zebra_test::init();