Skip to content

test(e2e): add support for testing different key types, including BLS#3513

Merged
melekes merged 17 commits intomainfrom
sergio/bls-on-e2e
Aug 6, 2024
Merged

test(e2e): add support for testing different key types, including BLS#3513
melekes merged 17 commits intomainfrom
sergio/bls-on-e2e

Conversation

@sergio-mena
Copy link
Copy Markdown
Collaborator

@sergio-mena sergio-mena commented Jul 12, 2024

While working on another crypto to be added to CometBFT, I realised we are not testing other key types in our nightlies, in particular BLS isn't tested.

This PR:

  • adds support for BLS in the runner, generator, and e2e tests
  • adds support for BLS, secp256k1, and ed25519 key types in the e2e generator (for the nightlies)
  • completes support for changing the key type at testnet level. Some support was existing but it wasn't complete (so it didn't work)

There is a TODO in the BLS crypto side. I added a temporary workaround and contacted the Zondax folks to include a small patch in github.com/cosmos/crypto. If they agree to merge the patch, GenPrivKeyFromSecret will become trivial.


PR checklist

  • Tests written/updated
  • Changelog entry added in .changelog (we use unclog to manage our changelog)
  • [ ] Updated relevant documentation (docs/ or spec/) and code comments
  • Title follows the Conventional Commits spec

@sergio-mena sergio-mena requested a review from a team as a code owner July 12, 2024 14:45
@sergio-mena sergio-mena requested a review from a team July 12, 2024 14:45
@sergio-mena sergio-mena self-assigned this Jul 12, 2024
@sergio-mena sergio-mena added the e2e Related to our end-to-end tests label Jul 12, 2024
Comment thread crypto/bls12381/key_bls12381.go Outdated
Comment thread crypto/bls12381/key_bls12381.go Outdated
Copy link
Copy Markdown

@cason cason left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Legit.

melekes added 3 commits July 28, 2024 16:03
to both Sign and VerifySignature
https://pkg.go.dev/github.com/cosmos/[email protected]/curves/bls12381#VerifySignature

even though Sign doesn't require [32]byte msg, passing arbitrary small
messages makes the TestGenPrivKeyFromSecret_SignVerify fail!
Copy link
Copy Markdown
Collaborator

@melekes melekes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Copy link
Copy Markdown
Collaborator

@melekes melekes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@melekes melekes enabled auto-merge August 6, 2024 03:39
@melekes melekes added this pull request to the merge queue Aug 6, 2024
Merged via the queue into main with commit 7333421 Aug 6, 2024
@melekes melekes deleted the sergio/bls-on-e2e branch August 6, 2024 03:47
@sergio-mena sergio-mena restored the sergio/bls-on-e2e branch August 16, 2024 14:59
@sergio-mena sergio-mena deleted the sergio/bls-on-e2e branch August 16, 2024 15:00
aljo242 pushed a commit that referenced this pull request Sep 4, 2025
…#3513)

While working on another crypto to be added to CometBFT, I realised we
are not testing other key types in our nightlies, in particular BLS
isn't tested.

This PR:
* adds support for BLS in the runner, generator, and `e2e` tests
* adds support for BLS, secp256k1, and ed25519 key types in the `e2e`
generator (for the nightlies)
* completes support for changing the key type at testnet level. Some
support was existing but it wasn't complete (so it didn't work)

There is a TODO in the BLS crypto side. I added a temporary workaround
and contacted the Zondax folks to include a small patch in
`github.com/cosmos/crypto`. If they agree to merge the patch,
`GenPrivKeyFromSecret` will become trivial.

---

- [x] Tests written/updated
- [x] Changelog entry added in `.changelog` (we use
[unclog](https://github.com/informalsystems/unclog) to manage our
changelog)
- ~[ ] Updated relevant documentation (`docs/` or `spec/`) and code
comments~
- [x] Title follows the [Conventional
Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec

---------

Co-authored-by: Anton Kaliaev <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

e2e Related to our end-to-end tests

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants