Merge pull request #380 from dusk-network/contract-does-not-exist

Fix behavior on calling non-existent contracts

contracts/double_counter/src/lib.rs

@@ -10,6 +10,7 @@
 #![no_std]
 
 use piecrust_uplink as uplink;
+use uplink::{ContractError, ContractId};
 
 /// Struct that describes the state of the DoubleCounter contract
 pub struct DoubleCounter {
@@ -40,6 +41,20 @@ impl DoubleCounter {
         let value = self.right_value + 1;
         self.right_value = value;
     }
+
+    /// Increment the counter by 1 and call the given contract, with the given
+    /// arguments.
+    ///
+    /// This is intended to test the behavior of the contract on calling a
+    /// contract that doesn't exist.
+    pub fn increment_left_and_call(
+        &mut self,
+        contract: ContractId,
+    ) -> Result<(), ContractError> {
+        let value = self.left_value + 1;
+        self.left_value = value;
+        uplink::call(contract, "hello", &())
+    }
 }
 
 /// Expose `Counter::read_value()` to the host
@@ -59,3 +74,11 @@ unsafe fn increment_left(arg_len: u32) -> u32 {
 unsafe fn increment_right(arg_len: u32) -> u32 {
     uplink::wrap_call(arg_len, |_: ()| STATE.increment_right())
 }
+
+/// Expose `Counter::increment_and_call()` to the host
+#[no_mangle]
+unsafe fn increment_left_and_call(arg_len: u32) -> u32 {
+    uplink::wrap_call_unchecked(arg_len, |arg| {
+        STATE.increment_left_and_call(arg)
+    })
+}

piecrust-uplink/CHANGELOG.md

@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Added
+
+- Add `ContractError::DoesNotExist` variant
+
 ## [0.16.0] - 2024-08-01
 
 ### Added

piecrust-uplink/src/error.rs

@@ -27,6 +27,7 @@ use core::str;
 pub enum ContractError {
     Panic(String),
     OutOfGas,
+    DoesNotExist,
     Unknown,
 }
 
@@ -57,6 +58,7 @@ impl ContractError {
         match code {
             -1 => Self::Panic(get_msg(slice)),
             -2 => Self::OutOfGas,
+            -3 => Self::DoesNotExist,
             i32::MIN => Self::Unknown,
             _ => unreachable!("The host must guarantee that the code is valid"),
         }
@@ -81,6 +83,7 @@ impl ContractError {
                 -1
             }
             Self::OutOfGas => -2,
+            Self::DoesNotExist => -3,
             Self::Unknown => i32::MIN,
         }
     }
@@ -91,6 +94,7 @@ impl From<ContractError> for i32 {
         match err {
             ContractError::Panic(_) => -1,
             ContractError::OutOfGas => -2,
+            ContractError::DoesNotExist => -3,
             ContractError::Unknown => i32::MIN,
         }
     }
@@ -101,6 +105,9 @@ impl Display for ContractError {
         match self {
             ContractError::Panic(msg) => write!(f, "Panic: {msg}"),
             ContractError::OutOfGas => write!(f, "OutOfGas"),
+            ContractError::DoesNotExist => {
+                write!(f, "Contract does not exist")
+            }
             ContractError::Unknown => write!(f, "Unknown"),
         }
     }

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(())
+}