Add linera_cache::Arc<T> newtype enforcing one-allocation-per-content invariant#6330
Open
ndr-ds wants to merge 7 commits into
Open
Add linera_cache::Arc<T> newtype enforcing one-allocation-per-content invariant#6330ndr-ds wants to merge 7 commits into
linera_cache::Arc<T> newtype enforcing one-allocation-per-content invariant#6330ndr-ds wants to merge 7 commits into
Conversation
ma2bd
reviewed
May 18, 2026
| /// | ||
| /// Use [`Arc::into_std`] or [`Arc::as_std`] to interoperate with APIs that | ||
| /// require [`std::sync::Arc<T>`] explicitly. | ||
| #[repr(transparent)] |
Contributor
There was a problem hiding this comment.
To be clear, this line only matters for unsafe transmute operations, which we don't use, correct?
Contributor
Author
There was a problem hiding this comment.
Good point, let me remove
1cb8555 to
c33a57b
Compare
afck
approved these changes
May 19, 2026
| pub fn insert_arc(&self, key: &K, value: Arc<V>) -> Arc<V> { | ||
| self.dedup_insert(key, value) | ||
| pub fn insert(&self, key: &K, value: V) -> crate::Arc<V> { | ||
| crate::Arc(self.dedup_insert(key, Arc::new(value))) |
Contributor
There was a problem hiding this comment.
(Wouldn't it be easier to keep crate::Arcs in the map, rather than (std) Arcs? Then we wouldn't have to explicitly create crate::Arcs everywhere.)
| remote_node | ||
| .handle_confirmed_certificate(certificate, CrossChainMessageDelivery::NonBlocking) | ||
| .handle_confirmed_certificate( | ||
| certificate.into_std(), |
Contributor
There was a problem hiding this comment.
Wouldn't it be safer (i.e. less prone to accidental duplicates) if handle_confirmed_certificate also required a CacheArc?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Follow-up to #6147 (reviewed by @ma2bd). The previous PR pushed
Arcdeeper through the certificate/block paths but left the door open for callers to bypass the dedup guarantee by constructing a freshArc::new(value)and inserting it — the type alone couldn't enforce "this came from the cache".Proposal
Introduce
linera_cache::Arc<T>: a#[repr(transparent)]newtype overstd::sync::Arc<T>with no public constructor. The only way to obtain one is throughValueCache::insert,ValueCache::insert_hashed, orValueCache::get. This makes the dedup invariant structural: if you hold aCacheArc<T>, it provably came from the cache.API surface:
into_std(self) -> std::sync::Arc<T>— for crossing trait/network/serde boundariesas_std(&self) -> &std::sync::Arc<T>— for read-only boundary crossingsunwrap_or_clone(this: Self) -> T— mirrorsArc::unwrap_or_cloneDeref<Target = T>,Clone,From<CacheArc<T>> for std::sync::Arc<T>Re-exported from
linera-storageaspub use linera_cache::Arc(aliased asCacheArcinside consuming crates).Test Plan
CI — existing tests exercise the cache paths and the type changes are mechanical (boundary conversions via
into_std()/as_std()).Release Plan
Links