Skip to content

Commit 31f44dd

Browse files
jayz22jayz22
committed
correct the negative-g tests
1 parent 91ece74 commit 31f44dd

File tree

4 files changed

+113
-47
lines changed

4 files changed

+113
-47
lines changed

‎soroban-env-host/observations/25/test__bn254__test_bn254_g1_deserialize_rejects_y_sign_bit.json‎

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,44 @@
11
{
22
" 0 begin": "cpu:0, mem:0, prngs:-/-, objs:-/-, vm:-/-, evt:-, store:-/-, foot:-, stk:-, auth:-/-",
33
" 1 call bytes_new_from_slice(64)": "cpu:688",
4-
" 2 ret bytes_new_from_slice -> Ok(Bytes(obj#1))": "cpu:1667, mem:160, objs:-/1@1eb8c8fe",
4+
" 2 ret bytes_new_from_slice -> Ok(Bytes(obj#1))": "cpu:1667, mem:160, objs:-/1@33e03491",
55
" 3 call bytes_copy_to_slice(Bytes(obj#1), U32(0), 64)": "",
66
" 4 ret bytes_copy_to_slice -> Ok(())": "cpu:1778",
7-
" 5 end": "cpu:2818, mem:320, prngs:-/-, objs:-/2@631f248e, vm:-/-, evt:-, store:-/-, foot:-, stk:-, auth:-/-"
7+
" 5 call bytes_new_from_slice(64)": "cpu:3506, mem:320, objs:-/2@fd8cc406",
8+
" 6 ret bytes_new_from_slice -> Ok(Bytes(obj#5))": "cpu:4485, mem:480, objs:-/3@557acb46",
9+
" 7 call bytes_copy_to_slice(Bytes(obj#5), U32(0), 64)": "",
10+
" 8 ret bytes_copy_to_slice -> Ok(())": "cpu:4596",
11+
" 9 call bytes_new_from_slice(64)": "cpu:6324, mem:640, objs:-/4@f61c921",
12+
" 10 ret bytes_new_from_slice -> Ok(Bytes(obj#9))": "cpu:7303, mem:800, objs:-/5@8336a1ce",
13+
" 11 call bytes_copy_to_slice(Bytes(obj#9), U32(0), 64)": "",
14+
" 12 ret bytes_copy_to_slice -> Ok(())": "cpu:7414",
15+
" 13 call bytes_new_from_slice(64)": "cpu:9142, mem:960, objs:-/6@168a749a",
16+
" 14 ret bytes_new_from_slice -> Ok(Bytes(obj#13))": "cpu:10121, mem:1120, objs:-/7@e0797ac1",
17+
" 15 call bytes_copy_to_slice(Bytes(obj#13), U32(0), 64)": "",
18+
" 16 ret bytes_copy_to_slice -> Ok(())": "cpu:10232",
19+
" 17 call bytes_new_from_slice(64)": "cpu:11960, mem:1280, objs:-/8@9b6cc918",
20+
" 18 ret bytes_new_from_slice -> Ok(Bytes(obj#17))": "cpu:12939, mem:1440, objs:-/9@2268b300",
21+
" 19 call bytes_copy_to_slice(Bytes(obj#17), U32(0), 64)": "",
22+
" 20 ret bytes_copy_to_slice -> Ok(())": "cpu:13050",
23+
" 21 call bytes_new_from_slice(64)": "cpu:14778, mem:1600, objs:-/10@170b9258",
24+
" 22 ret bytes_new_from_slice -> Ok(Bytes(obj#21))": "cpu:15757, mem:1760, objs:-/11@dea52bdc",
25+
" 23 call bytes_copy_to_slice(Bytes(obj#21), U32(0), 64)": "",
26+
" 24 ret bytes_copy_to_slice -> Ok(())": "cpu:15868",
27+
" 25 call bytes_new_from_slice(64)": "cpu:17596, mem:1920, objs:-/12@b5e452bd",
28+
" 26 ret bytes_new_from_slice -> Ok(Bytes(obj#25))": "cpu:18575, mem:2080, objs:-/13@3f368ae0",
29+
" 27 call bytes_copy_to_slice(Bytes(obj#25), U32(0), 64)": "",
30+
" 28 ret bytes_copy_to_slice -> Ok(())": "cpu:18686",
31+
" 29 call bytes_new_from_slice(64)": "cpu:20414, mem:2240, objs:-/14@69cceb0c",
32+
" 30 ret bytes_new_from_slice -> Ok(Bytes(obj#29))": "cpu:21393, mem:2400, objs:-/15@d92c241",
33+
" 31 call bytes_copy_to_slice(Bytes(obj#29), U32(0), 64)": "",
34+
" 32 ret bytes_copy_to_slice -> Ok(())": "cpu:21504",
35+
" 33 call bytes_new_from_slice(64)": "cpu:23232, mem:2560, objs:-/16@f9a4c09c",
36+
" 34 ret bytes_new_from_slice -> Ok(Bytes(obj#33))": "cpu:24211, mem:2720, objs:-/17@1c9000f2",
37+
" 35 call bytes_copy_to_slice(Bytes(obj#33), U32(0), 64)": "",
38+
" 36 ret bytes_copy_to_slice -> Ok(())": "cpu:24322",
39+
" 37 call bytes_new_from_slice(64)": "cpu:26050, mem:2880, objs:-/18@3d7d48a9",
40+
" 38 ret bytes_new_from_slice -> Ok(Bytes(obj#37))": "cpu:27029, mem:3040, objs:-/19@e3a7db9a",
41+
" 39 call bytes_copy_to_slice(Bytes(obj#37), U32(0), 64)": "",
42+
" 40 ret bytes_copy_to_slice -> Ok(())": "cpu:27140",
43+
" 41 end": "cpu:28180, mem:3200, prngs:-/-, objs:-/20@dbec9ab, vm:-/-, evt:-, store:-/-, foot:-, stk:-, auth:-/-"
844
}

