What the developer wrote
Authored by Tobin C. Harding
Don't abuse psbt Deserialize
I'm not sure what is the intention of this code or exactly how the
derives are working in the `taproot` module because we are importing
`psbt::serialize::Deserialize` and then using it in `serde` derives?
Also the `TapTree` arbitrary stuff is a bit odd, that is what led me
here while I was attempting to delete the `psbt` module.
So, I don't know if this is a bug or an internal change? Introduced in
commit: `37c9fce2a684 Implement `Arbitrary` for psbt types`.
What changed, and why it matters
This commit is a small internal cleanup in the rust-bitcoin library. It stops using a PSBT-specific deserialization helper to build test-only 'Arbitrary' instances of TapTree, and instead builds the tree directly from its internal NodeInfo. It also switches serde derives to use the proper serde::Deserialize trait. There is no direct evidence this fixes an exploitable security bug, but it removes a confusing and potentially incorrect reuse of PSBT serialization logic outside the PSBT module.
Treat as a low-risk refactoring/cleanup commit. Reviewers may want to confirm that the new NodeInfo-based arbitrary generation covers the same valid TapTree shapes as the old deserialize path, and that no serde derive behavior changed for users. No urgent security action is indicated by the diff alone.
Security signals we found
Removed reuse of PSBT deserialization trait in non-PSBT taproot code
Potential semantic mismatch between PSBT serialization format and TapTree internal representation
Test-only arbitrary generation path changed; no runtime deserialization change for users
Commit author explicitly questions whether change is a bug fix or internal cleanup
Evidence from the diff
In bitcoin/src/taproot/mod.rs, the patch removes an import of crate::psbt::serialize::Deserialize under #[cfg(feature = “arbitrary”)] and replaces the TapTree::arbitrary implementation. Previously it called Self::deserialize on raw bytes produced by the Arbitrary implementation for Vec
Changed components
bitcoin/src/taproot/mod.rsTapTree::arbitrary implementation (feature = "arbitrary")serde derives in taproot module (feature = "serde")Inspect captured patch +4 / −3
diff --git a/bitcoin/src/taproot/mod.rs b/bitcoin/src/taproot/mod.rs
index ac46e36e..e5037fed 100644
--- a/bitcoin/src/taproot/mod.rs
+++ b/bitcoin/src/taproot/mod.rs
@@ -18,6 +18,8 @@ use internals::array::ArrayExt;
#[allow(unused)] // MSRV polyfill
use internals::slice::SliceExt;
use io::Write;
+#[cfg(feature = "serde")]
+use serde::Deserialize;
use crate::consensus::Encodable;
use crate::crypto::key::{
@@ -48,8 +50,6 @@ pub use self::error::{
InvalidMerkleBranchSizeError, InvalidMerkleTreeDepthError, InvalidTaprootLeafVersionError,
SigFromSliceError, TaprootBuilderError, TaprootError,
};
-#[cfg(feature = "arbitrary")]
-use crate::psbt::serialize::Deserialize;
#[doc(inline)]
pub use crate::XOnlyPublicKey;
@@ -1470,7 +1470,8 @@ impl<'a> Arbitrary<'a> for TapLeaf {
#[cfg(feature = "arbitrary")]
impl<'a> Arbitrary<'a> for TapTree {
fn arbitrary(u: &mut Unstructured<'a>) -> arbitrary::Result<Self> {
- Ok(Self::deserialize(u.arbitrary()?).map_err(|_| arbitrary::Error::IncorrectFormat)?)
+ let node_info = NodeInfo::arbitrary(u)?;
+ Ok(Self(node_info))
}
}
Why this scored 15/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.