-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Conversation
Introduce timestamp into new key files; keep filename around, so that we don't accidentally duplicate keys.
fn insert(&self, account: SafeAccount) -> Result<(), Error> { | ||
self.dir.insert(account) | ||
fn insert(&self, account: SafeAccount) -> Result<SafeAccount, Error> { | ||
self.dir.insert(account.clone()) |
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.
clone unnecessary?
@@ -15,6 +15,8 @@ rustc-serialize = "0.3" | |||
rust-crypto = "0.2.36" | |||
tiny-keccak = "1.0" | |||
docopt = { version = "0.6", optional = true } | |||
time = "0.1.34" | |||
log = "0.3" |
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.
Is this used? I cant't see any logging added
…y into timestamp-in-key-filenames
@@ -35,16 +36,17 @@ pub struct SafeAccount { | |||
pub version: Version, | |||
pub address: Address, | |||
pub crypto: Crypto, | |||
pub path: Option<PathBuf>, |
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.
What do you think about just storing a filename: Option<String>
instead? It will allow migrations between two DiskDirectories
- right know accounts with path.is_some()
can basically come from anywhere on the disk and whole DiskDirectory
is kind of useless and misleading.
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.
a potential improvement, sure, but not strictly necessary for this feature.
Introduce timestamp into new key files; keep filename around, so
that we don't accidentally duplicate keys.