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

Fix commitment lengths for side-loaded verification keys #9761

Merged

Conversation

mrmr1993
Copy link
Member

@mrmr1993 mrmr1993 commented Nov 3, 2021

This error was introduced in df12992, where the generic version was updated correctly to use h everywhere, but the generic' version incorrectly used n instead of h. Since we weren't using side-loaded keys anywhere at the time, this was also missed during testing.

NOTE: This is deliberately against develop rather than compatible, because this change will cause snark keys to change!

Checklist:

  • Modified the current draft of release notes with details on what is completed or incomplete within this project
  • Document code purpose, how to use it
    • Mention expected invariants, implicit constraints
  • Tests were added for the new behavior
    • Document test purpose, significance of failures
    • Test names should reflect their purpose
  • All tests pass (CI will check this if you didn't)
  • Serialized types are in stable-versioned modules
  • Does this close issues? List them

This error was introduced in df12992,
where the `generic` version was updated correctly to use `h` everywhere,
but the `generic'` version incorrectly used `n` instead of `h`. Since we
weren't using side-loaded keys anywhere at the time, this was also
missed during testing.
@mrmr1993 mrmr1993 added the ci-build-me Add this label to trigger a circle+buildkite build for this branch label Nov 3, 2021
@mrmr1993 mrmr1993 requested a review from a team as a code owner November 3, 2021 18:29
@deepthiskumar
Copy link
Member

Seeing verification failure in gossip consistency integration tests


bad verify: dlog_check\ncombined_inner_product: 0xfe0b4669da7a5589f9c7ba2f9efa80f02718b397156aa59c007e1f2af989b938 != 0x73e0d0d9b58f5b6b1080453a48ebcae5aff0b7436804f6684bae6f3bc1598d2c\nxi: 0xbb2cda42e5edbcf6717e964b1c84c999ade9c7d5f3f2e9bd57e15a5fae461b2d != 0xa1ea6a27dc9f0d5236758c5ea22567225a4f2f3f734d835cfbb21422d0598105\nlinearization_check\nbad verify: dlog_check\ncombined_inner_product: 0xfe0b4669da7a5589f9c7ba2f9efa80f02718b397156aa59c007e1f2af989b938 != 0x73e0d0d9b58f5b6b1080453a48ebcae5aff0b7436804f6684bae6f3bc1598d2c\nxi: 0xbb2cda42e5edbcf6717e964b1c84c999ade9c7d5f3f2e9bd57e15a5fae461b2d != 0xa1ea6a27dc9f0d5236758c5ea22567225a4f2f3f734d835cfbb21422d0598105\nlinearization_check\nbad verify: dlog_check\ncombined_inner_product: 0xfe0b4669da7a5589f9c7ba2f9efa80f02718b397156aa59c007e1f2af989b938 != 0x73e0d0d9b58f5b6b1080453a48ebcae5aff0b7436804f6684bae6f3bc1598d2c\nxi: 0xbb2cda42e5edbcf6717e964b1c84c999ade9c7d5f3f2e9bd57e15a5fae461b2d != 0xa1ea6a27dc9f0d5236758c5ea22567225a4f2f3f734d835cfbb21422d0598105\nlinearization_check

@mrmr1993
Copy link
Member Author

mrmr1993 commented Nov 4, 2021

Seeing verification failure in gossip consistency integration tests

Spent a while looking into this, but it turns out it's not a fatal issue, and appears to be unrelated. For example, you can see the same in the gossip consistency test logs for the unrelated PR #9759.

@mrmr1993 mrmr1993 force-pushed the feature/pickles-fix-side-loaded-vk-proof-verification branch from 262175e to de1fcf2 Compare November 4, 2021 15:10
@mrmr1993
Copy link
Member Author

mrmr1993 commented Nov 4, 2021

Compatibility checks are failing, but they should be optional on develop. Merging.

@mrmr1993 mrmr1993 merged commit af76cb7 into develop Nov 4, 2021
@mrmr1993 mrmr1993 deleted the feature/pickles-fix-side-loaded-vk-proof-verification branch November 4, 2021 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci-build-me Add this label to trigger a circle+buildkite build for this branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants