Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

research: refactor adapter/wallet initialize() #431

Closed
wants to merge 3 commits into from

Conversation

mrnerdhair
Copy link
Contributor

  • Makes initialize() on adapters private and removes unnecessary dummy implementations
  • Makes initialize() on wallets always return a boolean
  • On adapters with do have initialize(), it consistently turns a single device into a wallet. pairDevice() now picks the default device and delegates to wallet creation to initialize().

@vercel
Copy link

vercel bot commented Feb 12, 2022

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/shapeshift/hdwallet/5FxCTFauUujVVxodjgwoKdQUqxhT
✅ Preview: https://hdwallet-git-initialization-refactor-shapeshift.vercel.app

@mrnerdhair
Copy link
Contributor Author

for reference, I've checked axiom and platform-shared and neither use the return value of initialize() at all (beyond awaiting it).

@mrnerdhair mrnerdhair added backwards-compatible reopen-later Stuff that's good to go but not a priority at the moment. labels Mar 4, 2022
@mrnerdhair mrnerdhair closed this Mar 5, 2022
@mrnerdhair mrnerdhair added this to the Low Priority milestone Mar 5, 2022
@mrnerdhair mrnerdhair mentioned this pull request Apr 7, 2022
@mrnerdhair mrnerdhair removed the reopen-later Stuff that's good to go but not a priority at the moment. label Apr 11, 2022
@mrnerdhair mrnerdhair reopened this Apr 11, 2022
@mrnerdhair mrnerdhair marked this pull request as draft April 11, 2022 16:43
@mrnerdhair mrnerdhair changed the title refactor adapter/wallet initialize() research: refactor adapter/wallet initialize() Apr 11, 2022
@mrnerdhair
Copy link
Contributor Author

I've reopened this as a draft; will be doing some refactoring to work towards making this non-breaking. My goal is to get to an Adapter<WalletType extends HDWallet> type in core to replace e.g. https://github.com/shapeshift/web/blob/60ffe3ffaa2e2198c72ad475dabbdcaa88d65819/src/context/WalletProvider/WalletProvider.tsx#L20-L23

@mrnerdhair mrnerdhair removed this from the Low Priority milestone Apr 12, 2022
@0xean
Copy link
Contributor

0xean commented Jan 23, 2024

closing as stale draft PR

@0xean 0xean closed this Jan 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

wallet.initialize() and adapter.initialize() work inconsistently across wallet implementations
2 participants