From 6d7add68dcb862d5e4dec5ca00354bb8ba9e2cdf Mon Sep 17 00:00:00 2001 From: PH <68947167+osslgtm@users.noreply.github.com> Date: Mon, 31 Jul 2023 09:21:11 +0000 Subject: [PATCH 1/2] feat: input validation --- .../title-escrow/acceptSurrendered.test.ts | 4 +- .../endorseNominatedBeneficiary.test.ts | 6 +- src/implementations/title-escrow/helpers.ts | 26 +-------- .../title-escrow/nominateBeneficiary.test.ts | 6 +- .../title-escrow/rejectSurrendered.test.ts | 6 +- .../title-escrow/rejectSurrendered.ts | 3 +- .../title-escrow/surrenderDocument.test.ts | 6 +- .../title-escrow/transferHolder.test.ts | 6 +- .../title-escrow/transferOwners.test.ts | 6 +- .../token-registry/issue.test.ts | 4 +- src/implementations/token-registry/issue.ts | 3 +- src/implementations/utils/connect.ts | 57 +++++++++++++++++++ 12 files changed, 86 insertions(+), 47 deletions(-) create mode 100644 src/implementations/utils/connect.ts diff --git a/src/implementations/title-escrow/acceptSurrendered.test.ts b/src/implementations/title-escrow/acceptSurrendered.test.ts index 10316c52..7aa5e293 100644 --- a/src/implementations/title-escrow/acceptSurrendered.test.ts +++ b/src/implementations/title-escrow/acceptSurrendered.test.ts @@ -7,8 +7,8 @@ import { acceptSurrendered } from "./acceptSurrendered"; jest.mock("@govtechsg/token-registry/contracts"); const acceptSurrenderedDocumentParams: TitleEscrowSurrenderDocumentCommand = { - tokenRegistry: "0x1122", - tokenId: "0x12345", + tokenRegistry: "0x0000000000000000000000000000000000000001", + tokenId: "0x0000000000000000000000000000000000000000000000000000000000000001", network: "sepolia", dryRun: false, }; diff --git a/src/implementations/title-escrow/endorseNominatedBeneficiary.test.ts b/src/implementations/title-escrow/endorseNominatedBeneficiary.test.ts index cd3a3422..3c5297e2 100644 --- a/src/implementations/title-escrow/endorseNominatedBeneficiary.test.ts +++ b/src/implementations/title-escrow/endorseNominatedBeneficiary.test.ts @@ -7,8 +7,8 @@ import { endorseNominatedBeneficiary } from "./endorseNominatedBeneficiary"; jest.mock("@govtechsg/token-registry/contracts"); const endorseNominatedBeneficiaryParams: TitleEscrowNominateBeneficiaryCommand = { - tokenId: "0xzyxw", - tokenRegistry: "0x1234", + tokenId: "0x0000000000000000000000000000000000000000000000000000000000000001", + tokenRegistry: "0x0000000000000000000000000000000000000001", newBeneficiary: "0x1232", network: "sepolia", dryRun: false, @@ -25,7 +25,7 @@ describe("title-escrow", () => { // @ts-ignore mock static method const mockedConnectTokenFactory: jest.Mock = mockedTokenFactory.connect; - const mockedTitleEscrowAddress = "0x2133"; + const mockedTitleEscrowAddress = "0x0000000000000000000000000000000000000002"; const mockedOwnerOf = jest.fn(); mockedOwnerOf.mockReturnValue(mockedTitleEscrowAddress); diff --git a/src/implementations/title-escrow/helpers.ts b/src/implementations/title-escrow/helpers.ts index 41593019..69c6e04b 100644 --- a/src/implementations/title-escrow/helpers.ts +++ b/src/implementations/title-escrow/helpers.ts @@ -1,28 +1,8 @@ -import { - TitleEscrow, - TitleEscrow__factory, - TradeTrustToken, - TradeTrustToken__factory, -} from "@govtechsg/token-registry/contracts"; -import { Wallet, constants } from "ethers"; +import { TitleEscrow } from "@govtechsg/token-registry/contracts"; +import { constants } from "ethers"; import signale from "signale"; -import { ConnectedSigner } from "../utils/wallet"; -interface ConnectToTitleEscrowArgs { - tokenId: string; - address: string; - wallet: Wallet | ConnectedSigner; -} - -export const connectToTitleEscrow = async ({ - tokenId, - address, - wallet, -}: ConnectToTitleEscrowArgs): Promise => { - const tokenRegistry: TradeTrustToken = await TradeTrustToken__factory.connect(address, wallet); - const titleEscrowAddress = await tokenRegistry.ownerOf(tokenId); - return await TitleEscrow__factory.connect(titleEscrowAddress, wallet); -}; +export { connectToTitleEscrow, connectToTitleEscrowAddress } from "../utils/connect"; interface validateEndorseChangeOwnerArgs { newHolder: string; diff --git a/src/implementations/title-escrow/nominateBeneficiary.test.ts b/src/implementations/title-escrow/nominateBeneficiary.test.ts index 6eaf918d..ded32a3f 100644 --- a/src/implementations/title-escrow/nominateBeneficiary.test.ts +++ b/src/implementations/title-escrow/nominateBeneficiary.test.ts @@ -8,8 +8,8 @@ jest.mock("@govtechsg/token-registry/contracts"); const nominateBeneficiaryParams: TitleEscrowNominateBeneficiaryCommand = { newBeneficiary: "0fosui", - tokenId: "0xzyxw", - tokenRegistry: "0x1234", + tokenId: "0x0000000000000000000000000000000000000000000000000000000000000001", + tokenRegistry: "0x0000000000000000000000000000000000000001", network: "sepolia", dryRun: false, }; @@ -26,7 +26,7 @@ describe("title-escrow", () => { const mockedConnectTokenFactory: jest.Mock = mockedTokenFactory.connect; const mockedOwnerOf = jest.fn(); const mockNominateBeneficiary = jest.fn(); - const mockedTitleEscrowAddress = "0x2133"; + const mockedTitleEscrowAddress = "0x0000000000000000000000000000000000000002"; const mockedBeneficiary = "0xdssfs"; const mockedHolder = "0xdsfls"; const mockGetBeneficiary = jest.fn(); diff --git a/src/implementations/title-escrow/rejectSurrendered.test.ts b/src/implementations/title-escrow/rejectSurrendered.test.ts index fe470db6..ec90f1e0 100644 --- a/src/implementations/title-escrow/rejectSurrendered.test.ts +++ b/src/implementations/title-escrow/rejectSurrendered.test.ts @@ -7,8 +7,8 @@ import { rejectSurrendered } from "./rejectSurrendered"; jest.mock("@govtechsg/token-registry/contracts"); const rejectSurrenderedDocumentParams: TitleEscrowSurrenderDocumentCommand = { - tokenRegistry: "0x1122", - tokenId: "0x12345", + tokenRegistry: "0x0000000000000000000000000000000000000001", + tokenId: "0x0000000000000000000000000000000000000000000000000000000000000001", network: "sepolia", dryRun: false, }; @@ -49,7 +49,7 @@ describe("title-escrow", () => { wait: () => Promise.resolve({ transactionHash: "transactionHash" }), }); mockTransferEvent.mockReturnValue({ - address: "0x1122", + address: "0x0000000000000000000000000000000000000002", topics: ["0x00000", null, null, "0x12345"], }); mockQueryFilter.mockReturnValue([ diff --git a/src/implementations/title-escrow/rejectSurrendered.ts b/src/implementations/title-escrow/rejectSurrendered.ts index 098fb0ef..34dfd221 100644 --- a/src/implementations/title-escrow/rejectSurrendered.ts +++ b/src/implementations/title-escrow/rejectSurrendered.ts @@ -5,6 +5,7 @@ import { getWalletOrSigner } from "../utils/wallet"; import { BaseTitleEscrowCommand as TitleEscrowSurrenderDocumentCommand } from "../../commands/title-escrow/title-escrow-command.type"; import { dryRunMode } from "../utils/dryRun"; import { TransactionReceipt } from "@ethersproject/providers"; +import { connectToTokenRegistry } from "../utils/connect"; const { trace } = getLogger("title-escrow:acceptSurrendered"); @@ -16,7 +17,7 @@ export const rejectSurrendered = async ({ ...rest }: TitleEscrowSurrenderDocumentCommand): Promise => { const wallet = await getWalletOrSigner({ network, ...rest }); - const tokenRegistryInstance: TradeTrustToken = await TradeTrustToken__factory.connect(address, wallet); + const tokenRegistryInstance: TradeTrustToken = await connectToTokenRegistry(address, wallet); if (dryRun) { await dryRunMode({ estimatedGas: await tokenRegistryInstance.estimateGas.restore(tokenId), diff --git a/src/implementations/title-escrow/surrenderDocument.test.ts b/src/implementations/title-escrow/surrenderDocument.test.ts index 25150301..7e99c364 100644 --- a/src/implementations/title-escrow/surrenderDocument.test.ts +++ b/src/implementations/title-escrow/surrenderDocument.test.ts @@ -7,8 +7,8 @@ import { surrenderDocument } from "./surrenderDocument"; jest.mock("@govtechsg/token-registry/contracts"); const surrenderDocumentParams: TitleEscrowSurrenderDocumentCommand = { - tokenRegistry: "0x1122", - tokenId: "0x12345", + tokenRegistry: "0x0000000000000000000000000000000000000001", + tokenId: "0x0000000000000000000000000000000000000000000000000000000000000001", network: "sepolia", dryRun: false, }; @@ -26,7 +26,7 @@ describe("title-escrow", () => { const mockedOwnerOf = jest.fn(); const mockSurrender = jest.fn(); const mockCallStaticSurrender = jest.fn().mockResolvedValue(undefined); - const mockedTitleEscrowAddress = "0x2133"; + const mockedTitleEscrowAddress = "0x0000000000000000000000000000000000000002"; beforeEach(() => { delete process.env.OA_PRIVATE_KEY; diff --git a/src/implementations/title-escrow/transferHolder.test.ts b/src/implementations/title-escrow/transferHolder.test.ts index 6bfc1ff0..03d50316 100644 --- a/src/implementations/title-escrow/transferHolder.test.ts +++ b/src/implementations/title-escrow/transferHolder.test.ts @@ -8,8 +8,8 @@ jest.mock("@govtechsg/token-registry/contracts"); const transferHolderParams: TitleEscrowTransferHolderCommand = { newHolder: "0xabcd", - tokenId: "0xzyxw", - tokenRegistry: "0x1234", + tokenId: "0x0000000000000000000000000000000000000000000000000000000000000001", + tokenRegistry: "0x0000000000000000000000000000000000000001", network: "sepolia", dryRun: false, }; @@ -28,7 +28,7 @@ describe("title-escrow", () => { const mockedOwnerOf = jest.fn(); const mockTransferHolder = jest.fn(); const mockCallStaticTransferHolder = jest.fn().mockResolvedValue(undefined); - const mockedTitleEscrowAddress = "0x2133"; + const mockedTitleEscrowAddress = "0x0000000000000000000000000000000000000002"; mockedOwnerOf.mockReturnValue(mockedTitleEscrowAddress); mockTransferHolder.mockReturnValue({ hash: "hash", diff --git a/src/implementations/title-escrow/transferOwners.test.ts b/src/implementations/title-escrow/transferOwners.test.ts index 5c0f9eae..5d3a6c9a 100644 --- a/src/implementations/title-escrow/transferOwners.test.ts +++ b/src/implementations/title-escrow/transferOwners.test.ts @@ -9,8 +9,8 @@ jest.mock("@govtechsg/token-registry/contracts"); const endorseChangeOwnersParams: TitleEscrowEndorseTransferOfOwnersCommand = { newHolder: "0xabcd", newOwner: "0fosui", - tokenId: "0xzyxw", - tokenRegistry: "0x1234", + tokenId: "0x0000000000000000000000000000000000000000000000000000000000000001", + tokenRegistry: "0x0000000000000000000000000000000000000001", network: "sepolia", dryRun: false, }; @@ -29,7 +29,7 @@ describe("title-escrow", () => { const mockedOwnerOf = jest.fn(); const mockTransferOwners = jest.fn(); const mockCallStaticTransferOwners = jest.fn().mockResolvedValue(undefined); - const mockedTitleEscrowAddress = "0x2133"; + const mockedTitleEscrowAddress = "0x0000000000000000000000000000000000000002"; const mockedBeneficiary = "0xdssfs"; const mockedHolder = "0xdsfls"; const mockGetBeneficiary = jest.fn(); diff --git a/src/implementations/token-registry/issue.test.ts b/src/implementations/token-registry/issue.test.ts index a05c44ac..3dad4ded 100644 --- a/src/implementations/token-registry/issue.test.ts +++ b/src/implementations/token-registry/issue.test.ts @@ -10,8 +10,8 @@ jest.mock("@govtechsg/token-registry/contracts"); const deployParams: TokenRegistryIssueCommand = { beneficiary: "0xabcd", holder: "0xabce", - tokenId: "0xzyxw", - address: "0x1234", + tokenId: "0x0000000000000000000000000000000000000000000000000000000000000001", + address: "0x0000000000000000000000000000000000000001", network: "sepolia", dryRun: false, }; diff --git a/src/implementations/token-registry/issue.ts b/src/implementations/token-registry/issue.ts index 1b845468..9c23d75e 100644 --- a/src/implementations/token-registry/issue.ts +++ b/src/implementations/token-registry/issue.ts @@ -5,6 +5,7 @@ import { getWalletOrSigner } from "../utils/wallet"; import { TokenRegistryIssueCommand } from "../../commands/token-registry/token-registry-command.type"; import { dryRunMode } from "../utils/dryRun"; import { TransactionReceipt } from "@ethersproject/providers"; +import { connectToTokenRegistry } from "../utils/connect"; const { trace } = getLogger("token-registry:issue"); @@ -18,7 +19,7 @@ export const issueToTokenRegistry = async ({ ...rest }: TokenRegistryIssueCommand): Promise => { const wallet = await getWalletOrSigner({ network, ...rest }); - const tokenRegistry: TradeTrustToken = await TradeTrustToken__factory.connect(address, wallet); + const tokenRegistry: TradeTrustToken = await connectToTokenRegistry(address, wallet); if (dryRun) { await dryRunMode({ diff --git a/src/implementations/utils/connect.ts b/src/implementations/utils/connect.ts new file mode 100644 index 00000000..1cfe11f1 --- /dev/null +++ b/src/implementations/utils/connect.ts @@ -0,0 +1,57 @@ +import { + TradeTrustToken, + TradeTrustToken__factory, + TitleEscrow, + TitleEscrow__factory, +} from "@govtechsg/token-registry/contracts"; +import { Wallet } from "ethers"; +import { isAddress } from "ethers/lib/utils"; +import { ConnectedSigner } from "../utils/wallet"; + +type UserWallet = Wallet | ConnectedSigner; + +export const connectToTokenRegistry = async ( + address: string, + wallet: UserWallet, +): Promise => { + const validAddress = isAddress(address); + if (!validAddress) throw new Error(`Invalid token registry address: ${address}`); + const tokenRegistryInstance: TradeTrustToken = await TradeTrustToken__factory.connect(address, wallet); + return tokenRegistryInstance; +}; + +const addressCheck = (maybeAddress: string): void => { + if (!isAddress(maybeAddress)) throw new Error(`Invalid contract address: ${maybeAddress}`); +}; + +const tokenIdCheck = (maybeTokenId: string): void => { + const validHex = /^(0x|0X)?[a-fA-F0-9]+$/.test(maybeTokenId) + if (!validHex) throw new Error(`Invalid token id: ${maybeTokenId}`); +}; + +interface ConnectToTitleEscrowArgs { + tokenId: string; + address: string; + wallet: Wallet | ConnectedSigner; +} + +export const connectToTitleEscrow = async ({ + tokenId, + address, + wallet, +}: ConnectToTitleEscrowArgs): Promise => { + const tokenRegistry: TradeTrustToken = await connectToTokenRegistry( address, wallet ); + const titleEscrowAddress = await getTitleEscrowAddress(tokenRegistry, tokenId); + return await connectToTitleEscrowAddress(titleEscrowAddress, wallet); +}; + +export const connectToTitleEscrowAddress = async (address: string, wallet: UserWallet): Promise => { + addressCheck(address); + const titleEscrow = TitleEscrow__factory.connect(address, wallet); + return titleEscrow; +}; + +const getTitleEscrowAddress = async (tokenRegistry: TradeTrustToken, tokenId: string): Promise => { + tokenIdCheck(tokenId); + return await tokenRegistry.ownerOf(tokenId); +} From 9bf5fe98d3f337564b259d6d597e358ac5da4170 Mon Sep 17 00:00:00 2001 From: PH <68947167+osslgtm@users.noreply.github.com> Date: Mon, 31 Jul 2023 09:25:48 +0000 Subject: [PATCH 2/2] fix: lint --- src/implementations/title-escrow/rejectSurrendered.ts | 2 +- src/implementations/token-registry/issue.ts | 2 +- src/implementations/utils/connect.ts | 11 ++++------- 3 files changed, 6 insertions(+), 9 deletions(-) diff --git a/src/implementations/title-escrow/rejectSurrendered.ts b/src/implementations/title-escrow/rejectSurrendered.ts index 34dfd221..aa6fb577 100644 --- a/src/implementations/title-escrow/rejectSurrendered.ts +++ b/src/implementations/title-escrow/rejectSurrendered.ts @@ -1,4 +1,4 @@ -import { TradeTrustToken, TradeTrustToken__factory } from "@govtechsg/token-registry/contracts"; +import { TradeTrustToken } from "@govtechsg/token-registry/contracts"; import signale from "signale"; import { getLogger } from "../../logger"; import { getWalletOrSigner } from "../utils/wallet"; diff --git a/src/implementations/token-registry/issue.ts b/src/implementations/token-registry/issue.ts index 9c23d75e..60c1ab59 100644 --- a/src/implementations/token-registry/issue.ts +++ b/src/implementations/token-registry/issue.ts @@ -1,4 +1,4 @@ -import { TradeTrustToken, TradeTrustToken__factory } from "@govtechsg/token-registry/contracts"; +import { TradeTrustToken } from "@govtechsg/token-registry/contracts"; import signale from "signale"; import { getLogger } from "../../logger"; import { getWalletOrSigner } from "../utils/wallet"; diff --git a/src/implementations/utils/connect.ts b/src/implementations/utils/connect.ts index 1cfe11f1..777d9ddd 100644 --- a/src/implementations/utils/connect.ts +++ b/src/implementations/utils/connect.ts @@ -10,10 +10,7 @@ import { ConnectedSigner } from "../utils/wallet"; type UserWallet = Wallet | ConnectedSigner; -export const connectToTokenRegistry = async ( - address: string, - wallet: UserWallet, -): Promise => { +export const connectToTokenRegistry = async (address: string, wallet: UserWallet): Promise => { const validAddress = isAddress(address); if (!validAddress) throw new Error(`Invalid token registry address: ${address}`); const tokenRegistryInstance: TradeTrustToken = await TradeTrustToken__factory.connect(address, wallet); @@ -25,7 +22,7 @@ const addressCheck = (maybeAddress: string): void => { }; const tokenIdCheck = (maybeTokenId: string): void => { - const validHex = /^(0x|0X)?[a-fA-F0-9]+$/.test(maybeTokenId) + const validHex = /^(0x|0X)?[a-fA-F0-9]+$/.test(maybeTokenId); if (!validHex) throw new Error(`Invalid token id: ${maybeTokenId}`); }; @@ -40,7 +37,7 @@ export const connectToTitleEscrow = async ({ address, wallet, }: ConnectToTitleEscrowArgs): Promise => { - const tokenRegistry: TradeTrustToken = await connectToTokenRegistry( address, wallet ); + const tokenRegistry: TradeTrustToken = await connectToTokenRegistry(address, wallet); const titleEscrowAddress = await getTitleEscrowAddress(tokenRegistry, tokenId); return await connectToTitleEscrowAddress(titleEscrowAddress, wallet); }; @@ -54,4 +51,4 @@ export const connectToTitleEscrowAddress = async (address: string, wallet: UserW const getTitleEscrowAddress = async (tokenRegistry: TradeTrustToken, tokenId: string): Promise => { tokenIdCheck(tokenId); return await tokenRegistry.ownerOf(tokenId); -} +};