Reject V5 transactions before NU5 activation (#2285)

* Add a `Transaction::version` getter

Returns the version of the transaction as a `u32`.

* Add `Transaction::is_overwintered` helper method

Returns if the `fOverwintered` flag should be set for the transaction's
version.

* Use new helpers to serialize transaction version

Reduce the repeated code and make it less error-prone with future
changes.

* Add getter methods to `transaction::Request` type

Refactor to move the type deconstruction code into the `Request` type.
The main objective is to make it easier to split the call handler into
methods that receive the request directly.

* Refactor to create `verify_v4_transaction` helper

Split the code specific to V4 transactions into a separate helper
method.

* Create `verify_v5_transaction` helper method

Prepare a separate method to have the validation code.

* Add `UnsupportedByNetworkUpgrade` error variant

An error for when a transaction's version isn't supported by the network
upgrade of the block it's included or for the current network upgrade if
the transaction is for the mempool.

* Verify a V5 transaction's network upgrade

For now, only NU5 supports V5 transactions.

* Test that V5 transaction is rejected on Canopy

Create a fake V5 transaction and try to verify it using a block height
from Canopy's activation. The verifier should reject the transaction
with an error saying that the network upgrade does not support that
transaction version.

* Test if V5 tx. is accepted after NU5 activation

Create a fake V5 transaction and pretend it is placed in a block that
has a height after the NU5 activation. The test should succeed, but
since the NU5 activation height has not been specified yet (neither for
the testnet nor the mainnet), for now this test is marked as
`should_panic`.

* Add `TODO` comment to the code

Add more detail to what's left to do, and link to the appropriate PRs.

* Use `u32` to store transaction version

Use a type consistent with how the version is specified.

Co-authored-by: teor <[email protected]>

Co-authored-by: teor <[email protected]>

zebra-chain/src/transaction.rs

@@ -151,6 +151,25 @@ impl Transaction {
 
     // header
 
+    /// Return if the `fOverwintered` flag of this transaction is set.
+    pub fn is_overwintered(&self) -> bool {
+        match self {
+            Transaction::V1 { .. } | Transaction::V2 { .. } => false,
+            Transaction::V3 { .. } | Transaction::V4 { .. } | Transaction::V5 { .. } => true,
+        }
+    }
+
+    /// Return the version of this transaction.
+    pub fn version(&self) -> u32 {
+        match self {
+            Transaction::V1 { .. } => 1,
+            Transaction::V2 { .. } => 2,
+            Transaction::V3 { .. } => 3,
+            Transaction::V4 { .. } => 4,
+            Transaction::V5 { .. } => 5,
+        }
+    }
+
     /// Get this transaction's lock time.
     pub fn lock_time(&self) -> LockTime {
         match self {

zebra-chain/src/transaction/serialize.rs

@@ -371,13 +371,19 @@ impl ZcashSerialize for Transaction {
         //
         // Since we checkpoint on Canopy activation, we won't ever need
         // to check the smaller pre-Sapling transaction size limit.
+
+        // header: Write version and set the fOverwintered bit if necessary
+        let overwintered_flag = if self.is_overwintered() { 1 << 31 } else { 0 };
+        let version = overwintered_flag | self.version();
+
+        writer.write_u32::<LittleEndian>(version)?;
+
         match self {
             Transaction::V1 {
                 inputs,
                 outputs,
                 lock_time,
             } => {
-                writer.write_u32::<LittleEndian>(1)?;
                 inputs.zcash_serialize(&mut writer)?;
                 outputs.zcash_serialize(&mut writer)?;
                 lock_time.zcash_serialize(&mut writer)?;
@@ -388,7 +394,6 @@ impl ZcashSerialize for Transaction {
                 lock_time,
                 joinsplit_data,
             } => {
-                writer.write_u32::<LittleEndian>(2)?;
                 inputs.zcash_serialize(&mut writer)?;
                 outputs.zcash_serialize(&mut writer)?;
                 lock_time.zcash_serialize(&mut writer)?;
@@ -405,8 +410,6 @@ impl ZcashSerialize for Transaction {
                 expiry_height,
                 joinsplit_data,
             } => {
-                // Write version 3 and set the fOverwintered bit.
-                writer.write_u32::<LittleEndian>(3 | (1 << 31))?;
                 writer.write_u32::<LittleEndian>(OVERWINTER_VERSION_GROUP_ID)?;
                 inputs.zcash_serialize(&mut writer)?;
                 outputs.zcash_serialize(&mut writer)?;
@@ -426,8 +429,6 @@ impl ZcashSerialize for Transaction {
                 sapling_shielded_data,
                 joinsplit_data,
             } => {
-                // Write version 4 and set the fOverwintered bit.
-                writer.write_u32::<LittleEndian>(4 | (1 << 31))?;
                 writer.write_u32::<LittleEndian>(SAPLING_VERSION_GROUP_ID)?;
                 inputs.zcash_serialize(&mut writer)?;
                 outputs.zcash_serialize(&mut writer)?;
@@ -492,8 +493,6 @@ impl ZcashSerialize for Transaction {
                 // Transaction V5 spec:
                 // https://zips.z.cash/protocol/nu5.pdf#txnencodingandconsensus
 
-                // header: Write version 5 and set the fOverwintered bit
-                writer.write_u32::<LittleEndian>(5 | (1 << 31))?;
                 writer.write_u32::<LittleEndian>(TX_V5_VERSION_GROUP_ID)?;
 
                 // header: Write the nConsensusBranchId

zebra-consensus/src/error.rs

@@ -47,6 +47,9 @@ pub enum TransactionError {
     #[error("transaction version number MUST be >= 4")]
     WrongVersion,
 
+    #[error("transaction version {0} not supported by the network upgrade {1:?}")]
+    UnsupportedByNetworkUpgrade(u32, zebra_chain::parameters::NetworkUpgrade),
+
     #[error("must have at least one input: transparent, shielded spend, or joinsplit")]
     NoInputs,