Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reimplement transactions as a stack and not through revert operations #335

Merged
merged 3 commits into from
Mar 22, 2023
Merged

Reimplement transactions as a stack and not through revert operations #335

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9c87b87
Reimplement transactions as a stack and not through revert operations
tomaka Mar 22, 2023
7949bcd
CHANGELOG
tomaka Mar 22, 2023
5607868
Merge branch 'main' into fix-327
tomaka Mar 22, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
159 changes: 71 additions & 88 deletions lib/src/executor/runtime_host.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ use crate::{

use alloc::{borrow::ToOwned as _, string::String, vec::Vec};
use core::fmt;
use hashbrown::{hash_map::Entry, HashMap, HashSet};
use hashbrown::HashSet;

pub use trie::TrieEntryVersion;

Expand Down Expand Up @@ -100,7 +100,7 @@ pub fn run(
.into(),
main_trie_changes: config.storage_main_trie_changes,
state_trie_version,
main_trie_transaction_revert: Vec::new(),
main_trie_transaction: Vec::new(),
offchain_storage_changes: config.offchain_storage_changes,
main_trie_root_calculation_cache: Some(
config.main_trie_root_calculation_cache.unwrap_or_default(),
Expand Down Expand Up @@ -384,26 +384,22 @@ impl PrefixKeys {

drop(after_overlay);

for key in keys_to_remove {
self.inner
.main_trie_root_calculation_cache
.as_mut()
.unwrap()
.storage_value_update(&key, false);
let (diff, calculation_cache) = self
.inner
.main_trie_transaction
.last_mut()
.map(|(a, b)| (a, b))
.unwrap_or((
&mut self.inner.main_trie_changes,
self.inner
.main_trie_root_calculation_cache
.as_mut()
.unwrap(),
));

let previous_value = self
.inner
.main_trie_changes
.diff_insert_erase(key.clone(), ())
.map(|(v, _)| v);

if let Some(main_trie_transaction_revert) =
self.inner.main_trie_transaction_revert.last_mut()
{
if let Entry::Vacant(entry) = main_trie_transaction_revert.entry(key) {
entry.insert(previous_value);
}
}
for key in keys_to_remove {
calculation_cache.storage_value_update(&key, false);
diff.diff_insert_erase(key.clone(), ());
}

self.inner.vm = req.resume(keys_removed_so_far, some_keys_remain);
Expand Down Expand Up @@ -615,14 +611,13 @@ struct Inner {
/// Pending changes to the top storage trie that this execution performs.
main_trie_changes: storage_diff::TrieDiff,

/// Contains pending storage reverts if and only if we're within a storage transaction.
/// When changes are applied to [`Inner::main_trie_changes`], the reverse operation is
/// added here.
/// Contains pending storage changes that haven't been applied to [`Inner::main_trie_changes`]
/// yet.
/// When the storage transaction ends, either this is applied to [`Inner::main_trie_changes`],
/// or is entirely discarded (to revert changes).
///
/// When the storage transaction ends, either this hash map is entirely discarded (to commit
/// changes), or applied to [`Inner::main_trie_changes`] (to revert).
main_trie_transaction_revert:
Vec<HashMap<Vec<u8>, Option<Option<Vec<u8>>>, fnv::FnvBuildHasher>>,
/// Contains a `Vec` in case transactions are stacked.
main_trie_transaction: Vec<(storage_diff::TrieDiff, calculate_root::CalculationCache)>,

/// State trie version indicated by the runtime. All the storage changes that are performed
/// use this version.
Expand Down Expand Up @@ -700,27 +695,20 @@ impl Inner {
host::HostVm::ExternalStorageSet(req) => {
// TODO: this is a dummy implementation and child tries are not implemented properly
if let host::StorageKey::MainTrie { key } = req.key() {
self.main_trie_root_calculation_cache
.as_mut()
.unwrap()
.storage_value_update(key.as_ref(), req.value().is_some());

let previous_value = if let Some(value) = req.value() {
self.main_trie_changes
.diff_insert(key.as_ref(), value.as_ref(), ())
let (diff, calculation_cache) = self
.main_trie_transaction
.last_mut()
.map(|(a, b)| (a, b))
.unwrap_or((
&mut self.main_trie_changes,
self.main_trie_root_calculation_cache.as_mut().unwrap(),
));

calculation_cache.storage_value_update(key.as_ref(), req.value().is_some());
if let Some(value) = req.value() {
diff.diff_insert(key.as_ref(), value.as_ref(), ());
} else {
self.main_trie_changes.diff_insert_erase(key.as_ref(), ())
}
.map(|(v, ())| v);

if let Some(main_trie_transaction_revert) =
self.main_trie_transaction_revert.last_mut()
{
if let Entry::Vacant(entry) =
main_trie_transaction_revert.entry(key.as_ref().to_vec())
{
entry.insert(previous_value);
}
diff.diff_insert_erase(key.as_ref(), ());
}
}

Expand All @@ -738,31 +726,22 @@ impl Inner {
}
};

self.main_trie_root_calculation_cache
.as_mut()
.unwrap()
.storage_value_update(key.as_ref(), true);
let (diff, calculation_cache) = self
.main_trie_transaction
.last_mut()
.map(|(a, b)| (a, b))
.unwrap_or((
&mut self.main_trie_changes,
self.main_trie_root_calculation_cache.as_mut().unwrap(),
));

let current_value = self
.main_trie_changes
.diff_get(key.as_ref())
.map(|(v, _)| v);
calculation_cache.storage_value_update(key.as_ref(), true);

let current_value = diff.diff_get(key.as_ref()).map(|(v, _)| v);
if let Some(current_value) = current_value {
let mut current_value = current_value.unwrap_or_default().to_vec();
append_to_storage_value(&mut current_value, req.value().as_ref());
let previous_value = self
.main_trie_changes
.diff_insert(key.as_ref().to_vec(), current_value, ())
.map(|(v, _)| v);
if let Some(main_trie_transaction_revert) =
self.main_trie_transaction_revert.last_mut()
{
if let Entry::Vacant(entry) =
main_trie_transaction_revert.entry(key.as_ref().to_vec())
{
entry.insert(previous_value);
}
}
diff.diff_insert(key.as_ref().to_vec(), current_value, ());
drop(key);
self.vm = req.resume();
} else {
Expand Down Expand Up @@ -900,31 +879,35 @@ impl Inner {
}

host::HostVm::StartStorageTransaction(tx) => {
self.main_trie_transaction_revert.push(Default::default());
self.main_trie_transaction.push((
storage_diff::TrieDiff::default(),
self.main_trie_root_calculation_cache
.as_ref()
.unwrap()
.clone(),
));

self.vm = tx.resume();
}

host::HostVm::EndStorageTransaction { resume, rollback } => {
// The inner implementation guarantees that a storage transaction can only
// end if it has earlier been started.
debug_assert!(!self.main_trie_transaction_revert.is_empty());
let last = self.main_trie_transaction_revert.pop().unwrap();

if rollback {
for (key, value) in last {
if let Some(value) = value {
if let Some(value) = value {
let _ = self.main_trie_changes.diff_insert(key, value, ());
} else {
let _ = self.main_trie_changes.diff_insert_erase(key, ());
}
} else {
let _ = self.main_trie_changes.diff_remove(&key);
}
}

// TODO: very slow; do this properly
self.main_trie_root_calculation_cache = Some(Default::default());
debug_assert!(!self.main_trie_transaction.is_empty());
let (diff, cache_update) = self.main_trie_transaction.pop().unwrap();

if !rollback {
let (previous_diff, previous_calculation_cache) = self
.main_trie_transaction
.last_mut()
.map(|(a, b)| (a, b))
.unwrap_or((
&mut self.main_trie_changes,
self.main_trie_root_calculation_cache.as_mut().unwrap(),
));

previous_diff.merge(&diff);
*previous_calculation_cache = cache_update;
}

self.vm = resume.resume();
Expand Down
3 changes: 2 additions & 1 deletion lib/src/trie/calculate_root.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,14 +75,15 @@ use core::{fmt, iter};
///
/// If the storage's content is modified, you **must** call the appropriate methods to invalidate
/// entries. Otherwise, the trie root calculation will yield an incorrect result.
#[derive(Clone)]
pub struct CalculationCache {
/// Structure of the trie.
/// If `Some`, the structure is either fully conforming to the trie.
structure: Option<trie_structure::TrieStructure<CacheEntry>>,
}

/// Custom data stored in each node in [`CalculationCache::structure`].
#[derive(Default)]
#[derive(Default, Clone)]
struct CacheEntry {
merkle_value: Option<trie_node::MerkleValueOutput>,
}
Expand Down
4 changes: 4 additions & 0 deletions wasm-node/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@
- Add an arbitrary limit to the size of unprocessed networking packets, in order to avoid DoS attacks. This limit is necessary in order to bypass limitations in the networking APIs exposed by browsers. ([#312](https://github.com/smol-dot/smoldot/pull/312))
- Rename `/webrtc` to `/webrtc-direct` in multiaddresses, in accordance with the rest of the libp2p ecosystem. ([#326](https://github.com/smol-dot/smoldot/pull/326))

### Fixed

- Fix runtime transactions not being handled properly when multiple transactions are stacked. ([#335](https://github.com/smol-dot/smoldot/pull/335))

## 1.0.0 - 2022-03-12

### Fixed
Expand Down