Skip to content

Commit

Permalink
fix(cache): use factory deps cache correctly (#1547)
Browse files Browse the repository at this point in the history
## What ❔

Factory deps should be known only for miniblocks when it was already
published

## Checklist

<!-- Check your PR fulfills the following items. -->
<!-- For draft PRs check the boxes as you complete them. -->

- [ ] PR title corresponds to the body of PR (we generate changelog
entries from PRs).
- [ ] Tests for the changes have been added / updated.
- [ ] Documentation comments have been added / updated.
- [ ] Code has been formatted via `zk fmt` and `zk lint`.
- [ ] Spellcheck has been run via `zk spellcheck`.
- [ ] Linkcheck has been run via `zk linkcheck`.
  • Loading branch information
perekopskiy committed Apr 4, 2024
1 parent 688ad78 commit a923e11
Show file tree
Hide file tree
Showing 6 changed files with 111 additions and 42 deletions.

This file was deleted.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 6 additions & 9 deletions core/lib/dal/src/storage_web3_dal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -241,30 +241,27 @@ impl StorageWeb3Dal<'_, '_> {
Ok(row.map(|row| row.bytecode))
}

/// This method doesn't check if block with number equals to `block_number`
/// is present in the database. For such blocks `None` will be returned.
pub async fn get_factory_dep_unchecked(
/// Given bytecode hash, returns `bytecode` and `miniblock_number` at which it was inserted.
pub async fn get_factory_dep(
&mut self,
hash: H256,
block_number: MiniblockNumber,
) -> sqlx::Result<Option<Vec<u8>>> {
) -> sqlx::Result<Option<(Vec<u8>, MiniblockNumber)>> {
let row = sqlx::query!(
r#"
SELECT
bytecode
bytecode,
miniblock_number
FROM
factory_deps
WHERE
bytecode_hash = $1
AND miniblock_number <= $2
"#,
hash.as_bytes(),
i64::from(block_number.0)
)
.fetch_optional(self.storage.conn())
.await?;

Ok(row.map(|row| row.bytecode))
Ok(row.map(|row| (row.bytecode, MiniblockNumber(row.miniblock_number as u32))))
}
}

Expand Down
6 changes: 6 additions & 0 deletions core/lib/state/src/cache/lru_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,12 @@ mod tests {

use crate::cache::{lru_cache::LruCache, *};

impl CacheValue<H256> for Vec<u8> {
fn cache_weight(&self) -> u32 {
self.len().try_into().expect("Cached bytes are too large")
}
}

#[test]
fn cache_with_zero_capacity() {
let zero_cache = LruCache::<H256, Vec<u8>>::new("test", 0);
Expand Down
34 changes: 25 additions & 9 deletions core/lib/state/src/postgres/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,20 @@ mod metrics;
#[cfg(test)]
mod tests;

#[derive(Debug, Clone, PartialEq, Eq)]
struct TimestampedFactoryDep {
bytecode: Vec<u8>,
inserted_at: MiniblockNumber,
}

/// Type alias for smart contract source code cache.
type FactoryDepsCache = LruCache<H256, Vec<u8>>;
type FactoryDepsCache = LruCache<H256, TimestampedFactoryDep>;

impl CacheValue<H256> for Vec<u8> {
impl CacheValue<H256> for TimestampedFactoryDep {
fn cache_weight(&self) -> u32 {
self.len().try_into().expect("Cached bytes are too large")
(self.bytecode.len() + mem::size_of::<MiniblockNumber>())
.try_into()
.expect("Cached bytes are too large")
}
}

Expand Down Expand Up @@ -553,25 +561,33 @@ impl ReadStorage for PostgresStorage<'_> {
.as_ref()
.and_then(|caches| caches.factory_deps.get(&hash));

let result = cached_value.or_else(|| {
let value = cached_value.or_else(|| {
let mut dal = self.connection.storage_web3_dal();
let value = self
.rt_handle
.block_on(dal.get_factory_dep_unchecked(hash, self.miniblock_number))
.expect("Failed executing `load_factory_dep`");
.block_on(dal.get_factory_dep(hash))
.expect("Failed executing `load_factory_dep`")
.map(|(bytecode, inserted_at)| TimestampedFactoryDep {
bytecode,
inserted_at,
});

if let Some(caches) = &self.caches {
// If we receive None, we won't cache it.
if let Some(dep) = value.clone() {
caches.factory_deps.insert(hash, dep);
if let Some(value) = value.clone() {
caches.factory_deps.insert(hash, value);
}
};

value
});

latency.observe();
result
Some(
value
.filter(|dep| dep.inserted_at <= self.miniblock_number)?
.bytecode,
)
}

fn get_enumeration_index(&mut self, key: &StorageKey) -> Option<u64> {
Expand Down
47 changes: 46 additions & 1 deletion core/lib/state/src/postgres/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,18 @@ fn test_factory_deps_cache(pool: &ConnectionPool<Core>, rt_handle: Handle) {
)
.unwrap();

let mut contracts = HashMap::new();
contracts.insert(H256::from_low_u64_be(1), vec![1, 2, 3, 4]);
storage
.rt_handle
.block_on(
storage
.connection
.factory_deps_dal()
.insert_factory_deps(MiniblockNumber(1), &contracts),
)
.unwrap();

// Create the storage that should have the cache filled.
let mut storage = PostgresStorage::new(
storage.rt_handle,
Expand All @@ -243,7 +255,40 @@ fn test_factory_deps_cache(pool: &ConnectionPool<Core>, rt_handle: Handle) {
// Fill the cache
let dep = storage.load_factory_dep(zero_addr);
assert_eq!(dep, Some(vec![1, 2, 3]));
assert_eq!(caches.factory_deps.get(&zero_addr), Some(vec![1, 2, 3]));
assert_eq!(
caches.factory_deps.get(&zero_addr),
Some(TimestampedFactoryDep {
bytecode: vec![1, 2, 3],
inserted_at: MiniblockNumber(0)
})
);

let dep = storage.load_factory_dep(H256::from_low_u64_be(1));
assert_eq!(dep, Some(vec![1, 2, 3, 4]));
assert_eq!(
caches.factory_deps.get(&H256::from_low_u64_be(1)),
Some(TimestampedFactoryDep {
bytecode: vec![1, 2, 3, 4],
inserted_at: MiniblockNumber(1)
})
);

// Create storage with `MiniblockNumber(0)`.
let mut storage = PostgresStorage::new(
storage.rt_handle,
storage.connection,
MiniblockNumber(0),
true,
)
.with_caches(caches.clone());

// First bytecode was published at miniblock 0, so it should be visible.
let dep = storage.load_factory_dep(zero_addr);
assert_eq!(dep, Some(vec![1, 2, 3]));

// Second bytecode was published at miniblock 1, so it shouldn't be visible.
let dep = storage.load_factory_dep(H256::from_low_u64_be(1));
assert!(dep.is_none());
}

#[tokio::test]
Expand Down

0 comments on commit a923e11

Please sign in to comment.