From c687791d92ca4f2fe0fa62e5818f5d8f472fa9e7 Mon Sep 17 00:00:00 2001 From: Aaron Blankstein Date: Mon, 1 Jul 2024 13:38:37 -0500 Subject: [PATCH 1/4] test: v0 signature assert fixes * in V0, the miner can assemble a block once it crosses the threshold, without all signers. the existing assertion would cause CI to flap. --- testnet/stacks-node/src/tests/signer/v0.rs | 37 +++++++++++++++------- 1 file changed, 25 insertions(+), 12 deletions(-) diff --git a/testnet/stacks-node/src/tests/signer/v0.rs b/testnet/stacks-node/src/tests/signer/v0.rs index c1d6169f7d..bdf1b4c33f 100644 --- a/testnet/stacks-node/src/tests/signer/v0.rs +++ b/testnet/stacks-node/src/tests/signer/v0.rs @@ -128,31 +128,44 @@ impl SignerTest { // Verify that the signers signed the proposed block let signature = self.wait_for_confirmed_block_v0(&proposed_signer_signature_hash, timeout); + // NOTE: signature.len() does not need to equal signers.len(); the stacks miner can finish the block + // whenever it has crossed the threshold. info!("Got {} signatures", signature.len()); - assert_eq!(signature.len(), num_signers); - let reward_cycle = self.get_current_reward_cycle(); let signers = self.get_reward_set_signers(reward_cycle); // Verify that the signers signed the proposed block - let all_signed = signers.iter().zip(signature).all(|(signer, signature)| { + let mut signer_index = 0; + let mut signature_index = 0; + let validated = loop { + let Some(signature) = signature.get(signature_index) else { + break true; + }; + let Some(signer) = signers.get(signer_index) else { + error!("Failed to validate the mined nakamoto block: ran out of signers to try to validate signatures"); + break false; + }; let stacks_public_key = Secp256k1PublicKey::from_slice(signer.signing_key.as_slice()) .expect("Failed to convert signing key to StacksPublicKey"); - - // let valid = stacks_public_key.verify(message, signature); let valid = stacks_public_key - .verify(&message, &signature) + .verify(&message, signature) .expect("Failed to verify signature"); if !valid { - error!( - "Failed to verify signature for signer: {:?}", - stacks_public_key + info!( + "Failed to verify signature for signer, will attempt to validate without this signer"; + "signer_pk" => stacks_public_key.to_hex(), + "signer_index" => signer_index, + "signature_index" => signature_index, ); + signer_index += 1; + } else { + signer_index += 1; + signature_index += 1; } - valid - }); - assert!(all_signed); + }; + + assert!(validated); } // Only call after already past the epoch 3.0 boundary From 9f13a1da505ab99273d31131b81046c7fb15f764 Mon Sep 17 00:00:00 2001 From: Jacinta Ferrant Date: Mon, 1 Jul 2024 15:03:51 -0400 Subject: [PATCH 2/4] Fix number of signatures to check against in mine 2 nakamoto tenures Signed-off-by: Jacinta Ferrant --- testnet/stacks-node/src/tests/signer/v0.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/testnet/stacks-node/src/tests/signer/v0.rs b/testnet/stacks-node/src/tests/signer/v0.rs index bdf1b4c33f..04c1b1a8a8 100644 --- a/testnet/stacks-node/src/tests/signer/v0.rs +++ b/testnet/stacks-node/src/tests/signer/v0.rs @@ -128,9 +128,11 @@ impl SignerTest { // Verify that the signers signed the proposed block let signature = self.wait_for_confirmed_block_v0(&proposed_signer_signature_hash, timeout); + info!("Got {} signatures", signature.len()); + // NOTE: signature.len() does not need to equal signers.len(); the stacks miner can finish the block // whenever it has crossed the threshold. - info!("Got {} signatures", signature.len()); + assert!(signature.len() >= num_signers / 7 * 10); let reward_cycle = self.get_current_reward_cycle(); let signers = self.get_reward_set_signers(reward_cycle); From 7768347f72d811e44c634cb475b2199d4c852cd6 Mon Sep 17 00:00:00 2001 From: Aaron Blankstein Date: Tue, 2 Jul 2024 08:05:19 -0500 Subject: [PATCH 3/4] test: fix threshold assertion in signer::v0 Co-authored-by: Jeff Bencin --- testnet/stacks-node/src/tests/signer/v0.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testnet/stacks-node/src/tests/signer/v0.rs b/testnet/stacks-node/src/tests/signer/v0.rs index 04c1b1a8a8..d66751cccf 100644 --- a/testnet/stacks-node/src/tests/signer/v0.rs +++ b/testnet/stacks-node/src/tests/signer/v0.rs @@ -132,7 +132,7 @@ impl SignerTest { // NOTE: signature.len() does not need to equal signers.len(); the stacks miner can finish the block // whenever it has crossed the threshold. - assert!(signature.len() >= num_signers / 7 * 10); + assert!(signature.len() >= num_signers * 7 / 10); let reward_cycle = self.get_current_reward_cycle(); let signers = self.get_reward_set_signers(reward_cycle); From 8ea1a8a4c1363bff654a4abe86731243423a9621 Mon Sep 17 00:00:00 2001 From: Aaron Blankstein Date: Tue, 2 Jul 2024 08:28:56 -0500 Subject: [PATCH 4/4] chore: add test assertion comment --- testnet/stacks-node/src/tests/signer/v0.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/testnet/stacks-node/src/tests/signer/v0.rs b/testnet/stacks-node/src/tests/signer/v0.rs index d66751cccf..a837365dad 100644 --- a/testnet/stacks-node/src/tests/signer/v0.rs +++ b/testnet/stacks-node/src/tests/signer/v0.rs @@ -141,6 +141,8 @@ impl SignerTest { let mut signer_index = 0; let mut signature_index = 0; let validated = loop { + // Since we've already checked `signature.len()`, this means we've + // validated all the signatures in this loop let Some(signature) = signature.get(signature_index) else { break true; };