Use fixed genesis coinbase data in generated genesis blocks (#2568)

* Return an error if genesis transparent coinbase data is invalid

This error prevents cryptic errors during genesis coinbase deserialization.

And fix and improve documentation.

* Use the fixed Zcash constant for generated genesis coinbase data

This change is required, because genesis transactions do not have a
coinbase height in their coinbase data.

zebra-chain/src/transparent.rs

@@ -9,6 +9,7 @@ mod utxo;
 
 pub use address::Address;
 pub use script::Script;
+pub use serialize::GENESIS_COINBASE_DATA;
 pub use utxo::{
     new_ordered_outputs, new_outputs, utxos_from_ordered_utxos, CoinbaseSpendRestriction,
     OrderedUtxo, Utxo,

zebra-chain/src/transparent/arbitrary.rs

@@ -2,7 +2,7 @@ use proptest::{arbitrary::any, collection::vec, prelude::*};
 
 use crate::{block, LedgerState};
 
-use super::{CoinbaseData, Input, OutPoint, Script};
+use super::{CoinbaseData, Input, OutPoint, Script, GENESIS_COINBASE_DATA};
 
 impl Input {
     /// Construct a strategy for creating valid-ish vecs of Inputs.
@@ -25,7 +25,11 @@ impl Arbitrary for Input {
             (vec(any::<u8>(), 0..95), any::<u32>())
                 .prop_map(move |(data, sequence)| Input::Coinbase {
                     height,
-                    data: CoinbaseData(data),
+                    data: if height == block::Height(0) {
+                        CoinbaseData(GENESIS_COINBASE_DATA.to_vec())
+                    } else {
+                        CoinbaseData(data)
+                    },
                     sequence,
                 })
                 .boxed()

zebra-chain/src/transparent/serialize.rs

@@ -17,7 +17,7 @@ use super::{CoinbaseData, Input, OutPoint, Output, Script};
 ///
 /// Zcash uses the same coinbase data for the Mainnet, Testnet, and Regtest
 /// genesis blocks.
-const GENESIS_COINBASE_DATA: [u8; 77] = [
+pub const GENESIS_COINBASE_DATA: [u8; 77] = [
     4, 255, 255, 7, 31, 1, 4, 69, 90, 99, 97, 115, 104, 48, 98, 57, 99, 52, 101, 101, 102, 56, 98,
     55, 99, 99, 52, 49, 55, 101, 101, 53, 48, 48, 49, 101, 51, 53, 48, 48, 57, 56, 52, 98, 54, 102,
     101, 97, 51, 53, 54, 56, 51, 97, 55, 99, 97, 99, 49, 52, 49, 97, 48, 52, 51, 99, 52, 50, 48,
@@ -48,6 +48,15 @@ impl ZcashDeserialize for OutPoint {
 // unrepresentable, we need just enough parsing of Bitcoin scripts to parse the
 // coinbase height and split off the rest of the (inert) coinbase data.
 
+// Starting at Network Upgrade 5, coinbase transactions also encode the block
+// height in the expiry height field. But Zebra does not use this field to
+// determine the coinbase height, because it is not present in older network
+// upgrades.
+
+/// Split `data` into a block height and remaining miner-controlled coinbase data.
+///
+/// The height may consume `0..=5` bytes at the stat of the coinbase data.
+/// The genesis block does not include an encoded coinbase height.
 fn parse_coinbase_height(
     mut data: Vec<u8>,
 ) -> Result<(block::Height, CoinbaseData), SerializationError> {
@@ -77,10 +86,14 @@ fn parse_coinbase_height(
         )),
         // The genesis block does not encode the block height by mistake; special case it.
         // The first five bytes are [4, 255, 255, 7, 31], the little-endian encoding of
-        // 520_617_983.  This is lucky because it means we can special-case the genesis block
-        // while remaining below the maximum `block::Height` of 500_000_000 forced by `LockTime`.
-        // While it's unlikely this code will ever process a block height that high, this means
-        // we don't need to maintain a cascade of different invariants for allowable heights.
+        // 520_617_983.
+        //
+        // In the far future, Zcash might reach this height, and the miner might use the
+        // same coinbase data as the genesis block. So we need an updated consensus rule
+        // to handle this edge case.
+        //
+        // TODO: update this check based on the consensus rule changes in
+        //       https://github.com/zcash/zips/issues/540
         (Some(0x04), _) if data[..] == GENESIS_COINBASE_DATA[..] => {
             Ok((Height(0), CoinbaseData(data)))
         }
@@ -103,6 +116,7 @@ fn parse_coinbase_height(
     }
 }
 
+/// Return the encoded length of `height`, as a prefix to the coinbase data.
 fn coinbase_height_len(height: block::Height) -> usize {
     // We can't write this as a match statement on stable until exclusive range
     // guards are stabilized.
@@ -123,13 +137,41 @@ fn coinbase_height_len(height: block::Height) -> usize {
     }
 }
 
-fn write_coinbase_height<W: io::Write>(height: block::Height, mut w: W) -> Result<(), io::Error> {
+/// Encode `height` into a block height, as a prefix of the coinbase data.
+/// Does not write `coinbase_data`.
+///
+/// The height may produce `0..=5` initial bytes of coinbase data.
+///
+/// # Errors
+///
+/// Returns an error if the coinbase height is zero,
+/// and the `coinbase_data` does not match the Zcash mainnet and testnet genesis coinbase data.
+/// (They are identical.)
+///
+/// This check is required, because the genesis block does not include an encoded
+/// coinbase height,
+fn write_coinbase_height<W: io::Write>(
+    height: block::Height,
+    coinbase_data: &CoinbaseData,
+    mut w: W,
+) -> Result<(), io::Error> {
     // We can't write this as a match statement on stable until exclusive range
     // guards are stabilized.
     // The Bitcoin encoding requires that the most significant byte is below 0x80,
     // so the ranges run up to 2^{n-1} rather than 2^n.
     if let 0 = height.0 {
-        // Genesis block does not include height.
+        // The genesis block's coinbase data does not have a height prefix.
+        // So we return an error if the entire coinbase data doesn't match genesis.
+        // (If we don't do this check, then deserialization will fail.)
+        //
+        // TODO: update this check based on the consensus rule changes in
+        //       https://github.com/zcash/zips/issues/540
+        if coinbase_data.0 != GENESIS_COINBASE_DATA {
+            return Err(io::Error::new(
+                io::ErrorKind::Other,
+                "invalid genesis coinbase data",
+            ));
+        }
     } else if let h @ 1..=16 = height.0 {
         w.write_u8(0x50 + (h as u8))?;
     } else if let h @ 17..=127 = height.0 {
@@ -153,6 +195,16 @@ fn write_coinbase_height<W: io::Write>(height: block::Height, mut w: W) -> Resul
 }
 
 impl ZcashSerialize for Input {
+    /// Serialize this transparent input.
+    ///
+    /// # Errors
+    ///
+    /// Returns an error if the coinbase height is zero,
+    /// and the coinbase data does not match the Zcash mainnet and testnet genesis coinbase data.
+    /// (They are identical.)
+    ///
+    /// This check is required, because the genesis block does not include an encoded
+    /// coinbase height,
     fn zcash_serialize<W: io::Write>(&self, mut writer: W) -> Result<(), io::Error> {
         match self {
             Input::PrevOut {
@@ -174,7 +226,7 @@ impl ZcashSerialize for Input {
                 let height_len = coinbase_height_len(*height);
                 let total_len = height_len + data.as_ref().len();
                 writer.write_compactsize(total_len as u64)?;
-                write_coinbase_height(*height, &mut writer)?;
+                write_coinbase_height(*height, data, &mut writer)?;
                 writer.write_all(data.as_ref())?;
                 writer.write_u32::<LittleEndian>(*sequence)?;
             }