core: add Ethereum-compatible BLS12-381 serializators and operations to CryptoLib#4050
core: add Ethereum-compatible BLS12-381 serializators and operations to CryptoLib#4050
Conversation
5d040cd to
11020de
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #4050 +/- ##
==========================================
+ Coverage 83.45% 83.49% +0.04%
==========================================
Files 352 352
Lines 42754 43158 +404
==========================================
+ Hits 35679 36036 +357
- Misses 5323 5366 +43
- Partials 1752 1756 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
It's not about aliases now, it's just a new function. |
AnnaShaleva
left a comment
There was a problem hiding this comment.
Workflows are failing.
Also, need some time to check the main math logic.
| google.golang.org/protobuf v1.36.9 // indirect | ||
| ) | ||
|
|
||
| replace github.com/nspcc-dev/neo-go/pkg/interop => ./pkg/interop No newline at end of file |
There was a problem hiding this comment.
Remove it once PR is finilized.
11020de to
5a566d4
Compare
0d9946b to
531864f
Compare
008e092 to
db38756
Compare
3aabc28 to
40f06e3
Compare
14c0d14 to
22bdc25
Compare
444f75c to
84e7b36
Compare
84e7b36 to
c0711db
Compare
e438231 to
f99c5a9
Compare
|
|
||
| func (p *blsPoint) FromBytesEth(buf []byte) error { | ||
| switch l := len(buf); l { | ||
| case bls12G1EncodedLength: |
There was a problem hiding this comment.
Add default case, return an appropriate error.
| } | ||
| } | ||
|
|
||
| func (p blsPoint) BytesEth() []byte { |
There was a problem hiding this comment.
Exported method needs a comment.
f99c5a9 to
88984f8
Compare
Close #4041. Signed-off-by: Tural Devrishev <tural@nspcc.ru>
88984f8 to
09d273a
Compare
|
@txhsl could you please review the implementation one more time? This CryptoLib API is the final suggestion from our side. Our primary concern was neo-project/neo#4186 (comment), so we believe it's solved in this PR. |
txhsl
left a comment
There was a problem hiding this comment.
Our primary concern was neo-project/neo#4186 (comment), so we believe it's solved in this PR.
Yes, I see the difference you've made, and the implementation is quite clear to me.
Maybe it's time for @Jim8y to have a look on this, he's a bit confused about the idea at the beginning.
| res = append(res, p.BytesEth()...) | ||
| scalarBE := scalar.Bytes() | ||
| scalarBytes := make([]byte, bls12ScalarLength) | ||
| copy(scalarBytes[bls12ScalarLength-len(scalarBE):], scalarBE) |
There was a problem hiding this comment.
It will panic with slice bounds out of range if scalarBE doesn't fit into bls12ScalarLength.
There was a problem hiding this comment.
It will panic with
slice bounds out of rangeifscalarBEdoesn't fit intobls12ScalarLength.
Oh, yes.
| const ( | ||
| g1Hex = "97f1d3a73197d7942695638c4fa9ac0fc3688c4f9774b905a14e3a3f171bac586c55e83ff97a1aeffb3af00adb22c6bb" | ||
| g2Hex = "93e02b6052719f607dacd3a088274f65596bd0d09920b61ab5da61bbdc7f5049334cf11213945d57e5ac7d055d042b7e024aa2b2f08f0a91260805272dc51051c6e47ad4fa403b02b4510b647ae3d1770bac0326a805bbefd48056c8c121bdb8" | ||
| ethG1MultiExpSingleInputHex = "0000000000000000000000000000000017f1d3a73197d7942695638c4fa9ac0fc3688c4f9774b905a14e3a3f171bac586c55e83ff97a1aeffb3af00adb22c6bb0000000000000000000000000000000008b3f481e3aaa0f1a09e30ed741d8ae4fcf5e095d5d00af600db18cb2c04b3edd03cc744a2888ae40caa232946c5e7e10000000000000000000000000000000000000000000000000000000000000011" |
There was a problem hiding this comment.
Maybe you can have more EVM test cases for operations other than MultiExp, although IMO they have already provided enough guarantees.
There was a problem hiding this comment.
Agree, at least we need to add:
- Compatibility tests for
bls12381PairingListwith some hex-encoded raw data that may be ported to C#. - Bi-directional serializators tests for all added APIs: ensure that result of
serialize(deserialize(input))matches theinputexactly.
| return stackitem.NewByteArray(res) | ||
| } | ||
|
|
||
| func (c *Crypto) bls12381SerializeEthList(_ *interop.Context, args []stackitem.Item) stackitem.Item { |
There was a problem hiding this comment.
Every introduced method needs a comment explaining which type of conversion it's performing, like it's done for bls12381SerializeList.
| res = append(res, p.BytesEth()...) | ||
| scalarBE := scalar.Bytes() | ||
| scalarBytes := make([]byte, bls12ScalarLength) | ||
| copy(scalarBytes[bls12ScalarLength-len(scalarBE):], scalarBE) |
There was a problem hiding this comment.
It will panic with slice bounds out of range if scalarBE doesn't fit into bls12ScalarLength.
| const ( | ||
| g1Hex = "97f1d3a73197d7942695638c4fa9ac0fc3688c4f9774b905a14e3a3f171bac586c55e83ff97a1aeffb3af00adb22c6bb" | ||
| g2Hex = "93e02b6052719f607dacd3a088274f65596bd0d09920b61ab5da61bbdc7f5049334cf11213945d57e5ac7d055d042b7e024aa2b2f08f0a91260805272dc51051c6e47ad4fa403b02b4510b647ae3d1770bac0326a805bbefd48056c8c121bdb8" | ||
| ethG1MultiExpSingleInputHex = "0000000000000000000000000000000017f1d3a73197d7942695638c4fa9ac0fc3688c4f9774b905a14e3a3f171bac586c55e83ff97a1aeffb3af00adb22c6bb0000000000000000000000000000000008b3f481e3aaa0f1a09e30ed741d8ae4fcf5e095d5d00af600db18cb2c04b3edd03cc744a2888ae40caa232946c5e7e10000000000000000000000000000000000000000000000000000000000000011" |
There was a problem hiding this comment.
Agree, at least we need to add:
- Compatibility tests for
bls12381PairingListwith some hex-encoded raw data that may be ported to C#. - Bi-directional serializators tests for all added APIs: ensure that result of
serialize(deserialize(input))matches theinputexactly.
Close #4041.