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

Relay entry BLS verification #605

Merged
merged 10 commits into from
Mar 12, 2019
Merged

Conversation

ngrinkevich
Copy link
Contributor

@ngrinkevich ngrinkevich commented Feb 26, 2019

Closes:#31

Add verification to make sure that the resulting groupSignature was produced by the specified groupPubKey for the corresponding previousEntry

@ngrinkevich ngrinkevich requested review from a team February 26, 2019 15:11
@ngrinkevich ngrinkevich force-pushed the relay-entry-bls-verification-updated branch from 596a9ed to 5970005 Compare February 27, 2019 01:15
@pdyraga
Copy link
Member

pdyraga commented Mar 6, 2019

@ngrinkevich How do we look here? Is it ready for review/testing?

@ngrinkevich
Copy link
Contributor Author

How do we look here? Is it ready for review/testing?

@pdyraga it is yeah

@ngrinkevich ngrinkevich force-pushed the relay-entry-bls-verification-updated branch from d73cf7f to 9534fee Compare March 6, 2019 17:09
On-chain verification expects compressed G1 point due gas optimization.
@ngrinkevich ngrinkevich force-pushed the relay-entry-bls-verification-updated branch from 383c0df to 24b3cbc Compare March 8, 2019 01:47
This number should match the number of the polynomial coefficients created
at the beginning of the DKG protocol.
@ngrinkevich ngrinkevich force-pushed the relay-entry-bls-verification-updated branch from 24b3cbc to 8c32122 Compare March 8, 2019 13:19
cmd/relay.go Outdated Show resolved Hide resolved
contracts/solidity/contracts/KeepRandomBeaconImplV1.sol Outdated Show resolved Hide resolved

// Initialize Keep Group contract
minimumStake = 200000;
groupThreshold = 15;
groupSize = 20;
timeoutInitial = 20;
timeoutSubmission = 40;
timeoutSubmission = 50;
Copy link
Member

Choose a reason for hiding this comment

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

Why did we have to increase this value?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this was the tweak to make group selection work on initialize in js tests, irrelevant now since I'm removing runGroupSelection from initialize

Copy link
Member

Choose a reason for hiding this comment

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

So can we switch it back to 40? This should speed up the tests.

contracts/solidity/test/TestKeepRandomBeaconUpgrade.js Outdated Show resolved Hide resolved
contracts/solidity/test/TestRelayEntry.js Outdated Show resolved Hide resolved
pkg/beacon/relay/relay.go Outdated Show resolved Hide resolved
@ngrinkevich
Copy link
Contributor Author

@piotr @nkuba ready for second pass

@pdyraga
Copy link
Member

pdyraga commented Mar 12, 2019

Two comments that still need to be addressed but we can do it in a followup PR:

@pdyraga
Copy link
Member

pdyraga commented Mar 12, 2019

I tested it on my local environment:

  • random beacon loops when no changes are made
  • modified submit command to send incorrect signature - transaction rejected by the chain as expected
  • modified signing protocol to send incorrect signature - transaction rejected by the chain as expected

@pdyraga
Copy link
Member

pdyraga commented Mar 12, 2019

:shipit:

@pdyraga pdyraga merged commit 68d8741 into master Mar 12, 2019
@pdyraga pdyraga deleted the relay-entry-bls-verification-updated branch March 12, 2019 11:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants