Merge pull request #4941 from namada-net/tiago/tx-wasm-panic-extract-from-guest

Pull panic info from tx wasm guest execution

Author: mergify[bot]

.changelog/unreleased/improvements/4941-tx-wasm-panic-extract-from-guest.md

@@ -0,0 +1,2 @@
+- Pull panic info from tx wasm guest execution.
+  ([\#4941](https://github.com/namada-net/namada/pull/4941))

Makefile

@@ -335,6 +335,12 @@ build-wasm-scripts:
 	make opt-wasm
 	make checksum-wasm
 
+build-wasm-scripts-with-unwind:
+	rm $(wasms)/*.wasm || true
+	make -C $(wasms) release-unwind
+	make opt-wasm
+	make checksum-wasm
+
 # Debug build the validity predicate and transactions wasm
 debug-wasm-scripts:
 	rm wasm/*.wasm || true
@@ -348,6 +354,11 @@ build-wasm-tests-scripts:
 	make -C $(wasms_for_tests)
 	make opt-wasm-tests
 
+build-wasm-tests-scripts-with-unwind:
+	rm $(wasms_for_tests)/*.wasm || true
+	make -C $(wasms_for_tests) release-unwind
+	make opt-wasm-tests
+
 # Debug build the validity predicate and transactions wasm for tests
 debug-wasm-tests-scripts:
 	rm $(wasms_for_tests)/*.wasm || true

crates/macros/src/lib.rs

@@ -52,6 +52,51 @@ pub fn transaction(_attr: TokenStream, input: TokenStream) -> TokenStream {
             // to "fake" it.
             let mut ctx = unsafe { namada_tx_prelude::Ctx::new() };
 
+            // Add a custom panic hook, in order to receive detailed
+            // information on the host of panicking transactions, to
+            // improve debugging
+            #[cfg(all(
+                target_family = "wasm",
+                feature = "debug-panic-hook"
+            ))]
+            std::panic::set_hook(Box::new(|panic_info| {
+                let mut ctx = unsafe { namada_tx_prelude::Ctx::new() };
+                ctx.yield_value({
+                    use std::fmt::Write;
+
+                    let mut buf = "Panic occurred".to_owned();
+
+                    // Append panic location
+                    if let Some(loc) = panic_info.location() {
+                        _ = write!(
+                            &mut buf,
+                            " in file {:?} at line {}",
+                            loc.file(),
+                            loc.line()
+                        );
+                    }
+
+                    // Append panic message
+                    if let Some(reason) = panic_info
+                        .payload()
+                        .downcast_ref::<&str>()
+                        .map(|&s| s)
+                        .or_else(|| {
+                            panic_info
+                                .payload()
+                                .downcast_ref::<String>()
+                                .map(String::as_str)
+                        })
+                    {
+                        _ = write!(&mut buf, ": {reason}");
+                    }
+
+                    // TODO(namada#2980): pass some proper error from txs, instead of a string
+                    buf.serialize_to_vec()
+                });
+                core::arch::wasm32::unreachable()
+            }));
+
             match #ident(&mut ctx, tx_data) {
                 Ok(()) => 1,
                 Err(err) => {

crates/vm/src/wasm/run.rs

@@ -341,13 +341,7 @@ where
     let ok = result.map_err(|err| {
         tracing::debug!("Tx WASM failed with {}", err);
 
-        match *sentinel.borrow() {
-            TxSentinel::None => Error::RuntimeError(err),
-            TxSentinel::OutOfGas => Error::GasError(err.to_string()),
-            TxSentinel::InvalidCommitment => {
-                Error::MissingSection(err.to_string())
-            }
-        }
+        extract_tx_error(&mut yielded_value, &sentinel, Some(err))
     })?;
 
     // NB: early drop this data to avoid memory errors
@@ -359,20 +353,47 @@ where
         let _store = RefCell::into_inner(store);
         Ok(verifiers)
     } else {
-        let err = yielded_value.take().map_or_else(
-            || Ok("Execution ended abruptly with an unknown error".to_owned()),
-            |borsh_encoded_err| {
-                let tx_err = TxError::try_from_slice(&borsh_encoded_err)
-                    .map_err(|e| Error::ConversionError(e.to_string()))?;
-                Ok(tx_err)
-            },
-        )?;
+        Err(extract_tx_error(&mut yielded_value, &sentinel, None))
+    }
+}
 
-        Err(match *sentinel.borrow() {
-            TxSentinel::None => Error::TxError(err),
-            TxSentinel::OutOfGas => Error::GasError(err),
-            TxSentinel::InvalidCommitment => Error::MissingSection(err),
-        })
+fn extract_tx_error(
+    yielded_value: &mut Option<Vec<u8>>,
+    sentinel: &RefCell<TxSentinel>,
+    runtime_err: Option<wasmer::RuntimeError>,
+) -> Error {
+    #[inline]
+    fn take(
+        yielded_value: &mut Option<Vec<u8>>,
+        runtime_err: Option<wasmer::RuntimeError>,
+    ) -> Error {
+        const UNKNOWN: &str = "Runtime panic caused execution to end abruptly";
+
+        let yielded_err = yielded_value.take().map(|borsh_encoded_err| {
+            TxError::try_from_slice(&borsh_encoded_err).map_err(|err| {
+                Error::ConversionError(format!(
+                    "Failed to extract yielded tx error: {err}"
+                ))
+            })
+        });
+
+        if let Some(result) = yielded_err {
+            result.map_or_else(|err| err, Error::TxError)
+        } else if let Some(err) = runtime_err {
+            Error::RuntimeError(err)
+        } else {
+            Error::TxError(UNKNOWN.to_owned())
+        }
+    }
+
+    match *sentinel.borrow() {
+        TxSentinel::OutOfGas => {
+            Error::GasError(take(yielded_value, runtime_err).to_string())
+        }
+        TxSentinel::InvalidCommitment => {
+            Error::MissingSection(take(yielded_value, runtime_err).to_string())
+        }
+        TxSentinel::None => take(yielded_value, runtime_err),
     }
 }
 

wasm/Cargo.toml

@@ -50,3 +50,8 @@ panic = "abort"
 # tell llvm to optimize for size (https://rustwasm.github.io/book/reference/code-size.html#tell-llvm-to-optimize-for-size-instead-of-speed)
 opt-level = 'z'
 strip = "debuginfo"
+
+[profile.release_unwind]
+inherits = "release"
+# unwind on panics, to extract debug information
+panic = "unwind"

wasm/Makefile

@@ -8,6 +8,10 @@ all:
 	RUSTFLAGS='-C link-arg=-s' $(cargo) build --release --target wasm32-unknown-unknown --target-dir 'target'
 	cp target/wasm32-unknown-unknown/release/*.wasm .
 
+release-unwind:
+	RUSTFLAGS='-C link-arg=-s' $(cargo) build --features debug-panic-hook --profile release_unwind --target wasm32-unknown-unknown --target-dir 'target'
+	cp target/wasm32-unknown-unknown/release_unwind/*.wasm .
+
 debug:
 	RUSTFLAGS='-C link-arg=-s' $(cargo) build --target wasm32-unknown-unknown --target-dir 'target'
 	cp target/wasm32-unknown-unknown/debug/*.wasm .
@@ -42,4 +46,4 @@ test-with-coverage:
 		-- -Z unstable-options --report-time
 
 
-.PHONY: all debug check clippy clippy-fix fmt fmt-check test clean deps
+.PHONY: all debug check clippy clippy-fix fmt fmt-check test clean deps

wasm/tx_become_validator/Cargo.toml

@@ -8,6 +8,9 @@ version.workspace = true
 
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
 
+[features]
+debug-panic-hook = []
+
 [dependencies]
 namada_tx_prelude.workspace = true
 

wasm/tx_bond/Cargo.toml

@@ -8,6 +8,9 @@ version.workspace = true
 
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
 
+[features]
+debug-panic-hook = []
+
 [dependencies]
 namada_tx_prelude.workspace = true
 

wasm/tx_change_bridge_pool/Cargo.toml

@@ -8,6 +8,9 @@ version.workspace = true
 
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
 
+[features]
+debug-panic-hook = []
+
 [dependencies]
 namada_tx_prelude.workspace = true
 

wasm/tx_change_consensus_key/Cargo.toml

@@ -8,6 +8,9 @@ version.workspace = true
 
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
 
+[features]
+debug-panic-hook = []
+
 [dependencies]
 namada_tx_prelude.workspace = true