‎soroban-env-host/observations/25/test__bn254__test_bn254_g2_deserialize_rejects_y_sign_bit.json‎

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,26 @@
11
{
22
" 0 begin": "cpu:0, mem:0, prngs:-/-, objs:-/-, vm:-/-, evt:-, store:-/-, foot:-, stk:-, auth:-/-",
33
" 1 call bytes_new_from_slice(128)": "cpu:1376",
4-
" 2 ret bytes_new_from_slice -> Ok(Bytes(obj#1))": "cpu:2371, mem:224, objs:-/1@4a50f3a2",
4+
" 2 ret bytes_new_from_slice -> Ok(Bytes(obj#1))": "cpu:2371, mem:224, objs:-/1@62ecc3ea",
55
" 3 call bytes_copy_to_slice(Bytes(obj#1), U32(0), 128)": "",
66
" 4 ret bytes_copy_to_slice -> Ok(())": "cpu:2490",
7-
" 5 end": "cpu:3546, mem:448, prngs:-/-, objs:-/2@72bbc3f0, vm:-/-, evt:-, store:-/-, foot:-, stk:-, auth:-/-"
7+
" 5 call bytes_copy_to_slice(Bytes(obj#1), U32(0), 128)": "cpu:3546, mem:448, objs:-/2@72f8d01b",
8+
" 6 ret bytes_copy_to_slice -> Ok(())": "cpu:3665",
9+
" 7 call bytes_copy_to_slice(Bytes(obj#1), U32(0), 128)": "cpu:4721, mem:672, objs:-/3@e68bd088",
10+
" 8 ret bytes_copy_to_slice -> Ok(())": "cpu:4840",
11+
" 9 call bytes_copy_to_slice(Bytes(obj#1), U32(0), 128)": "cpu:5896, mem:896, objs:-/4@8a54f28",
12+
" 10 ret bytes_copy_to_slice -> Ok(())": "cpu:6015",
13+
" 11 call bytes_copy_to_slice(Bytes(obj#1), U32(0), 128)": "cpu:7071, mem:1120, objs:-/5@3a682175",
14+
" 12 ret bytes_copy_to_slice -> Ok(())": "cpu:7190",
15+
" 13 call bytes_copy_to_slice(Bytes(obj#1), U32(0), 128)": "cpu:8246, mem:1344, objs:-/6@8dda692d",
16+
" 14 ret bytes_copy_to_slice -> Ok(())": "cpu:8365",
17+
" 15 call bytes_copy_to_slice(Bytes(obj#1), U32(0), 128)": "cpu:9421, mem:1568, objs:-/7@e3cc5f61",
18+
" 16 ret bytes_copy_to_slice -> Ok(())": "cpu:9540",
19+
" 17 call bytes_copy_to_slice(Bytes(obj#1), U32(0), 128)": "cpu:10596, mem:1792, objs:-/8@fd6b01b1",
20+
" 18 ret bytes_copy_to_slice -> Ok(())": "cpu:10715",
21+
" 19 call bytes_copy_to_slice(Bytes(obj#1), U32(0), 128)": "cpu:11771, mem:2016, objs:-/9@b5ec8a9e",
22+
" 20 ret bytes_copy_to_slice -> Ok(())": "cpu:11890",
23+
" 21 call bytes_copy_to_slice(Bytes(obj#1), U32(0), 128)": "cpu:12946, mem:2240, objs:-/10@b0a16390",
24+
" 22 ret bytes_copy_to_slice -> Ok(())": "cpu:13065",
25+
" 23 end": "cpu:14121, mem:2464, prngs:-/-, objs:-/11@5ca70dcd, vm:-/-, evt:-, store:-/-, foot:-, stk:-, auth:-/-"
826
}

