runtime: Verify read/write sets during transaction execution

oasisprotocol · Sep 12, 2019 · fe7b0d8 · fe7b0d8
1 parent bdb1b4d
commit fe7b0d8
Show file tree

Hide file tree

Showing 9 changed files with 328 additions and 47 deletions.
diff --git a/client/src/transaction/snapshot.rs b/client/src/transaction/snapshot.rs
@@ -110,8 +110,8 @@ impl BlockSnapshot {
 }
 
 impl MKVS for BlockSnapshot {
-    fn get(&self, ctx: Context, key: &[u8]) -> Option<Vec<u8>> {
-        MKVS::get(&self.mkvs, ctx, key)
+    fn get(&mut self, ctx: Context, key: &[u8]) -> Option<Vec<u8>> {
+        MKVS::get(&mut self.mkvs, ctx, key)
     }
 
     fn insert(&mut self, _ctx: Context, _key: &[u8], _value: &[u8]) -> Option<Vec<u8>> {

diff --git a/runtime/src/dispatcher.rs b/runtime/src/dispatcher.rs
@@ -36,10 +36,12 @@ use crate::{
             },
             UrkelTree,
         },
-        StorageContext,
+        KeyValue, StorageContext,
     },
     transaction::{
-        dispatcher::Dispatcher as TxnDispatcher, tree::Tree as TxnTree, types::TxnBatch,
+        dispatcher::{DispatchStorageContext, Dispatcher as TxnDispatcher},
+        tree::Tree as TxnTree,
+        types::TxnBatch,
         Context as TxnContext,
     },
     types::{Body, ComputedBatch},
@@ -267,15 +269,19 @@ impl Dispatcher {
             },
         );
 
-        let untrusted_local = Arc::new(ProtocolUntrustedLocalStorage::new(
+        let untrusted_local: Arc<dyn KeyValue> = Arc::new(ProtocolUntrustedLocalStorage::new(
             Context::create_child(&ctx),
             protocol.clone(),
         ));
         let txn_ctx = TxnContext::new(ctx.clone(), &block.header, check_only);
-        let (mut outputs, mut tags) =
-            StorageContext::enter(&mut cache.mkvs, untrusted_local.clone(), || {
-                txn_dispatcher.dispatch_batch(&inputs, txn_ctx)
-            });
+        let (mut outputs, mut tags) = txn_dispatcher.dispatch_batch(
+            DispatchStorageContext {
+                mkvs: &mut cache.mkvs,
+                untrusted_local: &untrusted_local,
+            },
+            &inputs,
+            txn_ctx,
+        );
 
         if check_only {
             debug!(self.logger, "Transaction batch check complete");

diff --git a/runtime/src/storage/context.rs b/runtime/src/storage/context.rs
@@ -1,8 +1,8 @@
 //! Thread-local storage context.
 //!
-//! The storage context is a convenient way to share CAS and MKVS
+//! The storage context is a convenient way to share MKVS
 //! implementations across the current thread.
-use std::{cell::RefCell, sync::Arc};
+use std::{cell::RefCell, mem::transmute, sync::Arc};
 
 use super::{KeyValue, MKVS};
 
@@ -15,29 +15,30 @@ thread_local! {
     static CTX: RefCell<Option<Ctx>> = RefCell::new(None);
 }
 
-struct CtxGuard;
+struct CtxGuard {
+    old_ctx: Option<Ctx>,
+}
 
 impl CtxGuard {
-    fn new<M>(mkvs: &mut M, untrusted_local: Arc<dyn KeyValue>) -> Self
-    where
-        M: MKVS + 'static,
-    {
-        CTX.with(|ctx| {
-            assert!(ctx.borrow().is_none(), "nested enter is not allowed");
+    fn new(mkvs: &mut dyn MKVS, untrusted_local: Arc<dyn KeyValue>) -> Self {
+        let old_ctx = CTX.with(|ctx| {
             ctx.borrow_mut().replace(Ctx {
-                mkvs,
+                // Need to fake the 'static lifetime on the trait. This is fine as we know
+                // that the pointer can only actually be used while the StorageContext is
+                // live.
+                mkvs: unsafe { transmute::<&mut dyn MKVS, &'static mut dyn MKVS>(mkvs) },
                 untrusted_local,
-            });
+            })
         });
 
-        CtxGuard
+        CtxGuard { old_ctx }
     }
 }
 
 impl Drop for CtxGuard {
     fn drop(&mut self) {
         CTX.with(|local| {
-            drop(local.borrow_mut().take());
+            drop(local.replace(self.old_ctx.take()));
         });
     }
 }
@@ -47,9 +48,8 @@ pub struct StorageContext;
 
 impl StorageContext {
     /// Enter the storage context.
-    pub fn enter<M, F, R>(mkvs: &mut M, untrusted_local: Arc<dyn KeyValue>, f: F) -> R
+    pub fn enter<F, R>(mkvs: &mut dyn MKVS, untrusted_local: Arc<dyn KeyValue>, f: F) -> R
     where
-        M: MKVS + 'static,
         F: FnOnce() -> R,
     {
         let _guard = CtxGuard::new(mkvs, untrusted_local);
@@ -69,8 +69,10 @@ impl StorageContext {
             let ctx = ctx.borrow();
             let ctx_ref = ctx.as_ref().expect("must only be called while entered");
             let mkvs_ref = unsafe { ctx_ref.mkvs.as_mut().expect("pointer is never null") };
+            let untrusted_local_ref = ctx_ref.untrusted_local.clone();
+            drop(ctx);
 
-            f(mkvs_ref, &ctx_ref.untrusted_local)
+            f(mkvs_ref, &untrusted_local_ref)
         })
     }
 }
diff --git a/runtime/src/storage/mkvs/mod.rs b/runtime/src/storage/mkvs/mod.rs
@@ -109,7 +109,7 @@ impl From<Vec<u8>> for Prefix {
 /// Merklized key-value store.
 pub trait MKVS: Send + Sync {
     /// Fetch entry with given key.
-    fn get(&self, ctx: Context, key: &[u8]) -> Option<Vec<u8>>;
+    fn get(&mut self, ctx: Context, key: &[u8]) -> Option<Vec<u8>>;
 
     /// Update entry with given key.
     ///

diff --git a/runtime/src/storage/mkvs/urkel/tree/mkvs.rs b/runtime/src/storage/mkvs/urkel/tree/mkvs.rs
@@ -12,9 +12,9 @@ unsafe impl Sync for UrkelTree {}
 // TODO: We should likely change the MKVS interface to propagate errors instead of unwrapping.
 
 impl MKVS for UrkelTree {
-    fn get(&self, ctx: Context, key: &[u8]) -> Option<Vec<u8>> {
+    fn get(&mut self, ctx: Context, key: &[u8]) -> Option<Vec<u8>> {
         let _lock = self.lock.lock().unwrap();
-        self.get(ctx, key).unwrap()
+        UrkelTree::get(self, ctx, key).unwrap()
     }
 
     fn insert(&mut self, ctx: Context, key: &[u8], value: &[u8]) -> Option<Vec<u8>> {