feat:wallet v1 (a.k.a Hub) #623

yeager-eren · 2024-03-04T06:20:04Z

Introducing Hub

Background

Initially, We developed the first version of our wallet management to meet our internal system demands. Over time, Our library has evolved to support approximately 35 wallets, with more on the horizon.

This period of the development has provided invaluable insights into the nuances of wallet management, revealing accessibility and functionality across different wallet types.

Despite the success of initial implementation, We recognized the need for a more robust solution to address emerging challenges and to accommodate future growth. Our objectives for the new version are:

Enhanced Flexibility: We aimed to redesign the core architecture from the ground up, enabling greater adaptability to diverse wallets
Improved Observability: Give more insightful and debugging information by using our new logging system.
Optimized Performance: Enabling asynchronously loading of providers to reduce load time.
Simplified Usage: Simpler and safer way to implement providers using builder pattern and Typescript.
Standard implementations: If any blockchains have some standards way to interact, we have them in core and can be imported.
Increased Safety: We've covered the library with automated tests to ensure that future changes don't break anything and also the usage has been covered by type checking.
Expanded Feature Set: We can now implement more features for work and interact beyond connecting and signing actions. we can implement more backends except Rango that have more features to work with blockchain nodes (e.g Infura or wallet itself).
Versioning: We can change core interface without breaking the wallets since core now supports for versioning which helps us to choose the desired behavior based on each version.

Moreover, we recognized the importance of making our library beyond our internal systems. So a key objective is to decouple it from a specific infrastructure dependencies (Rango), transforming it to a public library to be used throughout the entire crypto space .

Technical details

All the technical details can be found in /wallets/core/docs.

Known issues

andUse has a type issue. Details: wallets/core/src/builders/namespace.ts.
react-docgen has been disabled during a bug they have with class private fields.

For reviewer

How to review

I suggest the following steps:

Read migration-guide.md and starts to migrate a provider. Choose an EVM provider for start.
Give feedback on your experience, it can be on the documentation, it can be on interface or any problem you've encountered with.
Watch my internal presentation on this PR.
Read technical docs
Read notable changes on this PR's description
Start to review by reading tests
Start review core
Start review the adapter and what I did to be compatible with legacy on walltets-react.
Write some of tests which flagged as todo.

Notable Changes

1. Migrating to latest ESM format

We are going to adopt the latest ES Modules standards.

Package entry points

ESM now supports defining multiple entry points for a package. Entry points will be defined in package.json and exports field.

For example, our wallets-core has several entry points:

import x from '@rango-dev/wallets-core'
import x from '@rango-dev/wallets-core/utils'
import x from '@rango-dev/wallets-core/legacy'
...

Read more about the format: https://nodejs.org/api/packages.html#package-entry-points

File extension

File extension is required which means import x from "./x.js" should be used instead of import x from './x'

tsconfig changes

These two options should be added into tsconfig

{
  "module": "NodeNext",
  "moduleResolution": "NodeNext"
}

It also needed to library users to add this option to their tsconfig if they want to use exports from a lib:

{
"moduleResolution": "Bundler"
}

I tried to migrate embedded to use exports field as well, but it needs lots of change. I solved it by by supporting both legacy and new format at the same time for wallets-core.

Only wallets-core migrated to NodeNext for now, but we can do migrate more packages in future after merging this changes.

Changes to build script

our scripts/build now accepts --inputs to accept more than one entry point. This is useful for supporting exports.

New package layout

As you can see I intentionally changed the index.ts to mod.ts, the reason behind this:

Explicitly show the package migrated to NodeNext.
It can be useful for supporting more runtimes/registries like JSR and Deni.
We can have some enforcement on this structure and explicitly define some rules to clearly communicate what a module is and how they can be exported.

Note: Our libraries will use this format, our clients don't need to follow this conventions.

2. Two `tsconfig` for each Package

We put our integration in tests directory besides src. If we consider the package root as entry point for typescript, it ruins the dist folder by including both dist/src/ and dist/tests. In summary, we have two phase: distributing package and running code (tests or on local). I couldn't find any straight forward approach to solve this issue.

One of workaround for this is having two tsconfig, When we are going to run tests or developing locally use tsconfig.json which is default and besides that, use tsconfig.build.json when we are going to build a package and publish the package. It's been added to /scripts/build.

3. Legacy vs Hub (or v1)

What we have on production is called legacy from now on and we call the new implementation hub which is going to be our first stable and final version for wallet management.

you can see a directory called /legacy inside provider-phantom, wallets-core and wallets-react. Most of them are moving old implementation into a separate directory with same filename.

