Consensys / jblst

MIT License
8 stars 11 forks source link

Performance issue in BLS.aggregate method for v 0.3.3-1 #12

Closed vikulin closed 2 years ago

vikulin commented 3 years ago

See details in: https://github.com/supranational/blst/issues/66

benjaminion commented 3 years ago

Thanks for the report!

This probably relates more directly to Teku's BLS interface than jblst itself.

It looks like the aggregate method from the Blst library is checking group membership for every signature that is aggregated. This slows down aggregation considerably.

I get a ~30x speedup in my naive benchmark if I modify Teku's signature aggregation as follows:

     P2 sum = new P2();
     for (BlstSignature finiteSignature : signatures) {
-      sum.aggregate(finiteSignature.ec2Point);
+      sum.add(finiteSignature.ec2Point);
     }

The group membership check needs to be done somewhere. Previously (Mikuli implementation) we did the membership check at signature deserialisation and not at aggregation. With Blst it looks like the check is not done at deserialisation, but at aggregation time instead.

For Teku, either way is safe, since all our signatures are received over the wire serialised, but I need to think about which place is best to check this.

benjaminion commented 2 years ago

Updating and closing this. In Teku we took care of this at an application level by caching the group check status of public key objects, without modifying jblst. Other applications can do something similar.