fix(tari-script): use tari script encoding for execution stack serde …

…de/serialization (#4791)

Description
---
- Uses tari script encoding (equivalent to consensus encoding) for `ExecutionStack` serde impl
- Rename as_bytes to to_bytes as per rust convention.
- adds migration to fix execution stack encoding in db

Motivation and Context
---
Resolves #4790 

How Has This Been Tested?
---
Added test to alert if breaking changes occur with serde serialization for execution stack.
Manual testing in progress

applications/tari_app_grpc/src/conversions/transaction_input.rs

@@ -119,8 +119,8 @@ impl TryFrom<TransactionInput> for grpc::TransactionInput {
                 script: input
                     .script()
                     .map_err(|_| "Non-compact Transaction input should contain script".to_string())?
-                    .as_bytes(),
-                input_data: input.input_data.as_bytes(),
+                    .to_bytes(),
+                input_data: input.input_data.to_bytes(),
                 script_signature,
                 sender_offset_public_key: input
                     .sender_offset_public_key()

applications/tari_app_grpc/src/conversions/transaction_output.rs

@@ -85,7 +85,7 @@ impl From<TransactionOutput> for grpc::TransactionOutput {
             features: Some(output.features.into()),
             commitment: Vec::from(output.commitment.as_bytes()),
             range_proof: Vec::from(output.proof.as_bytes()),
-            script: output.script.as_bytes(),
+            script: output.script.to_bytes(),
             sender_offset_public_key: output.sender_offset_public_key.as_bytes().to_vec(),
             metadata_signature: Some(grpc::ComSignature {
                 public_nonce_commitment: Vec::from(output.metadata_signature.public_nonce().as_bytes()),

applications/tari_app_grpc/src/conversions/unblinded_output.rs

@@ -41,8 +41,8 @@ impl From<UnblindedOutput> for grpc::UnblindedOutput {
             value: u64::from(output.value),
             spending_key: output.spending_key.as_bytes().to_vec(),
             features: Some(output.features.into()),
-            script: output.script.as_bytes(),
-            input_data: output.input_data.as_bytes(),
+            script: output.script.to_bytes(),
+            input_data: output.input_data.to_bytes(),
             script_private_key: output.script_private_key.as_bytes().to_vec(),
             sender_offset_public_key: output.sender_offset_public_key.as_bytes().to_vec(),
             metadata_signature: Some(grpc::ComSignature {

base_layer/core/src/chain_storage/lmdb_db/lmdb.rs

@@ -445,3 +445,51 @@ pub fn lmdb_clear(txn: &WriteTransaction<'_>, db: &Database) -> Result<usize, Ch
     }
     Ok(num_deleted)
 }
+
+/// Used for migrations, you probably dont want to use this.
+pub(super) fn lmdb_map_inplace<F, V, R>(
+    txn: &WriteTransaction<'_>,
+    db: &Database,
+    f: F,
+) -> Result<(), ChainStorageError>
+where
+    F: Fn(V) -> Option<R>,
+    V: DeserializeOwned,
+    R: Serialize,
+{
+    let mut access = txn.access();
+    let mut cursor = txn.cursor(db).map_err(|e| {
+        error!(target: LOG_TARGET, "Could not get read cursor from lmdb: {:?}", e);
+        ChainStorageError::AccessError(e.to_string())
+    })?;
+    let iter = CursorIter::new(
+        MaybeOwned::Borrowed(&mut cursor),
+        &access,
+        |c, a| c.first(a),
+        Cursor::next::<[u8], [u8]>,
+    )?;
+    let items = iter
+        .map(|r| r.map(|(k, v)| (k.to_vec(), v.to_vec())))
+        .collect::<Result<Vec<_>, _>>()?;
+
+    for (key, val) in items {
+        // let (key, val) = row?;
+        let val = deserialize::<V>(&val)?;
+        if let Some(ret) = f(val) {
+            let ret_bytes = serialize(&ret)?;
+            access.put(db, &key, &ret_bytes, put::Flags::empty()).map_err(|e| {
+                if let lmdb_zero::Error::Code(code) = &e {
+                    if *code == lmdb_zero::error::MAP_FULL {
+                        return ChainStorageError::DbResizeRequired;
+                    }
+                }
+                error!(
+                    target: LOG_TARGET,
+                    "Could not replace value in lmdb transaction: {:?}", e
+                );
+                ChainStorageError::AccessError(e.to_string())
+            })?;
+        }
+    }
+    Ok(())
+}

base_layer/core/src/chain_storage/lmdb_db/lmdb_db.rs

@@ -264,6 +264,8 @@ impl LMDBDatabase {
             _file_lock: Arc::new(file_lock),
         };
 
+        run_migrations(&db)?;
+
         Ok(db)
     }
 
@@ -2436,6 +2438,7 @@ enum MetadataKey {
     HorizonData,
     DeletedBitmap,
     BestBlockTimestamp,
+    MigrationVersion,
 }
 
 impl MetadataKey {
@@ -2448,14 +2451,15 @@ impl MetadataKey {
 impl fmt::Display for MetadataKey {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match self {
-            MetadataKey::ChainHeight => f.write_str("Current chain height"),
-            MetadataKey::AccumulatedWork => f.write_str("Total accumulated work"),
-            MetadataKey::PruningHorizon => f.write_str("Pruning horizon"),
-            MetadataKey::PrunedHeight => f.write_str("Effective pruned height"),
-            MetadataKey::BestBlock => f.write_str("Chain tip block hash"),
-            MetadataKey::HorizonData => f.write_str("Database info"),
-            MetadataKey::DeletedBitmap => f.write_str("Deleted bitmap"),
-            MetadataKey::BestBlockTimestamp => f.write_str("Chain tip block timestamp"),
+            MetadataKey::ChainHeight => write!(f, "Current chain height"),
+            MetadataKey::AccumulatedWork => write!(f, "Total accumulated work"),
+            MetadataKey::PruningHorizon => write!(f, "Pruning horizon"),
+            MetadataKey::PrunedHeight => write!(f, "Effective pruned height"),
+            MetadataKey::BestBlock => write!(f, "Chain tip block hash"),
+            MetadataKey::HorizonData => write!(f, "Database info"),
+            MetadataKey::DeletedBitmap => write!(f, "Deleted bitmap"),
+            MetadataKey::BestBlockTimestamp => write!(f, "Chain tip block timestamp"),
+            MetadataKey::MigrationVersion => write!(f, "Migration version"),
         }
     }
 }
@@ -2471,6 +2475,7 @@ enum MetadataValue {
     HorizonData(HorizonData),
     DeletedBitmap(DeletedBitmap),
     BestBlockTimestamp(u64),
+    MigrationVersion(u64),
 }
 
 impl fmt::Display for MetadataValue {
@@ -2486,6 +2491,7 @@ impl fmt::Display for MetadataValue {
                 write!(f, "Deleted Bitmap ({} indexes)", deleted.bitmap().cardinality())
             },
             MetadataValue::BestBlockTimestamp(timestamp) => write!(f, "Chain tip block timestamp is {}", timestamp),
+            MetadataValue::MigrationVersion(n) => write!(f, "Migration version {}", n),
         }
     }
 }
@@ -2579,3 +2585,103 @@ impl CompositeKey {
 type InputKey = CompositeKey;
 type KernelKey = CompositeKey;
 type OutputKey = CompositeKey;
+
+fn run_migrations(db: &LMDBDatabase) -> Result<(), ChainStorageError> {
+    const MIGRATION_VERSION: u64 = 1;
+    let txn = db.read_transaction()?;
+
+    let k = MetadataKey::MigrationVersion;
+    let val = lmdb_get::<_, MetadataValue>(&*txn, &db.metadata_db, &k.as_u32())?;
+    let n = match val {
+        Some(MetadataValue::MigrationVersion(n)) => n,
+        Some(_) | None => 0,
+    };
+    info!(
+        target: LOG_TARGET,
+        "Blockchain database is at v{} (required version: {})", n, MIGRATION_VERSION
+    );
+    drop(txn);
+
+    if n < MIGRATION_VERSION {
+        tari_script_execution_stack_bug_migration::migrate(db)?;
+        info!(target: LOG_TARGET, "Migrated database to version {}", MIGRATION_VERSION);
+        let txn = db.write_transaction()?;
+        lmdb_replace(
+            &txn,
+            &db.metadata_db,
+            &k.as_u32(),
+            &MetadataValue::MigrationVersion(MIGRATION_VERSION),
+        )?;
+        txn.commit()?;
+    }
+
+    Ok(())
+}
+
+// TODO: remove
+mod tari_script_execution_stack_bug_migration {
+    use serde::{Deserialize, Serialize};
+    use tari_common_types::types::{ComSignature, PublicKey};
+    use tari_crypto::ristretto::{pedersen::PedersenCommitment, RistrettoPublicKey, RistrettoSchnorr};
+    use tari_script::{ExecutionStack, HashValue, ScalarValue, StackItem};
+
+    use super::*;
+    use crate::{
+        chain_storage::lmdb_db::lmdb::lmdb_map_inplace,
+        transactions::transaction_components::{SpentOutput, TransactionInputVersion},
+    };
+
+    pub fn migrate(db: &LMDBDatabase) -> Result<(), ChainStorageError> {
+        unsafe {
+            LMDBStore::resize(&db.env, &LMDBConfig::new(0, 1024 * 1024 * 1024, 0))?;
+        }
+        let txn = db.write_transaction()?;
+        lmdb_map_inplace(&txn, &db.inputs_db, |mut v: TransactionInputRowDataV0| {
+            let mut items = Vec::with_capacity(v.input.input_data.items.len());
+            while let Some(item) = v.input.input_data.items.pop() {
+                if let StackItemV0::Commitment(ref commitment) = item {
+                    let pk = PublicKey::from_bytes(commitment.as_bytes()).unwrap();
+                    items.push(StackItem::PublicKey(pk));
+                } else {
+                    items.push(unsafe { mem::transmute(item) });
+                }
+            }
+            let mut v = unsafe { mem::transmute::<_, TransactionInputRowData>(v) };
+            v.input.input_data = ExecutionStack::new(items);
+            Some(v)
+        })?;
+        txn.commit()?;
+        Ok(())
+    }
+
+    #[derive(Debug, Serialize, Deserialize)]
+    pub(crate) struct TransactionInputRowDataV0 {
+        pub input: TransactionInputV0,
+        pub header_hash: HashOutput,
+        pub mmr_position: u32,
+        pub hash: HashOutput,
+    }
+
+    #[derive(Debug, Serialize, Deserialize)]
+    pub struct TransactionInputV0 {
+        version: TransactionInputVersion,
+        spent_output: SpentOutput,
+        input_data: ExecutionStackV0,
+        script_signature: ComSignature,
+    }
+
+    #[derive(Debug, Serialize, Deserialize)]
+    struct ExecutionStackV0 {
+        items: Vec<StackItemV0>,
+    }
+
+    #[derive(Debug, Serialize, Deserialize)]
+    enum StackItemV0 {
+        Number(i64),
+        Hash(HashValue),
+        Scalar(ScalarValue),
+        Commitment(PedersenCommitment),
+        PublicKey(RistrettoPublicKey),
+        Signature(RistrettoSchnorr),
+    }
+}

base_layer/core/src/consensus/consensus_encoding/script.rs

@@ -31,7 +31,7 @@ use crate::consensus::{ConsensusDecoding, ConsensusEncoding, ConsensusEncodingSi
 
 impl ConsensusEncoding for TariScript {
     fn consensus_encode<W: Write>(&self, writer: &mut W) -> Result<(), io::Error> {
-        self.as_bytes().consensus_encode(writer)
+        self.to_bytes().consensus_encode(writer)
     }
 }
 
@@ -54,7 +54,7 @@ impl ConsensusDecoding for TariScript {
 
 impl ConsensusEncoding for ExecutionStack {
     fn consensus_encode<W: io::Write>(&self, writer: &mut W) -> Result<(), io::Error> {
-        self.as_bytes().consensus_encode(writer)
+        self.to_bytes().consensus_encode(writer)
     }
 }
 

base_layer/core/src/proto/transaction.rs

@@ -168,7 +168,7 @@ impl TryFrom<TransactionInput> for proto::types::TransactionInput {
         if input.is_compact() {
             let output_hash = input.output_hash();
             Ok(Self {
-                input_data: input.input_data.as_bytes(),
+                input_data: input.input_data.to_bytes(),
                 script_signature: Some(input.script_signature.into()),
                 output_hash: output_hash.to_vec(),
                 ..Default::default()
@@ -192,8 +192,8 @@ impl TryFrom<TransactionInput> for proto::types::TransactionInput {
                 script: input
                     .script()
                     .map_err(|_| "Non-compact Transaction input should contain script".to_string())?
-                    .as_bytes(),
-                input_data: input.input_data.as_bytes(),
+                    .to_bytes(),
+                input_data: input.input_data.to_bytes(),
                 script_signature: Some(input.script_signature.clone().into()),
                 sender_offset_public_key: input
                     .sender_offset_public_key()
@@ -277,7 +277,7 @@ impl From<TransactionOutput> for proto::types::TransactionOutput {
             features: Some(output.features.into()),
             commitment: Some(output.commitment.into()),
             range_proof: output.proof.to_vec(),
-            script: output.script.as_bytes(),
+            script: output.script.to_bytes(),
             sender_offset_public_key: output.sender_offset_public_key.as_bytes().to_vec(),
             metadata_signature: Some(output.metadata_signature.into()),
             covenant: output.covenant.to_bytes(),

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

@@ -270,9 +270,11 @@ impl TransactionInput {
             SpentOutput::OutputData { ref script, .. } => {
                 match script.execute_with_context(&self.input_data, &context)? {
                     StackItem::PublicKey(pubkey) => Ok(pubkey),
-                    _ => Err(TransactionError::ScriptExecutionError(
-                        "The script executed successfully but it did not leave a public key on the stack".to_string(),
-                    )),
+                    item => Err(TransactionError::ScriptExecutionError(format!(
+                        "The script executed successfully but it did not leave a public key on the stack. Remaining \
+                         stack item was {:?}",
+                        item
+                    ))),
                 }
             },
         }

base_layer/core/src/transactions/transaction_protocol/proto/transaction_sender.rs

@@ -138,7 +138,7 @@ impl From<SingleRoundSenderData> for proto::SingleRoundSenderData {
             metadata: Some(sender_data.metadata.into()),
             message: sender_data.message,
             features: Some(sender_data.features.into()),
-            script: sender_data.script.as_bytes(),
+            script: sender_data.script.to_bytes(),
             sender_offset_public_key: sender_data.sender_offset_public_key.to_vec(),
             public_commitment_nonce: sender_data.public_commitment_nonce.to_vec(),
             covenant: sender_data.covenant.to_consensus_bytes(),

base_layer/core/tests/chain_storage_tests/chain_storage.rs

@@ -1852,7 +1852,7 @@ mod malleability {
         fn script() {
             check_output_malleability(|block: &mut Block| {
                 let output = &mut block.body.outputs_mut()[0];
-                let mut script_bytes = output.script.as_bytes();
+                let mut script_bytes = output.script.to_bytes();
                 Opcode::PushZero.to_bytes(&mut script_bytes);
                 let mod_script = TariScript::from_bytes(&script_bytes).unwrap();
                 output.script = mod_script;

base_layer/wallet/src/output_manager_service/storage/sqlite_db/mod.rs

@@ -1417,8 +1417,8 @@ impl From<KnownOneSidedPaymentScript> for KnownOneSidedPaymentScriptSql {
         let script_lock_height = known_script.script_lock_height as i64;
         let script_hash = known_script.script_hash;
         let private_key = known_script.private_key.as_bytes().to_vec();
-        let script = known_script.script.as_bytes().to_vec();
-        let input = known_script.input.as_bytes().to_vec();
+        let script = known_script.script.to_bytes().to_vec();
+        let input = known_script.input.to_bytes().to_vec();
         KnownOneSidedPaymentScriptSql {
             script_hash,
             private_key,

base_layer/wallet/src/output_manager_service/storage/sqlite_db/new_output_sql.rs

@@ -83,8 +83,8 @@ impl NewOutputSql {
             status: status as i32,
             received_in_tx_id: received_in_tx_id.map(|i| i.as_u64() as i64),
             hash: Some(output.hash.to_vec()),
-            script: output.unblinded_output.script.as_bytes(),
-            input_data: output.unblinded_output.input_data.as_bytes(),
+            script: output.unblinded_output.script.to_bytes(),
+            input_data: output.unblinded_output.input_data.to_bytes(),
             script_private_key: output.unblinded_output.script_private_key.to_vec(),
             metadata: Some(output.unblinded_output.features.metadata.clone()),
             sender_offset_public_key: output.unblinded_output.sender_offset_public_key.to_vec(),

infrastructure/tari_script/src/lib.rs

@@ -24,7 +24,7 @@ mod serde;
 mod stack;
 
 pub use error::ScriptError;
-pub use op_codes::{slice_to_boxed_hash, slice_to_hash, HashValue, Opcode};
+pub use op_codes::{slice_to_boxed_hash, slice_to_hash, HashValue, Message, Opcode, ScalarValue};
 pub use script::TariScript;
 pub use script_commitment::{ScriptCommitment, ScriptCommitmentError, ScriptCommitmentFactory};
 pub use script_context::ScriptContext;

infrastructure/tari_script/src/script.rs

@@ -119,7 +119,7 @@ impl TariScript {
         }
     }
 
-    pub fn as_bytes(&self) -> Vec<u8> {
+    pub fn to_bytes(&self) -> Vec<u8> {
         self.script.iter().fold(Vec::new(), |mut bytes, op| {
             op.to_bytes(&mut bytes);
             bytes
@@ -137,7 +137,7 @@ impl TariScript {
         if D::output_size() < 32 {
             return Err(ScriptError::InvalidDigest);
         }
-        let h = D::digest(&self.as_bytes());
+        let h = D::digest(&self.to_bytes());
         Ok(slice_to_hash(&h.as_slice()[..32]))
     }
 
@@ -178,7 +178,7 @@ impl TariScript {
     pub fn script_message(&self, pub_key: &RistrettoPublicKey) -> Result<RistrettoSecretKey, ScriptError> {
         let b = Blake256::new()
             .chain(pub_key.as_bytes())
-            .chain(&self.as_bytes())
+            .chain(&self.to_bytes())
             .finalize();
         RistrettoSecretKey::from_bytes(b.as_slice()).map_err(|_| ScriptError::InvalidSignature)
     }
@@ -562,7 +562,7 @@ impl Hex for TariScript {
     }
 
     fn to_hex(&self) -> String {
-        to_hex(&self.as_bytes())
+        to_hex(&self.to_bytes())
     }
 }
 
@@ -948,7 +948,7 @@ mod test {
     #[test]
     fn serialisation() {
         let script = script!(Add Sub Add);
-        assert_eq!(&script.as_bytes(), &[0x93, 0x94, 0x93]);
+        assert_eq!(&script.to_bytes(), &[0x93, 0x94, 0x93]);
         assert_eq!(TariScript::from_bytes(&[0x93, 0x94, 0x93]).unwrap(), script);
         assert_eq!(script.to_hex(), "939493");
         assert_eq!(TariScript::from_hex("939493").unwrap(), script);