What to test/check

Experimental wallets is an option which is only enabled in dev mode. If you are testing it locally, make sure you are checking disabled mode as well.
We accept providers from config (WidgetConfig) they should works as well.
There is a discoverNamespace in wallets/react/src/hub/utils.ts, that would be great to make sure i didn't miss anything.

Roadmap

Adding more namespaces like bitcoin, cosmos, ... to core.
Migrating all providers to hub and remove all legacy implementations.
Remove legacy from core.
Migrate embedded to use core directly and remove most of the react functionalities.
Adding signers functionality to core as actions and depreacte those libraries.
We can create a separate repository and move out wallets to the repository.
Launching a doc website using docusaurus or nextra for public usage.

Related PRs

…namespaces

RanGojo · 2024-06-18T05:44:31Z

wallets/core/docs/hub.md

+// Call connect on evm namespace of phantom provider 
+phantomProvider.get('evm').connect('0x1');
+```


Please add to document how could we connect to a provider. (all included namespaces)

wallets/provider-all/src/index.ts

RanGojo · 2024-06-18T05:50:59Z

wallets/provider-all/src/index.ts

+import {
+  legacyProviderImportsToVersionsInterface,
+  type Versions,
+} from '@rango-dev/wallets-core/utils';


Is there any reason that this is not exported from wallet-core root? Also there are several similar cases if you search.

importing using @rango-dev/wallets-core/utils is using ESM's exports feature which is what we use from now on. We only directly use @rango-dev/wallets-core whenever the environment doesn't support this feature. Currently, embedded is the only env that doesn't support for this. In future we will migrate that one as well since it needs some major changes and is out of scope of this task.

RanGojo · 2024-06-18T06:03:04Z

wallets/provider-argentx/src/index.ts

-} from '@rango-dev/wallets-shared';
+} from '@rango-dev/wallets-core/legacy';
 import type { BlockchainMeta, SignerFactory } from 'rango-types';

-import { Networks, WalletTypes } from '@rango-dev/wallets-shared';
+import { Networks } from '@rango-dev/wallets-core/legacy';
+import { WalletTypes } from '@rango-dev/wallets-shared';


For some legacy providers like argentx, you've changed these imports but for most of them, you didn't. I think we should update all if there was a reason behind these changes.

First I tried to remove Networks from shared but it affected many places, and in some places core wasn't available. I decided to keep Networks in shared but it actually is a re-export of wallets-core (wallets/shared/src/rango.ts:19).

So using import { Networks } from '@rango-dev/wallets-core/legacy' or import { Networks } from '@rango-dev/wallets-shared'; basically identical. To avoid put too much effort on changing this, I gave up on changing them in this PR. This change can be continue when we are migrating legacy providers to Hub.

If you think they shouldn't be changed, I can revert back import { Networks } from '@rango-dev/wallets-core/legacy'; to use shared as before.

widget/embedded/src/components/WalletNamespacesModal/WalletNamespacesModal.types.tsx

RanGojo · 2024-06-18T07:20:36Z

wallets/provider-phantom/src/legacy/helpers.ts

+export function evmPhantom() {
+  const instances = phantom();
+
+  const evmInstance = instances?.get(Networks.ETHEREUM);


I think this produces a lot of duplicate code if we want to create a function for each provider-blockchain of each provider based on this approach. I think we should avoid this approach.

You also used these functions in legacy code in v1 provider.

I moved them to src/utils.ts since they will be used in Hub even legacy being removed.

I tried to make phantom, evmPhantom and solanaPhantom into one function but it doesn't improve the code. phantom() is using to detect wether the wallet is installed or not and also it's used in legacy implementation.

evmPhantom and solanaPhantom are using phantom to access to each namespace separately and throw an error if they are not available. this is because they are using inside hub's namespaces and they should throw error if they are not available. And also they are reusing in namespace, for example, evmPhantom is passing to changeAccountSubscriber and connect separetly.

By removing those function the provider-phantom/namespaces/evm.ts will look like this:

const [changeAccountSubscriber, changeAccountCleanup] = actions.changeAccountSubscriber(() => { const evmInstance = phantom()?.get(Networks.ETHEREUM) || undefined; if (!evmInstance) { throw new Error( 'Are you sure Phantom injected and you have enabled evm correctly?' ); } return evmInstance; }); const evm = new NamespaceBuilder<EvmActions>('evm', WALLET_ID) .action('init', () => { console.log('[phantom]init called from evm cb'); }) .action([ ...actions.recommended, actions.connect(() => { const evmInstance = phantom()?.get(Networks.ETHEREUM) || undefined; if (!evmInstance) { throw new Error( 'Are you sure Phantom injected and you have enabled evm correctly?' ); } return evmInstance; }), ]) ...

RanGojo · 2024-06-18T07:21:12Z

wallets/provider-phantom/src/legacy/helpers.ts

+export function solanaPhantom() {
+  const instance = phantom();
+  const solanaInstance = instance?.get(Networks.SOLANA);
+
+  if (!instance || !solanaInstance) {
+    throw new Error(
+      'Are you sure Phantom injected and you have enabled solana correctly?'
+    );
+  }
+
+  return solanaInstance;


almost duplicate as previous function.

Let's keeping the discussion here: #623 (comment)
I will update this part as well if needed.

RanGojo · 2024-06-18T07:24:46Z

wallets/provider-phantom/src/namespaces/evm.ts

+utils.apply(
+  'before',
+  [...before.recommended, ['connect', changeAccountSubscriber]],
+  evm
+);
+utils.apply(
+  'after',
+  [...after.recommended, ['disconnect', changeAccountCleanup]],
+  evm
+);


I think the logical way is to inject these methods using namespace builder (evm object) not using util.

wallets/core/src/namespaces/common/actions.ts

RanGojo · 2024-06-18T07:46:26Z

wallets/provider-phantom/src/namespaces/evm.ts

+const evm = new NamespaceBuilder<EvmActions>('evm', WALLET_ID)
+  .action('init', () => {
+    console.log('[phantom]init called from evm cb');
+  })
+  .action([...actions.recommended, actions.connect(evmPhantom)])
+  .andUse(and.recommended)
+  .build();


I like the idea of using builders but the main problem for me is that if I want to write a new provider, I should go and find what we have in code or documentation and don't know what should be implemented.
We don't have a self-explanatory interface that asks us what needs to be implemented.

The other problem for me is that I can't understand what is this code doing without clicking on each function one by one to see their implementation details.

The other problem is that the life-cycle of provider and related hooks are ambiguous for me.

As we've talked offline, each action can be imported from core and pass one by one using .action to builder. This is the explicit way and we already have support for it, another approach is use recommended values from core which basically are actions that can be used in all providers. they are crafted and maintained carefully so without needing any extra efforts to update provider package (e.g phantom), it can receives some recommended actions at anytime easily.

This is my suggestion on how you could improve this approach by having before, after and other hooks composed with action in single object instead of separating them and importing duplicate stuff everywhere.
You could still have separate functions to maintain separation of concerns, but you could export them together as well like this instead of exporting them separately as several recommended methods which is not meaningful as it could produce mistake if a developer forgets to do something and also reduces code readability.

This is not the exact syntax but you could get the idea.

const connect = new ActionBuilder('connect', () => {}) .('after', () => {}) // or .after(() => {}) .('before', () => {}) .('and', () => {}) .build() const disconnect = new ActionBuilder('disconnect', () => {}) .('after', () => {}) .('before', () => {}) .('blablabla', () => {}) .build() const evm = new NamespaceBuilder<EvmActions>('evm', WALLET_ID) .action('init', () => {}) .action([connect, disconnect]) .build();

I created a new builder called ActionBuilder to achieve what you've proposed here. I only applied it to evm namespace for Phantom. If the approach has been finalized, I will update solana namespace for phatom as well.

yeager-eren · 2024-06-19T14:19:11Z

Please check why phantom is not included here. (in confirm wallet step)

Phantom has support for Polygon and Ethereum on EVM only. You've tried to connect BSC.

…en triggering accountChange

RanGojo · 2024-06-20T11:35:43Z

wallets/core/src/namespaces/evm/actions.ts

+export function connect(
+  instance: () => ProviderApi
+): ['connect', FunctionWithContext<EvmActions['connect'], Context>] {
+  return [
+    'connect',
+    async (_context, chain) => {
+      const evmInstance = instance();


Can you elaborate how eager connect is implemented in EVM or Solana? I wasn't able to find related code.

Please take a look at wallets/react/src/hub/useAutoConnect.ts for hub, and wallets/react/src/legacy/useAutoConnect.ts for legacy.

…ction in one place

RyukTheCoder · 2024-07-07T14:32:32Z

When I install a fresh phantom extension and no account is added to the extension, the injected phantom object contains only solana and it does not have any ethereum property. So when I run the project, I immediately get the following error:

RyukTheCoder · 2024-07-07T14:41:41Z

When I cancel the connect popup in phantom extension, no error shows up in widget containing the error message.

RyukTheCoder · 2024-07-07T14:45:22Z

Every time I connect or disconnect a wallet in wallets page, all the wallet items get rerendered resulting in fetching all the icons of wallet items:

RyukTheCoder

I recommend to use provider everywhere instead o fwallet.

RyukTheCoder · 2024-07-07T15:32:07Z

wallets/provider-phantom/src/provider.ts

+  .init(function (context) {
+    const [, setState] = context.state();
+
+    if (phantomInstance()) {


Suggested change

if (phantomInstance()) {

if (getPhantomInstance()) {

RyukTheCoder · 2024-07-09T13:51:46Z

wallets/provider-phantom/src/utils.ts

+  const evmInstance = instances?.get(Networks.ETHEREUM);
+
+  if (!instances || !evmInstance) {
+    throw new Error(


You are not supposed to throw error if the instance is not available. Throwing error should be done on handling an action from user like connecting and signing transactions.

RyukTheCoder · 2024-07-09T14:04:37Z

wallets/provider-phantom/src/utils.ts

+  return instances;
+}
+
+export function evmPhantom(): EvmProviderApi {


Suggested change

export function evmPhantom(): EvmProviderApi {

export function getNetworkInstance(network: Networks) {

const instance = getProviderInstance();

const networkInstance = instance?.get(network);

if (!networkInstance) {

throw new Error(

`Are you sure ${WALLET_ID} injected and you have enabled ${network} correctly?`

);

}

return networkInstance;

}

… Action class

yeager-eren changed the title ~~wip:wallet v2~~ !WIP feat:wallet v2 Mar 4, 2024

yeager-eren requested review from RanGojo and mikasackermn as code owners March 24, 2024 08:41

This was referenced Apr 2, 2024

!WIP feat: add selectable chain connect modal #660

Closed

feat(wallets): add evm to phantom #349

Closed

yeager-eren force-pushed the feat/hub-first-iteration branch from bd7418c to 832116c Compare April 3, 2024 10:49

yeager-eren requested review from Ikari-Shinji-re and nikaaru as code owners May 31, 2024 13:39

yeager-eren force-pushed the feat/hub-first-iteration branch 2 times, most recently from 543f538 to dbd0832 Compare May 31, 2024 14:27

yeager-eren deployed to storybook-preview June 1, 2024 04:09 — with GitHub Actions View deployment

yeager-eren deployed to playground-preview June 1, 2024 04:09 — with GitHub Actions View deployment

yeager-eren deployed to app-preview June 1, 2024 04:09 — with GitHub Actions View deployment

yeager-eren requested a review from samobasquiat as a code owner June 8, 2024 08:39

yeager-eren changed the title ~~!WIP feat:wallet v2~~ !WIP feat:wallet v1 Jun 8, 2024

yeager-eren changed the title ~~!WIP feat:wallet v1~~ !WIP feat:wallet v1 (a.k.a Hub) Jun 8, 2024

yeager-eren force-pushed the feat/hub-first-iteration branch 4 times, most recently from 14dad39 to 69e9625 Compare June 12, 2024 10:17

feat: introducing hub for wallet management

6b107e7

yeager-eren force-pushed the feat/hub-first-iteration branch from 69e9625 to 6b107e7 Compare June 12, 2024 12:34

yeager-eren deployed to storybook-preview June 12, 2024 12:43 — with GitHub Actions View deployment

yeager-eren deployed to app-preview June 12, 2024 12:43 — with GitHub Actions View deployment

yeager-eren deployed to playground-preview June 12, 2024 12:43 — with GitHub Actions View deployment

fix: fix storing all accounts if they are passing separetely through …

8142da8

…namespaces

yeager-eren changed the title ~~!WIP feat:wallet v1 (a.k.a Hub)~~ feat:wallet v1 (a.k.a Hub) Jun 12, 2024

yeager-eren deployed to storybook-preview June 12, 2024 16:39 — with GitHub Actions View deployment

yeager-eren deployed to app-preview June 12, 2024 16:39 — with GitHub Actions View deployment

yeager-eren deployed to playground-preview June 12, 2024 16:39 — with GitHub Actions View deployment

yeager-eren deployed to storybook-preview June 14, 2024 15:00 — with GitHub Actions View deployment

yeager-eren added 3 commits June 16, 2024 08:00

fix: removing feature and move it hooks. also and accepts a list

d572c31

test: cover after/before accepting a list

a3bfa2e

fix: connecting state

28cc674

RanGojo reviewed Jun 18, 2024

View reviewed changes

This comment was marked as resolved.

Sign in to view

yeager-eren added 2 commits June 18, 2024 15:39

feat: add hook and fix cleanup on failed connect action

30260ec

fix: mostly renaming stuff to more appropriate ones

781ff9a

yeager-eren force-pushed the feat/hub-first-iteration branch from 2338595 to 781ff9a Compare June 19, 2024 13:50

fix: removing connect's fixed namespace and add a check for solana wh…

1afcf4b

…en triggering accountChange

RanGojo reviewed Jun 20, 2024

View reviewed changes

feat: add ActionBuilder to keep all the related functionality of an a…

6453314

…ction in one place

yeager-eren force-pushed the feat/hub-first-iteration branch from f723f83 to 6453314 Compare July 2, 2024 10:36

RyukTheCoder requested changes Jul 9, 2024

View reviewed changes

yeager-eren added 8 commits July 10, 2024 08:19

chore: migrate phantom's solana to ActionBuilder

7ef7128

fix: fix crash on first load

3cbae42

fix: missing passing after and before hooks to namespace

afab041

feat: and can be list now

7f8050b

feat: or accepts list now

aa5398c

chore: simplify builders, remove andUse & orUse from Provider, remove…

7d307b8

… Action class

fix: disconnect phantom when pk is null

92aca83

fix: not throwing error on request rejections

36981c7

This was referenced Aug 16, 2024

chore: restructure shared and react to use /core package #838

Merged

feat: introducing hub, our new wallet management #826

Merged

This was referenced Oct 1, 2024

feat: hub adapter and refactor wallets store #852

Open

test: add tests for wallets/react #824

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat:wallet v1 (a.k.a Hub) #623

feat:wallet v1 (a.k.a Hub) #623

yeager-eren commented Mar 4, 2024 •

edited

Loading

RanGojo Jun 18, 2024

RanGojo Jun 18, 2024

yeager-eren Jun 19, 2024

RanGojo Jun 18, 2024

yeager-eren Jun 19, 2024

RanGojo Jun 18, 2024

RanGojo Jun 18, 2024 •

edited

Loading

yeager-eren Jun 19, 2024

yeager-eren Jun 19, 2024 •

edited

Loading

RanGojo Jun 18, 2024

yeager-eren Jun 19, 2024

RanGojo Jun 18, 2024

RanGojo Jun 18, 2024

yeager-eren Jun 19, 2024

RanGojo Jun 20, 2024 •

edited

Loading

yeager-eren Jul 2, 2024

This comment was marked as resolved.

yeager-eren commented Jun 19, 2024

RanGojo Jun 20, 2024

yeager-eren Jul 2, 2024

RyukTheCoder commented Jul 7, 2024

RyukTheCoder commented Jul 7, 2024

RyukTheCoder commented Jul 7, 2024

RyukTheCoder left a comment

RyukTheCoder Jul 7, 2024

RyukTheCoder Jul 9, 2024

RyukTheCoder Jul 9, 2024

-export function evmPhantom(): EvmProviderApi {
+export function getNetworkInstance(network: Networks) {
+  const instance = getProviderInstance();
+  const networkInstance = instance?.get(network);
+  if (!networkInstance) {
+    throw new Error(
+      `Are you sure ${WALLET_ID} injected and you have enabled ${network} correctly?`
+    );
+  }
+  return networkInstance;
+}

feat:wallet v1 (a.k.a Hub) #623

Are you sure you want to change the base?

feat:wallet v1 (a.k.a Hub) #623

Conversation

yeager-eren commented Mar 4, 2024 • edited Loading

Introducing Hub

Background

Technical details

Known issues

For reviewer

How to review

Notable Changes

1. Migrating to latest ESM format

Package entry points

File extension

tsconfig changes

Changes to build script

New package layout

2. Two tsconfig for each Package

3. Legacy vs Hub (or v1)

What to test/check

Roadmap

Related PRs

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RanGojo Jun 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yeager-eren Jun 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RanGojo Jun 20, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment was marked as resolved.

yeager-eren commented Jun 19, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RyukTheCoder commented Jul 7, 2024

RyukTheCoder commented Jul 7, 2024

RyukTheCoder commented Jul 7, 2024

RyukTheCoder left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yeager-eren commented Mar 4, 2024 •

edited

Loading

2. Two `tsconfig` for each Package

RanGojo Jun 18, 2024 •

edited

Loading

yeager-eren Jun 19, 2024 •

edited

Loading

RanGojo Jun 20, 2024 •

edited

Loading