Add tally wallet #506
Add tally wallet #506
Conversation
|
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/shapeshift/hdwallet/2wqytyTi8cRS2wDwSwfHLjpkuqqa |
There was a problem hiding this comment.
LGTM, a few comments regarding references to Metamask that should be changed to Tally.
Suggesting changes as this doesn't build yet (running yarn && yarn build and cding into packages/hdwallet-tally there is no dist folder). An entry should be added in the root tsconfig https://github.com/shapeshift/hdwallet/blob/master/tsconfig.json#L63 :
diff --git a/tsconfig.json b/tsconfig.json
index 09798d13..19bdf295 100644
--- a/tsconfig.json
+++ b/tsconfig.json
@@ -60,6 +60,7 @@
{ "path": "./packages/hdwallet-portis" },
{ "path": "./packages/hdwallet-trezor" },
{ "path": "./packages/hdwallet-trezor-connect" },
- { "path": "./packages/hdwallet-xdefi" }
+ { "path": "./packages/hdwallet-xdefi" },
+ { "path": "./packages/hdwallet-tally" }
]
}Tested it out locally and it successfully builds a packages/hdwallet-tally/dist folder after this entry is added.
There was a problem hiding this comment.
- There's a bunch of quality improvements needed that haven't made it into the main hdwallet-metamask package yet; not your fault, but we do need to make sure we're not multiplying the maintenance problems via cut-n-paste.
- This will need unit and integration tests; you may find hdwallet-xdefi a good starting point for those.
|
@0xzoz Does the Tally wallet support offline signing? |
Co-authored-by: MrNerdHair <[email protected]>
Co-authored-by: MrNerdHair <[email protected]>
…into add-tally-wallet
There was a problem hiding this comment.
Addressed the comments and issues @0xdef1cafe @pastaghost. Please let me know if there is anything else
| return wallet; | ||
| } | ||
|
|
||
| public async detectTallyProvider(): Promise<TallyHoEthereumProvider | null> { |
There was a problem hiding this comment.
| public async detectTallyProvider(): Promise<TallyHoEthereumProvider | null> { | |
| /** | |
| * Tally works the same way as metamask. | |
| * This code is copied from the @metamask/detect-provider package | |
| * @see https://www.npmjs.com/package/@metamask/detect-provider | |
| */ | |
| private async detectTallyProvider(): Promise<TallyHoEthereumProvider | null> { |
cjthompson
dismissed stale reviews from mrnerdhair, gomesalexandre, 0xdef1cafe, and pastaghost
changes made
Preliminary work to be able to use the Tally Ho wallet in the Shapeshift app. The new tally-hdwallet package is mostly similar to the Metamask package but refactored for Tally.
Testing
Was built as a standalone wallet here and imported to in the Shapeshift UI. All tests succeeded. It was then modified to use the @shapeshiftoss namespace. I'm unsure how these packages are built but I'm guessing it's during the CI?