Ecosystem/serai
Ecosystem/serai
1
0
Fork 0
mirror of https://github.com/serai-dex/serai.git synced 2024-12-31 16:09:47 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 1

Merge branch 'bitcoin-audit' into develop

Browse source
This commit is contained in:
Luke Parker 2023-08-21 01:16:50 -04:00
commit d2a0ff13f2
No known key found for this signature in database
6 changed files with 185 additions and 52 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

16
coins/bitcoin/src/crypto.rs
View file

@ -32,13 +32,14 @@ pub fn x(key: &ProjectivePoint) -> [u8; 32] {
(*encoded.x().expect("point at infinity")).into()
}
/// Convert a non-infinite even point to a XOnlyPublicKey. Panics on invalid input.
/// Convert a non-infinity even point to a XOnlyPublicKey. Panics on invalid input.
pub fn x_only(key: &ProjectivePoint) -> XOnlyPublicKey {
XOnlyPublicKey::from_slice(&x(key)).unwrap()
XOnlyPublicKey::from_slice(&x(key)).expect("x_only was passed a point which was infinity or odd")
}
/// Make a point even by adding the generator until it is even. Returns the even point and the
/// amount of additions required.
/// Make a point even by adding the generator until it is even.
///
/// Returns the even point and the amount of additions required.
pub fn make_even(mut key: ProjectivePoint) -> (ProjectivePoint, u64) {
let mut c = 0;
while key.to_encoded_point(true).tag() == Tag::CompressedOddY {
@ -51,8 +52,10 @@ pub fn make_even(mut key: ProjectivePoint) -> (ProjectivePoint, u64) {
/// A BIP-340 compatible HRAm for use with the modular-frost Schnorr Algorithm.
///
/// If passed an odd nonce, it will have the generator added until it is even.
///
/// If the key is odd, this will panic.
#[derive(Clone, Copy, Debug)]
pub struct Hram {}
pub struct Hram;
lazy_static! {
static ref TAG_HASH: [u8; 32] = Sha256::digest(b"BIP0340/challenge").into();
@ -145,7 +148,8 @@ impl<T: Sync + Clone + Debug + Transcript> Algorithm<Secp256k1> for Schnorr<T> {
// s = r + cx. Since we added to the r, add to s
sig.s += Scalar::from(offset);
// Convert to a secp256k1 signature
Signature::from_slice(&sig.serialize()[1 ..]).unwrap()
Signature::from_slice(&sig.serialize()[1 ..])
.expect("couldn't convert SchnorrSignature to Signature")
})
}

91
coins/bitcoin/src/rpc.rs
View file

@ -1,10 +1,13 @@
use core::fmt::Debug;
use std::collections::HashSet;
use thiserror::Error;
use serde::{Deserialize, de::DeserializeOwned};
use serde_json::json;
use reqwest::Client;
use bitcoin::{
hashes::{Hash, hex::FromHex},
consensus::encode,
@ -26,7 +29,10 @@ enum RpcResponse<T> {
/// A minimal asynchronous Bitcoin RPC client.
#[derive(Clone, Debug)]
pub struct Rpc(String);
pub struct Rpc {
client: Client,
url: String,
}
#[derive(Clone, PartialEq, Eq, Debug, Error)]
pub enum RpcError {
@ -34,15 +40,64 @@ pub enum RpcError {
ConnectionError,
#[error("request had an error: {0:?}")]
RequestError(Error),
#[error("node sent an invalid response")]
InvalidResponse,
#[error("node replied with invalid JSON")]
InvalidJson(serde_json::error::Category),
#[error("node sent an invalid response ({0})")]
InvalidResponse(&'static str),
#[error("node was missing expected methods")]
MissingMethods(HashSet<&'static str>),
}
impl Rpc {
/// Create a new connection to a Bitcoin RPC.
///
/// An RPC call is performed to ensure the node is reachable (and that an invalid URL wasn't
/// provided).
///
/// Additionally, a set of expected methods is checked to be offered by the Bitcoin RPC. If these
/// methods aren't provided, an error with the missing methods is returned. This ensures all RPC
/// routes explicitly provided by this library are at least possible.
///
/// Each individual RPC route may still fail at time-of-call, regardless of the arguments
/// provided to this library, if the RPC has an incompatible argument layout. That is not checked
/// at time of RPC creation.
pub async fn new(url: String) -> Result<Rpc, RpcError> {
let rpc = Rpc(url);
let rpc = Rpc { client: Client::new(), url };
// Make an RPC request to verify the node is reachable and sane
rpc.get_latest_block_number().await?;
let res: String = rpc.rpc_call("help", json!([])).await?;
// Verify all methods we expect are present
// If we had a more expanded RPC, due to differences in RPC versions, it wouldn't make sense to
// error if all methods weren't present
// We only provide a very minimal set of methods which have been largely consistent, hence why
// this is sane
let mut expected_methods = HashSet::from([
"help",
"getblockcount",
"getblockhash",
"getblockheader",
"getblock",
"sendrawtransaction",
"getrawtransaction",
]);
for line in res.split('\n') {
// This doesn't check if the arguments are as expected
// This is due to Bitcoin supporting a large amount of optional arguments, which
// occassionally change, with their own mechanism of text documentation, making matching off
// it a quite involved task
// Instead, once we've confirmed the methods are present, we assume our arguments are aligned
// Else we'll error at time of call
if expected_methods.remove(line.split(' ').next().unwrap_or("")) &&
expected_methods.is_empty()
{
break;
}
}
if !expected_methods.is_empty() {
Err(RpcError::MissingMethods(expected_methods))?;
};
Ok(rpc)
}
@ -52,9 +107,9 @@ impl Rpc {
method: &str,
params: serde_json::Value,
) -> Result<Response, RpcError> {
let client = reqwest::Client::new();
let res = client
.post(&self.0)
let res = self
.client
.post(&self.url)
.json(&json!({ "jsonrpc": "2.0", "method": method, "params": params }))
.send()
.await
@ -64,7 +119,7 @@ impl Rpc {
.map_err(|_| RpcError::ConnectionError)?;
let res: RpcResponse<Response> =
serde_json::from_str(&res).map_err(|_| RpcError::InvalidResponse)?;
serde_json::from_str(&res).map_err(|e| RpcError::InvalidJson(e.classify()))?;
match res {
RpcResponse::Ok { result } => Ok(result),
RpcResponse::Err { error } => Err(RpcError::RequestError(error)),
@ -107,13 +162,15 @@ impl Rpc {
/// Get a block by its hash.
pub async fn get_block(&self, hash: &[u8; 32]) -> Result<Block, RpcError> {
let hex = self.rpc_call::<String>("getblock", json!([hex::encode(hash), 0])).await?;
let bytes: Vec<u8> = FromHex::from_hex(&hex).map_err(|_| RpcError::InvalidResponse)?;
let block: Block = encode::deserialize(&bytes).map_err(|_| RpcError::InvalidResponse)?;
let bytes: Vec<u8> = FromHex::from_hex(&hex)
.map_err(|_| RpcError::InvalidResponse("node didn't use hex to encode the block"))?;
let block: Block = encode::deserialize(&bytes)
.map_err(|_| RpcError::InvalidResponse("node sent an improperly serialized block"))?;
let mut block_hash = *block.block_hash().as_raw_hash().as_byte_array();
block_hash.reverse();
if hash != &block_hash {
Err(RpcError::InvalidResponse)?;
Err(RpcError::InvalidResponse("node replied with a different block"))?;
}
Ok(block)
@ -123,7 +180,7 @@ impl Rpc {
pub async fn send_raw_transaction(&self, tx: &Transaction) -> Result<Txid, RpcError> {
let txid = self.rpc_call("sendrawtransaction", json!([encode::serialize_hex(tx)])).await?;
if txid != tx.txid() {
Err(RpcError::InvalidResponse)?;
Err(RpcError::InvalidResponse("returned TX ID inequals calculated TX ID"))?;
}
Ok(txid)
}
@ -131,13 +188,15 @@ impl Rpc {
/// Get a transaction by its hash.
pub async fn get_transaction(&self, hash: &[u8; 32]) -> Result<Transaction, RpcError> {
let hex = self.rpc_call::<String>("getrawtransaction", json!([hex::encode(hash)])).await?;
let bytes: Vec<u8> = FromHex::from_hex(&hex).map_err(|_| RpcError::InvalidResponse)?;
let tx: Transaction = encode::deserialize(&bytes).map_err(|_| RpcError::InvalidResponse)?;
let bytes: Vec<u8> = FromHex::from_hex(&hex)
.map_err(|_| RpcError::InvalidResponse("node didn't use hex to encode the transaction"))?;
let tx: Transaction = encode::deserialize(&bytes)
.map_err(|_| RpcError::InvalidResponse("node sent an improperly serialized transaction"))?;
let mut tx_hash = *tx.txid().as_raw_hash().as_byte_array();
tx_hash.reverse();
if hash != &tx_hash {
Err(RpcError::InvalidResponse)?;
Err(RpcError::InvalidResponse("node replied with a different transaction"))?;
}
Ok(tx)

32
coins/bitcoin/src/wallet/mod.rs
View file

@ -15,7 +15,8 @@ use frost::{
use bitcoin::{
consensus::encode::{Decodable, serialize},
key::TweakedPublicKey,
OutPoint, ScriptBuf, TxOut, Transaction, Block, Network, Address,
address::Payload,
OutPoint, ScriptBuf, TxOut, Transaction, Block,
};
use crate::crypto::{x_only, make_even};
@ -24,18 +25,23 @@ mod send;
pub use send::*;
/// Tweak keys to ensure they're usable with Bitcoin.
///
/// Taproot keys, which these keys are used as, must be even. This offsets the keys until they're
/// even.
pub fn tweak_keys(keys: &ThresholdKeys<Secp256k1>) -> ThresholdKeys<Secp256k1> {
let (_, offset) = make_even(keys.group_key());
keys.offset(Scalar::from(offset))
}
/// Return the Taproot address for a public key.
pub fn address(network: Network, key: ProjectivePoint) -> Option<Address> {
/// Return the Taproot address payload for a public key.
///
/// If the key is odd, this will return None.
pub fn address_payload(key: ProjectivePoint) -> Option<Payload> {
if key.to_encoded_point(true).tag() != Tag::CompressedEvenY {
return None;
}
Some(Address::p2tr_tweaked(TweakedPublicKey::dangerous_assume_tweaked(x_only(&key)), network))
Some(Payload::p2tr_tweaked(TweakedPublicKey::dangerous_assume_tweaked(x_only(&key))))
}
/// A spendable output.
@ -104,8 +110,7 @@ impl Scanner {
/// Returns None if this key can't be scanned for.
pub fn new(key: ProjectivePoint) -> Option<Scanner> {
let mut scripts = HashMap::new();
// Uses Network::Bitcoin since network is irrelevant here
scripts.insert(address(Network::Bitcoin, key)?.script_pubkey(), Scalar::ZERO);
scripts.insert(address_payload(key)?.script_pubkey(), Scalar::ZERO);
Some(Scanner { key, scripts })
}
@ -114,9 +119,15 @@ impl Scanner {
/// Due to Bitcoin's requirement that points are even, not every offset may be used.
/// If an offset isn't usable, it will be incremented until it is. If this offset is already
/// present, None is returned. Else, Some(offset) will be, with the used offset.
///
/// This means offsets are surjective, not bijective, and the order offsets are registered in
/// may determine the validity of future offsets.
pub fn register_offset(&mut self, mut offset: Scalar) -> Option<Scalar> {
// This loop will terminate as soon as an even point is found, with any point having a ~50%
// chance of being even
// That means this should terminate within a very small amount of iterations
loop {
match address(Network::Bitcoin, self.key + (ProjectivePoint::GENERATOR * offset)) {
match address_payload(self.key + (ProjectivePoint::GENERATOR * offset)) {
Some(address) => {
let script = address.script_pubkey();
if self.scripts.contains_key(&script) {
@ -134,11 +145,16 @@ impl Scanner {
pub fn scan_transaction(&self, tx: &Transaction) -> Vec<ReceivedOutput> {
let mut res = vec![];
for (vout, output) in tx.output.iter().enumerate() {
// If the vout index exceeds 2**32, stop scanning outputs
let Ok(vout) = u32::try_from(vout) else {
break
};
if let Some(offset) = self.scripts.get(&output.script_pubkey) {
res.push(ReceivedOutput {
offset: *offset,
output: output.clone(),
outpoint: OutPoint::new(tx.txid(), u32::try_from(vout).unwrap()),
outpoint: OutPoint::new(tx.txid(), vout),
});
}
}

56
coins/bitcoin/src/wallet/send.rs
View file

@ -16,12 +16,12 @@ use bitcoin::{
sighash::{TapSighashType, SighashCache, Prevouts},
absolute::LockTime,
script::{PushBytesBuf, ScriptBuf},
OutPoint, Sequence, Witness, TxIn, TxOut, Transaction, Network, Address,
OutPoint, Sequence, Witness, TxIn, TxOut, Transaction, Address,
};
use crate::{
crypto::Schnorr,
wallet::{address, ReceivedOutput},
wallet::{ReceivedOutput, address_payload},
};
#[rustfmt::skip]
@ -29,8 +29,22 @@ use crate::{
const MAX_STANDARD_TX_WEIGHT: u64 = 400_000;
#[rustfmt::skip]
//https://github.com/bitcoin/bitcoin/blob/a245429d680eb95cf4c0c78e58e63e3f0f5d979a/src/test/transaction_tests.cpp#L815-L816
const DUST: u64 = 674;
// https://github.com/bitcoin/bitcoin/blob/306ccd4927a2efe325c8d84be1bdb79edeb29b04/src/policy/policy.cpp#L26-L63
// As the above notes, a lower amount may not be considered dust if contained in a SegWit output
// This doesn't bother with delineation due to how marginal these values are, and because it isn't
// worth the complexity to implement differentation
const DUST: u64 = 546;
#[rustfmt::skip]
// The constant is from:
// https://github.com/bitcoin/bitcoin/blob/306ccd4927a2efe325c8d84be1bdb79edeb29b04/src/policy/policy.h#L56-L57
// It's used here:
// https://github.com/bitcoin/bitcoin/blob/296735f7638749906243c9e203df7bd024493806/src/net_processing.cpp#L5386-L5390
// Peers won't relay TXs below the filter's fee rate, yet they calculate the fee not against weight yet vsize
// https://github.com/bitcoin/bitcoin/blob/296735f7638749906243c9e203df7bd024493806/src/net_processing.cpp#L5721-L5732
// And then the fee itself is fee per thousand units, not fee per unit
// https://github.com/bitcoin/bitcoin/blob/306ccd4927a2efe325c8d84be1bdb79edeb29b04/src/policy/feerate.cpp#L23-L37
const MIN_FEE_PER_KILO_VSIZE: u64 = 1000;
#[derive(Clone, PartialEq, Eq, Debug, Error)]
pub enum TransactionError {
@ -42,6 +56,8 @@ pub enum TransactionError {
DustPayment,
#[error("too much data was specified")]
TooMuchData,
#[error("fee was too low to pass the default minimum fee rate")]
TooLowFee,
#[error("not enough funds for these payments")]
NotEnoughFunds,
#[error("transaction was too large")]
@ -163,6 +179,26 @@ impl SignableTransaction {
let mut weight = Self::calculate_weight(tx_ins.len(), payments, None);
let mut needed_fee = fee_per_weight * weight;
// "Virtual transaction size" is weight ceildiv 4 per
// https://github.com/bitcoin/bips/blob/master/bip-0141.mediawiki
// https://github.com/bitcoin/bitcoin/blob/306ccd4927a2efe325c8d84be1bdb79edeb29b04/
// src/policy/policy.cpp#L295-L298
// implements this as expected
// Technically, it takes whatever's greater, the weight or the amount of signature operatons
// multiplied by DEFAULT_BYTES_PER_SIGOP (20)
// We only use 1 signature per input, and our inputs have a weight exceeding 20
// Accordingly, our inputs' weight will always be greater than the cost of the signature ops
let vsize = (weight + 3) / 4;
// Technically, if there isn't change, this TX may still pay enough of a fee to pass the
// minimum fee. Such edge cases aren't worth programming when they go against intent, as the
// specified fee rate is too low to be valid
if needed_fee < ((MIN_FEE_PER_KILO_VSIZE * vsize) / 1000) {
Err(TransactionError::TooLowFee)?;
}
if input_sat < (payment_sat + needed_fee) {
Err(TransactionError::NotEnoughFunds)?;
}
@ -221,12 +257,12 @@ impl SignableTransaction {
let mut sigs = vec![];
for i in 0 .. tx.input.len() {
let mut transcript = transcript.clone();
// This unwrap is safe since any transaction with this many inputs violates the maximum
// size allowed under standards, which this lib will error on creation of
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
{
if address_payload(offset.group_key())?.script_pubkey() != self.prevouts[i].script_pubkey {
None?;
}
@ -243,7 +279,7 @@ impl SignableTransaction {
/// A FROST signing machine to produce a Bitcoin transaction.
///
/// This does not support caching its preprocess. When sign is called, the message must be empty.
/// This will panic if it isn't.
/// This will panic if either `cache` is called or the message isn't empty.
pub struct TransactionMachine {
tx: SignableTransaction,
sigs: Vec<AlgorithmMachine<Secp256k1, Schnorr<RecommendedTranscript>>>,
@ -339,7 +375,9 @@ impl SignMachine<Transaction> for TransactionSignMachine {
commitments[i].clone(),
cache
.taproot_key_spend_signature_hash(i, &prevouts, TapSighashType::Default)
.unwrap()
// This should never happen since the inputs align with the TX the cache was
// constructed with, and because i is always < prevouts.len()
.expect("taproot_key_spend_signature_hash failed to return a hash")
.as_ref(),
)?;
shares.push(share);

28
coins/bitcoin/tests/wallet.rs
View file

@ -22,9 +22,12 @@ use bitcoin_serai::{
hashes::Hash as HashTrait,
blockdata::opcodes::all::OP_RETURN,
script::{PushBytesBuf, Instruction, Instructions, Script},
address::NetworkChecked,
OutPoint, TxOut, Transaction, Network, Address,
},
wallet::{tweak_keys, address, ReceivedOutput, Scanner, TransactionError, SignableTransaction},
wallet::{
tweak_keys, address_payload, ReceivedOutput, Scanner, TransactionError, SignableTransaction,
},
rpc::Rpc,
};
@ -43,7 +46,10 @@ async fn send_and_get_output(rpc: &Rpc, scanner: &Scanner, key: ProjectivePoint)
rpc
.rpc_call::<Vec<String>>(
"generatetoaddress",
serde_json::json!([1, address(Network::Regtest, key).unwrap()]),
serde_json::json!([
1,
Address::<NetworkChecked>::new(Network::Regtest, address_payload(key).unwrap())
]),
)
.await
.unwrap();
@ -187,7 +193,7 @@ async_sequential! {
assert_eq!(output.offset(), Scalar::ZERO);
let inputs = vec![output];
let addr = || address(Network::Regtest, key).unwrap();
let addr = || Address::<NetworkChecked>::new(Network::Regtest, address_payload(key).unwrap());
let payments = vec![(addr(), 1000)];
assert!(SignableTransaction::new(inputs.clone(), &payments, None, None, FEE).is_ok());
@ -222,13 +228,18 @@ async_sequential! {
Err(TransactionError::TooMuchData),
);
assert_eq!(
SignableTransaction::new(inputs.clone(), &[], Some(addr()), None, 0),
Err(TransactionError::TooLowFee),
);
assert_eq!(
SignableTransaction::new(inputs.clone(), &[(addr(), inputs[0].value() * 2)], None, None, FEE),
Err(TransactionError::NotEnoughFunds),
);
assert_eq!(
SignableTransaction::new(inputs, &vec![(addr(), 1000); 10000], None, None, 0),
SignableTransaction::new(inputs, &vec![(addr(), 1000); 10000], None, None, FEE),
Err(TransactionError::TooLargeTransaction),
);
}
@ -250,13 +261,14 @@ async_sequential! {
// Declare payments, change, fee
let payments = [
(address(Network::Regtest, key).unwrap(), 1005),
(address(Network::Regtest, offset_key).unwrap(), 1007)
(Address::<NetworkChecked>::new(Network::Regtest, address_payload(key).unwrap()), 1005),
(Address::<NetworkChecked>::new(Network::Regtest, address_payload(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();
let change_addr =
Address::<NetworkChecked>::new(Network::Regtest, address_payload(change_key).unwrap());
// Create and sign the TX
let tx = SignableTransaction::new(
@ -327,7 +339,7 @@ async_sequential! {
SignableTransaction::new(
vec![output],
&[],
address(Network::Regtest, key),
Some(Address::<NetworkChecked>::new(Network::Regtest, address_payload(key).unwrap())),
Some(data.clone()),
FEE
).unwrap()

14
processor/src/networks/bitcoin.rs
View file

@ -17,10 +17,11 @@ use bitcoin_serai::{
hashes::Hash as HashTrait,
consensus::{Encodable, Decodable},
script::Instruction,
address::{NetworkChecked, Address as BAddress},
OutPoint, Transaction, Block, Network as BitcoinNetwork,
},
wallet::{
tweak_keys, address, ReceivedOutput, Scanner, TransactionError,
tweak_keys, address_payload, ReceivedOutput, Scanner, TransactionError,
SignableTransaction as BSignableTransaction, TransactionMachine,
},
rpc::{RpcError, Rpc},
@ -33,7 +34,7 @@ use bitcoin_serai::bitcoin::{
sighash::{EcdsaSighashType, SighashCache},
script::{PushBytesBuf, Builder},
absolute::LockTime,
Sequence, Script, Witness, TxIn, TxOut, Address as BAddress,
Sequence, Script, Witness, TxIn, TxOut,
};
use serai_client::{
@ -326,7 +327,7 @@ impl Network for Bitcoin {
}
fn address(key: ProjectivePoint) -> Address {
Address(address(BitcoinNetwork::Bitcoin, key).unwrap())
Address(BAddress::<NetworkChecked>::new(BitcoinNetwork::Bitcoin, address_payload(key).unwrap()))
}
fn branch_address(key: ProjectivePoint) -> Self::Address {
@ -482,7 +483,12 @@ impl Network for Bitcoin {
}
// No outputs left and the change isn't worth enough
Err(TransactionError::NoOutputs) => None,
// amortize_fee removes payments which fall below the dust threshold
Err(TransactionError::DustPayment) => panic!("dust payment despite removing dust"),
Err(TransactionError::TooMuchData) => panic!("too much data despite not specifying data"),
Err(TransactionError::TooLowFee) => {
panic!("created a transaction whose fee is below the minimum")
}
Err(TransactionError::NotEnoughFunds) => {
if tx_fee.is_none() {
// Mot even enough funds to pay the fee
@ -491,8 +497,6 @@ impl Network for Bitcoin {
panic!("not enough funds for bitcoin TX despite amortizing the fee")
}
}
// amortize_fee removes payments which fall below the dust threshold
Err(TransactionError::DustPayment) => panic!("dust payment despite removing dust"),
Err(TransactionError::TooLargeTransaction) => {
panic!("created a too large transaction despite limiting inputs/outputs")
}