-
Notifications
You must be signed in to change notification settings - Fork 58
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
BLS extensions #274
BLS extensions #274
Conversation
One comment I want to make is in a long sequence of operations, it's a lot cheaper to keep numbers in projective form and defer normalizing to affine form until the end. Projective form is analogous to a fraction This inversion is expensive, and deferring it to the end to provide a canonical affine representation can greatly reduce the number of inversions needed over a series of operations. So it might be useful to perform all operations in the projective space, and automatically turn affine coordinates into projective (by setting Z = 1), and require an affine normalization at the end before any comparison or whatever is done. It might even be possible to stay in projective coordinates the whole way through. |
f26390d
to
3ad371c
Compare
e4fb5c6
to
495b417
Compare
@richardkiss I think the way we should solve the issue you raise is by making the Allocator interface support more kinds of atoms natively (not just bytes). That way we can at least convert BigNum, G1 and G2 points lazily into bytes (and hopefully not at all). This strategy will likely make it a lot cheaper, but it also means the CLVM cost won't accurately represent the true compute cost anymore. |
As for naming, I wonder if the |
And maybe we could call |
op-tests/test-blspy-g1.txt
Outdated
@@ -0,0 +1,120 @@ | |||
; This file was generated by tools/generate-bls-tests.py | |||
|
|||
bls_g1_add 0xb756ecad886b575d2c95014c20bcd9b52f1f72e3b5fbc657bacf1e72be707f9aaf76a9156a1628b99d873036beecec10 0xa73e3a434a117bed5c0eab24dce24b2e03bc75254b878900eb01f98cf389329cdd7d83cd8033475ff1ec6b2e10abbb09 => 0x932e81cd868b85b0608705ee603f15e9aa087811dae86254e5496008debed51377bc5c19a6ef9db547e3ac3fd7046bdf | 2789534 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need bls_g1_add
with no arguments (to check the identity).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess we also want some tests with one argument.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we have this test case (for point_add
) here: https://github.com/Chia-Network/clvm_rs/blob/main/test-more-ops.txt#L711
Generally I've split the test cases between test-bls-ops.txt
for manually added test cases and the test-blspy-*.txt
for the generated ones. I suppose most of the G1 and G2 points can't be validated by manual inspection, and should be generated.
op-tests/test-blspy-g1.txt
Outdated
bls_g1_add 0xb811345fe6b8f59414c486179782a5ccc4ab39e48e9b57583df5c52da17a593cd22d94719fd1ce5feef24c1482b4bd16 0x8f3eb0208a3f0808df538ebc1f582d01c62baabe12842171f7d6c01f9b31f7c9d386190d9349d53378998d0162c7ecb2 => 0x9673643ce42edcccd383d87f9da4deb11704e81d0ca10bda31505b69d1150b6612fc5658746182d18042634744375495 | 2789534 | ||
bls_g1_add 0x9673643ce42edcccd383d87f9da4deb11704e81d0ca10bda31505b69d1150b6612fc5658746182d18042634744375495 0x8b202593319bce41b090f3309986de59861ab1e2ff32aef871d83f9aac232c7253c01f1f649c6f69879c441286319de4 => 0xb258bb9e5acfb96360f58395d76cab3327e8f4462ce4fad2a516efc6e9d625d9412e1f12064c986b3ab27f18dbeaa4e6 | 2789534 | ||
bls_g1_add 0xb258bb9e5acfb96360f58395d76cab3327e8f4462ce4fad2a516efc6e9d625d9412e1f12064c986b3ab27f18dbeaa4e6 0xae9e0aed7144528e7629157798a88790d0941ecbf2d267eaf2f5abfc21a9438b415355afb7f76963def089ec1fd108a4 => 0xa83f432d0effff104489db859fa24786793c5626fbcf69cf32b3e6fbf24f7995cb0d620009a9745bdae4a3960089b2e9 | 2789534 | ||
bls_g1_subtract 0xb756ecad886b575d2c95014c20bcd9b52f1f72e3b5fbc657bacf1e72be707f9aaf76a9156a1628b99d873036beecec10 0xa73e3a434a117bed5c0eab24dce24b2e03bc75254b878900eb01f98cf389329cdd7d83cd8033475ff1ec6b2e10abbb09 => 0x94fa83a873e3549b553fbca0024a81c109a1a499af990edd669e4dd1707a1faccbabbc5584e7ed47f10eb57ad48552bb | 2857918 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, we want a test 0 arguments and tests with 1 argument.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, I'll add this to test-bls-ops.txt
op-tests/test-blspy-g1.txt
Outdated
bls_g1_subtract 0x8a40ed8849cf0863c0215378ac2830fa1cee5019bc229970fc981a985b0b98f2d6a68c41c77e5132035c70557cdfb82c 0x8f3eb0208a3f0808df538ebc1f582d01c62baabe12842171f7d6c01f9b31f7c9d386190d9349d53378998d0162c7ecb2 => 0x958ce163da9553aa378bb4978610c773858e2dc4787f634f840cbe4105980567d4295d75cc7e93314536b487ade942e5 | 2857918 | ||
bls_g1_subtract 0x958ce163da9553aa378bb4978610c773858e2dc4787f634f840cbe4105980567d4295d75cc7e93314536b487ade942e5 0x8b202593319bce41b090f3309986de59861ab1e2ff32aef871d83f9aac232c7253c01f1f649c6f69879c441286319de4 => 0x8194c717fec442f11a05fc0ecdd81c6fcb6de2c5e37e4290d46fea614ad4e1eb27bb4433fe666bd19749a6e4582293ac | 2857918 | ||
bls_g1_subtract 0x8194c717fec442f11a05fc0ecdd81c6fcb6de2c5e37e4290d46fea614ad4e1eb27bb4433fe666bd19749a6e4582293ac 0xae9e0aed7144528e7629157798a88790d0941ecbf2d267eaf2f5abfc21a9438b415355afb7f76963def089ec1fd108a4 => 0xa3ba50c108402a11c27ca3e0b528aa204e1a4ae390beec1f405fa2d36fbb13dfade5d35527ebcc0030a14a2edc75c918 | 2857918 | ||
bls_g1_multiply 0xb756ecad886b575d2c95014c20bcd9b52f1f72e3b5fbc657bacf1e72be707f9aaf76a9156a1628b99d873036beecec10 -56 => 0x9303d1db4e1f65b6b253ac6de90068c0d5f9348234ec518ebaf24542bcddba3d0581d28649085224dec05ddcc27a8c99 | 2154839 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We want to test multiplying by 0
, and multiplying by large scalars (at least 50% of the size of the order of the group, so bit 253 set). I believe multiplying by large scalars could use significantly more CPU. Well, actually probably not since I expect relic is trying to protect against side channel leak.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll add the zero case to test-bls-ops.txt
and I'll add multiplying with large scalars to the generated test cases
op-tests/test-blspy-g1.txt
Outdated
bls_g1_multiply 0x8f3eb0208a3f0808df538ebc1f582d01c62baabe12842171f7d6c01f9b31f7c9d386190d9349d53378998d0162c7ecb2 93 => 0x887004d9a617003c6d62a29bfc25fa03299ca723db860d72ed502dbd6ce706df0c98f0365a7b6c8d36b446e65fca7707 | 2154839 | ||
bls_g1_multiply 0x8b202593319bce41b090f3309986de59861ab1e2ff32aef871d83f9aac232c7253c01f1f649c6f69879c441286319de4 -22 => 0xa68e15cafe9d77c4ab64d0d5741efb33488ac097fd590db1cfc02e5972a601077651a1ad872e860535e8b6a74c8f564e | 2154839 | ||
bls_g1_multiply 0xae9e0aed7144528e7629157798a88790d0941ecbf2d267eaf2f5abfc21a9438b415355afb7f76963def089ec1fd108a4 -74 => 0xb87fe1405ab0806df2283784d848aa554870045738c7c4fa38d51bb6ff284103f8e8f01b00f2eaa18984c1b317722733 | 2154839 | ||
bls_g1_negate 0xb756ecad886b575d2c95014c20bcd9b52f1f72e3b5fbc657bacf1e72be707f9aaf76a9156a1628b99d873036beecec10 => 0x9756ecad886b575d2c95014c20bcd9b52f1f72e3b5fbc657bacf1e72be707f9aaf76a9156a1628b99d873036beecec10 | 471259 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems like it's way too expensive when you consider it's just copying the value and changing a bit. There's not much point in making it cost more than logxor
with a constant.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This operator also validates that the point is on the curve, which isn't trivial. I could implement this as just a bit operation, but it would make it accept invalid points, I believe.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we're concerned about invalid (ie. untrusted) points, we could have a separate operator to validate them. Or just do (g1_add untrusted_point)
, which should also do an implicit validation. (Just spitballing here.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's not likely that we're going to take a point and negate it and not perform any other operations on it that would validate it, so not validating here may not be a big concern.
op-tests/test-blspy-g2.txt
Outdated
@@ -0,0 +1,120 @@ | |||
; This file was generated by tools/generate-bls-tests.py | |||
|
|||
bls_g2_add 0x96bbcfbca241004ba03968b7433664c1b944245828af7a1ebda771495c90d4816508d65db40b601bce51a3689c251f731699411b56e2172dd82eb96f571a691ba2984f1e4e39a05646ceb9c82d1108e78fe3fdfd94cff4fb447efe498dcecc25 0xa35068bd876cd804a57e9695ad9585271b20f2a67c0f70a05ec47c597a6c3754083d1a974b6bf9a90c1e4e5208b73d7a120ce4190f328a3b90a15c2c7a7973a5894a8e16a63c36adb4e4995d165192d9b8c26fc15a84655c90a92b846f7952fa => 0x9666a3ff8474a9c084fe71d8ad1107f6e89e1bf9fbff0d6e0f20507e79ef4e35783fca3fedbb8f0d0e7f4434b5df37631170fe5be0096fdc72183b2de89aed593060bbc8700df230329afdb9020333007955c4e405a5776f6df81cb39ca55af4 | 11135562 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test with 0 and 1 parameter.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the case of no arguments is covered here: 83ff075#diff-8e926fa1adc34a468edbbfcb76a3996682c288dc76919bad5a6876cbe68fde84R66
I'll add the single argument case next to it
op-tests/test-blspy-g2.txt
Outdated
bls_g2_add 0x8c4441872d3b5ea5d69a82829d910263eba8e4ef5bb0e1cd475b6ebb0267b885c3ae73ec840fb5234d669aa8df5369a2018c55abc032796bc847fb806db92508872a5e3d225427d6bccfdac274297bbea554edd6c3ca13d758d9e79cf293d113 0x84250bba4dc02a41dfbb4913386abb67b2b03823a246aa8d7aab78e547469552746cda9e9da25fc867fe0109219b5b1618d5941e1298231220bf93267ebe984fc70d1969afdf0bdf61886c325d316b77dc4c3516fb02c7827db5f819218c6ab2 => 0x95857a3a5cc48db0557d5ae6465fe003116c685bab95c059ab0d91667b0442a3a451ffa10698e571988d1cffe692941015815e4b0f20bf1cbf3c1c298a0c24c2d521ff3892e1232bf237a5f30fcc4d05889c77de21f0ffb577d8afbfec38481e | 11135562 | ||
bls_g2_add 0x95857a3a5cc48db0557d5ae6465fe003116c685bab95c059ab0d91667b0442a3a451ffa10698e571988d1cffe692941015815e4b0f20bf1cbf3c1c298a0c24c2d521ff3892e1232bf237a5f30fcc4d05889c77de21f0ffb577d8afbfec38481e 0xaab6a407228638c872bd60e4112a5b4ae1ee15ff5123ae6a762dc97cd2184dc0cf6aee27db0221454c662138c93d369e094cf1ecf824f2c1fa374dd302c33f62d4497cc6cf04c6fcf0f4da811f2f72dec379629a1b370eb746db0b9447ecf74b => 0x9661ca919fbbf0022cf493691a0fc5d14e3ce66c5ffbbd3e20b1d8f5f9e96984fb54777d8ef5f3199b8def4e58423e9c1933930cfacee37267b7c06e049eb8df55ad86ba03b9bdb6d7ff30e8259a65f6f318e47ef24578dd5d514c4554935cad | 11135562 | ||
bls_g2_add 0x9661ca919fbbf0022cf493691a0fc5d14e3ce66c5ffbbd3e20b1d8f5f9e96984fb54777d8ef5f3199b8def4e58423e9c1933930cfacee37267b7c06e049eb8df55ad86ba03b9bdb6d7ff30e8259a65f6f318e47ef24578dd5d514c4554935cad 0x8e112b9b019c4c83475ad272f96496e85497a689b14bd8fd031a6fb2247217a501d14152d68a17bfc8860baec4cc126013641b9626dacd761aa54cf11c87542433f855a735ca790ae8f154db815b99b0c155065b5169729f522c0698c9f29cd0 => 0x85e3bdc96b539f17a952e185b0868bea9548c176e481e8a37dd33789af0c7c9599300c5fe991ca9f9e76f0c6f2c3367e17c0eb4d094c73d2488d9b15eb989c6eb31c8b94cd4334aa7dafcd968e4d0fbd6e80efd791c9a89e5815a40ae681852c | 11135562 | ||
bls_g2_subtract 0x96bbcfbca241004ba03968b7433664c1b944245828af7a1ebda771495c90d4816508d65db40b601bce51a3689c251f731699411b56e2172dd82eb96f571a691ba2984f1e4e39a05646ceb9c82d1108e78fe3fdfd94cff4fb447efe498dcecc25 0xa35068bd876cd804a57e9695ad9585271b20f2a67c0f70a05ec47c597a6c3754083d1a974b6bf9a90c1e4e5208b73d7a120ce4190f328a3b90a15c2c7a7973a5894a8e16a63c36adb4e4995d165192d9b8c26fc15a84655c90a92b846f7952fa => 0xb681d21f5b5f31c5dc5a7983992c3be169cbd684a84f4d1ffe0c0c21923f7a023bc57ed4d3c445a3c9709203104e16b5091abec762e3ebc5edc5e7f170a2be4f71e868dc25b90b4c9dd9952edaaff163a177acf977f36e5d66f485fd94962428 | 11137794 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0 & 1 parameter
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And more than 2 parameters.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the no argument case is covered here: 83ff075#diff-8e926fa1adc34a468edbbfcb76a3996682c288dc76919bad5a6876cbe68fde84R75
I'll add the single argument case next to it
op-tests/test-blspy-g2.txt
Outdated
bls_g2_subtract 0xa3b4111e01594b6f97d09718e6886940be81697f7e0377546846e14d7298cf201a0f684d7a38da7fbc359ce9ce3b28360043aa7bed503a78119d36638f272c8c6f8979663e3e3ff30889750b37991ae24b0b2d89317bcdfae4b84e71688f6b3c 0x84250bba4dc02a41dfbb4913386abb67b2b03823a246aa8d7aab78e547469552746cda9e9da25fc867fe0109219b5b1618d5941e1298231220bf93267ebe984fc70d1969afdf0bdf61886c325d316b77dc4c3516fb02c7827db5f819218c6ab2 => 0x8cf8a6f1b0e2ce982c444f1e1868ce1b6ef7e35eaa233022a593315bd3db29b7224cea5e3028dd457c672063819127be11a7d72a1aa797f2d851a79a8b80f9b496130ea83919c6894e58408795a96bf2b93b6dde20214e0de4166dc065dd4dad | 11137794 | ||
bls_g2_subtract 0x8cf8a6f1b0e2ce982c444f1e1868ce1b6ef7e35eaa233022a593315bd3db29b7224cea5e3028dd457c672063819127be11a7d72a1aa797f2d851a79a8b80f9b496130ea83919c6894e58408795a96bf2b93b6dde20214e0de4166dc065dd4dad 0xaab6a407228638c872bd60e4112a5b4ae1ee15ff5123ae6a762dc97cd2184dc0cf6aee27db0221454c662138c93d369e094cf1ecf824f2c1fa374dd302c33f62d4497cc6cf04c6fcf0f4da811f2f72dec379629a1b370eb746db0b9447ecf74b => 0x80edc25c019da2edda1e30b05b0b9a9a405f5267aeabd1652d66f25397605664913f1895d19386f0730da5479a2c033d044fb5d6c4ccc259c10747528b29e57e9ef0ccf7139c3c27ba5a7718dc68ffd74e6fff5bb8c3193143f6eddc53c20d92 | 11137794 | ||
bls_g2_subtract 0x80edc25c019da2edda1e30b05b0b9a9a405f5267aeabd1652d66f25397605664913f1895d19386f0730da5479a2c033d044fb5d6c4ccc259c10747528b29e57e9ef0ccf7139c3c27ba5a7718dc68ffd74e6fff5bb8c3193143f6eddc53c20d92 0x8e112b9b019c4c83475ad272f96496e85497a689b14bd8fd031a6fb2247217a501d14152d68a17bfc8860baec4cc126013641b9626dacd761aa54cf11c87542433f855a735ca790ae8f154db815b99b0c155065b5169729f522c0698c9f29cd0 => 0x90bea7b7d2d4a969911467ad7199503988e5bd7641b5c6c9129b064ec35c2fda2ffea220f34f6a25cab23e8f5e4ed52b03c6e795949a059d0d9d3106717033a29058662931080a6ce9e9416552968e367e0e846b93433b7d5df827c51253ca11 | 11137794 | ||
bls_g2_multiply 0x96bbcfbca241004ba03968b7433664c1b944245828af7a1ebda771495c90d4816508d65db40b601bce51a3689c251f731699411b56e2172dd82eb96f571a691ba2984f1e4e39a05646ceb9c82d1108e78fe3fdfd94cff4fb447efe498dcecc25 82 => 0xafc1f4f785f8b2122e5e3a001405bb21e13c7292a84e3742448bf614bc3f129a117fbc14c4bbcc5f9e3ed06d6fd867c802a0fe0cd20aab3166985b0659874e61cfb8fb0120c517db30e4bf28dd627b5e2217ba59c4a7447431b626169fa57879 | 10079117 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add tests for 0 and large scalars.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll add the case of multiplying with 0 to test-bls-ops.txt
and large scalars to these generated test cases
op-tests/test-blspy-g2.txt
Outdated
bls_g2_multiply 0x84250bba4dc02a41dfbb4913386abb67b2b03823a246aa8d7aab78e547469552746cda9e9da25fc867fe0109219b5b1618d5941e1298231220bf93267ebe984fc70d1969afdf0bdf61886c325d316b77dc4c3516fb02c7827db5f819218c6ab2 2 => 0xb4cbc49624252576796118c0ca180b5e2be36055f063321930f4288151f1da227d5b6f2b3b672f964b4192f38c7dc49d0a8f39df18d68311ecd2a8463f07c5c35a10097de6dcced7683b751f0faea25aa7827cd30dafa0775db4ea84f0254647 | 10079117 | ||
bls_g2_multiply 0xaab6a407228638c872bd60e4112a5b4ae1ee15ff5123ae6a762dc97cd2184dc0cf6aee27db0221454c662138c93d369e094cf1ecf824f2c1fa374dd302c33f62d4497cc6cf04c6fcf0f4da811f2f72dec379629a1b370eb746db0b9447ecf74b -100 => 0xad0610c976a123f231267c8fdd25a7f3f44fa6bf0c790dc79b85e091d6210208f8e489a51cd76e84f7d2567f8f81f51008c60ca6439edd88dedf412a2bda1848df6e5db9bc4fb211b6c8e9c37d5fa3311aa6c8efc73b6f7496b201854683b1e0 | 10079117 | ||
bls_g2_multiply 0x8e112b9b019c4c83475ad272f96496e85497a689b14bd8fd031a6fb2247217a501d14152d68a17bfc8860baec4cc126013641b9626dacd761aa54cf11c87542433f855a735ca790ae8f154db815b99b0c155065b5169729f522c0698c9f29cd0 64 => 0x8d9eb47cfa1c11203619589cc9e64506733d965b2b250339063b29ad24582f229fed7ca416058384dd52cd4edb8af84f1302a875574a817668562e5da57dfa06e59136db3ec9c05585743550cf4a7e5f09b3d380d7f551a2efcc8e678d01c87c | 10079117 | ||
bls_g2_negate 0x96bbcfbca241004ba03968b7433664c1b944245828af7a1ebda771495c90d4816508d65db40b601bce51a3689c251f731699411b56e2172dd82eb96f571a691ba2984f1e4e39a05646ceb9c82d1108e78fe3fdfd94cff4fb447efe498dcecc25 => 0xb6bbcfbca241004ba03968b7433664c1b944245828af7a1ebda771495c90d4816508d65db40b601bce51a3689c251f731699411b56e2172dd82eb96f571a691ba2984f1e4e39a05646ceb9c82d1108e78fe3fdfd94cff4fb447efe498dcecc25 | 1882659 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment about costs. Literally, a single bit changes.
op-tests/test-blspy-pairing.txt
Outdated
@@ -0,0 +1,32 @@ | |||
; This file was generated by tools/generate-bls-tests.py | |||
|
|||
bls_pairing_identity ((0xa4a108e5dabd6d6fde629fe9515cf397ab2a2d44c5740d39525aa7f5230df38d6753cb7a0683a75e2f4f3d82fd46cdb0 . 0xa9c8ab9f27d84f764c709c82c516e5d6782d43751c55a48cb39999e321f3a896696878838608b3764c43e20af5ab1c2e14cf7a274ebd8f9f418a050bab94ffd44d7a054c41b08425fdf0c7b2d1317cd5cf121749f456068b113ef606aebacd2c) (0xb66837657cd1564bb04e9acb1efd96b53ef90502bb000e6ca44da7e07f655e6fb50606f3cc47fd936ba4108ccf592625 . 0xb4e0d25a3588446f00043183bd3e072eb50765ce3f57ec7cca1f10014117b33530aec7ceff37c291d267a66c557f49d000d255b10ca4804d0bebcec9cb073577eaa416673f9f3bc6bf2936c0be7154fc1cccc430003aaae5311f0d67673942d3) (0x8f3eb0208a3f0808df538ebc1f582d01c62baabe12842171f7d6c01f9b31f7c9d386190d9349d53378998d0162c7ecb2 . 0x8bb781464f4a21418284f8307755e88a696585b56edbfe3c927998e66aea3fccaded0830ef3500f2c8a46bf16bff7dbf1388b511109da614b928f0d17b8e8d7d0cded6dc2477e9c3c3cd5c97b41067ed3200cd7016eff07e45b79112cc1d6a13) (0xb066ce9e6484fe574ed55003449fae4d08652f85de45d718c2397c667cd80ed5571458d58c7d28b101ab1ef986973e77 . 0x8962435666bd7d6d6a94210e0f52f45d0b516139e4e9925bad4f0bf3e96dc6c62db5e50d9f86c1cef5e34caa79f4585e036a43550b14f0269bdbdbcee627b004613c96a7f4195970a0084214a1c2a40decf032efb94e657005a0da78c77d3d0d) (0xa73e3a434a117bed5c0eab24dce24b2e03bc75254b878900eb01f98cf389329cdd7d83cd8033475ff1ec6b2e10abbb09 . 0xa7ce0b706c4b6de43444ce02353bbee0d953fe403880fb49668ac1244bad2eeb5e3f83f3f526676d3513f56e32c7dfdd143061616b34d784edcde312c5b2d024878cb3b965895660d6727f5c210f73f60ed91be68b5008ad09048585c74950df) (0xb756ecad886b575d2c95014c20bcd9b52f1f72e3b5fbc657bacf1e72be707f9aaf76a9156a1628b99d873036beecec10 . 0xaa898708a47aa64d7d2d1d5d5b80d5609bb6dd088f8d40c3a82983fa517878a748c54acd162176871c12b404073a8f4119a7f4d04779af2b2d59949fa5d80c57bd8ed4262e4c186953ec28ecc69a02789474816f62b40184a448fcaaba7b67b6) (0xae0154514ac83b8efea5b2eee425fc41ead032bb04b8a977eff85e80f9dd8940833c0d7d85921bf18fe84f639f417cca . 0xa63a338eb15d6b1f3c4d6b6989f851841306dc93bad8963176a8c3e10af3de84254905d9bdb0afbe2a34cb2e3d98e4a6020ae15384e0a35b36658a10fb3c2d7391a545019acbc6892cbf9447628be9852e788f31e7be1078937102eb976bbac4) (0xb7f1d3a73197d7942695638c4fa9ac0fc3688c4f9774b905a14e3a3f171bac586c55e83ff97a1aeffb3af00adb22c6bb . 0x8ded853eb71f6015bc2c338cc2e2f2540e1d4cdc7d662965cd8827d6482e321e94ebd2cd299f77063336a531e294d879047d7017b93b232ce95a26e52cef34a1120aa4a3ab256242044d33e15b5430ff58612102bc80ef195a3adb4477cb7e1b) ) => 1 | 41122591 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not convinced that this shape for the arguments makes the most sense. I think need to actually implement a proof to see. If our loops produce N values from g1 and N values from g2, to call this, we'd need to do a zip
operation in clvm which might better be done in the vm implementation. Compare to (bls_pairing_identity g1_list g2_list)
. I don't know it's better, which is why I think we need to implement the toy to see.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good point. my impression so far is that this list is unlikely to be dynamic. in the snarkjs case, it's hard-coded to be 4 pairs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here's the equivalent call in snarkjs' proof verification: https://github.com/iden3/snarkjs/blob/master/src/groth16_verify.js#L62-L68
It has a fixed number of arguments.
const res = await curve.pairingEq(
curve.G1.neg(pi_a) , pi_b,
cpub , vk_gamma_2,
pi_c , vk_delta_2,
vk_alpha_1, vk_beta_2
);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this is typical, then yes, varargs makes the most sense.
op-tests/test-blspy-verify.txt
Outdated
@@ -0,0 +1,32 @@ | |||
; This file was generated by tools/generate-bls-tests.py | |||
|
|||
bls_verify ((0xa4a108e5dabd6d6fde629fe9515cf397ab2a2d44c5740d39525aa7f5230df38d6753cb7a0683a75e2f4f3d82fd46cdb0 . 0xdb16a69196ce95ba4e04d0022f66b25b8f) (0xb66837657cd1564bb04e9acb1efd96b53ef90502bb000e6ca44da7e07f655e6fb50606f3cc47fd936ba4108ccf592625 . 0x40779f85fac1529f4eebe90c68ed3d3389d15111a797fdf9434045841b3a375310d5) (0x8f3eb0208a3f0808df538ebc1f582d01c62baabe12842171f7d6c01f9b31f7c9d386190d9349d53378998d0162c7ecb2 . 0xff27e2213e1faf566c4565ffede6bc89ec96bf89eaf4dbf343f01dfa7f55bdcad87845) (0xb066ce9e6484fe574ed55003449fae4d08652f85de45d718c2397c667cd80ed5571458d58c7d28b101ab1ef986973e77 . 0x5720658a115091d4f50561fb82f84b75a0f6ac41a808f0) (0xa73e3a434a117bed5c0eab24dce24b2e03bc75254b878900eb01f98cf389329cdd7d83cd8033475ff1ec6b2e10abbb09 . 0x31e471716c9f23491d) (0xb756ecad886b575d2c95014c20bcd9b52f1f72e3b5fbc657bacf1e72be707f9aaf76a9156a1628b99d873036beecec10 . 0x61e43e08f2) (0xae0154514ac83b8efea5b2eee425fc41ead032bb04b8a977eff85e80f9dd8940833c0d7d85921bf18fe84f639f417cca . 0x724dbefe2f80c6c12bd19cb407d847) ) 0x8ded853eb71f6015bc2c338cc2e2f2540e1d4cdc7d662965cd8827d6482e321e94ebd2cd299f77063336a531e294d879047d7017b93b232ce95a26e52cef34a1120aa4a3ab256242044d33e15b5430ff58612102bc80ef195a3adb4477cb7e1b => 1 | 36664624 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here, I think the signature should come first so the varargs
can come last. It would Probably simplify the rust implementation too.
Oh wait, I guess it's not actually varargs
... it looks like it's a single argument that's a list.
Here again I wonder if we want the zipping of g1
and g2
to be done in the VM rather than clvm. We need to come up with a use case so we can check.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, I think I'm leaning towards varargs rather than a list of pairs. both for bls_pairing_identity
and bls_verify
|
||
# bls_g1_multiply | ||
for g1 in g1_points: | ||
scalar = randint(-100, 100) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We definitely want to test 0
and we want to test large scalars.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly requesting more test cases, and we need to consider more whether pairings should be zipped or not.
I also find it curious that there is no access to the |
I suggested an operator that would just return the generator as a constant, but Bram did not like that idea. He preferred relying on compression and referencing previous blocks to de-duplicate constants like that. |
As I noted above, we already have one that implicitly uses and can create the g1 generator (well, sort of), so it's a bit strange that we don't have one for g2. Also, I'm still extremely wary of cross-block compression. |
I like the shorter names |
I have addressed all review comments (afaict). I appended individual commits for each issue, to make it easier to review. Please review the 7 most recent commits. I also pulled out the one commit to move the |
check_arg_count(&args, 1, "g1_negate")?; | ||
let point = g1_atom(args.first()?)?; | ||
let total = -point; | ||
new_atom_and_cost(a, BLS_G1_NEGATE_BASE_COST, &total.to_compressed()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@richardkiss is the compressed form guaranteed to just flip the most significant bit when negating? If so, perhaps it would make sense to implement this as just a validation check + the bit-flip. It's probably faster than to_compressed()
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe so, yes. My understanding is that the encoding has a sign bit, so just figure out what that is and flip it. The only time this doesn't work is the zero (aka "infinity" just to be confusing) value, which I think has its own bit indicating 0, and (unsurprisingly) should pass through unchanged.
I'm inclined to submit my patch to update the costs (along with the benchmarking tool) to a separate PR. I think this one is big enough as it is. Unless someone would prefer I amend this one. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My comments have been addressed, but I don't know enough about the rust implementation here to really judge it, so my approval should be taken as an approval of the syntax and test cases only.
tools/generate-bls-tests.py
Outdated
@@ -28,23 +28,26 @@ def print_validation_test_case(f1, f2, num_cases, filter_pk, filter_msg, filter_ | |||
cost = 4999087 | |||
messages = [] | |||
sigs = [] | |||
f1.write("bls_verify (") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Curiously, this file makes test-blspy-verify.txt and test-blspy-pairing.txt a bit redundant since it can generate them. That means technically we could get away with not checking in the .txt files since they could just be generated as-needed by the CI or tests. I don't think we need to make this change at this time, but it's an interesting thing to think about.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah. I'm tempted to make the CI ensure the generated test cases are up-to-date. It's convenient to be able to run cargo test
locally without having to generate these files too.
Looks good. |
I'm squashing the fixup commits into their respective original commits they change:
That's how I ensured re-juggling the commits didn't alter the overall change. |
Pull Request Test Coverage Report for Build 4988488162
💛 - Coveralls |
This is a patch set, please review one commit at a time.
The BLS extension feature is not quite complete yet, it's missing:
BLS softfork extension
These new operators are added under the BLS softfork extension operator set. This set currently only contains the new
coinid
operator. They will be available under the invocation:(softfork <cost> 0 <program> <env>)
. This operator set is also enabled outside the softfork guard when run with the hard-fork flag set. The intention is to make all of these operators available by default after the 2.0 hard fork activates.unit tests
With this feature, I'm adding quite a lot of new test cases for operators, so the files
test-core-ops.txt
,test-more-ops.txt
etc. were moved into a sub directory,op-tests
.There's a new python script.
tools/generate-bls-tests.py
, which generates theop-tests/test-blspy-*.txt
test cases. This primarily ensures that the BLS operations are equivalent to the corresponding operations inblspy
(which is what we use in the full node)Some of these tests can be quite slow, especially in debug builds. In order to speed up
cargo test
, I madetest_ops
parameterized on the different test files to run them in parallel.new operators
point_add
operator.msg
hashes to.msg
hashes to.1
if the pairing of all G1 and G2 pairs is the (Gt) identity andnull
otherwise.msg
) given their public key (G1
) against the signature (G2
).g1_map
Hashes the specified message to G1 using SHA-256 and ExpandMsgXmd, using the specified
dst
(domain separation tag).dst
is optional, defaulting toBLS_SIG_BLS12381G1_XMD:SHA-256_SSWU_RO_AUG_
.g2_map
Hashes the specified message to G2 using SHA-256 and ExpandMsgXmd, using the specified
dst
(domain separation tag).dst
is optional, defaulting toBLS_SIG_BLS12381G2_XMD:SHA-256_SSWU_RO_AUG_
.bls_verify
Returns 1 if the signature is valid,
null
otherwise. The validation uses the Augmented scheme, which means the public keys (G1
) are prepended to the messages before being hashed to G2 points. The domain separation tag isBLS_SIG_BLS12381G2_XMD:SHA-256_SSWU_RO_AUG_
. The validation includes the pair of the negated G1 generator and the signature in the underlying pairing operation.Changes from Cameron's patch
This is based on Cameron's original BLS operator patch with the following main changes:
bool
indicating whether it's the identity or not. This is the check we care about for validating signatures.AGG_SIG_ME
andAGG_SIG_UNSAFE
bls_pairing_identity
and (G1, message)-pairs forbls_verify
. The pairs are passed as single cons boxes with the two elements as its children. They used to be passed as lists of 2 items.