ava-labs
Deserialization of a node in a freed area probably produces the wrong error
The problem here is that we currently deserialize StoredArea<Area<Node, FreeArea>> but only when it's expted to be a freed area. If there happens to be a node on a free list, bad things are likely to happen since we'll deserialize the node using serde.
The right fix is to remove serde everywhere.
We're not using it in many places any more, and it's clearly slower. Although it's in Cargo.toml for the firewood subdirectory, it isn't referenced anywhere in that directory. Only storage is currently using it, and it's only being used to serialize some stuff related to free space management, such as this one and another for AreaIndex.
This would also remove a lot of the benchmarks related to serde serialization timings, which show serde is almost always slower.
Bonus points if you can figure out why manual serialization of a full node is a tiny bit slower than serde.
Running benches/serializer.rs (/Users/rkuris/open-source/firewood/target/release/deps/serializer-6c232d65bd4e75a9)
leaf/serde time: [69.045 ns 69.287 ns 69.553 ns]
Found 1 outliers among 100 measurements (1.00%)
1 (1.00%) high mild
leaf/manual time: [39.281 ns 39.360 ns 39.450 ns]
Found 9 outliers among 100 measurements (9.00%)
9 (9.00%) high mild
has_value/serde time: [239.35 ns 243.83 ns 247.95 ns]
has_value/manual time: [80.964 ns 81.715 ns 83.037 ns]
Found 2 outliers among 100 measurements (2.00%)
1 (1.00%) high mild
1 (1.00%) high severe
1_child/serde time: [153.20 ns 154.21 ns 155.28 ns]
1_child/manual time: [73.337 ns 73.438 ns 73.569 ns]
Found 14 outliers among 100 measurements (14.00%)
7 (7.00%) high mild
7 (7.00%) high severe
2_child/serde time: [246.32 ns 251.62 ns 256.86 ns]
2_child/manual time: [106.19 ns 106.60 ns 107.05 ns]
Found 5 outliers among 100 measurements (5.00%)
5 (5.00%) high mild
16_child/serde time: [483.69 ns 494.48 ns 504.73 ns]
16_child/manual time: [502.68 ns 505.68 ns 509.86 ns]
Found 4 outliers among 100 measurements (4.00%)
1 (1.00%) high mild
3 (3.00%) high severe
git grep bincode points to all the places that require new code:
firewood/Cargo.toml:bincode = "1.3.3"
storage/Cargo.toml:bincode = "1.3.3"
storage/benches/serializer.rs:use bincode::Options;
storage/benches/serializer.rs: let serializer = bincode::DefaultOptions::new().with_varint_encoding();
storage/benches/serializer.rs: let serializer = bincode::DefaultOptions::new().with_varint_encoding();
storage/src/nodestore.rs:use bincode::{DefaultOptions, Options as _};
storage/src/nodestore.rs:fn serializer() -> impl bincode::Options {
git grep '\(Des\|S\)erialize' points to all the places you can remove code, as you'll no longer need to derive or implement serialization using serde for all these structures:
grpc-testtool/proto/sync/sync.proto: SerializedPath key = 1;
grpc-testtool/proto/sync/sync.proto:message SerializedPath {
grpc-testtool/src/bin/process-server.rs:use serde::Deserialize;
grpc-testtool/src/bin/process-server.rs:#[derive(Clone, Debug, Deserialize)]
storage/src/node/branch.rs:use serde::ser::SerializeStruct as _;
storage/src/node/branch.rs:use serde::{Deserialize, Serialize};
storage/src/node/branch.rs: use serde::{Deserialize, Serialize};
storage/src/node/branch.rs: #[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
storage/src/node/branch.rs:impl Serialize for BranchNode {
storage/src/node/branch.rs: S: serde::Serializer,
storage/src/node/branch.rs:impl<'de> Deserialize<'de> for BranchNode {
storage/src/node/branch.rs: D: serde::Deserializer<'de>,
storage/src/node/branch.rs: #[derive(Deserialize)]
storage/src/node/branch.rs: struct SerializedBranchNode {
storage/src/node/branch.rs: let s: SerializedBranchNode = Deserialize::deserialize(deserializer)?;
storage/src/node/leaf.rs:use serde::{Deserialize, Serialize};
storage/src/node/leaf.rs:#[derive(PartialEq, Eq, Clone, Serialize, Deserialize)]
storage/src/node/mod.rs:use serde::{Deserialize, Serialize};
storage/src/node/mod.rs:#[derive(PartialEq, Eq, Clone, Debug, EnumAsInner, Serialize, Deserialize)]
storage/src/node/path.rs:use serde::{Deserialize, Serialize};
storage/src/node/path.rs:#[derive(PartialEq, Eq, Clone, Serialize, Deserialize)]
storage/src/nodestore.rs:use serde::{Deserialize, Serialize};
storage/src/nodestore.rs:#[derive(PartialEq, Eq, Clone, Debug, Deserialize, Serialize)]
storage/src/nodestore.rs:#[derive(PartialEq, Eq, Clone, Debug, Deserialize, Serialize)]
storage/src/nodestore.rs:#[derive(Debug, Clone, Copy, PartialEq, Eq, Deserialize, Serialize, NoUninit, AnyBitPattern)]
storage/src/nodestore.rs:#[derive(Copy, Debug, PartialEq, Eq, Serialize, Deserialize, Clone, NoUninit, AnyBitPattern)]
storage/src/nodestore.rs:#[derive(Debug, PartialEq, Eq, Clone, Copy, Serialize, Deserialize)]
storage/src/trie_hash.rs:use serde::{Deserialize, Serialize};
storage/src/trie_hash.rs:impl Serialize for TrieHash {
storage/src/trie_hash.rs: S: serde::Serializer,
storage/src/trie_hash.rs:impl<'de> Deserialize<'de> for TrieHash {
storage/src/trie_hash.rs: D: serde::Deserializer<'de>,
grpc-testtool might still want or need serde, or this can be done in another ticket, although it looks like it's not used much there.
Fix will include fixes for https://github.com/ava-labs/firewood/issues/1056 and https://github.com/ava-labs/firewood/issues/1057 as well but will require breaking the storage file format.
