new module for ledger related utils #193
Conversation
🦋 Changeset detectedLatest commit: 4749846 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #193 +/- ##
=======================================
Coverage 84.47% 84.47%
=======================================
Files 16 16
Lines 161 161
Branches 25 25
=======================================
Hits 136 136
Misses 25 25 ☔ View full report in Codecov by Sentry. |
packages/ledger/README.md
Outdated
| }; | ||
|
|
||
| const signDoc = createSignDoc(aminoMsg, fee, chainId, memo, account); | ||
| const broadcastResponse = await signAndBroadcast(app, path, signDoc, client, nativeAddress); |
There was a problem hiding this comment.
instead of adding a signAndBroadcast helper, can we create an offline signer that can be plugged into SigningStargateClient?
There was a problem hiding this comment.
Ok, attempted refactoring
packages/ledger/src/wallet/utils.ts
Outdated
| * @param memo transaction memo | ||
| * @param account account | ||
| */ | ||
| export const createSignDoc = (aminoMsg: AminoMsg, fee: StdFee, chainId: string, memo: string, account: Account) => { |
There was a problem hiding this comment.
can aminoMsg be extended to be a list of messages in case a user wants to sign multiple messages?
There was a problem hiding this comment.
With latest refactoring, might be obsolete
|
@besated Switched to offline signer, but am not sure about implementation. Please have a look |
packages/ledger/src/wallet/utils.ts
Outdated
| /** | ||
| * A signer implementation that uses a Ledger device to sign transactions | ||
| */ | ||
| export class LedgerOfflineAminoSigner implements OfflineAminoSigner { |
There was a problem hiding this comment.
nice, this is great! should we name this SeiLedgerOfflineAminoSigner so it's more specific?
packages/ledger/src/wallet/utils.ts
Outdated
| * | ||
| * @returns {Promise<{transport: Transport, app: SeiApp}>} transport and app instances | ||
| */ | ||
| export const createTransportAndApp = async () => { |
There was a problem hiding this comment.
nit: can we keep this in the utils.ts file and move LedgerOfflineAminoSigner into another file to make it cleaner (maybe called signer.ts)?
|
@besated addressed the latest comment, ptal |
Helper methods for working with ledger hardware wallet on Sei
Testing