Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: custom serde for unique keys #1329

Merged
merged 5 commits into from
Feb 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions release-plz.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ dependencies_update = false
git_release_enable = true
publish_allow_dirty = false
semver_check = false
git_release_type = "auto"

[[package]]
name = "sn_build_info"
Expand Down
1 change: 1 addition & 0 deletions sn_transfers/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ lazy_static = "~1.4.0"
rand = { version = "~0.8.5", features = ["small_rng"] }
rmp-serde = "1.1.1"
serde = { version = "1.0.133", features = [ "derive", "rc" ]}
serde_json = "1.0.108"
thiserror = "1.0.24"
tiny-keccak = { version = "~2.0.2", features = [ "sha3" ] }
tracing = { version = "~0.1.26" }
Expand Down
92 changes: 91 additions & 1 deletion sn_transfers/src/cashnotes/unique_keys.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,12 @@ impl DerivationIndex {
}
}

#[derive(Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Serialize, Deserialize, Hash)]
// NB TODO remove Serialize impl when enough nodes have updated to support the new serialisation format!

Check notice

Code scanning / devskim

A "TODO" or similar was left in source code, possibly indicating incomplete functionality

Suspicious comment
// cf below for the new Serialize impl
/// A Unique Public Key is the unique identifier of a CashNote and its SignedSpend on the Network when it is spent.
/// It is the mechanism that makes transactions untraceable to the real owner (MainPubkey).
/// It is the equivalent to using a different key for each transaction in bitcoin.
#[derive(Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Hash, Serialize)]
pub struct UniquePubkey(PublicKey);

impl UniquePubkey {
Expand Down Expand Up @@ -69,6 +74,69 @@ impl UniquePubkey {
}
}

// NB TODO enable this when enough nodes have updated to support the new serialisation format!

Check notice

Code scanning / devskim

A "TODO" or similar was left in source code, possibly indicating incomplete functionality

Suspicious comment
// cf above (remove the derived Serialize impl and enable this one)
// /// Custom implementation of Serialize and Deserialize for UniquePubkey to make it an actionable
// /// hex string that can be copy pasted in apps, instead of a useless array of numbers
// /// Caveat: this is slower than the default implementation
// impl Serialize for UniquePubkey {
// fn serialize<S: serde::Serializer>(&self, serializer: S) -> Result<S::Ok, S::Error> {
// serializer.serialize_str(&self.to_hex())
// }
// }

impl<'de> Deserialize<'de> for UniquePubkey {
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
where
D: serde::Deserializer<'de>,
{
// Backwards compatible deserialize
// this was implemented to support the old serialisation format as well
#[derive(Deserialize)]
#[serde(remote = "UniquePubkey")]
struct UniquePubkeyRep(PublicKey);
impl<'de> Deserialize<'de> for UniquePubkeyRep {
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
where
D: serde::Deserializer<'de>,
{
let key = <PublicKey>::deserialize(deserializer)?;
Ok(UniquePubkeyRep(key))
}
}

let deserialized = serde_json::Value::deserialize(deserializer)?;

// the new serialisation format is a string
if deserialized.is_string() {
let hex: String = serde::Deserialize::deserialize(deserialized).map_err(|e| {
serde::de::Error::custom(format!(
"Failed to deserialize UniquePubkey string representation: {e}",
))
})?;
UniquePubkey::from_hex(hex).map_err(|e| {
serde::de::Error::custom(format!(
"Failed to deserialize UniquePubkey from hex: {e}",
))
})
// the old serialisation format is an array
} else if deserialized.is_array() {
warn!("Detected old serialisation format for UniquePubkey, please update to the new format!");
let key: UniquePubkeyRep =
serde::Deserialize::deserialize(deserialized).map_err(|e| {
serde::de::Error::custom(format!(
"Failed to deserialize UniquePubkey array representation: {e}",
))
})?;
Ok(UniquePubkey(key.0))
} else {
Err(serde::de::Error::custom(
"Failed to deserialize UniquePubkey: unknown serialisation format",
))
}
}
}

/// Actionable way to print a UniquePubkey
/// This way to print it is lengthier but allows to copy/paste it into the safe cli or other apps
/// To use for verification purposes
Expand Down Expand Up @@ -273,4 +341,26 @@ mod tests {
assert_eq!(unique_pubkey, unique_pubkey_from_hex);
Ok(())
}

#[test]
fn test_backwards_compatibility_deserialisation() -> eyre::Result<()> {
let pk = bls::SecretKey::random().public_key();
let main_pubkey = MainPubkey::new(pk);
let unique_pk =
main_pubkey.new_unique_pubkey(&DerivationIndex::random(&mut rand::thread_rng()));

// make sure str deserialisation works
let str_serialised = serde_json::to_string(&unique_pk)?;
println!("str_serialised: {str_serialised}");
let str_deserialised: UniquePubkey = serde_json::from_str(&str_serialised)?;
assert_eq!(str_deserialised, unique_pk);

// make sure array deserialisation works
let array_serialised = serde_json::to_string(&unique_pk.0)?;
println!("array_serialised: {array_serialised}");
let array_deserialised: UniquePubkey = serde_json::from_str(&array_serialised)?;
assert_eq!(array_deserialised, unique_pk);

Ok(())
}
}