Refactor - Transaction Constructor Types

src/constants.rs

@@ -5,7 +5,6 @@ pub const MAX_METADATA_BYTES: usize = 800;
 pub const TX_HASH_LENGTH: usize = 32;
 
 /*------- ADDRESS CONSTANTS -------*/
-pub const V0_ADDRESS_LENGTH: usize = 16;
 pub const STANDARD_ADDRESS_LENGTH: usize = 64;
 // Prepending character for a P2SH address
 pub const P2SH_PREPEND: u8 = b'H';
@@ -14,11 +13,6 @@ pub const P2SH_PREPEND: u8 = b'H';
 // Current network version: Always bump immediately after a version is deployed.
 pub const NETWORK_VERSION: u32 = 6;
 pub const NETWORK_VERSION_SERIALIZED: &[u8] = b"6";
-// Network version 0
-pub const NETWORK_VERSION_V0: u64 = 0;
-// Network version to support temporary address structure on wallet
-// TODO: Deprecate after addresses retire
-pub const NETWORK_VERSION_TEMP: u64 = 99999;
 
 /*------- VALUE HANDLING CONSTANTS --------*/
 // Number of decimal places to divide to in display
@@ -229,8 +223,6 @@ pub const OPWITHIN_DESC: &str = "Substitutes the three numbers on top of the the
 // crypto
 pub const OPSHA3: &str = "OP_SHA3";
 pub const OPHASH256: &str = "OP_HASH256";
-pub const OPHASH256V0: &str = "OP_HASH256_V0";
-pub const OPHASH256TEMP: &str = "OP_HASH256_TEMP";
 pub const OPCHECKSIG: &str = "OP_CHECKSIG";
 pub const OPCHECKSIGVERIFY: &str = "OP_CHECKSIGVERIFY";
 pub const OPCHECKMULTISIG: &str = "OP_CHECKMULTISIG";
@@ -239,10 +231,6 @@ pub const OPCHECKMULTISIGVERIFY: &str = "OP_CHECKMULTISIGVERIFY";
 pub const OPSHA3_DESC: &str = "Hashes the top item on the stack using SHA3-256";
 pub const OPHASH256_DESC: &str =
     "Creates standard address from public key and pushes it onto the stack";
-pub const OPHASH256V0_DESC: &str =
-    "Creates v0 address from public key and pushes it onto the stack";
-pub const OPHASH256TEMP_DESC: &str =
-    "Creates temporary address from public key and pushes it onto the stack";
 pub const OPCHECKSIG_DESC: &str =
     "Pushes ONE onto the stack if the signature is valid, ZERO otherwise";
 pub const OPCHECKSIGVERIFY_DESC: &str = "Runs OP_CHECKSIG and OP_VERIFY in sequence";

src/primitives/transaction.rs

@@ -1,6 +1,6 @@
 #![allow(unused)]
 use crate::constants::*;
-use crate::crypto::sign_ed25519::{PublicKey, Signature};
+use crate::crypto::sign_ed25519::{PublicKey, SecretKey, Signature};
 use crate::primitives::{
     asset::{Asset, ItemAsset, TokenAmount},
     druid::{DdeValues, DruidExpectation},
@@ -29,15 +29,6 @@ impl GenesisTxHashSpec {
     }
 }
 
-/// A user-friendly construction struct for a TxIn
-#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
-pub struct TxConstructor {
-    pub previous_out: OutPoint,
-    pub signatures: Vec<Signature>,
-    pub pub_keys: Vec<PublicKey>,
-    pub address_version: Option<u64>,
-}
-
 /// An outpoint - a combination of a transaction hash and an index n into its vout
 #[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd, Serialize, Deserialize)]
 pub struct OutPoint {
@@ -64,6 +55,34 @@ impl Default for OutPoint {
     }
 }
 
