From 84b740c4a4f603f42f94d279c0dab92d24f08f17 Mon Sep 17 00:00:00 2001 From: Josh Jones Date: Mon, 24 Jul 2023 21:30:40 -0500 Subject: [PATCH 01/33] [WIP] draft changes --- src/blocks/tipset.rs | 31 ++++++----- src/ipld/cid_vec.rs | 123 +++++++++++++++++++++++++++++++++++++++++++ src/ipld/mod.rs | 1 + src/ipld/util.rs | 4 +- 4 files changed, 143 insertions(+), 16 deletions(-) create mode 100644 src/ipld/cid_vec.rs diff --git a/src/blocks/tipset.rs b/src/blocks/tipset.rs index 992e8435c0c6..6120fd95abd9 100644 --- a/src/blocks/tipset.rs +++ b/src/blocks/tipset.rs @@ -3,6 +3,7 @@ use std::{fmt, sync::OnceLock}; +use crate::ipld::cid_vec::CidVec; use crate::networks::{calibnet, mainnet}; use crate::shim::{address::Address, clock::ChainEpoch}; use crate::utils::cid::CidCborExt; @@ -25,17 +26,17 @@ use super::{Block, BlockHeader, Error, Ticket}; #[derive(Clone, Debug, PartialEq, Eq, Hash, Default, Serialize, Deserialize)] #[serde(transparent)] pub struct TipsetKeys { - pub cids: Vec, + pub cids: CidVec, } impl TipsetKeys { - pub fn new(cids: Vec) -> Self { + pub fn new(cids: CidVec) -> Self { Self { cids } } /// Returns tipset header `cids` - pub fn cids(&self) -> &[Cid] { - &self.cids + pub fn cids(self) -> CidVec { + self.cids } // Special encoding to match Lotus. @@ -53,7 +54,7 @@ impl fmt::Display for TipsetKeys { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { let s = self .cids() - .iter() + .into_iter() .map(|cid| cid.to_string()) .collect::>() .join(", "); @@ -141,8 +142,8 @@ impl Tipset { pub fn load(store: impl Blockstore, tsk: &TipsetKeys) -> anyhow::Result> { Ok(tsk .cids() - .iter() - .map(|key| BlockHeader::load(&store, *key)) + .into_iter() + .map(|key| BlockHeader::load(&store, key)) .collect::>>()? .map(Tipset::new) .transpose()?) @@ -220,7 +221,7 @@ impl Tipset { } /// Returns slice of `CIDs` for the current tipset pub fn cids(&self) -> &[Cid] { - self.key().cids() + self.key().cids.cids() } /// Returns the keys of the parents of the blocks in the tipset. pub fn parents(&self) -> &TipsetKeys { @@ -446,7 +447,7 @@ pub mod tipset_keys_json { where S: Serializer, { - crate::json::cid::vec::serialize(m.cids(), serializer) + crate::json::cid::vec::serialize(m.cids().into(), serializer) } pub fn deserialize<'de, D>(deserializer: D) -> Result @@ -454,7 +455,7 @@ pub mod tipset_keys_json { D: Deserializer<'de>, { Ok(TipsetKeys { - cids: crate::json::cid::vec::deserialize(deserializer)?, + cids: crate::json::cid::vec::deserialize(deserializer)?.into(), }) } } @@ -509,7 +510,7 @@ pub mod tipset_json { } TipsetSer { blocks: &m.headers, - cids: m.key(), + cids: &m.key(), height: m.epoch(), } .serialize(serializer) @@ -535,6 +536,7 @@ pub mod tipset_json { #[cfg(test)] mod property_tests { + use crate::ipld::cid_vec::CidVec; use quickcheck_macros::quickcheck; use serde_json; @@ -547,7 +549,7 @@ mod property_tests { impl quickcheck::Arbitrary for TipsetKeys { fn arbitrary(g: &mut quickcheck::Gen) -> Self { Self { - cids: Vec::arbitrary(g), + cids: CidVec::arbitrary(g), } } } @@ -569,6 +571,7 @@ mod property_tests { #[cfg(test)] mod test { + use crate::ipld::cid_vec::CidVec; use crate::json::vrf::VRFProof; use crate::shim::address::Address; use cid::{ @@ -719,10 +722,10 @@ mod test { .unwrap(); let h1 = BlockHeader::builder() .miner_address(Address::new_id(1)) - .parents(TipsetKeys::new(vec![Cid::new_v1( + .parents(TipsetKeys::new(CidVec::new_from_cid(Cid::new_v1( DAG_CBOR, Identity.digest(&[]), - )])) + )))) .build() .unwrap(); assert_eq!( diff --git a/src/ipld/cid_vec.rs b/src/ipld/cid_vec.rs new file mode 100644 index 000000000000..4a261f46f74e --- /dev/null +++ b/src/ipld/cid_vec.rs @@ -0,0 +1,123 @@ +// Copyright 2019-2023 ChainSafe Systems +// SPDX-License-Identifier: Apache-2.0, MIT + +use crate::utils::cid::{CidVariant, BLAKE2B256_SIZE}; +use cid::{ + multihash::{self, MultihashDigest}, + Cid, +}; +use fvm_ipld_encoding::DAG_CBOR; +use serde::{Deserialize, Serialize}; + +#[derive(Clone, Debug, Default, Deserialize, Eq, Hash, PartialEq, Serialize)] +pub struct CidVec { + pub v1_dagcbor_blake2b_vec: Vec<[u8; BLAKE2B256_SIZE]>, + pub fallback_vec: Vec, +} + +impl FromIterator for CidVec { + fn from_iter>(iter: T) -> Self { + let mut vec = Self::new(); + for i in iter { + vec.push(i); + } + vec + } +} + +impl From> for CidVec { + fn from(vec: Vec) -> Self { + vec.into_iter().collect() + } +} + +impl From for &[Cid] { + fn from(vec: CidVec) -> Self { + //TODO: Add v1_dagcbor_blake2b_vec + &vec.fallback_vec.as_slice() + } +} + +impl CidVec { + pub fn new() -> Self { + Self { + v1_dagcbor_blake2b_vec: Vec::new(), + fallback_vec: Vec::new(), + } + } + + pub fn new_from_cid(cid: Cid) -> Self { + let mut vec = Self::new(); + vec.push(cid); + vec + } + + pub fn push(&mut self, cid: Cid) { + match cid.try_into() { + Ok(CidVariant::V1DagCborBlake2b(bytes)) => self.v1_dagcbor_blake2b_vec.push(bytes), + Err(()) => self.fallback_vec.push(cid), + } + } + + pub fn is_empty(&self) -> bool { + self.v1_dagcbor_blake2b_vec.is_empty() && self.fallback_vec.is_empty() + } + + pub fn to_vec(&self) -> Vec { + self.v1_dagcbor_blake2b_vec + .iter() + .map(|bytes| Cid::new_v1(DAG_CBOR, multihash::Code::Blake2b256.digest(bytes))) + .chain(self.fallback_vec.iter().cloned()) + .collect() + } + + pub fn contains(&self, cid: Cid) -> bool { + match cid.try_into() { + Ok(CidVariant::V1DagCborBlake2b(bytes)) => self.v1_dagcbor_blake2b_vec.contains(&bytes), + Err(()) => self.fallback_vec.contains(&cid), + } + } + + pub fn cids(&self) -> &[Cid] { + //TODO: Add v1_dagcbor_blake2b_vec + &self.fallback_vec + } +} + +impl Iterator for CidVec { + type Item = Cid; + + fn next(&mut self) -> Option { + if let Some(bytes) = self.v1_dagcbor_blake2b_vec.pop() { + Some(Cid::new_v1( + DAG_CBOR, + multihash::Code::Blake2b256.digest(&bytes), + )) + } else { + self.fallback_vec.pop() + } + } +} + +#[cfg(test)] +mod test { + use super::*; + use quickcheck::Arbitrary; + + impl Arbitrary for CidVec { + fn arbitrary(g: &mut quickcheck::Gen) -> Self { + let arbitrary_vec: Vec = Vec::arbitrary(g); + Self { + v1_dagcbor_blake2b_vec: arbitrary_vec + .iter() + .map(|x| { + let mut bytes = [0u8; BLAKE2B256_SIZE]; + bytes.copy_from_slice(&x.to_be_bytes()); + bytes + }) + .collect(), + fallback_vec: Vec::arbitrary(g), + } + } + } +} diff --git a/src/ipld/mod.rs b/src/ipld/mod.rs index a2c006d108eb..5dbcc2d77fbd 100644 --- a/src/ipld/mod.rs +++ b/src/ipld/mod.rs @@ -2,6 +2,7 @@ // SPDX-License-Identifier: Apache-2.0, MIT mod cid_hashset; +pub mod cid_vec; pub mod json; pub mod selector; pub mod util; diff --git a/src/ipld/util.rs b/src/ipld/util.rs index 58513eda5415..e1be4ac9a432 100644 --- a/src/ipld/util.rs +++ b/src/ipld/util.rs @@ -165,11 +165,11 @@ where if h.epoch() > 0 { for p in h.parents().cids() { - blocks_to_walk.push_back(*p); + blocks_to_walk.push_back(p); } } else { for p in h.parents().cids() { - load_block(*p).await?; + load_block(p).await?; } } From 08d9345fb89eca3ec5147d50c9b09a89a75c6c6c Mon Sep 17 00:00:00 2001 From: Josh Jones Date: Tue, 25 Jul 2023 07:46:25 -0500 Subject: [PATCH 02/33] [WIP] `CidVec` updates --- Cargo.lock | 36 +++++++++++------------ src/blocks/tipset.rs | 12 ++++---- src/chain/store/chain_store.rs | 7 +++-- src/chain_sync/chain_muxer.rs | 2 +- src/cli/subcommands/archive_cmd.rs | 6 ++-- src/cli/subcommands/chain_cmd.rs | 4 ++- src/cli/subcommands/snapshot_cmd.rs | 2 +- src/genesis/mod.rs | 4 ++- src/ipld/cid_vec.rs | 7 ++--- src/ipld/mod.rs | 3 +- src/ipld/util.rs | 4 +-- src/libp2p/chain_exchange/provider.rs | 2 +- src/message_pool/msgpool/test_provider.rs | 2 +- src/rpc/mpool_api.rs | 2 +- 14 files changed, 50 insertions(+), 43 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3862ab1561c4..91fa6006771e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1166,9 +1166,9 @@ dependencies = [ [[package]] name = "clap" -version = "4.3.17" +version = "4.3.19" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5b0827b011f6f8ab38590295339817b0d26f344aa4932c3ced71b45b0c54b4a9" +checksum = "5fd304a20bff958a57f04c4e96a2e7594cc4490a0e809cbd48bb6437edaa452d" dependencies = [ "clap_builder", "clap_derive", @@ -1177,9 +1177,9 @@ dependencies = [ [[package]] name = "clap_builder" -version = "4.3.17" +version = "4.3.19" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9441b403be87be858db6a23edb493e7f694761acdc3343d5a0fcaafd304cbc9e" +checksum = "01c6a3f08f1fe5662a35cfe393aec09c4df95f60ee93b7556505260f75eee9e1" dependencies = [ "anstream", "anstyle", @@ -2171,9 +2171,9 @@ dependencies = [ [[package]] name = "either" -version = "1.8.1" +version = "1.9.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7fcaabb2fef8c910e7f4c7ce9f67a1283a1715879a7c230ca9d6d1ae31f16d91" +checksum = "a26ae43d7bcc3b814de94796a5e736d4029efb0ee900c12e2d54c993ad1a1e07" [[package]] name = "elliptic-curve" @@ -4762,9 +4762,9 @@ dependencies = [ [[package]] name = "libz-sys" -version = "1.1.9" +version = "1.1.10" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "56ee889ecc9568871456d42f603d6a0ce59ff328d291063a45cbdf0036baf6db" +checksum = "24e6ab01971eb092ffe6a7d42f49f9ff42662f17604681e2843ad65077ba47dc" dependencies = [ "cc", "pkg-config", @@ -6251,9 +6251,9 @@ dependencies = [ [[package]] name = "quinn-proto" -version = "0.9.3" +version = "0.9.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "67c10f662eee9c94ddd7135043e544f3c82fa839a1e7b865911331961b53186c" +checksum = "f31999cfc7927c4e212e60fd50934ab40e8e8bfd2d493d6095d2d306bc0764d9" dependencies = [ "bytes 1.4.0", "rand 0.8.5", @@ -6269,9 +6269,9 @@ dependencies = [ [[package]] name = "quote" -version = "1.0.31" +version = "1.0.32" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5fe8a65d69dd0808184ebb5f836ab526bb259db23c657efa38711b1072ee47f0" +checksum = "50f3b39ccfb720540debaa0164757101c08ecb8d326b15358ce76a62c7e85965" dependencies = [ "proc-macro2", ] @@ -6964,9 +6964,9 @@ checksum = "b0293b4b29daaf487284529cc2f5675b8e57c61f70167ba415a463651fd6a918" [[package]] name = "serde" -version = "1.0.174" +version = "1.0.175" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3b88756493a5bd5e5395d53baa70b194b05764ab85b59e43e4b8f4e1192fa9b1" +checksum = "5d25439cd7397d044e2748a6fe2432b5e85db703d6d097bd014b3c0ad1ebff0b" dependencies = [ "serde_derive", ] @@ -6991,9 +6991,9 @@ dependencies = [ [[package]] name = "serde_derive" -version = "1.0.174" +version = "1.0.175" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6e5c3a298c7f978e53536f95a63bdc4c4a64550582f31a0359a9afda6aede62e" +checksum = "b23f7ade6f110613c0d63858ddb8b94c1041f550eab58a16b371bdf2c9c80ab4" dependencies = [ "proc-macro2", "quote", @@ -9370,9 +9370,9 @@ checksum = "1a515f5799fe4961cb532f983ce2b23082366b898e52ffbce459c86f67c8378a" [[package]] name = "winnow" -version = "0.5.0" +version = "0.5.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "81fac9742fd1ad1bd9643b991319f72dd031016d44b77039a26977eb667141e7" +checksum = "25b5872fa2e10bd067ae946f927e726d7d603eaeb6e02fa6a350e0722d2b8c11" dependencies = [ "memchr", ] diff --git a/src/blocks/tipset.rs b/src/blocks/tipset.rs index 6120fd95abd9..c6c5c17c4b0c 100644 --- a/src/blocks/tipset.rs +++ b/src/blocks/tipset.rs @@ -3,7 +3,7 @@ use std::{fmt, sync::OnceLock}; -use crate::ipld::cid_vec::CidVec; +use crate::ipld::CidVec; use crate::networks::{calibnet, mainnet}; use crate::shim::{address::Address, clock::ChainEpoch}; use crate::utils::cid::CidCborExt; @@ -35,8 +35,8 @@ impl TipsetKeys { } /// Returns tipset header `cids` - pub fn cids(self) -> CidVec { - self.cids + pub fn cids(&self) -> &[Cid] { + &self.cids.cids() } // Special encoding to match Lotus. @@ -143,7 +143,7 @@ impl Tipset { Ok(tsk .cids() .into_iter() - .map(|key| BlockHeader::load(&store, key)) + .map(|key| BlockHeader::load(&store, *key)) .collect::>>()? .map(Tipset::new) .transpose()?) @@ -536,7 +536,7 @@ pub mod tipset_json { #[cfg(test)] mod property_tests { - use crate::ipld::cid_vec::CidVec; + use crate::ipld::CidVec; use quickcheck_macros::quickcheck; use serde_json; @@ -571,7 +571,7 @@ mod property_tests { #[cfg(test)] mod test { - use crate::ipld::cid_vec::CidVec; + use crate::ipld::CidVec; use crate::json::vrf::VRFProof; use crate::shim::address::Address; use cid::{ diff --git a/src/chain/store/chain_store.rs b/src/chain/store/chain_store.rs index 4c08fb8d3569..67f2e6118853 100644 --- a/src/chain/store/chain_store.rs +++ b/src/chain/store/chain_store.rs @@ -5,6 +5,7 @@ use std::{ops::DerefMut, path::Path, sync::Arc, time::SystemTime}; use crate::blocks::{BlockHeader, Tipset, TipsetKeys, TxMeta}; use crate::interpreter::BlockMessages; +use crate::ipld::CidVec; use crate::ipld::{walk_snapshot, WALK_SNAPSHOT_PROGRESS_EXPORT}; use crate::libp2p_bitswap::{BitswapStoreRead, BitswapStoreReadWrite}; use crate::message::{ChainMessage, Message as MessageTrait, SignedMessage}; @@ -129,13 +130,15 @@ where let file_backed_heaviest_tipset_keys = Mutex::new({ let mut head_store = FileBacked::load_from_file_or_create( chain_data_root.join("HEAD"), - || TipsetKeys::new(vec![*genesis_block_header.cid()]), + || TipsetKeys::new(CidVec::new_from_cid(*genesis_block_header.cid())), None, )?; let is_valid = chain_index.load_tipset(head_store.inner()).is_ok(); if !is_valid { // If the stored HEAD is invalid, reset it to the genesis tipset. - head_store.set_inner(TipsetKeys::new(vec![*genesis_block_header.cid()]))?; + head_store.set_inner(TipsetKeys::new(CidVec::new_from_cid( + *genesis_block_header.cid(), + )))?; } head_store }); diff --git a/src/chain_sync/chain_muxer.rs b/src/chain_sync/chain_muxer.rs index ab8d37ca2064..5bc0c475f3df 100644 --- a/src/chain_sync/chain_muxer.rs +++ b/src/chain_sync/chain_muxer.rs @@ -373,7 +373,7 @@ where metrics::LIBP2P_MESSAGE_TOTAL .with_label_values(&[metrics::values::HELLO_RESPONSE_OUTBOUND]) .inc(); - let tipset_keys = TipsetKeys::new(request.heaviest_tip_set); + let tipset_keys = TipsetKeys::new(request.heaviest_tip_set.into()); let tipset = match Self::get_full_tipset( network.clone(), chain_store.clone(), diff --git a/src/cli/subcommands/archive_cmd.rs b/src/cli/subcommands/archive_cmd.rs index 804be13610b3..269d344b809c 100644 --- a/src/cli/subcommands/archive_cmd.rs +++ b/src/cli/subcommands/archive_cmd.rs @@ -161,7 +161,7 @@ async fn do_export( tmp_chain_dir.path(), )?); - let ts = chain_store.tipset_from_keys(&TipsetKeys::new(chain_store.db.roots()))?; + let ts = chain_store.tipset_from_keys(&TipsetKeys::new(chain_store.db.roots().into()))?; info!("looking up a tipset by epoch: {}", epoch); @@ -231,7 +231,7 @@ impl ArchiveInfo { let store = UncompressedCarV1BackedBlockstore::new(reader) .context("couldn't read input CAR file - is it compressed?")?; - let root = Tipset::load(&store, &TipsetKeys::new(store.roots()))? + let root = Tipset::load(&store, &TipsetKeys::new(store.roots().into()))? .context("Missing root tipset")?; let root_epoch = root.epoch(); @@ -319,7 +319,7 @@ fn print_checkpoints(snapshot: PathBuf) -> anyhow::Result<()> { let file = std::fs::File::open(snapshot)?; let store = UncompressedCarV1BackedBlockstore::new(file) .context("couldn't read input CAR file - is it compressed?")?; - let root = Tipset::load_required(&store, &TipsetKeys::new(store.roots()))?; + let root = Tipset::load_required(&store, &TipsetKeys::new(store.roots().into()))?; let genesis = root.genesis(&store)?; let chain_name = if genesis.cid() == &*calibnet::GENESIS_CID { diff --git a/src/cli/subcommands/chain_cmd.rs b/src/cli/subcommands/chain_cmd.rs index c8df506590c1..8046400d9054 100644 --- a/src/cli/subcommands/chain_cmd.rs +++ b/src/cli/subcommands/chain_cmd.rs @@ -92,7 +92,9 @@ impl ChainCommands { } => { maybe_confirm(*no_confirm, SET_HEAD_CONFIRMATION_MESSAGE)?; chain_set_head( - (TipsetKeys { cids: cids.clone() },), + (TipsetKeys { + cids: cids.clone().into(), + },), &config.client.rpc_token, ) .await diff --git a/src/cli/subcommands/snapshot_cmd.rs b/src/cli/subcommands/snapshot_cmd.rs index 411af3fa110a..7d0f141c6726 100644 --- a/src/cli/subcommands/snapshot_cmd.rs +++ b/src/cli/subcommands/snapshot_cmd.rs @@ -277,7 +277,7 @@ async fn validate_with_blockstore( where BlockstoreT: Blockstore + Send + Sync + 'static, { - let tipset_key = TipsetKeys::new(roots); + let tipset_key = TipsetKeys::new(roots.into()); let store_clone = Arc::clone(&store); let ts = Tipset::load(&store_clone, &tipset_key)?.context("missing root tipset")?; diff --git a/src/genesis/mod.rs b/src/genesis/mod.rs index 5677a32282d5..65e4e81744b8 100644 --- a/src/genesis/mod.rs +++ b/src/genesis/mod.rs @@ -118,7 +118,9 @@ where meta.sync()?; } - let ts = sm.chain_store().tipset_from_keys(&TipsetKeys::new(cids))?; + let ts = sm + .chain_store() + .tipset_from_keys(&TipsetKeys::new(cids.into()))?; if !skip_load { let gb = sm.chain_store().chain_index.tipset_by_height( diff --git a/src/ipld/cid_vec.rs b/src/ipld/cid_vec.rs index 4a261f46f74e..5cb9c7b4fa76 100644 --- a/src/ipld/cid_vec.rs +++ b/src/ipld/cid_vec.rs @@ -31,10 +31,9 @@ impl From> for CidVec { } } -impl From for &[Cid] { - fn from(vec: CidVec) -> Self { - //TODO: Add v1_dagcbor_blake2b_vec - &vec.fallback_vec.as_slice() +impl From<&[Cid]> for CidVec { + fn from(vec: &[Cid]) -> Self { + vec.iter().cloned().collect() } } diff --git a/src/ipld/mod.rs b/src/ipld/mod.rs index 5dbcc2d77fbd..24ebe08120a0 100644 --- a/src/ipld/mod.rs +++ b/src/ipld/mod.rs @@ -2,7 +2,7 @@ // SPDX-License-Identifier: Apache-2.0, MIT mod cid_hashset; -pub mod cid_vec; +mod cid_vec; pub mod json; pub mod selector; pub mod util; @@ -12,6 +12,7 @@ pub use libipld_core::ipld::Ipld; pub use util::*; pub use self::cid_hashset::CidHashSet; +pub use self::cid_vec::CidVec; pub use libipld_core::serde::{from_ipld, to_ipld}; #[cfg(test)] diff --git a/src/ipld/util.rs b/src/ipld/util.rs index e1be4ac9a432..58513eda5415 100644 --- a/src/ipld/util.rs +++ b/src/ipld/util.rs @@ -165,11 +165,11 @@ where if h.epoch() > 0 { for p in h.parents().cids() { - blocks_to_walk.push_back(p); + blocks_to_walk.push_back(*p); } } else { for p in h.parents().cids() { - load_block(p).await?; + load_block(*p).await?; } } diff --git a/src/libp2p/chain_exchange/provider.rs b/src/libp2p/chain_exchange/provider.rs index aaf0050e55ce..cbdfe75984a3 100644 --- a/src/libp2p/chain_exchange/provider.rs +++ b/src/libp2p/chain_exchange/provider.rs @@ -27,7 +27,7 @@ where loop { let mut tipset_bundle: TipsetBundle = TipsetBundle::default(); - let tipset = match cs.tipset_from_keys(&TipsetKeys::new(curr_tipset_cids)) { + let tipset = match cs.tipset_from_keys(&TipsetKeys::new(curr_tipset_cids.into())) { Ok(tipset) => tipset, Err(err) => { debug!("Cannot get tipset from keys: {}", err); diff --git a/src/message_pool/msgpool/test_provider.rs b/src/message_pool/msgpool/test_provider.rs index c36948db427a..cf3d12b7940a 100644 --- a/src/message_pool/msgpool/test_provider.rs +++ b/src/message_pool/msgpool/test_provider.rs @@ -189,7 +189,7 @@ impl Provider for TestApi { fn load_tipset(&self, tsk: &TipsetKeys) -> Result, Error> { let inner = self.inner.lock(); for ts in &inner.tipsets { - if tsk.cids == ts.cids() { + if tsk.cids == ts.cids().into() { return Ok(ts.clone().into()); } } diff --git a/src/rpc/mpool_api.rs b/src/rpc/mpool_api.rs index ffce38222a63..71ac54f7a594 100644 --- a/src/rpc/mpool_api.rs +++ b/src/rpc/mpool_api.rs @@ -28,7 +28,7 @@ where DB: Blockstore + Clone + Send + Sync + 'static, { let (CidJsonVec(cid_vec),) = params; - let tsk = TipsetKeys::new(cid_vec); + let tsk = TipsetKeys::new(cid_vec.into()); let mut ts = data.state_manager.chain_store().tipset_from_keys(&tsk)?; let (mut pending, mpts) = data.mpool.pending()?; From 738c0cd34581ed7f4b4fad946f8d62a6a8f09d69 Mon Sep 17 00:00:00 2001 From: Josh Jones Date: Tue, 25 Jul 2023 16:48:07 -0500 Subject: [PATCH 03/33] changes to `Tipset::cids` and `quickcheck` --- src/blocks/tipset.rs | 10 ++++---- src/chain/store/chain_store.rs | 4 +-- src/chain_sync/tipset_syncer.rs | 12 ++++----- src/ipld/cid_vec.rs | 43 +++++++++++++++------------------ src/ipld/util.rs | 4 +-- 5 files changed, 33 insertions(+), 40 deletions(-) diff --git a/src/blocks/tipset.rs b/src/blocks/tipset.rs index c6c5c17c4b0c..32bb9bbd40bb 100644 --- a/src/blocks/tipset.rs +++ b/src/blocks/tipset.rs @@ -35,8 +35,8 @@ impl TipsetKeys { } /// Returns tipset header `cids` - pub fn cids(&self) -> &[Cid] { - &self.cids.cids() + pub fn cids(&self) -> Vec { + self.cids.cids() } // Special encoding to match Lotus. @@ -143,7 +143,7 @@ impl Tipset { Ok(tsk .cids() .into_iter() - .map(|key| BlockHeader::load(&store, *key)) + .map(|key| BlockHeader::load(&store, key)) .collect::>>()? .map(Tipset::new) .transpose()?) @@ -220,7 +220,7 @@ impl Tipset { }) } /// Returns slice of `CIDs` for the current tipset - pub fn cids(&self) -> &[Cid] { + pub fn cids(&self) -> Vec { self.key().cids.cids() } /// Returns the keys of the parents of the blocks in the tipset. @@ -447,7 +447,7 @@ pub mod tipset_keys_json { where S: Serializer, { - crate::json::cid::vec::serialize(m.cids().into(), serializer) + crate::json::cid::vec::serialize(&m.cids(), serializer) } pub fn deserialize<'de, D>(deserializer: D) -> Result diff --git a/src/chain/store/chain_store.rs b/src/chain/store/chain_store.rs index 67f2e6118853..fb6e8561f05a 100644 --- a/src/chain/store/chain_store.rs +++ b/src/chain/store/chain_store.rs @@ -274,9 +274,9 @@ where file.insert(*cid); } - pub fn unmark_block_as_validated(&self, cid: &Cid) { + pub fn unmark_block_as_validated(&self, cid: Cid) { let mut file = self.validated_blocks.lock(); - let _did_work = file.remove(cid); + let _did_work = file.remove(&cid); } // FIXME: This function doesn't use the chain store at all. diff --git a/src/chain_sync/tipset_syncer.rs b/src/chain_sync/tipset_syncer.rs index 769fbe7a6565..03567da12ea8 100644 --- a/src/chain_sync/tipset_syncer.rs +++ b/src/chain_sync/tipset_syncer.rs @@ -882,7 +882,7 @@ async fn sync_headers_in_reverse( descendant_blocks: &[Cid], ) -> Result<(), TipsetRangeSyncerError> { for cid in tipset.cids() { - if let Some(reason) = bad_block_cache.get(cid) { + if let Some(reason) = bad_block_cache.get(&cid) { for block_cid in descendant_blocks { bad_block_cache.put(*block_cid, format!("chain contained {cid}")); } - return Err(TipsetRangeSyncerError::TipsetRangeWithBadBlock( - *cid, reason, - )); + return Err(TipsetRangeSyncerError::TipsetRangeWithBadBlock(cid, reason)); } } Ok(()) diff --git a/src/ipld/cid_vec.rs b/src/ipld/cid_vec.rs index 5cb9c7b4fa76..4b6c9ed0c07d 100644 --- a/src/ipld/cid_vec.rs +++ b/src/ipld/cid_vec.rs @@ -37,6 +37,12 @@ impl From<&[Cid]> for CidVec { } } +impl From for Vec { + fn from(vec: CidVec) -> Self { + vec.cids() + } +} + impl CidVec { pub fn new() -> Self { Self { @@ -58,29 +64,14 @@ impl CidVec { } } - pub fn is_empty(&self) -> bool { - self.v1_dagcbor_blake2b_vec.is_empty() && self.fallback_vec.is_empty() - } - - pub fn to_vec(&self) -> Vec { + pub fn cids(&self) -> Vec { self.v1_dagcbor_blake2b_vec .iter() .map(|bytes| Cid::new_v1(DAG_CBOR, multihash::Code::Blake2b256.digest(bytes))) - .chain(self.fallback_vec.iter().cloned()) + .into_iter() + .chain(self.fallback_vec.clone()) .collect() } - - pub fn contains(&self, cid: Cid) -> bool { - match cid.try_into() { - Ok(CidVariant::V1DagCborBlake2b(bytes)) => self.v1_dagcbor_blake2b_vec.contains(&bytes), - Err(()) => self.fallback_vec.contains(&cid), - } - } - - pub fn cids(&self) -> &[Cid] { - //TODO: Add v1_dagcbor_blake2b_vec - &self.fallback_vec - } } impl Iterator for CidVec { @@ -100,23 +91,27 @@ impl Iterator for CidVec { #[cfg(test)] mod test { + use crate::utils::encoding::blake2b_256; + use super::*; use quickcheck::Arbitrary; + use quickcheck_macros::quickcheck; impl Arbitrary for CidVec { fn arbitrary(g: &mut quickcheck::Gen) -> Self { - let arbitrary_vec: Vec = Vec::arbitrary(g); + let arbitrary_vec: Vec = Vec::arbitrary(g); Self { v1_dagcbor_blake2b_vec: arbitrary_vec .iter() - .map(|x| { - let mut bytes = [0u8; BLAKE2B256_SIZE]; - bytes.copy_from_slice(&x.to_be_bytes()); - bytes - }) + .map(|i| blake2b_256(&i.to_be_bytes())) .collect(), fallback_vec: Vec::arbitrary(g), } } } + + #[quickcheck] + fn cidvec_to_vec_of_cids_to_cidvec(cidvec: CidVec) { + assert_eq!(cidvec, CidVec::from(Vec::::from(cidvec.clone()))); + } } diff --git a/src/ipld/util.rs b/src/ipld/util.rs index 58513eda5415..e1be4ac9a432 100644 --- a/src/ipld/util.rs +++ b/src/ipld/util.rs @@ -165,11 +165,11 @@ where if h.epoch() > 0 { for p in h.parents().cids() { - blocks_to_walk.push_back(*p); + blocks_to_walk.push_back(p); } } else { for p in h.parents().cids() { - load_block(*p).await?; + load_block(p).await?; } } From 3e82f76ee96a2ec2367bd48e36a64f32cb51378d Mon Sep 17 00:00:00 2001 From: Josh Jones Date: Wed, 26 Jul 2023 16:27:54 -0500 Subject: [PATCH 04/33] updates to `cid_vec` --- src/ipld/cid_vec.rs | 115 ++++++++++++++++++++++++++------------------ src/rpc/sync_api.rs | 2 +- 2 files changed, 69 insertions(+), 48 deletions(-) diff --git a/src/ipld/cid_vec.rs b/src/ipld/cid_vec.rs index 4b6c9ed0c07d..fb55f27454c5 100644 --- a/src/ipld/cid_vec.rs +++ b/src/ipld/cid_vec.rs @@ -2,17 +2,21 @@ // SPDX-License-Identifier: Apache-2.0, MIT use crate::utils::cid::{CidVariant, BLAKE2B256_SIZE}; -use cid::{ - multihash::{self, MultihashDigest}, - Cid, -}; +use cid::{multihash, Cid}; use fvm_ipld_encoding::DAG_CBOR; use serde::{Deserialize, Serialize}; -#[derive(Clone, Debug, Default, Deserialize, Eq, Hash, PartialEq, Serialize)] -pub struct CidVec { - pub v1_dagcbor_blake2b_vec: Vec<[u8; BLAKE2B256_SIZE]>, - pub fallback_vec: Vec, +// CidVec takes advantage of the fact that the V1 DAG-CBOR Blake2b-256 variant (which can be stored in 32 bytes vs 96 bytes for a `Cid` type) is +99.99% of all CIDs. CidVec defaults to the `Vec<[u8; BLAKE2B256_SIZE]>` type, only using the more expensive `Vec` type when necessary. +#[derive(Clone, Debug, PartialEq, Eq, Hash, Serialize, Deserialize)] +pub enum CidVec { + V1Cids(Vec<[u8; BLAKE2B256_SIZE]>), + AllCids(Vec), +} + +impl Default for CidVec { + fn default() -> Self { + Self::V1Cids(Vec::new()) + } } impl FromIterator for CidVec { @@ -27,6 +31,7 @@ impl FromIterator for CidVec { impl From> for CidVec { fn from(vec: Vec) -> Self { + // TODO: add logic to convert to V1Cids if possible vec.into_iter().collect() } } @@ -45,73 +50,89 @@ impl From for Vec { impl CidVec { pub fn new() -> Self { - Self { - v1_dagcbor_blake2b_vec: Vec::new(), - fallback_vec: Vec::new(), - } + Self::default() } pub fn new_from_cid(cid: Cid) -> Self { - let mut vec = Self::new(); - vec.push(cid); - vec - } - - pub fn push(&mut self, cid: Cid) { match cid.try_into() { - Ok(CidVariant::V1DagCborBlake2b(bytes)) => self.v1_dagcbor_blake2b_vec.push(bytes), - Err(()) => self.fallback_vec.push(cid), + Ok(CidVariant::V1DagCborBlake2b(bytes)) => Self::V1Cids(vec![bytes]), + _ => Self::AllCids(vec![cid]), } } pub fn cids(&self) -> Vec { - self.v1_dagcbor_blake2b_vec - .iter() - .map(|bytes| Cid::new_v1(DAG_CBOR, multihash::Code::Blake2b256.digest(bytes))) - .into_iter() - .chain(self.fallback_vec.clone()) - .collect() + match self { + Self::V1Cids(cids) => cids + .iter() + .map(|c| { + Cid::new_v1( + DAG_CBOR, + multihash::Multihash::wrap(DAG_CBOR, c) + .expect("failed to convert digest to CID"), + ) + }) + .collect(), + Self::AllCids(cids) => cids.clone(), + } } -} - -impl Iterator for CidVec { - type Item = Cid; - fn next(&mut self) -> Option { - if let Some(bytes) = self.v1_dagcbor_blake2b_vec.pop() { - Some(Cid::new_v1( - DAG_CBOR, - multihash::Code::Blake2b256.digest(&bytes), - )) - } else { - self.fallback_vec.pop() + pub fn push(&mut self, cid: Cid) { + match self { + Self::V1Cids(cids) => { + if let Ok(CidVariant::V1DagCborBlake2b(bytes)) = cid.try_into() { + cids.push(bytes); + } else { + let mut cids: Vec = std::mem::take(cids) + .into_iter() + .map(|c| { + Cid::new_v1( + DAG_CBOR, + multihash::Multihash::wrap(DAG_CBOR, &c) + .expect("failed to convert digest to CID"), + ) + }) + .collect(); + cids.push(cid); + *self = Self::AllCids(cids); + } + } + Self::AllCids(cids) => cids.push(cid), } } } #[cfg(test)] mod test { - use crate::utils::encoding::blake2b_256; - use super::*; + use cid::multihash::MultihashDigest; use quickcheck::Arbitrary; use quickcheck_macros::quickcheck; impl Arbitrary for CidVec { fn arbitrary(g: &mut quickcheck::Gen) -> Self { - let arbitrary_vec: Vec = Vec::arbitrary(g); - Self { - v1_dagcbor_blake2b_vec: arbitrary_vec - .iter() - .map(|i| blake2b_256(&i.to_be_bytes())) - .collect(), - fallback_vec: Vec::arbitrary(g), + // Although the vast majority of CIDs are V1DagCborBlake2b, we want to generate the variants of CidVec with equal probability. + if bool::arbitrary(g) { + Vec::arbitrary(g).into_iter().collect() + } else { + // Quickcheck does not reliably generate the DAG_CBOR/Blake2b variant of V1 CIDs, but we can manually create them from an arbitrary Vec. + let vec: Vec = Vec::arbitrary(g); + vec.into_iter() + .map(|bytes| { + Cid::new_v1( + DAG_CBOR, + multihash::Code::Blake2b256.digest(&bytes.to_be_bytes()), + ) + }) + .collect() } } } #[quickcheck] fn cidvec_to_vec_of_cids_to_cidvec(cidvec: CidVec) { + // TODO: remove println statements after resolving failing case (i.e., conversion from Vec back to V1Cids) + println!("cidvec: {:?}", cidvec); + println!("vec_cid: {:?}", Vec::::from(cidvec.clone())); assert_eq!(cidvec, CidVec::from(Vec::::from(cidvec.clone()))); } } diff --git a/src/rpc/sync_api.rs b/src/rpc/sync_api.rs index 4cb9ff65db62..efef8c033143 100644 --- a/src/rpc/sync_api.rs +++ b/src/rpc/sync_api.rs @@ -118,7 +118,7 @@ mod tests { let header = from_slice::(&bz).unwrap(); let ts = Tipset::from(header); let db = cs_for_test.blockstore(); - let tsk = ts.key().cids.clone(); + let tsk = ts.key().cids.cids().clone(); cs_for_test.set_heaviest_tipset(Arc::new(ts)).unwrap(); for i in tsk { From 4adae1a918ce3f0b76d88b0606cc329f2a4c3e52 Mon Sep 17 00:00:00 2001 From: Josh Jones Date: Thu, 27 Jul 2023 09:30:27 -0500 Subject: [PATCH 05/33] resolve issue with `From` impl --- src/blocks/tipset.rs | 2 +- src/ipld/cid_vec.rs | 20 ++++++++++++-------- src/rpc/sync_api.rs | 4 ++-- 3 files changed, 15 insertions(+), 11 deletions(-) diff --git a/src/blocks/tipset.rs b/src/blocks/tipset.rs index 32bb9bbd40bb..2078b9fb5088 100644 --- a/src/blocks/tipset.rs +++ b/src/blocks/tipset.rs @@ -510,7 +510,7 @@ pub mod tipset_json { } TipsetSer { blocks: &m.headers, - cids: &m.key(), + cids: m.key(), height: m.epoch(), } .serialize(serializer) diff --git a/src/ipld/cid_vec.rs b/src/ipld/cid_vec.rs index fb55f27454c5..3a009d4797ad 100644 --- a/src/ipld/cid_vec.rs +++ b/src/ipld/cid_vec.rs @@ -2,7 +2,10 @@ // SPDX-License-Identifier: Apache-2.0, MIT use crate::utils::cid::{CidVariant, BLAKE2B256_SIZE}; -use cid::{multihash, Cid}; +use cid::{ + multihash::{self, Code::Blake2b256}, + Cid, +}; use fvm_ipld_encoding::DAG_CBOR; use serde::{Deserialize, Serialize}; @@ -31,8 +34,12 @@ impl FromIterator for CidVec { impl From> for CidVec { fn from(vec: Vec) -> Self { - // TODO: add logic to convert to V1Cids if possible - vec.into_iter().collect() + // Converts `Vec` to `CidVec::V1Cids` if possible; otherwise, converts to `CidVec::AllCids`. + let mut cid_vec = CidVec::new(); + for cid in vec { + cid_vec.push(cid); + } + cid_vec } } @@ -67,7 +74,7 @@ impl CidVec { .map(|c| { Cid::new_v1( DAG_CBOR, - multihash::Multihash::wrap(DAG_CBOR, c) + multihash::Multihash::wrap(Blake2b256.into(), c) .expect("failed to convert digest to CID"), ) }) @@ -87,7 +94,7 @@ impl CidVec { .map(|c| { Cid::new_v1( DAG_CBOR, - multihash::Multihash::wrap(DAG_CBOR, &c) + multihash::Multihash::wrap(Blake2b256.into(), &c) .expect("failed to convert digest to CID"), ) }) @@ -130,9 +137,6 @@ mod test { #[quickcheck] fn cidvec_to_vec_of_cids_to_cidvec(cidvec: CidVec) { - // TODO: remove println statements after resolving failing case (i.e., conversion from Vec back to V1Cids) - println!("cidvec: {:?}", cidvec); - println!("vec_cid: {:?}", Vec::::from(cidvec.clone())); assert_eq!(cidvec, CidVec::from(Vec::::from(cidvec.clone()))); } } diff --git a/src/rpc/sync_api.rs b/src/rpc/sync_api.rs index efef8c033143..3fa06359d322 100644 --- a/src/rpc/sync_api.rs +++ b/src/rpc/sync_api.rs @@ -102,7 +102,7 @@ mod tests { ChainStore::new( db, chain_config.clone(), - genesis_header.clone(), + genesis_header, chain_data_root.path(), ) .unwrap(), @@ -118,7 +118,7 @@ mod tests { let header = from_slice::(&bz).unwrap(); let ts = Tipset::from(header); let db = cs_for_test.blockstore(); - let tsk = ts.key().cids.cids().clone(); + let tsk = ts.key().cids.cids(); cs_for_test.set_heaviest_tipset(Arc::new(ts)).unwrap(); for i in tsk { From 97dc34f4af754d334c160d067fb4258c2e46691a Mon Sep 17 00:00:00 2001 From: Josh Jones Date: Thu, 27 Jul 2023 10:57:28 -0500 Subject: [PATCH 06/33] fix issues from merge conflict resolution --- src/cli/subcommands/archive_cmd.rs | 4 ++-- src/ipld/util.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/cli/subcommands/archive_cmd.rs b/src/cli/subcommands/archive_cmd.rs index 9a0d50950a3f..8134fb567535 100644 --- a/src/cli/subcommands/archive_cmd.rs +++ b/src/cli/subcommands/archive_cmd.rs @@ -138,7 +138,7 @@ async fn do_export( let store = Arc::new(AnyCar::new(reader).context("couldn't read input CAR file")?); let index = ChainIndex::new(store.clone()); - let ts = index.load_tipset(&TipsetKeys::new(store.roots()))?; + let ts = index.load_tipset(&TipsetKeys::new(store.roots().into()))?; let genesis = ts.genesis(&store)?; let network = if genesis.cid() == &*calibnet::GENESIS_CID { @@ -373,7 +373,7 @@ mod tests { fn genesis_timestamp(reader: impl Read + Seek) -> u64 { let db = crate::db::car::PlainCar::new(reader).unwrap(); - let ts = Tipset::load_required(&db, &TipsetKeys::new(db.roots())).unwrap(); + let ts = Tipset::load_required(&db, &TipsetKeys::new(db.roots().into())).unwrap(); ts.genesis(&db).unwrap().timestamp() } diff --git a/src/ipld/util.rs b/src/ipld/util.rs index 7649231f6cb5..c9069a54c296 100644 --- a/src/ipld/util.rs +++ b/src/ipld/util.rs @@ -357,7 +357,7 @@ impl + Unpin> Stream for ChainStream< if block.epoch() == 0 { // The genesis block has some kind of dummy parent that needs to be emitted. for p in block.parents().cids() { - this.dfs.push_back(Emit(*p)); + this.dfs.push_back(Emit(p)); } } From f7a168095fe0daa239371bb15ab3cdc2656dc4f4 Mon Sep 17 00:00:00 2001 From: Josh Jones Date: Mon, 31 Jul 2023 12:42:43 -0500 Subject: [PATCH 07/33] update `CidVec` (de)serialization Co-authored-by: Aatif Syed --- src/ipld/cid_vec.rs | 44 +++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 41 insertions(+), 3 deletions(-) diff --git a/src/ipld/cid_vec.rs b/src/ipld/cid_vec.rs index 3a009d4797ad..f364cd60f533 100644 --- a/src/ipld/cid_vec.rs +++ b/src/ipld/cid_vec.rs @@ -1,21 +1,43 @@ // Copyright 2019-2023 ChainSafe Systems // SPDX-License-Identifier: Apache-2.0, MIT -use crate::utils::cid::{CidVariant, BLAKE2B256_SIZE}; +use crate::{ + json::cid::vec::CidJsonVec, + utils::cid::{CidVariant, BLAKE2B256_SIZE}, +}; use cid::{ multihash::{self, Code::Blake2b256}, Cid, }; use fvm_ipld_encoding::DAG_CBOR; -use serde::{Deserialize, Serialize}; +use serde::{Deserialize, Deserializer, Serialize, Serializer}; // CidVec takes advantage of the fact that the V1 DAG-CBOR Blake2b-256 variant (which can be stored in 32 bytes vs 96 bytes for a `Cid` type) is +99.99% of all CIDs. CidVec defaults to the `Vec<[u8; BLAKE2B256_SIZE]>` type, only using the more expensive `Vec` type when necessary. -#[derive(Clone, Debug, PartialEq, Eq, Hash, Serialize, Deserialize)] +#[derive(Clone, Debug, PartialEq, Eq, Hash)] pub enum CidVec { V1Cids(Vec<[u8; BLAKE2B256_SIZE]>), AllCids(Vec), } +impl Serialize for CidVec { + fn serialize(&self, serializer: S) -> Result + where + S: Serializer, + { + CidJsonVec(self.cids()).serialize(serializer) + } +} + +impl<'de> Deserialize<'de> for CidVec { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + let items: Vec = CidJsonVec::deserialize(deserializer)?.0; + Ok(items.into()) + } +} + impl Default for CidVec { fn default() -> Self { Self::V1Cids(Vec::new()) @@ -139,4 +161,20 @@ mod test { fn cidvec_to_vec_of_cids_to_cidvec(cidvec: CidVec) { assert_eq!(cidvec, CidVec::from(Vec::::from(cidvec.clone()))); } + + #[quickcheck] + fn serialize_vec_of_cids_deserialize_cidvec(vec_of_cids: Vec) { + let serialized: String = + crate::to_string_with!(&vec_of_cids, crate::json::cid::vec::serialize); + let parsed: CidVec = serde_json::from_str(&serialized).unwrap(); + assert_eq!(vec_of_cids, Vec::::from(parsed)); + } + + #[quickcheck] + fn serialize_cidvec_deserialize_vec_of_cids(cidvec: CidVec) { + let serialized: String = serde_json::to_string(&cidvec).unwrap(); + let parsed: Vec = + crate::from_str_with!(&serialized, crate::json::cid::vec::deserialize); + assert_eq!(Vec::::from(cidvec), parsed); + } } From 02f7ee2bf806d0e01e54271928a592c0994d5bba Mon Sep 17 00:00:00 2001 From: Josh Jones Date: Mon, 31 Jul 2023 12:53:43 -0500 Subject: [PATCH 08/33] fix issues from merge conflict resolution --- src/db/car/forest.rs | 2 +- src/db/car/plain.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/db/car/forest.rs b/src/db/car/forest.rs index 16da732604fb..049057ffd8e0 100644 --- a/src/db/car/forest.rs +++ b/src/db/car/forest.rs @@ -136,7 +136,7 @@ impl ForestCar { } pub fn heaviest_tipset(&self) -> anyhow::Result { - Tipset::load_required(self, &TipsetKeys::new(self.roots())) + Tipset::load_required(self, &TipsetKeys::new(self.roots().into())) } pub fn into_dyn(self) -> ForestCar> { diff --git a/src/db/car/plain.rs b/src/db/car/plain.rs index 59a6a25cfa21..2ac541b27891 100644 --- a/src/db/car/plain.rs +++ b/src/db/car/plain.rs @@ -149,7 +149,7 @@ impl PlainCar { } pub fn heaviest_tipset(&self) -> anyhow::Result { - Tipset::load_required(self, &TipsetKeys::new(self.roots())) + Tipset::load_required(self, &TipsetKeys::new(self.roots().into())) } /// In an arbitrary order From 455625ac884be341c872afea32b7fa517247b7ef Mon Sep 17 00:00:00 2001 From: Josh Jones Date: Tue, 1 Aug 2023 11:18:24 -0500 Subject: [PATCH 09/33] modify (de)serialization --- src/ipld/cid_vec.rs | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/src/ipld/cid_vec.rs b/src/ipld/cid_vec.rs index f364cd60f533..39b465760693 100644 --- a/src/ipld/cid_vec.rs +++ b/src/ipld/cid_vec.rs @@ -1,10 +1,7 @@ // Copyright 2019-2023 ChainSafe Systems // SPDX-License-Identifier: Apache-2.0, MIT -use crate::{ - json::cid::vec::CidJsonVec, - utils::cid::{CidVariant, BLAKE2B256_SIZE}, -}; +use crate::utils::cid::{CidVariant, BLAKE2B256_SIZE}; use cid::{ multihash::{self, Code::Blake2b256}, Cid, @@ -24,7 +21,7 @@ impl Serialize for CidVec { where S: Serializer, { - CidJsonVec(self.cids()).serialize(serializer) + self.cids().serialize(serializer) } } @@ -33,8 +30,7 @@ impl<'de> Deserialize<'de> for CidVec { where D: Deserializer<'de>, { - let items: Vec = CidJsonVec::deserialize(deserializer)?.0; - Ok(items.into()) + Ok(Self::from(>::deserialize(deserializer)?)) } } @@ -164,17 +160,15 @@ mod test { #[quickcheck] fn serialize_vec_of_cids_deserialize_cidvec(vec_of_cids: Vec) { - let serialized: String = - crate::to_string_with!(&vec_of_cids, crate::json::cid::vec::serialize); + let serialized = serde_json::to_string(&vec_of_cids).unwrap(); let parsed: CidVec = serde_json::from_str(&serialized).unwrap(); assert_eq!(vec_of_cids, Vec::::from(parsed)); } #[quickcheck] fn serialize_cidvec_deserialize_vec_of_cids(cidvec: CidVec) { - let serialized: String = serde_json::to_string(&cidvec).unwrap(); - let parsed: Vec = - crate::from_str_with!(&serialized, crate::json::cid::vec::deserialize); + let serialized = serde_json::to_string(&cidvec).unwrap(); + let parsed: Vec = serde_json::from_str(&serialized).unwrap(); assert_eq!(Vec::::from(cidvec), parsed); } } From 0057d8dc924af57de42a2829a1c366ff4fd4e583 Mon Sep 17 00:00:00 2001 From: Josh Jones Date: Tue, 1 Aug 2023 12:12:07 -0500 Subject: [PATCH 10/33] rm unused import --- src/blocks/tipset.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/blocks/tipset.rs b/src/blocks/tipset.rs index 1f0ea0ecc021..4990b239e5fa 100644 --- a/src/blocks/tipset.rs +++ b/src/blocks/tipset.rs @@ -537,7 +537,6 @@ pub mod tipset_json { #[cfg(test)] mod property_tests { - use crate::ipld::CidVec; use quickcheck_macros::quickcheck; use serde_json; From e89e123ef426fa6114c4f84dd49833aef8e5ce66 Mon Sep 17 00:00:00 2001 From: Josh Jones Date: Tue, 1 Aug 2023 15:09:01 -0500 Subject: [PATCH 11/33] add doc comments --- src/ipld/cid_vec.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/ipld/cid_vec.rs b/src/ipld/cid_vec.rs index 39b465760693..d86d350877be 100644 --- a/src/ipld/cid_vec.rs +++ b/src/ipld/cid_vec.rs @@ -74,10 +74,12 @@ impl From for Vec { } impl CidVec { + /// Creates a new empty `V1Cids` variant of `CidVec`. pub fn new() -> Self { Self::default() } + /// Creates a new `CidVec` from a single `Cid`, with the resulting variant of `CidVec` depending on the variant of the `Cid`. pub fn new_from_cid(cid: Cid) -> Self { match cid.try_into() { Ok(CidVariant::V1DagCborBlake2b(bytes)) => Self::V1Cids(vec![bytes]), @@ -85,6 +87,7 @@ impl CidVec { } } + /// Converts a `CidVec` to a `Vec`. pub fn cids(&self) -> Vec { match self { Self::V1Cids(cids) => cids @@ -101,6 +104,7 @@ impl CidVec { } } + /// Adds a CID to the `CidVec`, converting the `CidVec` to the `AllCids` variant if necessary. pub fn push(&mut self, cid: Cid) { match self { Self::V1Cids(cids) => { From 24cf7dbd3521e44d0c2eadcba8c886172a738593 Mon Sep 17 00:00:00 2001 From: Josh Jones Date: Wed, 2 Aug 2023 09:25:01 -0500 Subject: [PATCH 12/33] Apply suggestions from code review Co-authored-by: David Himmelstrup --- src/chain_sync/chain_muxer.rs | 2 +- src/chain_sync/tipset_syncer.rs | 2 +- src/cli/subcommands/chain_cmd.rs | 4 +--- src/db/car/forest.rs | 2 +- src/genesis/mod.rs | 2 +- src/libp2p/chain_exchange/provider.rs | 2 +- src/message_pool/msgpool/test_provider.rs | 2 +- src/rpc/sync_api.rs | 2 +- 8 files changed, 8 insertions(+), 10 deletions(-) diff --git a/src/chain_sync/chain_muxer.rs b/src/chain_sync/chain_muxer.rs index e96755409eb0..fc6e866e379f 100644 --- a/src/chain_sync/chain_muxer.rs +++ b/src/chain_sync/chain_muxer.rs @@ -375,7 +375,7 @@ where metrics::LIBP2P_MESSAGE_TOTAL .with_label_values(&[metrics::values::HELLO_RESPONSE_OUTBOUND]) .inc(); - let tipset_keys = TipsetKeys::new(request.heaviest_tip_set.into()); + let tipset_keys = TipsetKeys::from(request.heaviest_tip_set); let tipset = match Self::get_full_tipset( network.clone(), chain_store.clone(), diff --git a/src/chain_sync/tipset_syncer.rs b/src/chain_sync/tipset_syncer.rs index 03567da12ea8..450cfb85e0b9 100644 --- a/src/chain_sync/tipset_syncer.rs +++ b/src/chain_sync/tipset_syncer.rs @@ -882,7 +882,7 @@ async fn sync_headers_in_reverse { maybe_confirm(*no_confirm, SET_HEAD_CONFIRMATION_MESSAGE)?; chain_set_head( - (TipsetKeys { - cids: cids.clone().into(), - },), + TipsetKeys::from(cids.clone()), &config.client.rpc_token, ) .await diff --git a/src/db/car/forest.rs b/src/db/car/forest.rs index a4aace9017ed..12c6f29fc931 100644 --- a/src/db/car/forest.rs +++ b/src/db/car/forest.rs @@ -136,7 +136,7 @@ impl ForestCar { } pub fn heaviest_tipset(&self) -> anyhow::Result { - Tipset::load_required(self, &TipsetKeys::new(self.roots().into())) + Tipset::load_required(self, &TipsetKeys::from(self.roots())) } pub fn into_dyn(self) -> ForestCar> { diff --git a/src/genesis/mod.rs b/src/genesis/mod.rs index 24f3ce44e82c..46149250fc48 100644 --- a/src/genesis/mod.rs +++ b/src/genesis/mod.rs @@ -121,7 +121,7 @@ where let ts = sm .chain_store() - .tipset_from_keys(&TipsetKeys::new(cids.into()))?; + .tipset_from_keys(&TipsetKeys::from(cids))?; if !skip_load { let gb = sm.chain_store().chain_index.tipset_by_height( diff --git a/src/libp2p/chain_exchange/provider.rs b/src/libp2p/chain_exchange/provider.rs index cbdfe75984a3..62d9a12feaeb 100644 --- a/src/libp2p/chain_exchange/provider.rs +++ b/src/libp2p/chain_exchange/provider.rs @@ -27,7 +27,7 @@ where loop { let mut tipset_bundle: TipsetBundle = TipsetBundle::default(); - let tipset = match cs.tipset_from_keys(&TipsetKeys::new(curr_tipset_cids.into())) { + let tipset = match cs.tipset_from_keys(&TipsetKeys::from(curr_tipset_cids)) { Ok(tipset) => tipset, Err(err) => { debug!("Cannot get tipset from keys: {}", err); diff --git a/src/message_pool/msgpool/test_provider.rs b/src/message_pool/msgpool/test_provider.rs index 63a3fd85856f..884e1b5b0e50 100644 --- a/src/message_pool/msgpool/test_provider.rs +++ b/src/message_pool/msgpool/test_provider.rs @@ -190,7 +190,7 @@ impl Provider for TestApi { fn load_tipset(&self, tsk: &TipsetKeys) -> Result, Error> { let inner = self.inner.lock(); for ts in &inner.tipsets { - if tsk.cids == ts.cids().into() { + if tsk == ts.key() { return Ok(ts.clone().into()); } } diff --git a/src/rpc/sync_api.rs b/src/rpc/sync_api.rs index 3fa06359d322..cbb6df7a94bd 100644 --- a/src/rpc/sync_api.rs +++ b/src/rpc/sync_api.rs @@ -118,7 +118,7 @@ mod tests { let header = from_slice::(&bz).unwrap(); let ts = Tipset::from(header); let db = cs_for_test.blockstore(); - let tsk = ts.key().cids.cids(); + let tsk = ts.key(); cs_for_test.set_heaviest_tipset(Arc::new(ts)).unwrap(); for i in tsk { From e175e1787a8df6c104c0c02025329c94f8beb940 Mon Sep 17 00:00:00 2001 From: Josh Jones Date: Wed, 2 Aug 2023 12:54:42 -0500 Subject: [PATCH 13/33] changes from initial code review Co-authored-by: Aatif Syed --- src/blocks/tipset.rs | 8 +++- src/chain/store/chain_store.rs | 6 +-- src/chain_sync/tipset_syncer.rs | 2 +- src/cli/subcommands/chain_cmd.rs | 9 ++-- src/genesis/mod.rs | 4 +- src/ipld/cid_vec.rs | 75 ++++++++++++++++++++++++++------ src/rpc/sync_api.rs | 6 ++- 7 files changed, 79 insertions(+), 31 deletions(-) diff --git a/src/blocks/tipset.rs b/src/blocks/tipset.rs index 4990b239e5fa..4ba2d794c247 100644 --- a/src/blocks/tipset.rs +++ b/src/blocks/tipset.rs @@ -51,6 +51,12 @@ impl TipsetKeys { } } +impl From> for TipsetKeys { + fn from(cids: Vec) -> Self { + Self::new(CidVec::from(cids)) + } +} + impl fmt::Display for TipsetKeys { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { let s = self @@ -714,7 +720,7 @@ mod test { .unwrap(); let h1 = BlockHeader::builder() .miner_address(Address::new_id(1)) - .parents(TipsetKeys::new(CidVec::new_from_cid(Cid::new_v1( + .parents(TipsetKeys::new(CidVec::from(Cid::new_v1( DAG_CBOR, Identity.digest(&[]), )))) diff --git a/src/chain/store/chain_store.rs b/src/chain/store/chain_store.rs index 56b064cc6112..62eecdbb9fa0 100644 --- a/src/chain/store/chain_store.rs +++ b/src/chain/store/chain_store.rs @@ -117,15 +117,13 @@ where let file_backed_heaviest_tipset_keys = Mutex::new({ let mut head_store = FileBacked::load_from_file_or_create( chain_data_root.join("HEAD"), - || TipsetKeys::new(CidVec::new_from_cid(*genesis_block_header.cid())), + || TipsetKeys::new(CidVec::from(*genesis_block_header.cid())), None, )?; let is_valid = chain_index.load_tipset(head_store.inner()).is_ok(); if !is_valid { // If the stored HEAD is invalid, reset it to the genesis tipset. - head_store.set_inner(TipsetKeys::new(CidVec::new_from_cid( - *genesis_block_header.cid(), - )))?; + head_store.set_inner(TipsetKeys::new(CidVec::from(*genesis_block_header.cid())))?; } head_store }); diff --git a/src/chain_sync/tipset_syncer.rs b/src/chain_sync/tipset_syncer.rs index 450cfb85e0b9..342faffb02e8 100644 --- a/src/chain_sync/tipset_syncer.rs +++ b/src/chain_sync/tipset_syncer.rs @@ -882,7 +882,7 @@ async fn sync_headers_in_reverse { maybe_confirm(*no_confirm, SET_HEAD_CONFIRMATION_MESSAGE)?; - chain_set_head( - TipsetKeys::from(cids.clone()), - &config.client.rpc_token, - ) - .await - .map_err(handle_rpc_err) + chain_set_head((TipsetKeys::from(cids.clone()),), &config.client.rpc_token) + .await + .map_err(handle_rpc_err) } } } diff --git a/src/genesis/mod.rs b/src/genesis/mod.rs index 46149250fc48..880f0d2b9659 100644 --- a/src/genesis/mod.rs +++ b/src/genesis/mod.rs @@ -119,9 +119,7 @@ where meta.sync()?; } - let ts = sm - .chain_store() - .tipset_from_keys(&TipsetKeys::from(cids))?; + let ts = sm.chain_store().tipset_from_keys(&TipsetKeys::from(cids))?; if !skip_load { let gb = sm.chain_store().chain_index.tipset_by_height( diff --git a/src/ipld/cid_vec.rs b/src/ipld/cid_vec.rs index d86d350877be..67ee73292f08 100644 --- a/src/ipld/cid_vec.rs +++ b/src/ipld/cid_vec.rs @@ -12,7 +12,7 @@ use serde::{Deserialize, Deserializer, Serialize, Serializer}; // CidVec takes advantage of the fact that the V1 DAG-CBOR Blake2b-256 variant (which can be stored in 32 bytes vs 96 bytes for a `Cid` type) is +99.99% of all CIDs. CidVec defaults to the `Vec<[u8; BLAKE2B256_SIZE]>` type, only using the more expensive `Vec` type when necessary. #[derive(Clone, Debug, PartialEq, Eq, Hash)] pub enum CidVec { - V1Cids(Vec<[u8; BLAKE2B256_SIZE]>), + V1DagCborBlake2bCids(Vec<[u8; BLAKE2B256_SIZE]>), AllCids(Vec), } @@ -36,7 +36,53 @@ impl<'de> Deserialize<'de> for CidVec { impl Default for CidVec { fn default() -> Self { - Self::V1Cids(Vec::new()) + Self::V1DagCborBlake2bCids(Vec::new()) + } +} + +pub struct CidVecIterator<'a> { + buffer: &'a CidVec, + current_ix: usize, +} + +impl Iterator for CidVecIterator<'_> { + type Item = Cid; + fn next(&mut self) -> Option { + match self.buffer { + CidVec::V1DagCborBlake2bCids(cids) => { + if self.current_ix < cids.len() { + let cid = Cid::new_v1( + DAG_CBOR, + multihash::Multihash::wrap(Blake2b256.into(), &cids[self.current_ix]) + .expect("failed to convert digest to CID"), + ); + self.current_ix += 1; + Some(cid) + } else { + None + } + } + CidVec::AllCids(cids) => { + if self.current_ix < cids.len() { + let cid = cids[self.current_ix]; + self.current_ix += 1; + Some(cid) + } else { + None + } + } + } + } +} + +impl<'a> IntoIterator for &'a CidVec { + type Item = Cid; + type IntoIter = CidVecIterator<'a>; + fn into_iter(self) -> Self::IntoIter { + CidVecIterator { + buffer: self, + current_ix: 0, + } } } @@ -50,9 +96,18 @@ impl FromIterator for CidVec { } } +impl From for CidVec { + fn from(cid: Cid) -> Self { + match cid.try_into() { + Ok(CidVariant::V1DagCborBlake2b(bytes)) => Self::V1DagCborBlake2bCids(vec![bytes]), + _ => Self::AllCids(vec![cid]), + } + } +} + impl From> for CidVec { fn from(vec: Vec) -> Self { - // Converts `Vec` to `CidVec::V1Cids` if possible; otherwise, converts to `CidVec::AllCids`. + // Converts `Vec` to `CidVec::V1DagCborBlake2bCids` if possible; otherwise, converts to `CidVec::AllCids`. let mut cid_vec = CidVec::new(); for cid in vec { cid_vec.push(cid); @@ -74,23 +129,15 @@ impl From for Vec { } impl CidVec { - /// Creates a new empty `V1Cids` variant of `CidVec`. + /// Creates a new empty `V1DagCborBlake2bCids` variant of `CidVec`. pub fn new() -> Self { Self::default() } - /// Creates a new `CidVec` from a single `Cid`, with the resulting variant of `CidVec` depending on the variant of the `Cid`. - pub fn new_from_cid(cid: Cid) -> Self { - match cid.try_into() { - Ok(CidVariant::V1DagCborBlake2b(bytes)) => Self::V1Cids(vec![bytes]), - _ => Self::AllCids(vec![cid]), - } - } - /// Converts a `CidVec` to a `Vec`. pub fn cids(&self) -> Vec { match self { - Self::V1Cids(cids) => cids + Self::V1DagCborBlake2bCids(cids) => cids .iter() .map(|c| { Cid::new_v1( @@ -107,7 +154,7 @@ impl CidVec { /// Adds a CID to the `CidVec`, converting the `CidVec` to the `AllCids` variant if necessary. pub fn push(&mut self, cid: Cid) { match self { - Self::V1Cids(cids) => { + Self::V1DagCborBlake2bCids(cids) => { if let Ok(CidVariant::V1DagCborBlake2b(bytes)) = cid.try_into() { cids.push(bytes); } else { diff --git a/src/rpc/sync_api.rs b/src/rpc/sync_api.rs index cbb6df7a94bd..a46c31f65a99 100644 --- a/src/rpc/sync_api.rs +++ b/src/rpc/sync_api.rs @@ -119,9 +119,11 @@ mod tests { let ts = Tipset::from(header); let db = cs_for_test.blockstore(); let tsk = ts.key(); - cs_for_test.set_heaviest_tipset(Arc::new(ts)).unwrap(); + cs_for_test + .set_heaviest_tipset(Arc::new(ts.clone())) + .unwrap(); - for i in tsk { + for i in tsk.cids.into_iter() { let bz2 = bz.clone(); db.put_keyed(&i, &bz2).unwrap(); } From fe015b5dc8de43a7debdcab5a314e83fca68d2ae Mon Sep 17 00:00:00 2001 From: Josh Jones Date: Wed, 2 Aug 2023 17:56:50 -0500 Subject: [PATCH 14/33] rm `cids()` method --- src/blocks/tipset.rs | 15 +++---- src/chain/mod.rs | 3 +- src/chain/store/chain_store.rs | 2 +- src/chain_sync/chain_muxer.rs | 2 +- src/chain_sync/network_context.rs | 2 +- src/chain_sync/tipset_syncer.rs | 2 +- src/interpreter/fvm2.rs | 4 +- src/interpreter/fvm3.rs | 4 +- src/ipld/cid_vec.rs | 59 +++++++++++++++++++++------ src/ipld/util.rs | 8 ++-- src/libp2p/chain_exchange/provider.rs | 2 +- src/rpc/chain_api.rs | 2 +- 12 files changed, 67 insertions(+), 38 deletions(-) diff --git a/src/blocks/tipset.rs b/src/blocks/tipset.rs index 4ba2d794c247..05ee93df7060 100644 --- a/src/blocks/tipset.rs +++ b/src/blocks/tipset.rs @@ -35,16 +35,11 @@ impl TipsetKeys { Self { cids } } - /// Returns tipset header `cids` - pub fn cids(&self) -> Vec { - self.cids.cids() - } - // Special encoding to match Lotus. pub fn cid(&self) -> anyhow::Result { use fvm_ipld_encoding::RawBytes; let mut bytes = Vec::new(); - for cid in self.cids() { + for cid in Vec::::from(&self.cids) { bytes.append(&mut cid.to_bytes()) } Ok(Cid::from_cbor_blake2b256(&RawBytes::new(bytes))?) @@ -60,7 +55,7 @@ impl From> for TipsetKeys { impl fmt::Display for TipsetKeys { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { let s = self - .cids() + .cids .into_iter() .map(|cid| cid.to_string()) .collect::>() @@ -148,7 +143,7 @@ impl Tipset { /// present but invalid. If the tipset is missing, None is returned. pub fn load(store: impl Blockstore, tsk: &TipsetKeys) -> anyhow::Result> { Ok(tsk - .cids() + .cids .into_iter() .map(|key| BlockHeader::load(&store, key)) .collect::>>()? @@ -228,7 +223,7 @@ impl Tipset { } /// Returns slice of `CIDs` for the current tipset pub fn cids(&self) -> Vec { - self.key().cids.cids() + Vec::::from(&self.key().cids) } /// Returns the keys of the parents of the blocks in the tipset. pub fn parents(&self) -> &TipsetKeys { @@ -454,7 +449,7 @@ pub mod tipset_keys_json { where S: Serializer, { - crate::json::cid::vec::serialize(&m.cids(), serializer) + crate::json::cid::vec::serialize(&Vec::::from(&m.cids), serializer) } pub fn deserialize<'de, D>(deserializer: D) -> Result diff --git a/src/chain/mod.rs b/src/chain/mod.rs index 00309359d8d3..fbaf0222020a 100644 --- a/src/chain/mod.rs +++ b/src/chain/mod.rs @@ -7,6 +7,7 @@ use crate::db::car::forest; use crate::ipld::{stream_chain, CidHashSet}; use crate::utils::io::{AsyncWriterWithChecksum, Checksum}; use anyhow::{Context, Result}; +use cid::Cid; use digest::Digest; use fvm_ipld_blockstore::Blockstore; use tokio::io::{AsyncWrite, AsyncWriteExt, BufWriter}; @@ -22,7 +23,7 @@ pub async fn export( skip_checksum: bool, ) -> Result>, Error> { let stateroot_lookup_limit = tipset.epoch() - lookup_depth; - let roots = tipset.key().cids().to_vec(); + let roots = Vec::::from(&tipset.key().cids); // Wrap writer in optional checksum calculator let mut writer = AsyncWriterWithChecksum::::new(BufWriter::new(writer), !skip_checksum); diff --git a/src/chain/store/chain_store.rs b/src/chain/store/chain_store.rs index 62eecdbb9fa0..8855725c1ed2 100644 --- a/src/chain/store/chain_store.rs +++ b/src/chain/store/chain_store.rs @@ -218,7 +218,7 @@ where /// Returns Tipset from key-value store from provided CIDs #[tracing::instrument(skip_all)] pub fn tipset_from_keys(&self, tsk: &TipsetKeys) -> Result, Error> { - if tsk.cids().is_empty() { + if tsk.cids.is_empty() { return Ok(self.heaviest_tipset()); } self.chain_index.load_tipset(tsk) diff --git a/src/chain_sync/chain_muxer.rs b/src/chain_sync/chain_muxer.rs index fc6e866e379f..41a0c3d95a95 100644 --- a/src/chain_sync/chain_muxer.rs +++ b/src/chain_sync/chain_muxer.rs @@ -262,7 +262,7 @@ where if network.peer_manager().is_peer_new(&peer_id).await { // Since the peer is new, send them a hello request let request = HelloRequest { - heaviest_tip_set: heaviest.cids().to_vec(), + heaviest_tip_set: heaviest.cids(), heaviest_tipset_height: heaviest.epoch(), heaviest_tipset_weight: heaviest.weight().clone().into(), genesis_cid: genesis_block_cid, diff --git a/src/chain_sync/network_context.rs b/src/chain_sync/network_context.rs index e0a61862a5e6..32c45fe8a8aa 100644 --- a/src/chain_sync/network_context.rs +++ b/src/chain_sync/network_context.rs @@ -225,7 +225,7 @@ where T: TryFrom + Send + Sync + 'static, { let request = ChainExchangeRequest { - start: tsk.cids().to_vec(), + start: Vec::::from(&tsk.cids), request_len, options, }; diff --git a/src/chain_sync/tipset_syncer.rs b/src/chain_sync/tipset_syncer.rs index 342faffb02e8..5c629b789024 100644 --- a/src/chain_sync/tipset_syncer.rs +++ b/src/chain_sync/tipset_syncer.rs @@ -1593,7 +1593,7 @@ fn validate_tipset_against_cache( tipset: &TipsetKeys, descendant_blocks: &[Cid], ) -> Result<(), TipsetRangeSyncerError> { - for cid in tipset.cids() { + for cid in &tipset.cids { if let Some(reason) = bad_block_cache.get(&cid) { for block_cid in descendant_blocks { bad_block_cache.put(*block_cid, format!("chain contained {cid}")); diff --git a/src/interpreter/fvm2.rs b/src/interpreter/fvm2.rs index 2f269cf0f6da..4c3b388a61e5 100644 --- a/src/interpreter/fvm2.rs +++ b/src/interpreter/fvm2.rs @@ -221,8 +221,8 @@ impl Consensus for ForestExternsV2 { let bh_3 = from_slice::(extra)?; if bh_1.parents() == bh_3.parents() && bh_1.epoch() == bh_3.epoch() - && bh_2.parents().cids().contains(bh_3.cid()) - && !bh_2.parents().cids().contains(bh_1.cid()) + && bh_2.parents().cids.contains(*bh_3.cid()) + && !bh_2.parents().cids.contains(*bh_1.cid()) { fault_type = Some(ConsensusFaultType::ParentGrinding); } diff --git a/src/interpreter/fvm3.rs b/src/interpreter/fvm3.rs index b4122344ba4a..b01568f817e2 100644 --- a/src/interpreter/fvm3.rs +++ b/src/interpreter/fvm3.rs @@ -242,8 +242,8 @@ impl Consensus for ForestExterns { let bh_3 = from_slice::(extra)?; if bh_1.parents() == bh_3.parents() && bh_1.epoch() == bh_3.epoch() - && bh_2.parents().cids().contains(bh_3.cid()) - && !bh_2.parents().cids().contains(bh_1.cid()) + && bh_2.parents().cids.contains(*bh_3.cid()) + && !bh_2.parents().cids.contains(*bh_1.cid()) { fault_type = Some(ConsensusFaultType::ParentGrinding); } diff --git a/src/ipld/cid_vec.rs b/src/ipld/cid_vec.rs index 67ee73292f08..4470adb2e955 100644 --- a/src/ipld/cid_vec.rs +++ b/src/ipld/cid_vec.rs @@ -21,7 +21,7 @@ impl Serialize for CidVec { where S: Serializer, { - self.cids().serialize(serializer) + Vec::::from(self).serialize(serializer) } } @@ -124,20 +124,26 @@ impl From<&[Cid]> for CidVec { impl From for Vec { fn from(vec: CidVec) -> Self { - vec.cids() + match vec { + CidVec::V1DagCborBlake2bCids(cids) => cids + .iter() + .map(|c| { + Cid::new_v1( + DAG_CBOR, + multihash::Multihash::wrap(Blake2b256.into(), c) + .expect("failed to convert digest to CID"), + ) + }) + .collect(), + CidVec::AllCids(cids) => cids, + } } } -impl CidVec { - /// Creates a new empty `V1DagCborBlake2bCids` variant of `CidVec`. - pub fn new() -> Self { - Self::default() - } - - /// Converts a `CidVec` to a `Vec`. - pub fn cids(&self) -> Vec { - match self { - Self::V1DagCborBlake2bCids(cids) => cids +impl From<&CidVec> for Vec { + fn from(vec: &CidVec) -> Self { + match vec { + CidVec::V1DagCborBlake2bCids(cids) => cids .iter() .map(|c| { Cid::new_v1( @@ -147,9 +153,16 @@ impl CidVec { ) }) .collect(), - Self::AllCids(cids) => cids.clone(), + CidVec::AllCids(cids) => cids.clone(), } } +} + +impl CidVec { + /// Creates a new empty `V1DagCborBlake2bCids` variant of `CidVec`. + pub fn new() -> Self { + Self::default() + } /// Adds a CID to the `CidVec`, converting the `CidVec` to the `AllCids` variant if necessary. pub fn push(&mut self, cid: Cid) { @@ -175,6 +188,26 @@ impl CidVec { Self::AllCids(cids) => cids.push(cid), } } + + pub fn is_empty(&self) -> bool { + match self { + Self::V1DagCborBlake2bCids(cids) => cids.is_empty(), + Self::AllCids(cids) => cids.is_empty(), + } + } + + pub fn contains(&self, cid: Cid) -> bool { + match self { + Self::V1DagCborBlake2bCids(cids) => { + if let Ok(CidVariant::V1DagCborBlake2b(bytes)) = cid.try_into() { + cids.contains(&bytes) + } else { + false + } + } + Self::AllCids(cids) => cids.contains(&cid), + } + } } #[cfg(test)] diff --git a/src/ipld/util.rs b/src/ipld/util.rs index d1aad1648d5c..7143ecc99c78 100644 --- a/src/ipld/util.rs +++ b/src/ipld/util.rs @@ -127,7 +127,7 @@ where let wp = WithProgressRaw::new(message, estimated_total_records); let mut seen = CidHashSet::default(); - let mut blocks_to_walk: VecDeque = tipset.cids().to_vec().into(); + let mut blocks_to_walk: VecDeque = tipset.cids().into(); let mut current_min_height = tipset.epoch(); let incl_roots_epoch = tipset.epoch() - recent_roots; @@ -170,11 +170,11 @@ where } if h.epoch() > 0 { - for p in h.parents().cids() { + for p in &h.parents().cids { blocks_to_walk.push_back(p); } } else { - for p in h.parents().cids() { + for p in &h.parents().cids { load_block(p).await?; } } @@ -392,7 +392,7 @@ impl + Unpin> Stream for ChainStream< if block.epoch() == 0 { // The genesis block has some kind of dummy parent that needs to be emitted. - for p in block.parents().cids() { + for p in &block.parents().cids { this.dfs.push_back(Emit(p)); } } diff --git a/src/libp2p/chain_exchange/provider.rs b/src/libp2p/chain_exchange/provider.rs index 62d9a12feaeb..1c9ec22ee6d6 100644 --- a/src/libp2p/chain_exchange/provider.rs +++ b/src/libp2p/chain_exchange/provider.rs @@ -55,7 +55,7 @@ where } } - curr_tipset_cids = tipset.parents().cids().to_vec(); + curr_tipset_cids = Vec::::from(&tipset.parents().cids); let tipset_epoch = tipset.epoch(); if request.include_blocks() { diff --git a/src/rpc/chain_api.rs b/src/rpc/chain_api.rs index 6194cd5e6369..d150a2853628 100644 --- a/src/rpc/chain_api.rs +++ b/src/rpc/chain_api.rs @@ -256,7 +256,7 @@ where let new_head = data.state_manager.chain_store().tipset_from_keys(¶ms)?; let mut current = data.state_manager.chain_store().heaviest_tipset(); while current.epoch() >= new_head.epoch() { - for cid in current.key().cids() { + for cid in ¤t.key().cids { data.state_manager .chain_store() .unmark_block_as_validated(cid); From 5d6151d89bc5a8bde76ffadee14f5aa07398a36d Mon Sep 17 00:00:00 2001 From: Josh Jones Date: Thu, 3 Aug 2023 15:45:58 -0500 Subject: [PATCH 15/33] replace `CidVec` with `FrozenCids` --- src/blocks/tipset.rs | 14 +- src/chain/store/chain_store.rs | 8 +- src/ipld/{cid_vec.rs => frozen_cids.rs} | 225 +++++++++++------------- src/ipld/mod.rs | 4 +- 4 files changed, 118 insertions(+), 133 deletions(-) rename src/ipld/{cid_vec.rs => frozen_cids.rs} (52%) diff --git a/src/blocks/tipset.rs b/src/blocks/tipset.rs index 05ee93df7060..4cb924b60486 100644 --- a/src/blocks/tipset.rs +++ b/src/blocks/tipset.rs @@ -3,7 +3,7 @@ use std::{fmt, sync::OnceLock}; -use crate::ipld::CidVec; +use crate::ipld::FrozenCids; use crate::networks::{calibnet, mainnet}; use crate::shim::{address::Address, clock::ChainEpoch}; use crate::utils::cid::CidCborExt; @@ -27,11 +27,11 @@ use super::{Block, BlockHeader, Error, Ticket}; #[cfg_attr(test, derive(derive_quickcheck_arbitrary::Arbitrary))] #[serde(transparent)] pub struct TipsetKeys { - pub cids: CidVec, + pub cids: FrozenCids, } impl TipsetKeys { - pub fn new(cids: CidVec) -> Self { + pub fn new(cids: FrozenCids) -> Self { Self { cids } } @@ -48,7 +48,7 @@ impl TipsetKeys { impl From> for TipsetKeys { fn from(cids: Vec) -> Self { - Self::new(CidVec::from(cids)) + Self::new(FrozenCids::from(cids)) } } @@ -564,7 +564,7 @@ mod property_tests { #[cfg(test)] mod test { - use crate::ipld::CidVec; + use crate::ipld::FrozenCids; use crate::json::vrf::VRFProof; use crate::shim::address::Address; use cid::{ @@ -715,10 +715,10 @@ mod test { .unwrap(); let h1 = BlockHeader::builder() .miner_address(Address::new_id(1)) - .parents(TipsetKeys::new(CidVec::from(Cid::new_v1( + .parents(TipsetKeys::new(FrozenCids::from_iter([Cid::new_v1( DAG_CBOR, Identity.digest(&[]), - )))) + )]))) .build() .unwrap(); assert_eq!( diff --git a/src/chain/store/chain_store.rs b/src/chain/store/chain_store.rs index 8855725c1ed2..1de66a7a2175 100644 --- a/src/chain/store/chain_store.rs +++ b/src/chain/store/chain_store.rs @@ -5,7 +5,7 @@ use std::{path::Path, sync::Arc}; use crate::blocks::{BlockHeader, Tipset, TipsetKeys, TxMeta}; use crate::interpreter::BlockMessages; -use crate::ipld::CidVec; +use crate::ipld::FrozenCids; use crate::libp2p_bitswap::{BitswapStoreRead, BitswapStoreReadWrite}; use crate::message::{ChainMessage, Message as MessageTrait, SignedMessage}; use crate::networks::ChainConfig; @@ -117,13 +117,15 @@ where let file_backed_heaviest_tipset_keys = Mutex::new({ let mut head_store = FileBacked::load_from_file_or_create( chain_data_root.join("HEAD"), - || TipsetKeys::new(CidVec::from(*genesis_block_header.cid())), + || TipsetKeys::new(FrozenCids::from_iter([*genesis_block_header.cid()])), None, )?; let is_valid = chain_index.load_tipset(head_store.inner()).is_ok(); if !is_valid { // If the stored HEAD is invalid, reset it to the genesis tipset. - head_store.set_inner(TipsetKeys::new(CidVec::from(*genesis_block_header.cid())))?; + head_store.set_inner(TipsetKeys::new(FrozenCids::from_iter([ + *genesis_block_header.cid(), + ])))?; } head_store }); diff --git a/src/ipld/cid_vec.rs b/src/ipld/frozen_cids.rs similarity index 52% rename from src/ipld/cid_vec.rs rename to src/ipld/frozen_cids.rs index 4470adb2e955..b144b38de28d 100644 --- a/src/ipld/cid_vec.rs +++ b/src/ipld/frozen_cids.rs @@ -9,48 +9,45 @@ use cid::{ use fvm_ipld_encoding::DAG_CBOR; use serde::{Deserialize, Deserializer, Serialize, Serializer}; -// CidVec takes advantage of the fact that the V1 DAG-CBOR Blake2b-256 variant (which can be stored in 32 bytes vs 96 bytes for a `Cid` type) is +99.99% of all CIDs. CidVec defaults to the `Vec<[u8; BLAKE2B256_SIZE]>` type, only using the more expensive `Vec` type when necessary. #[derive(Clone, Debug, PartialEq, Eq, Hash)] -pub enum CidVec { - V1DagCborBlake2bCids(Vec<[u8; BLAKE2B256_SIZE]>), - AllCids(Vec), -} +pub struct FrozenCids(CidBox); -impl Serialize for CidVec { - fn serialize(&self, serializer: S) -> Result - where - S: Serializer, - { - Vec::::from(self).serialize(serializer) +impl Default for FrozenCids { + fn default() -> Self { + Self(CidBox::V1DagCborBlake2bCids(Box::new([]))) } } -impl<'de> Deserialize<'de> for CidVec { - fn deserialize(deserializer: D) -> Result - where - D: Deserializer<'de>, - { - Ok(Self::from(>::deserialize(deserializer)?)) - } +#[derive(Clone, Debug, PartialEq, Eq, Hash)] +enum CidBox { + V1DagCborBlake2bCids(Box<[[u8; BLAKE2B256_SIZE]]>), + AllCids(Box<[Cid]>), } -impl Default for CidVec { - fn default() -> Self { - Self::V1DagCborBlake2bCids(Vec::new()) - } +pub struct FrozenCidsIterator<'a> { + buffer: &'a FrozenCids, + current_ix: usize, } -pub struct CidVecIterator<'a> { - buffer: &'a CidVec, - current_ix: usize, +impl<'a> IntoIterator for &'a FrozenCids { + type Item = Cid; + type IntoIter = FrozenCidsIterator<'a>; + fn into_iter(self) -> Self::IntoIter { + FrozenCidsIterator { + buffer: self, + current_ix: 0, + } + } } -impl Iterator for CidVecIterator<'_> { +impl Iterator for FrozenCidsIterator<'_> { type Item = Cid; fn next(&mut self) -> Option { - match self.buffer { - CidVec::V1DagCborBlake2bCids(cids) => { - if self.current_ix < cids.len() { + match &self.buffer.0 { + CidBox::V1DagCborBlake2bCids(cids) => { + if self.current_ix >= cids.len() { + None + } else { let cid = Cid::new_v1( DAG_CBOR, multihash::Multihash::wrap(Blake2b256.into(), &cids[self.current_ix]) @@ -58,154 +55,140 @@ impl Iterator for CidVecIterator<'_> { ); self.current_ix += 1; Some(cid) - } else { - None } } - CidVec::AllCids(cids) => { - if self.current_ix < cids.len() { + CidBox::AllCids(cids) => { + if self.current_ix >= cids.len() { + None + } else { let cid = cids[self.current_ix]; self.current_ix += 1; Some(cid) - } else { - None } } } } } -impl<'a> IntoIterator for &'a CidVec { - type Item = Cid; - type IntoIter = CidVecIterator<'a>; - fn into_iter(self) -> Self::IntoIter { - CidVecIterator { - buffer: self, - current_ix: 0, - } +impl Serialize for FrozenCids { + fn serialize(&self, serializer: S) -> Result + where + S: Serializer, + { + Vec::::from(self).serialize(serializer) + } +} + +impl<'de> Deserialize<'de> for FrozenCids { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + Ok(Self::from(>::deserialize(deserializer)?)) } } -impl FromIterator for CidVec { +impl FromIterator for FrozenCids { fn from_iter>(iter: T) -> Self { - let mut vec = Self::new(); + let mut vec = Vec::new(); for i in iter { vec.push(i); } - vec + FrozenCids::from(vec) } } -impl From for CidVec { - fn from(cid: Cid) -> Self { - match cid.try_into() { - Ok(CidVariant::V1DagCborBlake2b(bytes)) => Self::V1DagCborBlake2bCids(vec![bytes]), - _ => Self::AllCids(vec![cid]), +impl From> for FrozenCids { + fn from(cids: Vec) -> Self { + let mut v1dagcborblake2bcids = Vec::new(); + let mut allcids = Vec::new(); + for cid in cids { + match cid.try_into() { + Ok(CidVariant::V1DagCborBlake2b(bytes)) => { + v1dagcborblake2bcids.push(bytes); + } + _ => { + allcids.push(cid); + } + } } - } -} - -impl From> for CidVec { - fn from(vec: Vec) -> Self { - // Converts `Vec` to `CidVec::V1DagCborBlake2bCids` if possible; otherwise, converts to `CidVec::AllCids`. - let mut cid_vec = CidVec::new(); - for cid in vec { - cid_vec.push(cid); + if allcids.is_empty() { + FrozenCids(CidBox::V1DagCborBlake2bCids( + v1dagcborblake2bcids.into_boxed_slice(), + )) + } else { + allcids.extend(v1dagcborblake2bcids.into_iter().map(|bytes| { + Cid::new_v1( + DAG_CBOR, + multihash::Multihash::wrap(Blake2b256.into(), &bytes) + .expect("failed to convert digest to CID"), + ) + })); + FrozenCids(CidBox::AllCids(allcids.into_boxed_slice())) } - cid_vec - } -} - -impl From<&[Cid]> for CidVec { - fn from(vec: &[Cid]) -> Self { - vec.iter().cloned().collect() } } -impl From for Vec { - fn from(vec: CidVec) -> Self { - match vec { - CidVec::V1DagCborBlake2bCids(cids) => cids +impl From for Vec { + fn from(frozen_cids: FrozenCids) -> Self { + match frozen_cids.0 { + CidBox::V1DagCborBlake2bCids(cids) => cids .iter() - .map(|c| { + .map(|bytes| { Cid::new_v1( DAG_CBOR, - multihash::Multihash::wrap(Blake2b256.into(), c) + multihash::Multihash::wrap(Blake2b256.into(), bytes) .expect("failed to convert digest to CID"), ) }) .collect(), - CidVec::AllCids(cids) => cids, + CidBox::AllCids(cids) => cids.to_vec(), } } } -impl From<&CidVec> for Vec { - fn from(vec: &CidVec) -> Self { - match vec { - CidVec::V1DagCborBlake2bCids(cids) => cids +impl From<&FrozenCids> for Vec { + fn from(frozen_cids: &FrozenCids) -> Self { + match &frozen_cids.0 { + CidBox::V1DagCborBlake2bCids(cids) => cids .iter() - .map(|c| { + .map(|bytes| { Cid::new_v1( DAG_CBOR, - multihash::Multihash::wrap(Blake2b256.into(), c) + multihash::Multihash::wrap(Blake2b256.into(), bytes) .expect("failed to convert digest to CID"), ) }) .collect(), - CidVec::AllCids(cids) => cids.clone(), + CidBox::AllCids(cids) => cids.to_vec(), } } } -impl CidVec { - /// Creates a new empty `V1DagCborBlake2bCids` variant of `CidVec`. - pub fn new() -> Self { - Self::default() - } - - /// Adds a CID to the `CidVec`, converting the `CidVec` to the `AllCids` variant if necessary. - pub fn push(&mut self, cid: Cid) { - match self { - Self::V1DagCborBlake2bCids(cids) => { - if let Ok(CidVariant::V1DagCborBlake2b(bytes)) = cid.try_into() { - cids.push(bytes); - } else { - let mut cids: Vec = std::mem::take(cids) - .into_iter() - .map(|c| { - Cid::new_v1( - DAG_CBOR, - multihash::Multihash::wrap(Blake2b256.into(), &c) - .expect("failed to convert digest to CID"), - ) - }) - .collect(); - cids.push(cid); - *self = Self::AllCids(cids); - } - } - Self::AllCids(cids) => cids.push(cid), - } +impl FrozenCids { + pub fn push(&self, cid: Cid) -> Self { + let mut cids = Vec::::from(self); + cids.push(cid); + FrozenCids::from(cids) } pub fn is_empty(&self) -> bool { - match self { - Self::V1DagCborBlake2bCids(cids) => cids.is_empty(), - Self::AllCids(cids) => cids.is_empty(), + match &self.0 { + CidBox::V1DagCborBlake2bCids(cids) => cids.is_empty(), + CidBox::AllCids(cids) => cids.is_empty(), } } pub fn contains(&self, cid: Cid) -> bool { - match self { - Self::V1DagCborBlake2bCids(cids) => { + match &self.0 { + CidBox::V1DagCborBlake2bCids(cids) => { if let Ok(CidVariant::V1DagCborBlake2b(bytes)) = cid.try_into() { cids.contains(&bytes) } else { false } } - Self::AllCids(cids) => cids.contains(&cid), + CidBox::AllCids(cids) => cids.contains(&cid), } } } @@ -217,7 +200,7 @@ mod test { use quickcheck::Arbitrary; use quickcheck_macros::quickcheck; - impl Arbitrary for CidVec { + impl Arbitrary for FrozenCids { fn arbitrary(g: &mut quickcheck::Gen) -> Self { // Although the vast majority of CIDs are V1DagCborBlake2b, we want to generate the variants of CidVec with equal probability. if bool::arbitrary(g) { @@ -238,19 +221,19 @@ mod test { } #[quickcheck] - fn cidvec_to_vec_of_cids_to_cidvec(cidvec: CidVec) { - assert_eq!(cidvec, CidVec::from(Vec::::from(cidvec.clone()))); + fn cidvec_to_vec_of_cids_to_cidvec(cidvec: FrozenCids) { + assert_eq!(cidvec, FrozenCids::from(Vec::::from(cidvec.clone()))); } #[quickcheck] fn serialize_vec_of_cids_deserialize_cidvec(vec_of_cids: Vec) { let serialized = serde_json::to_string(&vec_of_cids).unwrap(); - let parsed: CidVec = serde_json::from_str(&serialized).unwrap(); + let parsed: FrozenCids = serde_json::from_str(&serialized).unwrap(); assert_eq!(vec_of_cids, Vec::::from(parsed)); } #[quickcheck] - fn serialize_cidvec_deserialize_vec_of_cids(cidvec: CidVec) { + fn serialize_cidvec_deserialize_vec_of_cids(cidvec: FrozenCids) { let serialized = serde_json::to_string(&cidvec).unwrap(); let parsed: Vec = serde_json::from_str(&serialized).unwrap(); assert_eq!(Vec::::from(cidvec), parsed); diff --git a/src/ipld/mod.rs b/src/ipld/mod.rs index 5642923f6071..a587751e76f4 100644 --- a/src/ipld/mod.rs +++ b/src/ipld/mod.rs @@ -3,7 +3,7 @@ mod cid_hashmap; mod cid_hashset; -mod cid_vec; +mod frozen_cids; pub mod json; pub mod selector; pub mod util; @@ -14,7 +14,7 @@ pub use util::*; pub use self::cid_hashmap::CidHashMap; pub use self::cid_hashset::CidHashSet; -pub use self::cid_vec::CidVec; +pub use self::frozen_cids::FrozenCids; pub use libipld_core::serde::{from_ipld, to_ipld}; #[cfg(test)] From bf558614c9aa03a3c21eaf494151ab6d5c89fcad Mon Sep 17 00:00:00 2001 From: Josh Jones Date: Thu, 3 Aug 2023 15:59:03 -0500 Subject: [PATCH 16/33] resolve issue in new `benchmark_cmd` --- src/tool/subcommands/benchmark_cmd.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/tool/subcommands/benchmark_cmd.rs b/src/tool/subcommands/benchmark_cmd.rs index 4213fd56b38c..f0f77bfa6bb0 100644 --- a/src/tool/subcommands/benchmark_cmd.rs +++ b/src/tool/subcommands/benchmark_cmd.rs @@ -10,6 +10,7 @@ use crate::ipld::{stream_chain, stream_graph}; use crate::shim::clock::ChainEpoch; use crate::utils::db::car_stream::CarStream; use anyhow::{Context as _, Result}; +use cid::Cid; use clap::Subcommand; use futures::{StreamExt, TryStreamExt}; use indicatif::{ProgressBar, ProgressStyle}; @@ -182,7 +183,12 @@ async fn benchmark_exporting( compression_level, blocks.map_err(anyhow::Error::from), ); - crate::db::car::forest::Encoder::write(&mut dest, ts.key().cids.clone(), frames).await?; + crate::db::car::forest::Encoder::write( + &mut dest, + Vec::::from(ts.key().cids.clone()), + frames, + ) + .await?; dest.flush().await?; Ok(()) } From e825824a19cf8c7a4a7d13c7d548c505ac49c9ed Mon Sep 17 00:00:00 2001 From: Josh Jones Date: Thu, 3 Aug 2023 18:09:06 -0500 Subject: [PATCH 17/33] update code documentation --- src/ipld/frozen_cids.rs | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/ipld/frozen_cids.rs b/src/ipld/frozen_cids.rs index b144b38de28d..d327596ec5f9 100644 --- a/src/ipld/frozen_cids.rs +++ b/src/ipld/frozen_cids.rs @@ -9,6 +9,11 @@ use cid::{ use fvm_ipld_encoding::DAG_CBOR; use serde::{Deserialize, Deserializer, Serialize, Serializer}; +// `FrozenCids` takes advantage of the fact that the V1 DAG-CBOR Blake2b-256 variant +// (which can be stored in 32 bytes vs 96 bytes for a `Cid` type) is +99.99% of +// all CIDs. `FrozenCids` defaults to the `Box<[u8; BLAKE2B256_SIZE]>` variant of +// `CidBox`, only using the more expensive `Box` variant when necessary. +// The Box type has been chosen to make `FrozenCids` explicitly immutable. #[derive(Clone, Debug, PartialEq, Eq, Hash)] pub struct FrozenCids(CidBox); @@ -51,7 +56,7 @@ impl Iterator for FrozenCidsIterator<'_> { let cid = Cid::new_v1( DAG_CBOR, multihash::Multihash::wrap(Blake2b256.into(), &cids[self.current_ix]) - .expect("failed to convert digest to CID"), + .expect("failed to convert Blake2b digest to V1 DAG-CBOR Blake2b CID"), ); self.current_ix += 1; Some(cid) @@ -98,6 +103,7 @@ impl FromIterator for FrozenCids { } } + // Converts `Vec` to `FrozenCids(CidBox::V1DagCborBlake2bCids)` if possible; otherwise, converts to `FrozenCids(CidBox::AllCids)`. impl From> for FrozenCids { fn from(cids: Vec) -> Self { let mut v1dagcborblake2bcids = Vec::new(); @@ -121,7 +127,7 @@ impl From> for FrozenCids { Cid::new_v1( DAG_CBOR, multihash::Multihash::wrap(Blake2b256.into(), &bytes) - .expect("failed to convert digest to CID"), + .expect("failed to convert Blake2b digest to V1 DAG-CBOR Blake2b CID"), ) })); FrozenCids(CidBox::AllCids(allcids.into_boxed_slice())) @@ -138,7 +144,7 @@ impl From for Vec { Cid::new_v1( DAG_CBOR, multihash::Multihash::wrap(Blake2b256.into(), bytes) - .expect("failed to convert digest to CID"), + .expect("failed to convert Blake2b digest to V1 DAG-CBOR Blake2b CID"), ) }) .collect(), @@ -156,7 +162,7 @@ impl From<&FrozenCids> for Vec { Cid::new_v1( DAG_CBOR, multihash::Multihash::wrap(Blake2b256.into(), bytes) - .expect("failed to convert digest to CID"), + .expect("failed to convert Blake2b digest to V1 DAG-CBOR Blake2b CID"), ) }) .collect(), @@ -166,6 +172,7 @@ impl From<&FrozenCids> for Vec { } impl FrozenCids { + /// Adds a CID to `FrozenCids`, returning the appropriate `FrozenCids` variant via the `FrozenCids::from` call. pub fn push(&self, cid: Cid) -> Self { let mut cids = Vec::::from(self); cids.push(cid); From f72fb81be29ff41f0cda687123400af57083e97e Mon Sep 17 00:00:00 2001 From: Josh Jones Date: Fri, 4 Aug 2023 08:39:32 -0500 Subject: [PATCH 18/33] cargo fmt --- src/ipld/frozen_cids.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/ipld/frozen_cids.rs b/src/ipld/frozen_cids.rs index d327596ec5f9..1a34f19e430d 100644 --- a/src/ipld/frozen_cids.rs +++ b/src/ipld/frozen_cids.rs @@ -9,9 +9,9 @@ use cid::{ use fvm_ipld_encoding::DAG_CBOR; use serde::{Deserialize, Deserializer, Serialize, Serializer}; -// `FrozenCids` takes advantage of the fact that the V1 DAG-CBOR Blake2b-256 variant -// (which can be stored in 32 bytes vs 96 bytes for a `Cid` type) is +99.99% of -// all CIDs. `FrozenCids` defaults to the `Box<[u8; BLAKE2B256_SIZE]>` variant of +// `FrozenCids` takes advantage of the fact that the V1 DAG-CBOR Blake2b-256 variant +// (which can be stored in 32 bytes vs 96 bytes for a `Cid` type) is +99.99% of +// all CIDs. `FrozenCids` defaults to the `Box<[u8; BLAKE2B256_SIZE]>` variant of // `CidBox`, only using the more expensive `Box` variant when necessary. // The Box type has been chosen to make `FrozenCids` explicitly immutable. #[derive(Clone, Debug, PartialEq, Eq, Hash)] @@ -103,7 +103,7 @@ impl FromIterator for FrozenCids { } } - // Converts `Vec` to `FrozenCids(CidBox::V1DagCborBlake2bCids)` if possible; otherwise, converts to `FrozenCids(CidBox::AllCids)`. +// Converts `Vec` to `FrozenCids(CidBox::V1DagCborBlake2bCids)` if possible; otherwise, converts to `FrozenCids(CidBox::AllCids)`. impl From> for FrozenCids { fn from(cids: Vec) -> Self { let mut v1dagcborblake2bcids = Vec::new(); @@ -172,7 +172,7 @@ impl From<&FrozenCids> for Vec { } impl FrozenCids { - /// Adds a CID to `FrozenCids`, returning the appropriate `FrozenCids` variant via the `FrozenCids::from` call. + /// Adds a CID to `FrozenCids`, returning the appropriate `FrozenCids` variant via the `FrozenCids::from` call. pub fn push(&self, cid: Cid) -> Self { let mut cids = Vec::::from(self); cids.push(cid); From 23a6dbbb27bde9fe5598a97d703a95d2966491da Mon Sep 17 00:00:00 2001 From: Josh Jones Date: Fri, 4 Aug 2023 11:10:10 -0500 Subject: [PATCH 19/33] update from merge conflict --- src/chain/store/chain_store.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/chain/store/chain_store.rs b/src/chain/store/chain_store.rs index 5c57834fdf80..a242baa4ed93 100644 --- a/src/chain/store/chain_store.rs +++ b/src/chain/store/chain_store.rs @@ -115,7 +115,7 @@ where .read_obj::(HEAD_KEY)? .is_some_and(|tipset_keys| chain_index.load_tipset(&tipset_keys).is_ok()) { - let tipset_keys = TipsetKeys::new(vec![*genesis_block_header.cid()]); + let tipset_keys = TipsetKeys::new(FrozenCids::from_iter([*genesis_block_header.cid()])); settings.write_obj(HEAD_KEY, &tipset_keys)?; } From 6c2ba6b51f484a9ffe06aaad7277091ce27e7463 Mon Sep 17 00:00:00 2001 From: Josh Jones Date: Mon, 7 Aug 2023 16:20:35 -0500 Subject: [PATCH 20/33] modify `FrozenCids` implementation --- src/ipld/frozen_cids.rs | 136 ++++++++++++++-------------------------- 1 file changed, 46 insertions(+), 90 deletions(-) diff --git a/src/ipld/frozen_cids.rs b/src/ipld/frozen_cids.rs index 1a34f19e430d..768272b3e654 100644 --- a/src/ipld/frozen_cids.rs +++ b/src/ipld/frozen_cids.rs @@ -9,24 +9,26 @@ use cid::{ use fvm_ipld_encoding::DAG_CBOR; use serde::{Deserialize, Deserializer, Serialize, Serializer}; -// `FrozenCids` takes advantage of the fact that the V1 DAG-CBOR Blake2b-256 variant -// (which can be stored in 32 bytes vs 96 bytes for a `Cid` type) is +99.99% of -// all CIDs. `FrozenCids` defaults to the `Box<[u8; BLAKE2B256_SIZE]>` variant of -// `CidBox`, only using the more expensive `Box` variant when necessary. -// The Box type has been chosen to make `FrozenCids` explicitly immutable. +// Similar to the `CidHashMap` implementation, `FrozenCids` optimizes storage of +// CIDs that would normally be stored as a vector of CIDs. The V1 DAG-CBOR +// Blake2b-256 variant (which can be stored in 32 bytes vs 96 bytes for a `Cid` +// type) is +99.99% of all CIDs, so very few CIDs need to be stored in the +// `Heap(Box)` variant of `SmallCid`. The Box type has been chosen to make +// `FrozenCids` explicitly immutable due to the fact that the vectors are parsed +// from CBOR and are immutable. #[derive(Clone, Debug, PartialEq, Eq, Hash)] -pub struct FrozenCids(CidBox); +pub struct FrozenCids(Box<[SmallCid]>); impl Default for FrozenCids { fn default() -> Self { - Self(CidBox::V1DagCborBlake2bCids(Box::new([]))) + FrozenCids(Box::new([])) } } #[derive(Clone, Debug, PartialEq, Eq, Hash)] -enum CidBox { - V1DagCborBlake2bCids(Box<[[u8; BLAKE2B256_SIZE]]>), - AllCids(Box<[Cid]>), +enum SmallCid { + Heap(Box), + Inline([u8; 32]), } pub struct FrozenCidsIterator<'a> { @@ -48,29 +50,23 @@ impl<'a> IntoIterator for &'a FrozenCids { impl Iterator for FrozenCidsIterator<'_> { type Item = Cid; fn next(&mut self) -> Option { - match &self.buffer.0 { - CidBox::V1DagCborBlake2bCids(cids) => { - if self.current_ix >= cids.len() { - None - } else { - let cid = Cid::new_v1( + if self.current_ix < self.buffer.0.len() { + let cid = &self.buffer.0[self.current_ix]; + self.current_ix += 1; + match cid { + SmallCid::Heap(cid) => Some(*cid.clone()), + SmallCid::Inline(bytes) => { + let mut cid = [0; BLAKE2B256_SIZE]; + cid.copy_from_slice(bytes); + Some(Cid::new_v1( DAG_CBOR, - multihash::Multihash::wrap(Blake2b256.into(), &cids[self.current_ix]) + multihash::Multihash::wrap(Blake2b256.into(), &cid) .expect("failed to convert Blake2b digest to V1 DAG-CBOR Blake2b CID"), - ); - self.current_ix += 1; - Some(cid) - } - } - CidBox::AllCids(cids) => { - if self.current_ix >= cids.len() { - None - } else { - let cid = cids[self.current_ix]; - self.current_ix += 1; - Some(cid) + )) } } + } else { + None } } } @@ -103,76 +99,47 @@ impl FromIterator for FrozenCids { } } -// Converts `Vec` to `FrozenCids(CidBox::V1DagCborBlake2bCids)` if possible; otherwise, converts to `FrozenCids(CidBox::AllCids)`. impl From> for FrozenCids { fn from(cids: Vec) -> Self { - let mut v1dagcborblake2bcids = Vec::new(); - let mut allcids = Vec::new(); + let mut small_cids = Vec::with_capacity(cids.len()); for cid in cids { match cid.try_into() { - Ok(CidVariant::V1DagCborBlake2b(bytes)) => { - v1dagcborblake2bcids.push(bytes); - } - _ => { - allcids.push(cid); - } + Ok(CidVariant::V1DagCborBlake2b(bytes)) => small_cids.push(SmallCid::Inline(bytes)), + _ => small_cids.push(SmallCid::Heap(Box::new(cid))), } } - if allcids.is_empty() { - FrozenCids(CidBox::V1DagCborBlake2bCids( - v1dagcborblake2bcids.into_boxed_slice(), - )) - } else { - allcids.extend(v1dagcborblake2bcids.into_iter().map(|bytes| { - Cid::new_v1( - DAG_CBOR, - multihash::Multihash::wrap(Blake2b256.into(), &bytes) - .expect("failed to convert Blake2b digest to V1 DAG-CBOR Blake2b CID"), - ) - })); - FrozenCids(CidBox::AllCids(allcids.into_boxed_slice())) - } + FrozenCids(small_cids.into_boxed_slice()) } } impl From for Vec { fn from(frozen_cids: FrozenCids) -> Self { - match frozen_cids.0 { - CidBox::V1DagCborBlake2bCids(cids) => cids - .iter() - .map(|bytes| { - Cid::new_v1( - DAG_CBOR, - multihash::Multihash::wrap(Blake2b256.into(), bytes) - .expect("failed to convert Blake2b digest to V1 DAG-CBOR Blake2b CID"), - ) - }) - .collect(), - CidBox::AllCids(cids) => cids.to_vec(), - } + Vec::::from(&frozen_cids) } } impl From<&FrozenCids> for Vec { fn from(frozen_cids: &FrozenCids) -> Self { - match &frozen_cids.0 { - CidBox::V1DagCborBlake2bCids(cids) => cids - .iter() - .map(|bytes| { - Cid::new_v1( + let mut cids = Vec::with_capacity(frozen_cids.0.len()); + for cid in frozen_cids.into_iter() { + match cid.try_into() { + Ok(CidVariant::V1DagCborBlake2b(bytes)) => { + let mut digest = [0; BLAKE2B256_SIZE]; + digest.copy_from_slice(&bytes); + cids.push(Cid::new_v1( DAG_CBOR, - multihash::Multihash::wrap(Blake2b256.into(), bytes) + multihash::Multihash::wrap(Blake2b256.into(), &digest) .expect("failed to convert Blake2b digest to V1 DAG-CBOR Blake2b CID"), - ) - }) - .collect(), - CidBox::AllCids(cids) => cids.to_vec(), + )) + } + _ => cids.push(cid), + } } + cids } } impl FrozenCids { - /// Adds a CID to `FrozenCids`, returning the appropriate `FrozenCids` variant via the `FrozenCids::from` call. pub fn push(&self, cid: Cid) -> Self { let mut cids = Vec::::from(self); cids.push(cid); @@ -180,23 +147,12 @@ impl FrozenCids { } pub fn is_empty(&self) -> bool { - match &self.0 { - CidBox::V1DagCborBlake2bCids(cids) => cids.is_empty(), - CidBox::AllCids(cids) => cids.is_empty(), - } + self.0.is_empty() } pub fn contains(&self, cid: Cid) -> bool { - match &self.0 { - CidBox::V1DagCborBlake2bCids(cids) => { - if let Ok(CidVariant::V1DagCborBlake2b(bytes)) = cid.try_into() { - cids.contains(&bytes) - } else { - false - } - } - CidBox::AllCids(cids) => cids.contains(&cid), - } + let cids = Vec::::from(self); + cids.contains(&cid) } } From 2204e19ad25622a1bbaa6d0f1c5653ea2dcf88f0 Mon Sep 17 00:00:00 2001 From: Josh Jones Date: Tue, 8 Aug 2023 09:03:04 -0500 Subject: [PATCH 21/33] Apply suggestions from code review Co-authored-by: Aatif Syed <38045910+aatifsyed@users.noreply.github.com> --- src/ipld/frozen_cids.rs | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/src/ipld/frozen_cids.rs b/src/ipld/frozen_cids.rs index 768272b3e654..aa9b6e3f2575 100644 --- a/src/ipld/frozen_cids.rs +++ b/src/ipld/frozen_cids.rs @@ -9,13 +9,13 @@ use cid::{ use fvm_ipld_encoding::DAG_CBOR; use serde::{Deserialize, Deserializer, Serialize, Serializer}; -// Similar to the `CidHashMap` implementation, `FrozenCids` optimizes storage of -// CIDs that would normally be stored as a vector of CIDs. The V1 DAG-CBOR -// Blake2b-256 variant (which can be stored in 32 bytes vs 96 bytes for a `Cid` -// type) is +99.99% of all CIDs, so very few CIDs need to be stored in the -// `Heap(Box)` variant of `SmallCid`. The Box type has been chosen to make -// `FrozenCids` explicitly immutable due to the fact that the vectors are parsed -// from CBOR and are immutable. +/// Similar to the `CidHashMap` implementation, `FrozenCids` optimizes storage of +/// CIDs that would normally be stored as a vector of CIDs. The V1 DAG-CBOR +/// Blake2b-256 variant (which can be stored in 32 bytes vs 96 bytes for a `Cid` +/// type) is +99.99% of all CIDs, so very few CIDs need to be stored in the +/// `Heap(Box)` variant of `SmallCid`. +/// +/// We use `Box<[...]>` to save memory, avoiding vector overallocation. #[derive(Clone, Debug, PartialEq, Eq, Hash)] pub struct FrozenCids(Box<[SmallCid]>); @@ -28,7 +28,7 @@ impl Default for FrozenCids { #[derive(Clone, Debug, PartialEq, Eq, Hash)] enum SmallCid { Heap(Box), - Inline([u8; 32]), + InlineDagCborV1([u8; 32]), } pub struct FrozenCidsIterator<'a> { @@ -91,11 +91,7 @@ impl<'de> Deserialize<'de> for FrozenCids { impl FromIterator for FrozenCids { fn from_iter>(iter: T) -> Self { - let mut vec = Vec::new(); - for i in iter { - vec.push(i); - } - FrozenCids::from(vec) + FrozenCids::from(iter.into_iter().collect::>()) } } From 5513c48b11a78b07a169232ab478bd8c72023bc6 Mon Sep 17 00:00:00 2001 From: Josh Jones Date: Tue, 8 Aug 2023 10:42:39 -0500 Subject: [PATCH 22/33] update `Inline` instances --- src/ipld/frozen_cids.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ipld/frozen_cids.rs b/src/ipld/frozen_cids.rs index aa9b6e3f2575..da48606092f3 100644 --- a/src/ipld/frozen_cids.rs +++ b/src/ipld/frozen_cids.rs @@ -55,7 +55,7 @@ impl Iterator for FrozenCidsIterator<'_> { self.current_ix += 1; match cid { SmallCid::Heap(cid) => Some(*cid.clone()), - SmallCid::Inline(bytes) => { + SmallCid::InlineDagCborV1(bytes) => { let mut cid = [0; BLAKE2B256_SIZE]; cid.copy_from_slice(bytes); Some(Cid::new_v1( @@ -100,7 +100,7 @@ impl From> for FrozenCids { let mut small_cids = Vec::with_capacity(cids.len()); for cid in cids { match cid.try_into() { - Ok(CidVariant::V1DagCborBlake2b(bytes)) => small_cids.push(SmallCid::Inline(bytes)), + Ok(CidVariant::V1DagCborBlake2b(bytes)) => small_cids.push(SmallCid::InlineDagCborV1(bytes)), _ => small_cids.push(SmallCid::Heap(Box::new(cid))), } } From 48c550669ffc7f5fbca909b904d33b45acd8c819 Mon Sep 17 00:00:00 2001 From: Josh Jones Date: Tue, 8 Aug 2023 12:36:28 -0500 Subject: [PATCH 23/33] modify `CidVariant` to replace `SmallCid` --- src/ipld/cid_hashmap.rs | 8 ++++---- src/ipld/frozen_cids.rs | 22 +++++++++------------- src/utils/cid/mod.rs | 21 +++++++++++++-------- 3 files changed, 26 insertions(+), 25 deletions(-) diff --git a/src/ipld/cid_hashmap.rs b/src/ipld/cid_hashmap.rs index a3d8e83a84f2..2746bd6db3a4 100644 --- a/src/ipld/cid_hashmap.rs +++ b/src/ipld/cid_hashmap.rs @@ -37,7 +37,7 @@ impl CidHashMap { Ok(CidVariant::V1DagCborBlake2b(bytes)) => { self.v1_dagcbor_blake2b_hash_map.contains_key(&bytes) } - Err(()) => self.fallback_hash_map.contains_key(&k), + Ok(CidVariant::Generic(_)) | Err(()) => self.fallback_hash_map.contains_key(&k), } } @@ -47,7 +47,7 @@ impl CidHashMap { Ok(CidVariant::V1DagCborBlake2b(bytes)) => { self.v1_dagcbor_blake2b_hash_map.insert(bytes, v) } - Err(()) => self.fallback_hash_map.insert(k, v), + Ok(CidVariant::Generic(_)) | Err(()) => self.fallback_hash_map.insert(k, v), } } @@ -58,7 +58,7 @@ impl CidHashMap { Ok(CidVariant::V1DagCborBlake2b(bytes)) => { self.v1_dagcbor_blake2b_hash_map.remove(&bytes) } - Err(()) => self.fallback_hash_map.remove(&k), + Ok(CidVariant::Generic(_)) | Err(()) => self.fallback_hash_map.remove(&k), } } @@ -71,7 +71,7 @@ impl CidHashMap { pub fn get(&self, k: Cid) -> Option<&V> { match k.try_into() { Ok(CidVariant::V1DagCborBlake2b(bytes)) => self.v1_dagcbor_blake2b_hash_map.get(&bytes), - Err(()) => self.fallback_hash_map.get(&k), + Ok(CidVariant::Generic(_)) | Err(()) => self.fallback_hash_map.get(&k), } } diff --git a/src/ipld/frozen_cids.rs b/src/ipld/frozen_cids.rs index da48606092f3..d17a991d9ae6 100644 --- a/src/ipld/frozen_cids.rs +++ b/src/ipld/frozen_cids.rs @@ -13,11 +13,11 @@ use serde::{Deserialize, Deserializer, Serialize, Serializer}; /// CIDs that would normally be stored as a vector of CIDs. The V1 DAG-CBOR /// Blake2b-256 variant (which can be stored in 32 bytes vs 96 bytes for a `Cid` /// type) is +99.99% of all CIDs, so very few CIDs need to be stored in the -/// `Heap(Box)` variant of `SmallCid`. -/// +/// `Generic(Box)` variant of `CidVariant`. +/// /// We use `Box<[...]>` to save memory, avoiding vector overallocation. #[derive(Clone, Debug, PartialEq, Eq, Hash)] -pub struct FrozenCids(Box<[SmallCid]>); +pub struct FrozenCids(Box<[CidVariant]>); impl Default for FrozenCids { fn default() -> Self { @@ -25,12 +25,6 @@ impl Default for FrozenCids { } } -#[derive(Clone, Debug, PartialEq, Eq, Hash)] -enum SmallCid { - Heap(Box), - InlineDagCborV1([u8; 32]), -} - pub struct FrozenCidsIterator<'a> { buffer: &'a FrozenCids, current_ix: usize, @@ -54,8 +48,8 @@ impl Iterator for FrozenCidsIterator<'_> { let cid = &self.buffer.0[self.current_ix]; self.current_ix += 1; match cid { - SmallCid::Heap(cid) => Some(*cid.clone()), - SmallCid::InlineDagCborV1(bytes) => { + CidVariant::Generic(cid) => Some(*cid.clone()), + CidVariant::V1DagCborBlake2b(bytes) => { let mut cid = [0; BLAKE2B256_SIZE]; cid.copy_from_slice(bytes); Some(Cid::new_v1( @@ -100,8 +94,10 @@ impl From> for FrozenCids { let mut small_cids = Vec::with_capacity(cids.len()); for cid in cids { match cid.try_into() { - Ok(CidVariant::V1DagCborBlake2b(bytes)) => small_cids.push(SmallCid::InlineDagCborV1(bytes)), - _ => small_cids.push(SmallCid::Heap(Box::new(cid))), + Ok(CidVariant::V1DagCborBlake2b(bytes)) => { + small_cids.push(CidVariant::V1DagCborBlake2b(bytes)) + } + _ => small_cids.push(CidVariant::Generic(Box::new(cid))), } } FrozenCids(small_cids.into_boxed_slice()) diff --git a/src/utils/cid/mod.rs b/src/utils/cid/mod.rs index 439d14431a71..29d836d21dd1 100644 --- a/src/utils/cid/mod.rs +++ b/src/utils/cid/mod.rs @@ -28,15 +28,20 @@ impl CidCborExt for Cid {} pub const BLAKE2B256_SIZE: usize = 32; -// `CidVariant` is an enumeration of known CID types that are used in the Filecoin blockchain. CIDs -// contain a significant amount of static data (such as version, codec, hash identifier, hash -// length). This static data represented by a single tag in the enum. -// -// Nearly all Filecoin CIDs are V1, DagCbor encoded, and hashed with Blake2b256 (which has a hash -// length of 256bits). Naively representing such a CID requires 96 bytes but `CidVariant` does it in -// only 40 bytes. If other types of CID become popular, they can be added to the CidVariant -// structure. +/// `CidVariant` is an enumeration of known CID types that are used in the Filecoin blockchain. CIDs +/// contain a significant amount of static data (such as version, codec, hash identifier, hash +/// length). This static data represented by a single tag in the enum. +/// +/// Nearly all Filecoin CIDs are V1, DagCbor encoded, and hashed with Blake2b256 (which has a hash +/// length of 256bits). Naively representing such a CID requires 96 bytes but `CidVariant` does it in +/// only 40 bytes. If other types of CID become popular, they can be added to the CidVariant +/// structure. +/// +/// The `Generic` variant is used for CIDs that do not fit into the other variants. +/// These variants are used for optimizing storage of CIDs in the `FrozenCids` structure. +#[derive(Clone, Debug, PartialEq, Eq, Hash)] pub enum CidVariant { + Generic(Box), V1DagCborBlake2b([u8; BLAKE2B256_SIZE]), } From 2bbf9ef8f9679c1c6f96c0ee0d0951fa96a4b6d5 Mon Sep 17 00:00:00 2001 From: Josh Jones Date: Tue, 8 Aug 2023 12:54:48 -0500 Subject: [PATCH 24/33] spellcheck --- .config/forest.dic | 1 + src/ipld/frozen_cids.rs | 6 +++--- src/utils/cid/mod.rs | 8 ++++---- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/.config/forest.dic b/.config/forest.dic index 08542e3aa083..120e73c48695 100644 --- a/.config/forest.dic +++ b/.config/forest.dic @@ -54,6 +54,7 @@ milliGas multihash multisig mutex +overallocation P2P parsable pointer/SM diff --git a/src/ipld/frozen_cids.rs b/src/ipld/frozen_cids.rs index d17a991d9ae6..f901400894ed 100644 --- a/src/ipld/frozen_cids.rs +++ b/src/ipld/frozen_cids.rs @@ -10,9 +10,9 @@ use fvm_ipld_encoding::DAG_CBOR; use serde::{Deserialize, Deserializer, Serialize, Serializer}; /// Similar to the `CidHashMap` implementation, `FrozenCids` optimizes storage of -/// CIDs that would normally be stored as a vector of CIDs. The V1 DAG-CBOR -/// Blake2b-256 variant (which can be stored in 32 bytes vs 96 bytes for a `Cid` -/// type) is +99.99% of all CIDs, so very few CIDs need to be stored in the +/// CIDs that would normally be stored as a vector of CIDs. The `V1 DAG-CBOR Blake2b-256` +/// variant (which can be stored in 32 bytes vs 96 bytes for a `Cid` +/// type) is `+99.99%` of all CIDs, so very few CIDs need to be stored in the /// `Generic(Box)` variant of `CidVariant`. /// /// We use `Box<[...]>` to save memory, avoiding vector overallocation. diff --git a/src/utils/cid/mod.rs b/src/utils/cid/mod.rs index 29d836d21dd1..85e3605cfc5f 100644 --- a/src/utils/cid/mod.rs +++ b/src/utils/cid/mod.rs @@ -30,11 +30,11 @@ pub const BLAKE2B256_SIZE: usize = 32; /// `CidVariant` is an enumeration of known CID types that are used in the Filecoin blockchain. CIDs /// contain a significant amount of static data (such as version, codec, hash identifier, hash -/// length). This static data represented by a single tag in the enum. +/// length). This static data represented by a single tag in the `enum`. /// -/// Nearly all Filecoin CIDs are V1, DagCbor encoded, and hashed with Blake2b256 (which has a hash -/// length of 256bits). Naively representing such a CID requires 96 bytes but `CidVariant` does it in -/// only 40 bytes. If other types of CID become popular, they can be added to the CidVariant +/// Nearly all Filecoin CIDs are `V1`,`DagCbor` encoded, and hashed with `Blake2b256` (which has a hash +/// length of 256 bits). Naively representing such a CID requires 96 bytes but `CidVariant` does it in +/// only 40 bytes. If other types of CID become popular, they can be added to the `CidVariant` /// structure. /// /// The `Generic` variant is used for CIDs that do not fit into the other variants. From 709f3abff42ce9572db634cb90294bcdcb4c6fec Mon Sep 17 00:00:00 2001 From: Josh Jones Date: Tue, 8 Aug 2023 15:24:10 -0500 Subject: [PATCH 25/33] additional changes from code review --- src/ipld/cid_hashmap.rs | 26 ++++++++++++-------------- src/ipld/frozen_cids.rs | 16 +++++----------- src/utils/cid/mod.rs | 40 +++++++++++++++++++++++++++++++++------- 3 files changed, 50 insertions(+), 32 deletions(-) diff --git a/src/ipld/cid_hashmap.rs b/src/ipld/cid_hashmap.rs index 2746bd6db3a4..9cb7314c2e1a 100644 --- a/src/ipld/cid_hashmap.rs +++ b/src/ipld/cid_hashmap.rs @@ -33,32 +33,30 @@ impl CidHashMap { /// Returns `true` if the map contains a value for the specified key. pub fn contains_key(&self, k: Cid) -> bool { - match k.try_into() { - Ok(CidVariant::V1DagCborBlake2b(bytes)) => { + match k.into() { + CidVariant::V1DagCborBlake2b(bytes) => { self.v1_dagcbor_blake2b_hash_map.contains_key(&bytes) } - Ok(CidVariant::Generic(_)) | Err(()) => self.fallback_hash_map.contains_key(&k), + CidVariant::Generic(_) => self.fallback_hash_map.contains_key(&k), } } /// Inserts a key-value pair into the map; if the map did not have this key present, [`None`] is returned. pub fn insert(&mut self, k: Cid, v: V) -> Option { - match k.try_into() { - Ok(CidVariant::V1DagCborBlake2b(bytes)) => { + match k.into() { + CidVariant::V1DagCborBlake2b(bytes) => { self.v1_dagcbor_blake2b_hash_map.insert(bytes, v) } - Ok(CidVariant::Generic(_)) | Err(()) => self.fallback_hash_map.insert(k, v), + CidVariant::Generic(_) => self.fallback_hash_map.insert(k, v), } } /// Removes a key from the map, returning the value at the key if the key /// was previously in the map. pub fn remove(&mut self, k: Cid) -> Option { - match k.try_into() { - Ok(CidVariant::V1DagCborBlake2b(bytes)) => { - self.v1_dagcbor_blake2b_hash_map.remove(&bytes) - } - Ok(CidVariant::Generic(_)) | Err(()) => self.fallback_hash_map.remove(&k), + match k.into() { + CidVariant::V1DagCborBlake2b(bytes) => self.v1_dagcbor_blake2b_hash_map.remove(&bytes), + CidVariant::Generic(_) => self.fallback_hash_map.remove(&k), } } @@ -69,9 +67,9 @@ impl CidHashMap { /// Returns a reference to the value corresponding to the key. pub fn get(&self, k: Cid) -> Option<&V> { - match k.try_into() { - Ok(CidVariant::V1DagCborBlake2b(bytes)) => self.v1_dagcbor_blake2b_hash_map.get(&bytes), - Ok(CidVariant::Generic(_)) | Err(()) => self.fallback_hash_map.get(&k), + match k.into() { + CidVariant::V1DagCborBlake2b(bytes) => self.v1_dagcbor_blake2b_hash_map.get(&bytes), + CidVariant::Generic(_) => self.fallback_hash_map.get(&k), } } diff --git a/src/ipld/frozen_cids.rs b/src/ipld/frozen_cids.rs index f901400894ed..47db4c024bd1 100644 --- a/src/ipld/frozen_cids.rs +++ b/src/ipld/frozen_cids.rs @@ -93,8 +93,8 @@ impl From> for FrozenCids { fn from(cids: Vec) -> Self { let mut small_cids = Vec::with_capacity(cids.len()); for cid in cids { - match cid.try_into() { - Ok(CidVariant::V1DagCborBlake2b(bytes)) => { + match cid.into() { + CidVariant::V1DagCborBlake2b(bytes) => { small_cids.push(CidVariant::V1DagCborBlake2b(bytes)) } _ => small_cids.push(CidVariant::Generic(Box::new(cid))), @@ -114,15 +114,9 @@ impl From<&FrozenCids> for Vec { fn from(frozen_cids: &FrozenCids) -> Self { let mut cids = Vec::with_capacity(frozen_cids.0.len()); for cid in frozen_cids.into_iter() { - match cid.try_into() { - Ok(CidVariant::V1DagCborBlake2b(bytes)) => { - let mut digest = [0; BLAKE2B256_SIZE]; - digest.copy_from_slice(&bytes); - cids.push(Cid::new_v1( - DAG_CBOR, - multihash::Multihash::wrap(Blake2b256.into(), &digest) - .expect("failed to convert Blake2b digest to V1 DAG-CBOR Blake2b CID"), - )) + match cid.into() { + CidVariant::V1DagCborBlake2b(bytes) => { + cids.push(Cid::from(CidVariant::V1DagCborBlake2b(bytes))) } _ => cids.push(cid), } diff --git a/src/utils/cid/mod.rs b/src/utils/cid/mod.rs index 85e3605cfc5f..e9a893350d4a 100644 --- a/src/utils/cid/mod.rs +++ b/src/utils/cid/mod.rs @@ -2,7 +2,7 @@ // SPDX-License-Identifier: Apache-2.0, MIT use cid::{ - multihash::{Code, MultihashDigest}, + multihash::{self, Code, Code::Blake2b256, MultihashDigest}, Cid, Version, }; use fvm_ipld_encoding::{Error, DAG_CBOR}; @@ -45,18 +45,30 @@ pub enum CidVariant { V1DagCborBlake2b([u8; BLAKE2B256_SIZE]), } -impl TryFrom for CidVariant { - type Error = (); - fn try_from(cid: Cid) -> Result { +impl From for CidVariant { + fn from(cid: Cid) -> Self { if cid.version() == Version::V1 && cid.codec() == DAG_CBOR { if let Ok(small_hash) = cid.hash().resize() { let (code, bytes, size) = small_hash.into_inner(); if code == u64::from(Code::Blake2b256) && size as usize == BLAKE2B256_SIZE { - return Ok(CidVariant::V1DagCborBlake2b(bytes)); + return CidVariant::V1DagCborBlake2b(bytes); } } } - Err(()) + CidVariant::Generic(Box::new(cid)) + } +} + +impl From for Cid { + fn from(variant: CidVariant) -> Self { + match variant { + CidVariant::Generic(cid) => *cid, + CidVariant::V1DagCborBlake2b(digest) => Cid::new_v1( + DAG_CBOR, + multihash::Multihash::wrap(Blake2b256.into(), &digest) + .expect("failed to convert Blake2b digest to V1 DAG-CBOR Blake2b CID"), + ), + } } } @@ -68,13 +80,27 @@ mod tests { use crate::utils::db::CborStoreExt; use anyhow::*; use cid::{ - multihash::{Code, MultihashDigest}, + multihash::{self, Code, MultihashDigest}, Cid, }; use fvm_ipld_encoding::DAG_CBOR; + use quickcheck::Arbitrary; use quickcheck_macros::quickcheck; use std::mem::size_of; + impl Arbitrary for CidVariant { + fn arbitrary(g: &mut quickcheck::Gen) -> Self { + // Quickcheck does not reliably generate the DAG_CBOR/Blake2b variant of V1 CIDs, but we can manually create them. This method will generate each variant of `CidVariant` with equal probability. + match bool::arbitrary(g) { + true => CidVariant::Generic(Box::new(Cid::arbitrary(g))), + false => CidVariant::from(Cid::new_v1( + DAG_CBOR, + multihash::Code::Blake2b256.digest(&u64::arbitrary(g).to_be_bytes()), + )), + } + } + } + #[quickcheck] fn test_cid_cbor_ext(s: String) -> Result<()> { let cid1 = Cid::from_cbor_blake2b256(&s)?; From 71b9cac0773608ec493193739f5cf3db4b955485 Mon Sep 17 00:00:00 2001 From: Josh Jones Date: Wed, 9 Aug 2023 09:06:49 -0500 Subject: [PATCH 26/33] modify `IntoIterator` impl --- src/ipld/frozen_cids.rs | 46 ++++++++++++++--------------------------- 1 file changed, 16 insertions(+), 30 deletions(-) diff --git a/src/ipld/frozen_cids.rs b/src/ipld/frozen_cids.rs index 47db4c024bd1..205582c2398a 100644 --- a/src/ipld/frozen_cids.rs +++ b/src/ipld/frozen_cids.rs @@ -1,12 +1,8 @@ // Copyright 2019-2023 ChainSafe Systems // SPDX-License-Identifier: Apache-2.0, MIT -use crate::utils::cid::{CidVariant, BLAKE2B256_SIZE}; -use cid::{ - multihash::{self, Code::Blake2b256}, - Cid, -}; -use fvm_ipld_encoding::DAG_CBOR; +use crate::utils::cid::CidVariant; +use cid::Cid; use serde::{Deserialize, Deserializer, Serialize, Serializer}; /// Similar to the `CidHashMap` implementation, `FrozenCids` optimizes storage of @@ -25,42 +21,31 @@ impl Default for FrozenCids { } } -pub struct FrozenCidsIterator<'a> { - buffer: &'a FrozenCids, - current_ix: usize, +pub struct IntoIter<'a> { + cids: std::slice::Iter<'a, CidVariant>, } impl<'a> IntoIterator for &'a FrozenCids { type Item = Cid; - type IntoIter = FrozenCidsIterator<'a>; + type IntoIter = IntoIter<'a>; fn into_iter(self) -> Self::IntoIter { - FrozenCidsIterator { - buffer: self, - current_ix: 0, + IntoIter { + cids: self.0.iter(), } } } -impl Iterator for FrozenCidsIterator<'_> { +impl<'a> Iterator for IntoIter<'a> { type Item = Cid; fn next(&mut self) -> Option { - if self.current_ix < self.buffer.0.len() { - let cid = &self.buffer.0[self.current_ix]; - self.current_ix += 1; - match cid { - CidVariant::Generic(cid) => Some(*cid.clone()), + match self.cids.next() { + Some(cid) => match cid { CidVariant::V1DagCborBlake2b(bytes) => { - let mut cid = [0; BLAKE2B256_SIZE]; - cid.copy_from_slice(bytes); - Some(Cid::new_v1( - DAG_CBOR, - multihash::Multihash::wrap(Blake2b256.into(), &cid) - .expect("failed to convert Blake2b digest to V1 DAG-CBOR Blake2b CID"), - )) + Some(Cid::from(CidVariant::V1DagCborBlake2b(*bytes))) } - } - } else { - None + CidVariant::Generic(cid) => Some(*cid.clone()), + }, + None => None, } } } @@ -145,7 +130,8 @@ impl FrozenCids { #[cfg(test)] mod test { use super::*; - use cid::multihash::MultihashDigest; + use cid::multihash::{self, MultihashDigest}; + use fvm_ipld_encoding::DAG_CBOR; use quickcheck::Arbitrary; use quickcheck_macros::quickcheck; From 90d2b1f8b39db2705c14a86d037ef33194c77db9 Mon Sep 17 00:00:00 2001 From: Josh Jones Date: Wed, 9 Aug 2023 10:49:36 -0500 Subject: [PATCH 27/33] some changes from code review --- src/ipld/frozen_cids.rs | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/src/ipld/frozen_cids.rs b/src/ipld/frozen_cids.rs index 205582c2398a..ad1d64815bc4 100644 --- a/src/ipld/frozen_cids.rs +++ b/src/ipld/frozen_cids.rs @@ -21,21 +21,21 @@ impl Default for FrozenCids { } } -pub struct IntoIter<'a> { +pub struct Iter<'a> { cids: std::slice::Iter<'a, CidVariant>, } impl<'a> IntoIterator for &'a FrozenCids { type Item = Cid; - type IntoIter = IntoIter<'a>; + type IntoIter = Iter<'a>; fn into_iter(self) -> Self::IntoIter { - IntoIter { + Iter { cids: self.0.iter(), } } } -impl<'a> Iterator for IntoIter<'a> { +impl<'a> Iterator for Iter<'a> { type Item = Cid; fn next(&mut self) -> Option { match self.cids.next() { @@ -111,19 +111,13 @@ impl From<&FrozenCids> for Vec { } impl FrozenCids { - pub fn push(&self, cid: Cid) -> Self { - let mut cids = Vec::::from(self); - cids.push(cid); - FrozenCids::from(cids) - } - pub fn is_empty(&self) -> bool { self.0.is_empty() } pub fn contains(&self, cid: Cid) -> bool { - let cids = Vec::::from(self); - cids.contains(&cid) + let cid = CidVariant::from(cid); + self.0.contains(&cid) } } From 487ef9b9b091a905e47a3f7eb3cb375c208c0e3d Mon Sep 17 00:00:00 2001 From: Josh Jones Date: Wed, 9 Aug 2023 12:55:43 -0500 Subject: [PATCH 28/33] code review change: modify (de)serialization impl --- src/ipld/frozen_cids.rs | 24 +++--------------------- src/utils/cid/mod.rs | 29 +++++++++++++++++++++++++++-- 2 files changed, 30 insertions(+), 23 deletions(-) diff --git a/src/ipld/frozen_cids.rs b/src/ipld/frozen_cids.rs index ad1d64815bc4..a2df5c998499 100644 --- a/src/ipld/frozen_cids.rs +++ b/src/ipld/frozen_cids.rs @@ -3,7 +3,7 @@ use crate::utils::cid::CidVariant; use cid::Cid; -use serde::{Deserialize, Deserializer, Serialize, Serializer}; +use serde::{Deserialize, Serialize}; /// Similar to the `CidHashMap` implementation, `FrozenCids` optimizes storage of /// CIDs that would normally be stored as a vector of CIDs. The `V1 DAG-CBOR Blake2b-256` @@ -12,7 +12,7 @@ use serde::{Deserialize, Deserializer, Serialize, Serializer}; /// `Generic(Box)` variant of `CidVariant`. /// /// We use `Box<[...]>` to save memory, avoiding vector overallocation. -#[derive(Clone, Debug, PartialEq, Eq, Hash)] +#[derive(Clone, Debug, PartialEq, Eq, Hash, Serialize, Deserialize)] pub struct FrozenCids(Box<[CidVariant]>); impl Default for FrozenCids { @@ -50,24 +50,6 @@ impl<'a> Iterator for Iter<'a> { } } -impl Serialize for FrozenCids { - fn serialize(&self, serializer: S) -> Result - where - S: Serializer, - { - Vec::::from(self).serialize(serializer) - } -} - -impl<'de> Deserialize<'de> for FrozenCids { - fn deserialize(deserializer: D) -> Result - where - D: Deserializer<'de>, - { - Ok(Self::from(>::deserialize(deserializer)?)) - } -} - impl FromIterator for FrozenCids { fn from_iter>(iter: T) -> Self { FrozenCids::from(iter.into_iter().collect::>()) @@ -131,7 +113,7 @@ mod test { impl Arbitrary for FrozenCids { fn arbitrary(g: &mut quickcheck::Gen) -> Self { - // Although the vast majority of CIDs are V1DagCborBlake2b, we want to generate the variants of CidVec with equal probability. + // Although the vast majority of CIDs are V1DagCborBlake2b, we want to generate the variants of FrozenCids with equal probability. if bool::arbitrary(g) { Vec::arbitrary(g).into_iter().collect() } else { diff --git a/src/utils/cid/mod.rs b/src/utils/cid/mod.rs index e9a893350d4a..11361882c6e4 100644 --- a/src/utils/cid/mod.rs +++ b/src/utils/cid/mod.rs @@ -6,6 +6,7 @@ use cid::{ Cid, Version, }; use fvm_ipld_encoding::{Error, DAG_CBOR}; +use serde::{Deserialize, Deserializer, Serialize, Serializer}; /// Extension methods for constructing `dag-cbor` [Cid] pub trait CidCborExt { @@ -45,6 +46,24 @@ pub enum CidVariant { V1DagCborBlake2b([u8; BLAKE2B256_SIZE]), } +impl Serialize for CidVariant { + fn serialize(&self, serializer: S) -> Result + where + S: Serializer, + { + Cid::from(self).serialize(serializer) + } +} + +impl<'de> Deserialize<'de> for CidVariant { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + Ok(Self::from(Cid::deserialize(deserializer)?)) + } +} + impl From for CidVariant { fn from(cid: Cid) -> Self { if cid.version() == Version::V1 && cid.codec() == DAG_CBOR { @@ -61,11 +80,17 @@ impl From for CidVariant { impl From for Cid { fn from(variant: CidVariant) -> Self { + Cid::from(&variant) + } +} + +impl From<&CidVariant> for Cid { + fn from(variant: &CidVariant) -> Self { match variant { - CidVariant::Generic(cid) => *cid, + CidVariant::Generic(cid) => **cid, CidVariant::V1DagCborBlake2b(digest) => Cid::new_v1( DAG_CBOR, - multihash::Multihash::wrap(Blake2b256.into(), &digest) + multihash::Multihash::wrap(Blake2b256.into(), digest) .expect("failed to convert Blake2b digest to V1 DAG-CBOR Blake2b CID"), ), } From 1b0d30698644835051c9f9c8c09de7bba5c32d7b Mon Sep 17 00:00:00 2001 From: Josh Jones Date: Mon, 14 Aug 2023 09:22:47 +0100 Subject: [PATCH 29/33] Apply suggestions from code review Co-authored-by: David Himmelstrup --- src/blocks/tipset.rs | 2 +- src/chain_sync/tipset_syncer.rs | 2 +- src/db/car/plain.rs | 2 +- src/ipld/frozen_cids.rs | 10 +--------- src/tool/subcommands/benchmark_cmd.rs | 2 +- 5 files changed, 5 insertions(+), 13 deletions(-) diff --git a/src/blocks/tipset.rs b/src/blocks/tipset.rs index 4cb924b60486..faa556c44a65 100644 --- a/src/blocks/tipset.rs +++ b/src/blocks/tipset.rs @@ -39,7 +39,7 @@ impl TipsetKeys { pub fn cid(&self) -> anyhow::Result { use fvm_ipld_encoding::RawBytes; let mut bytes = Vec::new(); - for cid in Vec::::from(&self.cids) { + for cid in &self.cids { bytes.append(&mut cid.to_bytes()) } Ok(Cid::from_cbor_blake2b256(&RawBytes::new(bytes))?) diff --git a/src/chain_sync/tipset_syncer.rs b/src/chain_sync/tipset_syncer.rs index f96ba1b0eaed..818a6a4d533a 100644 --- a/src/chain_sync/tipset_syncer.rs +++ b/src/chain_sync/tipset_syncer.rs @@ -882,7 +882,7 @@ async fn sync_headers_in_reverse PlainCar { } pub fn heaviest_tipset(&self) -> anyhow::Result { - Tipset::load_required(self, &TipsetKeys::new(self.roots().into())) + Tipset::load_required(self, &TipsetKeys::from(self.roots())) } /// In an arbitrary order diff --git a/src/ipld/frozen_cids.rs b/src/ipld/frozen_cids.rs index a2df5c998499..c1ee42b5a59c 100644 --- a/src/ipld/frozen_cids.rs +++ b/src/ipld/frozen_cids.rs @@ -38,15 +38,7 @@ impl<'a> IntoIterator for &'a FrozenCids { impl<'a> Iterator for Iter<'a> { type Item = Cid; fn next(&mut self) -> Option { - match self.cids.next() { - Some(cid) => match cid { - CidVariant::V1DagCborBlake2b(bytes) => { - Some(Cid::from(CidVariant::V1DagCborBlake2b(*bytes))) - } - CidVariant::Generic(cid) => Some(*cid.clone()), - }, - None => None, - } + self.cids.next().map(Cid::from) } } diff --git a/src/tool/subcommands/benchmark_cmd.rs b/src/tool/subcommands/benchmark_cmd.rs index b86a7b448507..82015efe7473 100644 --- a/src/tool/subcommands/benchmark_cmd.rs +++ b/src/tool/subcommands/benchmark_cmd.rs @@ -186,7 +186,7 @@ async fn benchmark_exporting( ); crate::db::car::forest::Encoder::write( &mut dest, - Vec::::from(ts.key().cids.clone()), + Vec::::from(&ts.key().cids), frames, ) .await?; From 65e9fb1db50c9409f97594736824552152b16739 Mon Sep 17 00:00:00 2001 From: Josh Jones Date: Mon, 14 Aug 2023 10:51:02 +0100 Subject: [PATCH 30/33] fix JSON issues --- src/blocks/tipset.rs | 2 +- src/lotus_json/tipset_keys.rs | 8 +++++--- src/tool/subcommands/benchmark_cmd.rs | 8 ++------ 3 files changed, 8 insertions(+), 10 deletions(-) diff --git a/src/blocks/tipset.rs b/src/blocks/tipset.rs index 9edad6f56217..d8563acddb6c 100644 --- a/src/blocks/tipset.rs +++ b/src/blocks/tipset.rs @@ -564,8 +564,8 @@ mod property_tests { #[cfg(test)] mod test { - use crate::ipld::FrozenCids; use crate::blocks::VRFProof; + use crate::ipld::FrozenCids; use crate::shim::address::Address; use cid::{ multihash::{Code::Identity, MultihashDigest}, diff --git a/src/lotus_json/tipset_keys.rs b/src/lotus_json/tipset_keys.rs index 245b3f15bfbf..c454bf5dc6d5 100644 --- a/src/lotus_json/tipset_keys.rs +++ b/src/lotus_json/tipset_keys.rs @@ -2,6 +2,8 @@ // SPDX-License-Identifier: Apache-2.0, MIT use crate::blocks::TipsetKeys; +use crate::ipld::FrozenCids; +use ::cid::Cid; use super::*; @@ -15,7 +17,7 @@ impl HasLotusJson for TipsetKeys { vec![( json!([{"/": "baeaaaaa"}]), TipsetKeys { - cids: vec![::cid::Cid::default()], + cids: FrozenCids::from(vec![::cid::Cid::default()]), }, )] } @@ -24,13 +26,13 @@ impl HasLotusJson for TipsetKeys { impl From for TipsetKeysLotusJson { fn from(value: TipsetKeys) -> Self { let TipsetKeys { cids } = value; - Self(cids.into()) + Self(VecLotusJson::::from(Vec::::from(cids))) } } impl From for TipsetKeys { fn from(value: TipsetKeysLotusJson) -> Self { let TipsetKeysLotusJson(cids) = value; - Self { cids: cids.into() } + Self { cids: FrozenCids::from(Vec::::from(cids)) } } } diff --git a/src/tool/subcommands/benchmark_cmd.rs b/src/tool/subcommands/benchmark_cmd.rs index 82015efe7473..122bf5ce5771 100644 --- a/src/tool/subcommands/benchmark_cmd.rs +++ b/src/tool/subcommands/benchmark_cmd.rs @@ -184,12 +184,8 @@ async fn benchmark_exporting( compression_level, par_buffer(1024, blocks.map_err(anyhow::Error::from)), ); - crate::db::car::forest::Encoder::write( - &mut dest, - Vec::::from(&ts.key().cids), - frames, - ) - .await?; + crate::db::car::forest::Encoder::write(&mut dest, Vec::::from(&ts.key().cids), frames) + .await?; dest.flush().await?; Ok(()) } From a276249d31c609aa1237f192a322af917d43e3f3 Mon Sep 17 00:00:00 2001 From: Josh Jones Date: Mon, 14 Aug 2023 14:26:23 +0100 Subject: [PATCH 31/33] changes from code review --- src/chain/store/chain_store.rs | 4 ++-- src/rpc/chain_api.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/chain/store/chain_store.rs b/src/chain/store/chain_store.rs index be3047a24e2b..e9eecac525bc 100644 --- a/src/chain/store/chain_store.rs +++ b/src/chain/store/chain_store.rs @@ -248,9 +248,9 @@ where file.insert(*cid); } - pub fn unmark_block_as_validated(&self, cid: Cid) { + pub fn unmark_block_as_validated(&self, cid: &Cid) { let mut file = self.validated_blocks.lock(); - let _did_work = file.remove(&cid); + let _did_work = file.remove(cid); } // FIXME: This function doesn't use the chain store at all. diff --git a/src/rpc/chain_api.rs b/src/rpc/chain_api.rs index 346bf531a868..e13661571c75 100644 --- a/src/rpc/chain_api.rs +++ b/src/rpc/chain_api.rs @@ -259,7 +259,7 @@ where for cid in ¤t.key().cids { data.state_manager .chain_store() - .unmark_block_as_validated(cid); + .unmark_block_as_validated(&cid); } let parents = current.blocks()[0].parents(); current = data.state_manager.chain_store().tipset_from_keys(parents)?; From bb0fb89036fd45ecb05753cb364e85536b81e247 Mon Sep 17 00:00:00 2001 From: Josh Jones Date: Mon, 14 Aug 2023 15:45:53 +0100 Subject: [PATCH 32/33] derive `Arbitrary` for `CidVariant` --- src/utils/cid/mod.rs | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/src/utils/cid/mod.rs b/src/utils/cid/mod.rs index 11361882c6e4..bc466749443e 100644 --- a/src/utils/cid/mod.rs +++ b/src/utils/cid/mod.rs @@ -6,6 +6,8 @@ use cid::{ Cid, Version, }; use fvm_ipld_encoding::{Error, DAG_CBOR}; +#[cfg(test)] +use quickcheck::Arbitrary; use serde::{Deserialize, Deserializer, Serialize, Serializer}; /// Extension methods for constructing `dag-cbor` [Cid] @@ -40,10 +42,14 @@ pub const BLAKE2B256_SIZE: usize = 32; /// /// The `Generic` variant is used for CIDs that do not fit into the other variants. /// These variants are used for optimizing storage of CIDs in the `FrozenCids` structure. +#[cfg_attr(test, derive(derive_quickcheck_arbitrary::Arbitrary))] #[derive(Clone, Debug, PartialEq, Eq, Hash)] pub enum CidVariant { Generic(Box), - V1DagCborBlake2b([u8; BLAKE2B256_SIZE]), + V1DagCborBlake2b( + #[cfg_attr(test, arbitrary(gen(|g: &mut quickcheck::Gen| std::array::from_fn(|_ix| Arbitrary::arbitrary(g)))))] + [u8; BLAKE2B256_SIZE], + ), } impl Serialize for CidVariant { @@ -105,27 +111,13 @@ mod tests { use crate::utils::db::CborStoreExt; use anyhow::*; use cid::{ - multihash::{self, Code, MultihashDigest}, + multihash::{Code, MultihashDigest}, Cid, }; use fvm_ipld_encoding::DAG_CBOR; - use quickcheck::Arbitrary; use quickcheck_macros::quickcheck; use std::mem::size_of; - impl Arbitrary for CidVariant { - fn arbitrary(g: &mut quickcheck::Gen) -> Self { - // Quickcheck does not reliably generate the DAG_CBOR/Blake2b variant of V1 CIDs, but we can manually create them. This method will generate each variant of `CidVariant` with equal probability. - match bool::arbitrary(g) { - true => CidVariant::Generic(Box::new(Cid::arbitrary(g))), - false => CidVariant::from(Cid::new_v1( - DAG_CBOR, - multihash::Code::Blake2b256.digest(&u64::arbitrary(g).to_be_bytes()), - )), - } - } - } - #[quickcheck] fn test_cid_cbor_ext(s: String) -> Result<()> { let cid1 = Cid::from_cbor_blake2b256(&s)?; From d7a9e31d5105b254480db20745b947c0451c4acb Mon Sep 17 00:00:00 2001 From: Josh Jones Date: Mon, 14 Aug 2023 16:45:34 +0100 Subject: [PATCH 33/33] derive `Arbitrary` for `FrozenCids` --- src/ipld/frozen_cids.rs | 31 +++++++------------------------ 1 file changed, 7 insertions(+), 24 deletions(-) diff --git a/src/ipld/frozen_cids.rs b/src/ipld/frozen_cids.rs index c1ee42b5a59c..175a7151cd68 100644 --- a/src/ipld/frozen_cids.rs +++ b/src/ipld/frozen_cids.rs @@ -12,8 +12,14 @@ use serde::{Deserialize, Serialize}; /// `Generic(Box)` variant of `CidVariant`. /// /// We use `Box<[...]>` to save memory, avoiding vector overallocation. +#[cfg_attr(test, derive(derive_quickcheck_arbitrary::Arbitrary))] #[derive(Clone, Debug, PartialEq, Eq, Hash, Serialize, Deserialize)] -pub struct FrozenCids(Box<[CidVariant]>); +pub struct FrozenCids( + #[cfg_attr(test, arbitrary(gen( + |g| Box::new([CidVariant::arbitrary(g)])) +))] + Box<[CidVariant]>, +); impl Default for FrozenCids { fn default() -> Self { @@ -98,31 +104,8 @@ impl FrozenCids { #[cfg(test)] mod test { use super::*; - use cid::multihash::{self, MultihashDigest}; - use fvm_ipld_encoding::DAG_CBOR; - use quickcheck::Arbitrary; use quickcheck_macros::quickcheck; - impl Arbitrary for FrozenCids { - fn arbitrary(g: &mut quickcheck::Gen) -> Self { - // Although the vast majority of CIDs are V1DagCborBlake2b, we want to generate the variants of FrozenCids with equal probability. - if bool::arbitrary(g) { - Vec::arbitrary(g).into_iter().collect() - } else { - // Quickcheck does not reliably generate the DAG_CBOR/Blake2b variant of V1 CIDs, but we can manually create them from an arbitrary Vec. - let vec: Vec = Vec::arbitrary(g); - vec.into_iter() - .map(|bytes| { - Cid::new_v1( - DAG_CBOR, - multihash::Code::Blake2b256.digest(&bytes.to_be_bytes()), - ) - }) - .collect() - } - } - } - #[quickcheck] fn cidvec_to_vec_of_cids_to_cidvec(cidvec: FrozenCids) { assert_eq!(cidvec, FrozenCids::from(Vec::::from(cidvec.clone())));