Skip to content

Add linera_cache::Arc<T> newtype enforcing one-allocation-per-content invariant#6330

Open
ndr-ds wants to merge 7 commits into
mainfrom
ndr-ds/arc-newtype
Open

Add linera_cache::Arc<T> newtype enforcing one-allocation-per-content invariant#6330
ndr-ds wants to merge 7 commits into
mainfrom
ndr-ds/arc-newtype

Conversation

@ndr-ds
Copy link
Copy Markdown
Contributor

@ndr-ds ndr-ds commented May 18, 2026

Motivation

Follow-up to #6147 (reviewed by @ma2bd). The previous PR pushed Arc deeper through the certificate/block paths but left the door open for callers to bypass the dedup guarantee by constructing a fresh Arc::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 over std::sync::Arc<T> with no public constructor. The only way to obtain one is through ValueCache::insert, ValueCache::insert_hashed, or ValueCache::get. This makes the dedup invariant structural: if you hold a CacheArc<T>, it provably came from the cache.

API surface:

  • into_std(self) -> std::sync::Arc<T> — for crossing trait/network/serde boundaries
  • as_std(&self) -> &std::sync::Arc<T> — for read-only boundary crossings
  • unwrap_or_clone(this: Self) -> T — mirrors Arc::unwrap_or_clone
  • Deref<Target = T>, Clone, From<CacheArc<T>> for std::sync::Arc<T>

Re-exported from linera-storage as pub use linera_cache::Arc (aliased as CacheArc inside 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

  • Nothing to do / These changes follow the usual release cycle.

Links

Comment thread linera-cache/src/arc.rs Outdated
///
/// Use [`Arc::into_std`] or [`Arc::as_std`] to interoperate with APIs that
/// require [`std::sync::Arc<T>`] explicitly.
#[repr(transparent)]
Copy link
Copy Markdown
Contributor

@ma2bd ma2bd May 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be clear, this line only matters for unsafe transmute operations, which we don't use, correct?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, let me remove

Copy link
Copy Markdown
Contributor

@ma2bd ma2bd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice

@ndr-ds ndr-ds force-pushed the ndr-ds/arc-newtype branch from 1cb8555 to c33a57b Compare May 18, 2026 18:40
@ma2bd ma2bd added this pull request to the merge queue May 19, 2026
Comment thread linera-cache/src/value_cache.rs Outdated
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)))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(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(),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it be safer (i.e. less prone to accidental duplicates) if handle_confirmed_certificate also required a CacheArc?

@ma2bd ma2bd removed this pull request from the merge queue due to a manual request May 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants