Open twoeths opened 4 months ago
I got a POC built and it doesnt add as much time as you noted above. Please check out my methodology.
that looks really great @matthewkeil
Please update with status on its testing with observations for inclusion! Thanks!
There is little appreciable difference in metrics between the blst rebuild and the branch that is adding in the randomization factor for multiple same message verifications. While present it seems much less than anticipated (2x reduction in performance of attestation verifications).
In all images below the vertical bar was placed about where the restart happened. The branch that was deployed to the left was mkeil/test-blst-7
and the one to the right was mkeil/test-blst-mult-in-verification-2
. Both branches are almost identical except for the randomization change. I did rebase mkeil/test-blst-7
on unstable when I started the multiplyBy
work so it is up to date with the current state for comparisons with the unstable group. The couple issues that jumped out at me are:
21.06.2024 17:46:05 UTC-05:00 - Matthew Keil: Hi @kevaundray and @jtraglia !!! We have a couple crypto questions and I wanted to reach out to you guys to make introductions with a couple of our team members that are interested in the discussion
21.06.2024 17:46:27 UTC-05:00 - System: @caymannan is our team lead and @tuyennhv is a super star that loves this stuff
21.06.2024 17:47:33 UTC-05:00 - Cayman: 👋 hey yall
21.06.2024 17:47:37 UTC-05:00 - Matthew Keil: We implemented a function for aggregating signatures for attestations and PR'd it to supranational/blst. We found it increases our performance by a large amount but to prevent attack surface we need to mult in some randomness similar to how mult_agg_ver works under the hood....
21.06.2024 17:48:50 UTC-05:00 - Kev: Hello!
21.06.2024 17:49:39 UTC-05:00 - Matthew Keil: https://github.com/supranational/blst/pull/225
21.06.2024 17:50:08 UTC-05:00 - Cayman: was poking around with Matt in the blst library but it is quite cryptic, was hoping to pick yalls brain
21.06.2024 17:50:10 UTC-05:00 - Matthew Keil: We found a few other "mult" related function in the blst repo but are unsure of the specifics for their use and it sparked us to want to reach out
21.06.2024 17:57:29 UTC-05:00 - Kev: Just reading the thread now to get context 🙂
21.06.2024 18:04:24 UTC-05:00 - System: Ah okay I think I understand, I'll re-iterate to make sure:
- You have a triplet of signature, message and public key.
- Currently there is a method to verify all of these faster than verifying them individually called verify_multiple_aggregate_signatures
- What you would like to do is:
- Aggregate the signatures and public keys with the same message first
- Call the verify_multiple_aggregate_signatures
on this new aggregated set where all messages are distinct
21.06.2024 18:05:11 UTC-05:00 - Cayman: yup
21.06.2024 18:06:09 UTC-05:00 - Matthew Keil: saves a ton of thread time to aggregate first and run a single pairing computation
21.06.2024 18:08:19 UTC-05:00 - Kev: I believe the "correct" API would be to have a multiply method on whatever the public key format is
21.06.2024 18:08:29 UTC-05:00 - System: Oh wait, actually we might be able to get something that is faster
21.06.2024 18:09:01 UTC-05:00 - Matthew Keil: exactly... that is how i had it at first but the conversion to and from affine to stay with the rust pk signature is not ideal
21.06.2024 18:09:23 UTC-05:00 - Kev: If you are doing multiple scalar multiplications and then adding them together, you can actually do it a lot faster by calling the msm algorithm
21.06.2024 18:09:37 UTC-05:00 - Matthew Keil: sweet. i knew we should ask you
21.06.2024 18:09:42 UTC-05:00 - System: what is msm algo?
21.06.2024 18:09:59 UTC-05:00 - Kev: An MSM computes a point P such that P = a_0 G_0 + a_1 G_1 +...+ a_n G_n
21.06.2024 18:10:00 UTC-05:00 - Matthew Keil: btw here is the post we followed to get us this far...
https://ethresear.ch/t/fast-verification-of-multiple-bls-signatures/5407
21.06.2024 18:10:47 UTC-05:00 - Kev: I think you can do everything in jacobian and then convert to affine when you are done, or perhaps keep the public key in jacobian and only convert to affine for something like serialization
21.06.2024 18:11:34 UTC-05:00 - System: isn't the computation done using libuv threadpool?
21.06.2024 18:11:43 UTC-05:00 - Matthew Keil: the rust struct has them in affine for some reason and we dont want to modify that... but yes we went from having a mulitply_by on the PK and Sig to the functions in the PR to stay in jacobian
21.06.2024 18:12:07 UTC-05:00 - Cayman: heh, we've been doing some work in background threads but not all
21.06.2024 18:12:07 UTC-05:00 - Matthew Keil: https://github.com/supranational/blst/pull/225/commits/4da63bddbf53bcdc299061d175d6649693894eed
21.06.2024 18:12:54 UTC-05:00 - System: yes. but sadly libuv does not play nicely with docker after a ton of exploration. works great on bare metal but when we perf tested on docker it was running like a dog....
21.06.2024 18:13:04 UTC-05:00 - Cayman: but this past week we started on a napi-rs binding of blst, and started on offloading a lot of work into tokio managed threads
21.06.2024 18:13:55 UTC-05:00 - System: today we were poking around the blst library on this "aggregate_with_randomness" function and definitely seemed like we were missing something
21.06.2024 18:14:01 UTC-05:00 - Matthew Keil: we are working with the lead maintainer of libuv Ben Noordhuis and he told us there are PR's in the works but there is no timeline for them to get merged....
21.06.2024 18:16:25 UTC-05:00 - Justin Traglia: Hello too 👋
21.06.2024 18:16:32 UTC-05:00 - Matthew Keil: HI!!!
21.06.2024 18:17:07 UTC-05:00 - Cayman: hiya
21.06.2024 18:17:18 UTC-05:00 - Justin Traglia: I’m not really able to help right now (on vacation), but you’re in good hands with Kev 🙂
21.06.2024 18:18:11 UTC-05:00 - Matthew Keil: Im writing to you from a cruise ship off the coast of Italy on "workation" so can totally appreciate that!!! enjoy vacation and ttys
21.06.2024 18:18:36 UTC-05:00 - Justin Traglia: Haha awesome. You too!
21.06.2024 18:21:13 UTC-05:00 - Kev: I think a function like this could make sense -- Is it not possible to validate the public keys outside of this method?
21.06.2024 18:22:15 UTC-05:00 - Cayman: yeah for our case, the pubkeys are already validated, but the signatures are not yet validated
21.06.2024 18:22:29 UTC-05:00 - Matthew Keil: they already are validated when we deserialize them from state
21.06.2024 18:23:05 UTC-05:00 - Kev: Is it possible to do the signature validation logic outside of that method?
21.06.2024 18:23:12 UTC-05:00 - System: so it only does aggregation
21.06.2024 18:23:17 UTC-05:00 - Matthew Keil: sure
21.06.2024 18:23:37 UTC-05:00 - Kev: I think we can do what is needed without modifying the blst repo
21.06.2024 18:24:23 UTC-05:00 - Cayman: that would be ideal really
21.06.2024 18:24:45 UTC-05:00 - Kev: Are you interfacing with the rust blst library?
21.06.2024 18:24:53 UTC-05:00 - Cayman: yeah
21.06.2024 18:25:13 UTC-05:00 - Matthew Keil: for napi-rs wrapping the rust bindings we need access to the non-pub point to use the native blst functions though
21.06.2024 18:25:34 UTC-05:00 - System: was the issue i ran into in our bindings layer
21.06.2024 18:25:41 UTC-05:00 - Cayman: our bindings are in a PR here https://github.com/ChainSafe/blst-ts/pull/149/files#diff-292c21dc26ee204d63939308bcec83b4957275c30f13c74fa3db036c0917860a
21.06.2024 18:26:40 UTC-05:00 - Kev: Ah could you elaborate? I'm not sure how napi and blst are linked here
21.06.2024 18:27:24 UTC-05:00 - Matthew Keil: we are using the blst/bindings/rust and wrapping it in napi-rs to run in lodestar
21.06.2024 18:31:11 UTC-05:00 - System: just realized the link is not auto expanding... this is the file that matters
https://github.com/ChainSafe/blst-ts/blob/cayman/napi-rs/next/src/lib.rs
21.06.2024 18:44:21 UTC-05:00 - Kev: Okay got it -- was looking at the bindings code -- hmm since Public key is not marked as reprC it would be dangerous to convert it to a raw pointer and cast to the pk_affine type
21.06.2024 18:48:07 UTC-05:00 - System: I couldn't find a way around creating a new PublicKey type in your repository which you would then operate on instead and call the relevant methods. (You could serialize and then deserialize, but this is expensive)
This is likely not desirable I'm guessing since you need to maintain it, so perhaps asking the blst maintainer to expose the inner pk_affine type would be the lowest hanging fruit
21.06.2024 18:51:48 UTC-05:00 - System: The code for reference would look something like this:
21.06.2024 18:59:40 UTC-05:00 - Cayman: yeah thats not exactly desirable
21.06.2024 18:59:57 UTC-05:00 - System: maybe the blst maintainers would be fine exposing msm?
21.06.2024 19:00:36 UTC-05:00 - Kev: It should already be exposed, I think the issue is that the inner type in PublicKey is not exposed
21.06.2024 19:00:46 UTC-05:00 - System: Because that’s what the msm method takes in
21.06.2024 19:01:02 UTC-05:00 - Cayman: oh right, I guess exposing it as a method on PublicKey or AggregatePublicKey
21.06.2024 19:01:11 UTC-05:00 - Matthew Keil: which is why i upstreamed that functionality...
21.06.2024 19:01:29 UTC-05:00 - Kev: Yep or expose the inner type
21.06.2024 19:01:49 UTC-05:00 - Cayman: yeah if they expose the inner type then we can just do whatever we want
21.06.2024 19:02:08 UTC-05:00 - Kev: It’s unclear to me if that PR will get merged in a timely manner though
21.06.2024 19:02:21 UTC-05:00 - Cayman: yea :(
21.06.2024 19:03:01 UTC-05:00 - Matthew Keil: is the approach we took to aggregate with randomness correct? this was the heart of why we reached out. we were wondering if there was a better blst function like _pippenger etc that might be better than the simple
pub struct LodestarPublicKey {
point : blst_p1_affine
}
impl LodestarPublicKey {
pub fn validate(&self) -> bool {
// convert to the blst public key type
let pk = self.convert_to_blst()
// call validate on blst public key
pk.validate()
}
pub fn aggregate_with_randomness(&self, ...) {
// custom logic
}
pub fn deserialize(bytes: &[u8]) -> Self {
// copy logic to deserialize from blst public key impl
}
}blst_p_mult
that we used... ie the MSM. but we are unsure of the way to implement those safely
21.06.2024 19:03:09 UTC-05:00 - Kev: For a single message, how many public keys on average would you be aggregating?
21.06.2024 19:04:16 UTC-05:00 - Matthew Keil: let me get back to you with that. off the cuff i think its up to 1000 but i need to ask @tuyen cuz it may be way less per batch... ill commit to getting you that answer asap though
21.06.2024 19:04:17 UTC-05:00 - Cayman: probably 100 or more?
21.06.2024 19:04:41 UTC-05:00 - Kev: I think the only footgun with the msm method the way it’s implemented in blst, is that you need to ensure that none of the points are “zero/point at infinity”
21.06.2024 19:05:08 UTC-05:00 - System: If that is the case, it will return the point at infinity as the result unconditionally
21.06.2024 19:05:14 UTC-05:00 - Matthew Keil: we are doing a sig check and we also do a pk validate for sure on all pks
21.06.2024 19:05:14 UTC-05:00 - Kev: Oh wow
21.06.2024 19:06:03 UTC-05:00 - System: This would likely give a good improvement in speed, the msm function can also be parallelised internally
21.06.2024 19:06:08 UTC-05:00 - Matthew Keil: but the sig check ideally would be implemented in the aggregation function
21.06.2024 19:08:26 UTC-05:00 - System: like the best function signature for us would be fn aggregate_with_randomness(pks: &[PublicKey], sigs: &[Uint8Array]) -> (PublicKey, Signature)
.... whether we pass in the randomness or its generated in the function is fine but guessing the preference would be to pass in however there is another footgun with too little randomness (found that the hard way) so may be safer to have it built within the function
21.06.2024 19:09:17 UTC-05:00 - System: and same goes for sig verification... that should not be optional for consumers
21.06.2024 19:10:15 UTC-05:00 - Cayman: do you know why the blst rust library has bool parameters for pk_validate but not for pk_infcheck in PublicKey methods? whereas in Signature methods, there's both sig_groupcheck and sig_infcheck?
21.06.2024 19:11:25 UTC-05:00 - Kev: It’s probably better to generate it by hashing all of the input — haven’t checked to see if the blst library does any sort of randomness generation currently, if it does then it wouldn’t be anything new
21.06.2024 19:12:41 UTC-05:00 - Matthew Keil: it does not. they specifically exclude RNG
21.06.2024 19:13:32 UTC-05:00 - System: but in our bindings (and the rust/lighthouse) implementation we gen the randomness in the bindings layer
21.06.2024 19:13:37 UTC-05:00 - Kev: I think possibly because a zero public key is reasonable but a zero signature is likely not
21.06.2024 19:13:56 UTC-05:00 - Cayman: right, we already have rng for verify_multiple_aggregate_signatures, we can reuse the logic there
21.06.2024 19:16:06 UTC-05:00 - Kev: Is there any concern with respects to security/audits here?
If this change gets merged in, then you would be using unaudited code (probably safe)
21.06.2024 19:16:50 UTC-05:00 - Cayman: I would say some concern around use of the blst primitives
21.06.2024 19:17:33 UTC-05:00 - System: just making sure we didn't screw anything up
21.06.2024 19:17:58 UTC-05:00 - Matthew Keil: i spoke with Fredrick at interop and he mentioned we could request audit of our bindings layer so if we were to make the stuct members pub we could implement more easily and could have it sent for audit (internal or external was what Fredrick mentioned)
21.06.2024 19:17:59 UTC-05:00 - Kev: I do think your approach in the PR is reasonable though, and in terms of moving forward, I guess it depends on what the maintainer is comfortable with merging
21.06.2024 19:18:59 UTC-05:00 - Cayman: Yeah, it may be worth trying for a smaller diff (just making the underlying point public)
21.06.2024 19:19:00 UTC-05:00 - Matthew Keil: i think from this the takeway is that we are approaching the problem correctly but may be better and will provide more flexibility to expose the point for use by use downstream...
21.06.2024 19:19:41 UTC-05:00 - System: yep, exactly... unless the lighthouse team also want to exploit the same perf benefit
21.06.2024 19:19:53 UTC-05:00 - System: we could build it for us and they will get the glory lol
21.06.2024 19:20:38 UTC-05:00 - System: im going to reach out to Age and Dapplion to see if they are interested....
21.06.2024 19:21:58 UTC-05:00 - System: @kevaundray you are a super star!!! thank you so much for your time tonight
21.06.2024 19:22:17 UTC-05:00 - Cayman: yeah very appreciated 😁
21.06.2024 19:23:36 UTC-05:00 - Kev: Glad I could be helpful :)
21.06.2024 19:23:59 UTC-05:00 - Matthew Keil: you are a 🥷
Unknown time - System:
22.06.2024 09:27:20 UTC-05:00 - Cayman: So it looks like the rust bindings already expose msm, but it operates on underlying points rather than PublicKey, Signature
22.06.2024 09:28:02 UTC-05:00 - System: https://docs.rs/blst/0.3.12/blst/struct.p1_affines.html
22.06.2024 09:39:15 UTC-05:00 - System: Their interface is a little annoying, the only way to construct that struct is by passing in jacobian points (which immediately get converted to affine). It's annoying bc in our case, we already have a vec of affine points that we would like to just... initialize as-is without going to and from jacobian. Another case where having a public field in the struct would be helpful...
22.06.2024 09:50:34 UTC-05:00 - System: ah, nvm, I guess slices of affine points implement the MultiPoint trait (and thats where the msm functionality is implemented)
https://docs.rs/blst/0.3.12/blst/trait.MultiPoint.html
22.06.2024 10:13:11 UTC-05:00 - Cayman: I suppose for now, in order to extract the underlying affine points from PublicKey and Signature, we can serialize and deserialize into blst_p1_affine and blst_p2_affine
22.06.2024 10:13:19 UTC-05:00 - System: or do some unsafe thing
22.06.2024 10:13:42 UTC-05:00 - Kev: Yep exactly — we only need the underlying point
22.06.2024 10:14:15 UTC-05:00 - System: I haven’t checked the conversion but it should be okay if the point was previously affine
22.06.2024 10:15:23 UTC-05:00 - System: I think any usage of unsafe would be undefined behaviour in this case because the structure doesn’t have a guaranteed memory layout
22.06.2024 10:15:42 UTC-05:00 - Cayman: ok darn
22.06.2024 10:20:56 UTC-05:00 - Kev: Yeah I think this is the only viable strategy, though it (deserialisation) might be relatively expensive depending on the number of points
22.06.2024 10:22:47 UTC-05:00 - Cayman: I guess its really just a one token "fix" to the underlying bindings that we want: pub
22.06.2024 10:23:21 UTC-05:00 - System: or methods to/from affine point, if the maintainer wanted to keep the field private
22.06.2024 10:25:06 UTC-05:00 - Kev: Yeah a method would probably make more sense (since then they can change the internal representation of public key without causing a breaking change) — they can then possibly add it behind a feature flag if they don’t want it to be on by default
22.06.2024 10:26:50 UTC-05:00 - Cayman: ok great, we'll clean up our PR like that. I think that has a better chance of getting merged
22.06.2024 10:47:02 UTC-05:00 - Kev: Heres what the unsafe method would look like for reference:
#[derive(Debug, Copy, Clone)]
pub struct P1Affine {
x : u64,
y : u64
}
//#[repr(C)]
pub struct PublicKey {
inner : P1Affine
}
impl PublicKey {
pub fn new(x : u64, y : u64) -> Self {
Self {
inner : P1Affine {x, y}
}
}
}
fn main() {
let pub_key = PublicKey::new(3, 2);
let pointer_p1_affine = &pub_key as const PublicKey as const P1Affine;
let p1_affine : P1Affine = unsafe {pointer_p1_affine};
dbg!(p1_affine);
}
mult
on a slice of <code>blst_p1_affine
s rather than converting to projective and going thru their built-in blst::p1_affines</code>
struct
23.06.2024 12:38:45 UTC-05:00 - System: if you're trying to benchmark it
23.06.2024 12:39:56 UTC-05:00 - Kev: Oh yeah true, I think we would need to use the raw pippenger API in that case
23.06.2024 12:42:12 UTC-05:00 - Cayman: should be able to use this implemented trait https://docs.rs/blst/0.3.12/blst/trait.MultiPoint.html#impl-MultiPoint-for-%5Bblst_p1_affine%5D
23.06.2024 12:42:53 UTC-05:00 - Kev: Ah sorry did not see that, will change
23.06.2024 12:47:58 UTC-05:00 - Cayman: I just pushed up similar usage to https://github.com/ChainSafe/blst-ts/pull/149BLS aggregation of 32 pubkeys and signatures 145.0401 ops/s 6894646 ns/op 290 runs
BLS aggregation of 32 pubkeys and signatures - aggregateWithRand 133.8267 ops/s 7472352 ns/op 268 runs
BLS aggregation of 128 pubkeys and signatures 36.30298 ops/s 2.754595e+7 ns/op 73 runs
BLS aggregation of 128 pubkeys and signatures - aggregateWithRan 34.59277 ops/s 2.890777e+7 ns/op 70 runs
BLS aggregation of 256 pubkeys and signatures 18.15399 ops/s 5.508431e+7 ns/op 37 runs
BLS aggregation of 256 pubkeys and signatures - aggregateWithRan 17.31709 ops/s 5.774643e+7 ns/op 35 runs
aggregatePubkeys(pks, false) + aggregateSerializedSignatures(sigs, true)
(without multiplying by randomness)aggregateWithRandomness
using msm
23.06.2024 13:11:17 UTC-05:00 - System: its barely slower (and doesn't have the vulnerability that we currently have, not multing in the randomness)
23.06.2024 13:11:40 UTC-05:00 - System: so thats already quite acceptable
23.06.2024 13:12:44 UTC-05:00 - System: if we could avoid serialization/deserialization it looks like it would still be a hair slower, so its not the end of the world if we can't upstream our to/from affine fns
23.06.2024 13:13:11 UTC-05:00 - Kev: Hmm it seems that both methods are roughly the same speed, which is suprising
23.06.2024 13:14:08 UTC-05:00 - Cayman: yeah, the first method is just using min_pk::AggregatePublicKey::aggregate(...).to_public_key()
and similar for signature
23.06.2024 13:15:12 UTC-05:00 - Kev: Hmm maybe I'm missing something, once you start aggregating more than 5/6 things the msm algorithm should be strictly faster
23.06.2024 13:45:19 UTC-05:00 - Cayman: I wonder if its slower because its now mult-ing in randomness. Can try benchmarking exact apples to apples, using AggregatePublicKey::aggregate() vs MultiPoint::add
23.06.2024 13:51:04 UTC-05:00 - System: BLS aggregation of 32 pubkeys and signatures 133.1341 ops/s 7511225 ns/op 267 runs
BLS aggregation of 32 pubkeys and signatures - aggregateWithMsm 131.3886 ops/s 7611011 ns/op 263 runs
BLS aggregation of 128 pubkeys and signatures 33.49414 ops/s 2.985597e+7 ns/op 67 runs
BLS aggregation of 128 pubkeys and signatures - aggregateWithMsm 33.17804 ops/s 3.014042e+7 ns/op 67 runs
BLS aggregation of 256 pubkeys and signatures 16.74945 ops/s 5.970346e+7 ns/op 34 runs
BLS aggregation of 256 pubkeys and signatures - aggregateWithMsm 16.61108 ops/s 6.020078e+7 ns/op 34 runs
verifiy_multiple_aggregate_signatures
over groups of those for verification ({msg, pk, sig}[]
). interface SignatureSet {
message: Uint8Array;
publicKey: PublicKey;
signature: Signature;
}
const signatureSets: SignatureSet = [
// many that came across the wire and were bundled in a batch for verification
// for this example assume all sets have the same message (vote for head)
];
// Option 1 - what we were originally doing
let isValid = verifyMultipleAggregateSignatures(signatureSets);
// Option 2 - the novel idea
const aggPk = aggregatePublicKey(signatureSets.map(set => set.publicKey));
const aggSig = aggregateSignatures(signatureSets.map(set => set.signature));
isValid = verify(signatureSets[0], aggPk, aggSig); // all the messages are the same so can pick any index
function getNonZeroBytes(length: number) {
const rand = crypto.randomBytes(length);
for (let i = 0; i < rand.length(); i++) {
if (rand[i] !== 0) return rand;
}
rand[0] = 0x99
return rand;
}
const SAME_RANDOM_BYTES_AS_MULT_VER = 8;
const rands = Array.from({length: signatureSets.length()}, () => {
return getNonZeroBytes(SAME_RANDOM_BYTES_AS_MULT_VER));
});
const aggPkWithRandomness = aggregatePublicKeys(
signatureSets.map(({publicKey}, i) => publicKey.multiplyBy(rands[i])
);
const aggSigWithRandomness = aggregateSignatures(
signatureSets.map(({signature}, i) => signature.multiplyBy(rands[i])
);
const isValid = verify(msg, aggPkWithRandomness, aggSigWithRandomness);
blst.hpp
. We found multithreaded performance is better using napi-rs
and rust so we are reimplementing the node bindings by wrapping the bindings/rust
from here.24.06.2024 08:10:11 UTC-05:00 - System: If you increase the num_signature set, the latency grows so I'm asking Michael for what is a realistic number of signature sets.
verify_signature_set/old -- num signatures 32, sig_sets 1
time: [1.0899 ms 1.1332 ms 1.1981 ms]
verify_signature_set/new -- num signatures 32, sig_sets 1
time: [1.0639 ms 1.0896 ms 1.1293 ms]
verify_signature_set/old -- num signatures 64, sig_sets 1
time: [1.0686 ms 1.0916 ms 1.1243 ms]
verify_signature_set/new -- num signatures 64, sig_sets 1
time: [1.0614 ms 1.0855 ms 1.1220 ms]
verify_signature_set/old -- num signatures 128, sig_sets 1
time: [1.0921 ms 1.1118 ms 1.1419 ms]
verify_signature_set/new -- num signatures 128, sig_sets 1
time: [1.0756 ms 1.0984 ms 1.1328 ms]
verify_signature_set/old -- num signatures 512, sig_sets 1
time: [1.2726 ms 1.2920 ms 1.3165 ms]
verify_signature_set/new -- num signatures 512, sig_sets 1
time: [1.0992 ms 1.1271 ms 1.1630 ms]
verify_signature_set/old -- num signatures 16384, sig_sets 1
time: [8.2746 ms 8.3283 ms 8.3918 ms]
verify_signature_set/new -- num signatures 16384, sig_sets 1
time: [1.4527 ms 1.4808 ms 1.5158 ms]
verify_signature_set/old -- num signatures 32768, sig_sets 1
time: [8.2644 ms 8.3192 ms 8.3848 ms]
verify_signature_set/new -- num signatures 32768, sig_sets 1
time: [1.4652 ms 1.5084 ms 1.5663 ms]
.add
looks like it should be slower than .mult
, especially for larger input since its just doing naive addition just with a threadpool now
24.06.2024 11:45:31 UTC-05:00 - Cayman: aww ok
24.06.2024 11:46:01 UTC-05:00 - Kev: Its still faster, but for larger inputs it seems to start regressing from .mult -- I'm not confident that I understood the implementation of .add, so I could be missing something
24.06.2024 12:12:51 UTC-05:00 - Cayman: if i'm reading it right, its using blst_p1s_add
and blst_p2s_add
under the hood, which appears to be implemented here https://github.com/supranational/blst/blob/master/src/bulk_addition.c#L11 in a macro w/ the function starting on L145
24.06.2024 12:14:37 UTC-05:00 - System: hoping you'd be able to tell if that should be faster or slower than pippenger
24.06.2024 12:17:07 UTC-05:00 - Kev: That bulk addition method should be faster since .mult essentially decomposes into a bunch of bulk_addition
24.06.2024 12:17:42 UTC-05:00 - System: So I think you are right in that we should use that instead, though its unclear to me as to why its slower
24.06.2024 13:54:44 UTC-05:00 - Cayman: he responded again
24.06.2024 13:55:10 UTC-05:00 - System: I dug up a comment from one of our chatsHey! We found a potential security vulnerability on Lodestar's optimization that batch verifies gossip attestations with the same message.24.06.2024 13:55:45 UTC-05:00 - System: so this is what we're vulnerable to when we aggregate sigs and pks per unique message before verifying 24.06.2024 13:56:12 UTC-05:00 - System: and I suppose what the random numbers are supposed to be preventing 24.06.2024 13:59:17 UTC-05:00 - Kev: Yep this makes sense, essentially if you want to verify something like:
This optimization allows an attacker to force a node running this optimization to forward invalid data.
Consider two honest attesters publishing unaggregated attestation signatures $S1$, $S2$. An attacker is connected to the victim node, where the victim implements this optimization. The attacker mutates the signatures such that $S1' = S1 + Δ$, $S2' = S2 - Δ$ and forwards them to the victim. The check $e(G, S1' + S2') =? e(P1 + P2, Hm)$ will pass. If the victim strictly uses aggregated pubkey from now on it will never attempt to include invalid data on-chain in other gossip topics. However, it will forward attestations with invalid signatures and will quickly be banned by all other peers. As such an attacker can isolate the victim node forcing it to connect exclusively to other nodes implementing the optimization and attackers.
-P1 - P2 - P3
. which means I need to know the secret key for P1, P2 and P3
24.06.2024 14:06:35 UTC-05:00 - System: I guess you would be a malicious validator here
24.06.2024 14:06:42 UTC-05:00 - Matthew Keil: Like the keys so not come across the wire. They are pulled from state according to validator index when an attestation is received
24.06.2024 14:07:09 UTC-05:00 - Kev: Yeah I think this is before that, ie how do the keys get "registered"
24.06.2024 14:07:36 UTC-05:00 - System: To register a validator key into the system, you would need to show that you own it by commonly signing over some message
24.06.2024 14:07:39 UTC-05:00 - Matthew Keil: Through the validator contact where a message is signed by the owner
24.06.2024 14:07:44 UTC-05:00 - System: Exactly
24.06.2024 14:07:51 UTC-05:00 - System: And then it gets added to state
24.06.2024 14:07:54 UTC-05:00 - Kev: Yeah thats the proof of possession part
24.06.2024 14:08:26 UTC-05:00 - System: If you allow me to add any validator key, I could cancel out the other keys and the aggregated key would just be the infinity point
24.06.2024 14:09:18 UTC-05:00 - Matthew Keil: Now I think I'm confused. Wouldn't that be proof enough. Like we get a validator index. We look up the index and get the corresponding key and then use the key we found, that was proven through signature before being added, to verify messages for that validator
24.06.2024 14:09:41 UTC-05:00 - Kev: This is assuming aggregation is done by doing P1 + P2 + P3A
and then a dishonest validator registers a public key -A
just after.-A
which would imply they know the secret key for A
since its just a negative sign difference.
24.06.2024 14:29:55 UTC-05:00 - Matthew Keil: Oooo... But knowing -A
could only make an aggregation of these two keys infinity not P4 in the message I responded to right?
24.06.2024 14:30:31 UTC-05:00 - System: Like the attacker would need to know all the Ps to make P4
24.06.2024 14:30:40 UTC-05:00 - Kev: Yep exactly!
24.06.2024 14:31:19 UTC-05:00 - System: The only way for an attacker to reasonably do that with proof of possession, is to own all of the committee
24.06.2024 14:31:32 UTC-05:00 - Matthew Keil: I need to finish packing but I'll be back in a bit
24.06.2024 14:31:50 UTC-05:00 - System: Which for Coinbase is possible
24.06.2024 14:31:52 UTC-05:00 - Kev: Heading to the gym, so may be offline for a while
24.06.2024 16:19:14 UTC-05:00 - Cayman: ok this makes sense intuitively... But does this still work in the pairing world?verify
/fast_aggregate_verify
checks e(S, G1) = e(P, Hm)
e(r S, G1) = e(r P, Hm)
x
then you should know 2 + x its just adding two to it or 2x or in our case H(m) x
24.06.2024 16:37:55 UTC-05:00 - System: So the equation we are checking is that [x
H(m)] 1 = [x
1] H(m)x
H(m) using G1 and then call this the signature S
1
using G2, in your example you called in it G1x
1 using G1 and this is generally called my public/signing key. In your example, you called it P
r
it's just r (a b) c = r a (b c) -- You can put the r with any of those terms and the equation would still hold true
24.06.2024 16:44:23 UTC-05:00 - System: Not sure if looking at it from an equation perspective makes it easier to reason about -- thats the way it makes sense to me though 😄Hm
24.06.2024 16:50:31 UTC-05:00 - Cayman: ok sweet, that clears that up
24.06.2024 16:51:50 UTC-05:00 - Kev: Oh for the napi-rs stuff, are you folks publishing a package and then using it in lodestar or is it built locally and pulled into lodestar?
24.06.2024 16:52:13 UTC-05:00 - Cayman: we're planning on publishing to npm under code>@chainsafe/blst</code
24.06.2024 16:52:14 UTC-05:00 - Matthew Keil: publishing
24.06.2024 16:53:17 UTC-05:00 - Cayman: just wanting to get this last aggregate with randomness function squared away, do some polishing and attach the existing test harness
24.06.2024 16:53:23 UTC-05:00 - System: then we should be ready to publish
24.06.2024 16:54:07 UTC-05:00 - Kev: Ah okay sweet, I'll check out your github action for publishing -- saw the default napi-rs one and didn't really like the use of qemu for cross-compilation
24.06.2024 16:55:21 UTC-05:00 - Cayman: hah yea some of their publishing stuff could be improved
24.06.2024 16:55:28 UTC-05:00 - Kev: Oh I guess you would be blocked by the blst PR then -- since you'd at least want the inner point to be exposed
24.06.2024 16:55:57 UTC-05:00 - Cayman: if we can't get it upstreamed, we can drop the blst repo into ours as a git submodule or something
24.06.2024 16:56:19 UTC-05:00 - System: definitely more annoying, having to maintain the fork, but an option
24.06.2024 16:57:20 UTC-05:00 - System: or we can do the unsafe thing, and rely on our prebuilt versions having the right behavior
24.06.2024 16:57:40 UTC-05:00 - Kev: That makes sense -- I guess signature verification is that much of a bottleneck
24.06.2024 16:57:54 UTC-05:00 - System: That would definitely keep me up at night 😅
24.06.2024 16:58:17 UTC-05:00 - Cayman: 😂 "whats the worst that could happen"
24.06.2024 16:58:24 UTC-05:00 - System: yea i guess not
24.06.2024 16:58:39 UTC-05:00 - Matthew Keil: is how its built now
24.06.2024 16:59:00 UTC-05:00 - Cayman: yeah as it stands, we're vulnerable to that attack, we were hoping to rebuild the bindings and fix the vulnerability in one swoop
24.06.2024 16:59:18 UTC-05:00 - Matthew Keil: its 35% of main thread time
24.06.2024 16:59:33 UTC-05:00 - System: and the attack surface as well
24.06.2024 17:05:54 UTC-05:00 - Cayman: can we make our randomness optimization better by adding randomness to the message instead of public keys? even tho the msg is in G2, that would be a single multiplication vs one mult per pubkey
24.06.2024 17:07:08 UTC-05:00 - System: not sure how to mult a list of scalars with single message tho
24.06.2024 17:07:32 UTC-05:00 - System: and the existing interfaces don't seem very helpful for doing this
24.06.2024 17:08:27 UTC-05:00 - Kev: Oh hmm -- I don't see why not -- though I'd need to double check to see if there might be an attack
24.06.2024 17:09:39 UTC-05:00 - System: I think you would do the multiplications separately, so combine the list of scalars and then do a scalar multiplication. Though you would need to use blst_fr instead of blst_scalar for that
24.06.2024 17:11:50 UTC-05:00 - System: It seems that it was also discussed on the original post: https://ethresear.ch/t/fast-verification-of-multiple-bls-signatures/5407/8
24.06.2024 17:12:10 UTC-05:00 - Cayman: yup, and i bet this is part of the confusion with dot-asm
24.06.2024 17:13:01 UTC-05:00 - System: he's wondering why we'd mult in randomness to pks when everyone just mults in randomness to msgs
24.06.2024 17:13:34 UTC-05:00 - System: ok so you'd sum the rand blst_frs and scalar mult the result with the message?
24.06.2024 17:14:24 UTC-05:00 - System: that sounds faster than msming the rands with pks
24.06.2024 17:15:15 UTC-05:00 - Kev: I'm not entirely sure why kirk and vitalik on that post said aggregating the public keys is faster than multiplying by the message
24.06.2024 17:15:51 UTC-05:00 - System: Yep, if you're doing r1 Hm + r2 Hm, we can factor out the Hm and do (r1 + r2)*Hm
24.06.2024 17:22:28 UTC-05:00 - Cayman: ah, i guess multiplying the message makes sense when there are more pubkeys than unique msgs, where you'd have to do more mults when multiplying the pubkeys (as we're seeing now)
24.06.2024 17:23:15 UTC-05:00 - System: and aggregating the public keys first vs doing separate verifications seems to just be fast_aggregate_verify
24.06.2024 17:24:48 UTC-05:00 - System: it seems they're kinda glossing over interweaving fast_aggregate_verify into this system
24.06.2024 17:25:30 UTC-05:00 - System: his construction starts with "Suppose that you received n signatures, S1 … Sn, where each signature Si is itself an aggregate signature"
24.06.2024 17:26:10 UTC-05:00 - Kev: Yeah that post is for the case where you've already aggregated all signatures+pubkeys with the same message
24.06.2024 17:26:42 UTC-05:00 - Cayman: we're extending this by starting with unaggregated signatures, and instead of a vec of random scalars, its a vec of vecs of random scalars
24.06.2024 17:27:32 UTC-05:00 - System: where the vecs of random scalars get multed into signatures when they get aggregated, and then the vecs of random scalars get summed before being multed into messages
24.06.2024 17:29:21 UTC-05:00 - System: but we wanted to do this in two steps because this construction is just a pass/fail, and if you fail, you have to re-verify everything individually. if we do it in two steps, we can "save" the signature and pubkey aggregation per unique message, rather than having to re-aggregate
24.06.2024 17:29:40 UTC-05:00 - Kev: right that makes sense
24.06.2024 17:29:48 UTC-05:00 - System: Why individually?
24.06.2024 17:30:28 UTC-05:00 - System: Or maybe its not possible to just halve the set and do two smaller aggregations, like a binary search
24.06.2024 17:30:59 UTC-05:00 - Cayman: ah, right.. we just fall back to naive individual verification
24.06.2024 17:35:21 UTC-05:00 - System: actually, i don't think the fact that it's in two steps matters to us at all
24.06.2024 17:36:08 UTC-05:00 - System: we're just falling back to naive individual verification, we're not using any of our saved aggregated sigs/pubkeys
24.06.2024 18:59:02 UTC-05:00 - Cayman: ok i have something compiling that first mults in randomness into unaggregated signatures, then sums the randoms and uses them at a higher level to do batch verification
24.06.2024 18:59:03 UTC-05:00 - System: https://github.com/ChainSafe/blst-ts/pull/149/commits/f0022349debc7f0d3c1587951b0e2d03dc8444e6
24.06.2024 18:59:14 UTC-05:00 - System: its not pretty rust, but i guess it proves the point
24.06.2024 19:00:33 UTC-05:00 - System: tired for today, but tomorrow i'll benchmark that against multing randoms into pks during aggregation, then doing batch verification
24.06.2024 19:56:04 UTC-05:00 - Cayman: this actually doesn't work bc the randomness will be applied to the signatures twice...
24.06.2024 19:56:15 UTC-05:00 - System: so it seems we can't reuse verify_multiple_aggregate_signatures
Unknown time - System:
25.06.2024 08:29:08 UTC-05:00 - Cayman: he responded again :) https://github.com/supranational/blst/pull/225#issuecomment-2188746319
25.06.2024 08:31:37 UTC-05:00 - Matthew Keil: He is in it to win it! Your last comment was epic btw
25.06.2024 08:37:28 UTC-05:00 - Kev: Something that I don't understand yet, is whether clients receive all signatures they need to verify and then they get aggregated in this two step process, or if clients receive signatures for the same message (then tentatively aggregate those and verify)
Problem description
Right now when we validate bls signatures of the same message, we aggregate signatures and public keys, if the aggregated signature is verified we consider all signatures are verified. But there's a chance 2 signatures are invalid and final verification result is still
true
.Solution description
Add randomizing factors following this approach https://ethresear.ch/t/fast-verification-of-multiple-bls-signatures/5407
quick benchmark:
shows this is 2x slower than the current approach but it's still ~2.65x faster than
verifyMultipleSignatures
Additional context
Given signature aggregation already took 25% of a mainnet node, it's not sustainable to still do this in main thread, we should offload the work to worker/libuv instead
Also right now we queue attestation messages in
IndexedGossipQueue
and bls queue, we should only queue inIndexedGossipQueue
. For bls, we need to write new worker api (or libuv implementation) and verify same message signatures immediately without queueing.cc @matthewkeil