Skip to content

Commit

Permalink
Merge branch 'tomas/pos-vp-no-catch' (#2145)
Browse files Browse the repository at this point in the history
* origin/tomas/pos-vp-no-catch:
  changelog: add #2145
  protocol: remove `panic::catch_unwind` for PoS VP
  • Loading branch information
Gianmarco Fraccaroli authored and brentstone committed Nov 20, 2023
2 parents ad6a789 + 517c81a commit 651d603
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 46 deletions.
2 changes: 2 additions & 0 deletions .changelog/unreleased/miscellaneous/2145-pos-vp-no-catch.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- Removed catching of panics from PoS VP.
([\#2145](https://github.com/anoma/namada/pull/2145))
25 changes: 0 additions & 25 deletions shared/src/ledger/pos/vp.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
//! Proof-of-Stake native validity predicate.
use std::collections::BTreeSet;
use std::panic::{RefUnwindSafe, UnwindSafe};

use namada_core::ledger::storage_api::governance;
// use borsh::BorshDeserialize;
Expand Down Expand Up @@ -58,30 +57,6 @@ where
}
}

// TODO this is temporarily to run PoS native VP in a new thread to avoid
// crashing the ledger (in apps/src/lib/node/ledger/protocol/mod.rs). The
// RefCells contained within PosVP are not thread-safe, but each thread has its
// own instances.
impl<DB, H, CA> UnwindSafe for PosVP<'_, DB, H, CA>
where
DB: 'static + ledger_storage::DB + for<'iter> ledger_storage::DBIter<'iter>,
H: 'static + StorageHasher,
CA: 'static + WasmCacheAccess,
{
}

// TODO this is temporarily to run PoS native VP in a new thread to avoid
// crashing the ledger (in apps/src/lib/node/ledger/protocol/mod.rs). The
// RefCells contained within PosVP are not thread-safe, but each thread has its
// own instances.
impl<DB, H, CA> RefUnwindSafe for PosVP<'_, DB, H, CA>
where
DB: 'static + ledger_storage::DB + for<'iter> ledger_storage::DBIter<'iter>,
H: 'static + StorageHasher,
CA: 'static + WasmCacheAccess,
{
}

impl<'a, DB, H, CA> NativeVp for PosVP<'a, DB, H, CA>
where
DB: 'static + ledger_storage::DB + for<'iter> ledger_storage::DBIter<'iter>,
Expand Down
28 changes: 7 additions & 21 deletions shared/src/ledger/protocol/mod.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
//! The ledger's protocol
use std::collections::BTreeSet;
use std::panic;

use borsh_ext::BorshSerializeExt;
use eyre::{eyre, WrapErr};
Expand Down Expand Up @@ -857,26 +856,13 @@ where
// and `RefUnwindSafe` in
// shared/src/ledger/pos/vp.rs)
let keys_changed_ref = &keys_changed;
let result =
match panic::catch_unwind(move || {
pos_ref
.validate_tx(
tx,
keys_changed_ref,
verifiers_addr_ref,
)
.map_err(Error::PosNativeVpError)
}) {
Ok(result) => result,
Err(err) => {
tracing::error!(
"PoS native VP failed with \
{:#?}",
err
);
Err(Error::PosNativeVpRuntime)
}
};
let result = pos_ref
.validate_tx(
tx,
keys_changed_ref,
verifiers_addr_ref,
)
.map_err(Error::PosNativeVpError);
// Take the gas meter and sentinel
// back
// out of the context
Expand Down

0 comments on commit 651d603

Please sign in to comment.