From f93214012d3c1edf975ffd2f8f9e2cd6242455d3 Mon Sep 17 00:00:00 2001 From: Luke Parker Date: Tue, 21 May 2024 06:44:59 -0400 Subject: [PATCH] Use ScriptBuf over Address where possible --- coins/bitcoin/src/wallet/send.rs | 26 +++---- coins/bitcoin/tests/wallet.rs | 21 +++--- processor/src/networks/bitcoin.rs | 25 ++++--- processor/src/tests/literal/mod.rs | 2 +- substrate/client/src/networks/bitcoin.rs | 78 +++++++++------------ tests/full-stack/src/tests/mint_and_burn.rs | 14 ++-- tests/processor/src/networks.rs | 16 ++--- 7 files changed, 80 insertions(+), 102 deletions(-) diff --git a/coins/bitcoin/src/wallet/send.rs b/coins/bitcoin/src/wallet/send.rs index d547c56a..1980a554 100644 --- a/coins/bitcoin/src/wallet/send.rs +++ b/coins/bitcoin/src/wallet/send.rs @@ -18,7 +18,7 @@ use bitcoin::{ absolute::LockTime, script::{PushBytesBuf, ScriptBuf}, transaction::{Version, Transaction}, - OutPoint, Sequence, Witness, TxIn, Amount, TxOut, Address, + OutPoint, Sequence, Witness, TxIn, Amount, TxOut, }; use crate::{ @@ -61,7 +61,11 @@ pub struct SignableTransaction { } impl SignableTransaction { - fn calculate_weight(inputs: usize, payments: &[(Address, u64)], change: Option<&Address>) -> u64 { + fn calculate_weight( + inputs: usize, + payments: &[(ScriptBuf, u64)], + change: Option<&ScriptBuf>, + ) -> u64 { // Expand this a full transaction in order to use the bitcoin library's weight function let mut tx = Transaction { version: Version(2), @@ -86,14 +90,14 @@ impl SignableTransaction { // The script pub key is not of a fixed size and does have to be used here .map(|payment| TxOut { value: Amount::from_sat(payment.1), - script_pubkey: payment.0.script_pubkey(), + script_pubkey: payment.0.clone(), }) .collect(), }; if let Some(change) = change { // Use a 0 value since we're currently unsure what the change amount will be, and since // the value is fixed size (so any value could be used here) - tx.output.push(TxOut { value: Amount::ZERO, script_pubkey: change.script_pubkey() }); + tx.output.push(TxOut { value: Amount::ZERO, script_pubkey: change.clone() }); } u64::from(tx.weight()) } @@ -121,8 +125,8 @@ impl SignableTransaction { /// If data is specified, an OP_RETURN output will be added with it. pub fn new( mut inputs: Vec, - payments: &[(Address, u64)], - change: Option<&Address>, + payments: &[(ScriptBuf, u64)], + change: Option, data: Option>, fee_per_weight: u64, ) -> Result { @@ -159,10 +163,7 @@ impl SignableTransaction { let payment_sat = payments.iter().map(|payment| payment.1).sum::(); let mut tx_outs = payments .iter() - .map(|payment| TxOut { - value: Amount::from_sat(payment.1), - script_pubkey: payment.0.script_pubkey(), - }) + .map(|payment| TxOut { value: Amount::from_sat(payment.1), script_pubkey: payment.0.clone() }) .collect::>(); // Add the OP_RETURN output @@ -213,12 +214,11 @@ impl SignableTransaction { // If there's a change address, check if there's change to give it if let Some(change) = change { - let weight_with_change = Self::calculate_weight(tx_ins.len(), payments, Some(change)); + let weight_with_change = Self::calculate_weight(tx_ins.len(), payments, Some(&change)); let fee_with_change = fee_per_weight * weight_with_change; if let Some(value) = input_sat.checked_sub(payment_sat + fee_with_change) { if value >= DUST { - tx_outs - .push(TxOut { value: Amount::from_sat(value), script_pubkey: change.script_pubkey() }); + tx_outs.push(TxOut { value: Amount::from_sat(value), script_pubkey: change }); weight = weight_with_change; needed_fee = fee_with_change; } diff --git a/coins/bitcoin/tests/wallet.rs b/coins/bitcoin/tests/wallet.rs index 8aa2546e..9db004f5 100644 --- a/coins/bitcoin/tests/wallet.rs +++ b/coins/bitcoin/tests/wallet.rs @@ -192,7 +192,7 @@ async_sequential! { assert_eq!(output.offset(), Scalar::ZERO); let inputs = vec![output]; - let addr = || Address::from_script(&p2tr_script_buf(key).unwrap(), Network::Regtest).unwrap(); + let addr = || p2tr_script_buf(key).unwrap(); let payments = vec![(addr(), 1000)]; assert!(SignableTransaction::new(inputs.clone(), &payments, None, None, FEE).is_ok()); @@ -205,7 +205,7 @@ async_sequential! { // No change assert!(SignableTransaction::new(inputs.clone(), &[(addr(), 1000)], None, None, FEE).is_ok()); // Consolidation TX - assert!(SignableTransaction::new(inputs.clone(), &[], Some(&addr()), None, FEE).is_ok()); + assert!(SignableTransaction::new(inputs.clone(), &[], Some(addr()), None, FEE).is_ok()); // Data assert!(SignableTransaction::new(inputs.clone(), &[], None, Some(vec![]), FEE).is_ok()); // No outputs @@ -228,7 +228,7 @@ async_sequential! { ); assert_eq!( - SignableTransaction::new(inputs.clone(), &[], Some(&addr()), None, 0), + SignableTransaction::new(inputs.clone(), &[], Some(addr()), None, 0), Err(TransactionError::TooLowFee), ); @@ -260,20 +260,19 @@ async_sequential! { // Declare payments, change, fee let payments = [ - (Address::from_script(&p2tr_script_buf(key).unwrap(), Network::Regtest).unwrap(), 1005), - (Address::from_script(&p2tr_script_buf(offset_key).unwrap(), Network::Regtest).unwrap(), 1007) + (p2tr_script_buf(key).unwrap(), 1005), + (p2tr_script_buf(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::from_script(&p2tr_script_buf(change_key).unwrap(), Network::Regtest).unwrap(); + let change_addr = p2tr_script_buf(change_key).unwrap(); // Create and sign the TX let tx = SignableTransaction::new( vec![output.clone(), offset_output.clone()], &payments, - Some(&change_addr), + Some(change_addr.clone()), None, FEE ).unwrap(); @@ -298,7 +297,7 @@ async_sequential! { 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: Amount::from_sat(payment.1) }, + &TxOut { script_pubkey: payment.0.clone(), value: Amount::from_sat(payment.1) }, ); assert_eq!(scanned.value(), payment.1 ); } @@ -313,7 +312,7 @@ async_sequential! { input_value - payments.iter().map(|payment| payment.1).sum::() - needed_fee; assert_eq!( tx.output[2], - TxOut { script_pubkey: change_addr.script_pubkey(), value: Amount::from_sat(change_amount) }, + TxOut { script_pubkey: change_addr, value: Amount::from_sat(change_amount) }, ); // This also tests send_raw_transaction and get_transaction, which the RPC test can't @@ -343,7 +342,7 @@ async_sequential! { &SignableTransaction::new( vec![output], &[], - Some(&Address::from_script(&p2tr_script_buf(key).unwrap(), Network::Regtest).unwrap()), + Some(p2tr_script_buf(key).unwrap()), Some(data.clone()), FEE ).unwrap() diff --git a/processor/src/networks/bitcoin.rs b/processor/src/networks/bitcoin.rs index b7c6c2fb..e89c9138 100644 --- a/processor/src/networks/bitcoin.rs +++ b/processor/src/networks/bitcoin.rs @@ -20,8 +20,7 @@ use bitcoin_serai::{ key::{Parity, XOnlyPublicKey}, consensus::{Encodable, Decodable}, script::Instruction, - address::Address as BAddress, - Transaction, Block, Network as BNetwork, ScriptBuf, + Transaction, Block, ScriptBuf, opcodes::all::{OP_SHA256, OP_EQUALVERIFY}, }, wallet::{ @@ -454,7 +453,7 @@ impl Bitcoin { match BSignableTransaction::new( inputs.iter().map(|input| input.output.clone()).collect(), &payments, - change.as_ref().map(AsRef::as_ref), + change.clone().map(Into::into), None, fee.0, ) { @@ -535,6 +534,8 @@ impl Bitcoin { input_index: usize, private_key: &PrivateKey, ) -> ScriptBuf { + use bitcoin_serai::bitcoin::{Network as BNetwork, Address as BAddress}; + let public_key = PublicKey::from_private_key(SECP256K1, private_key); let main_addr = BAddress::p2pkh(public_key, BNetwork::Regtest); @@ -581,13 +582,9 @@ const MAX_OUTPUTS: usize = 520; fn address_from_key(key: ProjectivePoint) -> Address { Address::new( - BAddress::from_script( - &p2tr_script_buf(key).expect("creating address from key which isn't properly tweaked"), - BNetwork::Bitcoin, - ) - .expect("couldn't go from p2tr script buf to address"), + p2tr_script_buf(key).expect("creating address from key which isn't properly tweaked"), ) - .expect("couldn't create Serai-representable address for bitcoin address") + .expect("couldn't create Serai-representable address for P2TR script") } #[async_trait] @@ -733,9 +730,7 @@ impl Network for Bitcoin { } tx.unwrap().output.swap_remove(usize::try_from(input.previous_output.vout).unwrap()) }; - BAddress::from_script(&spent_output.script_pubkey, BNetwork::Bitcoin) - .ok() - .and_then(Address::new) + Address::new(spent_output.script_pubkey) }; let data = Self::extract_serai_data(tx); for output in &mut outputs { @@ -903,6 +898,8 @@ impl Network for Bitcoin { #[cfg(test)] async fn mine_block(&self) { + use bitcoin_serai::bitcoin::{Network as BNetwork, Address as BAddress}; + self .rpc .rpc_call::>( @@ -915,6 +912,8 @@ impl Network for Bitcoin { #[cfg(test)] async fn test_send(&self, address: Address) -> Block { + use bitcoin_serai::bitcoin::{Network as BNetwork, Address as BAddress}; + let secret_key = SecretKey::new(&mut rand_core::OsRng); let private_key = PrivateKey::new(secret_key, BNetwork::Regtest); let public_key = PublicKey::from_private_key(SECP256K1, &private_key); @@ -939,7 +938,7 @@ impl Network for Bitcoin { }], output: vec![TxOut { value: tx.output[0].value - BAmount::from_sat(10000), - script_pubkey: address.as_ref().script_pubkey(), + script_pubkey: address.clone().into(), }], }; tx.input[0].script_sig = Self::sign_btc_input_for_p2pkh(&tx, 0, &private_key); diff --git a/processor/src/tests/literal/mod.rs b/processor/src/tests/literal/mod.rs index 238dde1a..15c27b8c 100644 --- a/processor/src/tests/literal/mod.rs +++ b/processor/src/tests/literal/mod.rs @@ -135,7 +135,7 @@ mod bitcoin { }], output: vec![TxOut { value: tx.output[0].value - BAmount::from_sat(10000), - script_pubkey: serai_btc_address.as_ref().script_pubkey(), + script_pubkey: serai_btc_address.into(), }], }; diff --git a/substrate/client/src/networks/bitcoin.rs b/substrate/client/src/networks/bitcoin.rs index 10965bdf..9f5ff1dd 100644 --- a/substrate/client/src/networks/bitcoin.rs +++ b/substrate/client/src/networks/bitcoin.rs @@ -7,19 +7,23 @@ use bitcoin::{ PubkeyHash, ScriptHash, network::Network, WitnessVersion, WitnessProgram, ScriptBuf, - address::{AddressType, NetworkChecked, Address as BAddressGeneric}, + address::{AddressType, NetworkChecked, Address as BAddress}, }; -type BAddress = BAddressGeneric; - #[derive(Clone, Eq, Debug)] -pub struct Address(BAddress); +pub struct Address(ScriptBuf); impl PartialEq for Address { fn eq(&self, other: &Self) -> bool { // Since Serai defines the Bitcoin-address specification as a variant of the script alone, // define equivalency as the script alone - self.0.script_pubkey() == other.0.script_pubkey() + self.0 == other.0 + } +} + +impl From
for ScriptBuf { + fn from(addr: Address) -> ScriptBuf { + addr.0 } } @@ -27,10 +31,11 @@ impl FromStr for Address { type Err = (); fn from_str(str: &str) -> Result { Address::new( - BAddressGeneric::from_str(str) + BAddress::from_str(str) .map_err(|_| ())? .require_network(Network::Bitcoin) - .map_err(|_| ())?, + .map_err(|_| ())? + .script_pubkey(), ) .ok_or(()) } @@ -38,7 +43,9 @@ impl FromStr for Address { impl fmt::Display for Address { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - self.0.fmt(f) + BAddress::::from_script(&self.0, Network::Bitcoin) + .map_err(|_| fmt::Error)? + .fmt(f) } } @@ -57,45 +64,40 @@ impl TryFrom> for Address { fn try_from(data: Vec) -> Result { Ok(Address(match EncodedAddress::decode(&mut data.as_ref()).map_err(|_| ())? { EncodedAddress::P2PKH(hash) => { - BAddress::p2pkh(PubkeyHash::from_raw_hash(Hash::from_byte_array(hash)), Network::Bitcoin) + ScriptBuf::new_p2pkh(&PubkeyHash::from_raw_hash(Hash::from_byte_array(hash))) } EncodedAddress::P2SH(hash) => { - let script_hash = ScriptHash::from_raw_hash(Hash::from_byte_array(hash)); - let res = - BAddress::from_script(&ScriptBuf::new_p2sh(&script_hash), Network::Bitcoin).unwrap(); - debug_assert_eq!(res.script_hash(), Some(script_hash)); - res + ScriptBuf::new_p2sh(&ScriptHash::from_raw_hash(Hash::from_byte_array(hash))) + } + EncodedAddress::P2WPKH(hash) => { + ScriptBuf::new_witness_program(&WitnessProgram::new(WitnessVersion::V0, &hash).unwrap()) + } + EncodedAddress::P2WSH(hash) => { + ScriptBuf::new_witness_program(&WitnessProgram::new(WitnessVersion::V0, &hash).unwrap()) + } + EncodedAddress::P2TR(key) => { + ScriptBuf::new_witness_program(&WitnessProgram::new(WitnessVersion::V1, &key).unwrap()) } - EncodedAddress::P2WPKH(hash) => BAddress::from_witness_program( - WitnessProgram::new(WitnessVersion::V0, &hash).unwrap(), - Network::Bitcoin, - ), - EncodedAddress::P2WSH(hash) => BAddress::from_witness_program( - WitnessProgram::new(WitnessVersion::V0, &hash).unwrap(), - Network::Bitcoin, - ), - EncodedAddress::P2TR(key) => BAddress::from_witness_program( - WitnessProgram::new(WitnessVersion::V1, &key).unwrap(), - Network::Bitcoin, - ), })) } } fn try_to_vec(addr: &Address) -> Result, ()> { let witness_program = |addr: &Address| { - let script = addr.0.script_pubkey(); - let program_push = script.as_script().instructions().last().ok_or(())?.map_err(|_| ())?; + let program_push = addr.0.as_script().instructions().last().ok_or(())?.map_err(|_| ())?; let program = program_push.push_bytes().ok_or(())?.as_bytes(); Ok::<_, ()>(program.to_vec()) }; + + let parsed_addr = + BAddress::::from_script(&addr.0, Network::Bitcoin).map_err(|_| ())?; Ok( - (match addr.0.address_type() { + (match parsed_addr.address_type() { Some(AddressType::P2pkh) => { - EncodedAddress::P2PKH(*addr.0.pubkey_hash().unwrap().as_raw_hash().as_byte_array()) + EncodedAddress::P2PKH(*parsed_addr.pubkey_hash().unwrap().as_raw_hash().as_byte_array()) } Some(AddressType::P2sh) => { - EncodedAddress::P2SH(*addr.0.script_hash().unwrap().as_raw_hash().as_byte_array()) + EncodedAddress::P2SH(*parsed_addr.script_hash().unwrap().as_raw_hash().as_byte_array()) } Some(AddressType::P2wpkh) => { let program = witness_program(addr)?; @@ -127,20 +129,8 @@ impl From
for Vec { } } -impl From
for BAddress { - fn from(addr: Address) -> BAddress { - addr.0 - } -} - -impl AsRef for Address { - fn as_ref(&self) -> &BAddress { - &self.0 - } -} - impl Address { - pub fn new(address: BAddress) -> Option { + pub fn new(address: ScriptBuf) -> Option { let res = Self(address); if try_to_vec(&res).is_ok() { return Some(res); diff --git a/tests/full-stack/src/tests/mint_and_burn.rs b/tests/full-stack/src/tests/mint_and_burn.rs index e1153bae..4093e47d 100644 --- a/tests/full-stack/src/tests/mint_and_burn.rs +++ b/tests/full-stack/src/tests/mint_and_burn.rs @@ -454,19 +454,17 @@ async fn mint_and_burn_test() { // Create a random Bitcoin/Monero address let bitcoin_addr = { - use bitcoin_serai::bitcoin::{network::Network, key::PublicKey, address::Address}; - // Uses Network::Bitcoin since it doesn't actually matter, Serai strips it out - // TODO: Move Serai to ScriptBuf from Address - Address::p2pkh( - loop { + use bitcoin_serai::bitcoin::{key::PublicKey, ScriptBuf}; + ScriptBuf::new_p2pkh( + &(loop { let mut bytes = [0; 33]; OsRng.fill_bytes(&mut bytes); bytes[0] %= 4; if let Ok(key) = PublicKey::from_slice(&bytes) { break key; } - }, - Network::Bitcoin, + }) + .pubkey_hash(), ) }; @@ -559,7 +557,7 @@ async fn mint_and_burn_test() { let received_output = block.txdata[1] .output .iter() - .find(|output| output.script_pubkey == bitcoin_addr.script_pubkey()) + .find(|output| output.script_pubkey == bitcoin_addr) .unwrap(); let tx_fee = 1_100_000_00 - diff --git a/tests/processor/src/networks.rs b/tests/processor/src/networks.rs index 81c18bfa..3ad2b59a 100644 --- a/tests/processor/src/networks.rs +++ b/tests/processor/src/networks.rs @@ -261,7 +261,6 @@ impl Wallet { OutPoint, Sequence, Witness, TxIn, Amount, TxOut, absolute::LockTime, transaction::{Version, Transaction}, - Network, Address, }; const AMOUNT: u64 = 100000000; @@ -281,13 +280,11 @@ impl Wallet { }, TxOut { value: Amount::from_sat(AMOUNT), - script_pubkey: Address::p2tr_tweaked( + script_pubkey: ScriptBuf::new_p2tr_tweaked( TweakedPublicKey::dangerous_assume_tweaked( XOnlyPublicKey::from_slice(&to[1 ..]).unwrap(), ), - Network::Bitcoin, - ) - .script_pubkey(), + ), }, ], }; @@ -521,13 +518,8 @@ impl Wallet { match self { Wallet::Bitcoin { public_key, .. } => { - use bitcoin_serai::bitcoin::{Network, Address}; - ExternalAddress::new( - networks::bitcoin::Address::new(Address::p2pkh(public_key, Network::Regtest)) - .unwrap() - .into(), - ) - .unwrap() + use bitcoin_serai::bitcoin::ScriptBuf; + ExternalAddress::new(ScriptBuf::new_p2pkh(&public_key.pubkey_hash()).into()).unwrap() } Wallet::Ethereum { key, .. } => ExternalAddress::new( ethereum_serai::crypto::address(&(ciphersuite::Secp256k1::generator() * key)).into(),