feat/trezor: cache session on filesystem #747
Conversation
ethers-signers/src/trezor/app.rs
Outdated
| .join("cache") | ||
| .join("trezor.session"); | ||
|
|
||
| if let Ok(file) = fs::File::open(path) { |
There was a problem hiding this comment.
should we return a None here if the file does not exist?
and an error if reading the file failed?
There was a problem hiding this comment.
done
although in the beginning, i was thinking of issuing a warning, without halting the execution
ethers-signers/src/trezor/app.rs
Outdated
| let mut file = std::io::BufReader::new(file); | ||
| file.read(&mut session).map_err(|e| TrezorError::CacheError(e.to_string()))?; |
There was a problem hiding this comment.
don't think a BufReader is necessary here for 32bytes?
also read does not guarantee to fill the buf entirely.
there is https://doc.rust-lang.org/stable/std/io/trait.Read.html#method.read_exact instead, or alternatively File::read_to_end
ethers-signers/src/trezor/app.rs
Outdated
| path = path.join("trezor.session"); | ||
|
|
||
| let file = fs::File::create(path).map_err(|e| TrezorError::CacheError(e.to_string()))?; | ||
| let mut file = std::io::BufWriter::new(file); |
There was a problem hiding this comment.
same, if the session id is only 32byte, probably not worth it, fs::write should do just fine I believe
ethers-signers/src/trezor/app.rs
Outdated
| let path = env::current_dir() | ||
| .map_err(|e| TrezorError::CacheError(e.to_string()))? | ||
| .join("cache") | ||
| .join("trezor.session"); |
There was a problem hiding this comment.
Maybe we should pass a cache_dir to trezor when initializing it? Or have a global ~/.ethers-rs/trezor/cache/trezor.session file? Don't think we always want the current dir to be populated?
There was a problem hiding this comment.
added all options
- accepts
cache_diras a parameter - if none, then tries
home_dir - if none, then tries
current_dir - if err, exit
gakonst
force-pushed
the
feat/trezor-cache
branch
from
3c8fc1f to
6558260
Compare
Motivation
Sessions are used to "keep" the passphrase on the device in-between different calls. And while it was being reused inside the same
TrezorEthereumobject, once we instantiated a new one, we would lose the session.For example, different
forgeorcastcalls would always request the user to insert their passphrase again and again. With this change, it won't.Relevant information from sessions @ docs.trezor.io :
Sessions only exist in RAM and are lost when Trezor is disconnected.New session is started by calling Initialize with no arguments. The response is a Features message, which contains a 32-byte session_id. All subsequent commands happen within the given session.To resume a previous session (after creating a new one), call Initialize with a stored session_id as an argument. Attempt to resume an unknown session ID will transparently allocate a new session ID.Solution
Read/write
session_idto./cache/trezor.sessionPR Checklist