From 59a93c56d7d546d56863377e160c18b08f1ca8e7 Mon Sep 17 00:00:00 2001 From: Ludo Galabru Date: Tue, 12 Dec 2023 13:25:17 -0500 Subject: [PATCH 1/3] fix: remove off-by-one block latency, improve display --- .../clarinet-cli/src/deployments/ui/ui.rs | 8 +- .../clarinet-deployments/src/onchain/mod.rs | 121 ++++++++++-------- 2 files changed, 74 insertions(+), 55 deletions(-) diff --git a/components/clarinet-cli/src/deployments/ui/ui.rs b/components/clarinet-cli/src/deployments/ui/ui.rs index 1488f20d3..7b8f5ca64 100644 --- a/components/clarinet-cli/src/deployments/ui/ui.rs +++ b/components/clarinet-cli/src/deployments/ui/ui.rs @@ -23,7 +23,9 @@ where TransactionStatus::Encoded(_, _) => { ("🟦", "Transaction encoded and queued".to_string()) } - TransactionStatus::Broadcasted(_) => ("🟨", "Transaction broadcasted".to_string()), + TransactionStatus::Broadcasted(_, txid) => { + ("🟨", format!("Transaction broadcasted (txid: {})", txid)) + } TransactionStatus::Confirmed => ("🟩", "Transaction confirmed".to_string()), TransactionStatus::Error(message) => ("🟥", message.to_string()), }; @@ -42,8 +44,8 @@ where .style(Style::default().fg(Color::White)) .widths(&[ Constraint::Length(3), - Constraint::Length(80), - Constraint::Length(120), + Constraint::Length(90), + Constraint::Length(110), ]); f.render_widget(t, area); } diff --git a/components/clarinet-deployments/src/onchain/mod.rs b/components/clarinet-deployments/src/onchain/mod.rs index cccf1ef10..8a72971a4 100644 --- a/components/clarinet-deployments/src/onchain/mod.rs +++ b/components/clarinet-deployments/src/onchain/mod.rs @@ -202,7 +202,7 @@ pub fn encode_contract_publish( pub enum TransactionStatus { Queued, Encoded(StacksTransaction, TransactionCheck), - Broadcasted(TransactionCheck), + Broadcasted(TransactionCheck, String), Confirmed, Error(String), } @@ -219,7 +219,7 @@ pub enum TransactionCheck { NonceCheck(StandardPrincipalData, u64), ContractPublish(StandardPrincipalData, ContractName), // TODO(lgalabru): Handle Bitcoin checks - // BtcTransfer(), + BtcTransfer, } #[derive(Clone, Debug)] @@ -404,12 +404,7 @@ pub fn apply_on_chain_deployment( for batch_spec in deployment.plan.batches.iter() { let epoch = match batch_spec.epoch { - Some(epoch) => { - if network != StacksNetwork::Devnet { - println!("warning: 'epoch' specified for a deployment batch is ignored when applying a deployment plan. This field should only be specified for deployments plans used to launch a devnet with 'clarinet devnet start'."); - } - epoch - } + Some(epoch) => epoch, None => default_epoch, }; let mut batch = Vec::new(); @@ -823,7 +818,7 @@ pub fn apply_on_chain_deployment( }; match stacks_rpc.post_transaction(&transaction) { Ok(res) => { - tracker.status = TransactionStatus::Broadcasted(check); + tracker.status = TransactionStatus::Broadcasted(check, res.txid.clone()); let _ = deployment_event_tx .send(DeploymentEvent::TransactionUpdate(tracker.clone())); @@ -840,9 +835,11 @@ pub fn apply_on_chain_deployment( } }; } + let mut last_stacks_chain_check_at_height = 0; + let mut last_bitcoin_chain_check_at_height = 0; loop { - let (burn_block_height, stacks_tip_height) = match stacks_rpc.get_info() { + let (bitcoin_tip_height, stacks_tip_height) = match stacks_rpc.get_info() { Ok(info) => (info.burn_block_height, info.stacks_tip_height), _ => { std::thread::sleep(std::time::Duration::from_secs(delay_between_checks)); @@ -850,58 +847,78 @@ pub fn apply_on_chain_deployment( } }; - // If no block has been mined since `delay_between_checks`, - // avoid flooding the stacks-node with status update requests. - if burn_block_height <= current_bitcoin_block_height { - std::thread::sleep(std::time::Duration::from_secs(delay_between_checks)); - continue; - } - - current_bitcoin_block_height = burn_block_height; - current_block_height = stacks_tip_height; - let mut keep_looping = false; - for (_txid, tracker) in ongoing_batch.iter_mut() { - match &tracker.status { - TransactionStatus::Broadcasted(TransactionCheck::ContractPublish( - deployer, - contract_name, - )) => { - let deployer_address = deployer.to_address(); - let res = stacks_rpc.get_contract_source(&deployer_address, contract_name); - match res { - Ok(_contract) => { - tracker.status = TransactionStatus::Confirmed; - let _ = deployment_event_tx - .send(DeploymentEvent::TransactionUpdate(tracker.clone())); + // Handle Stacks releated checks + if stacks_tip_height > last_stacks_chain_check_at_height { + for (txid, tracker) in ongoing_batch.iter_mut() { + let TransactionStatus::Broadcasted(brodcasting_status, _) = &tracker.status + else { + continue; + }; + + match &brodcasting_status { + TransactionCheck::ContractPublish(deployer, contract_name) => { + let deployer_address = deployer.to_address(); + let res = + stacks_rpc.get_contract_source(&deployer_address, contract_name); + match res { + Ok(_contract) => { + tracker.status = TransactionStatus::Confirmed; + let _ = deployment_event_tx + .send(DeploymentEvent::TransactionUpdate(tracker.clone())); + } + Err(_e) => { + keep_looping = true; + break; + } } - Err(_e) => { - keep_looping = true; - break; + } + TransactionCheck::NonceCheck(tx_sender, expected_nonce) => { + let tx_sender_address = tx_sender.to_address(); + let res = stacks_rpc.get_nonce(&tx_sender_address); + if let Ok(current_nonce) = res { + if current_nonce.gt(expected_nonce) { + tracker.status = TransactionStatus::Confirmed; + let _ = deployment_event_tx + .send(DeploymentEvent::TransactionUpdate(tracker.clone())); + } else { + keep_looping = true; + break; + } } } + _ => {} } - TransactionStatus::Broadcasted(TransactionCheck::NonceCheck( - tx_sender, - expected_nonce, - )) => { - let tx_sender_address = tx_sender.to_address(); - let res = stacks_rpc.get_nonce(&tx_sender_address); - if let Ok(current_nonce) = res { - if current_nonce.gt(expected_nonce) { - tracker.status = TransactionStatus::Confirmed; - let _ = deployment_event_tx - .send(DeploymentEvent::TransactionUpdate(tracker.clone())); - } else { - keep_looping = true; - break; - } + } + } else { + std::thread::sleep(std::time::Duration::from_secs(delay_between_checks)); + continue; + } + + // Handle Bitcoin releated checks + if bitcoin_tip_height > last_bitcoin_chain_check_at_height { + for (txid, tracker) in ongoing_batch.iter_mut() { + let TransactionStatus::Broadcasted(brodcasting_status, _) = &tracker.status + else { + continue; + }; + match &brodcasting_status { + TransactionCheck::BtcTransfer => { + // TODO } + TransactionCheck::ContractPublish(_, _) + | TransactionCheck::NonceCheck(_, _) => {} } - _ => {} } + } else { + std::thread::sleep(std::time::Duration::from_secs(delay_between_checks)); + continue; } + + last_stacks_chain_check_at_height = stacks_tip_height; + last_bitcoin_chain_check_at_height = bitcoin_tip_height; + if !keep_looping { break; } From e73f31608c91ffec0d2aa8663f26e5261af6a8fb Mon Sep 17 00:00:00 2001 From: Ludo Galabru Date: Tue, 12 Dec 2023 13:51:37 -0500 Subject: [PATCH 2/3] fix: improve mnemonic related errors handling --- components/clarinet-cli/src/frontend/cli.rs | 14 ++++++++++---- components/clarinet-deployments/src/onchain/mod.rs | 4 ++-- components/clarinet-files/src/network_manifest.rs | 4 ++-- 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/components/clarinet-cli/src/frontend/cli.rs b/components/clarinet-cli/src/frontend/cli.rs index e05dff85d..c73d8a8c5 100644 --- a/components/clarinet-cli/src/frontend/cli.rs +++ b/components/clarinet-cli/src/frontend/cli.rs @@ -799,13 +799,19 @@ pub fn main() { let network_moved = network.clone(); std::thread::spawn(move || { let manifest = manifest_moved; - let network_manifest = NetworkManifest::from_project_manifest_location( + let res = NetworkManifest::from_project_manifest_location( &manifest.location, &network_moved.get_networks(), Some(&manifest.project.cache_location), None, - ) - .expect("unable to load network manifest"); + ); + let network_manifest = match res { + Ok(network_manifest) => network_manifest, + Err(e) => { + let _ = event_tx.send(DeploymentEvent::Interrupted(e)); + return; + } + }; apply_on_chain_deployment( network_manifest, deployment, @@ -828,7 +834,7 @@ pub fn main() { match cmd { DeploymentEvent::Interrupted(message) => { println!( - "{} Error publishing transactions: {}", + "{} Error publishing transactions (x): {}", red!("x"), message ); diff --git a/components/clarinet-deployments/src/onchain/mod.rs b/components/clarinet-deployments/src/onchain/mod.rs index 8a72971a4..486e2c13c 100644 --- a/components/clarinet-deployments/src/onchain/mod.rs +++ b/components/clarinet-deployments/src/onchain/mod.rs @@ -851,7 +851,7 @@ pub fn apply_on_chain_deployment( // Handle Stacks releated checks if stacks_tip_height > last_stacks_chain_check_at_height { - for (txid, tracker) in ongoing_batch.iter_mut() { + for (_, tracker) in ongoing_batch.iter_mut() { let TransactionStatus::Broadcasted(brodcasting_status, _) = &tracker.status else { continue; @@ -898,7 +898,7 @@ pub fn apply_on_chain_deployment( // Handle Bitcoin releated checks if bitcoin_tip_height > last_bitcoin_chain_check_at_height { - for (txid, tracker) in ongoing_batch.iter_mut() { + for (_, tracker) in ongoing_batch.iter_mut() { let TransactionStatus::Broadcasted(brodcasting_status, _) = &tracker.status else { continue; diff --git a/components/clarinet-files/src/network_manifest.rs b/components/clarinet-files/src/network_manifest.rs index 99211e7c9..1b5afb876 100644 --- a/components/clarinet-files/src/network_manifest.rs +++ b/components/clarinet-files/src/network_manifest.rs @@ -417,8 +417,8 @@ impl NetworkManifest { Ok(result) => result.to_string(), Err(e) => { return Err(format!( - "mnemonic for wallet '{}' invalid: {}", - account_name, e + "mnemonic (located in ./settings/{:?}.toml) for deploying address is invalid: {}", + networks.1 , e )); } } From 6dd6102167bc35d74769fcb8b03556ada599d800 Mon Sep 17 00:00:00 2001 From: Ludo Galabru Date: Tue, 12 Dec 2023 15:25:05 -0500 Subject: [PATCH 3/3] chore: address feedbacks --- components/clarinet-cli/src/frontend/cli.rs | 2 +- components/clarinet-deployments/src/onchain/mod.rs | 6 +----- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/components/clarinet-cli/src/frontend/cli.rs b/components/clarinet-cli/src/frontend/cli.rs index c73d8a8c5..e9134c477 100644 --- a/components/clarinet-cli/src/frontend/cli.rs +++ b/components/clarinet-cli/src/frontend/cli.rs @@ -834,7 +834,7 @@ pub fn main() { match cmd { DeploymentEvent::Interrupted(message) => { println!( - "{} Error publishing transactions (x): {}", + "{} Error publishing transactions: {}", red!("x"), message ); diff --git a/components/clarinet-deployments/src/onchain/mod.rs b/components/clarinet-deployments/src/onchain/mod.rs index 486e2c13c..9d7e29537 100644 --- a/components/clarinet-deployments/src/onchain/mod.rs +++ b/components/clarinet-deployments/src/onchain/mod.rs @@ -218,7 +218,6 @@ pub struct TransactionTracker { pub enum TransactionCheck { NonceCheck(StandardPrincipalData, u64), ContractPublish(StandardPrincipalData, ContractName), - // TODO(lgalabru): Handle Bitcoin checks BtcTransfer, } @@ -403,10 +402,7 @@ pub fn apply_on_chain_deployment( } for batch_spec in deployment.plan.batches.iter() { - let epoch = match batch_spec.epoch { - Some(epoch) => epoch, - None => default_epoch, - }; + let epoch = batch_spec.epoch.unwrap_or(default_epoch); let mut batch = Vec::new(); for transaction in batch_spec.transactions.iter() { let tracker = match transaction {