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

feat: AWS KMS Signer #4289

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
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
4 changes: 4 additions & 0 deletions packages/thirdweb/.env.test.example
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
TEST_AWS_KMS_KEY_ID=""
TEST_AWS_KMS_ACCESS_KEY_ID=""
TEST_AWS_KMS_SECRET_ACCESS_KEY=""
TEST_AWS_KMS_REGION=""
3 changes: 2 additions & 1 deletion packages/thirdweb/.gitignore
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
dist/
coverage/
.watchmanconfig
*storybook.log
*storybook.log
.env.test
4 changes: 3 additions & 1 deletion packages/thirdweb/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,7 @@
"@walletconnect/ethereum-provider": "2.12.2",
"@walletconnect/sign-client": "^2.13.3",
"abitype": "1.0.5",
"aws-kms-signer": "^0.5.3",
"fast-text-encoding": "^1.0.6",
"fuse.js": "7.0.0",
"input-otp": "^1.2.4",
Expand Down Expand Up @@ -310,6 +311,7 @@
"node": ">=18"
},
"devDependencies": {
"@aws-sdk/client-kms": "^3.637.0",
"@aws-sdk/client-lambda": "3.577.0",
"@aws-sdk/credential-providers": "3.577.0",
"@chromatic-com/storybook": "^1.5.0",
Expand Down Expand Up @@ -343,8 +345,8 @@
"react-native-passkey": "3.0.0-beta2",
"react-native-quick-crypto": "0.7.0-rc.6",
"react-native-svg": "15.3.0",
"typescript": "5.5.4",
"storybook": "^8.2.9",
"typescript": "5.5.4",
"vite": "^5.3.1",
"vitest": "1.6.0"
}
Expand Down
143 changes: 143 additions & 0 deletions packages/thirdweb/src/wallets/aws-kms.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,143 @@
import { beforeAll, expect, test } from "vitest";
import { TEST_CLIENT } from "~test/test-clients.js";
import { typedData } from "~test/typed-data.js";
import { ANVIL_CHAIN } from "../../test/src/chains.js";
import { sendTransaction } from "../exports/thirdweb.js";
import { prepareTransaction } from "../transaction/prepare-transaction.js";
import { getAwsKmsAccount } from "./aws-kms.js";
import { getWalletBalance } from "./utils/getWalletBalance.js";

import {
http,
createTestClient,
parseUnits,
verifyMessage,
verifyTypedData,
} from "viem";
import { TEST_AWS_KMS_CONFIG } from "~test/test-aws-kms-config.js";
import { toWei } from "../utils/units.js";

let account: Awaited<ReturnType<typeof getAwsKmsAccount>>;

const anvilTestClient = createTestClient({
mode: "anvil",
transport: http(ANVIL_CHAIN.rpc),
});

beforeAll(async () => {
account = await getAwsKmsAccount({
Copy link
Member

Choose a reason for hiding this comment

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

So this will always hit a live KMS account? And is there any way to keep that address constant?

Copy link
Member Author

Choose a reason for hiding this comment

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

Same KMS keyId would get you the same address. One way to enforce this on a test would be to also expose TEST_AWS_KMS_EXPECTED_ADDRESS and verify it matches.

keyId: TEST_AWS_KMS_CONFIG.keyId,
client: TEST_CLIENT,
config: {
credentials: {
accessKeyId: TEST_AWS_KMS_CONFIG.accessKeyId,
secretAccessKey: TEST_AWS_KMS_CONFIG.secretAccessKey,
},
region: TEST_AWS_KMS_CONFIG.region,
},
});
});

test("account address is valid", () => {
expect(account.address).toMatch(/^0x[a-fA-F0-9]{40}$/);
});

test("sign message", async () => {
const message = "hello world";
const signature = await account.signMessage({ message });

expect(signature).toMatch(/^0x[a-fA-F0-9]{130}$/);

const isValid = await verifyMessage({
d4mr marked this conversation as resolved.
Show resolved Hide resolved
address: account.address,
message,
signature,
});
expect(isValid).toBe(true);
});

test("sign transaction", async () => {
const tx = {
chainId: ANVIL_CHAIN.id,
maxFeePerGas: parseUnits("20", 9),
gas: 21000n,
to: "0x70997970c51812dc3a010c7d01b50e0d17dc79c8",
value: parseUnits("1", 18),
};

expect(account.signTransaction).toBeDefined();

const signedTx = await account.signTransaction?.(tx);
expect(signedTx).toMatch(/^0x[a-fA-F0-9]+$/);

// Optionally, you can use viem to parse the transaction and verify its contents
// This step depends on the exact format of your signed transaction
});

