Simplify onboarding #231

grod220 · 2024-11-07T18:49:27Z

Closes #226
Closes #227
Closes #221

Lots of deletions related to onboarding flow
Removes wallet birthday from being a user-facing concept
Onboarding automatically chooses random rpc + frontend for user
Removes "Forgot password?" flow
Only generates 12 word seedphrases

grod220 · 2024-11-07T19:43:19Z

apps/extension/src/routes/page/index.tsx

-  if (!grpcEndpoint) {
-    return redirect(PagePath.SET_GRPC_ENDPOINT);
-  }
-  if (!frontendUrl) {
-    return redirect(PagePath.SET_DEFAULT_FRONTEND);
-  }


Checks no longer necessary as we now set all of these at once and not separate steps

grod220 · 2024-11-07T19:45:00Z

apps/extension/src/routes/page/onboarding/generate.tsx

@@ -65,36 +58,11 @@ export const GenerateSeedPhrase = () => {
              disabled={!reveal}
              text={phrase.join(' ')}
              label={<span className='font-bold text-muted-foreground'>Copy to clipboard</span>}
-              className='m-auto w-48'
+              className='m-auto'


UI bug fix. After clicking copy, it would jump to the left. This fixes that.

grod220 · 2024-11-07T19:45:42Z

apps/extension/src/routes/popup/login.tsx

          <p className='text-center text-muted-foreground'>
            Need help? Contact{' '}
            <a
              className='cursor-pointer text-teal hover:underline'
              target='_blank'
-              href='https://discord.com/channels/824484045370818580/1077672871251415141'
+              href='https://discord.com/channels/824484045370818580/1245108769143394457'


The support channel

grod220 · 2024-11-07T19:47:31Z

apps/extension/src/storage/base.ts

+   * Removes key/value from db (waits on ongoing migration). If there is a default, sets that.
   */
  async remove<K extends keyof T>(key: K): Promise<void> {
    await this.withDbLock(async () => {
-      await this.storage.remove(String(key));
+      // Prevent removing dbVersion
+      if (key === 'dbVersion') {
+        throw new Error('Cannot remove dbVersion');
+      }
+
+      const specificKey = key as K & Exclude<keyof T, 'dbVersion'>;
+      const defaultValue = this.defaults[specificKey];
+      if (defaultValue !== undefined) {
+        await this._set({ [specificKey]: defaultValue } as Record<K, T[K]>);
+      } else {
+        await this.storage.remove(String(specificKey));
+      }


Sort of a bug here. When removing from storage, we should 1) ensure the consumer is not deleted the version key and 2) if it has a default (like say an empty array) we should set it as that and not undefined. This caused a bug while I was working on this PR.

TalDerei · 2024-11-07T21:16:18Z

apps/extension/src/hooks/latest-block-height.ts

-// Fetch the block height from a specific RPC endpoint with a request-level timeout that superceeds
-// the channel transport-level timeout to prevent hanging requests.


suggestion: could we add this comment back? It’s helpful for contextualizing the DEFAULT_TRANSPORT_OPTS timeout.

TalDerei · 2024-11-07T21:21:40Z

apps/extension/src/hooks/latest-block-height.ts

+  const { appParameters } = await createPromiseClient(
+    AppService,
+    createGrpcWebTransport({ baseUrl: randomRpc.url }),
+  ).appParameters({}, DEFAULT_TRANSPORT_OPTS);
+  if (!appParameters?.chainId) {
+    throw new Error('No chain id');


suggestion: move the numeraires selection logic after an rpc endpoint has been chosen and the block height has been successfully queried.

Can move the numeraires query down, but curious, what makes it better being lower than the other queries?

TalDerei · 2024-11-07T21:22:30Z

apps/extension/src/hooks/latest-block-height.ts

-  const result = await tendermintClient.getStatus({}, { signal: AbortSignal.timeout(timeoutMs) });
-  if (!result.syncInfo) {
-    throw new Error('No syncInfo in getStatus result');
+  if (seedPhraseOrigin === SEED_PHRASE_ORIGIN.NEWLY_GENERATED) {


suggestion: based on other comments below, maybe we should avoid guarding the grpc promise client logic by wallet type (seed versus imported) and instead apply the guard only to the wallet birthday height logic?

const walletBirthday = Number(result.syncInfo.latestBlockHeight); await localExtStorage.set('walletCreationBlockHeight', walletBirthday);

to also prevent setting a faulty grpc endpoint in local extension storage for imported wallets

await localExtStorage.set('grpcEndpoint', randomRpc.url);

which would similarly block onboarding by causing syncing to fail.

good idea 👍

TalDerei · 2024-11-07T21:27:50Z

apps/extension/src/hooks/latest-block-height.ts

+    const tendermintClient = createPromiseClient(
+      TendermintProxyService,
+      createGrpcWebTransport({ baseUrl: randomRpc.url }),
+    );
+    const result = await tendermintClient.getStatus({}, DEFAULT_TRANSPORT_OPTS);
+    if (!result.syncInfo) {
+      throw new Error('No syncInfo in getStatus result');
+    }


suggestion: I think we should revert to using react query (useLatestBlockHeightWithFallback) for randomly sampling the rpc. The dynamic block height fetching needs fallback support, using a recursive callback to try another endpoint if the current one fails. Currently, if a faulty rpc isn't queryable due to an unresponsive server or network error, it would cause onboarding to fail with an error. WDYT?

Don't think we need react query for this call (given there is not reliance upon the render lifecycle). However, it does feel like we already have this code to do retries and should use it. Will add 👍

TalDerei · 2024-11-07T23:25:25Z

apps/extension/src/shared/components/grpc-endpoint-form/use-grpc-endpoint-form.ts

@@ -138,6 +138,7 @@ export const useGrpcEndpointForm = (isOnboarding: boolean) => {
        setConfirmChangedChainIdPromise(undefined);
      }

+      await clearWalletCreationHeight(); // changing chain id means the wallet birthday is no longer valid


comment: manually tested across different chain Ids, and it properly clears the wallet height 👍

TalDerei · 2024-11-07T23:31:19Z

left some suggestions mainly around rpc selection; great work streamlining the onboarding flow!

grod220 · 2024-11-08T03:41:24Z

apps/extension/src/hooks/latest-block-height.test.ts

@TalDerei a little dependency injection allowed me to add these lovely tests

grod220 · 2024-11-08T03:48:36Z

Will need final leadership review before deploying, but can follow up with necessary follow up PRs for additional changes

grod220 commented Nov 7, 2024

View reviewed changes

grod220 requested a review from a team November 7, 2024 19:49

TalDerei approved these changes Nov 7, 2024

View reviewed changes

grod220 commented Nov 8, 2024

View reviewed changes

grod220 added 6 commits November 7, 2024 19:43

Remove steps from onboarding

4f2a97e

add origin path

2d68d0e

remove 12 toggle

9473bc9

clearing wallet height on chain id change

925bf67

Add better checks for remove in storage

78b9d18

Review updates

c8f6e51

grod220 force-pushed the simplify-onboarding branch from 2e90c28 to c8f6e51 Compare November 8, 2024 03:43

grod220 merged commit 2d0e9d1 into main Nov 8, 2024
3 checks passed

grod220 deleted the simplify-onboarding branch November 8, 2024 03:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Simplify onboarding #231

Simplify onboarding #231

grod220 commented Nov 7, 2024

grod220 Nov 7, 2024

grod220 Nov 7, 2024

grod220 Nov 7, 2024

grod220 Nov 7, 2024

TalDerei Nov 7, 2024

TalDerei Nov 7, 2024 •

edited

Loading

grod220 Nov 8, 2024

TalDerei Nov 7, 2024 •

edited

Loading

grod220 Nov 8, 2024

TalDerei Nov 7, 2024 •

edited

Loading

grod220 Nov 8, 2024

TalDerei Nov 7, 2024 •

edited

Loading

TalDerei commented Nov 7, 2024

grod220 Nov 8, 2024

TalDerei Nov 8, 2024

grod220 commented Nov 8, 2024

		// Fetch the block height from a specific RPC endpoint with a request-level timeout that superceeds
		// the channel transport-level timeout to prevent hanging requests.

Simplify onboarding #231

Simplify onboarding #231

Conversation

grod220 commented Nov 7, 2024

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

TalDerei Nov 7, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TalDerei Nov 7, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TalDerei Nov 7, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TalDerei Nov 7, 2024 • edited Loading

Choose a reason for hiding this comment

TalDerei commented Nov 7, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

grod220 commented Nov 8, 2024

TalDerei Nov 7, 2024 •

edited

Loading

TalDerei Nov 7, 2024 •

edited

Loading

TalDerei Nov 7, 2024 •

edited

Loading

TalDerei Nov 7, 2024 •

edited

Loading