From 67b7325582172ff10d624bd6d38c553ca0f26c71 Mon Sep 17 00:00:00 2001 From: teor Date: Fri, 11 Nov 2022 13:42:05 +1000 Subject: [PATCH] fix(consensus): Check network and P2SH addresses for mining config and funding streams (#5620) * Check network and P2SH addresses for mining config and funding streams * Inline format string variables --- zebra-chain/src/transparent/address.rs | 5 +++ .../src/block/subsidy/funding_streams.rs | 31 ++++++++--------- .../block/subsidy/funding_streams/tests.rs | 34 ++++++++++++++++--- zebra-rpc/src/server.rs | 16 +++++++++ 4 files changed, 66 insertions(+), 20 deletions(-) diff --git a/zebra-chain/src/transparent/address.rs b/zebra-chain/src/transparent/address.rs index bc696c6f0..c21af557a 100644 --- a/zebra-chain/src/transparent/address.rs +++ b/zebra-chain/src/transparent/address.rs @@ -223,6 +223,11 @@ impl Address { } } + /// Returns `true` if the address is `PayToScriptHash`, and `false` if it is `PayToPublicKeyHash`. + pub fn is_script_hash(&self) -> bool { + matches!(self, Address::PayToScriptHash { .. }) + } + /// Returns the hash bytes for this address, regardless of the address type. /// /// # Correctness diff --git a/zebra-consensus/src/block/subsidy/funding_streams.rs b/zebra-consensus/src/block/subsidy/funding_streams.rs index ddac2f948..867f89081 100644 --- a/zebra-consensus/src/block/subsidy/funding_streams.rs +++ b/zebra-consensus/src/block/subsidy/funding_streams.rs @@ -116,35 +116,34 @@ pub fn funding_stream_address( .expect("there is always another hash map as value for a given valid network") .get(&receiver) .expect("in the inner hash map there is always a vector of strings with addresses")[index]; - Address::from_str(address).expect("Address should deserialize") + Address::from_str(address).expect("address should deserialize") } -/// Given a funding stream address, create a script and check if it is the same +/// Given a funding stream P2SH address, create a script and check if it is the same /// as the given lock_script as described in [protocol specification §7.10][7.10] /// /// [7.10]: https://zips.z.cash/protocol/protocol.pdf#fundingstreams pub fn check_script_form(lock_script: &Script, address: Address) -> bool { - // TODO: Verify P2SH multisig funding stream addresses (#5577). - // As of NU5, the funding streams do not use multisig addresses, - // so this is optional. - // - // # Consensus - // - // > The standard redeem script hash is specified in [Bitcoin-Multisig] for P2SH multisig - // > addresses... - // [protocol specification §7.10][7.10] - // - // [7.10]: https://zips.z.cash/protocol/protocol.pdf#fundingstreams - // [Bitcoin-Multisig]: https://developer.bitcoin.org/ devguide/transactions.html#multisig + assert!( + address.is_script_hash(), + "incorrect funding stream address constant: {address} \ + Zcash only supports transparent 'pay to script hash' (P2SH) addresses", + ); - // Verify a Bitcoin P2SH single address. + // Verify a Bitcoin P2SH single or multisig address. let standard_script_hash = new_coinbase_script(address); lock_script == &standard_script_hash } -/// Returns a new funding stream coinbase output lock script, which pays to `address`. +/// Returns a new funding stream coinbase output lock script, which pays to the P2SH `address`. pub fn new_coinbase_script(address: Address) -> Script { + assert!( + address.is_script_hash(), + "incorrect coinbase script address: {address} \ + Zebra only supports transparent 'pay to script hash' (P2SH) addresses", + ); + let address_hash = address .zcash_serialize_to_vec() .expect("we should get address bytes here"); diff --git a/zebra-consensus/src/block/subsidy/funding_streams/tests.rs b/zebra-consensus/src/block/subsidy/funding_streams/tests.rs index 8d5b14ea1..ad42db8ee 100644 --- a/zebra-consensus/src/block/subsidy/funding_streams/tests.rs +++ b/zebra-consensus/src/block/subsidy/funding_streams/tests.rs @@ -1,9 +1,11 @@ -use super::*; -use color_eyre::Report; -use std::convert::TryFrom; +//! Tests for funding streams. +use color_eyre::Report; + +use super::*; + +/// Check mainnet funding stream values are correct for the entire period. #[test] -// Check funding streams are correct in the entire period. fn test_funding_stream_values() -> Result<(), Report> { let _init_guard = zebra_test::init(); let network = Network::Mainnet; @@ -54,3 +56,27 @@ fn test_funding_stream_values() -> Result<(), Report> { Ok(()) } + +/// Check mainnet and testnet funding stream addresses are valid transparent P2SH addresses. +#[test] +fn test_funding_stream_addresses() -> Result<(), Report> { + let _init_guard = zebra_test::init(); + + for (network, receivers) in FUNDING_STREAM_ADDRESSES.iter() { + for (receiver, addresses) in receivers { + for address in addresses { + let address = Address::from_str(address).expect("address should deserialize"); + assert_eq!( + &address.network(), + network, + "incorrect network for {receiver:?} funding stream address constant: {address}", + ); + + // Asserts if address is not a P2SH address. + let _script = new_coinbase_script(address); + } + } + } + + Ok(()) +} diff --git a/zebra-rpc/src/server.rs b/zebra-rpc/src/server.rs index 080c41599..3caf6f56a 100644 --- a/zebra-rpc/src/server.rs +++ b/zebra-rpc/src/server.rs @@ -120,6 +120,22 @@ impl RpcServer { #[cfg(feature = "getblocktemplate-rpcs")] { + // Prevent loss of miner funds due to an unsupported or incorrect address type. + if let Some(miner_address) = mining_config.miner_address { + assert_eq!( + miner_address.network(), + network, + "incorrect miner address config: {miner_address} \ + network.network {network} and miner address network {} must match", + miner_address.network(), + ); + assert!( + miner_address.is_script_hash(), + "incorrect miner address config: {miner_address} \ + Zebra only supports transparent 'pay to script hash' (P2SH) addresses", + ); + } + // Initialize the getblocktemplate rpc method handler let get_block_template_rpc_impl = GetBlockTemplateRpcImpl::new( network,