Skip to content

Commit

Permalink
chore(walletconnect-signing-manager): address comments - ensure event…
Browse files Browse the repository at this point in the history 
… listeners are correctly deregistered
  • Loading branch information
@F-OBrien
F-OBrien committed May 30, 2024
1 parent 622f77e commit fd3b550
Show file tree
Hide file tree
Showing 2 changed files with 96 additions and 67 deletions.
113 changes: 61 additions & 52 deletions packages/walletconnect-signing-manager/src/lib/walletconnect/walletconnect.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,20 +136,6 @@ describe('WalletConnect', () => {
});
});

describe('reset', () => {
it('should reset client, session, and signer', () => {
walletConnect.client = clientMock;
walletConnect.session = sessionMock;
walletConnect.signer = new WalletConnectSigner(clientMock, sessionMock);

(walletConnect as any).reset();

expect(walletConnect.client).toBeUndefined();
expect(walletConnect.session).toBeUndefined();
expect(walletConnect.signer).toBeUndefined();
});
});

describe('connect', () => {
beforeEach(() => {
jest.clearAllMocks();
Expand Down Expand Up @@ -254,6 +240,54 @@ describe('WalletConnect', () => {
expect(walletConnect.signer).toBeUndefined();
expect(config.onSessionDelete).toHaveBeenCalled();
});

it('should call onSessionDelete callback on session_expire event', async () => {
await walletConnect.connect();
const sessionExpireListener = clientMock.on.mock.calls.find(
([event]) => event === 'session_expire'
)?.[1];

if (sessionExpireListener) {
sessionExpireListener({
topic: '0fad0dec80bf1226eb1646defde76536a86f0a06e604bd28f98c08c564b0e035',
});
}

expect(walletConnect.session).toBeUndefined();
expect(walletConnect.signer).toBeUndefined();
expect(config.onSessionDelete).toHaveBeenCalled();
});

it('should call onSessionDelete callback on session_extend event', async () => {
await walletConnect.connect();
const sessionUpdateListener = clientMock.on.mock.calls.find(
([event]) => event === 'session_extend'
)?.[1];
const sessionData = { ...sessionMock, expiry: Math.floor(Date.now() / 1000) + 1000 };
(clientMock.session.getAll as jest.Mock).mockReturnValueOnce([sessionData]);

if (sessionUpdateListener) {
sessionUpdateListener(sessionData);
}

expect(walletConnect.session).toEqual(sessionData);
expect(walletConnect.signer?.session).toEqual(sessionData);
});

it('should not update session and signer if no session data is available', async () => {
await walletConnect.connect();
const sessionUpdateListener = clientMock.on.mock.calls.find(
([event]) => event === 'session_extend'
)?.[1];
(clientMock.session.getAll as jest.Mock).mockReturnValueOnce([]);

if (sessionUpdateListener) {
sessionUpdateListener(sessionMock);
}

expect(walletConnect.session).toBeUndefined();
expect(walletConnect.signer).toBeUndefined();
});
});

describe('getAccounts', () => {
Expand Down Expand Up @@ -331,25 +365,33 @@ describe('WalletConnect', () => {

describe('disconnect', () => {
it('should disconnect client and reset wallet', async () => {
walletConnect.client = clientMock;
walletConnect.session = sessionMock;
walletConnect.signer = new WalletConnectSigner(clientMock, sessionMock);
await walletConnect.connect();
expect(walletConnect.client).not.toBeUndefined();
expect(walletConnect.session).not.toBeUndefined();
expect(walletConnect.signer).not.toBeUndefined();

await walletConnect.disconnect();

expect(walletConnect.client).toBeUndefined();
expect(walletConnect.session).toBeUndefined();
expect(walletConnect.signer).toBeUndefined();
expect(clientMock.disconnect).toBeCalled();
// Ensure event listeners are removed
expect(clientMock.off).toHaveBeenCalledWith('session_delete', expect.any(Function));
expect(clientMock.off).toHaveBeenCalledWith('session_update', expect.any(Function));
expect(clientMock.off).toHaveBeenCalledWith('session_expire', expect.any(Function));
expect(clientMock.off).toHaveBeenCalledWith('session_extend', expect.any(Function));
});

it('should reset the wallet even if there is no active session', async () => {
walletConnect.client = clientMock;

await walletConnect.connect();
walletConnect.session = undefined;
await walletConnect.disconnect();

expect(walletConnect.client).toBeUndefined();
expect(walletConnect.session).toBeUndefined();
expect(walletConnect.signer).toBeUndefined();
expect(clientMock.disconnect).not.toBeCalled();

// Ensure event listeners are removed
expect(clientMock.off).toHaveBeenCalledWith('session_delete', expect.any(Function));
Expand All @@ -372,37 +414,4 @@ describe('WalletConnect', () => {
expect(walletConnect.isConnected()).toBe(false);
});
});

describe('handleSessionDeleteOrExpire', () => {
it('should clear session and signer and call onSessionDelete if provided', () => {
(walletConnect as any).handleSessionDeleteOrExpire();

expect(walletConnect.session).toBeUndefined();
expect(walletConnect.signer).toBeUndefined();
expect(walletConnect.config.onSessionDelete).toHaveBeenCalled();
});
});

describe('handleSessionChange', () => {
it('should update session and signer if client and session data are available', () => {
walletConnect = new WalletConnect(config, 'TestApp');
walletConnect.client = clientMock;
const sessionData = { ...sessionMock, expiry: Math.floor(Date.now() / 1000) + 1000 };
(clientMock.session.getAll as jest.Mock).mockReturnValueOnce([sessionData]);
(walletConnect as any).handleSessionChange();

expect(walletConnect.session).toEqual(sessionData);
// Ensure signer creation logic is properly tested in WalletConnectSigner tests
expect(walletConnect.signer?.session).toEqual(sessionData);
});

it('should not update session and signer if no session data is available', () => {
walletConnect.client = clientMock;
(clientMock.session.getAll as jest.Mock).mockReturnValueOnce([]);
(walletConnect as any).handleSessionChange();

expect(walletConnect.session).toBeUndefined();
expect(walletConnect.signer).toBeUndefined();
});
});
});
50 changes: 35 additions & 15 deletions packages/walletconnect-signing-manager/src/lib/walletconnect/walletconnect.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@ export class WalletConnect implements BaseWallet {
session: SessionTypes.Struct | undefined;
signer: WalletConnectSigner | undefined;

private _handleSessionDeleteOrExpire: () => void;
private _handleSessionChange: () => void;
private unsubscribeEvents: (() => void) | undefined;
/**
* Creates an instance of WalletConnectWallet.
* @param config - Configuration for Wallet Connect.
Expand All @@ -58,6 +61,10 @@ export class WalletConnect implements BaseWallet {
iconUrl: config.metadata?.icons[0] || '',
version: WC_VERSION,
};

// Bind event handlers and store references
this._handleSessionDeleteOrExpire = this.handleSessionDeleteOrExpire.bind(this);
this._handleSessionChange = this.handleSessionChange.bind(this);
}

/**
Expand All @@ -77,12 +84,22 @@ export class WalletConnect implements BaseWallet {
this.reset();

this.client = await SignClient.init(this.config);
const eventHandlers = [
{ event: 'session_delete' as const, handler: this._handleSessionDeleteOrExpire },
{ event: 'session_expire' as const, handler: this._handleSessionDeleteOrExpire },
{ event: 'session_update' as const, handler: this._handleSessionChange },
{ event: 'session_extend' as const, handler: this._handleSessionChange },
];

this.client.on('session_delete', this.handleSessionDeleteOrExpire.bind(this));
this.client.on('session_expire', this.handleSessionDeleteOrExpire.bind(this));
this.client.on('session_update', this.handleSessionChange.bind(this));
this.client.on('session_extend', this.handleSessionChange.bind(this));
eventHandlers.forEach(({ event, handler }) => {
this.client?.on(event, handler);
});

this.unsubscribeEvents = () => {
eventHandlers.forEach(({ event, handler }) => {
this.client?.off(event, handler);
});
};
const sessions = this.client.session.getAll();
const lastKeyIndex = sessions.length - 1;
const lastSession = sessions[lastKeyIndex];
Expand Down Expand Up @@ -179,15 +196,21 @@ export class WalletConnect implements BaseWallet {

handler();

this.client?.on('session_delete', handler);
this.client?.on('session_expire', handler);
this.client?.on('session_update', handler);
if (this.client) {
this.client.on('session_delete', handler);
this.client.on('session_expire', handler);
this.client.on('session_update', handler);
}

return () => {
this.client?.off('session_delete', handler);
this.client?.off('session_expire', handler);
this.client?.off('session_update', handler);
const unsubscribe = () => {
if (this.client) {
this.client.off('session_delete', handler);
this.client.off('session_expire', handler);
this.client.off('session_update', handler);
}
};

return unsubscribe;
}

/**
Expand All @@ -204,10 +227,7 @@ export class WalletConnect implements BaseWallet {
});
}

this.client?.off('session_delete', this.handleSessionDeleteOrExpire.bind(this));
this.client?.off('session_expire', this.handleSessionDeleteOrExpire.bind(this));
this.client?.off('session_update', this.handleSessionChange.bind(this));
this.client?.off('session_extend', this.handleSessionChange.bind(this));
this.unsubscribeEvents?.();

this.reset();
}
Expand Down

0 comments on commit fd3b550

Please sign in to comment.