test("sign typed data", async () => {
const signature = await account.signTypedData({
...typedData.basic,
primaryType: "Mail",
});

expect(signature).toMatch(/^0x[a-fA-F0-9]{130}$/);

const isValid = await verifyTypedData({
address: account.address,
...typedData.basic,
primaryType: "Mail",
signature,
});
expect(isValid).toBe(true);
});

test("send transaction", async () => {
const recipient = "0x70997970c51812dc3a010c7d01b50e0d17dc79c8";

await anvilTestClient.setBalance({
address: account.address,
value: toWei("10"),
});

const startingBalance = await getWalletBalance({
address: account.address,
chain: ANVIL_CHAIN,
client: TEST_CLIENT,
});

const startingBalanceRecipient = await getWalletBalance({
address: recipient,
chain: ANVIL_CHAIN,
client: TEST_CLIENT,
});

const tx = prepareTransaction({
client: TEST_CLIENT,
chain: ANVIL_CHAIN,
to: recipient,
value: parseUnits("1", 18),
});

const result = await sendTransaction({
d4mr marked this conversation as resolved.
Show resolved Hide resolved
account,
transaction: tx,
});

expect(result.transactionHash).toMatch(/^0x[a-fA-F0-9]{64}$/);

const endingBalance = await getWalletBalance({
address: account.address,
client: TEST_CLIENT,
chain: ANVIL_CHAIN,
});
const endingBalanceRecipient = await getWalletBalance({
address: recipient,
client: TEST_CLIENT,
chain: ANVIL_CHAIN,
});

expect(endingBalance.value).toBeLessThan(startingBalance.value);
expect(endingBalanceRecipient.value).toBeGreaterThan(
startingBalanceRecipient.value,
);
});
129 changes: 129 additions & 0 deletions packages/thirdweb/src/wallets/aws-kms.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
import type { KMSClientConfig } from "@aws-sdk/client-kms";
import { KmsSigner } from "aws-kms-signer";
import type {
Copy link
Member

Choose a reason for hiding this comment

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

Why viem?

Copy link
Member Author

Choose a reason for hiding this comment

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

just used private-key.ts as reference, is there a different import I should do?

SignableMessage,
TransactionSerializable,
TypedData,
TypedDataDefinition,
} from "viem";
import { hashTypedData, toBytes } from "viem";
import { getCachedChain } from "../chains/utils.js";
import type { ThirdwebClient } from "../client/client.js";
import { eth_sendRawTransaction } from "../rpc/actions/eth_sendRawTransaction.js";
import { getRpcClient } from "../rpc/rpc.js";
import { serializeTransaction } from "../transaction/serialize-transaction.js";
import type { Address } from "../utils/address.js";
import type { Hex } from "../utils/encoding/hex.js";
import { keccak256 } from "../utils/hashing/keccak256.js";
import type { Account } from "./interfaces/wallet.js";

const Buffer = globalThis.Buffer;

type SendTransactionResult = {
transactionHash: Hex;
};

type SendTransactionOption = TransactionSerializable & {
chainId: number;
};

type AwsKmsAccountOptions = {
keyId: string;
config?: KMSClientConfig;
client: ThirdwebClient;
};

