Replace serde derive with call-through impl
What the developer wrote
Authored by Mitchell Bagot
Replace serde derive with call-through impl
The existing serde implementation for XOnlyPublicKey derives from the
implementation on the inner type. Once additional data is introduced,
like the parity, XOnlyPublicKey will no longer serialise or
deserialise correctly.
Replace serde derived implementations with manual implementations that
call through to inner type's serde implementations.
What changed, and why it matters
This commit changes how a Bitcoin public-key type is serialized and deserialized when the optional serde feature is enabled. The old code automatically used the inner key type's format, which would break if extra data (such as key parity) is added later. The new code explicitly forwards to the inner type, keeping the wire format stable even as the wrapper type evolves. It is a hardening/preventive change rather than a fix for an active vulnerability.
Review the new manual serde implementations for correctness, especially that Serialize and Deserialize remain symmetric and that any future parity field is handled consistently. If this change affects persisted or network-exchanged data, add regression tests covering round-trip serialization and cross-version compatibility. No urgent patch is required unless downstream consumers rely on the previous derived format in a way that now differs.
Security signals we found
Serialization format stability for a cryptographic public-key type
Preventive hardening against future struct layout changes breaking serde compatibility
Manual serde delegation instead of derived forwarding
Potential for deserialization mismatch if inner and wrapper invariants diverge in the future
Evidence from the diff
The patch removes #[derive(Serialize, Deserialize)] from the XOnlyPublicKey wrapper struct in bitcoin/src/crypto/key.rs and replaces it with manual serde trait implementations that delegate to secp256k1::XOnlyPublicKey. The Serialize impl calls Serialize::serialize on self.as_inner(), and the Deserialize impl reconstructs the wrapper via Self::new(…) after deserializing the inner type. The commit message states this is necessary because once additional fields such as parity are introduced, derived serialization would no longer match the inner type’s format. No current extra fields are present in the diff, so the change is forward-looking.
Changed components
bitcoin/src/crypto/key.rsXOnlyPublicKey serde implementation (behind 'serde' feature flag)Inspect captured patch +24 / −1
diff --git a/bitcoin/src/crypto/key.rs b/bitcoin/src/crypto/key.rs
index 14ac1cee..ca16f1f9 100644
--- a/bitcoin/src/crypto/key.rs
+++ b/bitcoin/src/crypto/key.rs
@@ -21,6 +21,8 @@ use crate::crypto::ecdsa;
use crate::internal_macros::impl_asref_push_bytes;
use crate::network::NetworkKind;
use crate::prelude::{DisplayHex, String, Vec};
+#[cfg(feature = "serde")]
+use crate::serde::{Serialize, Serializer, Deserialize, Deserializer};
use crate::script::{self, WitnessScriptBuf};
use crate::taproot::{TapNodeHash, TapTweakHash};
@@ -37,7 +39,6 @@ pub use secp256k1::rand;
mod encapsulate {
/// A Bitcoin Schnorr X-only public key used for BIP-0340 signatures.
#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
- #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
pub struct XOnlyPublicKey(secp256k1::XOnlyPublicKey);
impl XOnlyPublicKey {
@@ -284,6 +285,28 @@ impl fmt::Display for XOnlyPublicKey {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { fmt::Display::fmt(self.as_inner(), f) }
}
+// XOnlyPublicKey should serialize/deserialize identically to the inner type.
+#[cfg(feature = "serde")]
+impl Serialize for XOnlyPublicKey {
+ fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+ where
+ S: Serializer,
+ {
+ <secp256k1::XOnlyPublicKey as Serialize>::serialize(self.as_inner(), serializer)
+ }
+}
+
+#[cfg(feature = "serde")]
+impl<'de> Deserialize<'de> for XOnlyPublicKey {
+ fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+ where
+ D: Deserializer<'de>,
+ {
+ Ok(Self::new(secp256k1::XOnlyPublicKey::deserialize(deserializer)?))
+ }
+}
+
+
impl Keypair {
/// Generates a new random key pair.
///
Why this scored 29/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.