perf: improve attestations efficiency

[ElFantasma]

Motivation

Attestation check is taking more than 2 seconds in two different moments of the slot processing.

After profiling it was discovered that most of the time was consumed in deserializing the attesters public keys.

On mainnet we have almost all slots with 128 attestations, and around 500 attesters on each. Taking about 20ms each attestation validation and more than 2 seconds for all the 128 attestations.

Note the limit of attester per attestation is 2048 so it can get worse. For example on Holesky we were taking about twice that time.

This matches our benchmark:

keys           ips        average  deviation         median         99th %
1          1541.45        0.65 ms    ±34.44%        0.62 ms        1.48 ms
10         1018.04        0.98 ms     ±3.41%        0.98 ms        1.06 ms
100         218.49        4.58 ms     ±1.43%        4.57 ms        4.76 ms
500          48.75       20.51 ms     ±1.07%       20.46 ms       21.15 ms
2048         12.19       82.05 ms     ±0.68%       81.92 ms       84.11 ms

Description

This fix is actually a hack. Instead of using the provided PublicKey::deserialization() by the bls library wich runs several key validations, we use a lighter deserialization that does not validate the key.

These validations seemed unnecessary as the attestation fails anyway if any of the public key is invalid or has a bad encoding.

In any case, the aggregated signature validation is spec-tested and it is passing all the tests.

New benchs:

keys           ips        average  deviation         median         99th %
1          1679.57        0.60 ms     ±6.87%        0.59 ms        0.70 ms
10         1454.03        0.69 ms     ±2.74%        0.68 ms        0.75 ms
100         611.09        1.64 ms     ±2.21%        1.63 ms        1.76 ms
500         171.12        5.84 ms     ±1.83%        5.83 ms        6.34 ms
2048         45.39       22.03 ms     ±0.87%       22.00 ms       22.67 ms

And on our application:

After

Before

[ElFantasma]

Note:

The validation included in PublicKey::deserialize is just a check that the key is on the eliptic curve:

            pub fn validate(&self) -> Result<(), BLST_ERROR> {
                unsafe {
                    if $pk_is_inf(&self.point) {
                        return Err(BLST_ERROR::BLST_PK_IS_INFINITY);
                    }
                    if !$pk_in_group(&self.point) {
                        return Err(BLST_ERROR::BLST_POINT_NOT_IN_GROUP);
                    }
                }
                Ok(())
            }

(As well as the Infinity Publick Check that we are still doing)
Avoiding this check does not prevent the aggregated signature verification to fail if some key is invalid.

[rodrigo-o]

LGTM given the explanation! I added just a small comment related to the unwrap.

[rodrigo-o]

Nit: Bubbling the error should be better than the unwrap even if its something that shouldn't be reached, in any case we'll have an issue, we could go with something like:

fn fast_public_key_deserialize(pk: &[u8]) -> Result<PublicKey, String> {
    if pk == &bls::INFINITY_PUBLIC_KEY[..] {
        Err("Infinity public Key".to_owned())
    } else {
        bls::impls::blst::types::PublicKey::from_bytes(pk)
            .map_err(|err| format!("BlstError({:?})", err))
            .and_then(|pk| {
                PublicKey::deserialize_uncompressed(pk.serialize().as_slice())
                    .map_err(|e| format!("Deserialization error: {:?}", e))
            })
    }
}