Skip to content

Commit aae9964

Browse files
authored
Merge pull request #32 from fjarri/safe-maps
Add a `SerializableMap` type
2 parents 420fd69 + 2dd2076 commit aae9964

File tree

8 files changed

+234
-15
lines changed

8 files changed

+234
-15
lines changed

CHANGELOG.md

+21
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
# Changelog
2+
3+
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
4+
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
5+
6+
## [0.0.2] - In development
7+
8+
### Added
9+
10+
- `SerializableMap` wrapper for `BTreeMap` supporting more formats and providing some safety features. (#[32])
11+
12+
13+
[#32]: https://github.com/entropyxyz/manul/pull/32
14+
15+
16+
## [0.0.1] - 2024-10-12
17+
18+
Initial release.
19+
20+
21+
[0.0.1]: https://github.com/entropyxyz/manul/releases/tag/v0.0.1

clippy.toml

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
allow-unwrap-in-tests = true

manul/Cargo.toml

+2
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@ impls = "1"
2525
sha3 = "0.10"
2626
rand = { version = "0.8", default-features = false }
2727
bincode = { version = "2.0.0-rc.3", default-features = false, features = ["alloc", "serde"] }
28+
serde_asn1_der = "0.8"
29+
serde_json = "1"
2830
criterion = "0.5"
2931

3032
[features]

manul/src/lib.rs

+1
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ extern crate alloc;
1717

1818
pub mod protocol;
1919
pub mod session;
20+
pub(crate) mod utils;
2021

2122
#[cfg(any(test, feature = "testing"))]
2223
pub mod testing;

manul/src/session/echo.rs

+11-6
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,12 @@ use super::{
1515
message::{MessageVerificationError, SignedMessage},
1616
LocalError,
1717
};
18-
use crate::protocol::{
19-
Artifact, DirectMessage, EchoBroadcast, FinalizeError, FinalizeOutcome, ObjectSafeRound, Payload, Protocol,
20-
ReceiveError, Round, RoundId,
18+
use crate::{
19+
protocol::{
20+
Artifact, DirectMessage, EchoBroadcast, FinalizeError, FinalizeOutcome, ObjectSafeRound, Payload, Protocol,
21+
ReceiveError, Round, RoundId,
22+
},
23+
utils::SerializableMap,
2124
};
2225

2326
#[derive(Debug)]
@@ -27,8 +30,8 @@ pub(crate) enum EchoRoundError<Id> {
2730
}
2831

2932
#[derive(Debug, Clone, Serialize, Deserialize)]
30-
pub struct EchoRoundMessage<Id: Ord, S> {
31-
pub(crate) echo_messages: BTreeMap<Id, SignedMessage<S, EchoBroadcast>>,
33+
pub struct EchoRoundMessage<Id: Debug + Clone + Ord, S> {
34+
pub(crate) echo_messages: SerializableMap<Id, SignedMessage<S, EchoBroadcast>>,
3235
}
3336

3437
pub struct EchoRound<P, Id, S> {
@@ -121,7 +124,9 @@ where
121124
)));
122125
}
123126

124-
let message = EchoRoundMessage { echo_messages };
127+
let message = EchoRoundMessage {
128+
echo_messages: echo_messages.into(),
129+
};
125130
let dm = DirectMessage::new::<P, _>(&message)?;
126131
Ok((dm, Artifact::empty()))
127132
}

manul/src/session/evidence.rs

+12-9
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,12 @@ use super::{
1010
transcript::Transcript,
1111
LocalError,
1212
};
13-
use crate::protocol::{
14-
DirectMessage, DirectMessageError, EchoBroadcast, EchoBroadcastError, MessageValidationError, Protocol,
15-
ProtocolError, ProtocolValidationError, RoundId,
13+
use crate::{
14+
protocol::{
15+
DirectMessage, DirectMessageError, EchoBroadcast, EchoBroadcastError, MessageValidationError, Protocol,
16+
ProtocolError, ProtocolValidationError, RoundId,
17+
},
18+
utils::SerializableMap,
1619
};
1720

1821
#[derive(Debug, Clone)]
@@ -113,9 +116,9 @@ where
113116
error,
114117
direct_message,
115118
echo_broadcast,
116-
direct_messages,
117-
echo_broadcasts,
118-
combined_echos,
119+
direct_messages: direct_messages.into(),
120+
echo_broadcasts: echo_broadcasts.into(),
121+
combined_echos: combined_echos.into(),
119122
}),
120123
})
121124
}
@@ -352,9 +355,9 @@ struct ProtocolEvidence<P: Protocol, S> {
352355
error: P::ProtocolError,
353356
direct_message: SignedMessage<S, DirectMessage>,
354357
echo_broadcast: Option<SignedMessage<S, EchoBroadcast>>,
355-
direct_messages: BTreeMap<RoundId, SignedMessage<S, DirectMessage>>,
356-
echo_broadcasts: BTreeMap<RoundId, SignedMessage<S, EchoBroadcast>>,
357-
combined_echos: BTreeMap<RoundId, SignedMessage<S, DirectMessage>>,
358+
direct_messages: SerializableMap<RoundId, SignedMessage<S, DirectMessage>>,
359+
echo_broadcasts: SerializableMap<RoundId, SignedMessage<S, EchoBroadcast>>,
360+
combined_echos: SerializableMap<RoundId, SignedMessage<S, DirectMessage>>,
358361
}
359362

360363
impl<P, S> ProtocolEvidence<P, S>

manul/src/utils.rs

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
//! Assorted utilities.
2+
3+
mod serializable_map;
4+
5+
pub(crate) use serializable_map::SerializableMap;

manul/src/utils/serializable_map.rs

+181
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,181 @@
1+
use alloc::{collections::BTreeMap, format};
2+
use core::{
3+
fmt::{self, Debug},
4+
marker::PhantomData,
5+
ops::Deref,
6+
};
7+
8+
use serde::{
9+
de::{self, SeqAccess, Visitor},
10+
ser::SerializeSeq,
11+
Deserialize, Deserializer, Serialize, Serializer,
12+
};
13+
14+
/// A wrapper for [`BTreeMap`](`alloc::collections::BTreeMap`)
15+
/// that allows it to be serialized in a wider range of formats.
16+
///
17+
/// Some serialization formats/implementations (e.g. `serde_asn1_der`) do not support serializing maps.
18+
/// This implementation serializes maps as sequences of key/value pairs,
19+
/// and checks for duplicate keys on deserialization.
20+
#[derive(Debug, Clone, PartialEq, Eq)]
21+
pub(crate) struct SerializableMap<K, V>(BTreeMap<K, V>);
22+
23+
impl<K, V> From<BTreeMap<K, V>> for SerializableMap<K, V> {
24+
fn from(source: BTreeMap<K, V>) -> Self {
25+
Self(source)
26+
}
27+
}
28+
29+
impl<K, V> Deref for SerializableMap<K, V> {
30+
type Target = BTreeMap<K, V>;
31+
32+
fn deref(&self) -> &Self::Target {
33+
&self.0
34+
}
35+
}
36+
37+
impl<K, V> Serialize for SerializableMap<K, V>
38+
where
39+
K: Serialize,
40+
V: Serialize,
41+
{
42+
fn serialize<S: Serializer>(&self, serializer: S) -> Result<S::Ok, S::Error> {
43+
// TODO: an error here can be covered by a custom `Serializer`,
44+
// but that's a lot of extra code to test just one line.
45+
// Is there an easier way?
46+
// Alternatively, we wait until `#[coverage]` is stabilized.
47+
let mut seq = serializer.serialize_seq(Some(self.0.len()))?;
48+
for e in self.0.iter() {
49+
seq.serialize_element(&e)?;
50+
}
51+
seq.end()
52+
}
53+
}
54+
55+
struct MapVisitor<K, V>(PhantomData<(K, V)>);
56+
57+
impl<'de, K, V> Visitor<'de> for MapVisitor<K, V>
58+
where
59+
K: Debug + Clone + Ord + Deserialize<'de>,
60+
V: Deserialize<'de>,
61+
{
62+
type Value = SerializableMap<K, V>;
63+
64+
fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result {
65+
formatter.write_str("A map serialized as a list of pairs")
66+
}
67+
68+
fn visit_seq<M>(self, mut access: M) -> Result<Self::Value, M::Error>
69+
where
70+
M: SeqAccess<'de>,
71+
{
72+
let mut map = SerializableMap(BTreeMap::new());
73+
74+
while let Some((key, value)) = access.next_element::<(K, V)>()? {
75+
// This clone, and the consequent `Debug` bound on the impl can be removed
76+
// when `BTreeMap::try_insert()` is stabilized.
77+
// Or we could call `BTreeMap::contains()` first, but it's more expensive than cloning a key
78+
// (which will be short).
79+
let key_clone = key.clone();
80+
if map.0.insert(key, value).is_some() {
81+
return Err(de::Error::custom(format!("Duplicate key: {key_clone:?}")));
82+
}
83+
}
84+
85+
Ok(map)
86+
}
87+
}
88+
89+
impl<'de, K, V> Deserialize<'de> for SerializableMap<K, V>
90+
where
91+
K: Debug + Clone + Ord + Deserialize<'de>,
92+
V: Deserialize<'de>,
93+
{
94+
fn deserialize<D: Deserializer<'de>>(deserializer: D) -> Result<Self, D::Error> {
95+
deserializer.deserialize_seq(MapVisitor::<K, V>(PhantomData))
96+
}
97+
}
98+
99+
#[cfg(test)]
100+
mod tests {
101+
use alloc::collections::BTreeMap;
102+
use alloc::string::{String, ToString};
103+
use alloc::{vec, vec::Vec};
104+
105+
use serde::{Deserialize, Serialize};
106+
107+
use super::SerializableMap;
108+
109+
fn asn1_serialize<T: Serialize>(value: &T) -> Result<Vec<u8>, String> {
110+
serde_asn1_der::to_vec(value).map_err(|err| err.to_string())
111+
}
112+
113+
fn asn1_deserialize<'de, T: Deserialize<'de>>(bytes: &'de [u8]) -> Result<T, String> {
114+
serde_asn1_der::from_bytes(bytes).map_err(|err| err.to_string())
115+
}
116+
117+
fn json_serialize<T: Serialize>(value: &T) -> String {
118+
serde_json::to_string(value).unwrap()
119+
}
120+
121+
fn json_deserialize<'de, T: Deserialize<'de>>(string: &'de str) -> Result<T, String> {
122+
serde_json::from_str::<T>(string).map_err(|err| err.to_string())
123+
}
124+
125+
#[test]
126+
fn roundtrip() {
127+
let map = SerializableMap::<u8, u8>(BTreeMap::from([(120, 130), (140, 150)]));
128+
let map_serialized = asn1_serialize(&map).unwrap();
129+
let map_back = asn1_deserialize(&map_serialized).unwrap();
130+
assert_eq!(map, map_back);
131+
}
132+
133+
#[test]
134+
fn representation() {
135+
// Test that the map is represented identically to a vector of tuples in the serialized data.
136+
let map = SerializableMap::<u8, u8>(BTreeMap::from([(120, 130), (140, 150)]));
137+
let map_as_vec = vec![(120u8, 130u8), (140, 150)];
138+
let map_serialized = asn1_serialize(&map).unwrap();
139+
let map_as_vec_serialized = asn1_serialize(&map_as_vec).unwrap();
140+
assert_eq!(map_serialized, map_as_vec_serialized);
141+
}
142+
143+
#[test]
144+
fn duplicate_key() {
145+
let map_as_vec = vec![(120u8, 130u8), (120, 150)];
146+
let map_serialized = asn1_serialize(&map_as_vec).unwrap();
147+
assert_eq!(
148+
asn1_deserialize::<SerializableMap<u8, u8>>(&map_serialized).unwrap_err(),
149+
"Serde error: Duplicate key: 120"
150+
);
151+
}
152+
153+
#[test]
154+
fn serialize_error() {
155+
// Coverage for possible errors during serialization.
156+
// ASN.1 cannot serialize BTreeMap, so we will use it to trigger an error.
157+
let map = SerializableMap(BTreeMap::from([(1u8, BTreeMap::from([(2u8, 3u8)]))]));
158+
assert!(asn1_serialize(&map)
159+
.unwrap_err()
160+
.starts_with("Unsupported Maps variants are not supported by this implementation"));
161+
}
162+
163+
#[test]
164+
fn unexpected_sequence_element() {
165+
// The deserializer will encounter an integer where it expects a tuple.
166+
let not_map_serialized = asn1_serialize(&[1u64, 2u64]).unwrap();
167+
assert!(asn1_deserialize::<SerializableMap<u8, u8>>(&not_map_serialized)
168+
.unwrap_err()
169+
.contains("Invalid encoding DER object is not a valid sequence"),);
170+
}
171+
172+
#[test]
173+
fn unexpected_type() {
174+
// Have to use JSON and not ASN1 here because `serde_asn1_der` doesn't seem to trigger `Visitor::expecting()`.
175+
let not_map_serialized = json_serialize(&1);
176+
assert_eq!(
177+
json_deserialize::<SerializableMap<u8, u8>>(&not_map_serialized).unwrap_err(),
178+
"invalid type: integer `1`, expected A map serialized as a list of pairs at line 1 column 1"
179+
);
180+
}
181+
}

0 commit comments

Comments
 (0)