PR feedback from @ralexstokes and add single pubkey aggregate tests

specs/altair/bls.md

@@ -46,9 +46,8 @@ def eth_aggregate_pubkeys(pubkeys: Sequence[BLSPubkey]) -> BLSPubkey:
     Refer to the BLS signature draft standard for more information.
     """
     assert len(pubkeys) > 0
-    for pubkey in pubkeys:
-        # Ensure that the given inputs are valid pubkeys
-        assert bls.KeyValidate(pubkey)
+    # Ensure that the given inputs are valid pubkeys
+    assert all(bls.KeyValidate(pubkey) for pubkey in pubkeys)
 
     result = copy(pubkeys[0])
     for pubkey in pubkeys[1:]:

tests/core/pyspec/eth2spec/test/phase0/block_processing/test_process_attester_slashing.py

@@ -306,7 +306,7 @@ def test_att1_empty_indices(spec, state):
     attester_slashing = get_valid_attester_slashing(spec, state, signed_1=False, signed_2=True)
 
     attester_slashing.attestation_1.attesting_indices = []
-    attester_slashing.attestation_1.signature = spec.bls.Z2_SIGNATURE
+    attester_slashing.attestation_1.signature = spec.bls.G2_POINT_AT_INFINITY
 
     yield from run_attester_slashing_processing(spec, state, attester_slashing, False)
 
@@ -318,7 +318,7 @@ def test_att2_empty_indices(spec, state):
     attester_slashing = get_valid_attester_slashing(spec, state, signed_1=True, signed_2=False)
 
     attester_slashing.attestation_2.attesting_indices = []
-    attester_slashing.attestation_2.signature = spec.bls.Z2_SIGNATURE
+    attester_slashing.attestation_2.signature = spec.bls.G2_POINT_AT_INFINITY
 
     yield from run_attester_slashing_processing(spec, state, attester_slashing, False)
 
@@ -330,10 +330,10 @@ def test_all_empty_indices(spec, state):
     attester_slashing = get_valid_attester_slashing(spec, state, signed_1=False, signed_2=False)
 
     attester_slashing.attestation_1.attesting_indices = []
-    attester_slashing.attestation_1.signature = spec.bls.Z2_SIGNATURE
+    attester_slashing.attestation_1.signature = spec.bls.G2_POINT_AT_INFINITY
 
     attester_slashing.attestation_2.attesting_indices = []
-    attester_slashing.attestation_2.signature = spec.bls.Z2_SIGNATURE
+    attester_slashing.attestation_2.signature = spec.bls.G2_POINT_AT_INFINITY
 
     yield from run_attester_slashing_processing(spec, state, attester_slashing, False)
 

tests/core/pyspec/eth2spec/utils/bls.py

@@ -10,9 +10,8 @@
 
 STUB_SIGNATURE = b'\x11' * 96
 STUB_PUBKEY = b'\x22' * 48
-Z1_PUBKEY = b'\xc0' + b'\x00' * 47
-Z2_SIGNATURE = b'\xc0' + b'\x00' * 95
-STUB_COORDINATES = _signature_to_G2(Z2_SIGNATURE)
+G2_POINT_AT_INFINITY = b'\xc0' + b'\x00' * 95
+STUB_COORDINATES = _signature_to_G2(G2_POINT_AT_INFINITY)
 
 
 def use_milagro():
@@ -96,8 +95,7 @@ def signature_to_G2(signature):
 @only_with_bls(alt_return=STUB_PUBKEY)
 def AggregatePKs(pubkeys):
     if bls == py_ecc_bls:
-        for pubkey in pubkeys:
-            assert bls.KeyValidate(pubkey)
+        assert all(bls.KeyValidate(pubkey) for pubkey in pubkeys)
     elif bls == milagro_bls:
         # milagro_bls._AggregatePKs checks KeyValidate internally
         pass

tests/generators/bls/main.py

@@ -54,9 +54,11 @@ def hex_to_int(x: str) -> int:
 PUBKEYS = [bls.SkToPk(privkey) for privkey in PRIVKEYS]
 
 ZERO_PUBKEY = b'\x00' * 48
-Z1_PUBKEY = b'\xc0' + b'\x00' * 47
-NO_SIGNATURE = b'\x00' * 96
-Z2_SIGNATURE = b'\xc0' + b'\x00' * 95
+G1_POINT_AT_INFINITY = b'\xc0' + b'\x00' * 47
+
+ZERO_SIGNATURE = b'\x00' * 96
+G2_POINT_AT_INFINITY = b'\xc0' + b'\x00' * 95
+
 ZERO_PRIVKEY = 0
 ZERO_PRIVKEY_BYTES = b'\x00' * 32
 
@@ -149,13 +151,13 @@ def case02_verify():
             }
 
     # Invalid pubkey and signature with the point at infinity
-    assert not bls.Verify(Z1_PUBKEY, SAMPLE_MESSAGE, Z2_SIGNATURE)
-    assert not milagro_bls.Verify(Z1_PUBKEY, SAMPLE_MESSAGE, Z2_SIGNATURE)
+    assert not bls.Verify(G1_POINT_AT_INFINITY, SAMPLE_MESSAGE, G2_POINT_AT_INFINITY)
+    assert not milagro_bls.Verify(G1_POINT_AT_INFINITY, SAMPLE_MESSAGE, G2_POINT_AT_INFINITY)
     yield f'verify_infinity_pubkey_and_infinity_signature', {
         'input': {
-            'pubkey': encode_hex(Z1_PUBKEY),
+            'pubkey': encode_hex(G1_POINT_AT_INFINITY),
             'message': encode_hex(SAMPLE_MESSAGE),
-            'signature': encode_hex(Z2_SIGNATURE),
+            'signature': encode_hex(G2_POINT_AT_INFINITY),
         },
         'output': False,
     }
@@ -181,10 +183,10 @@ def case03_aggregate():
     }
 
     # Valid to aggregate G2 point at infinity
-    aggregate_sig = bls.Aggregate([Z2_SIGNATURE])
-    assert aggregate_sig == milagro_bls.Aggregate([Z2_SIGNATURE]) == Z2_SIGNATURE
+    aggregate_sig = bls.Aggregate([G2_POINT_AT_INFINITY])
+    assert aggregate_sig == milagro_bls.Aggregate([G2_POINT_AT_INFINITY]) == G2_POINT_AT_INFINITY
     yield f'aggregate_infinity_signature', {
-        'input': [encode_hex(Z2_SIGNATURE)],
+        'input': [encode_hex(G2_POINT_AT_INFINITY)],
         'output': encode_hex(aggregate_sig),
     }
 
@@ -240,32 +242,32 @@ def case04_fast_aggregate_verify():
         }
 
     # Invalid pubkeys and signature -- len(pubkeys) == 0 and signature == Z1_SIGNATURE
-    assert not bls.FastAggregateVerify([], message, Z2_SIGNATURE)
-    assert not milagro_bls.FastAggregateVerify([], message, Z2_SIGNATURE)
+    assert not bls.FastAggregateVerify([], message, G2_POINT_AT_INFINITY)
+    assert not milagro_bls.FastAggregateVerify([], message, G2_POINT_AT_INFINITY)
     yield f'fast_aggregate_verify_na_pubkeys_and_infinity_signature', {
         'input': {
             'pubkeys': [],
             'message': encode_hex(message),
-            'signature': encode_hex(Z2_SIGNATURE),
+            'signature': encode_hex(G2_POINT_AT_INFINITY),
         },
         'output': False,
     }
 
     # Invalid pubkeys and signature -- len(pubkeys) == 0 and signature == 0x00...
-    assert not bls.FastAggregateVerify([], message, NO_SIGNATURE)
-    assert not milagro_bls.FastAggregateVerify([], message, NO_SIGNATURE)
-    yield f'fast_aggregate_verify_na_pubkeys_and_na_signature', {
+    assert not bls.FastAggregateVerify([], message, ZERO_SIGNATURE)
+    assert not milagro_bls.FastAggregateVerify([], message, ZERO_SIGNATURE)
+    yield f'fast_aggregate_verify_na_pubkeys_and_zero_signature', {
         'input': {
             'pubkeys': [],
             'message': encode_hex(message),
-            'signature': encode_hex(NO_SIGNATURE),
+            'signature': encode_hex(ZERO_SIGNATURE),
         },
         'output': False,
     }
 
     # Invalid pubkeys and signature -- pubkeys contains point at infinity
     pubkeys = PUBKEYS.copy()
-    pubkeys_with_infinity = pubkeys + [Z1_PUBKEY]
+    pubkeys_with_infinity = pubkeys + [G1_POINT_AT_INFINITY]
     signatures = [bls.Sign(privkey, SAMPLE_MESSAGE) for privkey in PRIVKEYS]
     aggregate_signature = bls.Aggregate(signatures)
     assert not bls.FastAggregateVerify(pubkeys_with_infinity, SAMPLE_MESSAGE, aggregate_signature)
@@ -320,31 +322,31 @@ def case05_aggregate_verify():
     }
 
     # Invalid pubkeys and signature -- len(pubkeys) == 0 and signature == Z1_SIGNATURE
-    assert not bls.AggregateVerify([], [], Z2_SIGNATURE)
-    assert not milagro_bls.AggregateVerify([], [], Z2_SIGNATURE)
+    assert not bls.AggregateVerify([], [], G2_POINT_AT_INFINITY)
+    assert not milagro_bls.AggregateVerify([], [], G2_POINT_AT_INFINITY)
     yield f'aggregate_verify_na_pubkeys_and_infinity_signature', {
         'input': {
             'pubkeys': [],
             'messages': [],
-            'signature': encode_hex(Z2_SIGNATURE),
+            'signature': encode_hex(G2_POINT_AT_INFINITY),
         },
         'output': False,
     }
 
     # Invalid pubkeys and signature -- len(pubkeys) == 0 and signature == 0x00...
-    assert not bls.AggregateVerify([], [], NO_SIGNATURE)
-    assert not milagro_bls.AggregateVerify([], [], NO_SIGNATURE)
-    yield f'aggregate_verify_na_pubkeys_and_na_signature', {
+    assert not bls.AggregateVerify([], [], ZERO_SIGNATURE)
+    assert not milagro_bls.AggregateVerify([], [], ZERO_SIGNATURE)
+    yield f'aggregate_verify_na_pubkeys_and_zero_signature', {
         'input': {
             'pubkeys': [],
             'messages': [],
-            'signature': encode_hex(NO_SIGNATURE),
+            'signature': encode_hex(ZERO_SIGNATURE),
         },
         'output': False,
     }
 
     # Invalid pubkeys and signature -- pubkeys contains point at infinity
-    pubkeys_with_infinity = pubkeys + [Z1_PUBKEY]
+    pubkeys_with_infinity = pubkeys + [G1_POINT_AT_INFINITY]
     messages_with_sample = messages + [SAMPLE_MESSAGE]
     assert not bls.AggregateVerify(pubkeys_with_infinity, messages_with_sample, aggregate_signature)
     assert not milagro_bls.AggregateVerify(pubkeys_with_infinity, messages_with_sample, aggregate_signature)
@@ -359,9 +361,21 @@ def case05_aggregate_verify():
 
 
 def case06_eth_aggregate_pubkeys():
+    for pubkey in PUBKEYS:
+        encoded_pubkey = encode_hex(pubkey)
+        aggregate_pubkey = spec.eth_aggregate_pubkeys([pubkey])
+        # Should be unchanged
+        assert aggregate_pubkey == milagro_bls._AggregatePKs([pubkey]) == pubkey
+        # Valid pubkey
+        yield f'eth_aggregate_pubkeys_valid_{(hash(bytes(encoded_pubkey, "utf-8"))[:8]).hex()}', {
+            'input': [encode_hex(pubkey)],
+            'output': encode_hex(aggregate_pubkey),
+        }
+
+    # Valid pubkeys
     aggregate_pubkey = spec.eth_aggregate_pubkeys(PUBKEYS)
     assert aggregate_pubkey == milagro_bls._AggregatePKs(PUBKEYS)
-    yield f'eth_aggregate_pubkeys_some_pubkeys', {
+    yield f'eth_aggregate_pubkeys_valid_pubkeys', {
         'input': [encode_hex(pubkey) for pubkey in PUBKEYS],
         'output': encode_hex(aggregate_pubkey),
     }
@@ -377,16 +391,16 @@ def case06_eth_aggregate_pubkeys():
     # Invalid pubkeys -- [ZERO_PUBKEY]
     expect_exception(spec.eth_aggregate_pubkeys, [ZERO_PUBKEY])
     expect_exception(milagro_bls._AggregatePKs, [ZERO_PUBKEY])
-    yield f'eth_aggregate_pubkeys_na_pubkey', {
+    yield f'eth_aggregate_pubkeys_zero_pubkey', {
         'input': [encode_hex(ZERO_PUBKEY)],
         'output': None,
     }
 
     # Invalid pubkeys -- G1 point at infinity
-    expect_exception(spec.eth_aggregate_pubkeys, [Z1_PUBKEY])
-    expect_exception(milagro_bls._AggregatePKs, [Z1_PUBKEY])
+    expect_exception(spec.eth_aggregate_pubkeys, [G1_POINT_AT_INFINITY])
+    expect_exception(milagro_bls._AggregatePKs, [G1_POINT_AT_INFINITY])
     yield f'eth_aggregate_pubkeys_infinity_pubkey', {
-        'input': [encode_hex(Z1_PUBKEY)],
+        'input': [encode_hex(G1_POINT_AT_INFINITY)],
         'output': None,
     }
 
@@ -450,31 +464,31 @@ def case07_eth_fast_aggregate_verify():
             'output': False,
         }
 
-    # NOTE: Unlike `FastAggregateVerify`, len(pubkeys) == 0 and signature == Z2_SIGNATURE is VALID
-    assert spec.eth_fast_aggregate_verify([], message, Z2_SIGNATURE)
+    # NOTE: Unlike `FastAggregateVerify`, len(pubkeys) == 0 and signature == G2_POINT_AT_INFINITY is VALID
+    assert spec.eth_fast_aggregate_verify([], message, G2_POINT_AT_INFINITY)
     yield f'eth_fast_aggregate_verify_na_pubkeys_and_infinity_signature', {
         'input': {
             'pubkeys': [],
             'message': encode_hex(message),
-            'signature': encode_hex(Z2_SIGNATURE),
+            'signature': encode_hex(G2_POINT_AT_INFINITY),
         },
         'output': True,
     }
 
     # Invalid pubkeys and signature -- len(pubkeys) == 0 and signature == 0x00...
-    assert not spec.eth_fast_aggregate_verify([], message, NO_SIGNATURE)
-    yield f'eth_fast_aggregate_verify_na_pubkeys_and_na_signature', {
+    assert not spec.eth_fast_aggregate_verify([], message, ZERO_SIGNATURE)
+    yield f'eth_fast_aggregate_verify_na_pubkeys_and_zero_signature', {
         'input': {
             'pubkeys': [],
             'message': encode_hex(message),
-            'signature': encode_hex(NO_SIGNATURE),
+            'signature': encode_hex(ZERO_SIGNATURE),
         },
         'output': False,
     }
 
     # Invalid pubkeys and signature -- pubkeys contains point at infinity
     pubkeys = PUBKEYS.copy()
-    pubkeys_with_infinity = pubkeys + [Z1_PUBKEY]
+    pubkeys_with_infinity = pubkeys + [G1_POINT_AT_INFINITY]
     signatures = [bls.Sign(privkey, SAMPLE_MESSAGE) for privkey in PRIVKEYS]
     aggregate_signature = bls.Aggregate(signatures)
     assert not spec.eth_fast_aggregate_verify(pubkeys_with_infinity, SAMPLE_MESSAGE, aggregate_signature)