feat: remove recovery byte (#4301)

Description
---
Removed recovery byte from the output features due to the recently added
encrypted value to a transaction output being able to fulfil the original
intent of the recovery byte, i.e. to let a wallet pinpoint the UTXO's it
owns.

Motivation and Context
---
Recovery byte is not needed anymore.

How Has This Been Tested?
---
Unit tests
Integration tests

.github/workflows/ci.yml

@@ -217,12 +217,12 @@ jobs:
         uses: actions-rs/cargo@v1
         with:
           command: test
-          args: --no-run --locked --all-features
+          args: --no-run --locked --all-features --release
       - name: cargo test
         uses: actions-rs/cargo@v1
         with:
           command: test
-          args: -v --all-features
+          args: -v --all-features --release
   # Allows other workflows to know the PR number
   artifacts:
     name: test

applications/tari_app_grpc/proto/types.proto

@@ -218,9 +218,6 @@ message OutputFeatures {
     // require a min maturity of the Coinbase_lock_height, this should be checked on receiving new blocks.
     uint64 maturity = 3;
     bytes metadata = 4;
-    // The recovery byte - not consensus critical - can help reduce the bandwidth with wallet recovery or in other
-    // instances when a wallet needs to request the complete UTXO set from a base node.
-    uint32 recovery_byte = 5;
     SideChainFeatures sidechain_features = 6;
     bytes unique_id = 7;
 

applications/tari_app_grpc/src/conversions/output_features.rs

@@ -71,7 +71,6 @@ impl TryFrom<grpc::OutputFeatures> for OutputFeatures {
             )?,
             OutputType::from_byte(output_type).ok_or_else(|| "Invalid or unrecognised output type".to_string())?,
             features.maturity,
-            u8::try_from(features.recovery_byte).map_err(|_| "Invalid recovery byte: overflowed u8")?,
             features.metadata,
             unique_id,
             sidechain_features,
@@ -92,7 +91,6 @@ impl From<OutputFeatures> for grpc::OutputFeatures {
             maturity: features.maturity,
             metadata: features.metadata,
             unique_id: features.unique_id.unwrap_or_default(),
-            recovery_byte: u32::from(features.recovery_byte),
             sidechain_features: features.sidechain_features.map(|v| *v).map(Into::into),
 
             // TODO: Deprecated

applications/tari_explorer/partials/OutputFeatures.hbs

@@ -30,7 +30,6 @@ Contract constitution change acceptance
   </dd>
   <dt>Maturity</dt><dd>{{maturity}}</dd>
   {{#if metadata.length}}<dt>Metadata</dt><dd>{{json metadata}}</dd>{{/if}}
-  <dt>Recovery byte</dt><dd>{{recovery_byte}}</dd>
   {{#if sidechain_features}}
     <dt>Sidechain Features</dt>
     {{>SideChainFeatures sidechain_features}}

base_layer/core/src/blocks/genesis_block.rs

@@ -242,7 +242,6 @@ fn get_dibbler_genesis_block_raw() -> Block {
                 version: OutputFeaturesVersion::get_current_version(),
                 output_type: OutputType::Coinbase,
                 maturity: 60,
-                recovery_byte: 0,
                 metadata: Vec::new(),
                 unique_id: None,
                 sidechain_features: None,

base_layer/core/src/chain_storage/tests/blockchain_database.rs

@@ -735,7 +735,7 @@ mod fetch_utxo_by_unique_id {
         // Height 1
         let (blocks, outputs) = add_many_chained_blocks(1, &db);
 
-        let mut features = OutputFeatures {
+        let features = OutputFeatures {
             output_type: OutputType::MintNonFungible,
             parent_public_key: Some(asset_pk.clone()),
             unique_id: Some(unique_id.clone()),
@@ -746,9 +746,8 @@ mod fetch_utxo_by_unique_id {
             to: vec![500 * T],
             fee: 5.into(),
             lock: 0,
-            features: features.clone()
+            features: features
         )]);
-        features.set_recovery_byte(tx_outputs[0].features.recovery_byte);
 
         let asset_utxo1 = tx_outputs.iter().find(|o| o.features == features).unwrap();
 
@@ -763,7 +762,6 @@ mod fetch_utxo_by_unique_id {
             .fetch_utxo_by_unique_id(Some(asset_pk.clone()), unique_id.clone(), None)
             .unwrap()
             .unwrap();
-        features.set_recovery_byte(info.output.as_transaction_output().unwrap().features.recovery_byte);
         assert_eq!(info.output.as_transaction_output().unwrap().features, features);
         let expected_commitment =
             CommitmentFactory::default().commit_value(&asset_utxo1.spending_key, asset_utxo1.value.as_u64());
@@ -772,7 +770,7 @@ mod fetch_utxo_by_unique_id {
             expected_commitment
         );
 
-        let mut features = OutputFeatures {
+        let features = OutputFeatures {
             parent_public_key: Some(asset_pk.clone()),
             unique_id: Some(unique_id.clone()),
             ..Default::default()
@@ -784,7 +782,6 @@ mod fetch_utxo_by_unique_id {
             lock: 0,
             features: features
         )]);
-        features.set_recovery_byte(tx_outputs[0].features.recovery_byte);
 
         let asset_utxo2 = tx_outputs.iter().find(|o| o.features == features).unwrap();
 

base_layer/core/src/covenants/fields.rs

@@ -606,7 +606,7 @@ mod test {
 
             #[test]
             fn it_retrieves_the_value_as_ref() {
-                let mut features = OutputFeatures {
+                let features = OutputFeatures {
                     maturity: 42,
                     ..Default::default()
                 };
@@ -616,7 +616,6 @@ mod test {
                 })
                 .pop()
                 .unwrap();
-                features.set_recovery_byte(output.features.recovery_byte);
                 let r = OutputField::Features.get_field_value_ref::<OutputFeatures>(&output);
                 assert_eq!(*r.unwrap(), features);
             }

base_layer/core/src/proto/transaction.proto

@@ -91,9 +91,6 @@ message OutputFeatures {
     // require a min maturity of the Coinbase_lock_height, this should be checked on receiving new blocks.
     uint64 maturity = 3;
     bytes metadata = 4;
-    // The recovery byte - not consensus critical - can help reduce the bandwidth with wallet recovery or in other
-    // instances when a wallet needs to request the complete UTXO set from a base node.
-    uint32 recovery_byte = 5;
     SideChainFeatures sidechain_features = 6;
 
 

base_layer/core/src/proto/transaction.rs

@@ -328,7 +328,6 @@ impl TryFrom<proto::types::OutputFeatures> for OutputFeatures {
             )?,
             OutputType::from_byte(flags).ok_or_else(|| "Invalid or unrecognised output type".to_string())?,
             features.maturity,
-            u8::try_from(features.recovery_byte).map_err(|_| "Invalid recovery byte: overflowed u8")?,
             features.metadata,
             unique_id,
             sidechain_features,
@@ -357,7 +356,6 @@ impl From<OutputFeatures> for proto::types::OutputFeatures {
             sidechain_checkpoint: features.sidechain_checkpoint.map(|s| s.into()),
             version: features.version as u32,
             committee_definition: features.committee_definition.map(|c| c.into()),
-            recovery_byte: u32::from(features.recovery_byte),
             sidechain_features: features.sidechain_features.map(|v| *v).map(Into::into),
         }
     }

base_layer/core/src/transactions/coinbase_builder.rs

@@ -195,8 +195,7 @@ impl CoinbaseBuilder {
             .factories
             .commitment
             .commit_value(&spending_key, total_reward.as_u64());
-        let recovery_byte = OutputFeatures::create_unique_recovery_byte(&commitment, self.rewind_data.as_ref());
-        let output_features = OutputFeatures::create_coinbase(height + constants.coinbase_lock_height(), recovery_byte);
+        let output_features = OutputFeatures::create_coinbase(height + constants.coinbase_lock_height());
         let excess = self.factories.commitment.commit_value(&spending_key, 0);
         let kernel_features = KernelFeatures::create_coinbase();
         let metadata = TransactionMetadata::default();
@@ -374,7 +373,6 @@ mod test {
 
         let rewind_data = RewindData {
             rewind_blinding_key: rewind_blinding_key.clone(),
-            recovery_byte_key: PrivateKey::random(&mut OsRng),
             encryption_key: PrivateKey::random(&mut OsRng),
         };
 

base_layer/core/src/transactions/test_helpers.rs

@@ -119,10 +119,7 @@ impl Default for UtxoTestParams {
         Self {
             value: 10.into(),
             script: script![Nop],
-            features: OutputFeatures {
-                recovery_byte: u8::MAX,
-                ..Default::default()
-            },
+            features: OutputFeatures::default(),
             input_data: None,
             covenant: Covenant::default(),
             output_version: None,
@@ -153,7 +150,6 @@ impl TestParams {
             transaction_weight: TransactionWeight::v2(),
             rewind_data: RewindData {
                 rewind_blinding_key: PrivateKey::random(&mut OsRng),
-                recovery_byte_key: PrivateKey::random(&mut OsRng),
                 encryption_key: PrivateKey::random(&mut OsRng),
             },
         }
@@ -175,8 +171,6 @@ impl TestParams {
         let commitment = self
             .commitment_factory
             .commit_value(&self.spend_key, params.value.as_u64());
-        let updated_features =
-            OutputFeatures::features_with_updated_recovery_byte(&commitment, rewind_data, &params.features);
 
         let encrypted_value = if let Some(rewind_data) = rewind_data {
             EncryptedValue::encrypt_value(&rewind_data.encryption_key, &commitment, params.value).unwrap()
@@ -188,7 +182,7 @@ impl TestParams {
             params.value,
             &self.spend_key,
             &params.script,
-            &updated_features,
+            &params.features,
             &self.sender_offset_private_key,
             &params.covenant,
             &encrypted_value,
@@ -201,7 +195,7 @@ impl TestParams {
                 .unwrap_or_else(TransactionOutputVersion::get_current_version),
             params.value,
             self.spend_key.clone(),
-            updated_features,
+            params.features,
             params.script.clone(),
             params
                 .input_data
@@ -215,38 +209,6 @@ impl TestParams {
         )
     }
 
-    pub fn update_unblinded_output_with_updated_output_features(
-        &self,
-        uo: UnblindedOutput,
-        updated_features: OutputFeatures,
-    ) -> UnblindedOutput {
-        let metadata_signature = TransactionOutput::create_final_metadata_signature(
-            TransactionOutputVersion::get_current_version(),
-            uo.value,
-            &uo.spending_key,
-            &uo.script,
-            &updated_features,
-            &self.sender_offset_private_key,
-            &uo.covenant,
-            &uo.encrypted_value,
-        )
-        .unwrap();
-
-        UnblindedOutput::new_current_version(
-            uo.value,
-            uo.spending_key.clone(),
-            updated_features,
-            uo.script,
-            uo.input_data.clone(),
-            uo.script_private_key.clone(),
-            uo.sender_offset_public_key.clone(),
-            metadata_signature,
-            uo.script_lock_height,
-            uo.covenant,
-            uo.encrypted_value,
-        )
-    }
-
     pub fn get_script_public_key(&self) -> PublicKey {
         PublicKey::from_secret_key(&self.script_private_key)
     }
@@ -331,7 +293,7 @@ pub fn create_unblinded_coinbase(test_params: &TestParams, height: u64) -> Unbli
     let constants = rules.consensus_constants(height);
     test_params.create_unblinded_output(UtxoTestParams {
         value: rules.get_block_reward_at(height),
-        features: OutputFeatures::create_coinbase(height + constants.coinbase_lock_height(), 0x00),
+        features: OutputFeatures::create_coinbase(height + constants.coinbase_lock_height()),
         ..Default::default()
     })
 }
@@ -364,14 +326,6 @@ pub fn create_unblinded_output_with_rewind_data(
     })
 }
 
-pub fn update_unblinded_output_with_updated_output_features(
-    test_params: &TestParams,
-    uo: UnblindedOutput,
-    updated_features: OutputFeatures,
-) -> UnblindedOutput {
-    test_params.update_unblinded_output_with_updated_output_features(uo, updated_features)
-}
-
 /// The tx macro is a convenience wrapper around the [create_tx] function, making the arguments optional and explicit
 /// via keywords.
 #[macro_export]
@@ -719,11 +673,6 @@ pub fn create_stx_protocol(schema: TransactionSchema) -> (SenderTransactionProto
     }
     for mut utxo in schema.to_outputs {
         let test_params = TestParams::new();
-        let commitment = factories
-            .commitment
-            .commit_value(&utxo.spending_key, utxo.value.as_u64());
-        let recovery_byte = OutputFeatures::create_unique_recovery_byte(&commitment, None);
-        utxo.features.set_recovery_byte(recovery_byte);
         utxo.metadata_signature = TransactionOutput::create_final_metadata_signature(
             output_version,
             utxo.value,
@@ -749,14 +698,7 @@ pub fn create_stx_protocol(schema: TransactionSchema) -> (SenderTransactionProto
 
     let script = script!(Nop);
     let covenant = Covenant::default();
-    let commitment = factories
-        .commitment
-        .commit_value(&test_params_change_and_txn.change_spend_key, change.as_u64());
-    let recovery_byte = OutputFeatures::create_unique_recovery_byte(&commitment, None);
-    let change_features = OutputFeatures {
-        recovery_byte,
-        ..Default::default()
-    };
+    let change_features = OutputFeatures::default();
 
     let encrypted_value = EncryptedValue::default();
 
@@ -827,22 +769,20 @@ pub fn create_utxo(
     let commitment = factories.commitment.commit_value(&keys.k, value.into());
     let proof = factories.range_proof.construct_proof(&keys.k, value.into()).unwrap();
 
-    let updated_features = OutputFeatures::features_with_updated_recovery_byte(&commitment, None, features);
-
     let metadata_sig = TransactionOutput::create_final_metadata_signature(
         TransactionOutputVersion::get_current_version(),
         value,
         &keys.k,
         script,
-        &updated_features,
+        features,
         &offset_keys.k,
         covenant,
         &EncryptedValue::default(),
     )
     .unwrap();
 
     let utxo = TransactionOutput::new_current_version(
-        updated_features,
+        features.clone(),
         commitment,
         proof.into(),
         script.clone(),