perf: memoize a message's CID and encoded length

src/chain_sync/validation.rs

@@ -8,11 +8,12 @@ use crate::chain::ChainStore;
 use crate::message::SignedMessage;
 use crate::shim::clock::ChainEpoch;
 use crate::shim::message::Message;
-use crate::utils::{cid::CidCborExt, db::CborStoreExt};
+use crate::utils::db::CborStoreExt;
 use cid::Cid;
 use fil_actors_shared::fvm_ipld_amt::{Amtv0 as Amt, Error as IpldAmtError};
 use fvm_ipld_blockstore::Blockstore;
 use fvm_ipld_encoding::Error as EncodingError;
+use itertools::Itertools as _;
 use thiserror::Error;
 
 use crate::chain_sync::bad_block_cache::{BadBlockCache, SeenBlockCache};
@@ -140,15 +141,11 @@ impl TipsetValidator<'_> {
         bls_msgs: &[Message],
         secp_msgs: &[SignedMessage],
     ) -> Result<Cid, TipsetValidationError> {
-        // Generate message CIDs
-        let bls_cids = bls_msgs
-            .iter()
-            .map(Cid::from_cbor_blake2b256)
-            .collect::<Result<Vec<Cid>, fvm_ipld_encoding::Error>>()?;
+        let bls_cids = bls_msgs.iter().map(Message::cid).collect_vec();
         let secp_cids = secp_msgs
             .iter()
-            .map(Cid::from_cbor_blake2b256)
-            .collect::<Result<Vec<Cid>, fvm_ipld_encoding::Error>>()?;
+            .map(SignedMessage::signed_cid)
+            .collect_vec();
 
         // Generate Amt and batch set message values
         let bls_message_root = Amt::new_from_iter(blockstore, bls_cids)?;

src/interpreter/vm.rs

@@ -22,7 +22,6 @@ use crate::shim::{
     state_tree::ActorState,
     version::NetworkVersion,
 };
-use crate::utils::encoding::calc_encoded_len;
 use ahash::{HashMap, HashSet};
 use anyhow::bail;
 use fvm_ipld_encoding::RawBytes;
@@ -457,7 +456,7 @@ impl VM {
         msg.message().check()?;
 
         let unsigned = msg.message().clone();
-        let raw_length = calc_encoded_len(msg)?;
+        let raw_length = msg.encoded_len();
         let ret: ApplyRet = match self {
             VM::VM2(fvm_executor) => {
                 let ret = fvm_executor.execute_message(

src/message/chain_message.rs

@@ -49,6 +49,15 @@ impl ChainMessage {
         delegate_chain_message!(self.cid())
     }
 
+    /// The length of this message's own encoding, signature included. Not
+    /// [`MessageRead::chain_length`], which drops a BLS signature.
+    pub fn encoded_len(&self) -> usize {
+        match self {
+            Self::Unsigned(msg) => msg.encoded_len(),
+            Self::Signed(msg) => msg.signed_encoded_len(),
+        }
+    }
+
     /// Tests if a message is equivalent to another replacing message.
     /// A replacing message is a message with a different CID,
     /// any of Gas values, and different signature, but with all
@@ -102,6 +111,46 @@ impl MessageReadWrite for ChainMessage {
 #[cfg(test)]
 mod tests {
     use super::*;
+    use crate::shim::crypto::BLS_SIG_LEN;
+    use crate::utils::encoding::calc_encoded_len;
+    use quickcheck_macros::quickcheck;
+
+    /// The memo travels with the `Arc`, so the derived `PartialEq`/`Hash` must stay blind to it
+    /// here too.
+    #[quickcheck]
+    fn computing_the_memo_is_invisible(msg: SignedMessage) -> bool {
+        use std::hash::Hasher as _;
+        let hash_of = |value: &ChainMessage| {
+            let mut hasher = std::collections::hash_map::DefaultHasher::new();
+            std::hash::Hash::hash(value, &mut hasher);
+            hasher.finish()
+        };
+        // Two independent `Arc`s: cloning a `ChainMessage` would alias one memo and warm both.
+        let cold: ChainMessage = msg.clone().into();
+        let warm: ChainMessage = msg.into();
+        let _ = warm.encoded_len();
+        warm == cold && hash_of(&warm) == hash_of(&cold)
+    }
+
+    /// `encoded_len` must measure the bytes this value serializes to, for every variant: the
+    /// `#[serde(untagged)]` encoding is the inner message's, signature included.
+    #[quickcheck]
+    fn encoded_len_measures_the_serialized_bytes(msg: Message, sig_bytes: Vec<u8>) -> bool {
+        let signatures = [
+            Signature::new_secp256k1(sig_bytes.clone()),
+            Signature::new(SignatureType::Delegated, sig_bytes),
+            Signature::new_bls(vec![0; BLS_SIG_LEN]),
+        ];
+        let unsigned: ChainMessage = msg.clone().into();
+        let mut candidates = std::iter::once(unsigned).chain(
+            signatures
+                .into_iter()
+                .map(|sig| SignedMessage::new_unchecked(msg.clone(), sig).into()),
+        );
+        candidates.all(|chain_msg: ChainMessage| {
+            chain_msg.encoded_len() == calc_encoded_len(&chain_msg).unwrap()
+        })
+    }
 
     fn dummy_msg() -> Message {
         Message::builder()

src/message/signed_message.rs

@@ -9,16 +9,37 @@ use crate::shim::{
     econ::TokenAmount,
     message::Message,
 };
-use crate::utils::encoding::calc_encoded_len;
-use fvm_ipld_encoding::tuple::*;
+use crate::utils::cid::{EncodedCbor, Memo};
 use get_size2::GetSize;
 
 /// Represents a wrapped message with signature bytes.
 #[cfg_attr(test, derive(derive_quickcheck_arbitrary::Arbitrary))]
-#[derive(Clone, Debug, PartialEq, Eq, Hash, GetSize, Serialize_tuple, Deserialize_tuple)]
+#[derive(Clone, Debug, PartialEq, Eq, Hash, GetSize)]
 pub struct SignedMessage {
     message: Message,
     signature: Signature,
+    /// The memoized CID and length of the signed encoding. Cleared whenever the message or
+    /// signature is replaced, so it cannot outlive what it was derived from, and excluded from
+    /// the derived `PartialEq`/`Eq`/`Hash`/`Debug` by [`Memo`].
+    #[cfg_attr(test, arbitrary(gen(|_| Memo::default())))]
+    encoded: Memo,
+}
+
+impl serde::Serialize for SignedMessage {
+    fn serialize<S: serde::Serializer>(&self, s: S) -> Result<S::Ok, S::Error> {
+        (&self.message, &self.signature).serialize(s)
+    }
+}
+
+impl<'de> serde::Deserialize<'de> for SignedMessage {
+    fn deserialize<D: serde::Deserializer<'de>>(deserializer: D) -> Result<Self, D::Error> {
+        let (message, signature) = serde::Deserialize::deserialize(deserializer)?;
+        Ok(Self {
+            message,
+            signature,
+            encoded: Memo::default(),
+        })
+    }
 }
 
 impl SignedMessage {
@@ -32,7 +53,11 @@ impl SignedMessage {
     /// Generate a new signed message from fields.
     /// The signature will not be verified.
     pub fn new_unchecked(message: Message, signature: Signature) -> SignedMessage {
-        SignedMessage { message, signature }
+        SignedMessage {
+            message,
+            signature,
+            encoded: Memo::default(),
+        }
     }
 
     /// Returns reference to the unsigned message.
@@ -46,10 +71,13 @@ impl SignedMessage {
     }
 
     pub fn set_signature(&mut self, signature: Signature) {
+        self.encoded.clear();
         self.signature = signature;
     }
 
+    /// Drops the memo up front, since the caller may change anything the signed encoding is derived from.
     pub fn message_mut(&mut self) -> &mut Message {
+        self.encoded.clear();
         &mut self.message
     }
 
@@ -90,11 +118,30 @@ impl SignedMessage {
         if self.is_bls() {
             self.message.cid()
         } else {
-            use crate::utils::cid::CidCborExt;
-            cid::Cid::from_cbor_blake2b256(self).expect("message serialization is infallible")
+            self.encoded().cid()
         }
     }
 
+    /// The CID of the signed encoding, which is what a block's `SECP` message root is built from.
+    /// Differs from [`SignedMessage::cid`] only for a BLS signature, which a `SECP` message list
+    /// may not carry.
+    pub fn signed_cid(&self) -> cid::Cid {
+        self.encoded().cid()
+    }
+
+    /// The length of the signed encoding, which is what a message's size on the wire is measured
+    /// against. Differs from [`MessageRead::chain_length`] for a BLS signature, which is excluded
+    /// from the chain length but not from the message itself.
+    pub fn signed_encoded_len(&self) -> usize {
+        self.encoded().byte_len()
+    }
+
+    fn encoded(&self) -> &EncodedCbor {
+        self.encoded.get_or_init(|| {
+            EncodedCbor::compute(self).expect("message serialization is infallible")
+        })
+    }
+
     /// Creates a mock signed message for testing purposes. The signature check will fail if
     /// invoked.
     #[cfg(test)]
@@ -111,9 +158,9 @@ impl MessageRead for SignedMessage {
     fn chain_length(&self) -> anyhow::Result<usize> {
         Ok(match self.signature.signature_type() {
             // BLS chain message length doesn't include the signature
-            SignatureType::Bls => calc_encoded_len(&self.message)?,
+            SignatureType::Bls => self.message.encoded_len(),
             // SECP and Delegated chain message length includes the signature
-            SignatureType::Secp256k1 | SignatureType::Delegated => calc_encoded_len(self)?,
+            SignatureType::Secp256k1 | SignatureType::Delegated => self.encoded().byte_len(),
         })
     }
     fn from(&self) -> Address {
@@ -170,6 +217,28 @@ mod tests {
     use fvm_ipld_encoding::to_vec;
     use quickcheck_macros::quickcheck;
 
+    fn hash_of<T: std::hash::Hash>(value: &T) -> u64 {
+        use std::hash::Hasher as _;
+        let mut hasher = std::collections::hash_map::DefaultHasher::new();
+        value.hash(&mut hasher);
+        hasher.finish()
+    }
+
+    /// `Arbitrary` always yields cold memos, so both must be warmed here to reach the case the
+    /// derived `PartialEq`/`Hash`/`Debug` must stay blind to.
+    #[quickcheck]
+    fn computing_the_memos_is_invisible(msg: SignedMessage) -> bool {
+        let warm = msg.clone();
+        let (_, _) = (warm.cid(), warm.signed_cid());
+        warm == msg && hash_of(&warm) == hash_of(&msg) && format!("{warm:?}") == format!("{msg:?}")
+    }
+
+    #[quickcheck]
+    fn signed_cid_and_len_match_the_unmemoized_path(msg: SignedMessage) -> bool {
+        msg.signed_cid() == Cid::from_cbor_blake2b256(&msg).unwrap()
+            && msg.signed_encoded_len() == to_vec(&msg).unwrap().len()
+    }
+
     #[track_caller]
     fn assert_measures_and_hashes(signed: &SignedMessage, encoded: &[u8]) {
         assert_eq!(signed.chain_length().unwrap(), encoded.len());
@@ -202,6 +271,26 @@ mod tests {
         }
     }
 
+    /// Pins the hand-written [`serde`] code to the `Serialize_tuple` derive it replaced, which
+    /// no round-trip test can do on its own.
+    #[quickcheck]
+    fn signed_encoding_matches_the_tuple_derive(msg: SignedMessage) -> bool {
+        use fvm_ipld_encoding::tuple::*;
+
+        #[derive(Serialize_tuple)]
+        struct Reference<'a> {
+            message: &'a Message,
+            signature: &'a Signature,
+        }
+
+        to_vec(&msg).unwrap()
+            == to_vec(&Reference {
+                message: msg.message(),
+                signature: msg.signature(),
+            })
+            .unwrap()
+    }
+
     /// The signature type selects one encoding for both the CID and the chain length, so the two
     /// cannot be allowed to disagree about which bytes they mean.
     #[quickcheck]

src/message_pool/msgpool/msg_pool.rs

@@ -34,7 +34,6 @@ use crate::shim::{
 use crate::state_manager::IdToAddressCache;
 use crate::state_manager::utils::is_valid_for_sending;
 use crate::utils::cache::SizeTrackingCache;
-use crate::utils::encoding::calc_encoded_len;
 use ahash::HashSet;
 use futures::StreamExt;
 use fvm_ipld_encoding::to_vec;
@@ -584,7 +583,7 @@ where
 }
 
 fn validate_static(msg: &SignedMessage) -> Result<(), Error> {
-    if calc_encoded_len(msg)? > MAX_MESSAGE_SIZE {
+    if msg.signed_encoded_len() > MAX_MESSAGE_SIZE {
         return Err(Error::MessageTooBig);
     }
     let to = msg.message().to();