+/// A constructor for a `TxIn`.
+#[derive(Clone, Copy, Debug)]
+pub enum TxInConstructor<'a> {
+    /// A constructor for a coinbase input.
+    Coinbase {
+        /// The number of the block which was mined.
+        block_number: u64,
+    },
+    /// A constructor for an asset creation input.
+    // TODO: jrabil: this needs to be tweaked a bit (e.g. we're still missing the GenesisHashSpec)
+    Create {
+        /// The block number which the asset was created at.
+        block_number: u64,
+        asset: &'a Asset,
+        public_key: &'a PublicKey,
+        secret_key: &'a SecretKey,
+    },
+    /// A constructor for a P2PKH input.
+    P2PKH {
+        /// The `OutPoint` being redeemed.
+        previous_out: &'a OutPoint,
+        /// The spender's public key.
+        public_key: &'a PublicKey,
+        /// The spender's private key.
+        secret_key: &'a SecretKey,
+    },
+}
+
 /// An input of a transaction. It contains the location of the previous
 /// transaction's output that it claims and a signature that matches the
 /// output's public key.
@@ -164,16 +183,6 @@ impl TxOut {
             _ => panic!("Cannot create TxOut for asset of type {:?}", asset),
         }
     }
-
-    /// Returns whether current tx_out is a P2SH
-    pub fn is_p2sh_tx_out(&self) -> bool {
-        if let Some(pk) = &self.script_public_key {
-            let pk_bytes = pk.as_bytes();
-            return pk_bytes[0] == P2SH_PREPEND;
-        }
-
-        false
-    }
 }
 
 /// The basic transaction that is broadcasted on the network and contained in
@@ -236,18 +245,4 @@ impl Transaction {
             .map(|a| !a.is_token())
             .unwrap_or_default()
     }
-
-    /// Returns whether current transaction is a P2SH tx
-    pub fn is_p2sh_tx(&self) -> bool {
-        if self.outputs.len() != 1 {
-            return false;
-        }
-
-        if let Some(pk) = &self.outputs[0].script_public_key {
-            let pk_bytes = pk.as_bytes();
-            return pk_bytes[0] == P2SH_PREPEND;
-        }
-
-        false
-    }
 }

src/script/interface_ops.rs

@@ -8,9 +8,7 @@ use crate::primitives::transaction::*;
 use crate::script::lang::{ConditionStack, Script, Stack};
 use crate::script::{OpCodes, StackEntry};
 use crate::utils::error_utils::*;
-use crate::utils::transaction_utils::{
-    construct_address, construct_address_temp, construct_address_v0,
-};
+use crate::utils::transaction_utils::construct_address;
 use bincode::de;
 use bincode::serialize;
 use bytes::Bytes;
@@ -601,7 +599,7 @@ pub fn op_cat(stack: &mut Stack) -> bool {
         error_item_size(op);
         return false;
     }
-    let cat = [s1, s2].join("");
+    let cat = [s1, s2].concat();
     stack.push(StackEntry::Bytes(cat))
 }
 
@@ -648,6 +646,10 @@ pub fn op_substr(stack: &mut Stack) -> bool {
             return false;
         }
     };
+    // TODO: As this was previously a hex string, the indices don't exactly correspond to what
+    //       they did originally. However, I don't think there are any existing transactions
+    //       on the chain which actually use this opcode, so I'm fairly confident it won't
+    //       matter. Double-check that this is the case before merging!
     if n1 >= s.len() {
         error_item_index(op);
         return false;
@@ -660,7 +662,7 @@ pub fn op_substr(stack: &mut Stack) -> bool {
         error_item_index(op);
         return false;
     }
-    let substr = s[n1..n1 + n2].to_string();
+    let substr = s[n1..n1 + n2].to_vec();
     stack.push(StackEntry::Bytes(substr))
 }
 
@@ -700,7 +702,11 @@ pub fn op_left(stack: &mut Stack) -> bool {
     if n >= s.len() {
         stack.push(StackEntry::Bytes(s))
     } else {
-        let left = s[..n].to_string();
+        // TODO: As this was previously a hex string, the indices don't exactly correspond to what
+        //       they did originally. However, I don't think there are any existing transactions
+        //       on the chain which actually use this opcode, so I'm fairly confident it won't
+        //       matter. Double-check that this is the case before merging!
+        let left = s[..n].to_vec();
         stack.push(StackEntry::Bytes(left))
     }
 }
