Skip to content

Commit

Permalink
fix: Close magic connection (#94)
Browse files Browse the repository at this point in the history 
* fix: Close magic connection

* fix: Wrong deactivate emission and closing

* fix: Remove unneessary async

* fix: Linting

* fix: Disconnect previous connection

* fix: Add function to check wether there's a connection or not

* fix: Use async-await in handleWeb3ReactDeactivate

* fix: Update wallet connect

* fix: Return on-going re-connection when doing it multiple times

* fix: ChainIdToConnect bug

* fix: Revert change

* fix: Tests
  • Loading branch information
@LautaroPetaccio
LautaroPetaccio authored May 21, 2024
1 parent 20c9a58 commit 22e2754
Show file tree
Hide file tree
Showing 8 changed files with 3,181 additions and 2,989 deletions.
5,945 changes: 3,043 additions & 2,902 deletions package-lock.json
View file

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@
"@dcl/schemas": "^11.4.0",
"@dcl/single-sign-on-client": "^0.1.0",
"@magic-ext/oauth": "^15.5.0",
"@walletconnect/ethereum-provider": "^2.9.2",
"@walletconnect/modal": "^2.6.1",
"@walletconnect/ethereum-provider": "^2.13.0",
"@walletconnect/modal": "^2.6.2",
"@web3-react/fortmatic-connector": "^6.1.6",
"@web3-react/injected-connector": "^6.0.7",
"@web3-react/network-connector": "^6.1.3",
Expand Down
92 changes: 56 additions & 36 deletions src/ConnectionManager.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,30 +24,48 @@ import './declarations'

export class ConnectionManager {
connector?: AbstractConnector
promiseOfConnection?: Promise<ConnectionResponse>

constructor(public storage: Storage) {}

async connect(
providerType: ProviderType,
chainId: ChainId = ChainId.ETHEREUM_MAINNET
chainIdToConnect: ChainId = ChainId.ETHEREUM_MAINNET
): Promise<ConnectionResponse> {
this.connector = this.buildConnector(providerType, chainId)
// If a previous connection existed, disconnect from it
if (this.connector) {
try {
await this.disconnect()
} catch (error) {
console.error('Error disconnecting previous connection', error)
}
}

this.connector.on(ConnectorEvent.Deactivate, this.handleWeb3ReactDeactivate)
const connector = this.buildConnector(providerType, chainIdToConnect)
connector.on(ConnectorEvent.Deactivate, this.handleWeb3ReactDeactivate)
const { provider, account }: ConnectorUpdate = await connector.activate()

if (providerType === ProviderType.MAGIC) {
this.connector.on(ConnectorEvent.Update, ({ chainId }) => {
connector.on(ConnectorEvent.Update, ({ chainId }) => {
if (chainId) {
this.setConnectionData(providerType, chainId)
}
})
}

const {
provider,
account
}: ConnectorUpdate = await this.connector.activate()
let chainId = chainIdToConnect

// We need to return the correct current chain id for the injected providers
if (providerType === ProviderType.INJECTED) {
const currentChainIdHex = (await provider.request({
method: 'eth_chainId'
})) as string
chainId = currentChainIdHex
? (parseInt(currentChainIdHex, 16) as ChainId)
: chainId
}

this.connector = connector
this.setConnectionData(providerType, chainId)

return {
Expand All @@ -58,6 +76,10 @@ export class ConnectionManager {
}
}

isConnected(): boolean {
return !!this.connector && !!this.getConnectionData()
}

async tryPreviousConnection(): Promise<ConnectionResponse> {
const connectionData = this.getConnectionData()
if (!connectionData) {
Expand All @@ -66,25 +88,23 @@ export class ConnectionManager {
)
}

const response = await this.connect(
connectionData.providerType,
connectionData.chainId
)

// If the provider type is injected, the chainId could have changed since previous connection and still connect successfully.
// We need to check if the chainId has changed, and update the connectionData if so.
if (response.providerType === ProviderType.INJECTED) {
const currentChainIdHex = (await response.provider.request({
method: 'eth_chainId'
})) as string
const currentChainId = currentChainIdHex
? (parseInt(currentChainIdHex, 16) as ChainId)
: null
if (currentChainId && connectionData.chainId !== currentChainId) {
this.setConnectionData(connectionData.providerType, currentChainId)
if (this.connector) {
return {
provider: await this.connector.getProvider(),
providerType: connectionData.providerType,
chainId: connectionData.chainId,
account: await this.connector.getAccount()
}
}

const response = this.promiseOfConnection
? await this.promiseOfConnection
: await (this.promiseOfConnection = this.connect(
connectionData.providerType,
connectionData.chainId
))
this.promiseOfConnection = undefined

return {
...response,
chainId: this.getConnectionData()!.chainId
Expand All @@ -106,10 +126,14 @@ export class ConnectionManager {
return available
}

async disconnect() {
async disconnect(): Promise<void> {
if (this.connector) {
this.connector.deactivate()
// Remove event listeners
this.connector.removeAllListeners(ConnectorEvent.Deactivate)
this.connector.removeAllListeners(ConnectorEvent.Update)

// Deactivate and close the connector if it's closable
this.connector.deactivate()
if (this.isClosableConnector()) {
await (this.connector as ClosableConnector).close()
}
Expand Down Expand Up @@ -200,19 +224,15 @@ export class ConnectionManager {
return this.connector && typeof this.connector['close'] !== 'undefined'
}

private handleWeb3ReactDeactivate = () => {
if (this.connector) {
this.connector.removeListener(
ConnectorEvent.Deactivate,
this.handleWeb3ReactDeactivate
)
this.connector.removeAllListeners(ConnectorEvent.Update)
}

private handleWeb3ReactDeactivate = async () => {
// Whenever the user manually disconnects the account from their wallet, the event will be
// intercepted by this handler, calling the disconnect method.
// Necessary to sanitize the state and prevent the continuation of a dead connection.
this.disconnect().catch(console.error)
try {
await this.disconnect()
} catch (error) {
console.error(error)
}
}
}

Expand Down
5 changes: 2 additions & 3 deletions src/connectors/FortmaticConnector.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,7 @@ export class FortmaticConnector extends AbstractConnector {
// tslint:disable-next-line
public deactivate() {}

public async close() {
await this.fortmatic.user.logout()
this.emitDeactivate()
public close(): Promise<unknown> {
return this.fortmatic.user.logout()
}
}
8 changes: 4 additions & 4 deletions src/connectors/MagicConnector.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,15 +84,15 @@ export class MagicConnector extends AbstractConnector {
return this.account
}

deactivate = (): void => {
public close(): Promise<boolean> {
if (!this.magic) {
throw new Error('Magic: instance was not initialized')
}

this.magic.user.logout()
this.emitDeactivate()
return this.magic.user.logout()
}

deactivate = (): void => undefined

private buildMagicInstance = async (chainId: ChainId): Promise<InstanceWithExtensions<SDKBase, OAuthExtension[]>> => {
const { Magic } = await import('magic-sdk')
const { OAuthExtension } = await import('@magic-ext/oauth')
Expand Down
18 changes: 9 additions & 9 deletions src/connectors/WalletConnectV2Connector.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,18 +114,18 @@ export class WalletConnectV2Connector extends AbstractConnector {
return this.provider?.session?.peer.metadata.name
}

deactivate = (): void => {
deactivate = (): void => undefined

close = async (): Promise<void> => {
if (!this.provider) {
return
}

this.emitDeactivate()

this.provider
.removeListener('accountsChanged', this.handleAccountsChanged)
.removeListener('chainChanged', this.handleChainChanged)
.removeListener('disconnect', this.handleDisconnect)
.disconnect()
return this.provider
.removeListener('accountsChanged', this.handleAccountsChanged)
.removeListener('chainChanged', this.handleChainChanged)
.removeListener('disconnect', this.handleDisconnect)
.disconnect()
}

handleAccountsChanged = (accounts: string[]): void => {
Expand All @@ -141,6 +141,6 @@ export class WalletConnectV2Connector extends AbstractConnector {
throw new Error('Provider is undefined')
}

this.deactivate()
return this.emitDeactivate()
}
}
92 changes: 62 additions & 30 deletions test/ConnectionManager.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ describe('ConnectionManager', () => {

it('should return the connection data', async () => {
const stubConnector = new StubConnector()
stubConnector.setChainId(ChainId.ETHEREUM_SEPOLIA)
sinon.stub(connectionManager, 'buildConnector').returns(stubConnector)

const result = await connectionManager.connect(
Expand All @@ -91,6 +92,7 @@ describe('ConnectionManager', () => {

it('should not patch the provider with the request method if it already exists', async () => {
const stubConnector = new StubConnector()
stubConnector.setChainId(ChainId.ETHEREUM_SEPOLIA)
sinon.stub(connectionManager, 'buildConnector').returns(stubConnector)

const result = await connectionManager.connect(
Expand All @@ -113,6 +115,7 @@ describe('ConnectionManager', () => {

it('should store the last provider and chain', async () => {
const stubConnector = new StubConnector()
stubConnector.setChainId(ChainId.ETHEREUM_SEPOLIA)
const configuration = getConfiguration()
sinon.stub(connectionManager, 'buildConnector').returns(stubConnector)

Expand All @@ -125,6 +128,37 @@ describe('ConnectionManager', () => {
providerType: ProviderType.NETWORK,
chainId: ChainId.ETHEREUM_SEPOLIA
})

expect(storage.get(configuration.storageKey)).to.eq(value)
})

it('should store and return the current chain id and not the one supplied', async () => {
const stubConnector = new StubConnector()
stubConnector.setChainId(ChainId.ETHEREUM_MAINNET)
const configuration = getConfiguration()
sinon.stub(connectionManager, 'buildConnector').returns(stubConnector)

const result = await connectionManager.connect(
ProviderType.INJECTED,
ChainId.ETHEREUM_SEPOLIA
)
const activateResult = await stubConnector.activate()
const value = JSON.stringify({
providerType: ProviderType.INJECTED,
chainId: ChainId.ETHEREUM_MAINNET
})

expect(JSON.stringify(result)).to.eq(
JSON.stringify({
provider: {
request: () => {},
send: () => {}
},
providerType: ProviderType.INJECTED,
account: activateResult.account,
chainId: ChainId.ETHEREUM_MAINNET
})
)
expect(storage.get(configuration.storageKey)).to.eq(value)
})
})
Expand All @@ -151,50 +185,24 @@ describe('ConnectionManager', () => {
expect(
getConnectorStub.firstCall.calledWith(ProviderType.FORTMATIC)
).to.eq(true)
expect(
getConnectorStub.secondCall.calledWith(ProviderType.FORTMATIC)
).to.eq(true)

expect(JSON.stringify(result)).to.eq(
JSON.stringify({
provider: {
request: () => {}
},
providerType: ProviderType.FORTMATIC,
account,
chainId: ChainId.ETHEREUM_MAINNET
chainId: ChainId.ETHEREUM_MAINNET,
account
})
)
})

it('should update the connection data if the providerType is "injected" and the chainId of the provider changed since the last connection', async () => {
const stubConnector = new StubConnector()

const getConnectorStub = sinon
.stub(connectionManager, 'buildConnector')
.returns(stubConnector)

// connect to the default network (mainnet)
await connectionManager.connect(ProviderType.INJECTED)

// mock change in the chainId
stubConnector.setChainId(ChainId.ETHEREUM_SEPOLIA)

const result = await connectionManager.tryPreviousConnection()

expect(
getConnectorStub.firstCall.calledWith(ProviderType.INJECTED)
).to.eq(true)
expect(result.chainId).to.eq(ChainId.ETHEREUM_SEPOLIA)
expect(connectionManager.getConnectionData()!.chainId).to.eq(
ChainId.ETHEREUM_SEPOLIA
)
})
})

describe('#getConnectionData', () => {
it('should return the data used on the last successful connection', async () => {
const stubConnector = new StubConnector()
stubConnector.setChainId(ChainId.ETHEREUM_SEPOLIA)
sinon.stub(connectionManager, 'buildConnector').returns(stubConnector)

await connectionManager.connect(
Expand All @@ -208,11 +216,35 @@ describe('ConnectionManager', () => {
})
})

it('should return undefined if no connection happneed', () => {
it('should return undefined if no connection happened', () => {
expect(connectionManager.getConnectionData()).to.eq(undefined)
})
})

describe('#isConnected', () => {
it('should return true if a connector exists and a connection happened', async () => {
const stubConnector = new StubConnector()
sinon.stub(connectionManager, 'buildConnector').returns(stubConnector)

await connectionManager.connect(
ProviderType.INJECTED,
ChainId.ETHEREUM_MAINNET
)

expect(connectionManager.isConnected()).to.eq(true)
})

it("should return false if there's no previous connection data", () => {
expect(connectionManager.isConnected()).to.eq(false)
})

it("should return false if there's no connector defined", async () => {
connectionManager.connector = new StubConnector()
await connectionManager.disconnect()
expect(connectionManager.isConnected()).to.eq(false)
})
})

describe('#disconnect', () => {
it('should not do anything if no connector exists', () => {
return expect(connectionManager.disconnect()).not.to.be.rejected
Expand Down
Loading

0 comments on commit 22e2754

Please sign in to comment.