-
Notifications
You must be signed in to change notification settings - Fork 56
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
Force trusted setup if circuits have been modified #1029
Force trusted setup if circuits have been modified #1029
Conversation
config/default.js
Outdated
@@ -31,7 +31,6 @@ module.exports = { | |||
TXHASH_TREE_HASH_TYPE: 'keccak256', | |||
STATE_GENESIS_BLOCK: process.env.STATE_GENESIS_BLOCK, | |||
CIRCUITS_HOME: process.env.CIRCUITS_HOME || '/app/circuits/', | |||
ALWAYS_DO_TRUSTED_SETUP: process.env.ALWAYS_DO_TRUSTED_SETUP || false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We still want to preserve the ability to force a trusted setup
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done, reverted this change
const fileBuffer = fs.readFileSync(`./circuits/${circuit}`); | ||
const hcircuit = crypto.createHash('md5').update(fileBuffer).digest('hex'); | ||
|
||
const checkHash = await axios.post( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There needs to be a way for force a trusted setup. Also, what happens if ther eis not vk[i] defined?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I restored the ability to force trusted setup.
from the tests I did in the case vk[i] is undefined there was always also a difference on the hash, but for safety I added the vk[i] check, if undefined trusted setup is forced
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since you already have tests, can you also include those? or were they manual?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no, sorry but they were all manual tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some additional questions:
- What happens if there are new circuits where hash doesnt match?
in case new circuits were added: -if the hash is present in the file but does not match the one just calculated, the hash contained in the file is updated and the trusted setup is generated. -if, on the other hand, the hash is not present in the file, it is added to the file and the trusted setup is generated |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adding the getCircuitHash enpdoint :)
What does this implement/fix? Explain your changes.
The goal of this pr is to check if there have been any changes to the circuits and if so, go to force a trusted setup.
To do this is kept within the
/output/circuithash.txt
(in the worker), the list of all circuits with their hash.So it checks if the hash of the current circuit differs, or is not in the file, and in case a trusted setup is forced
Does this close any currently open issues?
Resolves #989