Redundant CBOR encoding of messages during block validation

[LesnyRumcajs]

Note

This was AI found and seems correct, but do your own checks. Regardless of that, a test will be useful.

Summary

Forest re-encodes every message in a block to CBOR at least three times while validating it, and
throws each encoding away. The root cause is that Cid::from_cbor_blake2b256 materialises the whole
encoding into a Vec<u8> purely to feed a hasher, and neither Message nor SignedMessage caches
its CID.

No consensus risk in fixing this: the changes below alter only how the bytes are produced, never
what they are.

Where the encodings happen

src/utils/cid/mod.rs, the root of the problem:

fn from_cbor_blake2b256<S: serde::ser::Serialize>(obj: &S) -> Result<Cid, Error> {
    let bytes = fvm_ipld_encoding::to_vec(obj)?;          // full Vec allocation
    Ok(Self::from_cbor_encoded_raw_bytes_blake2b256(&bytes))  // hash it, drop it
}

Message::cid() (src/shim/message.rs:188) and SignedMessage::cid()
(src/message/signed_message.rs:83) both route through it. Neither memoises.

Per BLS message, inside one check_block_messages call
(src/chain_sync/tipset_syncer.rs):

#	Site	Fate of the bytes
1	tipset_syncer.rs:397 — m.cid().to_bytes(), for the BLS aggregate check	hashed, dropped
2	tipset_syncer.rs:424 — to_vec(msg)?.len(), for price_list.on_chain_message	length read, dropped
3	tipset_syncer.rs:511 → TipsetValidator::compute_msg_root → Cid::from_cbor_blake2b256	hashed, dropped

Per SECP message, three as well, in different shapes:

#	Site	What is encoded
1	tipset_syncer.rs:493 — check_msg(msg.message(), ..) → to_vec	the inner unsigned Message
2	src/shim/crypto.rs:160 — msg.message().cid().to_bytes() inside authenticate_msg	the inner Message again
3	tipset_syncer.rs:511 → compute_msg_root	the full SignedMessage

Delegated (EIP-155/1559) signatures cost two more: src/shim/crypto.rs:147 compares
msg.message().cid() == filecoin_msg.cid(), building both sides from scratch.

It is worse across a block's lifetime

compute_msg_root is reached from three independent places on the same data, with no memoisation
between them:

• src/chain_sync/validation.rs:101 — gossip block validation
• src/blocks/tipset.rs:613 — FullTipset::persist
• src/chain_sync/tipset_syncer.rs:511 — check_block_messages

A block that arrives over gossip, gets persisted, then gets fully validated re-encodes every message
it contains three additional times.

Proposed work

1. Stream into the hasher instead of buffering (highest value/effort ratio)

Change Cid::from_cbor_blake2b256 in src/utils/cid/mod.rs to serialise directly into the Blake2b
digest via fvm_ipld_encoding::to_writer and a small io::Write adapter around the hasher.

• Removes one heap allocation per call, sized to the encoded object.
• No API change, no call-site churn.
• Blast radius is much wider than block validation: 17 non-test call sites, plus every
  Message::cid() / SignedMessage::cid() in the mempool, chain store, and eth-mapping paths.

Keep from_cbor_encoded_raw_bytes_blake2b256 as-is — it has callers that already hold the bytes.

2. Count bytes without allocating

to_vec(x)?.len() appears at:

• src/chain_sync/tipset_syncer.rs:424
• src/interpreter/vm.rs:433 and :463 (these run per message per epoch for every FVM
  version, not just the pre-nv18 path — the most valuable of the set)
• src/message_pool/msgpool/msg_pool.rs:587
• src/libp2p/chain_exchange/provider.rs:348
• src/message/signed_message.rs:88 (SignedMessage::chain_length)

A counting io::Write sink passed to fvm_ipld_encoding::to_writer makes all of them
allocation-free. Consider exposing it as crate::utils::encoding::encoded_len(&impl Serialize).

3. Cache the CID on the message (separate, larger change)

The deeper fix, and the one that actually removes the 3×. CachingBlockHeader already does exactly
this for headers with a OnceCell; Message has no equivalent.

Bigger blast radius — Message is widely used and derives Clone/Hash/PartialEq, so a cache
field needs care (skip it in comparisons and hashing, and in the serde impls). Worth doing only
after 1 and 2 are measured. Note the subtlety already documented at
src/message/signed_message.rs:75-78: SignedMessage::cid() is not
Cid::from_cbor_blake2b256(signed_msg) for BLS messages — it delegates to the inner message. Any
cache has to preserve that.

Measuring

check_block_messages runs on validated sync, not on snapshot import, so the headline win is live
sync and forest-tool validation runs — not import throughput. Measure before claiming
otherwise.

Suggested: forest-tool benchmark against a calibnet snapshot, plus a targeted criterion bench over
compute_msg_root on a realistic block (mainnet blocks carry up to BLOCK_MESSAGE_LIMIT = 10000
messages; typical tipsets are in the hundreds to low thousands).

Correctness is easy to pin down: CIDs and roots must be byte-identical before and after. Existing
coverage in src/chain_sync/validation.rs:372,386 (compute_msg_root) and the
src/message/signed_message.rs CID tests should catch a regression, and both changes are
mechanical enough that a differential test over arbitrary Message values is cheap to add.

Suggested split

• PR 1: items 1 and 2. Mechanical, independently benchmarkable, no consensus risk.
• PR 2: item 3, only if 1 and 2 leave a measurable gap.

[0xDevNinja]

Would like to take this, scoped to PR 1 (items 1 and 2 from the split).

Plan:

• Change Cid::from_cbor_blake2b256 to serialise straight into the Blake2b hasher through fvm_ipld_encoding::to_writer with a small io::Write adapter, dropping the throwaway Vec. from_cbor_encoded_raw_bytes_blake2b256 stays as is for callers that already hold the bytes.
• Add an allocation free encoded_len helper in crate::utils::encoding backed by a counting io::Write sink, and route the to_vec(x).len() sites through it (tipset_syncer, vm.rs, msg_pool, chain_exchange provider, SignedMessage::chain_length).
• Assert CIDs and encoded lengths stay byte identical before and after with a differential test over arbitrary Message and SignedMessage values, alongside the existing compute_msg_root and CID coverage.

Leaving item 3 (caching the CID on Message) as a follow up once 1 and 2 are measured, like you suggested. Happy to wait for the go ahead before pushing anything.