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.

Sign up for GitHub

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

Jump to bottom

[ID-1897] Improve magic session management #1965

Open
wants to merge 23 commits into
base: main
Choose a base branch
from
Open

[ID-1897] Improve magic session management #1965

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
1000eff
ID-1897 WIP
haydenfowler Jul 10, 2024
62f79e3
fix: WIP magicAdapter/authManager tests
matthewmuscat Jul 11, 2024
090d837
ID-1897 Add logging and error handling
haydenfowler Jul 11, 2024
b6e9069
ID-1897: Fixed up signer initialisation
haydenfowler Jul 11, 2024
dcbd6f9
ID-1897: Updated passportImxProvider signer implementation
haydenfowler Jul 12, 2024
2c5917c
fix: Clean up tests and remove Login func
matthewmuscat Jul 12, 2024
bf83143
Merge branch 'feature/ID-1897-magic-session-management' of https://gi…
matthewmuscat Jul 12, 2024
d6910d8
cd '/Users/matthewmuscat/Documents/Work/ts-immutable-sdk'
matthewmuscat Jul 15, 2024
85c1ed6
fix: Refactor re-initialise of signer to avoid recursiveness
matthewmuscat Jul 15, 2024
b8ceb12
WIP
haydenfowler Jul 16, 2024
15da5dd
feat: Initialise signers for zkevm RPC calls
matthewmuscat Jul 22, 2024
71ed63b
Merge branch 'main' into feature/ID-1897-magic-session-management
matthewmuscat Jul 22, 2024
f07aaff
feat: Initialise magic signer on RPC calls
matthewmuscat Jul 23, 2024
b9397aa
fix: optional chain
matthewmuscat Jul 23, 2024
24508bd
fix: remove accidental file
matthewmuscat Jul 23, 2024
0633c3d
fix: Only get signer for rpc
matthewmuscat Jul 24, 2024
b39866b
Merge branch 'main' into feature/ID-1897-magic-session-management
matthewmuscat Jul 24, 2024
a5695e4
feat: Zkevm updates
matthewmuscat Jul 24, 2024
520c29e
fix: Passport provider tests to use new magicAdapter
matthewmuscat Jul 24, 2024
25ecf46
fix: Tests
matthewmuscat Jul 24, 2024
56c9dd7
fix: typecheck
matthewmuscat Jul 24, 2024
ac1424a
fix: fail silently on initial user fetch from cache
matthewmuscat Jul 26, 2024
a765274
Merge branch 'main' into feature/ID-1897-magic-session-management
matthewmuscat Jul 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 15 additions & 8 deletions packages/passport/sdk/src/magicAdapter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,13 +64,13 @@ export default class MagicAdapter {
* @see #getSigner
*
*/
public initialiseSigner(user: User) {
const generateSigner = async (): Promise<Signer> => {
initialiseSigner(user: User | null) {
const generateSigner = async (u: User): Promise<Signer> => {
const startTime = performance.now();

const magicClient = await this.magicClient;
await magicClient.openid.loginWithOIDC({
jwt: user.idToken,
jwt: u.idToken,
providerId: this.config.magicProviderId,
});

Expand All @@ -84,11 +84,21 @@ export default class MagicAdapter {
return web3Provider.getSigner();
};

const getUser = async (initialUser: User | null): Promise<User> => {
if (initialUser) return initialUser;

// Attempt to refetch user
const newUser = await this.authManager.getUser();
if (!newUser) throw new Error('Cannot initialise signer without user');
return newUser;
};

this.magicSignerInitialisationError = undefined;
// eslint-disable-next-line no-async-promise-executor
this.magicSigner = new Promise(async (resolve) => {
try {
resolve(await generateSigner());
const userForSigner = await getUser(user);
resolve(await generateSigner(userForSigner));
} catch (err) {
// Capture and store the initialization error
this.magicSignerInitialisationError = err;
Expand All @@ -104,14 +114,11 @@ export default class MagicAdapter {
return this.lazyMagicClient;
}

public async getSigner(): Promise<Signer> {
public async getSigner(user: User | null): Promise<Signer> {
return withPassportError<Signer>(async () => {
// getSigner may be called without `initialiseSigner` being called when the user is pulled from localStorage
const magicClient = await this.magicClient;
const isLoggedIn = await magicClient.user.isLoggedIn();
if (!this.magicSigner || !isLoggedIn) {
const user = await this.authManager.getUser();
if (!user) throw new Error('Cannot initialise signer without user');
this.initialiseSigner(user);
}

Expand Down
67 changes: 24 additions & 43 deletions packages/passport/sdk/src/zkEvm/zkEvmProvider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import MagicAdapter from '../magicAdapter';
import TypedEventEmitter from '../utils/typedEventEmitter';
import { PassportConfiguration } from '../config';
import {
isUserZkEvm, PassportEventMap, PassportEvents, UserZkEvm,
isUserZkEvm, PassportEventMap, PassportEvents, User, UserZkEvm,
} from '../types';
import { RelayerClient } from './relayerClient';
import { JsonRpcError, ProviderErrorCode, RpcErrorCode } from './JsonRpcError';
Expand Down Expand Up @@ -110,9 +110,9 @@ export class ZkEvmProvider implements Provider {
this.#providerEventEmitter.emit(ProviderEvent.ACCOUNTS_CHANGED, []);
};

async #callSessionActivity(zkEvmAddress: string) {
async #callSessionActivity(zkEvmAddress: string, user: User | null) {
const sendTransactionClosure = async (params: Array<any>, flow: Flow) => {
const ethSigner = await this.#magicAdapter.getSigner();
const ethSigner = await this.#magicAdapter.getSigner(user);
return await sendTransaction({
params,
ethSigner,
Expand All @@ -131,71 +131,61 @@ export class ZkEvmProvider implements Provider {
});
}

async #getZkEvmUser(): Promise<UserZkEvm | undefined> {
try {
const user = await this.#authManager.getUser();
if (user && isUserZkEvm(user)) {
return user;
}
return undefined;
} catch {
return undefined;
// eslint-disable-next-line class-methods-use-this
#getZkEvmUser(user: User | null): UserZkEvm | undefined {
if (user && isUserZkEvm(user)) {
return user;
}
}

// Used to get the registered zkEvm address from the User session
async #getZkEvmAddress() {
const user = await this.#getZkEvmUser();
return user ? user.zkEvm.ethAddress : undefined;
return undefined;
}

async #performRequest(request: RequestArguments): Promise<any> {
// This is required for sending session activity events

// Get user from local storage and initialise signer for all RPC calls
const user = await this.#authManager.getUser();
const zkEvmUser = this.#getZkEvmUser(user);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will have unintended consequences on the passthrough methods when getUser is called. Two scenarios that I can think of:

  1. If a token refresh is triggered, then we'll need to wait for that process to resolve before we can pass the method along to the node
  2. If a token refresh is triggered and it fails, then this error will be surfaced to the caller

Copy link
Contributor Author

@matthewmuscat matthewmuscat Jul 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If a token refresh is triggered, then we'll need to wait for that process to resolve before we can pass the method along to the node

await this.#authManager.getUser(); already awaits the refreshing promise so I think this is fine?

If a token refresh is triggered and it fails, then this error will be surfaced to the caller

Edit: we can just fail silently on this check and let the methods logic handle it with how they see fit.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ac1424a

const magicSigner = this.#magicAdapter.getSigner(user); // preemptively load the magic signer

switch (request.method) {
case 'eth_requestAccounts': {
const requestAccounts = async () => {
const zkEvmUser = await this.#getZkEvmUser();
if (zkEvmUser) {
this.#magicAdapter.initialiseSigner(zkEvmUser);
return [zkEvmUser.zkEvm.ethAddress];
}

const flow = trackFlow('passport', 'ethRequestAccounts');

try {
const user = await this.#authManager.getUserOrLogin();

this.#magicAdapter.initialiseSigner(user);

const loggedInUser = await this.#authManager.getUserOrLogin();
flow.addEvent('endGetUserOrLogin');

let userZkEvmEthAddress;

if (!isUserZkEvm(user)) {
if (!isUserZkEvm(loggedInUser)) {
flow.addEvent('startUserRegistration');

const ethSigner = await this.#magicAdapter.getSigner();
const ethSigner = await this.#magicAdapter.getSigner(loggedInUser);
flow.addEvent('ethSignerResolved');

userZkEvmEthAddress = await registerZkEvmUser({
ethSigner,
authManager: this.#authManager,
multiRollupApiClients: this.#multiRollupApiClients,
accessToken: user.accessToken,
accessToken: loggedInUser.accessToken,
rpcProvider: this.#rpcProvider,
flow,
});
flow.addEvent('endUserRegistration');
} else {
userZkEvmEthAddress = user.zkEvm.ethAddress;
userZkEvmEthAddress = loggedInUser.zkEvm.ethAddress;
}

this.#providerEventEmitter.emit(ProviderEvent.ACCOUNTS_CHANGED, [
userZkEvmEthAddress,
]);
identify({
passportId: user.profile.sub,
passportId: loggedInUser.profile.sub,
});
return [userZkEvmEthAddress];
} catch (error) {
Expand All @@ -211,28 +201,25 @@ export class ZkEvmProvider implements Provider {

const addresses = await requestAccounts();
const [zkEvmAddress] = addresses;
this.#callSessionActivity(zkEvmAddress);
this.#callSessionActivity(zkEvmAddress, user);
return addresses;
}
case 'eth_sendTransaction': {
const zkEvmUser = await this.#getZkEvmUser();
if (!zkEvmUser) {
throw new JsonRpcError(
ProviderErrorCode.UNAUTHORIZED,
'Unauthorised - call eth_requestAccounts first',
);
}

this.#magicAdapter.initialiseSigner(zkEvmUser);

const flow = trackFlow('passport', 'ethSendTransaction');

try {
return await this.#guardianClient.withConfirmationScreen({
width: 480,
height: 720,
})(async () => {
const ethSigner = await this.#magicAdapter.getSigner();
const ethSigner = await magicSigner;
flow.addEvent('endGetSigner');

return await sendTransaction({
Expand All @@ -256,28 +243,25 @@ export class ZkEvmProvider implements Provider {
}
}
case 'eth_accounts': {
const zkEvmAddress = await this.#getZkEvmAddress();
const zkEvmAddress = zkEvmUser?.zkEvm.ethAddress;
matthewmuscat marked this conversation as resolved.
Show resolved Hide resolved
return zkEvmAddress ? [zkEvmAddress] : [];
}
case 'personal_sign': {
const zkEvmUser = await this.#getZkEvmUser();
if (!zkEvmUser) {
throw new JsonRpcError(
ProviderErrorCode.UNAUTHORIZED,
'Unauthorised - call eth_requestAccounts first',
);
}

this.#magicAdapter.initialiseSigner(zkEvmUser);

const flow = trackFlow('passport', 'personalSign');

try {
return await this.#guardianClient.withConfirmationScreen({
width: 480,
height: 720,
})(async () => {
const ethSigner = await this.#magicAdapter.getSigner();
const ethSigner = await magicSigner;
flow.addEvent('endGetSigner');

return await personalSign({
Expand All @@ -302,24 +286,21 @@ export class ZkEvmProvider implements Provider {
}
case 'eth_signTypedData':
case 'eth_signTypedData_v4': {
const zkEvmUser = await this.#getZkEvmUser();
if (!zkEvmUser) {
throw new JsonRpcError(
ProviderErrorCode.UNAUTHORIZED,
'Unauthorised - call eth_requestAccounts first',
);
}

this.#magicAdapter.initialiseSigner(zkEvmUser);

const flow = trackFlow('passport', 'ethSignTypedDataV4');

try {
return await this.#guardianClient.withConfirmationScreen({
width: 480,
height: 720,
})(async () => {
const ethSigner = await this.#magicAdapter.getSigner();
const ethSigner = await magicSigner;
flow.addEvent('endGetSigner');

return await signTypedDataV4({
Expand Down
Loading