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

[EPIC] BLS aggregate signatures for validator registration #546

Open
blockchainluffy opened this issue Nov 4, 2024 · 3 comments
Open

[EPIC] BLS aggregate signatures for validator registration #546

blockchainluffy opened this issue Nov 4, 2024 · 3 comments

Comments

@blockchainluffy
Copy link
Contributor

blockchainluffy commented Nov 4, 2024

This issue tracks what changes need to be implemented for keyper and observer, if the spec PR 53 "Use BLS aggregate signatures for validator registry" gets accepted:

  • Thanks to the version byte in the message, 0x01, that will be used for BLS aggregate messages, we can handle the change backward compatible.

  • Signature validation on both versions of messages is to be handled separately.

  • After message decoding and validation on keyper and observer the updates for each signing validator can be handled separately by destructuring the aggregated message into one per involved validator, with the corresponding validatorIndex field in the keyper and observer database.

  • No changes needed for contract and databases in keyper and observer.

  • Edge case: nonces from legacy version that are higher than uint32 will mean that the involved validator can not use the new message format, legacy format can still be supported by keyper and observer

  • We should exchange some test vectors with nethermind, to make sure our BLS libraries aggree on the aggregate signature scheme.

@konradkonrad
Copy link
Contributor

Here is a draft PR, that adds the new signature scheme for unit tests: #547

@fredo
Copy link
Contributor

fredo commented Nov 4, 2024

Looks very good. Did you align with marc on the backwards compatibility? I think he asked to make this a breaking change.

@blockchainluffy
Copy link
Contributor Author

The PR for this is already merged to main, #548

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

3 participants