feat(keystore): Implement eth2util/keystore#218
Conversation
iamquang95
changed the title
|
|
||
| /// A share in the context of a Charon cluster, alongside its index. | ||
| #[derive(Debug, Clone, PartialEq, Eq)] | ||
| pub struct IndexedKeyShare { |
There was a problem hiding this comment.
Shouldn't it be in the eth2util/keystore (as it's done in the go implementation)?
There was a problem hiding this comment.
It's because of the cyclic dependency, the cluster depends on eth2utils
And to implement these methods in eth2utils it has to depend on cluster
Anw, these methods are all cluster related, then I move them to the cluster
| /// validator public key associated to no keyshare. | ||
| pub fn keyshares_to_validator_pubkey( |
There was a problem hiding this comment.
Also seems like it should be in the eth2util/keystore
varex83
left a comment
varex83
left a comment
There was a problem hiding this comment.
Will take a look at crypto part later, after checking the specs
emlautarom1
left a comment
emlautarom1
left a comment
There was a problem hiding this comment.
Generally LGTM, some small things only due to potential concurrency issues.
I did not review keystorev4 since it's a bit outside my set of skills. Couple of questions on this:
- Did you come up with this implementation?
- If not, from where?
- Do you know anyone who could take a look at it and validate it?
I'm leaning on the fact that we've ported tests from Go to claim that it's OK but again I cannot fully validate it myself.
As in the description, the |
emlautarom1
left a comment
emlautarom1
left a comment
There was a problem hiding this comment.
@iamquang95 I missed the link in the PR description, my bad! All good then.
3 participants
Fix: #167
This include: https://github.com/NethermindEth/charon-rs/pulls
Notice that crypto dependencies need to be built with optimize level 3, otherwise the test will be really slow (~20s)
The
keystorev4is based on https://github.com/sigp/lighthouse/tree/stable/crypto/eth2_keystore and https://eips.ethereum.org/EIPS/eip-2335