Refactor load consensus keys logic in node-data #3525
Conversation
d-sonuga
changed the title
There was a problem hiding this comment.
Instead of adding rusk-wallet to node-data it would be better to do the opposite (rusk-wallet is quite a big dependency to add to a light data crate)
So, instead of having node-data that does rusk_wallet::load_consensus_keys(&path_buf, &pwd)?; we should have rusk-wallet that does node_data::save_consensus_keys(....)
|
@herr-seppia, the point of moving the saving of consensus keys into the rusk-wallet is to put all the hashing and encryption of wallet-related data in the same place so that when changes like the ones required for this issue #3391 are made, changes will only have to be made in one place. |
|
The only thing I'm strongly opinionated is that we cannot add rusk-wallet as dependency to rusk (and to node-data) Furthermore. keep in mind that consensus keys storage should not be considered "wallet-related data" imo, but they're indeed "node-related data" |
d-sonuga
force-pushed
the
load-consensus-keys
branch
from
d816a11 to
f1f762e
Compare
| warn!("Your consensus keys are in the old format"); | ||
| warn!("Consider to export them using a new version of the wallet"); |
There was a problem hiding this comment.
Did you remove the legacy support?
If so, please add it to the changelog
|
|
||
| // export key-pair to disk | ||
| fs::write(path.with_extension("keys"), bytes)?; | ||
| node_data::bls::save_consensus_keys(&path, &filename, &pk, &sk, pwd)?; | ||
|
|
||
| Ok((path.with_extension("keys"), path.with_extension("cpk"))) |
There was a problem hiding this comment.
Assuming that the extensions are .keys and .cpk is error prone (chaning node_data will break this)
You could change node_data::bls::save_consensus_keys to return the paths and display them here
| use serde::{Serialize, Serializer}; | ||
|
|
||
| pub fn serialize<S: Serializer>(v: &[u8], s: S) -> Result<S::Ok, S::Error> { | ||
| let base64 = BASE64.encode(v); |
There was a problem hiding this comment.
Is the base64 dependency still used?
For #3514