Replace Block PartialEq/Eq derive with manual impl
What the developer wrote
Authored by Mitchell Bagot
Replace Block PartialEq/Eq derive with manual impl
The PartialEq/Eq derive on Block results in incorrect equality
comparisons depending on the method of construction due to the
witness_root cache. Instead, equality for Block should be implemented
manually, comparing the header and transaction lists only.
Replace PartialEq/Eq derive with manual implementations that correctly
compare only the header and transaction lists.
Add test to kill mutants.
What changed, and why it matters
This commit fixes a bug in how Bitcoin 'Block' objects are compared for equality. The old code automatically compared every internal field, including a cached 'witness_root' value that can be missing or different depending on how the block was built. That meant two blocks with the same header and transactions could incorrectly look different, or two blocks with different cached values could look the same. The fix manually compares only the header and the list of transactions, which is the correct definition of block equality. It also adds tests to prevent the bug from returning.
Review downstream code that relies on Block equality or uses Block as a HashMap/BTreeMap key to ensure no logic depended on the previous incorrect behavior. The patch should be merged and backported if the affected release is stable. No immediate incident response is required.
Security signals we found
Incorrect equality semantics on a core consensus-adjacent data structure (Block)
Derived equality included a cache field (witness_root) that is not part of the canonical block identity
Potential for logic bugs, test failures, or incorrect caching/lookup behavior downstream
No direct memory-safety or cryptographic vulnerability in the diff
Evidence from the diff
The Block struct in primitives/src/block.rs previously derived PartialEq/Eq, which performed structural equality over all fields including the witness_root cache. Because witness_root is a lazily computed or externally supplied cache, two semantically identical blocks could compare unequal, or blocks with divergent cache state could compare equal. The patch removes the derive and supplies manual PartialEq and Eq impls that compare only self.header and self.transactions. A unit test is added covering equal blocks, differing headers, and differing transaction lists.
Changed components
primitives/src/block.rsBlock<V> struct equality operationswitness_root cache handlingInspect captured patch +43 / −1
diff --git a/primitives/src/block.rs b/primitives/src/block.rs
index 63a4cb08..06bbd4bb 100644
--- a/primitives/src/block.rs
+++ b/primitives/src/block.rs
@@ -81,7 +81,7 @@ pub trait Validation: sealed::Validation + Sync + Send + Sized + Unpin {
///
/// * [CBlock definition](https://github.com/bitcoin/bitcoin/blob/345457b542b6a980ccfbc868af0970a6f91d1b82/src/primitives/block.h#L62)
#[cfg(feature = "alloc")]
-#[derive(PartialEq, Eq, Clone, Debug)]
+#[derive(Clone, Debug)]
pub struct Block<V = Unchecked>
where
V: Validation,
@@ -252,6 +252,16 @@ impl<V: Validation> Block<V> {
pub fn block_hash(&self) -> BlockHash { self.header.block_hash() }
}
+#[cfg(feature = "alloc")]
+impl<V: Validation> PartialEq for Block<V> {
+ fn eq(&self, other: &Self) -> bool {
+ self.header == other.header && self.transactions == other.transactions
+ }
+}
+
+#[cfg(feature = "alloc")]
+impl<V: Validation> Eq for Block<V> {}
+
#[cfg(feature = "alloc")]
impl From<Block> for BlockHash {
#[inline]
@@ -1714,6 +1724,38 @@ mod tests {
assert_eq!(result, (true, Some(expected)));
}
+ #[test]
+ #[cfg(feature = "alloc")]
+ fn block_eq() {
+ let coinbase = Transaction {
+ version: crate::transaction::Version::ONE,
+ lock_time: crate::absolute::LockTime::ZERO,
+ inputs: vec![crate::TxIn::EMPTY_COINBASE],
+ outputs: vec![crate::TxOut {
+ amount: units::Amount::MIN,
+ script_pubkey: crate::script::ScriptBuf::new(),
+ }],
+ };
+
+ let header = dummy_header();
+ let other_header = Header { nonce: header.nonce + 1, ..header };
+
+ assert_eq!(
+ Block::new_unchecked(header, vec![coinbase.clone()]),
+ Block::new_unchecked(header, vec![coinbase.clone()]),
+ );
+
+ assert_ne!(
+ Block::new_unchecked(header, vec![coinbase.clone()]),
+ Block::new_unchecked(other_header, vec![coinbase.clone()]),
+ );
+
+ assert_ne!(
+ Block::new_unchecked(header, vec![coinbase.clone()]),
+ Block::new_unchecked(header, vec![coinbase.clone(), coinbase]),
+ );
+ }
+
#[test]
#[cfg(feature = "alloc")]
#[cfg(feature = "hex")]
Why this scored 43/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.