From b848bed03c770c3a5279fa3265df4f01d030b9ef Mon Sep 17 00:00:00 2001 From: templexxx Date: Thu, 23 Jun 2022 23:01:02 +0800 Subject: [PATCH 1/6] fix wrong behavior: submit multisig txn only there are enough signatures --- cmd/starcoin/src/cli_state.rs | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/cmd/starcoin/src/cli_state.rs b/cmd/starcoin/src/cli_state.rs index 74ad343dbb..64539f4445 100644 --- a/cmd/starcoin/src/cli_state.rs +++ b/cmd/starcoin/src/cli_state.rs @@ -304,7 +304,7 @@ impl CliState { AccountPublicKey::Multi(m) => m.clone(), }; - let _ = self.sign_multisig_txn_to_file_or_submit( + let multisig_txn_path = self.sign_multisig_txn_to_file_or_submit( sender.address, multisig_public_key, None, @@ -314,6 +314,11 @@ impl CliState { blocking, ); + eprintln!( + "multisig-txn: {}", + multisig_txn_path.unwrap().as_path().to_str().unwrap() + ); + Ok(execute_result) } @@ -365,7 +370,10 @@ impl CliState { merged_signatures.threshold(), merged_signatures.signatures().len() ); - if !merged_signatures.is_enough() { + + let signatures_is_enough = merged_signatures.is_enough(); + + if !signatures_is_enough { eprintln!( "still require {} signatures", merged_signatures.threshold() as usize - merged_signatures.signatures().len() @@ -383,7 +391,7 @@ impl CliState { SignedUserTransaction::new(partial_signed_txn.into_raw_transaction(), authenticator) }; - if submit { + if submit && signatures_is_enough { let _ = self.submit_txn(signed_txn, blocking); return Ok(PathBuf::new()); } From eec81f29e5bf392a5eabc541f3e5074981455f78 Mon Sep 17 00:00:00 2001 From: templexxx Date: Fri, 24 Jun 2022 11:45:14 +0800 Subject: [PATCH 2/6] cmd/starcoin/cmd/starcoin: eprint multisig singatures filepath in txn execution 1. eprint multisig signatures filepath in txn execution 2. get txn output when there are enough multisig signatures --- .../src/account/sign_multisig_txn_cmd.rs | 10 +++-- cmd/starcoin/src/cli_state.rs | 40 ++++++++++++------- 2 files changed, 31 insertions(+), 19 deletions(-) diff --git a/cmd/starcoin/src/account/sign_multisig_txn_cmd.rs b/cmd/starcoin/src/account/sign_multisig_txn_cmd.rs index 3638952354..aeac1b2046 100644 --- a/cmd/starcoin/src/account/sign_multisig_txn_cmd.rs +++ b/cmd/starcoin/src/account/sign_multisig_txn_cmd.rs @@ -196,15 +196,17 @@ impl CommandAction for GenerateMultisigTxnCommand { } } - let output_dir = opt.output_dir.clone().unwrap_or(current_dir()?); - ctx.state().sign_multisig_txn_to_file_or_submit( + let mut output_dir = opt.output_dir.clone().unwrap_or(current_dir()?); + let _ = ctx.state().sign_multisig_txn_to_file_or_submit( raw_txn.sender(), account_public_key, existing_signatures, account_client.sign_txn(raw_txn, sender)?, - output_dir, + &mut output_dir, false, false, - ) + )?; + + Ok(output_dir) } } diff --git a/cmd/starcoin/src/cli_state.rs b/cmd/starcoin/src/cli_state.rs index 64539f4445..86f09fe9aa 100644 --- a/cmd/starcoin/src/cli_state.rs +++ b/cmd/starcoin/src/cli_state.rs @@ -304,20 +304,31 @@ impl CliState { AccountPublicKey::Multi(m) => m.clone(), }; - let multisig_txn_path = self.sign_multisig_txn_to_file_or_submit( + let mut output_dir = current_dir()?; + + let execute_output_view = self.sign_multisig_txn_to_file_or_submit( sender.address, multisig_public_key, None, signed_txn, - current_dir()?, + &mut output_dir, true, blocking, - ); + )?; - eprintln!( - "multisig-txn: {}", - multisig_txn_path.unwrap().as_path().to_str().unwrap() - ); + let cur_dir = current_dir().unwrap().to_str().unwrap().to_string(); + if output_dir.to_str().unwrap() != cur_dir { + // There is signature file, print the file path. + eprintln!( + "multisig txn signatures filepath: {}", + output_dir.to_str().unwrap() + ) + } + + match execute_output_view { + Some(o) => execute_result.execute_output = Some(o), + _ => {} + } Ok(execute_result) } @@ -343,10 +354,10 @@ impl CliState { multisig_public_key: MultiEd25519PublicKey, existing_signatures: Option, partial_signed_txn: SignedUserTransaction, - output_dir: PathBuf, + output_dir: &mut PathBuf, submit: bool, blocking: bool, - ) -> Result { + ) -> Result> { let my_signatures = if let TransactionAuthenticator::MultiEd25519 { signature, .. } = partial_signed_txn.authenticator() { @@ -392,23 +403,22 @@ impl CliState { }; if submit && signatures_is_enough { - let _ = self.submit_txn(signed_txn, blocking); - return Ok(PathBuf::new()); + let execute_output = self.submit_txn(signed_txn, blocking)?; + return Ok(Some(execute_output)); } // output the txn, send this to other participants to sign, or just submit it. let output_file = { - let mut output_dir = output_dir; // use hash's as output file name let file_name = signed_txn.crypto_hash().to_hex(); output_dir.push(file_name); output_dir.set_extension("multisig-txn"); - output_dir + output_dir.clone() }; - let mut file = File::create(output_file.clone())?; + let mut file = File::create(output_file)?; // write txn to file bcs_ext::serialize_into(&mut file, &signed_txn)?; - Ok(output_file) + Ok(None) } pub fn submit_txn( From b1c2bf7646fbddbbf438aa19e4bfff922ddf627f Mon Sep 17 00:00:00 2001 From: templexxx Date: Fri, 24 Jun 2022 13:34:50 +0800 Subject: [PATCH 3/6] cmd/starcoin/cmd/starcoin: add multisig singatures file integration testing 1. submit directly 2. sign to file first, then submit --- testsuite/features/cmd.feature | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/testsuite/features/cmd.feature b/testsuite/features/cmd.feature index 0d4d250294..b1f73be97f 100644 --- a/testsuite/features/cmd.feature +++ b/testsuite/features/cmd.feature @@ -59,6 +59,17 @@ Feature: cmd integration test Then cmd: "account generate-keypair -c 3" Then cmd: "account derive-address -t 2 -p @$[0].public_key@ -p @$[1].public_key@ -p @$[2].public_key@" Then cmd: "account transfer --blocking -r @$.receipt_identifier@ -t 0x1::STC::STC -v 10000000" + # account for testing only + Then cmd: "account import-multisig --pubkey 0x934e8a5a557229f90be7c95ec17fab84e64dcc3cf2dc934ff83ffc0915fad13e --pubkey 0x28358c05692e6758ba1398835525687c16d65abc9e1dc89023b46298ed2c575a --prikey 0x0c84a983ff0bfab39570c2ceed3e1c1feb645e84eccf9fd6baf4f49351a52329 --prikey 0x3695d6e08e3ad41962cba8c55ebb0552827807ae4cd6236d35195c769437272e -t 2" + Then cmd: "account unlock" + Then cmd: "account transfer --blocking -r 0x056d9bed868f8e8c74cf19109a2b375a -v 200000000" + Then cmd: "account unlock 0x056d9bed868f8e8c74cf19109a2b375a" + # enough signatures, submit directly + Then cmd: "account transfer -s 0x056d9bed868f8e8c74cf19109a2b375a -r 0x056d9bed868f8e8c74cf19109a2b375b -v 10000000 -b" + Then cmd: "account unlock 0x056d9bed868f8e8c74cf19109a2b375a" + # sign to file first + Then cmd: "account sign-multisig-txn -s 0x056d9bed868f8e8c74cf19109a2b375a --function 0x1::TransferScripts::peer_to_peer_v2 -t 0x1::STC::STC --arg 0x991c2f856a1e32985d9793b449c0f9d3 --arg 1000000u128 --output-dir /tmp" + Then cmd: "account submit-txn /tmp/417f79dd7e1be305a0db57ca731a2ed68ff06a5ca8567748c76f3793718fea63.multisig-txn -b" Then stop Examples: From d62bc57073f5e80e915245940221e47f40db3b49 Mon Sep 17 00:00:00 2001 From: templexxx Date: Fri, 24 Jun 2022 13:43:34 +0800 Subject: [PATCH 4/6] cmd/starcoin/cmd/starcoin: refine error handling --- cmd/starcoin/src/cli_state.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/cmd/starcoin/src/cli_state.rs b/cmd/starcoin/src/cli_state.rs index 86f09fe9aa..695a799e81 100644 --- a/cmd/starcoin/src/cli_state.rs +++ b/cmd/starcoin/src/cli_state.rs @@ -316,7 +316,7 @@ impl CliState { blocking, )?; - let cur_dir = current_dir().unwrap().to_str().unwrap().to_string(); + let cur_dir = current_dir()?.to_str().unwrap().to_string(); if output_dir.to_str().unwrap() != cur_dir { // There is signature file, print the file path. eprintln!( @@ -325,10 +325,9 @@ impl CliState { ) } - match execute_output_view { - Some(o) => execute_result.execute_output = Some(o), - _ => {} - } + if let Some(o) = execute_output_view { + execute_result.execute_output = Some(o) + }; Ok(execute_result) } From e24839c2c4c4297125d3636d83aa9f1325f0e498 Mon Sep 17 00:00:00 2001 From: templexxx Date: Fri, 24 Jun 2022 17:04:37 +0800 Subject: [PATCH 5/6] cmd/starcoin/cmd/starcoin: try to fix integration testing (submit with signatures file) --- testsuite/features/cmd.feature | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testsuite/features/cmd.feature b/testsuite/features/cmd.feature index b1f73be97f..7b098dd34e 100644 --- a/testsuite/features/cmd.feature +++ b/testsuite/features/cmd.feature @@ -69,7 +69,7 @@ Feature: cmd integration test Then cmd: "account unlock 0x056d9bed868f8e8c74cf19109a2b375a" # sign to file first Then cmd: "account sign-multisig-txn -s 0x056d9bed868f8e8c74cf19109a2b375a --function 0x1::TransferScripts::peer_to_peer_v2 -t 0x1::STC::STC --arg 0x991c2f856a1e32985d9793b449c0f9d3 --arg 1000000u128 --output-dir /tmp" - Then cmd: "account submit-txn /tmp/417f79dd7e1be305a0db57ca731a2ed68ff06a5ca8567748c76f3793718fea63.multisig-txn -b" + Then cmd: "account submit-txn @$ -b" Then stop Examples: From 7377a354b81ce4634f3074b279ae1e08b32f23cb Mon Sep 17 00:00:00 2001 From: templexxx Date: Sat, 25 Jun 2022 13:43:09 +0800 Subject: [PATCH 6/6] cmd/starcoin/cmd/starcoin: fix testing by getting right sign-multi-txn output --- testsuite/features/cmd.feature | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testsuite/features/cmd.feature b/testsuite/features/cmd.feature index 7b098dd34e..69c02e74ed 100644 --- a/testsuite/features/cmd.feature +++ b/testsuite/features/cmd.feature @@ -69,7 +69,7 @@ Feature: cmd integration test Then cmd: "account unlock 0x056d9bed868f8e8c74cf19109a2b375a" # sign to file first Then cmd: "account sign-multisig-txn -s 0x056d9bed868f8e8c74cf19109a2b375a --function 0x1::TransferScripts::peer_to_peer_v2 -t 0x1::STC::STC --arg 0x991c2f856a1e32985d9793b449c0f9d3 --arg 1000000u128 --output-dir /tmp" - Then cmd: "account submit-txn @$ -b" + Then cmd: "account submit-txn @$@ -b" Then stop Examples: