test: add integration tests for Upgradable (#79)

* Add integration tests

* Add Deserialize to derived traits

* Remove unit tests

* Remove dead util code for unit tests

All plugins are now tested in integration tests.

* Remove outdated paragraph from README

README.md

@@ -112,9 +112,6 @@ Tests should verify that once the macros provided by this crate are expanded, th
     - Compiles and deploys the contract on chain via [NEAR `workspaces`](https://docs.rs/workspaces/0.7.0/workspaces/).
     - Sends transactions to the deployed contract to verify plugin functionality.
 
-> **Note**
-> Currently some plugins are still tested by unit tests in `near-plugins/src/<plugin_name>.rs`, not by integration tests. Migrating these unit tests to integration tests as described above is WIP.
-
 ## Contributors Notes
 
 Traits doesn't contain any implementation, even though some interfaces are self-contained enough to have it.

near-plugins/src/lib.rs

@@ -4,8 +4,6 @@ pub mod events;
 pub mod full_access_key_fallback;
 pub mod ownable;
 pub mod pausable;
-#[cfg(not(target_arch = "wasm32"))]
-mod test_utils;
 pub mod upgradable;
 
 pub use access_control_role::AccessControlRole;

near-plugins/src/upgradable.rs

@@ -44,8 +44,8 @@
 //! [batch transaction]: https://docs.near.org/concepts/basics/transactions/overview
 //! [time between scheduling and execution]: https://docs.near.org/sdk/rust/promises/intro
 use crate::events::{AsEvent, EventMetadata};
+use near_sdk::serde::Serialize;
 use near_sdk::{AccountId, CryptoHash, Promise};
-use serde::Serialize;
 
 /// Trait describing the functionality of the _Upgradable_ plugin.
 pub trait Upgradable {
@@ -106,66 +106,3 @@ impl AsEvent<DeployCode> for DeployCode {
         }
     }
 }
-
-#[cfg(not(target_arch = "wasm32"))]
-#[cfg(test)]
-mod tests {
-    // TODO: Make simulation test that verifies code is deployed
-    use crate as near_plugins;
-    use crate::test_utils::get_context;
-    use crate::{Ownable, Upgradable};
-    use near_sdk::env::sha256;
-    use near_sdk::{near_bindgen, testing_env, VMContext};
-    use std::convert::TryInto;
-
-    #[near_bindgen]
-    #[derive(Ownable, Upgradable)]
-    struct Counter;
-
-    #[near_bindgen]
-    impl Counter {
-        /// Specify the owner of the contract in the constructor
-        #[init]
-        fn new() -> Self {
-            let mut contract = Self {};
-            contract.owner_set(Some(near_sdk::env::predecessor_account_id()));
-            contract
-        }
-    }
-
-    /// Setup basic account. Owner of the account is `eli.test`
-    fn setup_basic() -> (Counter, VMContext) {
-        let ctx = get_context();
-        testing_env!(ctx.clone());
-        let mut counter = Counter::new();
-        counter.owner_set(Some("eli.test".to_string().try_into().unwrap()));
-        (counter, ctx)
-    }
-
-    #[test]
-    #[should_panic(expected = r#"Ownable: Method must be called from owner"#)]
-    fn test_stage_code_not_owner() {
-        let (mut counter, _) = setup_basic();
-        counter.up_stage_code(vec![1]);
-    }
-
-    #[test]
-    fn test_stage_code() {
-        let (mut counter, mut ctx) = setup_basic();
-
-        ctx.predecessor_account_id = "eli.test".to_string().try_into().unwrap();
-        testing_env!(ctx);
-
-        assert_eq!(counter.up_staged_code(), None);
-        counter.up_stage_code(vec![1]);
-
-        assert_eq!(counter.up_staged_code(), Some(vec![1]));
-
-        assert_eq!(
-            counter.up_staged_code_hash(),
-            Some(sha256(vec![1].as_slice()).try_into().unwrap())
-        );
-
-        counter.up_deploy_code();
-    }
-}

near-plugins/tests/common/mod.rs

@@ -3,4 +3,5 @@ pub mod full_access_key_fallback_contract;
 pub mod ownable_contract;
 pub mod pausable_contract;
 pub mod repo;
+pub mod upgradable_contract;
 pub mod utils;

near-plugins/tests/common/upgradable_contract.rs

@@ -0,0 +1,81 @@
+use near_sdk::serde_json::json;
+use near_sdk::CryptoHash;
+use near_sdk::Duration;
+use workspaces::result::ExecutionFinalResult;
+use workspaces::{Account, Contract};
+
+/// Wrapper for a contract that derives `Upgradable`. It allows implementing helpers for calling
+/// contract methods provided by `Upgradable`.
+pub struct UpgradableContract {
+    contract: Contract,
+}
+
+impl UpgradableContract {
+    pub fn new(contract: Contract) -> Self {
+        Self { contract }
+    }
+
+    pub fn contract(&self) -> &Contract {
+        &self.contract
+    }
+
+    pub async fn up_stage_code(
+        &self,
+        caller: &Account,
+        code: Vec<u8>,
+    ) -> workspaces::Result<ExecutionFinalResult> {
+        caller
+            .call(self.contract.id(), "up_stage_code")
+            .args_borsh(code)
+            .max_gas()
+            .transact()
+            .await
+    }
+
+    pub async fn up_staged_code(&self, caller: &Account) -> anyhow::Result<Option<Vec<u8>>> {
+        let res = caller
+            .call(self.contract.id(), "up_staged_code")
+            .max_gas()
+            .transact()
+            .await?;
+        Ok(res.borsh::<Option<Vec<u8>>>()?)
+    }
+
+    pub async fn up_staged_code_hash(
+        &self,
+        caller: &Account,
+    ) -> anyhow::Result<Option<CryptoHash>> {
+        let res = caller
+            .call(self.contract.id(), "up_staged_code_hash")
+            .max_gas()
+            .transact()
+            .await?;
+        Ok(res.json::<Option<CryptoHash>>()?)
+    }
+
+    /// The `Promise` returned by trait method `up_deploy_code` is resolved in the `workspaces`
+    /// transaction.
+    pub async fn up_deploy_code(
+        &self,
+        caller: &Account,
+    ) -> workspaces::Result<ExecutionFinalResult> {
+        caller
+            .call(self.contract.id(), "up_deploy_code")
+            .max_gas()
+            .transact()
+            .await
+    }
+
+    pub async fn up_init_staging_duration(
+        &self,
+        caller: &Account,
+        staging_duration: Duration,
+    ) -> workspaces::Result<ExecutionFinalResult> {
+        caller
+            .call(self.contract.id(), "up_init_staging_duration")
+            .args_json(json!({ "staging_duration": staging_duration }))
+            .max_gas()
+            .transact()
+            .await
+    }
+}

near-plugins/tests/common/utils.rs

@@ -1,15 +1,39 @@
 use near_sdk::serde::de::DeserializeOwned;
+use near_sdk::Duration;
 use std::cmp::PartialEq;
 use std::fmt::Debug;
-use workspaces::result::ExecutionFinalResult;
+use std::str::FromStr;
+use workspaces::network::Sandbox;
+use workspaces::result::{ExecutionFinalResult, ExecutionOutcome};
+use workspaces::{AccountId, Block, Worker};
+
+/// Converts `account_id` to a `near_sdk::AccountId` and panics on failure.
+///
+/// Only available in tests, hence favoring simplicity over efficiency.
+pub fn as_sdk_account_id(account_id: &AccountId) -> near_sdk::AccountId {
+    near_sdk::AccountId::from_str(account_id.as_str())
+        .expect("Conversion to near_sdk::AccountId should succeed")
+}
+
+/// Convenience function to create a new `near_sdk::Duration`. Panics if the conversion fails.
+pub fn sdk_duration_from_secs(seconds: u64) -> Duration {
+    std::time::Duration::from_secs(seconds)
+        .as_nanos()
+        .try_into()
+        .expect("Conversion from std Duration to near_sdk Duration should succeed")
+}
 
 /// Asserts execution was successful and returned `()`.
 pub fn assert_success_with_unit_return(res: ExecutionFinalResult) {
-    assert!(res.is_success(), "Transaction should have succeeded");
-    assert!(
-        res.raw_bytes().unwrap().is_empty(),
-        "Unexpected return value"
-    );
+    match res.into_result() {
+        Ok(res) => {
+            assert!(
+                res.raw_bytes().unwrap().is_empty(),
+                "Unexpected return value"
+            );
+        }
+        Err(err) => panic!("Transaction should have succeeded but failed with: {err}"),
+    }
 }
 
 /// Asserts execution was successful and returned the `expected` value.
@@ -136,3 +160,44 @@ pub fn assert_failure_with(res: ExecutionFinalResult, must_contain: &str) {
         err,
     );
 }
+
+/// Returns the block timestamp in nanoseconds. Panics on failure.
+async fn block_timestamp(worker: &Worker<Sandbox>) -> u64 {
+    worker
+        .view_block()
+        .await
+        .expect("Should view block")
+        .timestamp()
+}
+
+/// Returns the block in which a transaction or receipt was included.
+pub async fn get_transaction_block(
+    worker: &Worker<Sandbox>,
+    result: &ExecutionOutcome,
+) -> workspaces::Result<Block> {
+    let block_hash = result.block_hash;
+    worker.view_block().block_hash(block_hash).await
+}
+
+/// [Time travels] `worker` forward by at least `duration`. This is achieved by a very naive
+/// approach: fast forward blocks until `duration` has passed. Keeping it simple since this function
+/// is available only in tests.
+///
+/// Due to this approach, it is recommended to pass only relatively small values as `duration`. Fast
+/// forwarding provided by this function is reasonly fast in our tests for durations that correspond
+/// to less than 100 seconds.
+///
+/// [Time travels]: https://github.com/near/workspaces-rs#time-traveling
+pub async fn fast_forward_beyond(worker: &Worker<Sandbox>, duration: Duration) {
+    let initial_timestamp = block_timestamp(worker).await;
+
+    // Estimating a number of blocks to skip based on `duration` and calling `fast_forward` only
+    // once seems more efficient. However, that leads to jittery tests as `fast_forward` may _not_
+    // forward the block timestamp significantly.
+    while block_timestamp(worker).await - initial_timestamp < duration {
+        worker
+            .fast_forward(1)
+            .await
+            .expect("Fast forward should succeed");
+    }
+}

