Skip to content

Commit

Permalink
[SDK] Fix: Account switching with connected smart wallets (#5592)
Browse files Browse the repository at this point in the history
Fixes #5537

<!-- start pr-codex -->

---

## PR-Codex overview
This PR focuses on improving wallet connection handling in the `thirdweb` project, specifically when switching signer accounts and simplifying connection logic for smart wallets.

### Detailed summary
- Updated `biome.json` to add `"noUselessElse": "off"`.
- Modified wallet connection logic in `packages/thirdweb/src/wallets/manager/index.ts` for better handling of active wallets.
- Added tests for connection management in `packages/thirdweb/src/wallets/manager/connection-manager.test.ts`.
- Removed environment variable logging across multiple API routes.

> ✨ Ask PR-Codex anything about this PR by commenting with `/codex {your question}`

<!-- end pr-codex -->
  • Loading branch information
gregfromstl committed Dec 3, 2024
1 parent ea855c6 commit c3d7b66
Show file tree
Hide file tree
Showing 8 changed files with 114 additions and 72 deletions.
5 changes: 5 additions & 0 deletions .changeset/silver-weeks-rescue.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"thirdweb": patch
---

Properly updates active smart wallet when switching signer account on EOA wallet
12 changes: 0 additions & 12 deletions apps/playground-web/src/app/api/airdrop/route.ts
Original file line number Diff line number Diff line change
@@ -1,22 +1,10 @@
import { Engine } from "@thirdweb-dev/engine";
import * as dotenv from "dotenv";
import type { NextRequest } from "next/server";
import { NextResponse } from "next/server";

dotenv.config();

const CHAIN_ID = "84532";
const BACKEND_WALLET_ADDRESS = process.env.ENGINE_BACKEND_WALLET as string;

console.log("Environment Variables:");
console.log("CHAIN_ID:", CHAIN_ID);
console.log("BACKEND_WALLET_ADDRESS:", BACKEND_WALLET_ADDRESS);
console.log("ENGINE_URL:", process.env.ENGINE_URL);
console.log(
"ACCESS_TOKEN:",
process.env.ENGINE_ACCESS_TOKEN ? "Set" : "Not Set",
);

const engine = new Engine({
url: process.env.ENGINE_URL as string,
accessToken: process.env.ENGINE_ACCESS_TOKEN as string,
Expand Down
12 changes: 0 additions & 12 deletions apps/playground-web/src/app/api/claimTo/route.ts
Original file line number Diff line number Diff line change
@@ -1,22 +1,10 @@
import { Engine } from "@thirdweb-dev/engine";
import * as dotenv from "dotenv";
import type { NextRequest } from "next/server";
import { NextResponse } from "next/server";

dotenv.config();

const BASESEP_CHAIN_ID = "84532";
const BACKEND_WALLET_ADDRESS = process.env.ENGINE_BACKEND_WALLET as string;

console.log("Environment Variables:");
console.log("CHAIN_ID:", BASESEP_CHAIN_ID);
console.log("BACKEND_WALLET_ADDRESS:", BACKEND_WALLET_ADDRESS);
console.log("ENGINE_URL:", process.env.ENGINE_URL);
console.log(
"ACCESS_TOKEN:",
process.env.ENGINE_ACCESS_TOKEN ? "Set" : "Not Set",
);

const engine = new Engine({
url: process.env.ENGINE_URL as string,
accessToken: process.env.ENGINE_ACCESS_TOKEN as string,
Expand Down
13 changes: 0 additions & 13 deletions apps/playground-web/src/app/api/erc20BatchMintTo/route.ts
Original file line number Diff line number Diff line change
@@ -1,23 +1,10 @@
import { Engine } from "@thirdweb-dev/engine";
import * as dotenv from "dotenv";
import type { NextRequest } from "next/server";
import { NextResponse } from "next/server";
import type { Address } from "thirdweb";

dotenv.config();

const BASESEP_CHAIN_ID = "84532";
const BACKEND_WALLET_ADDRESS = process.env.ENGINE_BACKEND_WALLET as string;

console.log("Environment Variables:");
console.log("CHAIN_ID:", BASESEP_CHAIN_ID);
console.log("BACKEND_WALLET_ADDRESS:", BACKEND_WALLET_ADDRESS);
console.log("ENGINE_URL:", process.env.ENGINE_URL);
console.log(
"ACCESS_TOKEN:",
process.env.ENGINE_ACCESS_TOKEN ? "Set" : "Not Set",
);

const engine = new Engine({
url: process.env.ENGINE_URL as string,
accessToken: process.env.ENGINE_ACCESS_TOKEN as string,
Expand Down
14 changes: 0 additions & 14 deletions apps/playground-web/src/app/api/mintTo/route.ts
Original file line number Diff line number Diff line change
@@ -1,24 +1,10 @@
import { Engine } from "@thirdweb-dev/engine";
import * as dotenv from "dotenv";
import { type NextRequest, NextResponse } from "next/server";

dotenv.config();

const CHAIN_ID = "84532";
const CONTRACT_ADDRESS = "0x8CD193648f5D4E8CD9fD0f8d3865052790A680f6";
const BACKEND_WALLET_ADDRESS = process.env.ENGINE_BACKEND_WALLET as string;

// Add logging for environment variables
console.log("Environment Variables:");
console.log("CHAIN_ID:", CHAIN_ID);
console.log("BACKEND_WALLET_ADDRESS:", BACKEND_WALLET_ADDRESS);
console.log("CONTRACT_ADDRESS:", CONTRACT_ADDRESS);
console.log("ENGINE_URL:", process.env.ENGINE_URL);
console.log(
"ACCESS_TOKEN:",
process.env.ENGINE_ACCESS_TOKEN ? "Set" : "Not Set",
);

const engine = new Engine({
url: process.env.ENGINE_URL as string,
accessToken: process.env.ENGINE_ACCESS_TOKEN as string,
Expand Down
3 changes: 2 additions & 1 deletion biome.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@
"noRestrictedGlobals": {
"options": { "deniedGlobals": ["Buffer"] },
"level": "error"
}
},
"noUselessElse": "off"
}
}
},
Expand Down
68 changes: 68 additions & 0 deletions packages/thirdweb/src/wallets/manager/connection-manager.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
import { beforeEach, describe, expect, it, vi } from "vitest";
import { TEST_CLIENT } from "../../../test/src/test-clients.js";
import { TEST_ACCOUNT_A } from "../../../test/src/test-wallets.js";
import { sepolia } from "../../chains/chain-definitions/sepolia.js";
import type { ThirdwebClient } from "../../client/client.js";
import type { AsyncStorage } from "../../utils/storage/AsyncStorage.js";
import type { Account, Wallet } from "../interfaces/wallet.js";
import type { SmartWalletOptions } from "../smart/types.js";
import { createConnectionManager } from "./index.js";

describe.runIf(process.env.TW_SECRET_KEY)("Connection Manager", () => {
let storage: AsyncStorage;
let client: ThirdwebClient;
let wallet: Wallet;
let account: Account;
let smartWalletOptions: SmartWalletOptions;

beforeEach(() => {
storage = {
getItem: vi.fn(),
setItem: vi.fn(),
removeItem: vi.fn(),
};
client = TEST_CLIENT;
account = TEST_ACCOUNT_A;
wallet = {
id: "wallet-id",
getAccount: vi.fn().mockReturnValue(account),
subscribe: vi.fn(),
disconnect: vi.fn(),
switchChain: vi.fn(),
getChain: vi.fn().mockReturnValue(sepolia),
getConfig: vi.fn(),
} as unknown as Wallet;
smartWalletOptions = {
chain: sepolia,
} as SmartWalletOptions;
});

it("connect should handle connection and call onConnect", async () => {
const manager = createConnectionManager(storage);
const onConnect = vi.fn();

await manager.connect(wallet, { client, onConnect });

expect(onConnect).toHaveBeenCalledWith(wallet);
expect(storage.setItem).toHaveBeenCalledWith(expect.any(String), wallet.id);
});

it("handleConnection should connect smart wallet", async () => {
const manager = createConnectionManager(storage);

const smartWallet = await manager.handleConnection(wallet, {
client,
accountAbstraction: smartWalletOptions,
});

expect(manager.activeWalletStore.getValue()).toBe(smartWallet);
});

it("handleConnection should add wallet to connected wallets", async () => {
const manager = createConnectionManager(storage);

await manager.handleConnection(wallet, { client });

expect(manager.connectedWallets.getValue()).toContain(wallet);
});
});
59 changes: 39 additions & 20 deletions packages/thirdweb/src/wallets/manager/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,37 +128,56 @@ export function createConnectionManager(storage: AsyncStorage) {
throw new Error("Can not set a wallet without an account as active");
}

const personalWallet = wallet;
let activeWallet = personalWallet;
const isInAppSmartAccount = hasSmartAccount(wallet);
if (options?.accountAbstraction && !isInAppSmartAccount) {
activeWallet = smartWallet(options.accountAbstraction);
await activeWallet.connect({
personalAccount: wallet.getAccount(),
client: options.client,
});
}
const activeWallet = await (async () => {
if (options?.accountAbstraction && !hasSmartAccount(wallet)) {
return await handleSmartWalletConnection(
account,
options.client,
options.accountAbstraction,
);
} else {
return wallet;
}
})();

// add personal wallet to connected wallets list
addConnectedWallet(personalWallet);
addConnectedWallet(wallet);

if (personalWallet.id !== "smart") {
await storage.setItem(LAST_ACTIVE_EOA_ID, personalWallet.id);
if (wallet.id !== "smart") {
await storage.setItem(LAST_ACTIVE_EOA_ID, wallet.id);
}

handleSetActiveWallet(activeWallet);

wallet.subscribe("accountChanged", async () => {
// We reimplement connect here to prevent memory leaks
const newWallet = await handleConnection(wallet, options);
options?.onConnect?.(newWallet);

Check warning on line 155 in packages/thirdweb/src/wallets/manager/index.ts

View check run for this annotation

Codecov / codecov/patch

packages/thirdweb/src/wallets/manager/index.ts#L154-L155

Added lines #L154 - L155 were not covered by tests
});

return activeWallet;
};

const handleSmartWalletConnection = async (
signer: Account,
client: ThirdwebClient,
options: SmartWalletOptions,
) => {
const wallet = smartWallet(options);

await wallet.connect({
personalAccount: signer,
client: client,
chain: options.chain,
});

return wallet;
};

const connect = async (wallet: Wallet, options?: ConnectManagerOptions) => {
// connectedWallet can be either wallet or smartWallet based on
// connectedWallet can be either wallet or smartWallet
const connectedWallet = await handleConnection(wallet, options);
options?.onConnect?.(connectedWallet);
handleSetActiveWallet(connectedWallet);
wallet.subscribe("accountChanged", async () => {
const newConnectedWallet = await handleConnection(wallet, options);
options?.onConnect?.(newConnectedWallet);
handleSetActiveWallet(newConnectedWallet);
});
return connectedWallet;
};

Expand Down

0 comments on commit c3d7b66

Please sign in to comment.