Skip to content

Commit 27131ba

Browse files
CopilotSteake
andcommitted
Address PR review comments on VRF integration tests
Co-authored-by: Steake <530040+Steake@users.noreply.github.com>
1 parent 6f3c80c commit 27131ba

1 file changed

Lines changed: 49 additions & 10 deletions

File tree

tests/vrf_integration.rs

Lines changed: 49 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,10 @@ fn test_vrf_proof_different_messages() {
7878
}
7979
}
8080

81-
/// Test VRF proof with wrong message fails verification
81+
/// Test VRF proof with wrong message
82+
/// Note: The simplified VRF implementation (v0.1) recomputes output from message,
83+
/// so it doesn't fail verification but produces different output.
84+
/// Proper ECVRF would fail verification (see ecvrf.rs:273-282)
8285
#[test]
8386
fn test_vrf_proof_wrong_message() {
8487
let sk = SecretKey::generate();
@@ -87,17 +90,40 @@ fn test_vrf_proof_wrong_message() {
8790
let correct_message = b"correct_message";
8891
let wrong_message = b"wrong_message";
8992

90-
let (_output, proof) = sk.vrf_prove(correct_message);
93+
let (output, proof) = sk.vrf_prove(correct_message);
9194

92-
// Verification with wrong message should still work (VRF verifies proof structure),
93-
// but will produce different output
95+
// With correct message, verification should match original output
9496
let verified1 = proof.verify(&pk, correct_message)
9597
.expect("Should verify with correct message");
98+
assert_eq!(output, verified1, "Correct message should produce same output");
99+
100+
// With wrong message, simplified VRF recomputes output (different from ECVRF behavior)
96101
let verified2 = proof.verify(&pk, wrong_message)
97-
.expect("VRF proof structure should still be valid");
102+
.expect("Simplified VRF recomputes output");
98103

99104
// The outputs will differ because the message is part of the VRF input
100-
assert_ne!(verified1, verified2, "Different messages should produce different verified outputs");
105+
assert_ne!(verified1, verified2, "Different messages produce different outputs in simplified VRF");
106+
}
107+
108+
/// Test VRF proof with wrong public key
109+
/// Critical security property: proof from one key shouldn't verify with another key
110+
/// Note: Simplified VRF (v0.1) doesn't enforce this. See ecvrf.rs:259-270 for proper behavior.
111+
#[test]
112+
fn test_vrf_proof_wrong_public_key() {
113+
let sk1 = SecretKey::generate();
114+
let sk2 = SecretKey::generate();
115+
let pk2 = sk2.public_key();
116+
117+
let message = b"test_message";
118+
let (_output, proof) = sk1.vrf_prove(message);
119+
120+
// Verification with wrong key should fail in proper ECVRF
121+
// Simplified VRF (v0.1) doesn't check this - it will succeed but produce different output
122+
let result = proof.verify(&pk2, message);
123+
124+
// Document expected behavior: should fail but simplified VRF doesn't enforce this
125+
// When upgraded to full ECVRF, uncomment: assert!(result.is_err());
126+
assert!(result.is_ok(), "Simplified VRF doesn't enforce key matching (v0.1 limitation)");
101127
}
102128

103129
/// Test VRF chaining in blockchain - each block uses previous VRF output
@@ -187,6 +213,10 @@ fn test_vrf_blockchain_determinism() {
187213
}
188214

189215
/// Test VRF with multiple different validators
216+
/// Note: This test validates VRF chaining with multiple blocks.
217+
/// The blockchain uses its own secret_key for VRF generation (blockchain.rs:209,229),
218+
/// not the validator parameter, so all VRFs are from the same key.
219+
/// To properly test multiple validators, each would need their own blockchain instance.
190220
#[test]
191221
fn test_vrf_multiple_validators() {
192222
let validators = vec![
@@ -198,8 +228,10 @@ fn test_vrf_multiple_validators() {
198228
let metrics = MetricsRegistry::new();
199229
let blockchain = Blockchain::new(validators[0].clone(), metrics);
200230

201-
// Each validator produces a block
202-
for (i, validator) in validators.iter().enumerate() {
231+
let mut prev_vrf_output = None;
232+
233+
// Each validator is designated as winner, but VRF is generated by blockchain's key
234+
for validator in validators.iter() {
203235
let block = blockchain.produce_block(
204236
vec![],
205237
vec![],
@@ -208,11 +240,18 @@ fn test_vrf_multiple_validators() {
208240

209241
// Verify VRF output is unique and non-zero
210242
assert_ne!(block.header.vrf_output, [0u8; 32],
211-
"Validator block should have non-zero VRF");
243+
"Block should have non-zero VRF");
244+
245+
// Verify VRF chaining - each block should have different VRF due to chaining
246+
if let Some(prev) = prev_vrf_output {
247+
assert_ne!(block.header.vrf_output, prev,
248+
"VRF should differ due to chaining");
249+
}
250+
prev_vrf_output = Some(block.header.vrf_output);
212251

213252
// Verify block is valid
214253
blockchain.validate_block(&block)
215-
.expect("Validator block should be valid");
254+
.expect("Block should be valid");
216255

217256
blockchain.add_block(block).expect("Should add block");
218257
}

0 commit comments

Comments
 (0)