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.

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

[SDK] Refactor: More Viem to Ox migration #5790

Merged
merged 18 commits into from
Jan 14, 2025
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
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
5 changes: 5 additions & 0 deletions .changeset/hip-llamas-wash.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"thirdweb": patch
---

Migrated underlying functionality to Ox
2 changes: 1 addition & 1 deletion .github/workflows/auto-assign.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ name: Auto Author Assign

on:
pull_request:
types: [opened, reopened]
types: [opened, reopened, ready_for_review, draft]

permissions:
pull-requests: write
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,12 @@ export const AbiSelector: React.FC<AbiSelectorProps> = ({
options={options}
defaultValue={options.find((o) => o.value === defaultValue)}
chakraStyles={{
// @ts-expect-error - this works fine
container: (provided) => ({
...provided,
width: "full",
}),
}}
value={options.find((o) => o.value === value)}
// @ts-expect-error - this works fine
onChange={(selectedFn) => {
if (selectedFn) {
onChange((selectedFn as { label: string; value: string }).value);
Expand Down
4 changes: 2 additions & 2 deletions packages/thirdweb/src/auth/is-erc6492-signature.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { sliceHex } from "viem";
import * as ox__Hex from "ox/Hex";
import type { Hex } from "../utils/encoding/hex.js";
import { ERC_6492_MAGIC_VALUE } from "./constants.js";

Expand All @@ -19,5 +19,5 @@ import { ERC_6492_MAGIC_VALUE } from "./constants.js";
* @auth
*/
export function isErc6492Signature(signature: Hex): boolean {
return sliceHex(signature, -32) === ERC_6492_MAGIC_VALUE;
return ox__Hex.slice(signature, -32) === ERC_6492_MAGIC_VALUE;
}
14 changes: 10 additions & 4 deletions packages/thirdweb/src/auth/parse-erc6492-signature.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import { decodeAbiParameters, isErc6492Signature } from "viem";
import * as ox__AbiParameters from "ox/AbiParameters";
import * as ox__Address from "ox/Address";
import { WrappedSignature as ox__WrappedSignature } from "ox/erc6492";
import type { Hex } from "../utils/encoding/hex.js";
import type { OneOf } from "../utils/type-utils.js";
import type { Erc6492Signature } from "./types.js";
Expand Down Expand Up @@ -29,13 +31,17 @@ export type ParseErc6492SignatureReturnType = OneOf<
export function parseErc6492Signature(
signature: Hex,
): ParseErc6492SignatureReturnType {
if (!isErc6492Signature(signature)) {
if (!ox__WrappedSignature.validate(signature)) {
return { signature };
}

const [address, data, originalSignature] = decodeAbiParameters(
const [address, data, originalSignature] = ox__AbiParameters.decode(
[{ type: "address" }, { type: "bytes" }, { type: "bytes" }],
signature,
);
return { address: address, data, signature: originalSignature };
return {
address: ox__Address.checksum(address),
data,
signature: originalSignature,
};
}
39 changes: 17 additions & 22 deletions packages/thirdweb/src/auth/verify-hash.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,8 @@
import {
type Signature,
encodeDeployData,
encodeFunctionData,
isErc6492Signature,
serializeSignature,
universalSignatureValidatorAbi,
universalSignatureValidatorByteCode,
} from "viem";
import * as ox__Abi from "ox/Abi";
import * as ox__AbiConstructor from "ox/AbiConstructor";
import * as ox__AbiFunction from "ox/AbiFunction";
import * as ox__Signature from "ox/Signature";
import { WrappedSignature as ox__WrappedSignature } from "ox/erc6492";
import type { Chain } from "../chains/types.js";
import type { ThirdwebClient } from "../client/client.js";
import { type ThirdwebContract, getContract } from "../contract/contract.js";
Expand All @@ -20,7 +16,7 @@

export type VerifyHashParams = {
hash: Hex;
signature: string | Uint8Array | Signature;
signature: string | Uint8Array | ox__Signature.Signature;
address: string;
client: ThirdwebClient;
chain: Chain;
Expand Down Expand Up @@ -71,7 +67,7 @@
const signatureHex = (() => {
if (isHex(signature)) return signature;
if (typeof signature === "object" && "r" in signature && "s" in signature)
return serializeSignature(signature);
return ox__Signature.toHex(signature);
if (signature instanceof Uint8Array) return fromBytes(signature, "hex");
// We should never hit this but TS doesn't know that
throw new Error(
Expand All @@ -85,7 +81,7 @@
if (!accountFactory) return signatureHex;

// If this sigature was already wrapped for ERC-6492, carry on
if (isErc6492Signature(signatureHex)) return signatureHex;
if (ox__WrappedSignature.validate(signatureHex)) return signatureHex;

Check warning on line 84 in packages/thirdweb/src/auth/verify-hash.ts

View check run for this annotation

Codecov / codecov/patch

packages/thirdweb/src/auth/verify-hash.ts#L84

Added line #L84 was not covered by tests

// Otherwise, serialize the signature for ERC-6492 validation
return serializeErc6492Signature({
Expand All @@ -100,23 +96,23 @@
data: Hex;
};
const zkSyncChain = await isZkSyncChain(chain);
const abi = ox__Abi.from(ox__WrappedSignature.universalSignatureValidatorAbi);
if (zkSyncChain) {
// zksync chains dont support deploying code with eth_call
// need to call a deployed contract instead
verificationData = {
to: ZKSYNC_VALIDATOR_ADDRESS,
data: encodeFunctionData({
abi: universalSignatureValidatorAbi,
functionName: "isValidSig",
args: [address, hash, wrappedSignature],
}),
data: ox__AbiFunction.encodeData(
ox__AbiFunction.fromAbi(abi, "isValidSig"),
[address, hash, wrappedSignature],
),

Check warning on line 108 in packages/thirdweb/src/auth/verify-hash.ts

View check run for this annotation

Codecov / codecov/patch

packages/thirdweb/src/auth/verify-hash.ts#L105-L108

Added lines #L105 - L108 were not covered by tests
};
} else {
const validatorConstructor = ox__AbiConstructor.fromAbi(abi);
verificationData = {
data: encodeDeployData({
abi: universalSignatureValidatorAbi,
data: ox__AbiConstructor.encode(validatorConstructor, {
args: [address, hash, wrappedSignature],
bytecode: universalSignatureValidatorByteCode,
bytecode: ox__WrappedSignature.universalSignatureValidatorBytecode,
}),
};
}
Expand All @@ -129,8 +125,7 @@
try {
const result = await eth_call(rpcRequest, verificationData);
return hexToBool(result);
} catch (err) {
console.error("Error verifying ERC-6492 signature", err);
} catch (_err) {
// Some chains do not support the eth_call simulation and will fail, so we fall back to regular EIP1271 validation
const validEip1271 = await verifyEip1271Signature({
hash,
Expand Down
50 changes: 48 additions & 2 deletions packages/thirdweb/src/auth/verify-signature.test.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,16 @@
import * as ox__Bytes from "ox/Bytes";
import { describe, expect, it, test } from "vitest";
import { FORKED_ETHEREUM_CHAIN } from "../../test/src/chains.js";
import { TEST_CLIENT } from "../../test/src/test-clients.js";
import { TEST_ACCOUNT_A } from "../../test/src/test-wallets.js";
import { mainnet } from "../chains/chain-definitions/ethereum.js";
import { ethereum, mainnet } from "../chains/chain-definitions/ethereum.js";
import { sepolia } from "../chains/chain-definitions/sepolia.js";
import { verifyEOASignature, verifySignature } from "./verify-signature.js";
import { smartWallet } from "../wallets/smart/smart-wallet.js";
import {
verifyContractWalletSignature,
verifyEOASignature,
verifySignature,
} from "./verify-signature.js";

describe("verifyEOASignature", () => {
test("should return true for a valid signature", async () => {
Expand Down Expand Up @@ -98,3 +104,43 @@ describe.runIf(process.env.TW_SECRET_KEY)(
});
},
);

describe.runIf(process.env.TW_SECRET_KEY)(
"verifyContractWalletSignature",
async () => {
const message = "Hakuna matata";
const wallet = smartWallet({
chain: ethereum,
gasless: true,
});
const smartAccount = await wallet.connect({
client: TEST_CLIENT,
personalAccount: TEST_ACCOUNT_A,
});

test("should verify a smart account signature", async () => {
const rawSignature = await smartAccount.signMessage({ message });
const result = await verifyContractWalletSignature({
signature: rawSignature,
message,
address: smartAccount.address,
chain: ethereum,
client: TEST_CLIENT,
});
expect(result).toBe(true);
});

test("should verify a smart account signature as bytes", async () => {
const rawSignature = await smartAccount.signMessage({ message });
const bytesSignature = ox__Bytes.fromHex(rawSignature);
const result = await verifyContractWalletSignature({
signature: bytesSignature,
message,
address: smartAccount.address,
chain: ethereum,
client: TEST_CLIENT,
});
expect(result).toBe(true);
});
},
);
31 changes: 24 additions & 7 deletions packages/thirdweb/src/auth/verify-signature.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,26 @@
import { type SignableMessage, type Signature, recoverAddress } from "viem";
import * as ox__Bytes from "ox/Bytes";
import * as ox__Secp256k1 from "ox/Secp256k1";
import * as ox__Signature from "ox/Signature";
import type { Chain } from "../chains/types.js";
import type { ThirdwebClient } from "../client/client.js";
import { type Hex, isHex } from "../utils/encoding/hex.js";
import { hashMessage } from "../utils/hashing/hashMessage.js";
import type { Prettify } from "../utils/type-utils.js";
import { verifyHash } from "./verify-hash.js";

type Message = Prettify<
| string
| {
raw: Hex | Uint8Array;
}
>;

/**
* @auth
*/
export type VerifyEOASignatureParams = {
message: string | SignableMessage;
signature: string | Uint8Array | Signature;
message: string | Message;
signature: string | Uint8Array;
address: string;
};

Expand Down Expand Up @@ -39,9 +48,9 @@ export async function verifyEOASignature(options: VerifyEOASignatureParams) {
return false;
}

const recoveredAddress = await recoverAddress({
hash: messageHash,
signature: options.signature,
const recoveredAddress = ox__Secp256k1.recoverAddress({
payload: messageHash,
signature: ox__Signature.fromHex(options.signature),
});

if (recoveredAddress.toLowerCase() === options.address.toLowerCase()) {
Expand Down Expand Up @@ -103,9 +112,17 @@ export async function verifyContractWalletSignature({
accountFactory,
}: VerifyContractWalletSignatureParams) {
const messageHash = hashMessage(message);

const parsedSignature = (() => {
if (ox__Bytes.validate(signature)) {
Copy link
Member

Choose a reason for hiding this comment

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

this seems a bit scary? maybe add tests for these sub cases?

Copy link
Member Author

Choose a reason for hiding this comment

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

Added a few tests and removed signature object support (we can't support it until we re-add 1271 support. What's scary about it though? It's just parsing the input to the format we prefer

return ox__Bytes.toHex(signature);
}
return signature;
})();

return verifyHash({
hash: messageHash,
signature,
signature: parsedSignature,
address,
client,
chain,
Expand Down
16 changes: 9 additions & 7 deletions packages/thirdweb/src/auth/verify-typed-data.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,20 @@
import type { Signature, TypedData, TypedDataDefinition } from "viem";
import { hashTypedData } from "viem";
import type * as ox__Signature from "ox/Signature";
import * as ox__TypedData from "ox/TypedData";
import type { Chain } from "../chains/types.js";
import type { ThirdwebClient } from "../client/client.js";
import type { Hex } from "../utils/encoding/hex.js";
import type { HashTypedDataParams } from "../utils/hashing/hashTypedData.js";
import { type VerifyHashParams, verifyHash } from "./verify-hash.js";

export type VerifyTypedDataParams<
typedData extends TypedData | Record<string, unknown> = TypedData,
typedData extends
| ox__TypedData.TypedData
| Record<string, unknown> = ox__TypedData.TypedData,
primaryType extends keyof typedData | "EIP712Domain" = keyof typedData,
> = Omit<VerifyHashParams, "hash"> &
TypedDataDefinition<typedData, primaryType> & {
ox__TypedData.Definition<typedData, primaryType> & {
address: string;
signature: string | Uint8Array | Signature;
signature: string | Uint8Array | ox__Signature.Signature;
client: ThirdwebClient;
chain: Chain;
accountFactory?: {
Expand Down Expand Up @@ -80,7 +82,7 @@ export type VerifyTypedDataParams<
* @auth
*/
export async function verifyTypedData<
typedData extends TypedData | Record<string, unknown>,
typedData extends ox__TypedData.TypedData | Record<string, unknown>,
primaryType extends keyof typedData | "EIP712Domain",
>({
address,
Expand All @@ -93,7 +95,7 @@ export async function verifyTypedData<
primaryType,
types,
}: VerifyTypedDataParams<typedData, primaryType>): Promise<boolean> {
const messageHash = hashTypedData({
const messageHash = ox__TypedData.getSignPayload({
message,
domain,
primaryType,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import { getContractAddress } from "viem";
import * as ox__ContractAddress from "ox/ContractAddress";
import { getGasPrice } from "../../../gas/get-gas-price.js";
import { eth_getBalance } from "../../../rpc/actions/eth_getBalance.js";
import { eth_sendRawTransaction } from "../../../rpc/actions/eth_sendRawTransaction.js";
import { getRpcClient } from "../../../rpc/rpc.js";
import { sendTransaction } from "../../../transaction/actions/send-transaction.js";
import { waitForReceipt } from "../../../transaction/actions/wait-for-tx-receipt.js";
import { prepareTransaction } from "../../../transaction/prepare-transaction.js";
import { getAddress } from "../../../utils/address.js";
import { isEIP155Enforced } from "../../../utils/any-evm/is-eip155-enforced.js";
import { getKeylessTransaction } from "../../../utils/any-evm/keyless-transaction.js";
import { isContractDeployed } from "../../../utils/bytecode/is-contract-deployed.js";
Expand Down Expand Up @@ -226,15 +227,15 @@ async function _getCreate2FactoryDeploymentInfo(
},
signature: SIGNATURE,
});
const create2FactoryAddress = getContractAddress({
const create2FactoryAddress = ox__ContractAddress.from({
from: deploymentTransaction.signerAddress,
nonce: 0n,
});

return {
...deploymentTransaction,
valueToSend: gasPrice * gas,
predictedAddress: create2FactoryAddress,
predictedAddress: getAddress(create2FactoryAddress),
};
}

Expand Down
Loading
Loading