Keystore V4 support #18
Conversation
Still a bit left to do, but I'd like some feedback on the structure and some of the pending changes
| match v["version"].as_i64() { | ||
| // TODO tests/test-key/my_keystore has no version field | ||
| // should we default to v3 to preserve compatibility, or raise an error? | ||
| Some(3) | None => { |
There was a problem hiding this comment.
this here was one of my main questions. Without Nnoe, some of the existing tests fail
There was a problem hiding this comment.
How about we keep the current structure of the project, while introducing a new feature-gated module v4 that has a separate EthKeystoreV4 struct.
Essentially, we don't make use of an Enum with the V3/V4 variants, but a separate struct altogether, which will also make serde issues less complicated?
|
@roynalnaruto does that mean we also have separate decrypt/encrypt functions for v4? I was assuming that was not desirable, beucase it forces the caller to have knowledge of the type of keystore being used. Or maybe I'm missing something obvious here |
I am still thinking about the design. I will get back to you on this. |
|
@roynalnaruto any update on this? |
This is a follow-up to #17 that attempts to preserve v3 functionality.
I'm not sure about some of the decisions made here, so I'm asking for some feedback before finishing the work
Summary of changes:
EthKeystorebecomes an enum, with both V3 and V4 variantscrypto.macfield in v4)questions:
Modulespec. However, the approach in Bump to version 4 following test vecs here: https://eips.ethereum.org/EIPS/eip-2335 #17 used more fine-grained structs for each one. I've got no clue which approach should be followed here