From 8d327754a8a9ad6e8ead8706cb6f91d2714877fa Mon Sep 17 00:00:00 2001 From: jesus Date: Mon, 26 Feb 2024 14:04:43 -0500 Subject: [PATCH 01/10] two minor checks --- stackslib/src/chainstate/stacks/boot/pox-4.clar | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/stackslib/src/chainstate/stacks/boot/pox-4.clar b/stackslib/src/chainstate/stacks/boot/pox-4.clar index d54f3b8d5..77c8ef255 100644 --- a/stackslib/src/chainstate/stacks/boot/pox-4.clar +++ b/stackslib/src/chainstate/stacks/boot/pox-4.clar @@ -32,6 +32,7 @@ (define-constant ERR_DELEGATION_ALREADY_REVOKED 34) (define-constant ERR_INVALID_SIGNATURE_PUBKEY 35) (define-constant ERR_INVALID_SIGNATURE_RECOVER 36) +(define-constant ERR_INVALID_REWARD_CYCLE 37) ;; Valid values for burnchain address versions. ;; These first four correspond to address hash modes in Stacks 2.1, @@ -1368,6 +1369,10 @@ (asserts! (is-eq (unwrap! (principal-construct? (if is-in-mainnet STACKS_ADDR_VERSION_MAINNET STACKS_ADDR_VERSION_TESTNET) (hash160 signer-key)) (err ERR_INVALID_SIGNER_KEY)) tx-sender) (err ERR_NOT_ALLOWED)) + ;; Must be called with positive period + (asserts! (>= period u1) (err ERR_STACKING_INVALID_LOCK_PERIOD)) + ;; Must be current or future reward cycle + (asserts! (>= reward-cycle (current-pox-reward-cycle)) (err ERR_INVALID_REWARD_CYCLE)) (map-set signer-key-authorizations { pox-addr: pox-addr, period: period, reward-cycle: reward-cycle, topic: topic, signer-key: signer-key } allowed) (ok allowed))) From 11a5ce92b7c7f50d14f91033d88779294985542b Mon Sep 17 00:00:00 2001 From: jesus Date: Mon, 26 Feb 2024 16:18:04 -0500 Subject: [PATCH 02/10] updated test_set_signer_key_auth passing again, still work remaining --- .../src/chainstate/stacks/boot/pox_4_tests.rs | 43 +++++++++++++++---- 1 file changed, 34 insertions(+), 9 deletions(-) diff --git a/stackslib/src/chainstate/stacks/boot/pox_4_tests.rs b/stackslib/src/chainstate/stacks/boot/pox_4_tests.rs index ebb8c5f07..33567e6d4 100644 --- a/stackslib/src/chainstate/stacks/boot/pox_4_tests.rs +++ b/stackslib/src/chainstate/stacks/boot/pox_4_tests.rs @@ -2642,6 +2642,7 @@ fn test_set_signer_key_auth() { let mut signer_nonce = 0; let signer_key = &keys[1]; let signer_public_key = StacksPublicKey::from_private(signer_key); + let signer_addr = key_to_stacks_addr(&signer_key); let pox_addr = pox_addr_from(&signer_key); // Only the address associated with `signer-key` can enable auth for that key @@ -2657,21 +2658,38 @@ fn test_set_signer_key_auth() { Some(&alice_key), ); + let current_cycle = get_current_reward_cycle(&peer, &burnchain); + println!("Current cycle: {}", current_cycle); + + // Test that period is at least u1 + let invalid_auth_tx_period: StacksTransaction = make_pox_4_set_signer_key_auth( + &pox_addr, + &signer_key, + 22, + &Pox4SignatureTopic::StackStx, + 0, + false, + signer_nonce, + Some(&signer_key), + ); + + // Test that confirmed reward cycle is at least current reward cycle + // Disable auth for `signer-key` - let disable_auth_nonce = signer_nonce; + signer_nonce += 1; let disable_auth_tx: StacksTransaction = make_pox_4_set_signer_key_auth( &pox_addr, &signer_key, - 1, + 22, &Pox4SignatureTopic::StackStx, lock_period, false, - disable_auth_nonce, + signer_nonce, None, ); let latest_block = - peer.tenure_with_txs(&[invalid_enable_tx, disable_auth_tx], &mut coinbase_nonce); + peer.tenure_with_txs(&[invalid_enable_tx, invalid_auth_tx_period, disable_auth_tx], &mut coinbase_nonce); let alice_txs = get_last_block_sender_transactions(&observer, alice_addr); let invalid_enable_tx_result = alice_txs @@ -2682,11 +2700,18 @@ fn test_set_signer_key_auth() { let expected_error = Value::error(Value::Int(19)).unwrap(); assert_eq!(invalid_enable_tx_result, expected_error); + let signer_txs = get_last_block_sender_transactions(&observer, signer_addr); + + // // Print all signer transaction receipts + println!("signer_txs: {:?}", signer_txs); + for tx in signer_txs { + println!("txs in signer_tx? {:?}", tx.result); + } let signer_key_enabled = get_signer_key_authorization_pox_4( &mut peer, &latest_block, &pox_addr, - 1, + 22, &Pox4SignatureTopic::StackStx, lock_period.try_into().unwrap(), &signer_public_key, @@ -2700,7 +2725,7 @@ fn test_set_signer_key_auth() { let enable_auth_tx = make_pox_4_set_signer_key_auth( &pox_addr, &signer_key, - 1, + 22, &Pox4SignatureTopic::StackStx, lock_period, true, @@ -2714,7 +2739,7 @@ fn test_set_signer_key_auth() { &mut peer, &latest_block, &pox_addr, - 1, + 22, &Pox4SignatureTopic::StackStx, lock_period.try_into().unwrap(), &signer_public_key, @@ -2728,7 +2753,7 @@ fn test_set_signer_key_auth() { let disable_auth_tx = make_pox_4_set_signer_key_auth( &pox_addr, &signer_key, - 1, + 22, &Pox4SignatureTopic::StackStx, lock_period, false, @@ -2742,7 +2767,7 @@ fn test_set_signer_key_auth() { &mut peer, &latest_block, &pox_addr, - 1, + 22, &Pox4SignatureTopic::StackStx, lock_period.try_into().unwrap(), &signer_public_key, From 2a63322ea619606706d63137954fad9bbe577805 Mon Sep 17 00:00:00 2001 From: jesus Date: Tue, 27 Feb 2024 07:25:43 -0500 Subject: [PATCH 03/10] completed updating test_set_signer_key_auth --- .../src/chainstate/stacks/boot/pox_4_tests.rs | 63 +++++++++++++------ 1 file changed, 44 insertions(+), 19 deletions(-) diff --git a/stackslib/src/chainstate/stacks/boot/pox_4_tests.rs b/stackslib/src/chainstate/stacks/boot/pox_4_tests.rs index 33567e6d4..0ddf9075c 100644 --- a/stackslib/src/chainstate/stacks/boot/pox_4_tests.rs +++ b/stackslib/src/chainstate/stacks/boot/pox_4_tests.rs @@ -2645,6 +2645,8 @@ fn test_set_signer_key_auth() { let signer_addr = key_to_stacks_addr(&signer_key); let pox_addr = pox_addr_from(&signer_key); + let current_reward_cycle = get_current_reward_cycle(&peer, &burnchain); + // Only the address associated with `signer-key` can enable auth for that key let invalid_enable_nonce = alice_nonce; let invalid_enable_tx = make_pox_4_set_signer_key_auth( @@ -2658,29 +2660,39 @@ fn test_set_signer_key_auth() { Some(&alice_key), ); - let current_cycle = get_current_reward_cycle(&peer, &burnchain); - println!("Current cycle: {}", current_cycle); - // Test that period is at least u1 - let invalid_auth_tx_period: StacksTransaction = make_pox_4_set_signer_key_auth( + let signer_invalid_period_nonce = signer_nonce; + signer_nonce += 1; + let invalid_tx_period: StacksTransaction = make_pox_4_set_signer_key_auth( &pox_addr, &signer_key, - 22, + current_reward_cycle, &Pox4SignatureTopic::StackStx, 0, false, - signer_nonce, + signer_invalid_period_nonce, Some(&signer_key), ); + let signer_invalid_cycle_nonce = signer_nonce; + signer_nonce += 1; // Test that confirmed reward cycle is at least current reward cycle + let invalid_tx_cycle: StacksTransaction = make_pox_4_set_signer_key_auth( + &pox_addr, + &signer_key, + 1, + &Pox4SignatureTopic::StackStx, + 1, + false, + signer_invalid_cycle_nonce, + Some(&signer_key), + ); // Disable auth for `signer-key` - signer_nonce += 1; let disable_auth_tx: StacksTransaction = make_pox_4_set_signer_key_auth( &pox_addr, &signer_key, - 22, + current_reward_cycle, &Pox4SignatureTopic::StackStx, lock_period, false, @@ -2689,7 +2701,7 @@ fn test_set_signer_key_auth() { ); let latest_block = - peer.tenure_with_txs(&[invalid_enable_tx, invalid_auth_tx_period, disable_auth_tx], &mut coinbase_nonce); + peer.tenure_with_txs(&[invalid_enable_tx, invalid_tx_period, invalid_tx_cycle, disable_auth_tx], &mut coinbase_nonce); let alice_txs = get_last_block_sender_transactions(&observer, alice_addr); let invalid_enable_tx_result = alice_txs @@ -2702,16 +2714,29 @@ fn test_set_signer_key_auth() { let signer_txs = get_last_block_sender_transactions(&observer, signer_addr); - // // Print all signer transaction receipts - println!("signer_txs: {:?}", signer_txs); - for tx in signer_txs { - println!("txs in signer_tx? {:?}", tx.result); - } + let invalid_tx_period_result = signer_txs.clone() + .get(signer_invalid_period_nonce as usize) + .unwrap() + .result + .clone(); + + // Check for invalid lock period err + assert_eq!(invalid_tx_period_result, Value::error(Value::Int(2)).unwrap()); + + let invalid_tx_cycle_result = signer_txs.clone() + .get(signer_invalid_cycle_nonce as usize) + .unwrap() + .result + .clone(); + + // Check for invalid cycle err + assert_eq!(invalid_tx_cycle_result, Value::error(Value::Int(37)).unwrap()); + let signer_key_enabled = get_signer_key_authorization_pox_4( &mut peer, &latest_block, &pox_addr, - 22, + current_reward_cycle.clone() as u64, &Pox4SignatureTopic::StackStx, lock_period.try_into().unwrap(), &signer_public_key, @@ -2725,7 +2750,7 @@ fn test_set_signer_key_auth() { let enable_auth_tx = make_pox_4_set_signer_key_auth( &pox_addr, &signer_key, - 22, + current_reward_cycle, &Pox4SignatureTopic::StackStx, lock_period, true, @@ -2739,7 +2764,7 @@ fn test_set_signer_key_auth() { &mut peer, &latest_block, &pox_addr, - 22, + current_reward_cycle.clone() as u64, &Pox4SignatureTopic::StackStx, lock_period.try_into().unwrap(), &signer_public_key, @@ -2753,7 +2778,7 @@ fn test_set_signer_key_auth() { let disable_auth_tx = make_pox_4_set_signer_key_auth( &pox_addr, &signer_key, - 22, + current_reward_cycle, &Pox4SignatureTopic::StackStx, lock_period, false, @@ -2767,7 +2792,7 @@ fn test_set_signer_key_auth() { &mut peer, &latest_block, &pox_addr, - 22, + current_reward_cycle.clone() as u64, &Pox4SignatureTopic::StackStx, lock_period.try_into().unwrap(), &signer_public_key, From 37a681dc04c8e173d906b5e2d45efabb692ed482 Mon Sep 17 00:00:00 2001 From: jesus Date: Tue, 27 Feb 2024 07:28:35 -0500 Subject: [PATCH 04/10] formatter --- .../src/chainstate/stacks/boot/pox_4_tests.rs | 33 ++++++++++++++----- 1 file changed, 24 insertions(+), 9 deletions(-) diff --git a/stackslib/src/chainstate/stacks/boot/pox_4_tests.rs b/stackslib/src/chainstate/stacks/boot/pox_4_tests.rs index 0ddf9075c..4f51b81e8 100644 --- a/stackslib/src/chainstate/stacks/boot/pox_4_tests.rs +++ b/stackslib/src/chainstate/stacks/boot/pox_4_tests.rs @@ -2678,9 +2678,9 @@ fn test_set_signer_key_auth() { signer_nonce += 1; // Test that confirmed reward cycle is at least current reward cycle let invalid_tx_cycle: StacksTransaction = make_pox_4_set_signer_key_auth( - &pox_addr, - &signer_key, - 1, + &pox_addr, + &signer_key, + 1, &Pox4SignatureTopic::StackStx, 1, false, @@ -2700,8 +2700,15 @@ fn test_set_signer_key_auth() { None, ); - let latest_block = - peer.tenure_with_txs(&[invalid_enable_tx, invalid_tx_period, invalid_tx_cycle, disable_auth_tx], &mut coinbase_nonce); + let latest_block = peer.tenure_with_txs( + &[ + invalid_enable_tx, + invalid_tx_period, + invalid_tx_cycle, + disable_auth_tx, + ], + &mut coinbase_nonce, + ); let alice_txs = get_last_block_sender_transactions(&observer, alice_addr); let invalid_enable_tx_result = alice_txs @@ -2714,23 +2721,31 @@ fn test_set_signer_key_auth() { let signer_txs = get_last_block_sender_transactions(&observer, signer_addr); - let invalid_tx_period_result = signer_txs.clone() + let invalid_tx_period_result = signer_txs + .clone() .get(signer_invalid_period_nonce as usize) .unwrap() .result .clone(); // Check for invalid lock period err - assert_eq!(invalid_tx_period_result, Value::error(Value::Int(2)).unwrap()); + assert_eq!( + invalid_tx_period_result, + Value::error(Value::Int(2)).unwrap() + ); - let invalid_tx_cycle_result = signer_txs.clone() + let invalid_tx_cycle_result = signer_txs + .clone() .get(signer_invalid_cycle_nonce as usize) .unwrap() .result .clone(); // Check for invalid cycle err - assert_eq!(invalid_tx_cycle_result, Value::error(Value::Int(37)).unwrap()); + assert_eq!( + invalid_tx_cycle_result, + Value::error(Value::Int(37)).unwrap() + ); let signer_key_enabled = get_signer_key_authorization_pox_4( &mut peer, From 5a54ae45a572403402846a762f5f869197afd546 Mon Sep 17 00:00:00 2001 From: jesus Date: Thu, 29 Feb 2024 08:18:45 -0500 Subject: [PATCH 05/10] remaining tests intact --- stackslib/src/chainstate/stacks/boot/pox-4.clar | 3 +++ 1 file changed, 3 insertions(+) diff --git a/stackslib/src/chainstate/stacks/boot/pox-4.clar b/stackslib/src/chainstate/stacks/boot/pox-4.clar index 77c8ef255..8ccf3567a 100644 --- a/stackslib/src/chainstate/stacks/boot/pox-4.clar +++ b/stackslib/src/chainstate/stacks/boot/pox-4.clar @@ -117,6 +117,9 @@ ;; (6) (reward-cycle-to-burn-height (+ lock-period first-reward-cycle)) == (get unlock-height (stx-account stacker)) ;; These invariants only hold while `cur-reward-cycle < (+ lock-period first-reward-cycle)` ;; +(define-map protocols principal (list 10000 uint)) +(define-map protocol-positions {protocol: principal, position-id: uint} {status: (string-ascii 32), collateral: uint}) + (define-map stacking-state { stacker: principal } { From 8490fd9f999e3005b44644ff9ef6a3944daf52b4 Mon Sep 17 00:00:00 2001 From: jesus Date: Thu, 29 Feb 2024 10:17:55 -0500 Subject: [PATCH 06/10] removed throwaway --- stackslib/src/chainstate/stacks/boot/pox-4.clar | 3 --- 1 file changed, 3 deletions(-) diff --git a/stackslib/src/chainstate/stacks/boot/pox-4.clar b/stackslib/src/chainstate/stacks/boot/pox-4.clar index 8ccf3567a..77c8ef255 100644 --- a/stackslib/src/chainstate/stacks/boot/pox-4.clar +++ b/stackslib/src/chainstate/stacks/boot/pox-4.clar @@ -117,9 +117,6 @@ ;; (6) (reward-cycle-to-burn-height (+ lock-period first-reward-cycle)) == (get unlock-height (stx-account stacker)) ;; These invariants only hold while `cur-reward-cycle < (+ lock-period first-reward-cycle)` ;; -(define-map protocols principal (list 10000 uint)) -(define-map protocol-positions {protocol: principal, position-id: uint} {status: (string-ascii 32), collateral: uint}) - (define-map stacking-state { stacker: principal } { From 533bf19f7c7f1819e2641c2cea87ff1cfea9c045 Mon Sep 17 00:00:00 2001 From: Jacinta Ferrant Date: Mon, 26 Feb 2024 18:28:37 -0500 Subject: [PATCH 07/10] Filter only invalid formed transactions and enforce one per signer in stacks-signer Signed-off-by: Jacinta Ferrant --- stacks-signer/src/client/mod.rs | 9 - stacks-signer/src/runloop.rs | 8 +- stacks-signer/src/signer.rs | 734 ++++++++++++-------------------- 3 files changed, 271 insertions(+), 480 deletions(-) diff --git a/stacks-signer/src/client/mod.rs b/stacks-signer/src/client/mod.rs index 959ce56db..138913ee6 100644 --- a/stacks-signer/src/client/mod.rs +++ b/stacks-signer/src/client/mod.rs @@ -207,15 +207,6 @@ pub(crate) mod tests { TcpListener::bind(config.node_host).unwrap() } - /// Create a mock server on the same port as the config and write a response to it - pub fn mock_server_from_config_and_write_response( - config: &GlobalConfig, - bytes: &[u8], - ) -> [u8; 1024] { - let mock_server = mock_server_from_config(config); - write_response(mock_server, bytes) - } - /// Write a response to the mock server and return the request bytes pub fn write_response(mock_server: TcpListener, bytes: &[u8]) -> [u8; 1024] { debug!("Writing a response..."); diff --git a/stacks-signer/src/runloop.rs b/stacks-signer/src/runloop.rs index 0b6a0c05a..02fb494c6 100644 --- a/stacks-signer/src/runloop.rs +++ b/stacks-signer/src/runloop.rs @@ -156,6 +156,12 @@ impl RunLoop { if signer.reward_cycle == prior_reward_cycle { // The signers have been calculated for the next reward cycle. Update the current one debug!("Signer #{}: Next reward cycle ({reward_cycle}) signer set calculated. Updating current reward cycle ({prior_reward_cycle}) signer.", signer.signer_id); + signer.next_signers = new_signer_config + .registered_signers + .signer_ids + .keys() + .copied() + .collect(); signer.next_signer_ids = new_signer_config .registered_signers .signer_ids @@ -201,7 +207,7 @@ impl RunLoop { if signer.approved_aggregate_public_key.is_none() { retry_with_exponential_backoff(|| { signer - .update_dkg(&self.stacks_client, current_reward_cycle) + .update_dkg(&self.stacks_client) .map_err(backoff::Error::transient) })?; } diff --git a/stacks-signer/src/signer.rs b/stacks-signer/src/signer.rs index 27f42dd69..f2e5c92e3 100644 --- a/stacks-signer/src/signer.rs +++ b/stacks-signer/src/signer.rs @@ -134,10 +134,14 @@ pub struct Signer { pub signer_ids: Vec, /// The addresses of other signers mapped to their signer slot ID pub signer_slot_ids: HashMap, + /// The addresses of other signers + pub signers: Vec, /// The other signer ids for the NEXT reward cycle's signers pub next_signer_ids: Vec, /// The signer addresses mapped to slot ID for the NEXT reward cycle's signers pub next_signer_slot_ids: HashMap, + /// The addresses of the signers for the NEXT reward cycle + pub next_signers: Vec, /// The reward cycle this signer belongs to pub reward_cycle: u64, /// The tx fee in uSTX to use if the epoch is pre Nakamoto (Epoch 3.0) @@ -209,8 +213,15 @@ impl From for Signer { .copied() .collect(), signer_slot_ids: signer_config.registered_signers.signer_slot_ids, + signers: signer_config + .registered_signers + .signer_ids + .keys() + .copied() + .collect(), next_signer_ids: vec![], next_signer_slot_ids: HashMap::new(), + next_signers: vec![], reward_cycle: signer_config.reward_cycle, tx_fee_ustx: signer_config.tx_fee_ustx, coordinator_selector, @@ -353,7 +364,6 @@ impl Signer { stacks_client: &StacksClient, block_validate_response: &BlockValidateResponse, res: Sender>, - current_reward_cycle: u64, ) { let block_info = match block_validate_response { BlockValidateResponse::Ok(block_validate_ok) => { @@ -364,11 +374,7 @@ impl Signer { 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, - current_reward_cycle, - ); + let is_valid = self.verify_block_transactions(stacks_client, &block_info.block); block_info.valid = Some(is_valid); info!( "Signer #{}: Treating block validation for block {} as valid: {:?}", @@ -413,7 +419,7 @@ impl Signer { msg: Message::NonceRequest(nonce_request), sig: vec![], }; - self.handle_packets(stacks_client, res, &[packet], current_reward_cycle); + self.handle_packets(stacks_client, res, &[packet]); } else { let coordinator_id = self.coordinator_selector.get_coordinator().0; if block_info.valid.unwrap_or(false) @@ -449,7 +455,6 @@ impl Signer { stacks_client: &StacksClient, res: Sender>, messages: &[SignerMessage], - current_reward_cycle: u64, ) { let coordinator_pubkey = self.coordinator_selector.get_coordinator().1; let packets: Vec = messages @@ -462,7 +467,7 @@ impl Signer { } }) .collect(); - self.handle_packets(stacks_client, res, &packets, current_reward_cycle); + self.handle_packets(stacks_client, res, &packets); } /// Handle proposed blocks submitted by the miners to stackerdb @@ -492,7 +497,6 @@ impl Signer { stacks_client: &StacksClient, res: Sender>, packets: &[Packet], - current_reward_cycle: u64, ) { let signer_outbound_messages = self .signing_round @@ -520,7 +524,7 @@ impl Signer { if !operation_results.is_empty() { // We have finished a signing or DKG round, either successfully or due to error. // Regardless of the why, update our state to Idle as we should not expect the operation to continue. - self.process_operation_results(stacks_client, &operation_results, current_reward_cycle); + self.process_operation_results(stacks_client, &operation_results); self.send_operation_results(res, operation_results); self.finish_operation(); } else if !packets.is_empty() && self.coordinator.state != CoordinatorState::Idle { @@ -631,7 +635,6 @@ impl Signer { &mut self, stacks_client: &StacksClient, block: &NakamotoBlock, - current_reward_cycle: u64, ) -> bool { if self.approved_aggregate_public_key.is_some() { // We do not enforce a block contain any transactions except the aggregate votes when it is NOT already set @@ -639,9 +642,7 @@ impl Signer { debug!("Signer #{}: Already have an aggregate key for reward cycle {}. Skipping transaction verification...", self.signer_id, self.reward_cycle); return true; } - if let Ok(expected_transactions) = - self.get_expected_transactions(stacks_client, current_reward_cycle) - { + if let Ok(expected_transactions) = self.get_expected_transactions(stacks_client) { //It might be worth building a hashset of the blocks' txids and checking that against the expected transaction's txid. let block_tx_hashset = block.txs.iter().map(|tx| tx.txid()).collect::>(); // Ensure the block contains the transactions we expect @@ -708,144 +709,73 @@ impl Signer { } } - /// Filter out transactions from the stackerdb that are not valid - /// i.e. not valid vote-for-aggregate-public-key transactions from registered signers - fn filter_invalid_transactions( + /// Select one transaction per address by sorting based first on nonce and then txid + fn filter_one_transaction_per_address( &self, - stacks_client: &StacksClient, - current_reward_cycle: u64, - signer_slot_ids: &HashMap, - transaction: StacksTransaction, - ) -> Option { - // Filter out transactions that have already been confirmed (can happen if a signer did not update stacker db since the last block was processed) + transactions: Vec, + ) -> Vec { + let mut filtered_transactions: HashMap = HashMap::new(); + for transaction in transactions { + let origin_address = transaction.origin_address(); + let origin_nonce = transaction.get_origin_nonce(); + if let Some(entry) = filtered_transactions.get_mut(&origin_address) { + let entry_nonce = entry.get_origin_nonce(); + if entry_nonce > origin_nonce + || (entry_nonce == origin_nonce && entry.txid() > transaction.txid()) + { + *entry = transaction; + } + } else { + filtered_transactions.insert(origin_address, transaction); + } + } + filtered_transactions.into_values().collect() + } + + /// Verify that the transaction is a valid vote for the aggregate public key + /// Note: it does not verify the function arguments, only that the transaction is validly formed + fn valid_vote_transaction( + &self, + account_nonces: &HashMap, + transaction: &StacksTransaction, + ) -> bool { let origin_address = transaction.origin_address(); let origin_nonce = transaction.get_origin_nonce(); - let Some(origin_signer_id) = signer_slot_ids.get(&origin_address) else { + let Some(account_nonce) = account_nonces.get(&origin_address) else { debug!( - "Signer #{}: Unrecognized origin address ({origin_address}). Filtering ({}).", + "Signer #{}: Unrecognized origin address ({origin_address}).", self.signer_id, - transaction.txid() ); - return None; + return false; }; - let Ok(account_nonce) = retry_with_exponential_backoff(|| { - stacks_client - .get_account_nonce(&origin_address) - .map_err(backoff::Error::transient) - }) else { - warn!( - "Signer #{}: Unable to get account for transaction origin address: {origin_address}. Filtering ({}).", - self.signer_id, - transaction.txid() - ); - return None; - }; - // TODO: add a check that we don't have two conflicting transactions in the same block from the same signer. This is a potential attack vector (will result in an invalid block) - if origin_nonce < account_nonce { - debug!("Signer #{}: Received a transaction with an outdated nonce ({account_nonce} < {origin_nonce}). Filtering ({}).", self.signer_id, transaction.txid()); - return None; - } if transaction.is_mainnet() != self.mainnet { debug!( - "Signer #{}: Received a transaction with an unexpected network. Filtering ({}).", + "Signer #{}: Received a transaction for an unexpected network.", self.signer_id, - transaction.txid() ); - return None; + return false; } - let Ok(valid) = retry_with_exponential_backoff(|| { - self.verify_payload( - stacks_client, - &transaction, - *origin_signer_id, - current_reward_cycle, - ) - .map_err(backoff::Error::transient) - }) else { - warn!( - "Signer #{}: Unable to validate transaction payload. Filtering ({}).", - self.signer_id, - transaction.txid() - ); - return None; - }; - if !valid { - debug!( - "Signer #{}: Received a transaction with an invalid payload. Filtering ({}).", - self.signer_id, - transaction.txid() - ); - return None; + if origin_nonce < *account_nonce { + debug!("Signer #{}: Received a transaction with an outdated nonce ({account_nonce} < {origin_nonce}).", self.signer_id); + return false; } - debug!( - "Signer #{}: Expect transaction {} ({transaction:?})", - self.signer_id, - transaction.txid() - ); - Some(transaction) + Self::parse_vote_for_aggregate_public_key(transaction).is_some() } - ///Helper function to verify the payload contents of a transaction are as expected - fn verify_payload( - &self, - stacks_client: &StacksClient, - transaction: &StacksTransaction, - origin_signer_id: u32, - current_reward_cycle: u64, - ) -> Result { - let Some((index, _point, round, reward_cycle)) = - Self::parse_vote_for_aggregate_public_key(transaction) - else { - // The transaction is not a valid vote-for-aggregate-public-key transaction - return Ok(false); - }; - if index != origin_signer_id as u64 { - // The signer is attempting to vote for another signer id than their own - return Ok(false); - } - let next_reward_cycle = current_reward_cycle.wrapping_add(1); - if reward_cycle != next_reward_cycle { - // The signer is attempting to vote for a reward cycle that is not the next reward cycle - return Ok(false); - } - - let vote = stacks_client.get_vote_for_aggregate_public_key( - round, - reward_cycle, - transaction.origin_address(), - )?; - if vote.is_some() { - // The signer has already voted for this round and reward cycle - return Ok(false); - } - - let last_round = stacks_client.get_last_round(reward_cycle)?; - // TODO: should we impose a limit on the number of special cased transactions allowed for a single signer at any given time?? In theory only 1 would be required per dkg round i.e. per block - if last_round.unwrap_or(0).saturating_add(1) < round { - // Do not allow future votes. This is to prevent signers sending a bazillion votes for a future round and clogging the block space - // The signer is attempting to vote for a round that is greater than one past the last round - return Ok(false); - } - Ok(true) - } - - /// Get this signer's transactions from stackerdb, filtering out any invalid transactions + /// Get transactions from stackerdb for the given addresses and account nonces, filtering out any malformed transactions fn get_signer_transactions( &mut self, - stacks_client: &StacksClient, - current_reward_cycle: u64, + nonces: &HashMap, ) -> Result, ClientError> { let transactions: Vec<_> = self .stackerdb .get_current_transactions_with_retry(self.signer_id)? .into_iter() .filter_map(|tx| { - self.filter_invalid_transactions( - stacks_client, - current_reward_cycle, - &self.signer_slot_ids, - tx, - ) + if !self.valid_vote_transaction(nonces, &tx) { + return None; + } + Some(tx) }) .collect(); Ok(transactions) @@ -855,7 +785,6 @@ impl Signer { fn get_expected_transactions( &mut self, stacks_client: &StacksClient, - current_reward_cycle: u64, ) -> Result, ClientError> { if self.next_signer_ids.is_empty() { debug!( @@ -864,20 +793,22 @@ impl Signer { ); return Ok(vec![]); } + // Get all the account nonces for the next signers + let account_nonces = self.get_account_nonces(stacks_client, &self.next_signers); let transactions: Vec<_> = self .stackerdb .get_next_transactions_with_retry(&self.next_signer_ids)? .into_iter() .filter_map(|tx| { - self.filter_invalid_transactions( - stacks_client, - current_reward_cycle, - &self.next_signer_slot_ids, - tx, - ) + if !self.valid_vote_transaction(&account_nonces, &tx) { + return None; + } + Some(tx) }) .collect(); - Ok(transactions) + + // We only allow enforcement of one special cased transaction per signer address per block + Ok(self.filter_one_transaction_per_address(transactions)) } /// Determine the vote for a block and update the block info and nonce request accordingly @@ -954,7 +885,6 @@ impl Signer { &mut self, stacks_client: &StacksClient, operation_results: &[OperationResult], - current_reward_cycle: u64, ) { for operation_result in operation_results { // Signers only every trigger non-taproot signing rounds over blocks. Ignore SignTaproot results @@ -967,7 +897,7 @@ impl Signer { debug!("Signer #{}: Received a signature result for a taproot signature. Nothing to broadcast as we currently sign blocks with a FROST signature.", self.signer_id); } OperationResult::Dkg(point) => { - self.process_dkg(stacks_client, point, current_reward_cycle); + self.process_dkg(stacks_client, point); } OperationResult::SignError(e) => { warn!("Signer #{}: Received a Sign error: {e:?}", self.signer_id); @@ -982,12 +912,7 @@ impl Signer { } /// Process a dkg result by broadcasting a vote to the stacks node - fn process_dkg( - &mut self, - stacks_client: &StacksClient, - point: &Point, - current_reward_cycle: u64, - ) { + fn process_dkg(&mut self, stacks_client: &StacksClient, point: &Point) { let epoch = retry_with_exponential_backoff(|| { stacks_client .get_node_epoch() @@ -1004,19 +929,41 @@ impl Signer { None }; // Get our current nonce from the stacks node and compare it against what we have sitting in the stackerdb instance - let nonce = self.get_next_nonce(stacks_client, current_reward_cycle); + let signer_address = stacks_client.get_signer_address(); + // Retreieve ALL account nonces as we may have transactions from other signers in our stackerdb slot that we care about + let account_nonces = self.get_account_nonces(stacks_client, &self.signers); + let account_nonce = account_nonces.get(signer_address).unwrap_or(&0); + let signer_transactions = retry_with_exponential_backoff(|| { + self.get_signer_transactions(&account_nonces) + .map_err(backoff::Error::transient) + }) + .map_err(|e| { + warn!( + "Signer #{}: Unable to get signer transactions: {e:?}", + self.signer_id + ); + }) + .unwrap_or_default(); + // If we have a transaction in the stackerdb slot, we need to increment the nonce hence the +1, else should use the account nonce + let next_nonce = signer_transactions + .first() + .map(|tx| tx.get_origin_nonce().wrapping_add(1)) + .unwrap_or(*account_nonce); match stacks_client.build_vote_for_aggregate_public_key( self.stackerdb.get_signer_slot_id(), self.coordinator.current_dkg_id, *point, self.reward_cycle, tx_fee, - nonce, + next_nonce, ) { - Ok(transaction) => { - if let Err(e) = - self.broadcast_dkg_vote(stacks_client, transaction, epoch, current_reward_cycle) - { + Ok(new_transaction) => { + if let Err(e) = self.broadcast_dkg_vote( + stacks_client, + epoch, + signer_transactions, + new_transaction, + ) { warn!( "Signer #{}: Failed to broadcast DKG vote ({point:?}): {e:?}", self.signer_id @@ -1032,42 +979,47 @@ impl Signer { } } - /// Get the next available nonce, taking into consideration the nonce we have sitting in stackerdb as well as the account nonce - fn get_next_nonce(&mut self, stacks_client: &StacksClient, current_reward_cycle: u64) -> u64 { - let signer_address = stacks_client.get_signer_address(); - let mut next_nonce = stacks_client - .get_account_nonce(signer_address) - .map_err(|e| { + // Get the account nonces for the provided list of signer addresses + fn get_account_nonces( + &self, + stacks_client: &StacksClient, + signer_addresses: &[StacksAddress], + ) -> HashMap { + let mut account_nonces = HashMap::with_capacity(signer_addresses.len()); + for address in signer_addresses { + let Ok(account_nonce) = retry_with_exponential_backoff(|| { + stacks_client + .get_account_nonce(address) + .map_err(backoff::Error::transient) + }) else { warn!( - "Signer #{}: Failed to get account nonce for signer: {e:?}", + "Signer #{}: Unable to get account nonce for address: {address}.", self.signer_id ); - }) - .unwrap_or(0); - - let current_transactions = self.get_signer_transactions(stacks_client, current_reward_cycle).map_err(|e| { - warn!("Signer #{}: Failed to get old transactions: {e:?}. Defaulting to account nonce.", self.signer_id); - }).unwrap_or_default(); - - for transaction in current_transactions { - let origin_nonce = transaction.get_origin_nonce(); - let origin_address = transaction.origin_address(); - if origin_address == *signer_address && origin_nonce >= next_nonce { - next_nonce = origin_nonce.wrapping_add(1); - } + continue; + }; + account_nonces.insert(*address, account_nonce); } - next_nonce + account_nonces } /// broadcast the dkg vote transaction according to the current epoch fn broadcast_dkg_vote( &mut self, stacks_client: &StacksClient, - new_transaction: StacksTransaction, epoch: StacksEpochId, - current_reward_cycle: u64, + mut signer_transactions: Vec, + new_transaction: StacksTransaction, ) -> Result<(), ClientError> { let txid = new_transaction.txid(); + if self.approved_aggregate_public_key.is_some() { + // We do not enforce a block contain any transactions except the aggregate votes when it is NOT already set + info!( + "Signer #{}: Already has an aggregate key for reward cycle {}. Do not broadcast the transaction ({txid:?}).", + self.signer_id, self.reward_cycle + ); + return Ok(()); + } if epoch >= StacksEpochId::Epoch30 { debug!("Signer #{}: Received a DKG result while in epoch 3.0. Broadcast the transaction only to stackerDB.", self.signer_id); } else if epoch == StacksEpochId::Epoch25 { @@ -1082,23 +1034,8 @@ impl Signer { return Ok(()); } // For all Pox-4 epochs onwards, broadcast the results also to stackerDB for other signers/miners to observe - // TODO: Should we even store transactions if not in prepare phase? Should the miner just ignore all signer transactions if not in prepare phase? - let txid = new_transaction.txid(); - let new_transactions = if self.approved_aggregate_public_key.is_some() { - // We do not enforce a block contain any transactions except the aggregate votes when it is NOT already set - info!( - "Signer #{}: Already has an aggregate key for reward cycle {}. Do not broadcast the transaction ({txid:?}).", - self.signer_id, self.reward_cycle - ); - vec![] - } else { - let mut new_transactions = self.get_signer_transactions(stacks_client, current_reward_cycle).map_err(|e| { - warn!("Signer #{}: Failed to get old transactions: {e:?}. Potentially overwriting our existing stackerDB transactions", self.signer_id); - }).unwrap_or_default(); - new_transactions.push(new_transaction); - new_transactions - }; - let signer_message = SignerMessage::Transactions(new_transactions); + signer_transactions.push(new_transaction); + let signer_message = SignerMessage::Transactions(signer_transactions); self.stackerdb.send_message_with_retry(signer_message)?; info!( "Signer #{}: Broadcasted DKG vote transaction ({txid}) to stacker DB", @@ -1225,11 +1162,7 @@ impl Signer { } /// Update the DKG for the provided signer info, triggering it if required - pub fn update_dkg( - &mut self, - stacks_client: &StacksClient, - current_reward_cycle: u64, - ) -> Result<(), ClientError> { + pub fn update_dkg(&mut self, stacks_client: &StacksClient) -> Result<(), ClientError> { let reward_cycle = self.reward_cycle; self.approved_aggregate_public_key = stacks_client.get_approved_aggregate_key(reward_cycle)?; @@ -1249,30 +1182,29 @@ impl Signer { let coordinator_id = self.coordinator_selector.get_coordinator().0; if self.signer_id == coordinator_id && self.state == State::Idle { debug!( - "Signer #{}: Checking if old transactions exist", + "Signer #{}: Checking if old vote transaction exists in StackerDB...", self.signer_id ); // Have I already voted and have a pending transaction? Check stackerdb for the same round number and reward cycle vote transaction - let old_transactions = self.get_signer_transactions(stacks_client, current_reward_cycle).map_err(|e| { - warn!("Signer #{}: Failed to get old transactions: {e:?}. Potentially overwriting our existing transactions", self.signer_id); + // Only get the account nonce of THIS signer as we only care about our own votes, not other signer votes + let signer_address = stacks_client.get_signer_address(); + let account_nonces = self.get_account_nonces(stacks_client, &[signer_address.clone()]); + let old_transactions = self.get_signer_transactions(&account_nonces).map_err(|e| { + warn!("Signer #{}: Failed to get old signer transactions: {e:?}. May trigger DKG unnecessarily", self.signer_id); }).unwrap_or_default(); // Check if we have an existing vote transaction for the same round and reward cycle for transaction in old_transactions.iter() { - let origin_address = transaction.origin_address(); - if &origin_address != stacks_client.get_signer_address() { - continue; - } - let Some((_index, point, round, _reward_cycle)) = - Self::parse_vote_for_aggregate_public_key(transaction) - else { - // The transaction is not a valid vote-for-aggregate-public-key transaction - error!("BUG: Signer #{}: Received an unrecognized transaction ({}) in an already filtered list: {transaction:?}", self.signer_id, transaction.txid()); - continue; - }; + let (_index, point, round, reward_cycle) = + Self::parse_vote_for_aggregate_public_key(transaction).expect(&format!("BUG: Signer #{}: Received an invalid {VOTE_FUNCTION_NAME} transaction in an already filtered list: {transaction:?}", self.signer_id)); if Some(point) == self.coordinator.aggregate_public_key && round == self.coordinator.current_dkg_id + && reward_cycle == self.reward_cycle { - debug!("Signer #{}: Not triggering a DKG round. Already have a pending vote transaction for aggregate public key {point:?} for round {round}...", self.signer_id); + debug!("Signer #{}: Not triggering a DKG round. Already have a pending vote transaction.", self.signer_id; + "txid" => %transaction.txid(), + "point" => %point, + "round" => round + ); return Ok(()); } } @@ -1312,12 +1244,7 @@ impl Signer { "Signer #{}: Received a block proposal result from the stacks node...", self.signer_id ); - self.handle_block_validate_response( - stacks_client, - block_validate_response, - res, - current_reward_cycle, - ) + self.handle_block_validate_response(stacks_client, block_validate_response, res) } Some(SignerEvent::SignerMessages(signer_set, messages)) => { if *signer_set != self.stackerdb.get_signer_set() { @@ -1329,7 +1256,7 @@ impl Signer { self.signer_id, messages.len() ); - self.handle_signer_messages(stacks_client, res, messages, current_reward_cycle); + self.handle_signer_messages(stacks_client, res, messages); } Some(SignerEvent::ProposedBlocks(blocks)) => { if current_reward_cycle != self.reward_cycle { @@ -1388,7 +1315,6 @@ impl Signer { #[cfg(test)] mod tests { - use std::thread::spawn; use blockstack_lib::chainstate::stacks::boot::SIGNERS_VOTING_NAME; use blockstack_lib::chainstate::stacks::{ @@ -1398,113 +1324,24 @@ mod tests { use blockstack_lib::util_lib::boot::boot_code_id; use blockstack_lib::util_lib::strings::StacksString; use clarity::vm::Value; + use hashbrown::HashMap; use rand::thread_rng; use rand_core::RngCore; - use serial_test::serial; use stacks_common::consts::CHAIN_ID_TESTNET; use stacks_common::types::chainstate::StacksPrivateKey; use wsts::curve::point::Point; use wsts::curve::scalar::Scalar; - use crate::client::tests::{ - build_account_nonce_response, build_get_approved_aggregate_key_response, - build_get_last_round_response, generate_signer_config, mock_server_from_config, - mock_server_from_config_and_write_response, write_response, - }; + use crate::client::tests::generate_signer_config; use crate::client::{StacksClient, VOTE_FUNCTION_NAME}; use crate::config::GlobalConfig; use crate::signer::Signer; #[test] - fn filter_invalid_transaction_bad_origin_id() { + fn valid_vote_transaction() { let config = GlobalConfig::load_from_file("./src/tests/conf/signer-0.toml").unwrap(); - let signer_config = generate_signer_config(&config, 2, 20); + let signer_config = generate_signer_config(&config, 5, 20); let signer = Signer::from(signer_config.clone()); - let stacks_client = StacksClient::from(&config); - let signer_private_key = StacksPrivateKey::new(); - let invalid_tx = StacksTransaction { - version: TransactionVersion::Testnet, - chain_id: CHAIN_ID_TESTNET, - auth: TransactionAuth::from_p2pkh(&signer_private_key).unwrap(), - anchor_mode: TransactionAnchorMode::Any, - post_condition_mode: TransactionPostConditionMode::Allow, - post_conditions: vec![], - payload: TransactionPayload::SmartContract( - TransactionSmartContract { - name: "test-contract".into(), - code_body: StacksString::from_str("(/ 1 0)").unwrap(), - }, - None, - ), - }; - assert!(signer - .filter_invalid_transactions(&stacks_client, 0, &signer.signer_slot_ids, invalid_tx) - .is_none()); - } - - #[test] - #[serial] - fn filter_invalid_transaction_bad_nonce() { - let config = GlobalConfig::load_from_file("./src/tests/conf/signer-0.toml").unwrap(); - let signer_config = generate_signer_config(&config, 2, 20); - let signer = Signer::from(signer_config.clone()); - let stacks_client = StacksClient::from(&config); - let signer_private_key = config.stacks_private_key; - let vote_contract_id = boot_code_id(SIGNERS_VOTING_NAME, signer.mainnet); - let contract_addr = vote_contract_id.issuer.into(); - let contract_name = vote_contract_id.name.clone(); - let signer_index = Value::UInt(signer.signer_id as u128); - let point = Point::from(Scalar::random(&mut thread_rng())); - let point_arg = - Value::buff_from(point.compress().data.to_vec()).expect("Failed to create buff"); - let round = thread_rng().next_u64(); - let round_arg = Value::UInt(round as u128); - let reward_cycle_arg = Value::UInt(signer.reward_cycle as u128); - let valid_function_args = vec![ - signer_index.clone(), - point_arg.clone(), - round_arg.clone(), - reward_cycle_arg.clone(), - ]; - let invalid_tx = StacksClient::build_signed_contract_call_transaction( - &contract_addr, - contract_name.clone(), - VOTE_FUNCTION_NAME.into(), - &valid_function_args, - &signer_private_key, - TransactionVersion::Testnet, - config.network.to_chain_id(), - 0, // Old nonce - 10, - ) - .unwrap(); - - let h = spawn(move || { - signer.filter_invalid_transactions( - &stacks_client, - 0, - &signer.signer_slot_ids, - invalid_tx, - ) - }); - - let response = build_account_nonce_response(1); - let mock_server = mock_server_from_config(&config); - write_response(mock_server, response.as_bytes()); - assert!(h.join().unwrap().is_none()); - } - - #[test] - #[serial] - fn verify_valid_transaction() { - // Create a runloop of a valid signer - let config = GlobalConfig::load_from_file("./src/tests/conf/signer-0.toml").unwrap(); - let mut signer_config = generate_signer_config(&config, 5, 20); - signer_config.reward_cycle = 1; - - // valid transaction - let signer = Signer::from(signer_config.clone()); - let stacks_client = StacksClient::from(&config); let signer_private_key = config.stacks_private_key; let vote_contract_id = boot_code_id(SIGNERS_VOTING_NAME, signer.mainnet); @@ -1523,7 +1360,7 @@ mod tests { round_arg.clone(), reward_cycle_arg.clone(), ]; - let valid_transaction = StacksClient::build_signed_contract_call_transaction( + let valid_tx = StacksClient::build_signed_contract_call_transaction( &contract_addr, contract_name.clone(), VOTE_FUNCTION_NAME.into(), @@ -1535,38 +1372,15 @@ mod tests { 10, ) .unwrap(); - - let vote_response = build_get_approved_aggregate_key_response(None); - let last_round_response = build_get_last_round_response(round); - - let h = spawn(move || { - assert!(signer - .verify_payload( - &stacks_client, - &valid_transaction, - signer.signer_id, - signer.reward_cycle.saturating_sub(1) - ) - .unwrap()) - }); - - let mock_server = mock_server_from_config(&config); - write_response(mock_server, vote_response.as_bytes()); - - let mock_server = mock_server_from_config(&config); - write_response(mock_server, last_round_response.as_bytes()); - - h.join().unwrap(); + let mut account_nonces = HashMap::new(); + account_nonces.insert(valid_tx.origin_address(), 1); + assert!(signer.valid_vote_transaction(&account_nonces, &valid_tx)); } #[test] - #[serial] - fn verify_transaction_filters_malformed_contract_calls() { - // Create a runloop of a valid signer + fn valid_vote_transaction_malformed_transactions() { let config = GlobalConfig::load_from_file("./src/tests/conf/signer-0.toml").unwrap(); - let mut signer_config = generate_signer_config(&config, 5, 20); - signer_config.reward_cycle = 1; - + let signer_config = generate_signer_config(&config, 5, 20); let signer = Signer::from(signer_config.clone()); let signer_private_key = config.stacks_private_key; @@ -1641,23 +1455,6 @@ mod tests { 10, ) .unwrap(); - let invalid_signer_id_argument = StacksClient::build_signed_contract_call_transaction( - &contract_addr, - contract_name.clone(), - VOTE_FUNCTION_NAME.into(), - &[ - Value::UInt(signer.signer_id.wrapping_add(1) as u128), // Not the signers id - point_arg.clone(), - round_arg.clone(), - reward_cycle_arg.clone(), - ], - &signer_private_key, - TransactionVersion::Testnet, - config.network.to_chain_id(), - 1, - 10, - ) - .unwrap(); let invalid_function_arg_signer_index = StacksClient::build_signed_contract_call_transaction( @@ -1733,42 +1530,44 @@ mod tests { ) .unwrap(); - let stacks_client = StacksClient::from(&config); + let invalid_nonce = StacksClient::build_signed_contract_call_transaction( + &contract_addr, + contract_name.clone(), + VOTE_FUNCTION_NAME.into(), + &valid_function_args, + &signer_private_key, + TransactionVersion::Testnet, + config.network.to_chain_id(), + 0, // Old nonce + 10, + ) + .unwrap(); + + let mut account_nonces = HashMap::new(); + account_nonces.insert(invalid_not_contract_call.origin_address(), 1); for tx in vec![ invalid_not_contract_call, invalid_signers_contract_addr, invalid_signers_contract_name, invalid_signers_vote_function, - invalid_signer_id_argument, invalid_function_arg_signer_index, invalid_function_arg_key, invalid_function_arg_round, invalid_function_arg_reward_cycle, + invalid_nonce, ] { - let result = signer - .verify_payload( - &stacks_client, - &tx, - signer.signer_id, - signer.reward_cycle.saturating_sub(1), - ) - .unwrap(); - assert!(!result); + assert!(!signer.valid_vote_transaction(&account_nonces, &tx)); } } #[test] - #[serial] - fn verify_transaction_filters_invalid_reward_cycle() { - // Create a runloop of a valid signer + fn filter_one_transaction_per_signer_multiple_addresses() { let config = GlobalConfig::load_from_file("./src/tests/conf/signer-0.toml").unwrap(); - let mut signer_config = generate_signer_config(&config, 5, 20); - signer_config.reward_cycle = 1; - + let signer_config = generate_signer_config(&config, 5, 20); let signer = Signer::from(signer_config.clone()); - let stacks_client = StacksClient::from(&config); - let signer_private_key = config.stacks_private_key; + let signer_private_key_1 = config.stacks_private_key; + let signer_private_key_2 = StacksPrivateKey::new(); let vote_contract_id = boot_code_id(SIGNERS_VOTING_NAME, signer.mainnet); let contract_addr = vote_contract_id.issuer.into(); let contract_name = vote_contract_id.name.clone(); @@ -1785,41 +1584,85 @@ mod tests { round_arg.clone(), reward_cycle_arg.clone(), ]; - // Invalid reward cycle (voting for the current is not allowed. only the next) - let signer = Signer::from(signer_config.clone()); - let invalid_reward_cycle = StacksClient::build_signed_contract_call_transaction( + + let valid_tx_1_address_1 = StacksClient::build_signed_contract_call_transaction( &contract_addr, contract_name.clone(), VOTE_FUNCTION_NAME.into(), &valid_function_args, - &signer_private_key, + &signer_private_key_1, TransactionVersion::Testnet, config.network.to_chain_id(), 1, 10, ) .unwrap(); - let h = spawn(move || { - assert!(!signer - .verify_payload( - &stacks_client, - &invalid_reward_cycle, - signer.signer_id, - signer.reward_cycle - ) - .unwrap()) - }); - h.join().unwrap(); + let valid_tx_2_address_1 = StacksClient::build_signed_contract_call_transaction( + &contract_addr, + contract_name.clone(), + VOTE_FUNCTION_NAME.into(), + &valid_function_args, + &signer_private_key_1, + TransactionVersion::Testnet, + config.network.to_chain_id(), + 2, + 10, + ) + .unwrap(); + let valid_tx_3_address_1 = StacksClient::build_signed_contract_call_transaction( + &contract_addr, + contract_name.clone(), + VOTE_FUNCTION_NAME.into(), + &valid_function_args, + &signer_private_key_1, + TransactionVersion::Testnet, + config.network.to_chain_id(), + 3, + 10, + ) + .unwrap(); + + let valid_tx_1_address_2 = StacksClient::build_signed_contract_call_transaction( + &contract_addr, + contract_name.clone(), + VOTE_FUNCTION_NAME.into(), + &valid_function_args, + &signer_private_key_2, + TransactionVersion::Testnet, + config.network.to_chain_id(), + 1, + 10, + ) + .unwrap(); + + let valid_tx_2_address_2 = StacksClient::build_signed_contract_call_transaction( + &contract_addr, + contract_name.clone(), + VOTE_FUNCTION_NAME.into(), + &valid_function_args, + &signer_private_key_2, + TransactionVersion::Testnet, + config.network.to_chain_id(), + 1, + 10, + ) + .unwrap(); + + let txs = signer.filter_one_transaction_per_address(vec![ + valid_tx_1_address_1.clone(), + valid_tx_3_address_1, + valid_tx_2_address_2, + valid_tx_2_address_1.clone(), + ]); + assert_eq!(txs.len(), 2); + assert!(txs.contains(&valid_tx_1_address_1)); + assert!(txs.contains(&valid_tx_1_address_2)); } #[test] - #[serial] - fn verify_transaction_filters_already_voted() { - // Create a runloop of a valid signer + fn filter_one_transaction_per_signer_duplicate_nonces() { let config = GlobalConfig::load_from_file("./src/tests/conf/signer-0.toml").unwrap(); - let mut signer_config = generate_signer_config(&config, 5, 20); - signer_config.reward_cycle = 1; - + let signer_config = generate_signer_config(&config, 5, 20); let signer = Signer::from(signer_config.clone()); let signer_private_key = config.stacks_private_key; @@ -1839,11 +1682,8 @@ mod tests { round_arg.clone(), reward_cycle_arg.clone(), ]; - - // Already voted - let signer = Signer::from(signer_config.clone()); - let stacks_client = StacksClient::from(&config); - let invalid_already_voted = StacksClient::build_signed_contract_call_transaction( + let nonce = 0; + let valid_tx_1 = StacksClient::build_signed_contract_call_transaction( &contract_addr, contract_name.clone(), VOTE_FUNCTION_NAME.into(), @@ -1851,57 +1691,11 @@ mod tests { &signer_private_key, TransactionVersion::Testnet, config.network.to_chain_id(), - 1, + nonce, 10, ) .unwrap(); - - let vote_response = build_get_approved_aggregate_key_response(Some(point)); - - let h = spawn(move || { - assert!(!signer - .verify_payload( - &stacks_client, - &invalid_already_voted, - signer.signer_id, - signer.reward_cycle.saturating_sub(1) - ) - .unwrap()) - }); - mock_server_from_config_and_write_response(&config, vote_response.as_bytes()); - h.join().unwrap(); - } - - #[test] - #[serial] - fn verify_transaction_filters_ivalid_round_number() { - // Create a runloop of a valid signer - let config = GlobalConfig::load_from_file("./src/tests/conf/signer-0.toml").unwrap(); - let mut signer_config = generate_signer_config(&config, 5, 20); - signer_config.reward_cycle = 1; - - let signer = Signer::from(signer_config.clone()); - - let signer_private_key = config.stacks_private_key; - let vote_contract_id = boot_code_id(SIGNERS_VOTING_NAME, signer.mainnet); - let contract_addr = vote_contract_id.issuer.into(); - let contract_name = vote_contract_id.name.clone(); - let signer_index = Value::UInt(signer.signer_id as u128); - let point = Point::from(Scalar::random(&mut thread_rng())); - let point_arg = - Value::buff_from(point.compress().data.to_vec()).expect("Failed to create buff"); - let round = thread_rng().next_u64(); - let round_arg = Value::UInt(round as u128); - let reward_cycle_arg = Value::UInt(signer.reward_cycle as u128); - let valid_function_args = vec![ - signer_index.clone(), - point_arg.clone(), - round_arg.clone(), - reward_cycle_arg.clone(), - ]; - let signer = Signer::from(signer_config.clone()); - let stacks_client = StacksClient::from(&config); - let invalid_round_number = StacksClient::build_signed_contract_call_transaction( + let valid_tx_2 = StacksClient::build_signed_contract_call_transaction( &contract_addr, contract_name.clone(), VOTE_FUNCTION_NAME.into(), @@ -1909,27 +1703,27 @@ mod tests { &signer_private_key, TransactionVersion::Testnet, config.network.to_chain_id(), - 1, + nonce, + 10, + ) + .unwrap(); + let valid_tx_3 = StacksClient::build_signed_contract_call_transaction( + &contract_addr, + contract_name.clone(), + VOTE_FUNCTION_NAME.into(), + &valid_function_args, + &signer_private_key, + TransactionVersion::Testnet, + config.network.to_chain_id(), + nonce, 10, ) .unwrap(); - // invalid round number - let vote_response = build_get_approved_aggregate_key_response(None); - let last_round_response = build_get_last_round_response(0); - - let h = spawn(move || { - assert!(!signer - .verify_payload( - &stacks_client, - &invalid_round_number, - signer.signer_id, - signer.reward_cycle.saturating_sub(1) - ) - .unwrap()) - }); - mock_server_from_config_and_write_response(&config, vote_response.as_bytes()); - mock_server_from_config_and_write_response(&config, last_round_response.as_bytes()); - h.join().unwrap(); + let mut txs = vec![valid_tx_2, valid_tx_1, valid_tx_3]; + let filtered_txs = signer.filter_one_transaction_per_address(txs.clone()); + txs.sort_by(|a, b| a.txid().cmp(&b.txid())); + assert_eq!(filtered_txs.len(), 1); + assert!(filtered_txs.contains(&txs.first().expect("failed to get first tx"))); } } From f6aa7e351439bfa492866faa7678cf5c932e5f18 Mon Sep 17 00:00:00 2001 From: Jacinta Ferrant Date: Tue, 27 Feb 2024 13:12:20 -0500 Subject: [PATCH 08/10] Make filtering of signer transactions global and use in miner Signed-off-by: Jacinta Ferrant --- stacks-signer/src/client/stacks_client.rs | 11 +- stacks-signer/src/signer.rs | 540 +------------ .../src/chainstate/nakamoto/signer_set.rs | 84 +- .../src/chainstate/nakamoto/tests/mod.rs | 759 +++++++++++++++++- stackslib/src/chainstate/stacks/boot/mod.rs | 3 +- stackslib/src/net/tests/mod.rs | 8 +- testnet/stacks-node/src/mockamoto.rs | 4 +- .../stacks-node/src/nakamoto_node/miner.rs | 65 +- .../src/tests/nakamoto_integrations.rs | 8 +- testnet/stacks-node/src/tests/signer.rs | 226 +++++- 10 files changed, 1101 insertions(+), 607 deletions(-) diff --git a/stacks-signer/src/client/stacks_client.rs b/stacks-signer/src/client/stacks_client.rs index caa5c7018..5e422b3c6 100644 --- a/stacks-signer/src/client/stacks_client.rs +++ b/stacks-signer/src/client/stacks_client.rs @@ -17,7 +17,9 @@ use std::net::SocketAddr; use blockstack_lib::burnchains::Txid; use blockstack_lib::chainstate::nakamoto::NakamotoBlock; -use blockstack_lib::chainstate::stacks::boot::{RewardSet, SIGNERS_NAME, SIGNERS_VOTING_NAME}; +use blockstack_lib::chainstate::stacks::boot::{ + RewardSet, SIGNERS_NAME, SIGNERS_VOTING_FUNCTION_NAME, SIGNERS_VOTING_NAME, +}; use blockstack_lib::chainstate::stacks::{ StacksTransaction, StacksTransactionSigner, TransactionAnchorMode, TransactionAuth, TransactionContractCall, TransactionPayload, TransactionPostConditionMode, @@ -47,9 +49,6 @@ use wsts::state_machine::PublicKeys; use crate::client::{retry_with_exponential_backoff, ClientError}; use crate::config::{GlobalConfig, RegisteredSignersInfo}; -/// The name of the function for casting a DKG result to signer vote contract -pub const VOTE_FUNCTION_NAME: &str = "vote-for-aggregate-public-key"; - /// The Stacks signer client used to communicate with the stacks node #[derive(Clone, Debug)] pub struct StacksClient { @@ -502,10 +501,10 @@ impl StacksClient { tx_fee: Option, nonce: u64, ) -> Result { - debug!("Building {VOTE_FUNCTION_NAME} transaction..."); + debug!("Building {SIGNERS_VOTING_FUNCTION_NAME} transaction..."); let contract_address = boot_code_addr(self.mainnet); let contract_name = ContractName::from(SIGNERS_VOTING_NAME); - let function_name = ClarityName::from(VOTE_FUNCTION_NAME); + let function_name = ClarityName::from(SIGNERS_VOTING_FUNCTION_NAME); let function_args = vec![ ClarityValue::UInt(signer_index as u128), ClarityValue::buff_from(point.compress().data.to_vec())?, diff --git a/stacks-signer/src/signer.rs b/stacks-signer/src/signer.rs index f2e5c92e3..e759f2ff7 100644 --- a/stacks-signer/src/signer.rs +++ b/stacks-signer/src/signer.rs @@ -17,11 +17,11 @@ use std::collections::VecDeque; use std::sync::mpsc::Sender; use std::time::Instant; +use blockstack_lib::chainstate::nakamoto::signer_set::NakamotoSigners; use blockstack_lib::chainstate::nakamoto::{NakamotoBlock, NakamotoBlockVote}; -use blockstack_lib::chainstate::stacks::boot::SIGNERS_VOTING_NAME; -use blockstack_lib::chainstate::stacks::{StacksTransaction, TransactionPayload}; +use blockstack_lib::chainstate::stacks::boot::SIGNERS_VOTING_FUNCTION_NAME; +use blockstack_lib::chainstate::stacks::StacksTransaction; use blockstack_lib::net::api::postblock_proposal::BlockValidateResponse; -use blockstack_lib::util_lib::boot::boot_code_id; use hashbrown::{HashMap, HashSet}; use libsigner::{BlockRejection, BlockResponse, RejectCode, SignerEvent, SignerMessage}; use slog::{slog_debug, slog_error, slog_info, slog_warn}; @@ -32,7 +32,7 @@ use stacks_common::util::hash::Sha512Trunc256Sum; use stacks_common::{debug, error, info, warn}; use wsts::common::{MerkleRoot, Signature}; use wsts::curve::keys::PublicKey; -use wsts::curve::point::{Compressed, Point}; +use wsts::curve::point::Point; use wsts::net::{Message, NonceRequest, Packet, SignatureShareRequest}; use wsts::state_machine::coordinator::fire::Coordinator as FireCoordinator; use wsts::state_machine::coordinator::{ @@ -42,9 +42,7 @@ use wsts::state_machine::signer::Signer as WSTSSigner; use wsts::state_machine::{OperationResult, SignError}; use wsts::v2; -use crate::client::{ - retry_with_exponential_backoff, ClientError, StackerDB, StacksClient, VOTE_FUNCTION_NAME, -}; +use crate::client::{retry_with_exponential_backoff, ClientError, StackerDB, StacksClient}; use crate::config::SignerConfig; use crate::coordinator::CoordinatorSelector; @@ -709,70 +707,17 @@ impl Signer { } } - /// Select one transaction per address by sorting based first on nonce and then txid - fn filter_one_transaction_per_address( - &self, - transactions: Vec, - ) -> Vec { - let mut filtered_transactions: HashMap = HashMap::new(); - for transaction in transactions { - let origin_address = transaction.origin_address(); - let origin_nonce = transaction.get_origin_nonce(); - if let Some(entry) = filtered_transactions.get_mut(&origin_address) { - let entry_nonce = entry.get_origin_nonce(); - if entry_nonce > origin_nonce - || (entry_nonce == origin_nonce && entry.txid() > transaction.txid()) - { - *entry = transaction; - } - } else { - filtered_transactions.insert(origin_address, transaction); - } - } - filtered_transactions.into_values().collect() - } - - /// Verify that the transaction is a valid vote for the aggregate public key - /// Note: it does not verify the function arguments, only that the transaction is validly formed - fn valid_vote_transaction( - &self, - account_nonces: &HashMap, - transaction: &StacksTransaction, - ) -> bool { - let origin_address = transaction.origin_address(); - let origin_nonce = transaction.get_origin_nonce(); - let Some(account_nonce) = account_nonces.get(&origin_address) else { - debug!( - "Signer #{}: Unrecognized origin address ({origin_address}).", - self.signer_id, - ); - return false; - }; - if transaction.is_mainnet() != self.mainnet { - debug!( - "Signer #{}: Received a transaction for an unexpected network.", - self.signer_id, - ); - return false; - } - if origin_nonce < *account_nonce { - debug!("Signer #{}: Received a transaction with an outdated nonce ({account_nonce} < {origin_nonce}).", self.signer_id); - return false; - } - Self::parse_vote_for_aggregate_public_key(transaction).is_some() - } - /// Get transactions from stackerdb for the given addresses and account nonces, filtering out any malformed transactions fn get_signer_transactions( &mut self, - nonces: &HashMap, + nonces: &std::collections::HashMap, ) -> Result, ClientError> { let transactions: Vec<_> = self .stackerdb .get_current_transactions_with_retry(self.signer_id)? .into_iter() .filter_map(|tx| { - if !self.valid_vote_transaction(nonces, &tx) { + if !NakamotoSigners::valid_vote_transaction(nonces, &tx, self.mainnet) { return None; } Some(tx) @@ -797,18 +742,16 @@ impl Signer { let account_nonces = self.get_account_nonces(stacks_client, &self.next_signers); let transactions: Vec<_> = self .stackerdb - .get_next_transactions_with_retry(&self.next_signer_ids)? - .into_iter() - .filter_map(|tx| { - if !self.valid_vote_transaction(&account_nonces, &tx) { - return None; - } - Some(tx) - }) - .collect(); - + .get_next_transactions_with_retry(&self.next_signer_ids)?; + let mut filtered_transactions = std::collections::HashMap::new(); + NakamotoSigners::update_filtered_transactions( + &mut filtered_transactions, + &account_nonces, + self.mainnet, + transactions, + ); // We only allow enforcement of one special cased transaction per signer address per block - Ok(self.filter_one_transaction_per_address(transactions)) + Ok(filtered_transactions.into_values().collect()) } /// Determine the vote for a block and update the block info and nonce request accordingly @@ -984,8 +927,8 @@ impl Signer { &self, stacks_client: &StacksClient, signer_addresses: &[StacksAddress], - ) -> HashMap { - let mut account_nonces = HashMap::with_capacity(signer_addresses.len()); + ) -> std::collections::HashMap { + let mut account_nonces = std::collections::HashMap::with_capacity(signer_addresses.len()); for address in signer_addresses { let Ok(account_nonce) = retry_with_exponential_backoff(|| { stacks_client @@ -1195,7 +1138,7 @@ impl Signer { // Check if we have an existing vote transaction for the same round and reward cycle for transaction in old_transactions.iter() { let (_index, point, round, reward_cycle) = - Self::parse_vote_for_aggregate_public_key(transaction).expect(&format!("BUG: Signer #{}: Received an invalid {VOTE_FUNCTION_NAME} transaction in an already filtered list: {transaction:?}", self.signer_id)); + NakamotoSigners::parse_vote_for_aggregate_public_key(transaction).expect(&format!("BUG: Signer #{}: Received an invalid {SIGNERS_VOTING_FUNCTION_NAME} transaction in an already filtered list: {transaction:?}", self.signer_id)); if Some(point) == self.coordinator.aggregate_public_key && round == self.coordinator.current_dkg_id && reward_cycle == self.reward_cycle @@ -1281,449 +1224,4 @@ impl Signer { } Ok(()) } - - fn parse_vote_for_aggregate_public_key( - transaction: &StacksTransaction, - ) -> Option<(u64, Point, u64, u64)> { - let TransactionPayload::ContractCall(payload) = &transaction.payload else { - // Not a contract call so not a special cased vote for aggregate public key transaction - return None; - }; - if payload.contract_identifier() - != boot_code_id(SIGNERS_VOTING_NAME, transaction.is_mainnet()) - || payload.function_name != VOTE_FUNCTION_NAME.into() - { - // This is not a special cased transaction. - return None; - } - if payload.function_args.len() != 4 { - return None; - } - let signer_index_value = payload.function_args.first()?; - let signer_index = u64::try_from(signer_index_value.clone().expect_u128().ok()?).ok()?; - let point_value = payload.function_args.get(1)?; - let point_bytes = point_value.clone().expect_buff(33).ok()?; - let compressed_data = Compressed::try_from(point_bytes.as_slice()).ok()?; - let point = Point::try_from(&compressed_data).ok()?; - let round_value = payload.function_args.get(2)?; - let round = u64::try_from(round_value.clone().expect_u128().ok()?).ok()?; - let reward_cycle = - u64::try_from(payload.function_args.get(3)?.clone().expect_u128().ok()?).ok()?; - Some((signer_index, point, round, reward_cycle)) - } -} - -#[cfg(test)] -mod tests { - - use blockstack_lib::chainstate::stacks::boot::SIGNERS_VOTING_NAME; - use blockstack_lib::chainstate::stacks::{ - StacksTransaction, TransactionAnchorMode, TransactionAuth, TransactionPayload, - TransactionPostConditionMode, TransactionSmartContract, TransactionVersion, - }; - use blockstack_lib::util_lib::boot::boot_code_id; - use blockstack_lib::util_lib::strings::StacksString; - use clarity::vm::Value; - use hashbrown::HashMap; - use rand::thread_rng; - use rand_core::RngCore; - use stacks_common::consts::CHAIN_ID_TESTNET; - use stacks_common::types::chainstate::StacksPrivateKey; - use wsts::curve::point::Point; - use wsts::curve::scalar::Scalar; - - use crate::client::tests::generate_signer_config; - use crate::client::{StacksClient, VOTE_FUNCTION_NAME}; - use crate::config::GlobalConfig; - use crate::signer::Signer; - - #[test] - fn valid_vote_transaction() { - let config = GlobalConfig::load_from_file("./src/tests/conf/signer-0.toml").unwrap(); - let signer_config = generate_signer_config(&config, 5, 20); - let signer = Signer::from(signer_config.clone()); - - let signer_private_key = config.stacks_private_key; - let vote_contract_id = boot_code_id(SIGNERS_VOTING_NAME, signer.mainnet); - let contract_addr = vote_contract_id.issuer.into(); - let contract_name = vote_contract_id.name.clone(); - let signer_index = Value::UInt(signer.signer_id as u128); - let point = Point::from(Scalar::random(&mut thread_rng())); - let point_arg = - Value::buff_from(point.compress().data.to_vec()).expect("Failed to create buff"); - let round = thread_rng().next_u64(); - let round_arg = Value::UInt(round as u128); - let reward_cycle_arg = Value::UInt(signer.reward_cycle as u128); - let valid_function_args = vec![ - signer_index.clone(), - point_arg.clone(), - round_arg.clone(), - reward_cycle_arg.clone(), - ]; - let valid_tx = StacksClient::build_signed_contract_call_transaction( - &contract_addr, - contract_name.clone(), - VOTE_FUNCTION_NAME.into(), - &valid_function_args, - &signer_private_key, - TransactionVersion::Testnet, - config.network.to_chain_id(), - 1, - 10, - ) - .unwrap(); - let mut account_nonces = HashMap::new(); - account_nonces.insert(valid_tx.origin_address(), 1); - assert!(signer.valid_vote_transaction(&account_nonces, &valid_tx)); - } - - #[test] - fn valid_vote_transaction_malformed_transactions() { - let config = GlobalConfig::load_from_file("./src/tests/conf/signer-0.toml").unwrap(); - let signer_config = generate_signer_config(&config, 5, 20); - let signer = Signer::from(signer_config.clone()); - - let signer_private_key = config.stacks_private_key; - let vote_contract_id = boot_code_id(SIGNERS_VOTING_NAME, signer.mainnet); - let contract_addr = vote_contract_id.issuer.into(); - let contract_name = vote_contract_id.name.clone(); - let signer_index = Value::UInt(signer.signer_id as u128); - let point = Point::from(Scalar::random(&mut thread_rng())); - let point_arg = - Value::buff_from(point.compress().data.to_vec()).expect("Failed to create buff"); - let round = thread_rng().next_u64(); - let round_arg = Value::UInt(round as u128); - let reward_cycle_arg = Value::UInt(signer.reward_cycle as u128); - let valid_function_args = vec![ - signer_index.clone(), - point_arg.clone(), - round_arg.clone(), - reward_cycle_arg.clone(), - ]; - - let signer = Signer::from(signer_config.clone()); - // Create a invalid transaction that is not a contract call - let invalid_not_contract_call = StacksTransaction { - version: TransactionVersion::Testnet, - chain_id: CHAIN_ID_TESTNET, - auth: TransactionAuth::from_p2pkh(&signer_private_key).unwrap(), - anchor_mode: TransactionAnchorMode::Any, - post_condition_mode: TransactionPostConditionMode::Allow, - post_conditions: vec![], - payload: TransactionPayload::SmartContract( - TransactionSmartContract { - name: "test-contract".into(), - code_body: StacksString::from_str("(/ 1 0)").unwrap(), - }, - None, - ), - }; - let invalid_signers_contract_addr = StacksClient::build_signed_contract_call_transaction( - &config.stacks_address, // Not the signers contract address - contract_name.clone(), - VOTE_FUNCTION_NAME.into(), - &valid_function_args, - &signer_private_key, - TransactionVersion::Testnet, - config.network.to_chain_id(), - 1, - 10, - ) - .unwrap(); - let invalid_signers_contract_name = StacksClient::build_signed_contract_call_transaction( - &contract_addr, - "bad-signers-contract-name".into(), - VOTE_FUNCTION_NAME.into(), - &valid_function_args, - &signer_private_key, - TransactionVersion::Testnet, - config.network.to_chain_id(), - 1, - 10, - ) - .unwrap(); - - let invalid_signers_vote_function = StacksClient::build_signed_contract_call_transaction( - &contract_addr, - contract_name.clone(), - "some-other-function".into(), - &valid_function_args, - &signer_private_key, - TransactionVersion::Testnet, - config.network.to_chain_id(), - 1, - 10, - ) - .unwrap(); - - let invalid_function_arg_signer_index = - StacksClient::build_signed_contract_call_transaction( - &contract_addr, - contract_name.clone(), - VOTE_FUNCTION_NAME.into(), - &[ - point_arg.clone(), - point_arg.clone(), - round_arg.clone(), - reward_cycle_arg.clone(), - ], - &signer_private_key, - TransactionVersion::Testnet, - config.network.to_chain_id(), - 1, - 10, - ) - .unwrap(); - - let invalid_function_arg_key = StacksClient::build_signed_contract_call_transaction( - &contract_addr, - contract_name.clone(), - VOTE_FUNCTION_NAME.into(), - &[ - signer_index.clone(), - signer_index.clone(), - round_arg.clone(), - reward_cycle_arg.clone(), - ], - &signer_private_key, - TransactionVersion::Testnet, - config.network.to_chain_id(), - 1, - 10, - ) - .unwrap(); - - let invalid_function_arg_round = StacksClient::build_signed_contract_call_transaction( - &contract_addr, - contract_name.clone(), - VOTE_FUNCTION_NAME.into(), - &[ - signer_index.clone(), - point_arg.clone(), - point_arg.clone(), - reward_cycle_arg.clone(), - ], - &signer_private_key, - TransactionVersion::Testnet, - config.network.to_chain_id(), - 1, - 10, - ) - .unwrap(); - - let invalid_function_arg_reward_cycle = - StacksClient::build_signed_contract_call_transaction( - &contract_addr, - contract_name.clone(), - VOTE_FUNCTION_NAME.into(), - &[ - signer_index.clone(), - point_arg.clone(), - round_arg.clone(), - point_arg.clone(), - ], - &signer_private_key, - TransactionVersion::Testnet, - config.network.to_chain_id(), - 1, - 10, - ) - .unwrap(); - - let invalid_nonce = StacksClient::build_signed_contract_call_transaction( - &contract_addr, - contract_name.clone(), - VOTE_FUNCTION_NAME.into(), - &valid_function_args, - &signer_private_key, - TransactionVersion::Testnet, - config.network.to_chain_id(), - 0, // Old nonce - 10, - ) - .unwrap(); - - let mut account_nonces = HashMap::new(); - account_nonces.insert(invalid_not_contract_call.origin_address(), 1); - for tx in vec![ - invalid_not_contract_call, - invalid_signers_contract_addr, - invalid_signers_contract_name, - invalid_signers_vote_function, - invalid_function_arg_signer_index, - invalid_function_arg_key, - invalid_function_arg_round, - invalid_function_arg_reward_cycle, - invalid_nonce, - ] { - assert!(!signer.valid_vote_transaction(&account_nonces, &tx)); - } - } - - #[test] - fn filter_one_transaction_per_signer_multiple_addresses() { - let config = GlobalConfig::load_from_file("./src/tests/conf/signer-0.toml").unwrap(); - let signer_config = generate_signer_config(&config, 5, 20); - let signer = Signer::from(signer_config.clone()); - - let signer_private_key_1 = config.stacks_private_key; - let signer_private_key_2 = StacksPrivateKey::new(); - let vote_contract_id = boot_code_id(SIGNERS_VOTING_NAME, signer.mainnet); - let contract_addr = vote_contract_id.issuer.into(); - let contract_name = vote_contract_id.name.clone(); - let signer_index = Value::UInt(signer.signer_id as u128); - let point = Point::from(Scalar::random(&mut thread_rng())); - let point_arg = - Value::buff_from(point.compress().data.to_vec()).expect("Failed to create buff"); - let round = thread_rng().next_u64(); - let round_arg = Value::UInt(round as u128); - let reward_cycle_arg = Value::UInt(signer.reward_cycle as u128); - let valid_function_args = vec![ - signer_index.clone(), - point_arg.clone(), - round_arg.clone(), - reward_cycle_arg.clone(), - ]; - - let valid_tx_1_address_1 = StacksClient::build_signed_contract_call_transaction( - &contract_addr, - contract_name.clone(), - VOTE_FUNCTION_NAME.into(), - &valid_function_args, - &signer_private_key_1, - TransactionVersion::Testnet, - config.network.to_chain_id(), - 1, - 10, - ) - .unwrap(); - let valid_tx_2_address_1 = StacksClient::build_signed_contract_call_transaction( - &contract_addr, - contract_name.clone(), - VOTE_FUNCTION_NAME.into(), - &valid_function_args, - &signer_private_key_1, - TransactionVersion::Testnet, - config.network.to_chain_id(), - 2, - 10, - ) - .unwrap(); - let valid_tx_3_address_1 = StacksClient::build_signed_contract_call_transaction( - &contract_addr, - contract_name.clone(), - VOTE_FUNCTION_NAME.into(), - &valid_function_args, - &signer_private_key_1, - TransactionVersion::Testnet, - config.network.to_chain_id(), - 3, - 10, - ) - .unwrap(); - - let valid_tx_1_address_2 = StacksClient::build_signed_contract_call_transaction( - &contract_addr, - contract_name.clone(), - VOTE_FUNCTION_NAME.into(), - &valid_function_args, - &signer_private_key_2, - TransactionVersion::Testnet, - config.network.to_chain_id(), - 1, - 10, - ) - .unwrap(); - - let valid_tx_2_address_2 = StacksClient::build_signed_contract_call_transaction( - &contract_addr, - contract_name.clone(), - VOTE_FUNCTION_NAME.into(), - &valid_function_args, - &signer_private_key_2, - TransactionVersion::Testnet, - config.network.to_chain_id(), - 1, - 10, - ) - .unwrap(); - - let txs = signer.filter_one_transaction_per_address(vec![ - valid_tx_1_address_1.clone(), - valid_tx_3_address_1, - valid_tx_2_address_2, - valid_tx_2_address_1.clone(), - ]); - assert_eq!(txs.len(), 2); - assert!(txs.contains(&valid_tx_1_address_1)); - assert!(txs.contains(&valid_tx_1_address_2)); - } - - #[test] - fn filter_one_transaction_per_signer_duplicate_nonces() { - let config = GlobalConfig::load_from_file("./src/tests/conf/signer-0.toml").unwrap(); - let signer_config = generate_signer_config(&config, 5, 20); - let signer = Signer::from(signer_config.clone()); - - let signer_private_key = config.stacks_private_key; - let vote_contract_id = boot_code_id(SIGNERS_VOTING_NAME, signer.mainnet); - let contract_addr = vote_contract_id.issuer.into(); - let contract_name = vote_contract_id.name.clone(); - let signer_index = Value::UInt(signer.signer_id as u128); - let point = Point::from(Scalar::random(&mut thread_rng())); - let point_arg = - Value::buff_from(point.compress().data.to_vec()).expect("Failed to create buff"); - let round = thread_rng().next_u64(); - let round_arg = Value::UInt(round as u128); - let reward_cycle_arg = Value::UInt(signer.reward_cycle as u128); - let valid_function_args = vec![ - signer_index.clone(), - point_arg.clone(), - round_arg.clone(), - reward_cycle_arg.clone(), - ]; - let nonce = 0; - let valid_tx_1 = StacksClient::build_signed_contract_call_transaction( - &contract_addr, - contract_name.clone(), - VOTE_FUNCTION_NAME.into(), - &valid_function_args, - &signer_private_key, - TransactionVersion::Testnet, - config.network.to_chain_id(), - nonce, - 10, - ) - .unwrap(); - let valid_tx_2 = StacksClient::build_signed_contract_call_transaction( - &contract_addr, - contract_name.clone(), - VOTE_FUNCTION_NAME.into(), - &valid_function_args, - &signer_private_key, - TransactionVersion::Testnet, - config.network.to_chain_id(), - nonce, - 10, - ) - .unwrap(); - let valid_tx_3 = StacksClient::build_signed_contract_call_transaction( - &contract_addr, - contract_name.clone(), - VOTE_FUNCTION_NAME.into(), - &valid_function_args, - &signer_private_key, - TransactionVersion::Testnet, - config.network.to_chain_id(), - nonce, - 10, - ) - .unwrap(); - - let mut txs = vec![valid_tx_2, valid_tx_1, valid_tx_3]; - let filtered_txs = signer.filter_one_transaction_per_address(txs.clone()); - txs.sort_by(|a, b| a.txid().cmp(&b.txid())); - assert_eq!(filtered_txs.len(), 1); - assert!(filtered_txs.contains(&txs.first().expect("failed to get first tx"))); - } } diff --git a/stackslib/src/chainstate/nakamoto/signer_set.rs b/stackslib/src/chainstate/nakamoto/signer_set.rs index c0d6b2371..a39d6bebc 100644 --- a/stackslib/src/chainstate/nakamoto/signer_set.rs +++ b/stackslib/src/chainstate/nakamoto/signer_set.rs @@ -46,7 +46,7 @@ use stacks_common::util::hash::{to_hex, Hash160, MerkleHashFunc, MerkleTree, Sha use stacks_common::util::retry::BoundReader; use stacks_common::util::secp256k1::MessageSignature; use stacks_common::util::vrf::{VRFProof, VRFPublicKey, VRF}; -use wsts::curve::point::Point; +use wsts::curve::point::{Compressed, Point}; use crate::burnchains::{Burnchain, PoxConstants, Txid}; use crate::chainstate::burn::db::sortdb::{ @@ -63,7 +63,7 @@ use crate::chainstate::stacks::address::PoxAddress; use crate::chainstate::stacks::boot::{ PoxVersions, RawRewardSetEntry, RewardSet, BOOT_TEST_POX_4_AGG_KEY_CONTRACT, BOOT_TEST_POX_4_AGG_KEY_FNAME, POX_4_NAME, SIGNERS_MAX_LIST_SIZE, SIGNERS_NAME, SIGNERS_PK_LEN, - SIGNERS_UPDATE_STATE, + SIGNERS_UPDATE_STATE, SIGNERS_VOTING_FUNCTION_NAME, SIGNERS_VOTING_NAME, }; use crate::chainstate::stacks::db::{ ChainstateTx, ClarityTx, DBConfig as ChainstateConfig, MinerPaymentSchedule, @@ -487,4 +487,84 @@ impl NakamotoSigners { } Ok(signers) } + + /// Verify that the transaction is a valid vote for the aggregate public key + /// Note: it does not verify the function arguments, only that the transaction is validly formed + /// and has a valid nonce from an expected address + pub fn valid_vote_transaction( + account_nonces: &HashMap, + transaction: &StacksTransaction, + is_mainnet: bool, + ) -> bool { + let origin_address = transaction.origin_address(); + let origin_nonce = transaction.get_origin_nonce(); + let Some(account_nonce) = account_nonces.get(&origin_address) else { + debug!("valid_vote_transaction: Unrecognized origin address ({origin_address}).",); + return false; + }; + if transaction.is_mainnet() != is_mainnet { + debug!("valid_vote_transaction: Received a transaction for an unexpected network.",); + return false; + } + if origin_nonce < *account_nonce { + debug!("valid_vote_transaction: Received a transaction with an outdated nonce ({account_nonce} < {origin_nonce})."); + return false; + } + Self::parse_vote_for_aggregate_public_key(transaction).is_some() + } + + pub fn parse_vote_for_aggregate_public_key( + transaction: &StacksTransaction, + ) -> Option<(u64, Point, u64, u64)> { + let TransactionPayload::ContractCall(payload) = &transaction.payload else { + // Not a contract call so not a special cased vote for aggregate public key transaction + return None; + }; + if payload.contract_identifier() + != boot_code_id(SIGNERS_VOTING_NAME, transaction.is_mainnet()) + || payload.function_name != SIGNERS_VOTING_FUNCTION_NAME.into() + { + // This is not a special cased transaction. + return None; + } + if payload.function_args.len() != 4 { + return None; + } + let signer_index_value = payload.function_args.first()?; + let signer_index = u64::try_from(signer_index_value.clone().expect_u128().ok()?).ok()?; + let point_value = payload.function_args.get(1)?; + let point_bytes = point_value.clone().expect_buff(33).ok()?; + let compressed_data = Compressed::try_from(point_bytes.as_slice()).ok()?; + let point = Point::try_from(&compressed_data).ok()?; + let round_value = payload.function_args.get(2)?; + let round = u64::try_from(round_value.clone().expect_u128().ok()?).ok()?; + let reward_cycle = + u64::try_from(payload.function_args.get(3)?.clone().expect_u128().ok()?).ok()?; + Some((signer_index, point, round, reward_cycle)) + } + + /// Update the map of filtered valid transactions, selecting one per address based first on lowest nonce, then txid + pub fn update_filtered_transactions( + filtered_transactions: &mut HashMap, + account_nonces: &HashMap, + mainnet: bool, + transactions: Vec, + ) { + for transaction in transactions { + if NakamotoSigners::valid_vote_transaction(&account_nonces, &transaction, mainnet) { + let origin_address = transaction.origin_address(); + let origin_nonce = transaction.get_origin_nonce(); + if let Some(entry) = filtered_transactions.get_mut(&origin_address) { + let entry_nonce = entry.get_origin_nonce(); + if entry_nonce > origin_nonce + || (entry_nonce == origin_nonce && entry.txid() > transaction.txid()) + { + *entry = transaction; + } + } else { + filtered_transactions.insert(origin_address, transaction); + } + } + } + } } diff --git a/stackslib/src/chainstate/nakamoto/tests/mod.rs b/stackslib/src/chainstate/nakamoto/tests/mod.rs index 284a0af64..c27505b51 100644 --- a/stackslib/src/chainstate/nakamoto/tests/mod.rs +++ b/stackslib/src/chainstate/nakamoto/tests/mod.rs @@ -15,17 +15,22 @@ // along with this program. If not, see . use std::borrow::BorrowMut; +use std::collections::HashMap; use std::fs; use clarity::types::chainstate::{PoxId, SortitionId, StacksBlockId}; use clarity::vm::clarity::ClarityConnection; use clarity::vm::costs::ExecutionCost; use clarity::vm::types::StacksAddressExtensions; +use clarity::vm::Value; +use rand::{thread_rng, RngCore}; use rusqlite::Connection; use stacks_common::address::AddressHashMode; use stacks_common::bitvec::BitVec; use stacks_common::codec::StacksMessageCodec; -use stacks_common::consts::{FIRST_BURNCHAIN_CONSENSUS_HASH, FIRST_STACKS_BLOCK_HASH}; +use stacks_common::consts::{ + CHAIN_ID_MAINNET, CHAIN_ID_TESTNET, FIRST_BURNCHAIN_CONSENSUS_HASH, FIRST_STACKS_BLOCK_HASH, +}; use stacks_common::types::chainstate::{ BlockHeaderHash, BurnchainHeaderHash, ConsensusHash, StacksAddress, StacksPrivateKey, StacksPublicKey, StacksWorkScore, TrieHash, VRFSeed, @@ -37,6 +42,8 @@ use stacks_common::util::secp256k1::{MessageSignature, Secp256k1PublicKey}; use stacks_common::util::vrf::{VRFPrivateKey, VRFProof, VRFPublicKey, VRF}; use stdext::prelude::Integer; use stx_genesis::GenesisData; +use wsts::curve::point::Point; +use wsts::curve::scalar::Scalar; use crate::burnchains::{BurnchainSigner, PoxConstants, Txid}; use crate::chainstate::burn::db::sortdb::tests::make_fork_run; @@ -52,13 +59,16 @@ use crate::chainstate::coordinator::tests::{ }; use crate::chainstate::nakamoto::coordinator::tests::boot_nakamoto; use crate::chainstate::nakamoto::miner::NakamotoBlockBuilder; +use crate::chainstate::nakamoto::signer_set::NakamotoSigners; use crate::chainstate::nakamoto::tenure::NakamotoTenure; use crate::chainstate::nakamoto::test_signers::TestSigners; use crate::chainstate::nakamoto::tests::node::TestStacker; use crate::chainstate::nakamoto::{ NakamotoBlock, NakamotoBlockHeader, NakamotoChainState, SortitionHandle, FIRST_STACKS_BLOCK_ID, }; -use crate::chainstate::stacks::boot::MINERS_NAME; +use crate::chainstate::stacks::boot::{ + MINERS_NAME, SIGNERS_VOTING_FUNCTION_NAME, SIGNERS_VOTING_NAME, +}; use crate::chainstate::stacks::db::{ ChainStateBootData, ChainstateAccountBalance, ChainstateAccountLockup, ChainstateBNSName, ChainstateBNSNamespace, StacksAccount, StacksBlockHeaderTypes, StacksChainState, @@ -67,13 +77,15 @@ use crate::chainstate::stacks::db::{ use crate::chainstate::stacks::{ CoinbasePayload, StacksBlock, StacksBlockHeader, StacksTransaction, StacksTransactionSigner, TenureChangeCause, TenureChangePayload, ThresholdSignature, TokenTransferMemo, - TransactionAnchorMode, TransactionAuth, TransactionPayload, TransactionVersion, + TransactionAnchorMode, TransactionAuth, TransactionContractCall, TransactionPayload, + TransactionPostConditionMode, TransactionSmartContract, TransactionVersion, }; use crate::core; use crate::core::{StacksEpochExtension, STACKS_EPOCH_3_0_MARKER}; use crate::net::codec::test::check_codec_and_corruption; use crate::util_lib::boot::boot_code_id; use crate::util_lib::db::Error as db_error; +use crate::util_lib::strings::StacksString; /// Get an address's account pub fn get_account( @@ -2068,3 +2080,744 @@ fn test_make_miners_stackerdb_config() { assert_eq!(miner_hashbytes[8].1, miner_hash160s[8]); assert_eq!(miner_hashbytes[9].1, miner_hash160s[8]); } + +#[test] +fn parse_vote_for_aggregate_public_key_valid() { + let signer_private_key = StacksPrivateKey::new(); + let mainnet = false; + let chainid = CHAIN_ID_TESTNET; + let vote_contract_id = boot_code_id(SIGNERS_VOTING_NAME, mainnet); + let contract_addr = vote_contract_id.issuer.into(); + let contract_name = vote_contract_id.name.clone(); + + let signer_index = thread_rng().next_u64(); + let signer_index_arg = Value::UInt(signer_index as u128); + + let point = Point::from(Scalar::random(&mut thread_rng())); + let point_arg = + Value::buff_from(point.compress().data.to_vec()).expect("Failed to create buff"); + let round = thread_rng().next_u64(); + let round_arg = Value::UInt(round as u128); + + let reward_cycle = thread_rng().next_u64(); + let reward_cycle_arg = Value::UInt(reward_cycle as u128); + + let valid_function_args = vec![ + signer_index_arg.clone(), + point_arg.clone(), + round_arg.clone(), + reward_cycle_arg.clone(), + ]; + let valid_tx = StacksTransaction { + version: TransactionVersion::Testnet, + chain_id: CHAIN_ID_TESTNET, + auth: TransactionAuth::from_p2pkh(&signer_private_key).unwrap(), + anchor_mode: TransactionAnchorMode::Any, + post_condition_mode: TransactionPostConditionMode::Allow, + post_conditions: vec![], + payload: TransactionPayload::ContractCall(TransactionContractCall { + address: contract_addr, + contract_name, + function_name: SIGNERS_VOTING_FUNCTION_NAME.into(), + function_args: valid_function_args, + }), + }; + let (res_signer_index, res_point, res_round, res_reward_cycle) = + NakamotoSigners::parse_vote_for_aggregate_public_key(&valid_tx).unwrap(); + assert_eq!(res_signer_index, signer_index); + assert_eq!(res_point, point); + assert_eq!(res_round, round); + assert_eq!(res_reward_cycle, reward_cycle); +} + +#[test] +fn parse_vote_for_aggregate_public_key_invalid() { + let signer_private_key = StacksPrivateKey::new(); + let mainnet = false; + let chainid = CHAIN_ID_TESTNET; + let vote_contract_id = boot_code_id(SIGNERS_VOTING_NAME, mainnet); + let contract_addr: StacksAddress = vote_contract_id.issuer.into(); + let contract_name = vote_contract_id.name.clone(); + + let signer_index = thread_rng().next_u32(); + let signer_index_arg = Value::UInt(signer_index as u128); + + let point = Point::from(Scalar::random(&mut thread_rng())); + let point_arg = + Value::buff_from(point.compress().data.to_vec()).expect("Failed to create buff"); + let round = thread_rng().next_u64(); + let round_arg = Value::UInt(round as u128); + + let reward_cycle = thread_rng().next_u64(); + let reward_cycle_arg = Value::UInt(reward_cycle as u128); + + let valid_function_args = vec![ + signer_index_arg.clone(), + point_arg.clone(), + round_arg.clone(), + reward_cycle_arg.clone(), + ]; + + let mut invalid_contract_address = StacksTransaction { + version: TransactionVersion::Testnet, + chain_id: CHAIN_ID_TESTNET, + auth: TransactionAuth::from_p2pkh(&signer_private_key).unwrap(), + anchor_mode: TransactionAnchorMode::Any, + post_condition_mode: TransactionPostConditionMode::Allow, + post_conditions: vec![], + payload: TransactionPayload::ContractCall(TransactionContractCall { + address: StacksAddress::p2pkh( + false, + &StacksPublicKey::from_private(&signer_private_key), + ), + contract_name: contract_name.clone(), + function_name: SIGNERS_VOTING_FUNCTION_NAME.into(), + function_args: valid_function_args.clone(), + }), + }; + invalid_contract_address.set_origin_nonce(1); + + let mut invalid_contract_name = StacksTransaction { + version: TransactionVersion::Testnet, + chain_id: CHAIN_ID_TESTNET, + auth: TransactionAuth::from_p2pkh(&signer_private_key).unwrap(), + anchor_mode: TransactionAnchorMode::Any, + post_condition_mode: TransactionPostConditionMode::Allow, + post_conditions: vec![], + payload: TransactionPayload::ContractCall(TransactionContractCall { + address: contract_addr.clone(), + contract_name: "bad-signers-contract-name".into(), + function_name: SIGNERS_VOTING_FUNCTION_NAME.into(), + function_args: valid_function_args.clone(), + }), + }; + invalid_contract_name.set_origin_nonce(1); + + let mut invalid_signers_vote_function = StacksTransaction { + version: TransactionVersion::Testnet, + chain_id: CHAIN_ID_TESTNET, + auth: TransactionAuth::from_p2pkh(&signer_private_key).unwrap(), + anchor_mode: TransactionAnchorMode::Any, + post_condition_mode: TransactionPostConditionMode::Allow, + post_conditions: vec![], + payload: TransactionPayload::ContractCall(TransactionContractCall { + address: contract_addr.clone(), + contract_name: contract_name.clone(), + function_name: "some-other-function".into(), + function_args: valid_function_args.clone(), + }), + }; + invalid_signers_vote_function.set_origin_nonce(1); + + let mut invalid_function_arg_signer_index = StacksTransaction { + version: TransactionVersion::Testnet, + chain_id: CHAIN_ID_TESTNET, + auth: TransactionAuth::from_p2pkh(&signer_private_key).unwrap(), + anchor_mode: TransactionAnchorMode::Any, + post_condition_mode: TransactionPostConditionMode::Allow, + post_conditions: vec![], + payload: TransactionPayload::ContractCall(TransactionContractCall { + address: contract_addr.clone(), + contract_name: contract_name.clone(), + function_name: SIGNERS_VOTING_FUNCTION_NAME.into(), + function_args: vec![ + point_arg.clone(), + point_arg.clone(), + round_arg.clone(), + reward_cycle_arg.clone(), + ], + }), + }; + invalid_function_arg_signer_index.set_origin_nonce(1); + + let mut invalid_function_arg_key = StacksTransaction { + version: TransactionVersion::Testnet, + chain_id: CHAIN_ID_TESTNET, + auth: TransactionAuth::from_p2pkh(&signer_private_key).unwrap(), + anchor_mode: TransactionAnchorMode::Any, + post_condition_mode: TransactionPostConditionMode::Allow, + post_conditions: vec![], + payload: TransactionPayload::ContractCall(TransactionContractCall { + address: contract_addr.clone(), + contract_name: contract_name.clone(), + function_name: SIGNERS_VOTING_FUNCTION_NAME.into(), + function_args: vec![ + signer_index_arg.clone(), + signer_index_arg.clone(), + round_arg.clone(), + reward_cycle_arg.clone(), + ], + }), + }; + invalid_function_arg_key.set_origin_nonce(1); + + let mut invalid_function_arg_round = StacksTransaction { + version: TransactionVersion::Testnet, + chain_id: CHAIN_ID_TESTNET, + auth: TransactionAuth::from_p2pkh(&signer_private_key).unwrap(), + anchor_mode: TransactionAnchorMode::Any, + post_condition_mode: TransactionPostConditionMode::Allow, + post_conditions: vec![], + payload: TransactionPayload::ContractCall(TransactionContractCall { + address: contract_addr.clone(), + contract_name: contract_name.clone(), + function_name: SIGNERS_VOTING_FUNCTION_NAME.into(), + function_args: vec![ + signer_index_arg.clone(), + point_arg.clone(), + point_arg.clone(), + reward_cycle_arg.clone(), + ], + }), + }; + invalid_function_arg_round.set_origin_nonce(1); + + let mut invalid_function_arg_reward_cycle = StacksTransaction { + version: TransactionVersion::Testnet, + chain_id: CHAIN_ID_TESTNET, + auth: TransactionAuth::from_p2pkh(&signer_private_key).unwrap(), + anchor_mode: TransactionAnchorMode::Any, + post_condition_mode: TransactionPostConditionMode::Allow, + post_conditions: vec![], + payload: TransactionPayload::ContractCall(TransactionContractCall { + address: contract_addr.clone(), + contract_name: contract_name.clone(), + function_name: SIGNERS_VOTING_FUNCTION_NAME.into(), + function_args: vec![ + signer_index_arg.clone(), + point_arg.clone(), + round_arg.clone(), + point_arg.clone(), + ], + }), + }; + invalid_function_arg_reward_cycle.set_origin_nonce(1); + + let mut account_nonces = std::collections::HashMap::new(); + account_nonces.insert(invalid_contract_name.origin_address(), 1); + for (i, tx) in vec![ + invalid_contract_address, + invalid_contract_name, + invalid_signers_vote_function, + invalid_function_arg_signer_index, + invalid_function_arg_key, + invalid_function_arg_round, + invalid_function_arg_reward_cycle, + ] + .iter() + .enumerate() + { + assert!( + NakamotoSigners::parse_vote_for_aggregate_public_key(&tx).is_none(), + "{}", + format!("parsed the {i}th transaction: {tx:?}") + ); + } +} + +#[test] +fn valid_vote_transaction() { + let signer_private_key = StacksPrivateKey::new(); + let mainnet = false; + let chainid = CHAIN_ID_TESTNET; + let vote_contract_id = boot_code_id(SIGNERS_VOTING_NAME, mainnet); + let contract_addr = vote_contract_id.issuer.into(); + let contract_name = vote_contract_id.name.clone(); + + let signer_index = thread_rng().next_u32(); + let signer_index_arg = Value::UInt(signer_index as u128); + + let point = Point::from(Scalar::random(&mut thread_rng())); + let point_arg = + Value::buff_from(point.compress().data.to_vec()).expect("Failed to create buff"); + let round = thread_rng().next_u64(); + let round_arg = Value::UInt(round as u128); + + let reward_cycle = thread_rng().next_u64(); + let reward_cycle_arg = Value::UInt(reward_cycle as u128); + + let valid_function_args = vec![ + signer_index_arg.clone(), + point_arg.clone(), + round_arg.clone(), + reward_cycle_arg.clone(), + ]; + let mut valid_tx = StacksTransaction { + version: TransactionVersion::Testnet, + chain_id: CHAIN_ID_TESTNET, + auth: TransactionAuth::from_p2pkh(&signer_private_key).unwrap(), + anchor_mode: TransactionAnchorMode::Any, + post_condition_mode: TransactionPostConditionMode::Allow, + post_conditions: vec![], + payload: TransactionPayload::ContractCall(TransactionContractCall { + address: contract_addr, + contract_name: contract_name, + function_name: SIGNERS_VOTING_FUNCTION_NAME.into(), + function_args: valid_function_args, + }), + }; + valid_tx.set_origin_nonce(1); + let mut account_nonces = std::collections::HashMap::new(); + account_nonces.insert(valid_tx.origin_address(), 1); + assert!(NakamotoSigners::valid_vote_transaction( + &account_nonces, + &valid_tx, + mainnet + )); +} + +#[test] +fn valid_vote_transaction_malformed_transactions() { + let signer_private_key = StacksPrivateKey::new(); + let mainnet = false; + let chainid = CHAIN_ID_TESTNET; + let vote_contract_id = boot_code_id(SIGNERS_VOTING_NAME, mainnet); + let contract_addr: StacksAddress = vote_contract_id.issuer.into(); + let contract_name = vote_contract_id.name.clone(); + + let signer_index = thread_rng().next_u32(); + let signer_index_arg = Value::UInt(signer_index as u128); + + let point = Point::from(Scalar::random(&mut thread_rng())); + let point_arg = + Value::buff_from(point.compress().data.to_vec()).expect("Failed to create buff"); + let round = thread_rng().next_u64(); + let round_arg = Value::UInt(round as u128); + + let reward_cycle = thread_rng().next_u64(); + let reward_cycle_arg = Value::UInt(reward_cycle as u128); + + let valid_function_args = vec![ + signer_index_arg.clone(), + point_arg.clone(), + round_arg.clone(), + reward_cycle_arg.clone(), + ]; + // Create a invalid transaction that is not a contract call + let mut invalid_not_contract_call = StacksTransaction { + version: TransactionVersion::Testnet, + chain_id: CHAIN_ID_TESTNET, + auth: TransactionAuth::from_p2pkh(&signer_private_key).unwrap(), + anchor_mode: TransactionAnchorMode::Any, + post_condition_mode: TransactionPostConditionMode::Allow, + post_conditions: vec![], + payload: TransactionPayload::SmartContract( + TransactionSmartContract { + name: "test-contract".into(), + code_body: StacksString::from_str("(/ 1 0)").unwrap(), + }, + None, + ), + }; + invalid_not_contract_call.set_origin_nonce(1); + + let mut invalid_contract_address = StacksTransaction { + version: TransactionVersion::Testnet, + chain_id: CHAIN_ID_TESTNET, + auth: TransactionAuth::from_p2pkh(&signer_private_key).unwrap(), + anchor_mode: TransactionAnchorMode::Any, + post_condition_mode: TransactionPostConditionMode::Allow, + post_conditions: vec![], + payload: TransactionPayload::ContractCall(TransactionContractCall { + address: StacksAddress::p2pkh( + mainnet, + &StacksPublicKey::from_private(&signer_private_key), + ), + contract_name: contract_name.clone(), + function_name: SIGNERS_VOTING_FUNCTION_NAME.into(), + function_args: valid_function_args.clone(), + }), + }; + invalid_contract_address.set_origin_nonce(1); + + let mut invalid_contract_name = StacksTransaction { + version: TransactionVersion::Testnet, + chain_id: CHAIN_ID_TESTNET, + auth: TransactionAuth::from_p2pkh(&signer_private_key).unwrap(), + anchor_mode: TransactionAnchorMode::Any, + post_condition_mode: TransactionPostConditionMode::Allow, + post_conditions: vec![], + payload: TransactionPayload::ContractCall(TransactionContractCall { + address: contract_addr.clone(), + contract_name: "bad-signers-contract-name".into(), + function_name: SIGNERS_VOTING_FUNCTION_NAME.into(), + function_args: valid_function_args.clone(), + }), + }; + invalid_contract_name.set_origin_nonce(1); + + let mut invalid_network = StacksTransaction { + version: TransactionVersion::Mainnet, + chain_id: CHAIN_ID_MAINNET, + auth: TransactionAuth::from_p2pkh(&signer_private_key).unwrap(), + anchor_mode: TransactionAnchorMode::Any, + post_condition_mode: TransactionPostConditionMode::Allow, + post_conditions: vec![], + payload: TransactionPayload::ContractCall(TransactionContractCall { + address: contract_addr.clone(), + contract_name: contract_name.clone(), + function_name: SIGNERS_VOTING_FUNCTION_NAME.into(), + function_args: valid_function_args.clone(), + }), + }; + invalid_network.set_origin_nonce(1); + + let mut invalid_signers_vote_function = StacksTransaction { + version: TransactionVersion::Testnet, + chain_id: CHAIN_ID_TESTNET, + auth: TransactionAuth::from_p2pkh(&signer_private_key).unwrap(), + anchor_mode: TransactionAnchorMode::Any, + post_condition_mode: TransactionPostConditionMode::Allow, + post_conditions: vec![], + payload: TransactionPayload::ContractCall(TransactionContractCall { + address: contract_addr.clone(), + contract_name: contract_name.clone(), + function_name: "some-other-function".into(), + function_args: valid_function_args.clone(), + }), + }; + invalid_signers_vote_function.set_origin_nonce(1); + + let mut invalid_function_arg_signer_index = StacksTransaction { + version: TransactionVersion::Testnet, + chain_id: CHAIN_ID_TESTNET, + auth: TransactionAuth::from_p2pkh(&signer_private_key).unwrap(), + anchor_mode: TransactionAnchorMode::Any, + post_condition_mode: TransactionPostConditionMode::Allow, + post_conditions: vec![], + payload: TransactionPayload::ContractCall(TransactionContractCall { + address: contract_addr.clone(), + contract_name: contract_name.clone(), + function_name: SIGNERS_VOTING_FUNCTION_NAME.into(), + function_args: vec![ + point_arg.clone(), + point_arg.clone(), + round_arg.clone(), + reward_cycle_arg.clone(), + ], + }), + }; + invalid_function_arg_signer_index.set_origin_nonce(1); + + let mut invalid_function_arg_key = StacksTransaction { + version: TransactionVersion::Testnet, + chain_id: CHAIN_ID_TESTNET, + auth: TransactionAuth::from_p2pkh(&signer_private_key).unwrap(), + anchor_mode: TransactionAnchorMode::Any, + post_condition_mode: TransactionPostConditionMode::Allow, + post_conditions: vec![], + payload: TransactionPayload::ContractCall(TransactionContractCall { + address: contract_addr.clone(), + contract_name: contract_name.clone(), + function_name: SIGNERS_VOTING_FUNCTION_NAME.into(), + function_args: vec![ + signer_index_arg.clone(), + signer_index_arg.clone(), + round_arg.clone(), + reward_cycle_arg.clone(), + ], + }), + }; + invalid_function_arg_key.set_origin_nonce(1); + + let mut invalid_function_arg_round = StacksTransaction { + version: TransactionVersion::Testnet, + chain_id: CHAIN_ID_TESTNET, + auth: TransactionAuth::from_p2pkh(&signer_private_key).unwrap(), + anchor_mode: TransactionAnchorMode::Any, + post_condition_mode: TransactionPostConditionMode::Allow, + post_conditions: vec![], + payload: TransactionPayload::ContractCall(TransactionContractCall { + address: contract_addr.clone(), + contract_name: contract_name.clone(), + function_name: SIGNERS_VOTING_FUNCTION_NAME.into(), + function_args: vec![ + signer_index_arg.clone(), + point_arg.clone(), + point_arg.clone(), + reward_cycle_arg.clone(), + ], + }), + }; + invalid_function_arg_round.set_origin_nonce(1); + + let mut invalid_function_arg_reward_cycle = StacksTransaction { + version: TransactionVersion::Testnet, + chain_id: CHAIN_ID_TESTNET, + auth: TransactionAuth::from_p2pkh(&signer_private_key).unwrap(), + anchor_mode: TransactionAnchorMode::Any, + post_condition_mode: TransactionPostConditionMode::Allow, + post_conditions: vec![], + payload: TransactionPayload::ContractCall(TransactionContractCall { + address: contract_addr.clone(), + contract_name: contract_name.clone(), + function_name: SIGNERS_VOTING_FUNCTION_NAME.into(), + function_args: vec![ + signer_index_arg.clone(), + point_arg.clone(), + round_arg.clone(), + point_arg.clone(), + ], + }), + }; + invalid_function_arg_reward_cycle.set_origin_nonce(1); + + let mut invalid_nonce = StacksTransaction { + version: TransactionVersion::Testnet, + chain_id: CHAIN_ID_TESTNET, + auth: TransactionAuth::from_p2pkh(&signer_private_key).unwrap(), + anchor_mode: TransactionAnchorMode::Any, + post_condition_mode: TransactionPostConditionMode::Allow, + post_conditions: vec![], + payload: TransactionPayload::ContractCall(TransactionContractCall { + address: contract_addr.clone(), + contract_name: contract_name.clone(), + function_name: SIGNERS_VOTING_FUNCTION_NAME.into(), + function_args: valid_function_args.clone(), + }), + }; + invalid_nonce.set_origin_nonce(0); // old nonce + + let mut account_nonces = std::collections::HashMap::new(); + account_nonces.insert(invalid_not_contract_call.origin_address(), 1); + for tx in vec![ + invalid_not_contract_call, + invalid_contract_address, + invalid_contract_name, + invalid_signers_vote_function, + invalid_function_arg_signer_index, + invalid_function_arg_key, + invalid_function_arg_round, + invalid_function_arg_reward_cycle, + invalid_nonce, + invalid_network, + ] { + assert!(!NakamotoSigners::valid_vote_transaction( + &account_nonces, + &tx, + mainnet + )); + } +} + +#[test] +fn filter_one_transaction_per_signer_multiple_addresses() { + let signer_private_key_1 = StacksPrivateKey::new(); + let signer_private_key_2 = StacksPrivateKey::new(); + let mainnet = false; + let chainid = CHAIN_ID_TESTNET; + let vote_contract_id = boot_code_id(SIGNERS_VOTING_NAME, mainnet); + let contract_addr: StacksAddress = vote_contract_id.issuer.into(); + let contract_name = vote_contract_id.name.clone(); + + let signer_index = thread_rng().next_u32(); + let signer_index_arg = Value::UInt(signer_index as u128); + + let point = Point::from(Scalar::random(&mut thread_rng())); + let point_arg = + Value::buff_from(point.compress().data.to_vec()).expect("Failed to create buff"); + let round = thread_rng().next_u64(); + let round_arg = Value::UInt(round as u128); + + let reward_cycle = thread_rng().next_u64(); + let reward_cycle_arg = Value::UInt(reward_cycle as u128); + + let function_args = vec![ + signer_index_arg.clone(), + point_arg.clone(), + round_arg.clone(), + reward_cycle_arg.clone(), + ]; + + let mut valid_tx_1_address_1 = StacksTransaction { + version: TransactionVersion::Testnet, + chain_id: CHAIN_ID_TESTNET, + auth: TransactionAuth::from_p2pkh(&signer_private_key_1).unwrap(), + anchor_mode: TransactionAnchorMode::Any, + post_condition_mode: TransactionPostConditionMode::Allow, + post_conditions: vec![], + payload: TransactionPayload::ContractCall(TransactionContractCall { + address: contract_addr.clone(), + contract_name: contract_name.clone(), + function_name: SIGNERS_VOTING_FUNCTION_NAME.into(), + function_args: function_args.clone(), + }), + }; + valid_tx_1_address_1.set_origin_nonce(1); + + let mut valid_tx_2_address_1 = StacksTransaction { + version: TransactionVersion::Testnet, + chain_id: CHAIN_ID_TESTNET, + auth: TransactionAuth::from_p2pkh(&signer_private_key_1).unwrap(), + anchor_mode: TransactionAnchorMode::Any, + post_condition_mode: TransactionPostConditionMode::Allow, + post_conditions: vec![], + payload: TransactionPayload::ContractCall(TransactionContractCall { + address: contract_addr.clone(), + contract_name: contract_name.clone(), + function_name: SIGNERS_VOTING_FUNCTION_NAME.into(), + function_args: function_args.clone(), + }), + }; + valid_tx_2_address_1.set_origin_nonce(2); + + let mut valid_tx_3_address_1 = StacksTransaction { + version: TransactionVersion::Testnet, + chain_id: CHAIN_ID_TESTNET, + auth: TransactionAuth::from_p2pkh(&signer_private_key_1).unwrap(), + anchor_mode: TransactionAnchorMode::Any, + post_condition_mode: TransactionPostConditionMode::Allow, + post_conditions: vec![], + payload: TransactionPayload::ContractCall(TransactionContractCall { + address: contract_addr.clone(), + contract_name: contract_name.clone(), + function_name: SIGNERS_VOTING_FUNCTION_NAME.into(), + function_args: function_args.clone(), + }), + }; + valid_tx_3_address_1.set_origin_nonce(3); + + let mut valid_tx_1_address_2 = StacksTransaction { + version: TransactionVersion::Testnet, + chain_id: CHAIN_ID_TESTNET, + auth: TransactionAuth::from_p2pkh(&signer_private_key_2).unwrap(), + anchor_mode: TransactionAnchorMode::Any, + post_condition_mode: TransactionPostConditionMode::Allow, + post_conditions: vec![], + payload: TransactionPayload::ContractCall(TransactionContractCall { + address: contract_addr.clone(), + contract_name: contract_name.clone(), + function_name: SIGNERS_VOTING_FUNCTION_NAME.into(), + function_args: function_args.clone(), + }), + }; + valid_tx_1_address_2.set_origin_nonce(1); + + let mut valid_tx_2_address_2 = StacksTransaction { + version: TransactionVersion::Testnet, + chain_id: CHAIN_ID_TESTNET, + auth: TransactionAuth::from_p2pkh(&signer_private_key_2).unwrap(), + anchor_mode: TransactionAnchorMode::Any, + post_condition_mode: TransactionPostConditionMode::Allow, + post_conditions: vec![], + payload: TransactionPayload::ContractCall(TransactionContractCall { + address: contract_addr, + contract_name, + function_name: SIGNERS_VOTING_FUNCTION_NAME.into(), + function_args, + }), + }; + valid_tx_2_address_2.set_origin_nonce(2); + let mut filtered_transactions = HashMap::new(); + let mut account_nonces = std::collections::HashMap::new(); + account_nonces.insert(valid_tx_1_address_1.origin_address(), 1); + account_nonces.insert(valid_tx_1_address_2.origin_address(), 1); + NakamotoSigners::update_filtered_transactions( + &mut filtered_transactions, + &account_nonces, + false, + vec![ + valid_tx_1_address_1.clone(), + valid_tx_3_address_1, + valid_tx_1_address_2.clone(), + valid_tx_2_address_2, + valid_tx_2_address_1, + ], + ); + let txs: Vec<_> = filtered_transactions.into_values().collect(); + assert_eq!(txs.len(), 2); + assert!(txs.contains(&valid_tx_1_address_1)); + assert!(txs.contains(&valid_tx_1_address_2)); +} + +#[test] +fn filter_one_transaction_per_signer_duplicate_nonces() { + let signer_private_key = StacksPrivateKey::new(); + let mainnet = false; + let chainid = CHAIN_ID_TESTNET; + let vote_contract_id = boot_code_id(SIGNERS_VOTING_NAME, mainnet); + let contract_addr: StacksAddress = vote_contract_id.issuer.into(); + let contract_name = vote_contract_id.name.clone(); + + let signer_index = thread_rng().next_u32(); + let signer_index_arg = Value::UInt(signer_index as u128); + + let point = Point::from(Scalar::random(&mut thread_rng())); + let point_arg = + Value::buff_from(point.compress().data.to_vec()).expect("Failed to create buff"); + let round = thread_rng().next_u64(); + let round_arg = Value::UInt(round as u128); + + let reward_cycle = thread_rng().next_u64(); + let reward_cycle_arg = Value::UInt(reward_cycle as u128); + + let function_args = vec![ + signer_index_arg.clone(), + point_arg.clone(), + round_arg.clone(), + reward_cycle_arg.clone(), + ]; + + let mut valid_tx_1 = StacksTransaction { + version: TransactionVersion::Testnet, + chain_id: CHAIN_ID_TESTNET, + auth: TransactionAuth::from_p2pkh(&signer_private_key).unwrap(), + anchor_mode: TransactionAnchorMode::Any, + post_condition_mode: TransactionPostConditionMode::Allow, + post_conditions: vec![], + payload: TransactionPayload::ContractCall(TransactionContractCall { + address: contract_addr.clone(), + contract_name: contract_name.clone(), + function_name: SIGNERS_VOTING_FUNCTION_NAME.into(), + function_args: function_args.clone(), + }), + }; + valid_tx_1.set_origin_nonce(0); + + let mut valid_tx_2 = StacksTransaction { + version: TransactionVersion::Testnet, + chain_id: CHAIN_ID_TESTNET, + auth: TransactionAuth::from_p2pkh(&signer_private_key).unwrap(), + anchor_mode: TransactionAnchorMode::Any, + post_condition_mode: TransactionPostConditionMode::Allow, + post_conditions: vec![], + payload: TransactionPayload::ContractCall(TransactionContractCall { + address: contract_addr.clone(), + contract_name: contract_name.clone(), + function_name: SIGNERS_VOTING_FUNCTION_NAME.into(), + function_args: function_args.clone(), + }), + }; + valid_tx_2.set_origin_nonce(0); + + let mut valid_tx_3 = StacksTransaction { + version: TransactionVersion::Testnet, + chain_id: CHAIN_ID_TESTNET, + auth: TransactionAuth::from_p2pkh(&signer_private_key).unwrap(), + anchor_mode: TransactionAnchorMode::Any, + post_condition_mode: TransactionPostConditionMode::Allow, + post_conditions: vec![], + payload: TransactionPayload::ContractCall(TransactionContractCall { + address: contract_addr, + contract_name, + function_name: SIGNERS_VOTING_FUNCTION_NAME.into(), + function_args, + }), + }; + valid_tx_3.set_origin_nonce(0); + + let mut account_nonces = std::collections::HashMap::new(); + account_nonces.insert(valid_tx_1.origin_address(), 0); + let mut txs = vec![valid_tx_2, valid_tx_1, valid_tx_3]; + let mut filtered_transactions = HashMap::new(); + NakamotoSigners::update_filtered_transactions( + &mut filtered_transactions, + &account_nonces, + false, + txs.clone(), + ); + let filtered_txs: Vec<_> = filtered_transactions.into_values().collect(); + txs.sort_by(|a, b| a.txid().cmp(&b.txid())); + assert_eq!(filtered_txs.len(), 1); + assert!(filtered_txs.contains(&txs.first().expect("failed to get first tx"))); +} diff --git a/stackslib/src/chainstate/stacks/boot/mod.rs b/stackslib/src/chainstate/stacks/boot/mod.rs index abba9be6c..834d83ed2 100644 --- a/stackslib/src/chainstate/stacks/boot/mod.rs +++ b/stackslib/src/chainstate/stacks/boot/mod.rs @@ -78,6 +78,7 @@ pub const POX_3_NAME: &'static str = "pox-3"; pub const POX_4_NAME: &'static str = "pox-4"; pub const SIGNERS_NAME: &'static str = "signers"; pub const SIGNERS_VOTING_NAME: &'static str = "signers-voting"; +pub const SIGNERS_VOTING_FUNCTION_NAME: &str = "vote-for-aggregate-public-key"; /// This is the name of a variable in the `.signers` contract which tracks the most recently updated /// reward cycle number. pub const SIGNERS_UPDATE_STATE: &'static str = "last-set-cycle"; @@ -1933,7 +1934,7 @@ pub mod test { let payload = TransactionPayload::new_contract_call( boot_code_test_addr(), SIGNERS_VOTING_NAME, - "vote-for-aggregate-public-key", + SIGNERS_VOTING_FUNCTION_NAME, vec![ Value::UInt(signer_index), aggregate_public_key, diff --git a/stackslib/src/net/tests/mod.rs b/stackslib/src/net/tests/mod.rs index acff183d9..9111f10fc 100644 --- a/stackslib/src/net/tests/mod.rs +++ b/stackslib/src/net/tests/mod.rs @@ -46,7 +46,9 @@ use crate::chainstate::stacks::address::PoxAddress; use crate::chainstate::stacks::boot::test::{ key_to_stacks_addr, make_pox_4_lockup, make_signer_key_signature, with_sortdb, }; -use crate::chainstate::stacks::boot::MINERS_NAME; +use crate::chainstate::stacks::boot::{ + MINERS_NAME, SIGNERS_VOTING_FUNCTION_NAME, SIGNERS_VOTING_NAME, +}; use crate::chainstate::stacks::db::{MinerPaymentTxFees, StacksAccount, StacksChainState}; use crate::chainstate::stacks::events::TransactionOrigin; use crate::chainstate::stacks::{ @@ -185,9 +187,9 @@ impl NakamotoBootPlan { function_name, .. }) => { - if contract_name.as_str() == "signers-voting" + if contract_name.as_str() == SIGNERS_VOTING_NAME && address.is_burn() - && function_name.as_str() == "vote-for-aggregate-public-key" + && function_name.as_str() == SIGNERS_VOTING_FUNCTION_NAME { false } else { diff --git a/testnet/stacks-node/src/mockamoto.rs b/testnet/stacks-node/src/mockamoto.rs index 2629f4f9b..011300bc8 100644 --- a/testnet/stacks-node/src/mockamoto.rs +++ b/testnet/stacks-node/src/mockamoto.rs @@ -49,7 +49,7 @@ use stacks::chainstate::nakamoto::{ NakamotoBlock, NakamotoBlockHeader, NakamotoChainState, SetupBlockResult, }; use stacks::chainstate::stacks::address::PoxAddress; -use stacks::chainstate::stacks::boot::SIGNERS_VOTING_NAME; +use stacks::chainstate::stacks::boot::{SIGNERS_VOTING_FUNCTION_NAME, SIGNERS_VOTING_NAME}; use stacks::chainstate::stacks::db::{ChainStateBootData, ClarityTx, StacksChainState}; use stacks::chainstate::stacks::miner::{ BlockBuilder, BlockBuilderSettings, BlockLimitFunction, MinerStatus, TransactionResult, @@ -934,7 +934,7 @@ impl MockamotoNode { let vote_payload = TransactionPayload::new_contract_call( boot_code_addr(false), SIGNERS_VOTING_NAME, - "vote-for-aggregate-public-key", + SIGNERS_VOTING_FUNCTION_NAME, vec![ ClarityValue::UInt(0), aggregate_public_key_val, diff --git a/testnet/stacks-node/src/nakamoto_node/miner.rs b/testnet/stacks-node/src/nakamoto_node/miner.rs index ec57fc3ef..9c886b434 100644 --- a/testnet/stacks-node/src/nakamoto_node/miner.rs +++ b/testnet/stacks-node/src/nakamoto_node/miner.rs @@ -331,48 +331,43 @@ impl BlockMinerThread { }) .collect(); - // There may be more than signer messages, but odds are there is only one transacton per signer - let mut transactions_to_include = Vec::with_capacity(signer_messages.len()); + if signer_messages.is_empty() { + return Ok(vec![]); + } + + // Get all nonces for the signers from clarity DB to use to validate transactions + let account_nonces = chainstate + .with_read_only_clarity_tx(&sortdb.index_conn(), &self.parent_tenure_id, |clarity_tx| { + clarity_tx.with_clarity_db_readonly(|clarity_db| { + addresses + .iter() + .map(|address| { + ( + address.clone(), + clarity_db + .get_account_nonce(&address.clone().into()) + .unwrap_or(0), + ) + }) + .collect::>() + }) + }) + .unwrap_or_default(); + let mut filtered_transactions: HashMap = HashMap::new(); for (_slot, signer_message) in signer_messages { match signer_message { SignerMessage::Transactions(transactions) => { - for transaction in transactions { - let address = transaction.origin_address(); - let nonce = transaction.get_origin_nonce(); - if !addresses.contains(&address) { - test_debug!("Miner: ignoring transaction ({:?}) with nonce {nonce} from address {address}", transaction.txid()); - continue; - } - - let cur_nonce = chainstate - .with_read_only_clarity_tx( - &sortdb.index_conn(), - &self.parent_tenure_id, - |clarity_tx| { - clarity_tx.with_clarity_db_readonly(|clarity_db| { - clarity_db.get_account_nonce(&address.into()).unwrap_or(0) - }) - }, - ) - .unwrap_or(0); - - if cur_nonce > nonce { - test_debug!("Miner: ignoring transaction ({:?}) with nonce {nonce} from address {address}", transaction.txid()); - continue; - } - debug!("Miner: including signer transaction."; - "nonce" => {nonce}, - "origin_address" => %address, - "txid" => %transaction.txid() - ); - // TODO : filter out transactions that are not valid votes. Do not include transactions with invalid/duplicate nonces for the same address. - transactions_to_include.push(transaction); - } + NakamotoSigners::update_filtered_transactions( + &mut filtered_transactions, + &account_nonces, + self.config.is_mainnet(), + transactions, + ) } _ => {} // Any other message is ignored } } - Ok(transactions_to_include) + Ok(filtered_transactions.into_values().collect()) } fn wait_for_signer_signature( diff --git a/testnet/stacks-node/src/tests/nakamoto_integrations.rs b/testnet/stacks-node/src/tests/nakamoto_integrations.rs index e97aefd42..3b46ce24a 100644 --- a/testnet/stacks-node/src/tests/nakamoto_integrations.rs +++ b/testnet/stacks-node/src/tests/nakamoto_integrations.rs @@ -30,7 +30,9 @@ use stacks::chainstate::nakamoto::miner::NakamotoBlockBuilder; use stacks::chainstate::nakamoto::test_signers::TestSigners; use stacks::chainstate::nakamoto::{NakamotoBlock, NakamotoChainState}; use stacks::chainstate::stacks::address::PoxAddress; -use stacks::chainstate::stacks::boot::{MINERS_NAME, SIGNERS_VOTING_NAME}; +use stacks::chainstate::stacks::boot::{ + MINERS_NAME, SIGNERS_VOTING_FUNCTION_NAME, SIGNERS_VOTING_NAME, +}; use stacks::chainstate::stacks::db::StacksChainState; use stacks::chainstate::stacks::miner::{BlockBuilder, BlockLimitFunction, TransactionResult}; use stacks::chainstate::stacks::{StacksTransaction, ThresholdSignature, TransactionPayload}; @@ -455,7 +457,7 @@ pub fn boot_to_epoch_3( 300, &StacksAddress::burn_address(false), SIGNERS_VOTING_NAME, - "vote-for-aggregate-public-key", + SIGNERS_VOTING_FUNCTION_NAME, &[ clarity::vm::Value::UInt(i as u128), aggregate_public_key.clone(), @@ -564,7 +566,7 @@ fn signer_vote_if_needed( 300, &StacksAddress::burn_address(false), SIGNERS_VOTING_NAME, - "vote-for-aggregate-public-key", + SIGNERS_VOTING_FUNCTION_NAME, &[ clarity::vm::Value::UInt(i as u128), aggregate_public_key.clone(), diff --git a/testnet/stacks-node/src/tests/signer.rs b/testnet/stacks-node/src/tests/signer.rs index d903f58c4..3fd265d79 100644 --- a/testnet/stacks-node/src/tests/signer.rs +++ b/testnet/stacks-node/src/tests/signer.rs @@ -7,23 +7,35 @@ use std::time::{Duration, Instant}; use std::{env, thread}; use clarity::boot_util::boot_code_id; +use clarity::vm::Value; use libsigner::{ BlockResponse, RejectCode, RunningSigner, Signer, SignerEventReceiver, SignerMessage, BLOCK_MSG_ID, }; +use rand::thread_rng; +use rand_core::RngCore; use stacks::burnchains::Txid; use stacks::chainstate::coordinator::comm::CoordinatorChannels; use stacks::chainstate::nakamoto::signer_set::NakamotoSigners; use stacks::chainstate::nakamoto::{NakamotoBlock, NakamotoBlockHeader, NakamotoBlockVote}; -use stacks::chainstate::stacks::boot::SIGNERS_NAME; +use stacks::chainstate::stacks::boot::{ + SIGNERS_NAME, SIGNERS_VOTING_FUNCTION_NAME, SIGNERS_VOTING_NAME, +}; use stacks::chainstate::stacks::miner::TransactionEvent; -use stacks::chainstate::stacks::{StacksPrivateKey, StacksTransaction, ThresholdSignature}; +use stacks::chainstate::stacks::{ + StacksPrivateKey, StacksTransaction, ThresholdSignature, TransactionAnchorMode, + TransactionAuth, TransactionPayload, TransactionPostConditionMode, TransactionSmartContract, + TransactionVersion, +}; use stacks::core::StacksEpoch; use stacks::net::api::postblock_proposal::BlockValidateResponse; +use stacks::util_lib::strings::StacksString; use stacks_common::bitvec::BitVec; use stacks_common::codec::{read_next, StacksMessageCodec}; -use stacks_common::consts::SIGNER_SLOTS_PER_USER; -use stacks_common::types::chainstate::{ConsensusHash, StacksBlockId, TrieHash}; +use stacks_common::consts::{CHAIN_ID_TESTNET, SIGNER_SLOTS_PER_USER}; +use stacks_common::types::chainstate::{ + ConsensusHash, StacksAddress, StacksBlockId, StacksPublicKey, TrieHash, +}; use stacks_common::types::StacksEpochId; use stacks_common::util::hash::{MerkleTree, Sha512Trunc256Sum}; use stacks_common::util::secp256k1::MessageSignature; @@ -545,38 +557,190 @@ impl SignerTest { .unwrap(); // Get the signer indices let reward_cycle = self.get_current_reward_cycle(); - let valid_signer_index = self.get_signer_index(reward_cycle); - let round = self - .stacks_client - .get_last_round(reward_cycle) - .expect("FATAL: failed to get round") - .unwrap_or(0) - .saturating_add(1); - let point = Point::from(Scalar::random(&mut rand::thread_rng())); - let invalid_nonce_tx = self - .stacks_client - .build_vote_for_aggregate_public_key( - valid_signer_index, - round, - point, - reward_cycle, + + let signer_private_key = self.signer_stacks_private_keys[0]; + + let vote_contract_id = boot_code_id(SIGNERS_VOTING_NAME, false); + let contract_addr = vote_contract_id.issuer.into(); + let contract_name = vote_contract_id.name.clone(); + + let signer_index = thread_rng().next_u64(); + let signer_index_arg = Value::UInt(signer_index as u128); + + let point = Point::from(Scalar::random(&mut thread_rng())); + let point_arg = + Value::buff_from(point.compress().data.to_vec()).expect("Failed to create buff"); + + let round = thread_rng().next_u64(); + let round_arg = Value::UInt(round as u128); + + let reward_cycle_arg = Value::UInt(reward_cycle as u128); + let valid_function_args = vec![ + signer_index_arg.clone(), + point_arg.clone(), + round_arg.clone(), + reward_cycle_arg.clone(), + ]; + + // Create a invalid transaction that is not a contract call + let invalid_not_contract_call = StacksTransaction { + version: TransactionVersion::Testnet, + chain_id: CHAIN_ID_TESTNET, + auth: TransactionAuth::from_p2pkh(&signer_private_key).unwrap(), + anchor_mode: TransactionAnchorMode::Any, + post_condition_mode: TransactionPostConditionMode::Allow, + post_conditions: vec![], + payload: TransactionPayload::SmartContract( + TransactionSmartContract { + name: "test-contract".into(), + code_body: StacksString::from_str("(/ 1 0)").unwrap(), + }, None, - 0, // Old nonce + ), + }; + let invalid_contract_address = StacksClient::build_signed_contract_call_transaction( + &StacksAddress::p2pkh(false, &StacksPublicKey::from_private(&signer_private_key)), + contract_name.clone(), + SIGNERS_VOTING_FUNCTION_NAME.into(), + &valid_function_args, + &signer_private_key, + TransactionVersion::Testnet, + CHAIN_ID_TESTNET, + 1, + 10, + ) + .unwrap(); + + let invalid_contract_name = StacksClient::build_signed_contract_call_transaction( + &contract_addr, + "bad-signers-contract-name".into(), + SIGNERS_VOTING_FUNCTION_NAME.into(), + &valid_function_args, + &signer_private_key, + TransactionVersion::Testnet, + CHAIN_ID_TESTNET, + 1, + 10, + ) + .unwrap(); + + let invalid_signers_vote_function = StacksClient::build_signed_contract_call_transaction( + &contract_addr, + contract_name.clone(), + "some-other-function".into(), + &valid_function_args, + &signer_private_key, + TransactionVersion::Testnet, + CHAIN_ID_TESTNET, + 1, + 10, + ) + .unwrap(); + + let invalid_function_arg_signer_index = + StacksClient::build_signed_contract_call_transaction( + &contract_addr, + contract_name.clone(), + SIGNERS_VOTING_FUNCTION_NAME.into(), + &[ + point_arg.clone(), + point_arg.clone(), + round_arg.clone(), + reward_cycle_arg.clone(), + ], + &signer_private_key, + TransactionVersion::Testnet, + CHAIN_ID_TESTNET, + 1, + 10, ) - .expect("FATAL: failed to build vote for aggregate public key"); + .unwrap(); + + let invalid_function_arg_key = StacksClient::build_signed_contract_call_transaction( + &contract_addr, + contract_name.clone(), + SIGNERS_VOTING_FUNCTION_NAME.into(), + &[ + signer_index_arg.clone(), + signer_index_arg.clone(), + round_arg.clone(), + reward_cycle_arg.clone(), + ], + &signer_private_key, + TransactionVersion::Testnet, + CHAIN_ID_TESTNET, + 1, + 10, + ) + .unwrap(); + + let invalid_function_arg_round = StacksClient::build_signed_contract_call_transaction( + &contract_addr, + contract_name.clone(), + SIGNERS_VOTING_FUNCTION_NAME.into(), + &[ + signer_index_arg.clone(), + point_arg.clone(), + point_arg.clone(), + reward_cycle_arg.clone(), + ], + &signer_private_key, + TransactionVersion::Testnet, + CHAIN_ID_TESTNET, + 1, + 10, + ) + .unwrap(); + + let invalid_function_arg_reward_cycle = + StacksClient::build_signed_contract_call_transaction( + &contract_addr, + contract_name.clone(), + SIGNERS_VOTING_FUNCTION_NAME.into(), + &[ + signer_index_arg.clone(), + point_arg.clone(), + round_arg.clone(), + point_arg.clone(), + ], + &signer_private_key, + TransactionVersion::Testnet, + CHAIN_ID_TESTNET, + 1, + 10, + ) + .unwrap(); + + let invalid_nonce = StacksClient::build_signed_contract_call_transaction( + &contract_addr, + contract_name.clone(), + SIGNERS_VOTING_FUNCTION_NAME.into(), + &valid_function_args, + &signer_private_key, + TransactionVersion::Testnet, + CHAIN_ID_TESTNET, + 0, // Old nonce + 10, + ) + .unwrap(); + let invalid_stacks_client = StacksClient::new(StacksPrivateKey::new(), host, false); let invalid_signer_tx = invalid_stacks_client - .build_vote_for_aggregate_public_key( - valid_signer_index, - round, - point, - reward_cycle, - None, - 0, - ) + .build_vote_for_aggregate_public_key(0, round, point, reward_cycle, None, 0) .expect("FATAL: failed to build vote for aggregate public key"); - // TODO: add invalid contract calls (one with non 'vote-for-aggregate-public-key' function call and one with invalid function args) - vec![invalid_nonce_tx, invalid_signer_tx] + + vec![ + invalid_nonce, + invalid_not_contract_call, + invalid_contract_name, + invalid_contract_address, + invalid_signers_vote_function, + invalid_function_arg_key, + invalid_function_arg_reward_cycle, + invalid_function_arg_round, + invalid_function_arg_signer_index, + invalid_signer_tx, + ] } fn shutdown(self) { From ce308f33c6df9d620a3d64c36eed8241b0281f04 Mon Sep 17 00:00:00 2001 From: Jacinta Ferrant Date: Wed, 28 Feb 2024 09:03:34 -0500 Subject: [PATCH 09/10] CRC: rename point vars to dkg_public_key for clarity Signed-off-by: Jacinta Ferrant --- stacks-signer/src/client/stacks_client.rs | 8 ++++---- stacks-signer/src/signer.rs | 18 +++++++++--------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/stacks-signer/src/client/stacks_client.rs b/stacks-signer/src/client/stacks_client.rs index 5e422b3c6..471cec068 100644 --- a/stacks-signer/src/client/stacks_client.rs +++ b/stacks-signer/src/client/stacks_client.rs @@ -483,12 +483,12 @@ impl StacksClient { "Failed to convert aggregate public key to compressed data: {e}" )) })?; - let point = Point::try_from(&compressed_data).map_err(|e| { + let dkg_public_key = Point::try_from(&compressed_data).map_err(|e| { ClientError::MalformedClarityValue(format!( "Failed to convert aggregate public key to a point: {e}" )) })?; - Ok(Some(point)) + Ok(Some(dkg_public_key)) } /// Helper function to create a stacks transaction for a modifying contract call @@ -496,7 +496,7 @@ impl StacksClient { &self, signer_index: u32, round: u64, - point: Point, + dkg_public_key: Point, reward_cycle: u64, tx_fee: Option, nonce: u64, @@ -507,7 +507,7 @@ impl StacksClient { let function_name = ClarityName::from(SIGNERS_VOTING_FUNCTION_NAME); let function_args = vec![ ClarityValue::UInt(signer_index as u128), - ClarityValue::buff_from(point.compress().data.to_vec())?, + ClarityValue::buff_from(dkg_public_key.compress().data.to_vec())?, ClarityValue::UInt(round as u128), ClarityValue::UInt(reward_cycle as u128), ]; diff --git a/stacks-signer/src/signer.rs b/stacks-signer/src/signer.rs index e759f2ff7..e67d20370 100644 --- a/stacks-signer/src/signer.rs +++ b/stacks-signer/src/signer.rs @@ -839,8 +839,8 @@ impl Signer { OperationResult::SignTaproot(_) => { debug!("Signer #{}: Received a signature result for a taproot signature. Nothing to broadcast as we currently sign blocks with a FROST signature.", self.signer_id); } - OperationResult::Dkg(point) => { - self.process_dkg(stacks_client, point); + OperationResult::Dkg(dkg_public_key) => { + self.process_dkg(stacks_client, dkg_public_key); } OperationResult::SignError(e) => { warn!("Signer #{}: Received a Sign error: {e:?}", self.signer_id); @@ -855,7 +855,7 @@ impl Signer { } /// Process a dkg result by broadcasting a vote to the stacks node - fn process_dkg(&mut self, stacks_client: &StacksClient, point: &Point) { + fn process_dkg(&mut self, stacks_client: &StacksClient, dkg_public_key: &Point) { let epoch = retry_with_exponential_backoff(|| { stacks_client .get_node_epoch() @@ -895,7 +895,7 @@ impl Signer { match stacks_client.build_vote_for_aggregate_public_key( self.stackerdb.get_signer_slot_id(), self.coordinator.current_dkg_id, - *point, + *dkg_public_key, self.reward_cycle, tx_fee, next_nonce, @@ -908,14 +908,14 @@ impl Signer { new_transaction, ) { warn!( - "Signer #{}: Failed to broadcast DKG vote ({point:?}): {e:?}", + "Signer #{}: Failed to broadcast DKG public key vote ({dkg_public_key:?}): {e:?}", self.signer_id ); } } Err(e) => { warn!( - "Signer #{}: Failed to build DKG vote ({point:?}) transaction: {e:?}.", + "Signer #{}: Failed to build DKG public key vote ({dkg_public_key:?}) transaction: {e:?}.", self.signer_id ); } @@ -1137,15 +1137,15 @@ impl Signer { }).unwrap_or_default(); // Check if we have an existing vote transaction for the same round and reward cycle for transaction in old_transactions.iter() { - let (_index, point, round, reward_cycle) = + let (_index, dkg_public_key, round, reward_cycle) = NakamotoSigners::parse_vote_for_aggregate_public_key(transaction).expect(&format!("BUG: Signer #{}: Received an invalid {SIGNERS_VOTING_FUNCTION_NAME} transaction in an already filtered list: {transaction:?}", self.signer_id)); - if Some(point) == self.coordinator.aggregate_public_key + if Some(dkg_public_key) == self.coordinator.aggregate_public_key && round == self.coordinator.current_dkg_id && reward_cycle == self.reward_cycle { debug!("Signer #{}: Not triggering a DKG round. Already have a pending vote transaction.", self.signer_id; "txid" => %transaction.txid(), - "point" => %point, + "dkg_public_key" => %dkg_public_key, "round" => round ); return Ok(()); From c533146b7d4d1dd38ac381fc54fcd18a3bd3a6bd Mon Sep 17 00:00:00 2001 From: Jacinta Ferrant Date: Fri, 1 Mar 2024 08:15:05 -0500 Subject: [PATCH 10/10] CRC: pull parsed aggregate key vote results into a struct Signed-off-by: Jacinta Ferrant --- stacks-signer/src/signer.rs | 14 ++++++------- .../src/chainstate/nakamoto/signer_set.rs | 20 +++++++++++++++---- .../src/chainstate/nakamoto/tests/mod.rs | 11 +++++----- 3 files changed, 28 insertions(+), 17 deletions(-) diff --git a/stacks-signer/src/signer.rs b/stacks-signer/src/signer.rs index e67d20370..fd80138aa 100644 --- a/stacks-signer/src/signer.rs +++ b/stacks-signer/src/signer.rs @@ -1131,22 +1131,22 @@ impl Signer { // Have I already voted and have a pending transaction? Check stackerdb for the same round number and reward cycle vote transaction // Only get the account nonce of THIS signer as we only care about our own votes, not other signer votes let signer_address = stacks_client.get_signer_address(); - let account_nonces = self.get_account_nonces(stacks_client, &[signer_address.clone()]); + let account_nonces = self.get_account_nonces(stacks_client, &[*signer_address]); let old_transactions = self.get_signer_transactions(&account_nonces).map_err(|e| { warn!("Signer #{}: Failed to get old signer transactions: {e:?}. May trigger DKG unnecessarily", self.signer_id); }).unwrap_or_default(); // Check if we have an existing vote transaction for the same round and reward cycle for transaction in old_transactions.iter() { - let (_index, dkg_public_key, round, reward_cycle) = - NakamotoSigners::parse_vote_for_aggregate_public_key(transaction).expect(&format!("BUG: Signer #{}: Received an invalid {SIGNERS_VOTING_FUNCTION_NAME} transaction in an already filtered list: {transaction:?}", self.signer_id)); - if Some(dkg_public_key) == self.coordinator.aggregate_public_key - && round == self.coordinator.current_dkg_id + let params = + NakamotoSigners::parse_vote_for_aggregate_public_key(transaction).unwrap_or_else(|| panic!("BUG: Signer #{}: Received an invalid {SIGNERS_VOTING_FUNCTION_NAME} transaction in an already filtered list: {transaction:?}", self.signer_id)); + if Some(params.aggregate_key) == self.coordinator.aggregate_public_key + && params.voting_round == self.coordinator.current_dkg_id && reward_cycle == self.reward_cycle { debug!("Signer #{}: Not triggering a DKG round. Already have a pending vote transaction.", self.signer_id; "txid" => %transaction.txid(), - "dkg_public_key" => %dkg_public_key, - "round" => round + "aggregate_key" => %params.aggregate_key, + "voting_round" => params.voting_round ); return Ok(()); } diff --git a/stackslib/src/chainstate/nakamoto/signer_set.rs b/stackslib/src/chainstate/nakamoto/signer_set.rs index a39d6bebc..504928690 100644 --- a/stackslib/src/chainstate/nakamoto/signer_set.rs +++ b/stackslib/src/chainstate/nakamoto/signer_set.rs @@ -99,6 +99,13 @@ pub struct SignerCalculation { pub events: Vec, } +pub struct AggregateKeyVoteParams { + pub signer_index: u64, + pub aggregate_key: Point, + pub voting_round: u64, + pub reward_cycle: u64, +} + impl RawRewardSetEntry { pub fn from_pox_4_tuple(is_mainnet: bool, tuple: TupleData) -> Result { let mut tuple_data = tuple.data_map; @@ -515,7 +522,7 @@ impl NakamotoSigners { pub fn parse_vote_for_aggregate_public_key( transaction: &StacksTransaction, - ) -> Option<(u64, Point, u64, u64)> { + ) -> Option { let TransactionPayload::ContractCall(payload) = &transaction.payload else { // Not a contract call so not a special cased vote for aggregate public key transaction return None; @@ -535,12 +542,17 @@ impl NakamotoSigners { let point_value = payload.function_args.get(1)?; let point_bytes = point_value.clone().expect_buff(33).ok()?; let compressed_data = Compressed::try_from(point_bytes.as_slice()).ok()?; - let point = Point::try_from(&compressed_data).ok()?; + let aggregate_key = Point::try_from(&compressed_data).ok()?; let round_value = payload.function_args.get(2)?; - let round = u64::try_from(round_value.clone().expect_u128().ok()?).ok()?; + let voting_round = u64::try_from(round_value.clone().expect_u128().ok()?).ok()?; let reward_cycle = u64::try_from(payload.function_args.get(3)?.clone().expect_u128().ok()?).ok()?; - Some((signer_index, point, round, reward_cycle)) + Some(AggregateKeyVoteParams { + signer_index, + aggregate_key, + voting_round, + reward_cycle, + }) } /// Update the map of filtered valid transactions, selecting one per address based first on lowest nonce, then txid diff --git a/stackslib/src/chainstate/nakamoto/tests/mod.rs b/stackslib/src/chainstate/nakamoto/tests/mod.rs index c27505b51..28d620b81 100644 --- a/stackslib/src/chainstate/nakamoto/tests/mod.rs +++ b/stackslib/src/chainstate/nakamoto/tests/mod.rs @@ -2122,12 +2122,11 @@ fn parse_vote_for_aggregate_public_key_valid() { function_args: valid_function_args, }), }; - let (res_signer_index, res_point, res_round, res_reward_cycle) = - NakamotoSigners::parse_vote_for_aggregate_public_key(&valid_tx).unwrap(); - assert_eq!(res_signer_index, signer_index); - assert_eq!(res_point, point); - assert_eq!(res_round, round); - assert_eq!(res_reward_cycle, reward_cycle); + let params = NakamotoSigners::parse_vote_for_aggregate_public_key(&valid_tx).unwrap(); + assert_eq!(params.signer_index, signer_index); + assert_eq!(params.aggregate_key, point); + assert_eq!(params.voting_round, round); + assert_eq!(params.reward_cycle, reward_cycle); } #[test]