-
Notifications
You must be signed in to change notification settings - Fork 36
Add BLST utility library #1152
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
base: bat/validator-set-registry-stubs
Are you sure you want to change the base?
Add BLST utility library #1152
Conversation
… RPC clients directly when possible
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.
Semgrep PRO found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
2ed633c to
e54ab79
Compare
michaelkaplan13
left a comment
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 should add a clear comment at the top of all these contracts that they are unaudited code that should not be used in production currently.
|
I tried to find an example of the disclaimer we had on unaudited contracts, but having trouble finding it. I think we might have moved away from disclaimers in every contract and opted instead for a disclaimer in the README in the folder for ValidatorManager when it was still under development. https://github.com/ava-labs/icm-contracts/blob/60e749e8c3b00e5698b75f2e2bc0fbed5f01b55d/contracts/validator-manager/README.md Also happy if you think it would be better directly in the contracts @michaelkaplan13 |
|
Curious but how did you generate the signatures, public keys, etc. to check against? i.e. the ground truth values |
Great question. I wrote the base version of these tests so long ago, I don't remember. I'll try to run that down for you. |
For ValidatorSetSig we did have this snippet in the code itself: https://github.com/ava-labs/icm-contracts/blob/14a958431c0bfece6019ed563a264fb2b13e0d66/contracts/governance/ValidatorSetSig.sol#L14 I'm open to either but readers should be able to easily distinguish between audited and non-audited code |
I have added the disclaimer in the code itself. Hopefully that suits everyone |
6e0f822 to
9e27cc9
Compare
Why this should be merged
This adds a library for verifying BLS signatures in Ethereum contracts. Needed for the
AvalancheValidatorSetRegistryHow this works
How this was tested
How is this documented