export async function getAwsKmsAccount(
options: AwsKmsAccountOptions,
): Promise<Account> {
if (typeof Buffer === "undefined") {
Copy link
Member Author

Choose a reason for hiding this comment

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

because we don't polyfill Buffer, and we don't expect folks to use AWS KMS on browsers, I have explicitly safeguarded against browser usage

throw new Error("AwsKmsAccount only works in Node.js environment");
d4mr marked this conversation as resolved.
Show resolved Hide resolved
}

const { keyId, config, client } = options;
const signer = new KmsSigner(keyId, config);

// Populate address immediately
const addressUnprefixed = await signer.getAddress();
const address = `0x${addressUnprefixed}` as Address;
d4mr marked this conversation as resolved.
Show resolved Hide resolved

async function signTransaction(tx: TransactionSerializable): Promise<Hex> {
const serializedTx = serializeTransaction({ transaction: tx });
const txHash = keccak256(serializedTx);

// we don't polyfill buffer, but signer.sign explicitly requires a buffer
// what do we do here?
d4mr marked this conversation as resolved.
Show resolved Hide resolved
const signature = await signer.sign(Buffer.from(txHash.slice(2), "hex"));

const r = `0x${signature.r.toString("hex")}` as Hex;
const s = `0x${signature.s.toString("hex")}` as Hex;
const v = signature.v;

const yParity = v % 2 === 0 ? 1 : (0 as 0 | 1);

const signedTx = serializeTransaction({
transaction: tx,
signature: {
r,
s,
yParity,
},
});

return signedTx;
}

async function signMessage({
message,
}: {
message: SignableMessage;
}): Promise<Hex> {
let messageHash: Hex;
if (typeof message === "string") {
const prefixedMessage = `\x19Ethereum Signed Message:\n${message.length}${message}`;
Copy link
Member

Choose a reason for hiding this comment

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

What's this prefix?

Copy link
Member Author

@d4mr d4mr Aug 28, 2024

Choose a reason for hiding this comment

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

This is the required prefix for eth_sign & personal_sign according to spec (EIP191): ethereum/go-ethereum#3731 (comment)
https://docs.walletconnect.com/advanced/multichain/rpc-reference/ethereum-rpc#personal_sign
(sorry for the poor sources, can't find good ones after eth migrated their wikis)

Although this raises another question:
SignableMessage can be string or { raw: Hex | ByteArray }. If a raw SignableMessage is passed along, I am not adding a prefix, because the expectation is that its already processed.

This is based on a large assumption I made around the interface. I am not certain this is the expectation from the interface. What do you think?

(to be clear, signing a message requires this prefix. If you sign without this prefix and try to verifyMessage it will fail)

messageHash = keccak256(toBytes(prefixedMessage));
} else if ("raw" in message) {
messageHash = keccak256(message.raw);
} else {
throw new Error("Invalid message format");
}

const signature = await signer.sign(
Buffer.from(messageHash.slice(2), "hex"),
);
return `0x${signature.toString()}`;
}

async function signTypedData<
const typedData extends TypedData | Record<string, unknown>,
primaryType extends keyof typedData | "EIP712Domain" = keyof typedData,
>(_typedData: TypedDataDefinition<typedData, primaryType>): Promise<Hex> {
const typedDataHash = hashTypedData(_typedData);
const signature = await signer.sign(
Buffer.from(typedDataHash.slice(2), "hex"),
);
return `0x${signature.toString()}`;
}

async function sendTransaction(
tx: SendTransactionOption,
): Promise<SendTransactionResult> {
const rpcRequest = getRpcClient({
client: client,
chain: getCachedChain(tx.chainId),
});

const signedTx = await signTransaction(tx);

const transactionHash = await eth_sendRawTransaction(rpcRequest, signedTx);
return { transactionHash };
}

return {
address,
sendTransaction,
signMessage,
signTypedData,
signTransaction,
} satisfies Account;
}
22 changes: 22 additions & 0 deletions packages/thirdweb/test/src/test-aws-kms-config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import { assert } from "vitest";

const TEST_AWS_KMS_ACCESS_KEY_ID = process.env.TEST_AWS_KMS_ACCESS_KEY_ID;
const TEST_AWS_KMS_SECRET_ACCESS_KEY =
process.env.TEST_AWS_KMS_SECRET_ACCESS_KEY;
const TEST_AWS_KMS_KEY_ID = process.env.TEST_AWS_KMS_KEY_ID;
const TEST_AWS_KMS_REGION = process.env.TEST_AWS_KMS_REGION;

assert(TEST_AWS_KMS_ACCESS_KEY_ID, "TEST_AWS_KMS_ACCESS_KEY_ID is required");
assert(
TEST_AWS_KMS_SECRET_ACCESS_KEY,
"TEST_AWS_KMS_SECRET_ACCESS_KEY is required",
);
assert(TEST_AWS_KMS_KEY_ID, "TEST_AWS_KMS_KEY_ID is required");
assert(TEST_AWS_KMS_REGION, "TEST_AWS_KMS_REGION is required");

export const TEST_AWS_KMS_CONFIG = {
accessKeyId: TEST_AWS_KMS_ACCESS_KEY_ID,
secretAccessKey: TEST_AWS_KMS_SECRET_ACCESS_KEY,
region: TEST_AWS_KMS_REGION,
keyId: TEST_AWS_KMS_KEY_ID,
};
Loading
Loading