← Watch feed
Remove serde impls from some primitive types
Public commit record
68/100 · Adequate
What the developer wrote
Authored by Tobin C. Harding
Remove serde impls from some primitive types
Remove `serde` impls for anything that is not one of the following:
- Already has a custom impl
- The derive is obvious and not likely to change i.e., its a simple
wrapper type (implies the getter just returns the inner type).
Explicitly this removes `serde` impls from:
- `Block`
- `block::Header`
- `Transaction`
- `TxIn`
- `TxOut`
- `OutPoint`
Note that these can still be serde de/serialized using consensus
encoding. Currently only by using `bitcoin` but if/when
`consensus_encoding` crate comes into existence we can add support
there as is done currently in `bitcoin::consensus::serde`.
✓ Descriptive subject✓ Names a concrete action or component✓ Provides detailed explanatory context
What changed, and why it matters
This commit removes automatic serialization/deserialization support (via the serde library) for several core Bitcoin data types like Block, Transaction, and TxOut. It is a deliberate API cleanup, not a bug fix. The change could break downstream code that relied on these types being directly serde-serializable, but it does not introduce a memory-safety or cryptographic vulnerability on its own. The project notes that these types can still be serialized using Bitcoin consensus encoding instead.
Recommended action
Treat as a breaking API change rather than a security vulnerability. Downstream projects using the serde feature should verify whether they relied on direct serde serialization of Block, Header, Transaction, TxIn, TxOut, or OutPoint and migrate to bitcoin::consensus::serde or custom wrappers as needed. No urgent patching is indicated by the commit content.
Security signals we found
01
API surface reduction for serialization traits
02
Removal of serde derive impls from consensus-critical data structures
03
No cryptographic, memory-safety, or input-validation changes visible in diff
04
Commit message explicitly describes rationale as design cleanup, not security fix
Evidence from the diff
The patch deletes #[derive(Serialize, Deserialize)] and related serde attributes from Block, block::Header, Transaction, TxIn, TxOut, and OutPoint in rust-bitcoin primitives. It also removes regression tests and test fixtures that exercised the removed serde impls, and adds a new OutPoint serde regression fixture. The commit message frames this as removing serde impls that are not obvious/stable, directing users to consensus-based serde support in bitcoin::consensus::serde. There is no evidence in the diff of a security bug being fixed; rather, this is an API surface reduction that may affect consumers depending on the removed impls.
Changed components
primitives/src/block.rsprimitives/src/transaction.rsbitcoin/src/blockdata/transaction.rsbitcoin/tests/serde.rsbitcoin/tests/data/serde/*Inspect captured patch +10 / −73
diff --git a/bitcoin/src/blockdata/transaction.rs b/bitcoin/src/blockdata/transaction.rs
index 999696b7..d6e3e672 100644
--- a/bitcoin/src/blockdata/transaction.rs
+++ b/bitcoin/src/blockdata/transaction.rs
@@ -1242,8 +1242,6 @@ impl<'a> Arbitrary<'a> for InputWeightPrediction {
mod tests {
use hex::FromHex;
use hex_lit::hex;
- #[cfg(feature = "serde")]
- use internals::serde_round_trip;
use units::parse;
use super::*;
@@ -1568,28 +1566,6 @@ mod tests {
);
}
- #[test]
- #[cfg(feature = "serde")]
- fn txn_encode_decode() {
- let tx_bytes = hex!("0100000001a15d57094aa7a21a28cb20b59aab8fc7d1149a3bdbcddba9c622e4f5f6a99ece010000006c493046022100f93bb0e7d8db7bd46e40132d1f8242026e045f03a0efe71bbb8e3f475e970d790221009337cd7f1f929f00cc6ff01f03729b069a7c21b59b1736ddfee5db5946c5da8c0121033b9b137ee87d5a812d6f506efdd37f0affa7ffc310711c06c7f3e097c9447c52ffffffff0100e1f505000000001976a9140389035a9225b3839e2bbf32d826a1e222031fd888ac00000000");
- let tx: Transaction = deserialize(&tx_bytes).unwrap();
- serde_round_trip!(tx);
- }
-
- // Test decoding transaction `4be105f158ea44aec57bf12c5817d073a712ab131df6f37786872cfc70734188`
- // from testnet, which is the first BIP144-encoded transaction I encountered.
- #[test]
- #[cfg(feature = "serde")]
- fn segwit_tx_decode() {
- let tx_bytes = hex!("010000000001010000000000000000000000000000000000000000000000000000000000000000ffffffff3603da1b0e00045503bd5704c7dd8a0d0ced13bb5785010800000000000a636b706f6f6c122f4e696e6a61506f6f6c2f5345475749542fffffffff02b4e5a212000000001976a914876fbb82ec05caa6af7a3b5e5a983aae6c6cc6d688ac0000000000000000266a24aa21a9edf91c46b49eb8a29089980f02ee6b57e7d63d33b18b4fddac2bcd7db2a39837040120000000000000000000000000000000000000000000000000000000000000000000000000");
- let tx: Transaction = deserialize(&tx_bytes).unwrap();
- assert_eq!(tx.weight(), Weight::from_wu(780));
- serde_round_trip!(tx);
-
- let consensus_encoded = serialize(&tx);
- assert_eq!(consensus_encoded, tx_bytes);
- }
-
#[test]
fn sighashtype_fromstr_display() {
let sighashtypes = [
diff --git a/bitcoin/tests/data/serde/block_bincode b/bitcoin/tests/data/serde/block_bincode
deleted file mode 100644
index 0bd56c8d..00000000
Binary files a/bitcoin/tests/data/serde/block_bincode and /dev/null differ
diff --git a/bitcoin/tests/data/serde/out_point_bincode b/bitcoin/tests/data/serde/out_point_bincode
new file mode 100644
index 00000000..6e8c0a51
Binary files /dev/null and b/bitcoin/tests/data/serde/out_point_bincode differ
diff --git a/bitcoin/tests/data/serde/transaction_bincode b/bitcoin/tests/data/serde/transaction_bincode
deleted file mode 100644
index 82ffd66a..00000000
Binary files a/bitcoin/tests/data/serde/transaction_bincode and /dev/null differ
diff --git a/bitcoin/tests/data/serde/txin_bincode b/bitcoin/tests/data/serde/txin_bincode
deleted file mode 100644
index 92a61a1a..00000000
Binary files a/bitcoin/tests/data/serde/txin_bincode and /dev/null differ
diff --git a/bitcoin/tests/data/serde/txout_bincode b/bitcoin/tests/data/serde/txout_bincode
deleted file mode 100644
index 1bbd8c33..00000000
Binary files a/bitcoin/tests/data/serde/txout_bincode and /dev/null differ
diff --git a/bitcoin/tests/serde.rs b/bitcoin/tests/serde.rs
index 0a4f4bbb..60a117e4 100644
--- a/bitcoin/tests/serde.rs
+++ b/bitcoin/tests/serde.rs
@@ -8,9 +8,6 @@
//! Types/tests were found using, and are ordered by, the output of: `git grep -l Serialize`.
//!
-// In tests below `deserialize` is consensus deserialize while `serialize` is serde serialize, that
-// is why we have two different serialized data files for tests that use binary serialized input.
-//
// To create a file with the expected serialized data do something like:
//
// use std::fs::File;
@@ -26,7 +23,6 @@ use std::collections::BTreeMap;
use bincode::serialize;
use bitcoin::bip32::{ChildNumber, KeySource, Xpriv, Xpub};
-use bitcoin::consensus::encode::deserialize;
use bitcoin::hashes::{hash160, ripemd160, sha256, sha256d};
use bitcoin::hex::FromHex;
use bitcoin::locktime::{absolute, relative};
@@ -36,23 +32,10 @@ use bitcoin::sighash::{EcdsaSighashType, TapSighashType};
use bitcoin::taproot::{self, ControlBlock, LeafVersion, TapTree, TaprootBuilder};
use bitcoin::witness::Witness;
use bitcoin::{
- ecdsa, transaction, Address, Amount, Block, NetworkKind, OutPoint, PrivateKey, PublicKey,
+ ecdsa, transaction, Address, Amount, NetworkKind, OutPoint, PrivateKey, PublicKey,
ScriptBuf, Sequence, Target, Transaction, TxIn, TxOut, Txid, Work,
};
-/// Implicitly does regression test for `BlockHeader` also.
-#[test]
-fn serde_regression_block() {
- let segwit = include_bytes!(
- "data/testnet_block_000000000000045e0b1660b6445b5e5c5ab63c9a4f956be7e1e69be04fa4497b.raw"
- );
- let block: Block = deserialize(segwit).unwrap();
-
- let got = serialize(&block).unwrap();
- let want = include_bytes!("data/serde/block_bincode");
- assert_eq!(got, want)
-}
-
#[test]
fn serde_regression_absolute_lock_time_height() {
let t = absolute::LockTime::from_height(741521).expect("valid height");
@@ -100,31 +83,16 @@ fn serde_regression_script() {
}
#[test]
-fn serde_regression_txin() {
- let ser = include_bytes!("data/serde/txin_ser");
- let txin: TxIn = deserialize(ser).unwrap();
-
- let got = serialize(&txin).unwrap();
- let want = include_bytes!("data/serde/txin_bincode") as &[_];
- assert_eq!(got, want)
-}
-
-#[test]
-fn serde_regression_txout() {
- let txout = TxOut { value: Amount::MAX, script_pubkey: ScriptBuf::from(vec![0u8, 1u8, 2u8]) };
-
- let got = serialize(&txout).unwrap();
- let want = include_bytes!("data/serde/txout_bincode") as &[_];
- assert_eq!(got, want)
-}
-
-#[test]
-fn serde_regression_transaction() {
- let ser = include_bytes!("data/serde/transaction_ser");
- let tx: Transaction = deserialize(ser).unwrap();
+fn serde_regression_out_point() {
+ let out_point = OutPoint {
+ txid: "e567952fb6cc33857f392efa3a46c995a28f69cca4bb1b37e0204dab1ec7a389"
+ .parse::<Txid>()
+ .unwrap(),
+ vout: 1,
+ };
- let got = serialize(&tx).unwrap();
- let want = include_bytes!("data/serde/transaction_bincode") as &[_];
+ let got = serialize(&out_point).unwrap();
+ let want = include_bytes!("data/serde/out_point_bincode") as &[_];
assert_eq!(got, want)
}
diff --git a/primitives/src/block.rs b/primitives/src/block.rs
index 97f07f12..b59e68ff 100644
--- a/primitives/src/block.rs
+++ b/primitives/src/block.rs
@@ -59,7 +59,6 @@ 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)]
-#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
pub struct Block<V = Unchecked>
where
V: Validation,
@@ -69,7 +68,6 @@ where
/// List of transactions contained in the block
transactions: Vec<Transaction>,
/// Cached witness root if it's been computed.
- #[cfg_attr(feature = "serde", serde(skip_serializing))]
witness_root: Option<WitnessMerkleNode>,
/// Validation marker.
marker: PhantomData<V>,
@@ -178,7 +176,6 @@ mod sealed {
///
/// * [CBlockHeader definition](https://github.com/bitcoin/bitcoin/blob/345457b542b6a980ccfbc868af0970a6f91d1b82/src/primitives/block.h#L20)
#[derive(Copy, PartialEq, Eq, Clone, PartialOrd, Ord, Hash)]
-#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
pub struct Header {
/// Block version, now repurposed for soft fork signalling.
pub version: Version,
diff --git a/primitives/src/transaction.rs b/primitives/src/transaction.rs
index 26dbfcdb..a07608a2 100644
--- a/primitives/src/transaction.rs
+++ b/primitives/src/transaction.rs
@@ -96,7 +96,6 @@ use crate::witness::Witness;
/// before this change, so users should not notice any breakage (here) when
/// transitioning from 0.29 to 0.30.
#[derive(Clone, PartialEq, Eq, Debug, Hash)]
-#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
#[cfg(feature = "alloc")]
pub struct Transaction {
/// The protocol version, is currently expected to be 1, 2 (BIP 68) or 3 (BIP 431).
@@ -314,7 +313,6 @@ fn hash_transaction(tx: &Transaction, uses_segwit_serialization: bool) -> sha256
///
/// * [CTxIn definition](https://github.com/bitcoin/bitcoin/blob/345457b542b6a980ccfbc868af0970a6f91d1b82/src/primitives/transaction.h#L65)
#[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Debug, Hash)]
-#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
#[cfg(feature = "alloc")]
pub struct TxIn {
/// The reference to the previous output that is being used as an input.
@@ -358,11 +356,9 @@ impl TxIn {
///
/// * [CTxOut definition](https://github.com/bitcoin/bitcoin/blob/345457b542b6a980ccfbc868af0970a6f91d1b82/src/primitives/transaction.h#L148)
#[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Debug, Hash)]
-#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
#[cfg(feature = "alloc")]
pub struct TxOut {
/// The value of the output, in satoshis.
- #[cfg_attr(feature = "serde", serde(with = "crate::amount::serde::as_sat"))]
pub value: Amount,
/// The script which must be satisfied for the output to be spent.
pub script_pubkey: ScriptBuf,
Risk score
Our methodology →Why this scored 37/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.