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.

Sign up for GitHub

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

Jump to bottom

crypto: Use BLS for AuthorityKeyPair #4645

Merged
merged 1 commit into from
Sep 16, 2022
Merged

crypto: Use BLS for AuthorityKeyPair #4645

merged 1 commit into from
Sep 16, 2022

Conversation

joyqvq
Copy link
Contributor

@joyqvq joyqvq commented Sep 15, 2022 •
edited
Loading

narwhal pointer using BLS

  • tested against mock data, explorer code is backward compatible

@joyqvq joyqvq changed the title Bls switching11 crypto: Use BLS for AuthorityKeyPair Sep 15, 2022
@joyqvq
Copy link
Contributor Author

joyqvq commented Sep 15, 2022

Running on legacy list auth sigs view works: image

Also works on new auth sig in one string:
image

@github-actions
Copy link
Contributor

github-actions bot commented Sep 15, 2022 •
edited
Loading

💳 Wallet Extension has been built, you can download the packaged extension here: https://github.com/MystenLabs/sui/actions/runs/3068352636#artifacts

@joyqvq joyqvq force-pushed the bls-switching11 branch 5 times, most recently from f4c8908 to 41bcda7 Compare September 15, 2022 19:21
@joyqvq joyqvq marked this pull request as ready for review September 15, 2022 22:03
@joyqvq joyqvq requested a review from huitseeker as a code owner September 15, 2022 22:03
kchalkias
kchalkias approved these changes Sep 15, 2022
View reviewed changes
explorer/client/src/pages/transaction-result/TransactionView.tsx
};
} else {
validatorSignatureData = {
title: 'Aggregated Validator Signature',
Copy link
Collaborator

Choose a reason for hiding this comment

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

<3

Jordan-Mysten
Jordan-Mysten approved these changes Sep 15, 2022
View reviewed changes
Copy link
Contributor

@Jordan-Mysten Jordan-Mysten left a comment

Choose a reason for hiding this comment

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

Explorer changes LGTM

kchalkias
kchalkias reviewed Sep 16, 2022
View reviewed changes
Copy link
Collaborator

@kchalkias kchalkias left a comment •
edited
Loading

Choose a reason for hiding this comment

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

After rebasing, check again w/ @huitseeker if we're good to go, so we can merge asap to avoid conflicts that would probably require an extra cycle of retesting locally for incompatibilities etc

@punwai
Copy link
Contributor

punwai commented Sep 16, 2022 •
edited
Loading

This is awesome, thank you so much @joyqvq for helping with the BLS PR splitting and pushing this to completion!

punwai
punwai reviewed Sep 16, 2022
View reviewed changes
explorer/client/src/pages/transaction-result/TransactionView.tsx
if (Array.isArray(txdata.authSignInfo.signature)) {
validatorSignatureData = {
title: 'Validator Signatures',
content: txdata.authSignInfo.signature.map(
Copy link
Contributor

@punwai punwai Sep 16, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Just curious, why do we need this? Don't we wipe the whole DB anyways for each devnet launch?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, just doing this for precaution, we can eventually deprecate the legacy display

@punwai punwai mentioned this pull request Sep 16, 2022
Closed
@huitseeker huitseeker mentioned this pull request Sep 16, 2022
Closed
@joyqvq joyqvq merged commit 42ac72c into main Sep 16, 2022
@joyqvq joyqvq deleted the bls-switching11 branch September 16, 2022 15:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@punwai punwai punwai left review comments

@kchalkias kchalkias kchalkias approved these changes

@Jordan-Mysten Jordan-Mysten Jordan-Mysten approved these changes

@huitseeker huitseeker Awaiting requested review from huitseeker

@666lcz 666lcz Awaiting requested review from 666lcz

Assignees
No one assigned
Labels
sui-explorer
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@joyqvq @punwai @kchalkias @Jordan-Mysten