Test bitcoin-serai

Also resolves a few rough edges.

coins/bitcoin/Cargo.toml

@@ -32,3 +32,6 @@ reqwest = { version = "0.11", features = ["json"] }
 frost = { package = "modular-frost", path = "../../crypto/frost", version = "0.6", features = ["tests"] }
 
 tokio = { version = "1", features = ["full"] }
+
+[features]
+hazmat = []

coins/bitcoin/src/lib.rs

@@ -2,11 +2,12 @@
 pub use bitcoin;
 
 /// Cryptographic helpers.
+#[cfg(feature = "hazmat")]
 pub mod crypto;
+#[cfg(not(feature = "hazmat"))]
+pub(crate) mod crypto;
+
 /// Wallet functionality to create transactions.
 pub mod wallet;
 /// A minimal asynchronous Bitcoin RPC client.
 pub mod rpc;
-
-#[cfg(test)]
-mod tests;

coins/bitcoin/src/rpc.rs

@@ -14,11 +14,17 @@ use bitcoin::{
   Txid, Transaction, BlockHash, Block,
 };
 
+#[derive(Clone, PartialEq, Eq, Debug, Deserialize)]
+pub struct Error {
+  code: isize,
+  message: String,
+}
+
 #[derive(Clone, Debug, Deserialize)]
 #[serde(untagged)]
 enum RpcResponse<T> {
   Ok { result: T },
-  Err { error: String },
+  Err { error: Error },
 }
 
 /// A minimal asynchronous Bitcoin RPC client.
@@ -29,8 +35,8 @@ pub struct Rpc(String);
 pub enum RpcError {
   #[error("couldn't connect to node")]
   ConnectionError,
-  #[error("request had an error: {0}")]
+  #[error("request had an error: {0:?}")]
-  RequestError(String),
+  RequestError(Error),
   #[error("node sent an invalid response")]
   InvalidResponse,
 }
@@ -69,7 +75,14 @@ impl Rpc {
   }
 
   /// Get the latest block's number.
+  ///
+  /// The genesis block's 'number' is zero. They increment from there.
   pub async fn get_latest_block_number(&self) -> Result<usize, RpcError> {
+    // getblockcount doesn't return the amount of blocks on the current chain, yet the "height"
+    // of the current chain. The "height" of the current chain is defined as the "height" of the
+    // tip block of the current chain. The "height" of a block is defined as the amount of blocks
+    // present when the block was created. Accordingly, the genesis block has height 0, and
+    // getblockcount will return 0 when it's only the only block, despite their being one block.
     self.rpc_call("getblockcount", json!([])).await
   }
 

coins/bitcoin/src/wallet/mod.rs

