breaking change: introduce custom Transaction type instead of `Vec<…

…u8>` in transact call (#63)

* Use typed Transaction instead of Vec<u8> in Call::transact

* Move Txid into types

* Nits

* Inline decode_transaction() in pallet-bitcoin

* Refactor TxIn

* Store Txid in reversed byte order in runtime

* Fix clippy

* Add test_runtime_transaction_type()

* Remove duplicated docs in book

* Strip 0x prefix in revert-h256 command

crates/pallet-bitcoin/Cargo.toml

@@ -19,6 +19,9 @@ sp-runtime = { workspace = true, default-features = false }
 sp-std = { workspace = true, default-features = false }
 subcoin-runtime-primitives = { workspace = true, default-features = false }
 
+[dev-dependencies]
+hex = { package = "hex-conservative", version = "0.2.0", features = ["alloc"] }
+
 [features]
 default = ["std"]
 std = [

crates/pallet-bitcoin/src/lib.rs

@@ -12,14 +12,12 @@
 
 #[cfg(test)]
 mod tests;
+pub mod types;
 
-use bitcoin::consensus::{Decodable, Encodable};
-use bitcoin::{OutPoint, Transaction as BitcoinTransaction};
-use codec::{Decode, Encode, MaxEncodedLen};
+use self::types::{OutPoint, Transaction, Txid};
+use bitcoin::consensus::Decodable;
 use frame_support::dispatch::DispatchResult;
 use frame_support::weights::Weight;
-use scale_info::TypeInfo;
-use sp_core::H256;
 use sp_runtime::traits::BlockNumberProvider;
 use sp_runtime::SaturatedConversion;
 use sp_std::prelude::*;
@@ -30,55 +28,7 @@ use subcoin_runtime_primitives::Coin;
 pub use pallet::*;
 
 /// Transaction output index.
-pub type Vout = u32;
-
-/// Wrapper type for Bitcoin txid in runtime as `bitcoin::Txid` does not implement codec.
-#[derive(Clone, TypeInfo, Encode, Decode, MaxEncodedLen)]
-pub struct Txid(H256);
-
-impl Txid {
-    fn from_bitcoin_txid(txid: bitcoin::Txid) -> Self {
-        let mut d = Vec::with_capacity(32);
-        txid.consensus_encode(&mut d)
-            .expect("txid must be encoded correctly; qed");
-
-        let d: [u8; 32] = d
-            .try_into()
-            .expect("Bitcoin txid is sha256 hash which must fit into [u8; 32]; qed");
-
-        Self(H256::from(d))
-    }
-
-    /// Converts the runtime [`Txid`] to a `bitcoin::Txid`.
-    pub fn into_bitcoin_txid(self) -> bitcoin::Txid {
-        bitcoin::consensus::Decodable::consensus_decode(&mut self.encode().as_slice())
-            .expect("Decode must succeed as txid was ensured to be encoded correctly; qed")
-    }
-}
-
-impl core::fmt::Debug for Txid {
-    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
-        for byte in self.0.as_bytes().iter().rev() {
-            write!(f, "{:02x}", byte)?;
-        }
-        Ok(())
-    }
-}
-
-#[derive(Debug, TypeInfo, Encode, Decode, MaxEncodedLen)]
-struct OutPointInner {
-    txid: Txid,
-    vout: Vout,
-}
-
-impl From<OutPoint> for OutPointInner {
-    fn from(out_point: OutPoint) -> Self {
-        Self {
-            txid: Txid::from_bitcoin_txid(out_point.txid),
-            vout: out_point.vout,
-        }
-    }
-}
+pub type OutputIndex = u32;
 
 #[frame_support::pallet]
 pub mod pallet {
@@ -99,14 +49,13 @@ pub mod pallet {
 
     #[pallet::call(weight(<T as Config>::WeightInfo))]
     impl<T: Config> Pallet<T> {
-        /// An internal unsigned extrinsic for including a Bitcoin transaction into the block.
+        /// An unsigned extrinsic for embedding a Bitcoin transaction into the Substrate block.
         #[pallet::call_index(0)]
         #[pallet::weight(Weight::zero())]
-        pub fn transact(origin: OriginFor<T>, btc_tx: Vec<u8>) -> DispatchResult {
+        pub fn transact(origin: OriginFor<T>, btc_tx: Transaction) -> DispatchResult {
             ensure_none(origin)?;
 
-            let bitcoin_transaction = Self::decode_transaction(btc_tx);
-            Self::process_bitcoin_transaction(bitcoin_transaction);
+            Self::process_bitcoin_transaction(btc_tx.into());
 
             Ok(())
         }
@@ -147,7 +96,13 @@ pub mod pallet {
     #[pallet::genesis_build]
     impl<T: Config> BuildGenesisConfig for GenesisConfig<T> {
         fn build(&self) {
-            let genesis_tx = Pallet::<T>::decode_transaction(self.genesis_tx.clone());
+            let genesis_tx =
+                bitcoin::Transaction::consensus_decode(&mut self.genesis_tx.clone().as_slice())
+                    .unwrap_or_else(|_| {
+                        panic!(
+                            "Transaction constructed internally must be decoded successfully; qed"
+                        )
+                    });
 
             let txid = Txid::from_bitcoin_txid(genesis_tx.compute_txid());
 
@@ -172,17 +127,18 @@ pub mod pallet {
 
     /// UTXO set.
     ///
-    /// (Txid, Vout, Coin)
+    /// (Txid, OutputIndex(vout), Coin)
     #[pallet::storage]
-    pub type Coins<T> = StorageDoubleMap<_, Identity, Txid, Identity, Vout, Coin, OptionQuery>;
+    pub type Coins<T> =
+        StorageDoubleMap<_, Identity, Txid, Identity, OutputIndex, Coin, OptionQuery>;
 }
 
 /// Returns the storage key for the referenced output.
-pub fn coin_storage_key<T: Config>(bitcoin_txid: bitcoin::Txid, index: Vout) -> Vec<u8> {
+pub fn coin_storage_key<T: Config>(bitcoin_txid: bitcoin::Txid, vout: OutputIndex) -> Vec<u8> {
     use frame_support::storage::generator::StorageDoubleMap;
 
     let txid = Txid::from_bitcoin_txid(bitcoin_txid);
-    Coins::<T>::storage_double_map_final_key(txid, index)
+    Coins::<T>::storage_double_map_final_key(txid, vout)
 }
 
 /// Returns the final storage prefix for the storage item `Coins`.
@@ -193,20 +149,14 @@ pub fn coin_storage_prefix<T: Config>() -> [u8; 32] {
 }
 
 impl<T: Config> Pallet<T> {
-    fn decode_transaction(btc_tx: Vec<u8>) -> BitcoinTransaction {
-        BitcoinTransaction::consensus_decode(&mut btc_tx.as_slice()).unwrap_or_else(|_| {
-            panic!("Transaction constructed internally must be decoded successfully; qed")
-        })
-    }
-
-    fn process_bitcoin_transaction(tx: BitcoinTransaction) {
+    fn process_bitcoin_transaction(tx: bitcoin::Transaction) {
         let txid = tx.compute_txid();
         let is_coinbase = tx.is_coinbase();
 
         let height = frame_system::Pallet::<T>::current_block_number();
 
         let new_coins = tx.output.into_iter().enumerate().map(|(index, txout)| {
-            let out_point = OutPoint {
+            let out_point = bitcoin::OutPoint {
                 txid,
                 vout: index as u32,
             };
@@ -221,26 +171,26 @@ impl<T: Config> Pallet<T> {
 
         if is_coinbase {
             for (out_point, coin) in new_coins {
-                let OutPointInner { txid, vout } = OutPointInner::from(out_point);
-                Coins::<T>::insert(txid, vout, coin);
+                let OutPoint { txid, output_index } = OutPoint::from(out_point);
+                Coins::<T>::insert(txid, output_index, coin);
             }
             return;
         }
 
         // Process inputs.
         for input in tx.input {
             let previous_output = input.previous_output;
-            let OutPointInner { txid, vout } = OutPointInner::from(previous_output);
-            if let Some(_spent) = Coins::<T>::take(txid, vout) {
+            let OutPoint { txid, output_index } = OutPoint::from(previous_output);
+            if let Some(_spent) = Coins::<T>::take(txid, output_index) {
             } else {
                 panic!("Corruputed state, UTXO {previous_output:?} not found");
             }
         }
 
         // Process outputs.
         for (out_point, coin) in new_coins {
-            let OutPointInner { txid, vout } = OutPointInner::from(out_point);
-            Coins::<T>::insert(txid, vout, coin);
+            let OutPoint { txid, output_index } = OutPoint::from(out_point);
+            Coins::<T>::insert(txid, output_index, coin);
         }
     }
 }

crates/pallet-bitcoin/src/tests.rs

@@ -1,4 +1,5 @@
-use bitcoin::consensus::Encodable;
+use bitcoin::consensus::{deserialize, Encodable};
+use hex::test_hex_unwrap as hex;
 use sp_core::Encode;
 
 #[test]
@@ -12,5 +13,22 @@ fn test_runtime_txid_type() {
     let mut d = Vec::new();
     txid.consensus_encode(&mut d)
         .expect("txid must be encoded correctly; qed");
+    d.reverse();
     assert_eq!(d, runtime_txid.encode());
 }
+
+#[test]
+fn test_runtime_transaction_type() {
+    let tx_bytes = hex!(
+        "02000000000101595895ea20179de87052b4046dfe6fd515860505d6511a9004cf12a1f93cac7c01000000\
+            00ffffffff01deb807000000000017a9140f3444e271620c736808aa7b33e370bd87cb5a078702483045022\
+            100fb60dad8df4af2841adc0346638c16d0b8035f5e3f3753b88db122e70c79f9370220756e6633b17fd271\
+            0e626347d28d60b0a2d6cbb41de51740644b9fb3ba7751040121028fa937ca8cba2197a37c007176ed89410\
+            55d3bcb8627d085e94553e62f057dcc00000000"
+    );
+    let tx: bitcoin::Transaction = deserialize(&tx_bytes).unwrap();
+
+    let runtime_tx: crate::types::Transaction = tx.clone().into();
+
+    assert_eq!(tx, bitcoin::Transaction::from(runtime_tx));
+}