piecrust: handle calling non-existent contracts

When the contract does exist it never gets pushed onto the callstack,
and instead results in a panic of the `c` import. This is clearly wrong
and this commit addresses that by introducing a new error path does not
revert the callstack.

piecrust/CHANGELOG.md

@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Changed
+
+- Fix behavior of calling a non-existent contract in `c` import
+
 ## [0.23.0] - 2024-08-01
 
 ### Changed

piecrust/src/error.rs

@@ -128,6 +128,7 @@ impl From<Error> for ContractError {
         match err {
             Error::OutOfGas => Self::OutOfGas,
             Error::Panic(msg) => Self::Panic(msg),
+            Error::ContractDoesNotExist(_) => Self::DoesNotExist,
             _ => Self::Unknown,
         }
     }

piecrust/src/imports.rs

@@ -245,7 +245,12 @@ pub(crate) fn c(
         div + rem
     };
 
-    let with_memory = |memory: &mut [u8]| -> Result<_, Error> {
+    enum WithMemoryError {
+        BeforePush(Error),
+        AfterPush(Error),
+    }
+
+    let with_memory = |memory: &mut [u8]| -> Result<_, WithMemoryError> {
         let arg_buf = &memory[argbuf_ofs..][..ARGBUF_LEN];
 
         let mut callee_bytes = [0; CONTRACT_ID_BYTES];
@@ -256,25 +261,31 @@ pub(crate) fn c(
 
         let callee_stack_element = env
             .push_callstack(callee_id, callee_limit)
-            .expect("pushing to the callstack should succeed");
+            .map_err(WithMemoryError::BeforePush)?;
         let callee = env
             .instance(&callee_stack_element.contract_id)
             .expect("callee instance should exist");
 
-        callee.snap().map_err(|err| Error::MemorySnapshotFailure {
-            reason: None,
-            io: Arc::new(err),
-        })?;
+        callee
+            .snap()
+            .map_err(|err| Error::MemorySnapshotFailure {
+                reason: None,
+                io: Arc::new(err),
+            })
+            .map_err(WithMemoryError::AfterPush)?;
 
-        let name = core::str::from_utf8(&memory[name_ofs..][..name_len])?;
+        let name = core::str::from_utf8(&memory[name_ofs..][..name_len])
+            .map_err(|e| WithMemoryError::AfterPush(e.into()))?;
 
         let arg = &arg_buf[..arg_len as usize];
 
         callee.write_argument(arg);
         let ret_len = callee
             .call(name, arg.len() as u32, callee_limit)
-            .map_err(Error::normalize)?;
-        check_arg(callee, ret_len as u32)?;
+            .map_err(Error::normalize)
+            .map_err(WithMemoryError::AfterPush)?;
+        check_arg(callee, ret_len as u32)
+            .map_err(WithMemoryError::AfterPush)?;
 
         // copy back result
         callee.read_argument(&mut memory[argbuf_ofs..][..ret_len as usize]);
@@ -291,7 +302,14 @@ pub(crate) fn c(
             instance.set_remaining_gas(caller_remaining - callee_spent);
             ret_len
         }
-        Err(mut err) => {
+        Err(WithMemoryError::BeforePush(err)) => {
+            let c_err = ContractError::from(err);
+            instance.with_arg_buf_mut(|buf| {
+                c_err.to_parts(buf);
+            });
+            c_err.into()
+        }
+        Err(WithMemoryError::AfterPush(mut err)) => {
             if let Err(io_err) = env.revert_callstack() {
                 err = Error::MemorySnapshotFailure {
                     reason: Some(Arc::new(err)),

piecrust/tests/deploy_with_id.rs → piecrust/tests/deploy.rs

@@ -4,8 +4,10 @@
 //
 // Copyright (c) DUSK NETWORK. All rights reserved.
 
-use piecrust::{contract_bytecode, ContractData, Error, SessionData, VM};
-use piecrust_uplink::ContractId;
+use piecrust::{
+    contract_bytecode, ContractData, ContractError, ContractId, Error,
+    SessionData, VM,
+};
 
 const OWNER: [u8; 32] = [0u8; 32];
 const LIMIT: u64 = 1_000_000;
@@ -44,3 +46,41 @@ pub fn deploy_with_id() -> Result<(), Error> {
 
     Ok(())
 }
+
+#[test]
+fn call_non_deployed() -> Result<(), Error> {
+    let vm = VM::ephemeral()?;
+
+    let bytecode = contract_bytecode!("double_counter");
+    let counter_id = ContractId::from_bytes([1; 32]);
+    let mut session = vm.session(SessionData::builder())?;
+    session.deploy(
+        bytecode,
+        ContractData::builder().owner(OWNER).contract_id(counter_id),
+        LIMIT,
+    )?;
+
+    let (value, _) = session
+        .call::<_, (i64, i64)>(counter_id, "read_values", &(), LIMIT)?
+        .data;
+    assert_eq!(value, 0xfc);
+
+    let bogus_id = ContractId::from_bytes([255; 32]);
+    let r = session
+        .call::<_, Result<(), ContractError>>(
+            counter_id,
+            "increment_left_and_call",
+            &bogus_id,
+            LIMIT,
+        )?
+        .data;
+
+    assert!(matches!(r, Err(ContractError::DoesNotExist)));
+
+    let (value, _) = session
+        .call::<_, (i64, i64)>(counter_id, "read_values", &(), LIMIT)?
+        .data;
+    assert_eq!(value, 0xfd);
+
+    Ok(())
+}