fix(core)!: include issuer public key in contract id hash (#4239)

Description
---
- includes issuer public key ~~and commitment~~ in contract hash
- update command "wizard" code to fetch contract id from resulting transaction 
- simplify FixedString code
- removes unused Hashable trait impls

Motivation and Context
---
~~The contract ID should be a hash of all contract definition data. Adding the commitment allows contracts with the same name label to exist on the blockchain at the same time.~~
The contract name functions as a label and does not have to be globally unique. This PR essentially makes the
<issuer_pk, contract_name> pair unique. 

How Has This Been Tested?
---
Existing tests, manually

applications/tari_app_grpc/src/conversions/sidechain_features.rs

@@ -27,7 +27,7 @@ use std::{
 
 use tari_common_types::types::{FixedHash, PublicKey};
 use tari_core::transactions::transaction_components::{
-    vec_into_fixed_string,
+    bytes_into_fixed_string,
     CheckpointParameters,
     CommitteeMembers,
     CommitteeSignatures,
@@ -149,7 +149,7 @@ impl TryFrom<grpc::ContractDefinition> for ContractDefinition {
     type Error = String;
 
     fn try_from(value: grpc::ContractDefinition) -> Result<Self, Self::Error> {
-        let contract_name = vec_into_fixed_string(value.contract_name);
+        let contract_name = bytes_into_fixed_string(value.contract_name);
 
         let contract_issuer =
             PublicKey::from_bytes(value.contract_issuer.as_bytes()).map_err(|err| format!("{:?}", err))?;
@@ -184,7 +184,7 @@ impl TryFrom<grpc::ContractSpecification> for ContractSpecification {
     type Error = String;
 
     fn try_from(value: grpc::ContractSpecification) -> Result<Self, Self::Error> {
-        let runtime = vec_into_fixed_string(value.runtime);
+        let runtime = bytes_into_fixed_string(value.runtime);
         let public_functions = value
             .public_functions
             .into_iter()
@@ -218,7 +218,7 @@ impl TryFrom<grpc::PublicFunction> for PublicFunction {
             .ok_or_else(|| "function is missing".to_string())??;
 
         Ok(Self {
-            name: vec_into_fixed_string(value.name),
+            name: bytes_into_fixed_string(value.name),
             function,
         })
     }

applications/tari_console_wallet/src/automation/commands.rs

@@ -1070,7 +1070,6 @@ async fn publish_contract_definition(wallet: &WalletSqlite, args: PublishFileArg
     let contract_definition: ContractDefinitionFileFormat =
         read_json_file(&args.file_path).map_err(|e| CommandError::JsonFile(e.to_string()))?;
     let contract_definition_features = ContractDefinition::from(contract_definition);
-    let contract_id_hex = contract_definition_features.calculate_contract_id().to_vec().to_hex();
 
     // create the contract definition transaction
     let mut asset_manager = wallet.asset_manager.clone();
@@ -1079,15 +1078,30 @@ async fn publish_contract_definition(wallet: &WalletSqlite, args: PublishFileArg
         .await?;
 
     // publish the contract definition transaction
-    let message = format!("Contract definition for contract {}", contract_id_hex);
+    let contract_id = transaction
+        .body
+        .outputs()
+        .iter()
+        .filter_map(|o| o.features.sidechain_features.as_ref())
+        .find_map(|f| {
+            if f.definition.is_some() {
+                Some(f.contract_id)
+            } else {
+                None
+            }
+        })
+        .ok_or_else(|| {
+            CommandError::General("Asset manager did not include contract definition in output set".to_string())
+        })?;
+    let message = format!("Contract definition for contract {}", contract_id);
     let mut transaction_service = wallet.transaction_service.clone();
     transaction_service
         .submit_transaction(tx_id, transaction, 0.into(), message)
         .await?;
 
     println!(
         "Contract definition submitted: contract_id is {} (TxID: {})",
-        contract_id_hex, tx_id,
+        contract_id, tx_id,
     );
     println!("Done!");
     Ok(())

applications/tari_console_wallet/src/automation/error.rs

@@ -77,6 +77,8 @@ pub enum CommandError {
     JsonFile(String),
     #[error(transparent)]
     IoError(#[from] io::Error),
+    #[error("General error: {0}")]
+    General(String),
 }
 
 impl From<CommandError> for ExitError {

base_layer/core/src/proto/transaction.rs

@@ -39,7 +39,7 @@ use crate::{
         aggregated_body::AggregateBody,
         tari_amount::MicroTari,
         transaction_components::{
-            vec_into_fixed_string,
+            bytes_into_fixed_string,
             AssetOutputFeatures,
             CheckpointParameters,
             CommitteeDefinitionFeatures,
@@ -951,7 +951,7 @@ impl TryFrom<proto::types::ContractDefinition> for ContractDefinition {
     type Error = String;
 
     fn try_from(value: proto::types::ContractDefinition) -> Result<Self, Self::Error> {
-        let contract_name = vec_into_fixed_string(value.contract_name);
+        let contract_name = bytes_into_fixed_string(value.contract_name);
 
         let contract_issuer =
             PublicKey::from_bytes(value.contract_issuer.as_bytes()).map_err(|err| format!("{:?}", err))?;
@@ -986,7 +986,7 @@ impl TryFrom<proto::types::ContractSpecification> for ContractSpecification {
     type Error = String;
 
     fn try_from(value: proto::types::ContractSpecification) -> Result<Self, Self::Error> {
-        let runtime = vec_into_fixed_string(value.runtime);
+        let runtime = bytes_into_fixed_string(value.runtime);
         let public_functions = value
             .public_functions
             .into_iter()
@@ -1020,7 +1020,7 @@ impl TryFrom<proto::types::PublicFunction> for PublicFunction {
             .ok_or_else(|| "function is missing".to_string())??;
 
         Ok(Self {
-            name: vec_into_fixed_string(value.name),
+            name: bytes_into_fixed_string(value.name),
             function,
         })
     }

base_layer/core/src/transactions/transaction_components/output_features.rs

@@ -540,6 +540,7 @@ mod test {
     use crate::{
         consensus::check_consensus_encoding_correctness,
         transactions::transaction_components::{
+            bytes_into_fixed_string,
             side_chain::{
                 CheckpointParameters,
                 CommitteeMembers,
@@ -549,7 +550,6 @@ mod test {
                 RequirementsForConstitutionChange,
                 SideChainConsensus,
             },
-            vec_into_fixed_string,
             CommitteeSignatures,
             ContractAcceptance,
             ContractAmendment,
@@ -607,20 +607,20 @@ mod test {
                 contract_id: FixedHash::zero(),
                 constitution: Some(constitution.clone()),
                 definition: Some(ContractDefinition {
-                    contract_name: vec_into_fixed_string("name".as_bytes().to_vec()),
+                    contract_name: bytes_into_fixed_string("name"),
                     contract_issuer: PublicKey::default(),
                     contract_spec: ContractSpecification {
-                        runtime: vec_into_fixed_string("runtime".as_bytes().to_vec()),
+                        runtime: bytes_into_fixed_string("runtime"),
                         public_functions: vec![
                             PublicFunction {
-                                name: vec_into_fixed_string("foo".as_bytes().to_vec()),
+                                name: bytes_into_fixed_string("foo"),
                                 function: FunctionRef {
                                     template_id: FixedHash::zero(),
                                     function_id: 0_u16,
                                 },
                             },
                             PublicFunction {
-                                name: vec_into_fixed_string("bar".as_bytes().to_vec()),
+                                name: bytes_into_fixed_string("bar"),
                                 function: FunctionRef {
                                     template_id: FixedHash::zero(),
                                     function_id: 1_u16,

base_layer/core/src/transactions/transaction_components/side_chain/contract_definition.rs

@@ -22,27 +22,17 @@
 
 use std::io::{Error, Read, Write};
 
-use integer_encoding::VarInt;
 use serde::{Deserialize, Serialize};
-use tari_common_types::{
-    array::copy_into_fixed_array_lossy,
-    types::{FixedHash, PublicKey},
-};
-use tari_utilities::Hashable;
+use tari_common_types::types::{FixedHash, PublicKey};
 
-use crate::consensus::{ConsensusDecoding, ConsensusEncoding, ConsensusEncodingSized, ConsensusHashWriter, MaxSizeVec};
+use crate::{
+    consensus::{ConsensusDecoding, ConsensusEncoding, ConsensusEncodingSized, ConsensusHashWriter, MaxSizeVec},
+    transactions::transaction_components::FixedString,
+};
 
 // Maximum number of functions allowed in a contract specification
 const MAX_FUNCTIONS: usize = u16::MAX as usize;
 
-// Fixed length of all string fields in the contract definition
-pub const STR_LEN: usize = 32;
-type FixedString = [u8; STR_LEN];
-
-pub fn vec_into_fixed_string(value: Vec<u8>) -> FixedString {
-    copy_into_fixed_array_lossy::<_, STR_LEN>(&value)
-}
-
 #[derive(Debug, Clone, PartialEq, Deserialize, Serialize, Eq, Hash)]
 pub struct ContractDefinition {
     pub contract_name: FixedString,
@@ -51,9 +41,7 @@ pub struct ContractDefinition {
 }
 
 impl ContractDefinition {
-    pub fn new(contract_name: Vec<u8>, contract_issuer: PublicKey, contract_spec: ContractSpecification) -> Self {
-        let contract_name = vec_into_fixed_string(contract_name);
-
+    pub fn new(contract_name: FixedString, contract_issuer: PublicKey, contract_spec: ContractSpecification) -> Self {
         Self {
             contract_name,
             contract_issuer,
@@ -62,21 +50,7 @@ impl ContractDefinition {
     }
 
     pub fn calculate_contract_id(&self) -> FixedHash {
-        ConsensusHashWriter::default()
-            .chain(&self.contract_name)
-            .chain(&self.contract_spec)
-            .finalize()
-            .into()
-    }
-
-    pub const fn str_byte_size() -> usize {
-        STR_LEN
-    }
-}
-
-impl Hashable for ContractDefinition {
-    fn hash(&self) -> Vec<u8> {
-        ConsensusHashWriter::default().chain(self).finalize().to_vec()
+        ConsensusHashWriter::default().chain(self).finalize().into()
     }
 }
 
@@ -92,7 +66,9 @@ impl ConsensusEncoding for ContractDefinition {
 
 impl ConsensusEncodingSized for ContractDefinition {
     fn consensus_encode_exact_size(&self) -> usize {
-        STR_LEN + self.contract_issuer.consensus_encode_exact_size() + self.contract_spec.consensus_encode_exact_size()
+        self.contract_name.consensus_encode_exact_size() +
+            self.contract_issuer.consensus_encode_exact_size() +
+            self.contract_spec.consensus_encode_exact_size()
     }
 }
 
@@ -116,12 +92,6 @@ pub struct ContractSpecification {
     pub public_functions: Vec<PublicFunction>,
 }
 
-impl Hashable for ContractSpecification {
-    fn hash(&self) -> Vec<u8> {
-        ConsensusHashWriter::default().chain(self).finalize().to_vec()
-    }
-}
-
 impl ConsensusEncoding for ContractSpecification {
     fn consensus_encode<W: Write>(&self, writer: &mut W) -> Result<(), Error> {
         self.runtime.consensus_encode(writer)?;
@@ -131,16 +101,7 @@ impl ConsensusEncoding for ContractSpecification {
     }
 }
 
-impl ConsensusEncodingSized for ContractSpecification {
-    fn consensus_encode_exact_size(&self) -> usize {
-        let public_function_size = match self.public_functions.first() {
-            None => 0,
-            Some(function) => function.consensus_encode_exact_size(),
-        };
-
-        STR_LEN + self.public_functions.len().required_space() + self.public_functions.len() * public_function_size
-    }
-}
+impl ConsensusEncodingSized for ContractSpecification {}
 
 impl ConsensusDecoding for ContractSpecification {
     fn consensus_decode<R: Read>(reader: &mut R) -> Result<Self, Error> {
@@ -160,12 +121,6 @@ pub struct PublicFunction {
     pub function: FunctionRef,
 }
 
-impl Hashable for PublicFunction {
-    fn hash(&self) -> Vec<u8> {
-        ConsensusHashWriter::default().chain(self).finalize().to_vec()
-    }
-}
-
 impl ConsensusEncoding for PublicFunction {
     fn consensus_encode<W: Write>(&self, writer: &mut W) -> Result<(), Error> {
         self.name.consensus_encode(writer)?;
@@ -177,7 +132,7 @@ impl ConsensusEncoding for PublicFunction {
 
 impl ConsensusEncodingSized for PublicFunction {
     fn consensus_encode_exact_size(&self) -> usize {
-        STR_LEN + self.function.consensus_encode_exact_size()
+        self.name.consensus_encode_exact_size() + self.function.consensus_encode_exact_size()
     }
 }
 
@@ -196,12 +151,6 @@ pub struct FunctionRef {
     pub function_id: u16,
 }
 
-impl Hashable for FunctionRef {
-    fn hash(&self) -> Vec<u8> {
-        ConsensusHashWriter::default().chain(self).finalize().to_vec()
-    }
-}
-
 impl ConsensusEncoding for FunctionRef {
     fn consensus_encode<W: Write>(&self, writer: &mut W) -> Result<(), Error> {
         self.template_id.consensus_encode(writer)?;
@@ -232,24 +181,27 @@ impl ConsensusDecoding for FunctionRef {
 #[cfg(test)]
 mod test {
     use super::*;
-    use crate::consensus::check_consensus_encoding_correctness;
+    use crate::{
+        consensus::check_consensus_encoding_correctness,
+        transactions::transaction_components::bytes_into_fixed_string,
+    };
 
     #[test]
     fn it_encodes_and_decodes_correctly() {
-        let contract_name = str_to_fixed_string("contract_name");
+        let contract_name = bytes_into_fixed_string("contract_name");
         let contract_issuer = PublicKey::default();
         let contract_spec = ContractSpecification {
-            runtime: str_to_fixed_string("runtime value"),
+            runtime: bytes_into_fixed_string("runtime value"),
             public_functions: vec![
                 PublicFunction {
-                    name: str_to_fixed_string("foo"),
+                    name: bytes_into_fixed_string("foo"),
                     function: FunctionRef {
                         template_id: FixedHash::zero(),
                         function_id: 0_u16,
                     },
                 },
                 PublicFunction {
-                    name: str_to_fixed_string("bar"),
+                    name: bytes_into_fixed_string("bar"),
                     function: FunctionRef {
                         template_id: FixedHash::zero(),
                         function_id: 1_u16,
@@ -258,12 +210,8 @@ mod test {
             ],
         };
 
-        let contract_definition = ContractDefinition::new(contract_name.to_vec(), contract_issuer, contract_spec);
+        let contract_definition = ContractDefinition::new(contract_name, contract_issuer, contract_spec);
 
         check_consensus_encoding_correctness(contract_definition).unwrap();
     }
-
-    fn str_to_fixed_string(s: &str) -> FixedString {
-        vec_into_fixed_string(s.as_bytes().to_vec())
-    }
 }

base_layer/core/src/transactions/transaction_components/side_chain/mod.rs

@@ -35,13 +35,7 @@ pub use contract_constitution::{
 };
 
 mod contract_definition;
-pub use contract_definition::{
-    vec_into_fixed_string,
-    ContractDefinition,
-    ContractSpecification,
-    FunctionRef,
-    PublicFunction,
-};
+pub use contract_definition::{ContractDefinition, ContractSpecification, FunctionRef, PublicFunction};
 
 mod contract_update_proposal;
 pub use contract_update_proposal::ContractUpdateProposal;
@@ -66,3 +60,11 @@ pub use sidechain_features::{SideChainFeatures, SideChainFeaturesBuilder};
 
 mod contract_checkpoint;
 pub use contract_checkpoint::ContractCheckpoint;
+
+// Length of FixedString
+pub const FIXED_STR_LEN: usize = 32;
+pub type FixedString = [u8; FIXED_STR_LEN];
+
+pub fn bytes_into_fixed_string<T: AsRef<[u8]>>(value: T) -> FixedString {
+    tari_common_types::array::copy_into_fixed_array_lossy::<_, FIXED_STR_LEN>(value.as_ref())
+}