Add STX support for Ledger accounts #212
Conversation
dhriaznov
requested review from
teebszet and
Imamah-Zafar
and removed request for
m-aboelenein
|
|
||
| return { address, publicKey: publicKey.toString('hex') }; | ||
| } | ||
|
|
||
| /** | ||
| * This function is used to sign a Stacks transaction with the ledger | ||
| * @param transport - the transport object with connected ledger device | ||
| * @param transaction - the transaction to sign | ||
| * @param transactionBuffer - the transaction to sign |
There was a problem hiding this comment.
was there a strong reason to change this interface and move the serialize call out of this function?
I noticed your web-extension PR didn't update confirmLedgerTransaction/index.ts L251 with the new param
and confirmStxTransaction/index.ts now does an extra .seralize() before calling this function
There was a problem hiding this comment.
Yeah, the serialization was added on the extension side because otherwise it threw an error:
Currently, confirmStxTransaction/index.ts makes the tx serialization and passes it to confirmLedgerTransaction/index.ts screen ready to be passed to this core function in a Buffer format
There was a problem hiding this comment.
right. basically because we have to pass around a serialized transaction between screens anyway, so this removes the multiple actions to serialize (for location.state) -> deserialize (for this function) -> serialize (within this function)
is that right?
There was a problem hiding this comment.
Yeah, that's right
|
Hey @teebszet, could you take a look once again please? |
|
teebszet
deleted the
denys/eng-2499-add-stx-support-for-ledger-accounts
branch
🔘 PR Type
📜 Background
We would like to support STX account import, regular transactions and incoming txs signing as well.
We already have some code for STX support for Ledger accounts, but we should update and test it.
Issue Link: #[ENG-2499]
Context Link (if applicable):
🔄 Changes
Does this PR introduce a breaking change?
Changes:
importStacksAccountFromLedgermethod logicsignLedgerStxTransactionmethod logicStacksRecipienttypeImpact:
🖼 Screenshot / 📹 Video
✅ Review checklist
Please ensure the following are true before merging: