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

Exp/bls-multisig-impl #175

Draft
wants to merge 5 commits into
base: dev
Choose a base branch
from
Draft

Conversation

naitik-supraoracles
Copy link

draft PR

  • in order to review changes have been done so far for bls signature implementation.

network_addresses: vector<u8>,
fullnode_addresses: vector<u8>,
) acquires AllowedValidators {
// Checks the public key is valid to prevent rogue-key attacks.
let valid_public_key = ed25519::new_validated_public_key_from_bytes(consensus_pubkey);
assert!(option::is_some(&valid_public_key), error::invalid_argument(EINVALID_PUBLIC_KEY));

//DO WE HAVE TO ADD CHECK FOR BLS KEY TOO? I AM NOT SURE.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, we will need to ensure that the bytes comprise a valid BLS key.

Choose a reason for hiding this comment

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

for this verification, we required native function in move VM.

@@ -1900,8 +1904,10 @@ module supra_framework::stake {
account::create_account_for_test(validator_address);
};

let bls_pk_bytes = vector[];
Copy link
Collaborator

Choose a reason for hiding this comment

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

If we are calling this function anywhere, then this could be causing deserialization problems. The value needs to be initialized.

use std::fmt;
use blsttc::{SecretKey, PublicKeyG2};

/// An Ed25519 private key
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why does this still say Ed25519? Several places in this file seem to, actually. Have you tested this code?

Choose a reason for hiding this comment

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

actually. i forked it from ed15519 for keeping consistent file structures and similar methods and then changes functionalities inside, so might be left off to update content in documented code lines. will update it while finishing off.

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

Successfully merging this pull request may close these issues.

2 participants