Skip to content

Commit

Permalink
rpcUrl as required and removed redundant code (#430)
Browse files Browse the repository at this point in the history 
* Added rpc url as required field and removed redundant code

* Added check comparisson for chain ids (signer-bundler)

* Fixed tests

* Improved fix, refactor rpcUrl back as optional + validate rpcUrl

---------

Co-authored-by: GabiDev <gv@popoo.io>
  • Loading branch information
@VGabriel45 @GabiDev45
VGabriel45 and GabiDev45 authored Feb 26, 2024
1 parent 6d2fb27 commit 1949e68
Show file tree
Hide file tree
Showing 7 changed files with 56 additions and 53 deletions.
18 changes: 15 additions & 3 deletions packages/account/src/BiconomySmartAccountV2.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import {
BatchUserOperationCallData,
SmartAccountSigner,
} from "@alchemy/aa-core";
import { isNullOrUndefined, packUserOp } from "./utils/Utils.js";
import { isNullOrUndefined, isValidRpcUrl, packUserOp } from "./utils/Utils.js";
import { BaseValidationModule, ModuleInfo, SendUserOpParams, createECDSAOwnershipValidationModule } from "@biconomy/modules";
import {
IHybridPaymaster,
Expand Down Expand Up @@ -197,17 +197,29 @@ export class BiconomySmartAccountV2 extends BaseSmartContractAccount {
*/
public static async create(biconomySmartAccountConfig: BiconomySmartAccountV2Config): Promise<BiconomySmartAccountV2> {
let chainId = biconomySmartAccountConfig.chainId;
let resolvedSmartAccountSigner!: SmartAccountSigner;
let rpcUrl = biconomySmartAccountConfig.rpcUrl;
let resolvedSmartAccountSigner!: SmartAccountSigner;

// Signer needs to be initialised here before defaultValidationModule is set
if (biconomySmartAccountConfig.signer) {
const signerResult = await convertSigner(biconomySmartAccountConfig.signer, !!chainId);
if (!chainId && !!signerResult.chainId) {
let chainIdFromBundler: number | undefined;
if (biconomySmartAccountConfig.bundlerUrl) {
chainIdFromBundler = extractChainIdFromBundlerUrl(biconomySmartAccountConfig.bundlerUrl);
} else if (biconomySmartAccountConfig.bundler) {
const bundlerUrlFromBundler = biconomySmartAccountConfig.bundler.getBundlerUrl();
chainIdFromBundler = extractChainIdFromBundlerUrl(bundlerUrlFromBundler);
}
if (chainIdFromBundler !== signerResult.chainId) {
throw new Error("ChainId from bundler and signer do not match");
}
chainId = signerResult.chainId;
}
if (!rpcUrl && !!signerResult.rpcUrl) {
rpcUrl = signerResult.rpcUrl;
if (isValidRpcUrl(signerResult.rpcUrl)) {
rpcUrl = signerResult.rpcUrl;
}
}
resolvedSmartAccountSigner = signerResult.signer;
}
Expand Down
2 changes: 1 addition & 1 deletion packages/account/src/utils/Types.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ export type BiconomySmartAccountV2ConfigBaseProps = {
implementationAddress?: Hex;
/** defaultFallbackHandler: override the default fallback contract address */
defaultFallbackHandler?: Hex;
/** rpcUrl: Explicitly set the rpc else it is pulled out of the signer. */
/** rpcUrl: Rpc url, optional, we set default rpc url if not passed. */
rpcUrl?: string; // as good as Provider
/** biconomyPaymasterApiKey: The API key retrieved from the Biconomy dashboard */
biconomyPaymasterApiKey?: string;
Expand Down
5 changes: 5 additions & 0 deletions packages/account/src/utils/Utils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,3 +43,8 @@ export function packUserOp(op: Partial<UserOperationStruct>, forSignature = true
export const isNullOrUndefined = (value: any): value is undefined => {
return value === null || value === undefined;
};

export const isValidRpcUrl = (url: string): boolean => {
const regex = /^(https:\/\/|wss:\/\/).*/;
return regex.test(url);
};
4 changes: 3 additions & 1 deletion packages/account/tests/account.e2e.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ describe("Account Tests", () => {

const reciepientSmartAccountBase = await createSmartAccountClient({
signer: recipientSignerBase,
bundlerUrl,
bundlerUrl: bundlerUrlBase,
});

const addresses = await Promise.all([
Expand Down Expand Up @@ -454,11 +454,13 @@ describe("Account Tests", () => {
const {
whale: { viemWallet: signer },
bundlerUrl,
viemChain,
} = mumbai;

const smartAccount = await createSmartAccountClient({
signer,
bundlerUrl,
rpcUrl: viemChain.rpcUrls.default.http[0],
});

expect(ecdsaOwnershipModule).toBe(smartAccount.activeValidationModule.getAddress());
Expand Down
69 changes: 25 additions & 44 deletions packages/account/tests/account.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { Wallet } from "@ethersproject/wallet";

describe("Account Tests", () => {
let ganache: TestData;
const mockBundlerUrl = "https://bundler.biconomy.io/api/v2/1337/nJPK7B3ru.dd7f7861-190d-41bd-af80-6877f74b8f14";

beforeEach(() => {
// @ts-ignore: Comes from setup-unit-tests
Expand All @@ -16,13 +17,13 @@ describe("Account Tests", () => {

it("should create a smartAccountClient from an ethers signer", async () => {
const {
bundlerUrl,
minnow: { ethersSigner: signer },
} = ganache;

const smartAccount = await createSmartAccountClient({
signer,
bundlerUrl,
bundlerUrl: mockBundlerUrl,
rpcUrl: localhost.rpcUrls.default.http[0],
});
const address = await smartAccount.getAccountAddress();
expect(address).toBeTruthy();
Expand All @@ -31,12 +32,12 @@ describe("Account Tests", () => {
it("should create a smartAccountClient from a walletClient", async () => {
const {
whale: { viemWallet: signer },
bundlerUrl,
} = ganache;

const smartAccount = await createSmartAccountClient({
signer,
bundlerUrl,
bundlerUrl: mockBundlerUrl,
rpcUrl: localhost.rpcUrls.default.http[0],
});
const address = await smartAccount.getAccountAddress();
expect(address).toBeTruthy();
Expand All @@ -45,7 +46,6 @@ describe("Account Tests", () => {
it("should pickup the rpcUrl from viem wallet and ethers", async () => {
const {
chainId,
bundlerUrl,
viemChain,
whale: { privateKey, viemWallet: originalViemSigner, ethersSigner: originalEthersSigner },
} = ganache;
Expand All @@ -68,22 +68,26 @@ describe("Account Tests", () => {
createSmartAccountClient({
chainId,
signer: ethersSignerWithNewRpcUrl,
bundlerUrl,
bundlerUrl: mockBundlerUrl,
rpcUrl: newRpcUrl,
}),
createSmartAccountClient({
chainId,
signer: walletClientWithNewRpcUrl,
bundlerUrl,
bundlerUrl: mockBundlerUrl,
rpcUrl: newRpcUrl,
}),
createSmartAccountClient({
chainId,
signer: originalEthersSigner,
bundlerUrl,
bundlerUrl: mockBundlerUrl,
rpcUrl: viemChain.rpcUrls.default.http[0],
}),
createSmartAccountClient({
chainId,
signer: originalViemSigner,
bundlerUrl,
bundlerUrl: mockBundlerUrl,
rpcUrl: viemChain.rpcUrls.default.http[0],
}),
]);

Expand Down Expand Up @@ -118,62 +122,41 @@ describe("Account Tests", () => {
const {
chainId,
whale: { alchemyWalletClientSigner: signer },
bundlerUrl,
} = ganache;

const smartAccount = await createSmartAccountClient({
chainId,
signer,
bundlerUrl,
bundlerUrl: mockBundlerUrl,
rpcUrl: localhost.rpcUrls.default.http[0],
});
const address = await smartAccount.getAccountAddress();
expect(address).toBeTruthy();
});

it("should provide an account address", async () => {
const {
bundlerUrl,
whale: { viemWallet: signer },
} = ganache;

const smartAccount = await createSmartAccountClient({
signer,
bundlerUrl,
bundlerUrl: mockBundlerUrl,
rpcUrl: localhost.rpcUrls.default.http[0],
});
const address = await smartAccount.getAccountAddress();
expect(address).toBeTruthy();
});

it("Nonce should be zero", async () => {
const {
entryPointAddress,
bundlerUrl,
whale: { viemWallet: signer },
minnow: { publicAddress: recipient },
} = ganache;

const smartAccount = await createSmartAccountClient({
entryPointAddress,
signer,
bundlerUrl,
});
const address = await smartAccount.getAccountAddress();
expect(address).toBeTruthy();

const builtUserOp = await smartAccount.buildUserOp([{ to: recipient, value: 1 }]);
console.log("builtUserOp", builtUserOp);
expect(builtUserOp?.nonce?.toString()).toBe("0x0");
}, 10000);

it("should have an active validation module", async () => {
const {
bundlerUrl,
whale: { viemWallet: signer },
} = ganache;

const smartAccount = await createSmartAccountClient({
signer,
bundlerUrl,
bundlerUrl: mockBundlerUrl,
rpcUrl: localhost.rpcUrls.default.http[0],
});

const module = smartAccount.activeValidationModule;
Expand All @@ -183,7 +166,6 @@ describe("Account Tests", () => {
it("Create a smart account with paymaster by creating instance", async () => {
const {
whale: { viemWallet: signer },
bundlerUrl,
biconomyPaymasterApiKey,
} = ganache;

Expand All @@ -192,16 +174,15 @@ describe("Account Tests", () => {

const smartAccount = await createSmartAccountClient({
signer,
bundlerUrl,
bundlerUrl: mockBundlerUrl,
paymaster,
rpcUrl: localhost.rpcUrls.default.http[0],
});
expect(smartAccount.paymaster).not.toBeNull();
expect(smartAccount.paymaster).not.toBeUndefined();
}, 10000);

it("should fail to create a smartAccountClient from a walletClient without a chainId", async () => {
const { bundlerUrl } = ganache;

const account = privateKeyToAccount(generatePrivateKey());
const viemWalletClientNoChainId = createWalletClient({
account,
Expand All @@ -212,23 +193,23 @@ describe("Account Tests", () => {
await expect(
createSmartAccountClient({
signer: viemWalletClientNoChainId,
bundlerUrl,
bundlerUrl: mockBundlerUrl,
rpcUrl: localhost.rpcUrls.default.http[0],
}),
).rejects.toThrow("Cannot consume a viem wallet without a chainId"),
);
});

it("should fail to create a smartAccountClient from a walletClient without an account", async () => {
const { bundlerUrl } = ganache;

const viemWalletNoAccount = createWalletClient({
transport: http(localhost.rpcUrls.default.http[0]),
});

expect(async () =>
createSmartAccountClient({
signer: viemWalletNoAccount,
bundlerUrl,
bundlerUrl: mockBundlerUrl,
rpcUrl: localhost.rpcUrls.default.http[0],
}),
).rejects.toThrow("Cannot consume a viem wallet without an account");
});
Expand Down
5 changes: 4 additions & 1 deletion packages/modules/tests/modules.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,12 @@ describe("Account Tests", () => {
const {
bundlerUrl,
whale: { ethersSigner: signer },
viemChain,
} = ganache;

const defaultValidationModule = await createMultiChainValidationModule({ signer });
// Should not require a signer or chainId
const smartAccount = await createSmartAccountClient({ bundlerUrl, defaultValidationModule });
const smartAccount = await createSmartAccountClient({ bundlerUrl, defaultValidationModule, rpcUrl: viemChain.rpcUrls.default.http[0], });
const address = await smartAccount.getAccountAddress();
expect(address).toBeTruthy();
// expect the relevant module to be set
Expand All @@ -29,13 +30,15 @@ describe("Account Tests", () => {
const {
bundlerUrl,
whale: { viemWallet: signer },
viemChain,
} = ganache;

const defaultValidationModule = await createECDSAOwnershipValidationModule({ signer });
// Should not require a signer or chainId
const smartAccount = await createSmartAccountClient({
bundlerUrl,
defaultValidationModule,
rpcUrl: viemChain.rpcUrls.default.http[0],
});
const address = await smartAccount.getAccountAddress();
expect(address).toBeTruthy();
Expand Down
6 changes: 3 additions & 3 deletions yarn.lock
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9175,9 +9175,9 @@ typedarray@^0.0.6:
integrity sha512-/aCDEGatGvZ2BIk+HmLf4ifCJFwvKFNb9/JeZPMulfgFracn9QFcAf5GO8B/mweUjSoblS5In0cWhqpfs/5PQA==

typedoc@^0.25.7:
version "0.25.8"
resolved "https://registry.yarnpkg.com/typedoc/-/typedoc-0.25.8.tgz#7d0e1bf12d23bf1c459fd4893c82cb855911ff12"
integrity sha512-mh8oLW66nwmeB9uTa0Bdcjfis+48bAjSH3uqdzSuSawfduROQLlXw//WSNZLYDdhmMVB7YcYZicq6e8T0d271A==
version "0.25.9"
resolved "https://registry.yarnpkg.com/typedoc/-/typedoc-0.25.9.tgz#0fb6608feec994eedc1e3276154fa8a486218ed2"
integrity sha512-jVoGmfNw848iW0L313+jqHbsknepwDV6F9nzk1H30oWhKXkw65uaENgR6QtTw9a5KqRWEb6nwNd54KxffBJyWw==
dependencies:
lunr "^2.3.9"
marked "^4.3.0"
Expand Down

0 comments on commit 1949e68

Please sign in to comment.