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

Add Verifiable Random Function and test cases #2470

Merged
merged 11 commits into from
May 19, 2021
Merged

Add Verifiable Random Function and test cases #2470

merged 11 commits into from
May 19, 2021

Conversation

Jim8y
Copy link
Contributor

@Jim8y Jim8y commented May 16, 2021

Implemented the class of VRF based on draft-irtf-cfrg-vrf-08 at https://tools.ietf.org/pdf/draft-irtf-cfrg-vrf-08.pdf.

Copy link
Member

@shargon shargon left a comment

Choose a reason for hiding this comment

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

Could you add the syscalls?

src/neo/Cryptography/VRF.cs Outdated Show resolved Hide resolved
src/neo/Cryptography/VRF.cs Outdated Show resolved Hide resolved
@Jim8y
Copy link
Contributor Author

Jim8y commented May 17, 2021

Could you add the syscalls?

I already did, will commit soon after I fully test it.

@erikzhang
Copy link
Member

What do we use as the input of VRF?

@Jim8y
Copy link
Contributor Author

Jim8y commented May 18, 2021

What do we use as the input of VRF?

The hash value of the previous block, that value is well known to the public, so the primary could not cheat on this. The random number is generated through VRF, the primary can only calculate it but can not determine the value, nor can the primary modify it.

@erikzhang
Copy link
Member

There is a small problem here. The consensus node of the previous block can control the hash of the block. And if the consensus node of the current block fails, the consensus node of the previous block will generate the current block. In this way, the consensus node has the possibility of temporarily control the random numbers.

@Jim8y
Copy link
Contributor Author

Jim8y commented May 18, 2021

There is a small problem here. The consensus node of the previous block can control the hash of the block. And if the consensus node of the current block fails, the consensus node of the previous block will generate the current block. In this way, the consensus node has the possibility of temporarily control the random numbers.

Should not the previous block already being published? No matter who generates the random number, he can not determine the vrf value with a publicly known value as input.

Jim8y added 2 commits May 18, 2021 10:30
remove exception in VRF verify function
@@ -238,7 +238,7 @@ protected internal int GetInvocationCounter()
/// <summary>
/// The implementation of System.Runtime.GetRandom.
/// Gets the random number genrated form the VRF
/// Primary geterates this random number with `prevHash`, the hash of the prevuous block
/// Primary geterates this random number with `prevHash`, the hash of the previous (validators.Length/3 + 1) block
Copy link
Member

Choose a reason for hiding this comment

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

What's the difference?

Copy link
Contributor Author

@Jim8y Jim8y May 19, 2021

Choose a reason for hiding this comment

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

What's the difference?

As you mentioned before, if we use the previous Block hash value as the feed, the primary might be able to control the value if the current consensus occurs view change, therefore, we should use the hash value of a block that is far away from the current index to make sure the dishonest primary could not know what the future Blockchain status is gonna be when it generates that hash value. https://github.com/neo-project/neo-modules/blob/f14c1a9d214bbdff43bb37ab33a09d76fc8c16d5/src/DBFTPlugin/ConsensusService.cs#L433

Copy link
Member

Choose a reason for hiding this comment

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

But the previous (validators.Length/3 + 1) block also has the opportunity to have the problem.

Copy link
Member

Choose a reason for hiding this comment

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

We should look backwards for the nearest block with a different primaryIndex.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We should look backwards for the nearest block with a different primaryIndex.

Now I use the hash of the previous 1000th Block. In this way, no primary could know what the future status would be when it generates Block.

@shargon shargon merged commit 1b2f912 into neo-project:master May 19, 2021
@shargon
Copy link
Member

shargon commented May 19, 2021

Miss click sorry

@erikzhang
Copy link
Member

The changes to the master branch have been reverted. @Liaojinghui Can you create a new PR for it?

@Jim8y
Copy link
Contributor Author

Jim8y commented May 19, 2021

Created a new pr in #2476

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.

None yet

3 participants