@@ -739,9 +745,13 @@ pub fn op_right(stack: &mut Stack) -> bool {
         }
     };
     if n >= s.len() {
-        stack.push(StackEntry::Bytes("".to_string()))
+        stack.push(StackEntry::Bytes(Vec::new()))
     } else {
-        let right = s[n..].to_string();
+        // TODO: As this was previously a hex string, the indices don't exactly correspond to what
+        //       they did originally. However, I don't think there are any existing transactions
+        //       on the chain which actually use this opcode, so I'm fairly confident it won't
+        //       matter. Double-check that this is the case before merging!
+        let right = s[n..].to_vec();
         stack.push(StackEntry::Bytes(right))
     }
 }
@@ -756,8 +766,8 @@ pub fn op_right(stack: &mut Stack) -> bool {
 pub fn op_size(stack: &mut Stack) -> bool {
     let (op, desc) = (OPSIZE, OPSIZE_DESC);
     trace(op, desc);
-    let s = match stack.last() {
-        Some(StackEntry::Bytes(s)) => s,
+    let len = match stack.last() {
+        Some(StackEntry::Bytes(s)) => s.len(),
         Some(_) => {
             error_item_type(op);
             return false;
@@ -767,7 +777,11 @@ pub fn op_size(stack: &mut Stack) -> bool {
             return false;
         }
     };
-    stack.push(StackEntry::Num(s.len()))
+    // TODO: As this was previously a hex string, the length doesn't exactly correspond to what
+    //       it did originally. However, I don't think there are any existing transactions
+    //       on the chain which actually use this opcode, so I'm fairly confident it won't
+    //       matter. Double-check that this is the case before merging!
+    stack.push(StackEntry::Num(len))
 }
 
 /*---- BITWISE LOGIC OPS ----*/
@@ -1924,7 +1938,11 @@ pub fn op_sha3(stack: &mut Stack) -> bool {
     let data = match stack.pop() {
         Some(StackEntry::Signature(sig)) => sig.as_ref().to_owned(),
         Some(StackEntry::PubKey(pk)) => pk.as_ref().to_owned(),
-        Some(StackEntry::Bytes(s)) => s.as_bytes().to_owned(),
+        Some(StackEntry::Bytes(s)) => {
+            // For legacy reasons, the hashed data is the hex representation of the data rather than
+            // the data itself.
+            hex::encode(&s).as_bytes().to_owned()
+        },
         Some(_) => {
             error_item_type(op);
             return false;
@@ -1934,7 +1952,8 @@ pub fn op_sha3(stack: &mut Stack) -> bool {
             return false;
         }
     };
-    let hash = hex::encode(sha3_256::digest(&data));
+    let hash = sha3_256::digest(&data).to_vec();
+    // TODO: Originally, the hash was converted back to hex!
     stack.push(StackEntry::Bytes(hash))
 }
 
@@ -1960,65 +1979,7 @@ pub fn op_hash256(stack: &mut Stack) -> bool {
         }
     };
     let addr = construct_address(&pk);
-    stack.push(StackEntry::Bytes(addr))
-}
-
-/// OP_HASH256_V0: Creates v0 address from public key and pushes it onto the stack
-///
-/// Example: OP_HASH256_V0([pk]) -> [addr_v0]
-///
-/// Info: Support for old 32-byte addresses
-///
-/// TODO: Deprecate after addresses retire
-///
-/// ### Arguments
-///
-/// * `stack`  - mutable reference to the stack
-pub fn op_hash256_v0(stack: &mut Stack) -> bool {
-    let (op, desc) = (OPHASH256V0, OPHASH256V0_DESC);
-    trace(op, desc);
-    let pk = match stack.pop() {
-        Some(StackEntry::PubKey(pk)) => pk,
-        Some(_) => {
-            error_item_type(op);
-            return false;
-        }
-        _ => {
-            error_num_items(op);
-            return false;
-        }
-    };
-    let addr_v0 = construct_address_v0(&pk);
-    stack.push(StackEntry::Bytes(addr_v0))
-}
-
-/// OP_HASH256_TEMP: Creates temporary address from public key and pushes it onto the stack
-///
-/// Example: OP_HASH256_TEMP([pk]) -> [addr_temp]
-///
-/// Info: Support for temporary address scheme used in wallet
-///
-/// TODO: Deprecate after addresses retire
-///
-/// ### Arguments
-///
-/// * `stack`  - mutable reference to the stack
-pub fn op_hash256_temp(stack: &mut Stack) -> bool {
-    let (op, desc) = (OPHASH256TEMP, OPHASH256TEMP_DESC);
-    trace(op, desc);
-    let pk = match stack.pop() {
-        Some(StackEntry::PubKey(pk)) => pk,
-        Some(_) => {
-            error_item_type(op);
-            return false;
-        }
-        _ => {
-            error_num_items(op);
-            return false;
-        }
-    };
-    let addr_temp = construct_address_temp(&pk);
-    stack.push(StackEntry::Bytes(addr_temp))
+    stack.push(StackEntry::Bytes(hex::decode(addr).unwrap()))
 }
 
 /// OP_CHECKSIG: Pushes ONE onto the stack if the signature is valid, ZERO otherwise
@@ -2067,8 +2028,13 @@ pub fn op_checksig(stack: &mut Stack) -> bool {
             return false;
         }
     };
-    trace!("Signature: {:?}", hex::encode(sig));
-    if (!sign::verify_detached(&sig, msg.as_bytes(), &pk)) {
+
+    // For legacy reasons, the signed message is the hex representation of the message rather than
+    // the message itself.
+    let msg_hex = hex::encode(msg);
+
+    trace!("Signature: {:?}", msg_hex);
+    if (!sign::verify_detached(&sig, msg_hex.as_bytes(), &pk)) {
         trace!("Signature verification failed");
         stack.push(StackEntry::Num(ZERO))
     } else {
@@ -2121,8 +2087,13 @@ pub fn op_checksigverify(stack: &mut Stack) -> bool {
             return false;
         }
     };
-    trace!("Signature: {:?}", hex::encode(sig));
-    if (!sign::verify_detached(&sig, msg.as_bytes(), &pk)) {
+
+    // For legacy reasons, the signed message is the hex representation of the message rather than
+    // the message itself.
+    let msg_hex = hex::encode(msg);
+
+    trace!("Signature: {:?}", msg_hex);
+    if (!sign::verify_detached(&sig, msg_hex.as_bytes(), &pk)) {
         trace!("Signature verification failed");
         error_invalid_signature(op);
         return false;
@@ -2298,13 +2269,17 @@ pub fn op_checkmultisigverify(stack: &mut Stack) -> bool {
 /// * `sigs` - signatures to verify
 /// * `msg`  - data to verify against
 /// * `pks`  - public keys to match against
-fn verify_multisig(sigs: &[Signature], msg: &String, pks: &mut Vec<PublicKey>) -> bool {
+fn verify_multisig(sigs: &[Signature], msg: &[u8], pks: &mut Vec<PublicKey>) -> bool {
+    // For legacy reasons, the signed message is the hex representation of the message rather than
+    // the message itself.
+    let msg_hex = hex::encode(msg);
+
     let mut num_valid_sigs = ZERO;
     for sig in sigs {
         if let Some((index, _)) = pks
             .iter()
             .enumerate()
-            .find(|(_, pk)| sign::verify_detached(sig, msg.as_bytes(), pk))
+            .find(|(_, pk)| sign::verify_detached(sig, msg_hex.as_bytes(), pk))
         {
             num_valid_sigs += ONE;
             pks.remove(index);