@@ -42,8 +42,6 @@ pub fn address(network: Network, key: ProjectivePoint) -> Option<Address> {
 #[derive(Clone, PartialEq, Eq, Debug)]
 pub struct ReceivedOutput {
   // The scalar offset to obtain the key usable to spend this output.
-  //
-  // This field exists in order to support HDKD schemes.
   offset: Scalar,
   // The output to spend.
   output: TxOut,
@@ -148,6 +146,10 @@ impl Scanner {
   }
 
   /// Scan a block.
+  ///
+  /// This will also scan the coinbase transaction which is bound by maturity. If received outputs
+  /// must be immediately spendable, a post-processing pass is needed to remove those outputs.
+  /// Alternatively, scan_transaction can be called on `block.txdata[1 ..]`.
   pub fn scan_block(&self, block: &Block) -> Vec<ReceivedOutput> {
     let mut res = vec![];
     for tx in &block.txdata {

coins/bitcoin/src/wallet/send.rs

@@ -15,10 +15,13 @@ use frost::{curve::Secp256k1, Participant, ThresholdKeys, FrostError, sign::*};
 use bitcoin::{
   hashes::Hash,
   util::sighash::{SchnorrSighashType, SighashCache, Prevouts},
-  OutPoint, Script, Sequence, Witness, TxIn, TxOut, PackedLockTime, Transaction, Address,
+  OutPoint, Script, Sequence, Witness, TxIn, TxOut, PackedLockTime, Transaction, Network, Address,
 };
 
-use crate::{crypto::Schnorr, wallet::ReceivedOutput};
+use crate::{
+  crypto::Schnorr,
+  wallet::{address, ReceivedOutput},
+};
 
 #[rustfmt::skip]
 // https://github.com/bitcoin/bitcoin/blob/306ccd4927a2efe325c8d84be1bdb79edeb29b04/src/policy/policy.h#L27
@@ -192,11 +195,13 @@ impl SignableTransaction {
   }
 
   /// Create a multisig machine for this transaction.
-  pub async fn multisig(
+  ///
+  /// Returns None if the wrong keys are used.
+  pub fn multisig(
     self,
     keys: ThresholdKeys<Secp256k1>,
     mut transcript: RecommendedTranscript,
-  ) -> Result<TransactionMachine, FrostError> {
+  ) -> Option<TransactionMachine> {
     transcript.domain_separate(b"bitcoin_transaction");
     transcript.append_message(b"root_key", keys.group_key().to_encoded_point(true).as_bytes());
 
@@ -215,13 +220,21 @@ impl SignableTransaction {
     for i in 0 .. tx.input.len() {
       let mut transcript = transcript.clone();
       transcript.append_message(b"signing_input", u32::try_from(i).unwrap().to_le_bytes());
+
+      let offset = keys.clone().offset(self.offsets[i]);
+      if address(Network::Bitcoin, offset.group_key())?.script_pubkey() !=
+        self.prevouts[i].script_pubkey
+      {
+        None?;
+      }
+
       sigs.push(AlgorithmMachine::new(
         Schnorr::new(transcript),
         keys.clone().offset(self.offsets[i]),
       ));
     }
 
-    Ok(TransactionMachine { tx: self, sigs })
+    Some(TransactionMachine { tx: self, sigs })
   }
 }
 

coins/bitcoin/tests/crypto.rs

@@ -3,7 +3,6 @@ use rand_core::OsRng;
 use sha2::{Digest, Sha256};
 
 use secp256k1::{SECP256K1, Message};
-use bitcoin::hashes::{Hash as HashTrait, sha256::Hash};
 
 use k256::Scalar;
 use transcript::{Transcript, RecommendedTranscript};
@@ -13,9 +12,9 @@ use frost::{
   tests::{algorithm_machines, key_gen, sign},
 };
 
-use crate::{
+use bitcoin_serai::{
+  bitcoin::hashes::{Hash as HashTrait, sha256::Hash},
   crypto::{x_only, make_even, Schnorr},
-  rpc::Rpc,
 };
 
 #[test]
@@ -46,14 +45,3 @@ fn test_algorithm() {
     )
     .unwrap()
 }
-
-#[tokio::test]
-async fn test_rpc() {
-  let rpc = Rpc::new("http://serai:seraidex@127.0.0.1:18443".to_string()).await.unwrap();
-
-  let latest = rpc.get_latest_block_number().await.unwrap();
-  assert_eq!(
-    rpc.get_block_number(&rpc.get_block_hash(latest).await.unwrap()).await.unwrap(),
-    latest
-  );
-}

coins/bitcoin/tests/rpc.rs

@@ -0,0 +1,25 @@
+use bitcoin_serai::{bitcoin::hashes::Hash as HashTrait, rpc::RpcError};
+
+mod runner;
+use runner::rpc;
+
+async_sequential! {
+  async fn test_rpc() {
+    let rpc = rpc().await;
+
+    // Test get_latest_block_number and get_block_hash by round tripping them
+    let latest = rpc.get_latest_block_number().await.unwrap();
+    let hash = rpc.get_block_hash(latest).await.unwrap();
+    assert_eq!(rpc.get_block_number(&hash).await.unwrap(), latest);
+
+    // Test this actually is the latest block number by checking asking for the next block's errors
+    assert!(matches!(rpc.get_block_hash(latest + 1).await, Err(RpcError::RequestError(_))));
+
+    // Test get_block by checking the received block's hash matches the request
+    let block = rpc.get_block(&hash).await.unwrap();
+    // Hashes are stored in reverse. It's bs from Satoshi
+    let mut block_hash = block.block_hash().as_hash().into_inner();
+    block_hash.reverse();
+    assert_eq!(hash, block_hash);
+  }
+}

coins/bitcoin/tests/runner.rs

@@ -0,0 +1,44 @@
+use bitcoin_serai::rpc::Rpc;
+
+use tokio::sync::Mutex;
+
+lazy_static::lazy_static! {
+  pub static ref SEQUENTIAL: Mutex<()> = Mutex::new(());
+}
+
+#[allow(dead_code)]
+pub(crate) async fn rpc() -> Rpc {
+  let rpc = Rpc::new("http://serai:seraidex@127.0.0.1:18443".to_string()).await.unwrap();
+
+  // If this node has already been interacted with, clear its chain
+  if rpc.get_latest_block_number().await.unwrap() > 0 {
+    rpc
+      .rpc_call(
+        "invalidateblock",
+        serde_json::json!([hex::encode(rpc.get_block_hash(1).await.unwrap())]),
+      )
+      .await
+      .unwrap()
+  }
+
+  rpc
+}
+
+#[macro_export]
+macro_rules! async_sequential {
+  ($(async fn $name: ident() $body: block)*) => {
+    $(
+      #[tokio::test]
+      async fn $name() {
+        let guard = runner::SEQUENTIAL.lock().await;
+        let local = tokio::task::LocalSet::new();
+        local.run_until(async move {
+          if let Err(err) = tokio::task::spawn_local(async move { $body }).await {
+            drop(guard);
+            Err(err).unwrap()
+          }
+        }).await;
+      }
+    )*
+  }
+}

coins/bitcoin/tests/wallet.rs

@@ -0,0 +1,241 @@
+use std::collections::HashMap;
+
+use rand_core::OsRng;
+
+use transcript::{Transcript, RecommendedTranscript};
+
+use k256::{
+  elliptic_curve::{
+    group::{ff::Field, Group},
+    sec1::{Tag, ToEncodedPoint},
+  },
+  Scalar, ProjectivePoint,
+};
+use frost::{
+  Participant,
+  tests::{THRESHOLD, key_gen, sign_without_caching},
+};
+
+use bitcoin_serai::{
+  bitcoin::{hashes::Hash as HashTrait, OutPoint, Script, TxOut, Network, Address},
+  wallet::{tweak_keys, address, ReceivedOutput, Scanner, SignableTransaction},
+  rpc::Rpc,
+};
+
+mod runner;
+use runner::rpc;
+
+fn is_even(key: ProjectivePoint) -> bool {
+  key.to_encoded_point(true).tag() == Tag::CompressedEvenY
+}
+
+async fn send(rpc: &Rpc, address: Address) -> usize {
+  let res = rpc.get_latest_block_number().await.unwrap() + 1;
+
+  rpc.rpc_call::<Vec<String>>("generatetoaddress", serde_json::json!([1, address])).await.unwrap();
+
+  // Mine until maturity
+  rpc
+    .rpc_call::<Vec<String>>(
+      "generatetoaddress",
+      serde_json::json!([100, Address::p2sh(&Script::new(), Network::Regtest).unwrap()]),
+    )
+    .await
+    .unwrap();
+
+  res
+}
+
+async fn send_and_get_output(rpc: &Rpc, scanner: &Scanner, key: ProjectivePoint) -> ReceivedOutput {
+  let block_number = send(rpc, address(Network::Regtest, key).unwrap()).await;
+  let block = rpc.get_block(&rpc.get_block_hash(block_number).await.unwrap()).await.unwrap();
+
+  let mut outputs = scanner.scan_block(&block);
+  assert_eq!(outputs, scanner.scan_transaction(&block.txdata[0]));
+
+  assert_eq!(outputs.len(), 1);
+  assert_eq!(outputs[0].outpoint(), &OutPoint::new(block.txdata[0].txid(), 0));
+  assert_eq!(outputs[0].value(), block.txdata[0].output[0].value);
+
+  assert_eq!(
+    ReceivedOutput::read::<&[u8]>(&mut outputs[0].serialize().as_ref()).unwrap(),
+    outputs[0]
+  );
+
+  outputs.swap_remove(0)
+}
+
+#[test]
+fn test_tweak_keys() {
+  let mut even = false;
+  let mut odd = false;
+
+  // Generate keys until we get an even set and an odd set
+  while !(even && odd) {
+    let mut keys = key_gen(&mut OsRng).drain().next().unwrap().1;
+    if is_even(keys.group_key()) {
+      // Tweaking should do nothing
+      assert_eq!(tweak_keys(&keys).group_key(), keys.group_key());
+
+      even = true;
+    } else {
+      let tweaked = tweak_keys(&keys).group_key();
+      assert_ne!(tweaked, keys.group_key());
+      // Tweaking should produce an even key
+      assert!(is_even(tweaked));
+
+      // Verify it uses the smallest possible offset
+      while keys.group_key().to_encoded_point(true).tag() == Tag::CompressedOddY {
+        keys = keys.offset(Scalar::ONE);
+      }
+      assert_eq!(tweaked, keys.group_key());
+
+      odd = true;
+    }
+  }
+}
+
+async_sequential! {
+  async fn test_scanner() {
+    // Test Scanners are creatable for even keys.
+    for _ in 0 .. 128 {
+      let key = ProjectivePoint::random(&mut OsRng);
+      assert_eq!(Scanner::new(key).is_some(), is_even(key));
+    }
+
+    let mut key = ProjectivePoint::random(&mut OsRng);
+    while !is_even(key) {
+      key += ProjectivePoint::GENERATOR;
+    }
+
+    {
+      let mut scanner = Scanner::new(key).unwrap();
+      for _ in 0 .. 128 {
+        let mut offset = Scalar::random(&mut OsRng);
+        let registered = scanner.register_offset(offset).unwrap();
+        // Registering this again should return None
+        assert!(scanner.register_offset(offset).is_none());
+
+        // We can only register offsets resulting in even keys
+        // Make this even
+        while !is_even(key + (ProjectivePoint::GENERATOR * offset)) {
+          offset += Scalar::ONE;
+        }
+        // Ensure it matches the registered offset
+        assert_eq!(registered, offset);
+        // Assert registering this again fails
+        assert!(scanner.register_offset(offset).is_none());
+      }
+    }
+
+    let rpc = rpc().await;
+    let mut scanner = Scanner::new(key).unwrap();
+
+    assert_eq!(send_and_get_output(&rpc, &scanner, key).await.offset(), Scalar::ZERO);
+
+    // Register an offset and test receiving to it
+    let offset = scanner.register_offset(Scalar::random(&mut OsRng)).unwrap();
+    assert_eq!(
+      send_and_get_output(&rpc, &scanner, key + (ProjectivePoint::GENERATOR * offset))
+        .await
+        .offset(),
+      offset
+    );
+  }
+
+  async fn test_send() {
+    let mut keys = key_gen(&mut OsRng);
+    for (_, keys) in keys.iter_mut() {
+      *keys = tweak_keys(keys);
+    }
+    let key = keys.values().next().unwrap().group_key();
+
+    let rpc = rpc().await;
+    let mut scanner = Scanner::new(key).unwrap();
+
+    // Get inputs, one not offset and one offset
+    let output = send_and_get_output(&rpc, &scanner, key).await;
+    assert_eq!(output.offset(), Scalar::ZERO);
+
+    let offset = scanner.register_offset(Scalar::random(&mut OsRng)).unwrap();
+    let offset_key = key + (ProjectivePoint::GENERATOR * offset);
+    let offset_output = send_and_get_output(&rpc, &scanner, offset_key).await;
+    assert_eq!(offset_output.offset(), offset);
+
+    // Declare payments, change, fee
+    let payments = [
+      (address(Network::Regtest, key).unwrap(), 1005),
+      (address(Network::Regtest, offset_key).unwrap(), 1007)
+    ];
+
+    let change_offset = scanner.register_offset(Scalar::random(&mut OsRng)).unwrap();
+    let change_key = key + (ProjectivePoint::GENERATOR * change_offset);
+    let change_addr = address(Network::Regtest, change_key).unwrap();
+
+    const FEE: u64 = 20;
+
+    // Create and sign the TX
+    let tx = SignableTransaction::new(
+      vec![output.clone(), offset_output.clone()],
+      &payments,
+      Some(change_addr.clone()),
+      None, // TODO: Test with data
+      FEE
+    ).unwrap();
+    let needed_fee = tx.needed_fee();
+
+    let mut machines = HashMap::new();
+    for i in (1 ..= THRESHOLD).map(|i| Participant::new(i).unwrap()) {
+      machines.insert(
+        i,
+        tx
+          .clone()
+          .multisig(keys[&i].clone(), RecommendedTranscript::new(b"bitcoin-serai Test Transaction"))
+          .unwrap()
+      );
+    }
+    let tx = sign_without_caching(&mut OsRng, machines, &[]);
+
+    assert_eq!(tx.output.len(), 3);
+
+    // Ensure we can scan it
+    let outputs = scanner.scan_transaction(&tx);
+    for (o, output) in outputs.iter().enumerate() {
+      assert_eq!(output.outpoint(), &OutPoint::new(tx.txid(), u32::try_from(o).unwrap()));
+      assert_eq!(&ReceivedOutput::read::<&[u8]>(&mut output.serialize().as_ref()).unwrap(), output);
+    }
+
+    assert_eq!(outputs[0].offset(), Scalar::ZERO);
+    assert_eq!(outputs[1].offset(), offset);
+    assert_eq!(outputs[2].offset(), change_offset);
+
+    // Make sure the payments were properly created
+    for ((output, scanned), payment) in tx.output.iter().zip(outputs.iter()).zip(payments.iter()) {
+      assert_eq!(output, &TxOut { script_pubkey: payment.0.script_pubkey(), value: payment.1 });
+      assert_eq!(scanned.value(), payment.1 );
+    }
+
+    // Make sure the change is correct
+    assert_eq!(needed_fee, u64::try_from(tx.weight()).unwrap() * FEE);
+    let input_value = output.value() + offset_output.value();
+    let output_value = tx.output.iter().map(|output| output.value).sum::<u64>();
+    assert_eq!(input_value - output_value, needed_fee);
+
+    let change_amount =
+      input_value - payments.iter().map(|payment| payment.1).sum::<u64>() - needed_fee;
+    assert_eq!(
+      tx.output[2],
+      TxOut { script_pubkey: change_addr.script_pubkey(), value: change_amount },
+    );
+
+    // This also tests send_raw_transaction and get_transaction, which the RPC test can't
+    // effectively test
+    rpc.send_raw_transaction(&tx).await.unwrap();
+    let mut hash = tx.txid().as_hash().into_inner();
+    hash.reverse();
+    assert_eq!(tx, rpc.get_transaction(&hash).await.unwrap());
+  }
+}
+
+// TODO: Test SignableTransaction error cases
+// TODO: Test x, x_only, make_even?

processor/src/coins/bitcoin.rs

@@ -79,6 +79,14 @@ impl OutputTrait for Output {
   fn id(&self) -> Self::Id {
     let mut res = OutputId::default();
     self.output.outpoint().consensus_encode(&mut res.as_mut()).unwrap();
+    debug_assert_eq!(
+      {
+        let mut outpoint = vec![];
+        self.output.outpoint().consensus_encode(&mut outpoint).unwrap();
+        outpoint
+      },
+      res.as_ref().to_vec()
+    );
     res
   }
 
@@ -189,6 +197,7 @@ lazy_static::lazy_static! {
   static ref CHANGE_OFFSET: Scalar = Secp256k1::hash_to_F(KEY_DST, b"change");
 }
 
+// Always construct the full scanner in order to ensure there's no collisions
 fn scanner(
   key: ProjectivePoint,
 ) -> (Scanner, HashMap<OutputType, Scalar>, HashMap<Vec<u8>, OutputType>) {
@@ -288,6 +297,8 @@ impl Coin for Bitcoin {
 
   fn tweak_keys(keys: &mut ThresholdKeys<Self::Curve>) {
     *keys = tweak_keys(keys);
+    // Also create a scanner to assert these keys, and all expected paths, are usable
+    scanner(keys.group_key());
   }
 
   fn address(key: ProjectivePoint) -> Address {
@@ -423,12 +434,11 @@ impl Coin for Bitcoin {
     &self,
     transaction: Self::SignableTransaction,
   ) -> Result<Self::TransactionMachine, CoinError> {
-    transaction
+    Ok(transaction
       .actual
       .clone()
-      .multisig(transaction.keys.clone(), transaction.transcript.clone())
+      .multisig(transaction.keys.clone(), transaction.transcript)
-      .await
+      .expect("used the wrong keys"))
-      .map_err(|_| CoinError::ConnectionError)
   }
 
   async fn publish_transaction(&self, tx: &Self::Transaction) -> Result<(), CoinError> {
@@ -466,10 +476,7 @@ impl Coin for Bitcoin {
       .rpc
       .rpc_call::<Vec<String>>(
         "generatetoaddress",
-        serde_json::json!([
+        serde_json::json!([1, BAddress::p2sh(&Script::new(), Network::Regtest).unwrap()]),
-          1,
-          BAddress::p2sh(&Script::new(), Network::Regtest).unwrap().to_string()
-        ]),
       )
       .await
       .unwrap();