Skip to content
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

Proposal: BLS serialization tests #24

Closed
hwwhww opened this issue Dec 4, 2020 · 7 comments
Closed

Proposal: BLS serialization tests #24

hwwhww opened this issue Dec 4, 2020 · 7 comments

Comments

@hwwhww
Copy link
Contributor

hwwhww commented Dec 4, 2020

BLS serialization tests

Background

Proposed new test suite

The input and output of our BLS APIs are all in minimal-pubkey-size form (compressed 48-byte pubkey and 96-byte signature). So the functions to test would be:

def compress_G1(decompressed_pubkey: bytes96) -> bytes48
  • input:
    • decompressed_pubkey: bytes96
  • output:
    • compressed_pubkey: (i) bytes48 or (ii) empty for the invalid cases.
def decompress_G1(compressed_pubkey: bytes48) -> bytes96
  • input:
    • compressed_pubkey: bytes48
  • output:
    • decompressed_pubkey: (i) bytes96 (ii) empty for the invalid cases.
def compress_G2(decompressed_signature: bytes192) -> bytes96
  • input:
    • decompressed_signature: bytes192
  • output:
    • compressed_signature: (i) bytes96 (ii) empty for the invalid cases.
def decompress_G2(compressed_signature: bytes96) -> bytes192
  • input:
    • compressed_signature: bytes96
  • output:
    • decompressed_signature: (i) bytes192 (ii) empty for the invalid cases.

Discussions

1. Are the APIs available?

  • 1.1. I naively assumed that the BLST binding does provide these APIs for all supported languages. Could the client teams help check if it's true? /cc @kirk-baird @michaelsproul @benjaminion @mratsim @nisdas and @dot-asm
  • 1.2. Do your compression and decompression APIs include subgroup membership checking?

2. Did the fuzzing already cover the 3-MSBs edge cases of BLS tests?

/cc @zedt3ster

3. Do you think it would help to reduce the consensus error risks?

/cc @JustinDrake @CarlBeek

@nisdas
Copy link

nisdas commented Dec 4, 2020

@hwwhww For testing compressing/decompressing of G1/G2 points, the Go API in BLST does have support for this so its not an issue on our end. Serialization test format looks good to me 👍

On 1.2 , BLST has removed subgroup checks when decompressing points as of v3 , this was just merged into Prysm here.
prysmaticlabs/prysm#7971 . I imagine it would be the same for the other language bindings too.

@dot-asm
Copy link

dot-asm commented Dec 4, 2020

  • 1.1. I naively assumed that the BLST binding does provide these APIs for all supported languages.

The group check is an expensive operation, and it's argued that application is entitled for a choice when to perform it. Primarily because there are situations when it's possible to perform multiple checks in parallel. In other words blst deserialization/uncompression subroutines don't perform group checks, and it's application's responsibility to either make explicit in-group-check call right after deserialization, or pass perform-group-checks-as-you-go flags to higher level procedures and utilize parallelism whenever possible.

Speaking of serialization format in more general terms. As data is deserialized and converted into internal representation format, it implicitly gets modulo reduced. In other words deserialization procedure can handle non-reduced input seamlessly. But it's only natural to assume that if input is not reduced, then somebody is trying to mess with you. For this reason first thing the [blst] deserialaztion procedure does is to ensure that input is fully reduced. However, this is not actually specified, and a formal concern can be and even was raised. In other words, while we are at it, it would be only appropriate to explicitly specify that input is expected to be fully reduced and provide corresponding test vector.

@mratsim
Copy link

mratsim commented Dec 10, 2020

1.1 The C API that we use has support for the decompression/uncompression from Zcash
1.2 We do infinity checks and subgroup checks after decompression and skip those at verification because we do a lot of verification with the same public key so no need to pay an expensive subgroup check every time.

@kirk-baird
Copy link

These APIs are all available in Apache Milagro Rust and could trivially be added to milagro_bls.

With respect to the subgroup checking in lighthouse we do that for PublicKeys at serialization time and Signatures at verification time. So it may not be on by default for lighthouse but can easily be added to tests. So happy eitherway if we are to include or not include the subgroup checks.

@zedt3ster
Copy link

From a fuzzing perspective, the existing BLS work (available here) included the 3 MSBs but was only aimed at the compressed version (i.e. MSB0 = 1)

@mratsim
Copy link

mratsim commented Dec 18, 2020

Here is a case from November 2019 where there was an infinity bit set and we didn't check properly that the rest of the bits were 0: status-im/nimbus-eth2#555

This was before we skipped the Ethereum signature for testing so it's from one of the old test vectors.

@mratsim
Copy link

mratsim commented Oct 1, 2021

Close?

https://github.com/ethereum/bls12-381-tests includes decompression tests (cc @asanso). Compression can be done by clients via round-trip tests to confirm internal consistency.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants