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

Introduce BLS signature aggregation scheme #151

Merged

Conversation

@Byeongjee
Copy link
Contributor

Byeongjee commented Feb 10, 2020

I didn't write private key generation part yet since it is subject to change.
Please comment if there is a better way of holding signatures/keys, and decode them.

@Byeongjee Byeongjee requested a review from HoOngEe Feb 10, 2020
@Byeongjee Byeongjee force-pushed the Byeongjee:introduce.bls.signature branch from 4f3a148 to 8e277de Feb 10, 2020
@Byeongjee

This comment has been minimized.

Copy link
Contributor Author

Byeongjee commented Feb 11, 2020

I'll implement aggregated BLS signature scheme without considering rogue key attack.
Also, BLS private key of a validator will depend on the secret of the validator's account.
These two problems should be changed after this PR.

@Byeongjee Byeongjee force-pushed the Byeongjee:introduce.bls.signature branch 4 times, most recently from 2a852e1 to e1cded9 Feb 12, 2020
@Byeongjee Byeongjee changed the title Introduce.bls.signature Introduce BLS signature aggregation scheme Feb 13, 2020
@Byeongjee Byeongjee force-pushed the Byeongjee:introduce.bls.signature branch 2 times, most recently from c4c745d to 126e81a Feb 14, 2020
@Byeongjee Byeongjee changed the title Introduce BLS signature aggregation scheme [WIP] Introduce BLS signature aggregation scheme Feb 14, 2020
@Byeongjee Byeongjee force-pushed the Byeongjee:introduce.bls.signature branch 2 times, most recently from 1ca511c to 2bd97dd Feb 17, 2020
key/src/bls.rs Outdated Show resolved Hide resolved
core/src/consensus/stake/actions.rs Outdated Show resolved Hide resolved
@Byeongjee Byeongjee force-pushed the Byeongjee:introduce.bls.signature branch from dada030 to 61ad23b Feb 18, 2020
@Byeongjee

This comment has been minimized.

Copy link
Contributor Author

Byeongjee commented Feb 18, 2020

I fixed all requested changes. I'm planning to rebase this after. #154 and implement remaining test cases. How do you think about it? @HoOngEe

@HoOngEe

This comment has been minimized.

Copy link
Contributor

HoOngEe commented Feb 18, 2020

@Byeongjee Ok, rebase after then. BTW, I'm still reviewing it. Sorry for the late review. Because I moved the ed25519 types into key/ed25519 module, few conflicts are expected

@HoOngEe HoOngEe changed the base branch from master to bls-consensus-signature Feb 18, 2020
@HoOngEe

This comment has been minimized.

Copy link
Contributor

HoOngEe commented Feb 18, 2020

@Byeongjee I changed the base from master to bls-consensus-signature

@Byeongjee

This comment has been minimized.

Copy link
Contributor Author

Byeongjee commented Feb 18, 2020

Okay I see. I'll wait for the remaining review.

key/src/bls.rs Outdated Show resolved Hide resolved
@Byeongjee Byeongjee force-pushed the Byeongjee:introduce.bls.signature branch from 61ad23b to 8bb9bde Feb 18, 2020
@HoOngEe

This comment has been minimized.

Copy link
Contributor

HoOngEe commented Feb 18, 2020

LGTM upto 443b5e1

@Byeongjee Byeongjee added the crypto label Feb 18, 2020
@Byeongjee Byeongjee force-pushed the Byeongjee:introduce.bls.signature branch 3 times, most recently from 9b848a1 to 596d213 Feb 19, 2020
@Byeongjee Byeongjee force-pushed the Byeongjee:introduce.bls.signature branch from 997e33a to cc92dbe Feb 27, 2020
@Byeongjee Byeongjee added the consensus label Feb 27, 2020
@Byeongjee Byeongjee force-pushed the Byeongjee:introduce.bls.signature branch 3 times, most recently from a4b9db8 to f0696c4 Feb 27, 2020
@Byeongjee

This comment has been minimized.

Copy link
Contributor Author

Byeongjee commented Feb 27, 2020

I fixed lint warning, and then organized commits.
I squashed many related commits, so now there are 8 commits only.

@Byeongjee Byeongjee force-pushed the Byeongjee:introduce.bls.signature branch from f0696c4 to 8053833 Feb 27, 2020
@Byeongjee

This comment has been minimized.

Copy link
Contributor Author

Byeongjee commented Feb 27, 2020

Now there are seven commits.

@Byeongjee Byeongjee force-pushed the Byeongjee:introduce.bls.signature branch 2 times, most recently from 8b7d90e to a9e0e35 Feb 27, 2020
key/src/bls.rs Outdated Show resolved Hide resolved
core/src/consensus/stake/action_data.rs Show resolved Hide resolved
@Byeongjee Byeongjee force-pushed the Byeongjee:introduce.bls.signature branch 3 times, most recently from adc70a0 to 06dc5af Feb 27, 2020
Byeongjee added 6 commits Feb 13, 2020
Validators need to provide their BLS public keys at self-nomination step,
since they cannot be inferred from their transaction signing keys
anymore.
Also, proof of posession step of the BLS private key is required to
prevent rogue key attacks.
For proof of posession, we should use the signature of BLS public key concatenated with its address.
Contatenation with the address is required to prevent other validators to reuse the signature, aka replay attacks.
Previously, an ECDSA public key was sufficient to represent a validator
because the validator's account address can be derived from the key.
However, a validator's BLS public key and an address cannot be derived
from each other, so we have to keep both of them.
An aggregated BLS signature of validators can replace the collection of Schnorr signatures in a Tendermint seal.
Throwing out old votes should be postponed because we cannot recover votes from a block seal anymore.
@Byeongjee Byeongjee force-pushed the Byeongjee:introduce.bls.signature branch 2 times, most recently from 247bec0 to 1b76d9b Feb 27, 2020
@HoOngEe

This comment has been minimized.

Copy link
Contributor

HoOngEe commented Feb 27, 2020

yarn-lint fail is due to yarn formatter. Apply yarn fmt to the commits involved with test code changes.

Validator initialization logic should be changed because the definition of validator set has been changed.
These hard-coded values should be replaced with sdk function calls in the future.
@Byeongjee Byeongjee force-pushed the Byeongjee:introduce.bls.signature branch from 1b76d9b to 5899b58 Feb 27, 2020
@Byeongjee

This comment has been minimized.

Copy link
Contributor Author

Byeongjee commented Feb 27, 2020

Thanks. I applied them.

@HoOngEe HoOngEe self-requested a review Feb 27, 2020
@HoOngEe HoOngEe merged commit 239fcc4 into CodeChain-io:bls-consensus-signature Feb 27, 2020
8 of 9 checks passed
8 of 9 checks passed
Actions - build (macOS-10.14) Actions - build (macOS-10.14)
Details
Actions - clippy
Details
Actions - lint
Details
Actions - build (ubuntu-18.04)
Details
Actions - rustfmt
Details
Actions - unit test (macOS-latest)
Details
Actions - unit test (ubuntu-latest)
Details
Travis CI - Pull Request Build Errored
Details
Summary no rules match, no planned actions
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.