Merge pull request #339 from dusk-network/optional-owner-336

piecrust: optional owner in `Session::migrate`

Author: Eduardo Leegwater Simões

piecrust/CHANGELOG.md

@@ -7,8 +7,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Added
+
+- Add `ContractDataBuilder::owner` to allow for setting the owner of a contract
+  on deploy time [#336]
+
 ### Changed
 
+- Change `migrate` to take the owner of the contract being replaced if it is
+  not set by the caller [#336]
+- Make `owner` field optional in `ContractData` and `ContractDataBuilder` [#336]
+- Change `ContractData` and `ContractDataBuilder` to take a `Vec<u8>` as owner
+  instead of `[u8; N]` [#336]
 - Use empty constructor arguments by default [#316]
 - Upgrade `dusk-wasmtime` to version `18`
 
@@ -32,7 +42,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Changed
 
 - Change `owner` import to accept the contract ID as argument and return
-  non-zero upon success 
+  non-zero upon success
 
 ## [0.14.1] - 2024-01-11
 
@@ -72,20 +82,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## Added
 
-- Add `Session::memory_pages` allowing for inclusion  proofs of pages [#273]
+- Add `Session::memory_pages` allowing for inclusion proofs of pages [#273]
 
 ## Changed
 
-- Change state tree to  distinguish between 32 and 64 bit smart contracts [#273]
+- Change state tree to distinguish between 32 and 64 bit smart contracts [#273]
 
 ## [0.12.0] - 2023-11-01
 
 ## Added
 
 - Support `memory64` smart contracts [#281]
 - Add some `Error` variants:
-  * `InvalidFunction`
-  * `InvalidMemory`
+    * `InvalidFunction`
+    * `InvalidMemory`
 - Add `once_cell` dependency
 
 ## Changed
@@ -104,19 +114,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Remove 4 page - 256KiB - minimum memory requirement for contracts
 - Remove `Clone` derivation for `Error`
 - Remove some `Error` variants, along with `From` implementations:
-  * `CompileError`
-  * `DeserializeError`
-  * `ExportError`
-  * `InstantiationError`
-  * `InvalidFunctionSignature`
-  * `MemorySetupError`
-  * `ParsingError`
-  * `SerializeError`
-  * `Trap`
+    * `CompileError`
+    * `DeserializeError`
+    * `ExportError`
+    * `InstantiationError`
+    * `InvalidFunctionSignature`
+    * `MemorySetupError`
+    * `ParsingError`
+    * `SerializeError`
+    * `Trap`
 
 ## Fixed
 
--  Fix  loading of compiled contracts from  state transported from different
+- Fix loading of compiled contracts from state transported from different
   platforms [#287]
 
 ## [0.11.0] - 2023-10-11
@@ -190,8 +200,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed
 
-- Change commit write behavior to  write dirty pages instead of diffs [#253]
-- Change memory backend  to use `crumbles` instead of `libc` directly  [#253]
+- Change commit write behavior to write dirty pages instead of diffs [#253]
+- Change memory backend to use `crumbles` instead of `libc` directly  [#253]
 
 ### Removed
 
@@ -209,13 +219,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed
 
-- Change imports 
+- Change imports
 - Change diffing algorithm to not delegate growth to `bsdiff`
 - Change memory growth algorithm to not require copying to temp file
 
 ### Fixed
 
-- Fix  behavior of imports on  out of bounds pointers [#249]
+- Fix behavior of imports on out of bounds pointers [#249]
 - Fix SIGBUS caused by improper memory growth
 
 ## [0.7.0] - 2023-07-19
@@ -310,7 +320,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed
 
-- Change `owner` parameter type in `ModuleData::builder` to be `[u8; N]` [#201] 
+- Change `owner` parameter type in `ModuleData::builder` to be `[u8; N]` [#201]
 
 ### Fixed
 
@@ -362,9 +372,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - First `piecrust` release
 
 <!-- PULLS -->
+
 [#234]: https://github.com/dusk-network/piecrust/pull/234
 
 <!-- ISSUES -->
+
+[#336]: https://github.com/dusk-network/piecrust/issues/336
 [#325]: https://github.com/dusk-network/piecrust/issues/325
 [#324]: https://github.com/dusk-network/piecrust/issues/324
 [#316]: https://github.com/dusk-network/piecrust/issues/316
@@ -400,6 +413,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 [#93]: https://github.com/dusk-network/piecrust/issues/93
 
 <!-- VERSIONS -->
+
 [Unreleased]: https://github.com/dusk-network/piecrust/compare/piecrust-0.16.0...HEAD
 [0.16.0]: https://github.com/dusk-network/piecrust/compare/piecrust-0.15.0...piecrust-0.16.0
 [0.15.0]: https://github.com/dusk-network/piecrust/compare/piecrust-0.14.1...piecrust-0.15.0

piecrust/src/contract.rs

@@ -13,59 +13,63 @@ use rkyv::{Archive, Deserialize, Serialize};
 
 use crate::error::Error;
 
-pub struct ContractData<'a, A, const N: usize> {
+pub struct ContractData<'a, A> {
     pub(crate) contract_id: Option<ContractId>,
     pub(crate) constructor_arg: Option<&'a A>,
-    pub(crate) owner: [u8; N],
+    pub(crate) owner: Option<Vec<u8>>,
 }
 
 // `()` is done on purpose, since by default it should be that the constructor
 // takes no argument.
-impl<'a, const N: usize> ContractData<'a, (), N> {
+impl<'a> ContractData<'a, ()> {
     /// Build a deploy data structure.
     ///
     /// This function returns a builder that can be used to set optional fields
     /// in contract deployment.
-    pub fn builder(owner: [u8; N]) -> ContractDataBuilder<'a, (), N> {
+    pub fn builder() -> ContractDataBuilder<'a, ()> {
         ContractDataBuilder {
             contract_id: None,
             constructor_arg: None,
-            owner,
+            owner: None,
         }
     }
 }
 
-impl<'a, A, const N: usize> From<ContractDataBuilder<'a, A, N>>
-    for ContractData<'a, A, N>
-{
-    fn from(builder: ContractDataBuilder<'a, A, N>) -> Self {
+impl<'a, A> From<ContractDataBuilder<'a, A>> for ContractData<'a, A> {
+    fn from(builder: ContractDataBuilder<'a, A>) -> Self {
         builder.build()
     }
 }
 
-pub struct ContractDataBuilder<'a, A, const N: usize> {
+pub struct ContractDataBuilder<'a, A> {
     contract_id: Option<ContractId>,
-    owner: [u8; N],
+    owner: Option<Vec<u8>>,
     constructor_arg: Option<&'a A>,
 }
 
-impl<'a, A, const N: usize> ContractDataBuilder<'a, A, N> {
+impl<'a, A> ContractDataBuilder<'a, A> {
     /// Set the deployment contract ID.
     pub fn contract_id(mut self, id: ContractId) -> Self {
         self.contract_id = Some(id);
         self
     }
 
     /// Set the constructor argument for deployment.
-    pub fn constructor_arg<B>(self, arg: &B) -> ContractDataBuilder<B, N> {
+    pub fn constructor_arg<B>(self, arg: &B) -> ContractDataBuilder<B> {
         ContractDataBuilder {
             contract_id: self.contract_id,
             owner: self.owner,
             constructor_arg: Some(arg),
         }
     }
 
-    pub fn build(self) -> ContractData<'a, A, N> {
+    /// Set the owner of the contract.
+    pub fn owner(mut self, owner: impl Into<Vec<u8>>) -> Self {
+        self.owner = Some(owner.into());
+        self
+    }
+
+    pub fn build(self) -> ContractData<'a, A> {
         ContractData {
             contract_id: self.contract_id,
             constructor_arg: self.constructor_arg,

piecrust/src/lib.rs

@@ -89,7 +89,13 @@
 //! const LIMIT: u64 = 1_000_000;
 //!
 //! let mut session = vm.session(SessionData::builder()).unwrap();
-//! let counter_id = session.deploy(contract_bytecode!("counter"), ContractData::builder(OWNER), LIMIT).unwrap();
+//! let counter_id = session
+//!     .deploy(
+//!         contract_bytecode!("counter"),
+//!         ContractData::builder().owner(OWNER),
+//!         LIMIT,
+//!     )
+//!     .unwrap();
 //!
 //! assert_eq!(session.call::<_, i64>(counter_id, "read_value", &(), LIMIT).unwrap().data, 0xfc);
 //! session.call::<_, ()>(counter_id, "increment", &(), LIMIT).unwrap();

piecrust/src/session.rs

@@ -34,6 +34,7 @@ const MAX_META_SIZE: usize = ARGBUF_LEN;
 pub const INIT_METHOD: &str = "init";
 
 unsafe impl Send for Session {}
+
 unsafe impl Sync for Session {}
 
 /// A running mutation to a state.
@@ -208,15 +209,18 @@ impl Session {
     ///
     /// [`ContractId`]: ContractId
     /// [`PersistenceError`]: PersistenceError
-    pub fn deploy<'a, A, D, const N: usize>(
+    ///
+    /// # Panics
+    /// If `deploy_data` does not specify an owner, this will panic.
+    pub fn deploy<'a, A, D>(
         &mut self,
         bytecode: &[u8],
         deploy_data: D,
         gas_limit: u64,
     ) -> Result<ContractId, Error>
     where
         A: 'a + for<'b> Serialize<StandardBufSerializer<'b>>,
-        D: Into<ContractData<'a, A, N>>,
+        D: Into<ContractData<'a, A>>,
     {
         let mut deploy_data = deploy_data.into();
 
@@ -247,7 +251,9 @@ impl Session {
             contract_id,
             bytecode,
             constructor_arg,
-            deploy_data.owner.to_vec(),
+            deploy_data
+                .owner
+                .expect("Owner must be specified when deploying a contract"),
             gas_limit,
         )?;
 
@@ -400,11 +406,19 @@ impl Session {
     /// given `contract` ID, and the old contract will be removed from the
     /// state.
     ///
+    /// If the `owner` of a contract is not set, it will be set to the owner of
+    /// the contract being replaced. If it is set, then it will be used as the
+    /// new owner.
+    ///
     /// # Errors
     /// The migration may error during execution for a myriad of reasons. The
     /// caller is encouraged to drop the `Session` should an error occur as it
     /// will more than likely be left in an inconsistent state.
-    pub fn migrate<'a, A, D, F, const N: usize>(
+    ///
+    /// # Panics
+    /// If the owner of the new contract is not set in `deploy_data`, and the
+    /// contract being replaced does not exist, this will panic.
+    pub fn migrate<'a, A, D, F>(
         mut self,
         contract: ContractId,
         bytecode: &[u8],
@@ -414,11 +428,28 @@ impl Session {
     ) -> Result<Self, Error>
     where
         A: 'a + for<'b> Serialize<StandardBufSerializer<'b>>,
-        D: Into<ContractData<'a, A, N>>,
+        D: Into<ContractData<'a, A>>,
         F: FnOnce(ContractId, &mut Session) -> Result<(), Error>,
     {
+        let mut new_contract_data = deploy_data.into();
+
+        // If the contract being replaced exists, and the caller did not specify
+        // an owner, set the owner to the owner of the contract being replaced.
+        if let Some(old_contract_data) = self
+            .inner
+            .contract_session
+            .contract(contract)
+            .map_err(|err| PersistenceError(Arc::new(err)))?
+        {
+            if new_contract_data.owner.is_none() {
+                new_contract_data.owner =
+                    Some(old_contract_data.metadata.data().owner.clone());
+            }
+        }
+
         let new_contract =
-            self.deploy(bytecode, deploy_data, deploy_gas_limit)?;
+            self.deploy(bytecode, new_contract_data, deploy_gas_limit)?;
+
         closure(new_contract, &mut self)?;
 
         self.inner

piecrust/tests/box.rs

@@ -18,7 +18,7 @@ pub fn box_set_get() -> Result<(), Error> {
 
     let id = session.deploy(
         contract_bytecode!("box"),
-        ContractData::builder(OWNER),
+        ContractData::builder().owner(OWNER),
         LIMIT,
     )?;
 
@@ -43,7 +43,7 @@ pub fn box_set_get_raw() -> Result<(), Error> {
 
     let id = session.deploy(
         contract_bytecode!("box"),
-        ContractData::builder(OWNER),
+        ContractData::builder().owner(OWNER),
         LIMIT,
     )?;