Feat/ledger signing #1278
Feat/ledger signing #1278
Conversation
so that is can use the ledger-transport::Exchange trait for the transport which is async using this trait allows us to use the Zemu transport which can connect to the Speculos emulator
still need to handle errors properly
i needed to look at the js implementation again (hw-app-str) - they have a note that says to pass the `signatureBase` into the `signTransaction` fn. I had forgotten about this. The `signatureBase` fn is defined in js-stellar-base and is the value that should be sent to the network. it is TransactionSignaturePayload.toXDR(). Which is also in the signer train and i didn't notice it. 🙈
Currently don't support signing transaction, just its hash or any blob
Co-authored-by: Elizabeth Engelman <4752801+elizabethengelman@users.noreply.github.com>
Also clean up based on PR review
feat: add blob signer and simplify
elizabethengelman
force-pushed
the
feat/ledger-signing
branch
from
03c891f
to
180757b
Compare
Co-authored-by: Chad Ostrowski <221614+chadoh@users.noreply.github.com>
| impl ImageArgs for Args { | ||
| fn into_iterator(self) -> Box<dyn Iterator<Item = String>> { | ||
| let container_elf_path = format!("{DEFAULT_APP_PATH}/stellarNanosApp.elf"); | ||
| let command_string = format!("/home/zondax/speculos/speculos.py --log-level speculos:DEBUG --color JADE_GREEN --display headless -s \"other base behind follow wet put glad muscle unlock sell income october\" -m nanos {container_elf_path}"); |
There was a problem hiding this comment.
Is the speculos file only used during testing? What's the significance of this hardcoded 12-word passphrase?
There was a problem hiding this comment.
speculos.rs is only used in testing - like I mentioned in the other comment, I'm thinking I should create a tests directory or something to put all test-only related files in there to make this more clear.
The hardcoded passphrase is the seed phrase for the account on the emulated device. It probably makes sense to pull this out into a constant.
I can make these changes in a follow-up PR!
There was a problem hiding this comment.
Ah got it. Yup makes sense.
We've kept tests in other repos in a tests/test module. Rust doesn't require it, but keeping tests in a separate module reduces rebuilds, supposedly, in large rust projects, which is why we use that pattern in other repos. This repo isn't big enough to see any benefit of that though, so not something we need to do. But moving stuff like this only used by tests makes sense.
There was a problem hiding this comment.
Just to close the loop, this PR makes this update: #1343
What
Add support for signing transactions on a Ledger device.
#1178
Why
Work toward key protection when submitting txns from the CLI. (#1255)
Known limitations
This does not implement the full
Stellarsigner trait yet. The stellar app deployed on the ledger device does not yet support signing soroban auth entries yet, so in the mean time we will only be able to do hash signing for soroban tx.Todo
txn sign#1311