‎soroban-env-host/src/crypto/bls12_381.rs‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,6 @@ impl Host {
4545
// (the base field element), and will be charged accordingly.
4646
// Validation of the deserialized entity must be performed outside of this
4747
// function, to keep budget charging isolated.
48-
49-
// TODO: this function has to be moved into curve_utils because it is used by both
5048
pub(crate) fn deserialize_uncompressed_no_validate<
5149
const EXPECTED_SIZE: usize,
5250
T: CanonicalDeserialize,

‎soroban-env-host/src/test/bn254.rs‎

Lines changed: 55 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,13 @@ fn negative_g1(host: &Host, rng: &mut StdRng) -> Result<BytesObject, HostError>
8080
// infinity
8181
continue;
8282
}
83-
if g1.y <= -g1.y {
83+
if g1.y > -g1.y {
84+
// check that the Y-sign bit is indeed set in arkworks by retrieving
85+
// the MSB (the last byte in little-endian serialized buffer) and
86+
// asserting on the flag bit
87+
let mut buf = [0u8; BN254_G1_SERIALIZED_SIZE];
88+
g1.serialize_uncompressed(&mut buf[..]).unwrap(); // this is little-endian
89+
assert!(buf[BN254_G1_SERIALIZED_SIZE - 1] & 0b1000_0000 != 0u8);
8490
// we get our point
8591
return host.bn254_g1_affine_serialize_uncompressed(&g1);
8692
}
@@ -176,7 +182,13 @@ fn negative_g2(host: &Host, rng: &mut StdRng) -> Result<BytesObject, HostError>
176182
// infinity
177183
continue;
178184
}
179-
if g2.y <= -g2.y {
185+
if g2.y > -g2.y {
186+
// check that the Y-sign bit is indeed set in arkworks by retrieving
187+
// the MSB (the last byte in little-endian serialized buffer) and
188+
// asserting on the flag bit
189+
let mut buf = [0u8; BN254_G2_SERIALIZED_SIZE];
190+
g2.serialize_uncompressed(&mut buf[..]).unwrap(); // this is little-endian
191+
assert!(buf[BN254_G2_SERIALIZED_SIZE - 1] & 0b1000_0000 != 0u8);
180192
// we get our point
181193
return host.bn254_g2_affine_serialize_uncompressed(&g2);
182194
}
@@ -811,30 +823,31 @@ fn test_bn254_g1_deserialize_rejects_y_sign_bit() -> Result<(), HostError> {
811823
host.enable_debug()?;
812824
let mut rng = StdRng::from_seed([0u8; 32]);
813825

814-
// This is a negative g1 point serialized by us, it should not set the y-sign bit
815-
let g1_bytes_obj = negative_g1(&host, &mut rng)?;
826+
for _ in 0..10 {
827+
// This is a negative g1 point serialized by us, it should not set the y-sign bit
828+
let g1_bytes_obj = negative_g1(&host, &mut rng)?;
816829

817-
// Get the bytes and verify the y-sign bit is NOT set
818-
let mut g1_bytes = vec![0u8; BN254_G1_SERIALIZED_SIZE];
819-
host.bytes_copy_to_slice(g1_bytes_obj, U32Val::from(0), &mut g1_bytes)?;
820-
assert_eq!(
821-
g1_bytes[0] & 0b1000_0000,
822-
0,
823-
"Y-sign bit should not be set even for negative y"
824-
);
830+
// Get the bytes and verify the y-sign bit is NOT set
831+
let mut g1_bytes = vec![0u8; BN254_G1_SERIALIZED_SIZE];
832+
host.bytes_copy_to_slice(g1_bytes_obj, U32Val::from(0), &mut g1_bytes)?;
833+
assert_eq!(
834+
g1_bytes[0] & 0b1000_0000,
835+
0,
836+
"Y-sign bit should not be set even for negative y"
837+
);
825838

826-
// Now manually set the y-sign bit (bit 0, which is 0x80 in the MSB)
827-
g1_bytes[0] |= 0b1000_0000;
839+
// Now manually set the y-sign bit (bit 0, which is 0x80 in the MSB)
840+
g1_bytes[0] |= 0b1000_0000;
828841

829-
// Try to deserialize - should fail
830-
let g1_bytes_obj_modified = host.test_bin_obj(&g1_bytes)?;
831-
let result = host.bn254_g1_affine_deserialize(g1_bytes_obj_modified);
832-
833-
assert!(HostError::result_matches_err(
834-
result,
835-
(ScErrorType::Crypto, ScErrorCode::InvalidInput)
836-
));
842+
// Try to deserialize - should fail
843+
let g1_bytes_obj_modified = host.test_bin_obj(&g1_bytes)?;
844+
let result = host.bn254_g1_affine_deserialize(g1_bytes_obj_modified);
837845

846+
assert!(HostError::result_matches_err(
847+
result,
848+
(ScErrorType::Crypto, ScErrorCode::InvalidInput)
849+
));
850+
}
838851
Ok(())
839852
}
840853

@@ -876,30 +889,31 @@ fn test_bn254_g2_deserialize_rejects_y_sign_bit() -> Result<(), HostError> {
876889
host.enable_debug()?;
877890
let mut rng = StdRng::from_seed([0u8; 32]);
878891

879-
// This is a negative g1 point serialized by us, it should not set the y-sign bit
892+
// This is a negative G1 point serialized by us, it should not set the y-sign bit
880893
let g2_bytes_obj = negative_g2(&host, &mut rng)?;
881894

882-
// Get the bytes and verify the y-sign bit is NOT set
883-
let mut g2_bytes = vec![0u8; BN254_G2_SERIALIZED_SIZE];
884-
host.bytes_copy_to_slice(g2_bytes_obj, U32Val::from(0), &mut g2_bytes)?;
885-
assert_eq!(
886-
g2_bytes[0] & 0b1000_0000,
887-
0,
888-
"Y-sign bit should not be set even for negative y"
889-
);
895+
for _ in 0..10 {
896+
// Get the bytes and verify the y-sign bit is NOT set
897+
let mut g2_bytes = vec![0u8; BN254_G2_SERIALIZED_SIZE];
898+
host.bytes_copy_to_slice(g2_bytes_obj, U32Val::from(0), &mut g2_bytes)?;
899+
assert_eq!(
900+
g2_bytes[0] & 0b1000_0000,
901+
0,
902+
"Y-sign bit should not be set even for negative y"
903+
);
890904

891-
// Now manually set the y-sign bit (bit 0, which is 0x80 in the MSB)
892-
g2_bytes[0] |= 0b1000_0000;
905+
// Now manually set the y-sign bit (bit 0, which is 0x80 in the MSB)
906+
g2_bytes[0] |= 0b1000_0000;
893907

894-
// Try to deserialize - should fail
895-
let g2_bytes_obj_modified = host.test_bin_obj(&g2_bytes)?;
896-
let result = host.bn254_g2_affine_deserialize(g2_bytes_obj_modified);
897-
898-
assert!(HostError::result_matches_err(
899-
result,
900-
(ScErrorType::Crypto, ScErrorCode::InvalidInput)
901-
));
908+
// Try to deserialize - should fail
909+
let g2_bytes_obj_modified = host.test_bin_obj(&g2_bytes)?;
910+
let result = host.bn254_g2_affine_deserialize(g2_bytes_obj_modified);
902911

912+
assert!(HostError::result_matches_err(
913+
result,
914+
(ScErrorType::Crypto, ScErrorCode::InvalidInput)
915+
));
916+
}
903917
Ok(())
904918
}
905919

0 commit comments

Comments
 (0)