From 5d129c8189cd45bfc2d3bc86af444fb119f9d84f Mon Sep 17 00:00:00 2001 From: Jacinta Ferrant Date: Fri, 16 Feb 2024 09:33:40 -0500 Subject: [PATCH] Fix copyright and logging and add todos where missing Signed-off-by: Jacinta Ferrant --- stacks-signer/src/client/stacks_client.rs | 56 ++++++----------------- stacks-signer/src/config.rs | 5 +- stacks-signer/src/coordinator.rs | 2 +- stacks-signer/src/main.rs | 2 +- stacks-signer/src/signer.rs | 22 +++++++-- 5 files changed, 35 insertions(+), 52 deletions(-) diff --git a/stacks-signer/src/client/stacks_client.rs b/stacks-signer/src/client/stacks_client.rs index 492c3f5dd..63d75b40f 100644 --- a/stacks-signer/src/client/stacks_client.rs +++ b/stacks-signer/src/client/stacks_client.rs @@ -1,5 +1,3 @@ -use std::net::SocketAddr; - // Copyright (C) 2013-2020 Blockstack PBC, a public benefit corporation // Copyright (C) 2020-2024 Stacks Open Internet Foundation // @@ -15,6 +13,8 @@ use std::net::SocketAddr; // // You should have received a copy of the GNU General Public License // along with this program. If not, see . +use std::net::SocketAddr; + use blockstack_lib::burnchains::Txid; use blockstack_lib::chainstate::nakamoto::NakamotoBlock; use blockstack_lib::chainstate::stacks::boot::{RewardSet, SIGNERS_VOTING_NAME}; @@ -117,10 +117,8 @@ impl StacksClient { &self.stacks_address } - /// Calculate the coordinator address by comparing the provided public keys against the stacks tip consensus hash + /// Calculate the ordered list of coordinator ids by comparing the provided public keys against the pox consensus hash pub fn calculate_coordinator_ids(&self, public_keys: &PublicKeys) -> Vec { - // TODO: return the entire list. Might be at the same block height for a long time and need to move to the second item in the list - // Add logic throughout signer to track the current coordinator list and offset in the list let pox_consensus_hash = match retry_with_exponential_backoff(|| { self.get_pox_consenus_hash() .map_err(backoff::Error::transient) @@ -181,8 +179,6 @@ impl StacksClient { value: ClarityValue, ) -> Result, ClientError> { debug!("Parsing signer slots..."); - // Due to .signers definition, the signer slots is always an OK result of a list of tuples of signer addresses and the number of slots they have - // If this fails, we have bigger problems than the signer crashing... let value = value.clone().expect_result_ok()?; let values = value.expect_list()?; let mut signer_slots = Vec::with_capacity(values.len()); @@ -535,8 +531,6 @@ impl StacksClient { value: ClarityValue, ) -> Result, ClientError> { debug!("Parsing aggregate public key..."); - // Due to pox 4 definition, the aggregate public key is always an optional clarity value of 33 bytes hence the use of expect - // If this fails, we have bigger problems than the signer crashing... let opt = value.clone().expect_optional()?; let Some(inner_data) = opt else { return Ok(None); @@ -577,43 +571,19 @@ impl StacksClient { ClarityValue::UInt(round as u128), ClarityValue::UInt(reward_cycle as u128), ]; + let tx_fee = tx_fee.unwrap_or(0); - let tx_payload = TransactionPayload::ContractCall(TransactionContractCall { - address: contract_address, + Self::build_signed_contract_call_transaction( + &contract_address, contract_name, function_name, - function_args, - }); - let public_key = StacksPublicKey::from_private(&self.stacks_private_key); - let tx_auth = TransactionAuth::Standard( - TransactionSpendingCondition::new_singlesig_p2pkh(public_key).ok_or( - ClientError::TransactionGenerationFailure(format!( - "Failed to create spending condition from public key: {}", - public_key.to_hex() - )), - )?, - ); - - let mut unsigned_tx = StacksTransaction::new(self.tx_version, tx_auth, tx_payload); - if let Some(tx_fee) = tx_fee { - unsigned_tx.set_tx_fee(tx_fee); - } - unsigned_tx.set_origin_nonce(nonce); - - unsigned_tx.anchor_mode = TransactionAnchorMode::Any; - unsigned_tx.post_condition_mode = TransactionPostConditionMode::Allow; - unsigned_tx.chain_id = self.chain_id; - - let mut tx_signer = StacksTransactionSigner::new(&unsigned_tx); - tx_signer - .sign_origin(&self.stacks_private_key) - .map_err(|e| ClientError::TransactionGenerationFailure(e.to_string()))?; - - tx_signer - .get_tx() - .ok_or(ClientError::TransactionGenerationFailure( - "Failed to generate transaction from a transaction signer".to_string(), - )) + &function_args, + &self.stacks_private_key, + self.tx_version, + self.chain_id, + nonce, + tx_fee, + ) } /// Helper function to submit a transaction to the Stacks mempool diff --git a/stacks-signer/src/config.rs b/stacks-signer/src/config.rs index 9d8fce6b0..a24f07b4f 100644 --- a/stacks-signer/src/config.rs +++ b/stacks-signer/src/config.rs @@ -1,5 +1,5 @@ // Copyright (C) 2013-2020 Blockstack PBC, a public benefit corporation -// Copyright (C) 2020-2023 Stacks Open Internet Foundation +// Copyright (C) 2020-2024 Stacks Open Internet Foundation // // This program is free software: you can redistribute it and/or modify // it under the terms of the GNU General Public License as published by @@ -34,7 +34,8 @@ use wsts::curve::scalar::Scalar; use wsts::state_machine::PublicKeys; const EVENT_TIMEOUT_MS: u64 = 5000; -//TODO: make this zero once special cased transactions are allowed in the stacks node +// Default transaction fee in microstacks (if unspecificed in the config file) +// TODO: Use the fee estimation endpoint to get the default fee. const TX_FEE_MS: u64 = 10_000; #[derive(thiserror::Error, Debug)] diff --git a/stacks-signer/src/coordinator.rs b/stacks-signer/src/coordinator.rs index 3943be3a2..817f8a5e9 100644 --- a/stacks-signer/src/coordinator.rs +++ b/stacks-signer/src/coordinator.rs @@ -1,5 +1,5 @@ // Copyright (C) 2013-2020 Blockstack PBC, a public benefit corporation -// Copyright (C) 2020-2023 Stacks Open Internet Foundation +// Copyright (C) 2020-2024 Stacks Open Internet Foundation // // This program is free software: you can redistribute it and/or modify // it under the terms of the GNU General Public License as published by diff --git a/stacks-signer/src/main.rs b/stacks-signer/src/main.rs index 0042b539d..1e988d125 100644 --- a/stacks-signer/src/main.rs +++ b/stacks-signer/src/main.rs @@ -4,7 +4,7 @@ //! //! // Copyright (C) 2013-2020 Blockstack PBC, a public benefit corporation -// Copyright (C) 2020-2023 Stacks Open Internet Foundation +// Copyright (C) 2020-2024 Stacks Open Internet Foundation // // This program is free software: you can redistribute it and/or modify // it under the terms of the GNU General Public License as published by diff --git a/stacks-signer/src/signer.rs b/stacks-signer/src/signer.rs index e3c2c1886..feec06bb2 100644 --- a/stacks-signer/src/signer.rs +++ b/stacks-signer/src/signer.rs @@ -338,7 +338,7 @@ impl Signer { // For mutability reasons, we need to take the block_info out of the map and add it back after processing let Some(mut block_info) = self.blocks.remove(&signer_signature_hash) else { // We have not seen this block before. Why are we getting a response for it? - debug!("Received a block validate response for a block we have not seen before. Ignoring..."); + debug!("Signer #{}: Received a block validate response for a block we have not seen before. Ignoring...", self.signer_id); return; }; let is_valid = self.verify_block_transactions(stacks_client, &block_info.block); @@ -378,7 +378,7 @@ impl Signer { } }; if let Some(mut nonce_request) = block_info.nonce_request.take() { - debug!("Received a block validate response from the stacks node for a block we already received a nonce request for. Responding to the nonce request..."); + debug!("Signer #{}: Received a block validate response from the stacks node for a block we already received a nonce request for. Responding to the nonce request...", self.signer_id); // We have received validation from the stacks node. Determine our vote and update the request message Self::determine_vote(self.signer_id, block_info, &mut nonce_request); // Send the nonce request through with our vote @@ -450,7 +450,10 @@ impl Signer { stacks_client .submit_block_for_validation(block.clone()) .unwrap_or_else(|e| { - warn!("Failed to submit block for validation: {e:?}"); + warn!( + "Signer #{}: Failed to submit block for validation: {e:?}", + self.signer_id + ); }); } } @@ -467,7 +470,10 @@ impl Signer { .signing_round .process_inbound_messages(packets) .unwrap_or_else(|e| { - error!("Failed to process inbound messages as a signer: {e:?}"); + error!( + "Signer #{}: Failed to process inbound messages as a signer: {e:?}", + self.signer_id + ); vec![] }); @@ -476,7 +482,10 @@ impl Signer { .coordinator .process_inbound_messages(packets) .unwrap_or_else(|e| { - error!("Failed to process inbound messages as a coordinator: {e:?}"); + error!( + "Signer #{}: Failed to process inbound messages as a coordinator: {e:?}", + self.signer_id + ); (vec![], vec![]) }); @@ -1208,6 +1217,9 @@ impl Signer { if new_aggregate_public_key.is_some() && old_aggregate_public_key != new_aggregate_public_key { + // TODO: this will never work as is. We need to have stored our party shares on the side etc for this particular aggregate key. + // Need to update state to store the necessary info, check against it to see if we have participated in the winning round and + // then overwrite our value accordingly. Otherwise, we will be locked out of the round and should not participate. debug!( "Signer #{}: Received a new aggregate public key ({new_aggregate_public_key:?}) for reward cycle {reward_cycle}. Overwriting its internal aggregate key ({old_aggregate_public_key:?})", self.signer_id