near-plugins/tests/contracts/upgradable/Cargo.toml

@@ -0,0 +1,21 @@
+[package]
+name = "upgradable"
+version = "0.0.0"
+edition = "2018"
+
+[lib]
+crate-type = ["cdylib", "rlib"]
+
+[dependencies]
+near-plugins = { path = "../../../../near-plugins" }
+near-sdk = "4.1.0"
+
+[profile.release]
+codegen-units = 1
+opt-level = "z"
+lto = true
+debug = false
+panic = "abort"
+overflow-checks = true
+
+[workspace]

near-plugins/tests/contracts/upgradable/Makefile

@@ -0,0 +1,8 @@
+build:
+	cargo build --target wasm32-unknown-unknown --release
+
+# Helpful for debugging. Requires `cargo-expand`.
+expand:
+	cargo expand > expanded.rs
+
+.PHONY: build expand

near-plugins/tests/contracts/upgradable/rust-toolchain

@@ -0,0 +1,3 @@
+[toolchain]
+channel = "1.66.1"
+components = ["clippy", "rustfmt"]

near-plugins/tests/contracts/upgradable/src/lib.rs

@@ -0,0 +1,36 @@
+use near_plugins::{Ownable, Upgradable};
+use near_sdk::borsh::{self, BorshDeserialize, BorshSerialize};
+use near_sdk::{near_bindgen, AccountId, PanicOnDefault};
+
+/// Deriving `Upgradable` requires the contract to be `Ownable.`
+#[near_bindgen]
+#[derive(Ownable, Upgradable, PanicOnDefault, BorshDeserialize, BorshSerialize)]
+pub struct Contract;
+
+#[near_bindgen]
+impl Contract {
+    /// Parameter `owner` allows setting the owner in the constructor if an `AccountId` is provided.
+    /// If `owner` is `None`, no owner will be set in the constructor. After contract initialization
+    /// it is possible to set an owner with `Ownable::owner_set`.
+    ///
+    /// Parameter `staging_duration` allows initializing the time that is required to pass between
+    /// staging and deploying code. This delay provides a safety mechanism to protect users against
+    /// unfavorable or malicious code upgrades. If `staging_duration` is `None`, no staging duration
+    /// will be set in the constructor. It is possible to set it later using
+    /// `Upgradable::up_init_staging_duration`. If no staging duration is set, it defaults to zero,
+    /// allowing immediate deployments of staged code.
+    ///
+    /// Since this constructor uses an `*_unchecked` method, it should be combined with code
+    /// deployment in a batch transaction.
+    #[init]
+    pub fn new(owner: Option<AccountId>) -> Self {
+        let mut contract = Self;
+
+        // Optionally set the owner.
+        if owner.is_some() {
+            contract.owner_set(owner);
+        }
+
+        contract
+    }
+}