Serialize contract with borsh (#734)

* update args serialized with borsh (broken) * add args struct * Fixed borsh serialization * Update paths in multichain-contract test --------- Co-authored-by: Phuong Nguyen <[email protected]>
near · Jul 27, 2024 · 037e0a7 · 037e0a7
1 parent 324dd49
commit 037e0a7
Show file tree

Hide file tree

Showing 5 changed files with 58 additions and 28 deletions.
diff --git a/.github/workflows/multichain-contract.yml b/.github/workflows/multichain-contract.yml
@@ -6,16 +6,18 @@ on:
       - develop
   pull_request:
     paths:
+      - .github/workflows/multichain-contract.yml
       - chain-signatures/contract/**
+      - chain-signatures/test-contracts/**
 
 env:
   RUSTFLAGS: -D warnings
 jobs:
   test:
-    name: Test
+    name: MPC Contract Test
     strategy:
       matrix:
-        os: [ubuntu-22.04-4core]
+        os: [warp-ubuntu-latest-x64-4x]
     runs-on: ${{ matrix.os }}
 
     steps:

diff --git a/.github/workflows/multichain-integration.yml b/.github/workflows/multichain-integration.yml
@@ -13,7 +13,7 @@ env:
   RUSTFLAGS: -D warnings
 jobs:
   test:
-    name: Test
+    name: Integration Test
     strategy:
       matrix:
         # FIXME: macos-latest-xl is disabled since colima is erroring out right now

diff --git a/chain-signatures/contract/src/lib.rs b/chain-signatures/contract/src/lib.rs
@@ -26,7 +26,7 @@ use crate::config::Config;
 use crate::errors::{
     InitError, JoinError, MpcContractError, PublicKeyError, RespondError, SignError, VoteError,
 };
-use crate::update::{ProposedUpdates, UpdateId};
+use crate::update::{ProposeUpdateArgs, ProposedUpdates, UpdateId};
 
 pub use state::{
     InitializingContractState, ProtocolContractState, ResharingContractState, RunningContractState,
@@ -524,22 +524,21 @@ impl VersionedMpcContract {
     #[handle_result]
     pub fn propose_update(
         &mut self,
-        code: Option<Vec<u8>>,
-        config: Option<Config>,
+        #[serializer(borsh)] args: ProposeUpdateArgs,
     ) -> Result<UpdateId, MpcContractError> {
         // Only voters can propose updates:
         let proposer = self.voter()?;
 
         let attached = env::attached_deposit();
-        let required = ProposedUpdates::required_deposit(&code, &config);
+        let required = ProposedUpdates::required_deposit(&args.code, &args.config);
         if attached < required {
             return Err(MpcContractError::from(VoteError::InsufficientDeposit(
                 attached.as_yoctonear(),
                 required.as_yoctonear(),
             )));
         }
 
-        let Some(id) = self.proposed_updates().propose(code, config) else {
+        let Some(id) = self.proposed_updates().propose(args.code, args.config) else {
             return Err(MpcContractError::from(VoteError::Unexpected(
                 "cannot propose update due to incorrect parameters".into(),
             )));
@@ -562,6 +561,11 @@ impl VersionedMpcContract {
     /// was not found or if the voter is not a participant in the protocol.
     #[handle_result]
     pub fn vote_update(&mut self, id: UpdateId) -> Result<bool, MpcContractError> {
+        log!(
+            "vote_update: signer={}, id={:?}",
+            env::signer_account_id(),
+            id,
+        );
         let threshold = self.threshold()?;
         let voter = self.voter()?;
         let Some(votes) = self.proposed_updates().vote(&id, voter) else {

diff --git a/chain-signatures/contract/src/update.rs b/chain-signatures/contract/src/update.rs
@@ -45,6 +45,12 @@ pub enum Update {
     Contract(Vec<u8>),
 }
 
+#[derive(BorshDeserialize, BorshSerialize, Clone, Debug, Default)]
+pub struct ProposeUpdateArgs {
+    pub code: Option<Vec<u8>>,
+    pub config: Option<Config>,
+}
+
 #[derive(Debug, BorshSerialize, BorshDeserialize)]
 struct UpdateEntry {
     updates: Vec<Update>,

diff --git a/chain-signatures/contract/tests/tests.rs b/chain-signatures/contract/tests/tests.rs
@@ -13,7 +13,7 @@ use mpc_contract::errors::{self, MpcContractError};
 use mpc_contract::primitives::{
     CandidateInfo, ParticipantInfo, Participants, SignRequest, SignatureRequest,
 };
-use mpc_contract::update::UpdateId;
+use mpc_contract::update::{ProposeUpdateArgs, UpdateId};
 use near_sdk::NearToken;
 use near_workspaces::network::Sandbox;
 use near_workspaces::{Account, AccountId, Contract, Worker};
@@ -27,6 +27,29 @@ const CONTRACT_FILE_PATH: &str = "../../target/wasm32-unknown-unknown/release/mp
 const INVALID_CONTRACT: &str = "../res/mpc_test_contract.wasm";
 const PARTICIPANT_LEN: usize = 3;
 
+fn dummy_contract() -> ProposeUpdateArgs {
+    ProposeUpdateArgs {
+        code: Some(vec![1, 2, 3]),
+        config: None,
+    }
+}
+
+fn current_contract() -> ProposeUpdateArgs {
+    let new_wasm = std::fs::read(CONTRACT_FILE_PATH).unwrap();
+    ProposeUpdateArgs {
+        code: Some(new_wasm),
+        config: None,
+    }
+}
+
+fn invalid_contract() -> ProposeUpdateArgs {
+    let new_wasm = std::fs::read(INVALID_CONTRACT).unwrap();
+    ProposeUpdateArgs {
+        code: Some(new_wasm),
+        config: None,
+    }
+}
+
 fn candidates(names: Option<Vec<AccountId>>) -> HashMap<AccountId, CandidateInfo> {
     let mut candidates: HashMap<AccountId, CandidateInfo> = HashMap::new();
     let names = names.unwrap_or_else(|| {
@@ -404,17 +427,15 @@ async fn test_contract_propose_update() {
     dbg!(contract.id());
 
     test_propose_update_config(&contract, &accounts).await;
-    // _test_propose_update_contract(&contract, &accounts).await;
+    test_propose_update_contract(&contract, &accounts).await;
     test_invalid_contract_deploy(&contract, &accounts).await;
 }
 
 async fn test_propose_update_config(contract: &Contract, accounts: &[Account]) {
     // contract should not be able to propose updates unless it's a part of the participant/voter set.
     let execution = contract
         .call("propose_update")
-        .args_json(serde_json::json!({
-            "code": vec![1, 2, 3],
-        }))
+        .args_borsh((dummy_contract(),))
         .transact()
         .await
         .unwrap();
@@ -426,20 +447,22 @@ async fn test_propose_update_config(contract: &Contract, accounts: &[Account]) {
         .contains(&MpcContractError::from(errors::VoteError::VoterNotParticipant).to_string()));
 
     // have each participant propose a new update:
-    let new_config = serde_json::json!(Config {
+    let new_config = Config {
         protocol: ProtocolConfig {
             max_concurrent_introduction: 2,
             ..ProtocolConfig::default()
         },
         ..Config::default()
-    });
+    };
+
     let mut proposals = Vec::with_capacity(accounts.len());
     for account in accounts {
         let propose_execution = account
             .call(contract.id(), "propose_update")
-            .args_json(serde_json::json!({
-                "config": &new_config,
-            }))
+            .args_borsh((ProposeUpdateArgs {
+                code: None,
+                config: Some(new_config.clone()),
+            },))
             .deposit(NearToken::from_millinear(100))
             .transact()
             .await
@@ -482,6 +505,7 @@ async fn test_propose_update_config(contract: &Contract, accounts: &[Account]) {
             );
         }
     }
+    let new_config = serde_json::json!(new_config);
     // check that the proposal executed since the threshold got changed.
     let config: serde_json::Value = contract.view("config").await.unwrap().json().unwrap();
     assert_ne!(config, old_config);
@@ -499,18 +523,15 @@ async fn test_propose_update_config(contract: &Contract, accounts: &[Account]) {
     assert_eq!(config, new_config);
 }
 
-async fn _test_propose_update_contract(contract: &Contract, accounts: &[Account]) {
-    const CONTRACT_DEPLOY: NearToken = NearToken::from_near(8);
+async fn test_propose_update_contract(contract: &Contract, accounts: &[Account]) {
+    const CONTRACT_DEPLOY: NearToken = NearToken::from_millinear(8500);
     let state: mpc_contract::ProtocolContractState =
         contract.view("state").await.unwrap().json().unwrap();
 
     // Let's propose a contract update instead now.
-    let new_wasm = std::fs::read(CONTRACT_FILE_PATH).unwrap();
     let execution = accounts[0]
         .call(contract.id(), "propose_update")
-        .args_json(serde_json::json!({
-            "code": &new_wasm,
-        }))
+        .args_borsh((current_contract(),))
         .max_gas()
         .deposit(CONTRACT_DEPLOY)
         .transact()
@@ -567,12 +588,9 @@ async fn test_invalid_contract_deploy(contract: &Contract, accounts: &[Account])
         contract.view("state").await.unwrap().json().unwrap();
 
     // Let's propose a contract update instead now.
-    let new_wasm = std::fs::read(INVALID_CONTRACT).unwrap();
     let execution = accounts[0]
         .call(contract.id(), "propose_update")
-        .args_json(serde_json::json!({
-            "code": &new_wasm,
-        }))
+        .args_borsh((invalid_contract(),))
         .max_gas()
         .deposit(CONTRACT_DEPLOY)
         .transact()