From 807bb3808181314da0596be35aec86c914c887b8 Mon Sep 17 00:00:00 2001 From: Jean-Michel <59661788+Jeanmichel7@users.noreply.github.com> Date: Thu, 17 Oct 2024 19:34:56 +0200 Subject: [PATCH] [bug] Client fails on block 91722: Block hash mismatch for BIP-30 (#271) --- packages/consensus/src/validation/block.cairo | 139 +----------------- .../consensus/src/validation/coinbase.cairo | 58 ++++---- scripts/data/regenerate_tests.sh | 2 + 3 files changed, 40 insertions(+), 159 deletions(-) diff --git a/packages/consensus/src/validation/block.cairo b/packages/consensus/src/validation/block.cairo index b1d1f3e3..d43f326f 100644 --- a/packages/consensus/src/validation/block.cairo +++ b/packages/consensus/src/validation/block.cairo @@ -2,9 +2,8 @@ use crate::types::utxo_set::{UtxoSet, UtxoSetTrait}; use crate::types::transaction::{OutPoint, Transaction}; use crate::codec::{Encode, TransactionCodec}; -use utils::{ - hash::{Digest, DigestTrait}, merkle_tree::merkle_root, double_sha256::double_sha256_byte_array, -}; +use crate::validation::coinbase::is_coinbase_txid_duplicated; +use utils::{hash::Digest, merkle_tree::merkle_root, double_sha256::double_sha256_byte_array,}; use super::transaction::validate_transaction; use core::num::traits::zero::Zero; @@ -69,6 +68,10 @@ pub fn compute_and_validate_tx_data( txids.append(txid); if (is_coinbase) { + // skip duplicated txid (it's not possible to spend these coinbase outputs) + if (is_coinbase_txid_duplicated(txid, block_height)) { + continue; + } let mut vout = 0; for output in *tx .outputs { @@ -111,133 +114,3 @@ pub fn compute_and_validate_tx_data( Result::Ok((total_fee, merkle_root(txids.span()), wtxid_root)) } - -/// Validates that the block hash at certain heights matches the expected hash according to BIP-30. -/// -/// This function ensures that for the two exceptional blocks affected by BIP-30 (heights 91722 and -/// 91812), the block hash matches the known hash for those heights. This prevents accepting -/// incorrect blocks at these critical heights, which could lead to security issues. -/// -/// BIP-30 - Reject duplicate (https://github.com/bitcoin/bips/blob/master/bip-0030.mediawiki) -pub fn validate_bip30_block_hash(block_height: u32, block_hash: @Digest,) -> Result<(), ByteArray> { - if block_height == 91722 { - let expected_hash = DigestTrait::new( - [ - 0x8ed04d57, - 0xf2f3cd6c, - 0xa6e55569, - 0xdc1654e1, - 0xf219847f, - 0x66e726dc, - 0xa2710200, - 0x00000000, - ] - ); - if *block_hash != expected_hash { - return Result::Err("Block hash mismatch for BIP-30 exception at height 91722"); - } - Result::Ok(()) - } else if block_height == 91812 { - let expected_hash = DigestTrait::new( - [ - 0x2f6f306f, - 0xd683deb8, - 0x5d9314ef, - 0xfdcf36af, - 0x66d9e3ac, - 0xaceb79d4, - 0xaed00a00, - 0x00000000, - ] - ); - if *block_hash != expected_hash { - return Result::Err("Block hash mismatch for BIP-30 exception at height 91812"); - } - Result::Ok(()) - } else { - Result::Ok(()) - } -} - - -#[cfg(test)] -mod tests { - use super::{validate_bip30_block_hash}; - use utils::hash::{DigestTrait}; - - #[test] - fn test_bip30_block_hash() { - // Expected hash for block at height 91722 - let expected_hash = DigestTrait::new( - [ - 0x8ed04d57, - 0xf2f3cd6c, - 0xa6e55569, - 0xdc1654e1, - 0xf219847f, - 0x66e726dc, - 0xa2710200, - 0x00000000, - ] - ); - let result = validate_bip30_block_hash(91722, @expected_hash); - assert_eq!(result, Result::Ok(())); - } - - #[test] - fn test_bip30_block_hash_wrong_hash() { - // Expected hash for block at height 91722 - let expected_hash = DigestTrait::new( - [ - 0x8ed04d56, - 0xf2f3cd6c, - 0xa6e55569, - 0xdc1654e1, - 0xf219847f, - 0x66e726dc, - 0xa2710200, - 0x00000000, - ] - ); - let result = validate_bip30_block_hash(91722, @expected_hash); - assert_eq!(result, Result::Err("Block hash mismatch for BIP-30 exception at height 91722")); - } - - #[test] - fn test_bip30_block_hash_height_91812() { - // Expected hash for block at height 91812 - let expected_hash = DigestTrait::new( - [ - 0x2f6f306f, - 0xd683deb8, - 0x5d9314ef, - 0xfdcf36af, - 0x66d9e3ac, - 0xaceb79d4, - 0xaed00a00, - 0x00000000, - ] - ); - let result = validate_bip30_block_hash(91812, @expected_hash); - assert_eq!(result, Result::Ok(())); - } - - #[test] - fn test_bip30_block_hash_wrong_hash_91812() { - // Expected hash for block at height 91812 - let expected_hash = DigestTrait::new( - [ - 0x9ed04d56, - 0xf2f3cd6c, - 0xa6e55569, - 0xdc1654e1, - 0xf219847f, - 0x66e726dc, - 0xa2710200, - 0x00000000, - ] - ); - let result = validate_bip30_block_hash(91812, @expected_hash); - assert_eq!(result, Result::Err("Block hash mismatch for BIP-30 exception at height 91812")); - } -} diff --git a/packages/consensus/src/validation/coinbase.cairo b/packages/consensus/src/validation/coinbase.cairo index fa702144..570bc5b4 100644 --- a/packages/consensus/src/validation/coinbase.cairo +++ b/packages/consensus/src/validation/coinbase.cairo @@ -9,6 +9,9 @@ const BIP_34_BLOCK_HEIGHT: u32 = 227_836; const BIP_141_BLOCK_HEIGHT: u32 = 481_824; const WTNS_PK_SCRIPT_LEN: u32 = 38; const WTNS_PK_SCRIPT_PREFIX: felt252 = 116705705699821; // 0x6a24aa21a9ed +const FIRST_DUP_TXID: u256 = 0xe3bf3d07d4b0375638d5f1db5255fe07ba2c4cb067cd81b84ee974b6585fb468; +const SECOND_DUP_TXID: u256 = 0xd5d27987d2a3dfc724e359870c6644b40e497bdc0589a033220fe15429d88599; + /// Validates coinbase transaction. pub fn validate_coinbase( @@ -172,14 +175,21 @@ fn validate_coinbase_outputs( Result::Ok(()) } - -/// Determines if the transaction outputs of a block at a given height are unspendable due to -/// BIP-30. +/// (BIP-30) Skip coinbase tx for duplicated txids +/// Only the first tx is valid, the duplicated tx is ignored /// -/// This function checks if the block height corresponds to one of the two exceptional blocks -/// where transactions with duplicate TXIDs were allowed. -pub fn is_bip30_unspendable(block_height: u32) -> bool { - block_height == 91722 || block_height == 91812 +/// First txid e3bf3d07d4b0375638d5f1db5255fe07ba2c4cb067cd81b84ee974b6585fb468 +/// at blocks 91722 and 91880 +/// Second txid d5d27987d2a3dfc724e359870c6644b40e497bdc0589a033220fe15429d88599 +/// at blocks 91812 and 91842 +pub fn is_coinbase_txid_duplicated(txid: Digest, block_height: u32) -> bool { + // TODO: allow duplicate transactions in case the previous instance of the transaction had no + // spendable outputs left. + if ((txid.into() == FIRST_DUP_TXID && block_height == 91880) + || (txid.into() == SECOND_DUP_TXID && block_height == 91842)) { + return true; + } + false } #[cfg(test)] @@ -188,10 +198,24 @@ mod tests { use super::{ compute_block_reward, validate_coinbase, validate_coinbase_input, validate_coinbase_sig_script, validate_coinbase_witness, validate_coinbase_outputs, - calculate_wtxid_commitment, is_bip30_unspendable + calculate_wtxid_commitment, is_coinbase_txid_duplicated, FIRST_DUP_TXID, SECOND_DUP_TXID, }; use utils::{hex::{from_hex, hex_to_hash_rev}, hash::Digest}; + #[test] + fn test_bip30_first_txid_dup() { + let txid: Digest = FIRST_DUP_TXID.into(); + assert!(!is_coinbase_txid_duplicated(txid, 91722)); + assert!(is_coinbase_txid_duplicated(txid, 91880)); + } + + #[test] + fn test_bip30_second_txid_dup() { + let txid: Digest = SECOND_DUP_TXID.into(); + assert!(!is_coinbase_txid_duplicated(txid, 91812)); + assert!(is_coinbase_txid_duplicated(txid, 91842)); + } + // Ref implementation here: // https://github.com/bitcoin/bitcoin/blob/0f68a05c084bef3e53e3f549c403bc90b1db319c/src/test/validation_tests.cpp#L24 #[test] @@ -664,22 +688,4 @@ mod tests { validate_coinbase(@tx, total_fees, block_height, wtxid_root_hash).unwrap(); } - - #[test] - fn test_is_bip30_unspendable() { - let block_height = 91722; - let result = is_bip30_unspendable(block_height); - - assert_eq!(result, true); - - let block_height = 91812; - let result = is_bip30_unspendable(block_height); - - assert_eq!(result, true); - - let block_height = 9; - let result = is_bip30_unspendable(block_height); - - assert_eq!(result, false); - } } diff --git a/scripts/data/regenerate_tests.sh b/scripts/data/regenerate_tests.sh index bc9018fe..c9b23921 100755 --- a/scripts/data/regenerate_tests.sh +++ b/scripts/data/regenerate_tests.sh @@ -22,6 +22,7 @@ light_test_cases=( 32255 # First target adjustment (32256) 57042 # Block containing pizza tx (57043) 72575 # Difficulty adjustment + 91721 # Duplicate coinbase txid (91722) 116927 # Difficulty adjustment 150012 # Small Block (150013) 209999 # First halving block (210000) @@ -45,6 +46,7 @@ full_test_cases=( 32255 # First target adjustment (32256) 57042 # Block containing pizza tx (57043) 72575 # Difficulty adjustment + 91721 # Duplicate coinbase txid (91722) 116927 # Difficulty adjustment 150012 # Small Block (150013) 209999 # First halving block (210000)