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: upgrade doc store #282

Merged
merged 8 commits into from
Sep 13, 2023
Merged
Show file tree
Hide file tree
Changes from 6 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
6 changes: 3 additions & 3 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@
},
"dependencies": {
"@govtechsg/dnsprove": "^2.3.0",
"@govtechsg/document-store": "^2.4.0",
"@govtechsg/document-store": "^2.6.1",
"@govtechsg/oa-encryption": "^1.3.3",
"@govtechsg/oa-verify": "^7.11.0",
"@govtechsg/open-attestation": "^6.4.1",
Expand Down
7 changes: 7 additions & 0 deletions src/commands/document-store/document-store-command.type.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,10 @@ export type DocumentStoreTransferOwnershipCommand = NetworkAndWalletSignerOption
address: string;
newOwner: string;
};

export type DocumentStoreRoleCommand = NetworkAndWalletSignerOption &
GasOption & {
address: string;
account: string;
role: string;
};
57 changes: 57 additions & 0 deletions src/commands/document-store/grant-role.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
import { Argv } from "yargs";
import { error, info, success } from "signale";
import { getLogger } from "../../logger";
import { DocumentStoreRoleCommand } from "./document-store-command.type";
import { grantDocumentStoreRole } from "../../implementations/document-store/grant-role";
import { withGasPriceOption, withNetworkAndWalletSignerOption } from "../shared";
import { getErrorMessage, getEtherscanAddress, addAddressPrefix } from "../../utils";
import { rolesList } from "../../implementations/document-store/document-store-roles";

const { trace } = getLogger("document-store:grant-role");

export const command = "grant-role [options]";

export const describe = "grant role of the document store to a wallet";

export const builder = (yargs: Argv): Argv =>
withGasPriceOption(
withNetworkAndWalletSignerOption(
yargs
.option("address", {
alias: "a",
description: "Address of document store to be granted role",
type: "string",
demandOption: true,
})
.option("account", {
alias: ["h", "newOwner"],
description: "Address of wallet to transfer role to",
type: "string",
demandOption: true,
})
.option("role", {
alias: "r",
description: "Role to be transferred",
type: "string",
options: rolesList,
demandOption: true,
})
)
);

export const handler = async (args: DocumentStoreRoleCommand): Promise<string | undefined> => {
trace(`Args: ${JSON.stringify(args, null, 2)}`);
try {
info(`Granting role to wallet ${args.account}`);
const { transactionHash } = await grantDocumentStoreRole({
...args,
// add 0x automatically in front of the hash if it's not provided
account: addAddressPrefix(args.account),
});
success(`Document store ${args.address}'s role of: ${args.role} has been granted to wallet ${args.account}`);
info(`Find more details at ${getEtherscanAddress({ network: args.network })}/tx/${transactionHash}`);
return args.address;
} catch (e) {
error(getErrorMessage(e));
}
};
57 changes: 57 additions & 0 deletions src/commands/document-store/revoke-role.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
import { Argv } from "yargs";
import { error, info, success } from "signale";
import { getLogger } from "../../logger";
import { DocumentStoreRoleCommand } from "./document-store-command.type";
import { revokeDocumentStoreRole } from "../../implementations/document-store/revoke-role";
import { withGasPriceOption, withNetworkAndWalletSignerOption } from "../shared";
import { getErrorMessage, getEtherscanAddress, addAddressPrefix } from "../../utils";
import { rolesList } from "../../implementations/document-store/document-store-roles";

const { trace } = getLogger("document-store:revoke-role");

export const command = "revoke-role [options]";

export const describe = "revoke role of the document store to a wallet";

export const builder = (yargs: Argv): Argv =>
withGasPriceOption(
withNetworkAndWalletSignerOption(
yargs
.option("address", {
alias: "a",
description: "Address of document store to be revoked role",
type: "string",
demandOption: true,
})
.option("account", {
alias: ["h", "newOwner"],
description: "Address of wallet to revoke role from",
type: "string",
demandOption: true,
})
.option("role", {
alias: "r",
description: "Role to be revoked",
type: "string",
options: rolesList,
demandOption: true,
})
)
);

export const handler = async (args: DocumentStoreRoleCommand): Promise<string | undefined> => {
trace(`Args: ${JSON.stringify(args, null, 2)}`);
try {
info(`Revoking role from wallet ${args.account}`);
const { transactionHash } = await revokeDocumentStoreRole({
...args,
// add 0x automatically in front of the hash if it's not provided
account: addAddressPrefix(args.account),
});
success(`Document store ${args.address}'s role of: ${args.role} has been revoked from wallet ${args.account}`);
info(`Find more details at ${getEtherscanAddress({ network: args.network })}/tx/${transactionHash}`);
return args.address;
} catch (e) {
error(getErrorMessage(e));
}
};
32 changes: 27 additions & 5 deletions src/commands/document-store/transfer-ownership.ts
Copy link
Contributor

Choose a reason for hiding this comment

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

Should the transfer-ownership command have its own tests too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i've created transfer-ownership as a new implementation as opposed to using grant and revoke seperately. tests written

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, got it.

Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@ import { Argv } from "yargs";
import { error, info, success } from "signale";
import { getLogger } from "../../logger";
import { DocumentStoreTransferOwnershipCommand } from "./document-store-command.type";
import { transferDocumentStoreOwnershipToWallet } from "../../implementations/document-store/transfer-ownership";
import { withGasPriceOption, withNetworkAndWalletSignerOption } from "../shared";
import { getErrorMessage, getEtherscanAddress, addAddressPrefix } from "../../utils";
import { grantDocumentStoreRole } from "../../implementations/document-store/grant-role";
import { revokeDocumentStoreRole } from "../../implementations/document-store/revoke-role";
import { getWalletOrSigner } from "../../implementations/utils/wallet";

const { trace } = getLogger("document-store:transfer-ownership");

Expand All @@ -23,7 +25,7 @@ export const builder = (yargs: Argv): Argv =>
demandOption: true,
})
.option("newOwner", {
alias: "h",
alias: ["h", "account"],
description: "Address of new wallet to transfer ownership to",
type: "string",
demandOption: true,
Expand All @@ -35,13 +37,33 @@ export const handler = async (args: DocumentStoreTransferOwnershipCommand): Prom
trace(`Args: ${JSON.stringify(args, null, 2)}`);
try {
info(`Transferring ownership to wallet ${args.newOwner}`);
const { transactionHash } = await transferDocumentStoreOwnershipToWallet({
const account = addAddressPrefix(args.newOwner);
const role = "admin";
const { transactionHash: grantTransactionHash } = await grantDocumentStoreRole({
...args,
// add 0x automatically in front of the hash if it's not provided
newOwner: addAddressPrefix(args.newOwner),
account,
role,
});

const wallet = await getWalletOrSigner({ ...args });
const walletAddress = await wallet.getAddress();

const { transactionHash: revokeTransactionHash } = await revokeDocumentStoreRole({
...args,
account: addAddressPrefix(walletAddress),
role,
});

success(`Ownership of document store ${args.address} has been transferred to new wallet ${args.newOwner}`);
info(`Find more details at ${getEtherscanAddress({ network: args.network })}/tx/${transactionHash}`);
info(
`Find more details at ${getEtherscanAddress({
network: args.network,
})}/tx/${grantTransactionHash} (grant) and ${getEtherscanAddress({
network: args.network,
})}/tx/${revokeTransactionHash} (revoke)`
);
Comment on lines +51 to +57
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we send the output as soon as the grant transaction is completed instead of waiting for both to finish then show them together? I'm thinking what if grant was successful but revoke failed? Then the display for the first successful transaction won't be known?


return args.address;
} catch (e) {
error(getErrorMessage(e));
Expand Down
16 changes: 16 additions & 0 deletions src/implementations/document-store/document-store-roles.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import { DocumentStore } from "@govtechsg/document-store";

export const getRoleString = async (documentStore: DocumentStore, role: string): Promise<string> => {
switch (role) {
case "admin":
return await documentStore.DEFAULT_ADMIN_ROLE();
case "issuer":
return await documentStore.ISSUER_ROLE();
case "revoker":
return await documentStore.REVOKER_ROLE();
default:
throw new Error("Invalid role");
}
};

export const rolesList = ["admin", "issuer", "revoker"];
Original file line number Diff line number Diff line change
@@ -1,15 +1,16 @@
import { transferDocumentStoreOwnershipToWallet } from "./transfer-ownership";
import { grantDocumentStoreRole } from "./grant-role";

import { Wallet } from "ethers";
import { DocumentStoreFactory } from "@govtechsg/document-store";
import { DocumentStoreTransferOwnershipCommand } from "../../commands/document-store/document-store-command.type";
import { DocumentStoreRoleCommand } from "../../commands/document-store/document-store-command.type";
import { addAddressPrefix } from "../../utils";
import { join } from "path";

jest.mock("@govtechsg/document-store");

const deployParams: DocumentStoreTransferOwnershipCommand = {
newOwner: "0xabcd",
const deployParams: DocumentStoreRoleCommand = {
account: "0xabcd",
role: "issuer",
address: "0x1234",
network: "sepolia",
key: "0000000000000000000000000000000000000000000000000000000000000001",
Expand All @@ -21,76 +22,84 @@ const deployParams: DocumentStoreTransferOwnershipCommand = {
describe("document-store", () => {
// increase timeout because ethers is throttling
jest.setTimeout(30000);
describe("transferDocumentStoreOwnershipToWallet", () => {
describe("grant document store issuer role to wallet", () => {
const mockedDocumentStoreFactory: jest.Mock<DocumentStoreFactory> = DocumentStoreFactory as any;
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore mock static method
const mockedConnect: jest.Mock = mockedDocumentStoreFactory.connect;
const mockedTransfer = jest.fn();
const mockCallStaticTransferOwnership = jest.fn().mockResolvedValue(undefined);
const mockedGrantRole = jest.fn();
const mockedCallStaticGrantRole = jest.fn().mockResolvedValue(undefined);

beforeEach(() => {
delete process.env.OA_PRIVATE_KEY;
mockedDocumentStoreFactory.mockReset();
mockedConnect.mockReset();
mockCallStaticTransferOwnership.mockClear();
mockedCallStaticGrantRole.mockClear();
mockedConnect.mockReturnValue({
transferOwnership: mockedTransfer,
grantRole: mockedGrantRole,
DEFAULT_ADMIN_ROLE: jest.fn().mockResolvedValue("ADMIN"),
ISSUER_ROLE: jest.fn().mockResolvedValue("ISSUER"),
REVOKER_ROLE: jest.fn().mockResolvedValue("REVOKER"),
callStatic: {
transferOwnership: mockCallStaticTransferOwnership,
grantRole: mockedCallStaticGrantRole,
},
});
mockedTransfer.mockReturnValue({
mockedGrantRole.mockReturnValue({
hash: "hash",
wait: () => Promise.resolve({ transactionHash: "transactionHash" }),
});
});
it("should pass in the correct params and return the deployed instance", async () => {
const instance = await transferDocumentStoreOwnershipToWallet(deployParams);
const instance = await grantDocumentStoreRole(deployParams);

const passedSigner: Wallet = mockedConnect.mock.calls[0][1];

expect(passedSigner.privateKey).toBe(`0x${deployParams.key}`);
expect(mockedConnect.mock.calls[0][0]).toEqual(deployParams.address);
expect(mockCallStaticTransferOwnership).toHaveBeenCalledTimes(1);
expect(mockedTransfer.mock.calls[0][0]).toEqual(deployParams.newOwner);
expect(mockedCallStaticGrantRole).toHaveBeenCalledTimes(1);
expect(mockedGrantRole.mock.calls[0][0]).toEqual("ISSUER");
expect(mockedGrantRole.mock.calls[0][1]).toEqual(deployParams.account);
expect(instance).toStrictEqual({ transactionHash: "transactionHash" });
});

it("should accept newOwner without 0x prefix and return deployed instance", async () => {
const instance = await transferDocumentStoreOwnershipToWallet({
it("should accept account without 0x prefix and return deployed instance", async () => {
const instance = await grantDocumentStoreRole({
...deployParams,
newOwner: addAddressPrefix("abcd"),
account: addAddressPrefix("abcd"),
});

const passedSigner: Wallet = mockedConnect.mock.calls[0][1];

expect(passedSigner.privateKey).toBe(`0x${deployParams.key}`);
expect(mockedConnect.mock.calls[0][0]).toEqual(deployParams.address);
expect(mockCallStaticTransferOwnership).toHaveBeenCalledTimes(1);
expect(mockedTransfer.mock.calls[0][0]).toEqual(deployParams.newOwner);
expect(mockedCallStaticGrantRole).toHaveBeenCalledTimes(1);
expect(mockedGrantRole.mock.calls[0][0]).toEqual("ISSUER");
expect(mockedGrantRole.mock.calls[0][1]).toEqual(deployParams.account);

expect(instance).toStrictEqual({ transactionHash: "transactionHash" });
});

it("should take in the key from environment variable", async () => {
process.env.OA_PRIVATE_KEY = "0000000000000000000000000000000000000000000000000000000000000002";
await transferDocumentStoreOwnershipToWallet({
newOwner: "0xabcd",
await grantDocumentStoreRole({
account: "0xabcd",
address: "0x1234",
network: "sepolia",
dryRun: false,
role: "admin",
});

const passedSigner: Wallet = mockedConnect.mock.calls[0][1];
expect(passedSigner.privateKey).toBe(`0x${process.env.OA_PRIVATE_KEY}`);
});
it("should take in the key from key file", async () => {
await transferDocumentStoreOwnershipToWallet({
newOwner: "0xabcd",
await grantDocumentStoreRole({
account: "0xabcd",
address: "0x1234",
network: "sepolia",
keyFile: join(__dirname, "..", "..", "..", "examples", "sample-key"),
dryRun: false,
role: "admin",
});

const passedSigner: Wallet = mockedConnect.mock.calls[0][1];
Expand Down
Original file line number Diff line number Diff line change
@@ -1,34 +1,35 @@
import { DocumentStoreFactory } from "@govtechsg/document-store";
import signale from "signale";
import { getLogger } from "../../logger";
import { DocumentStoreTransferOwnershipCommand } from "../../commands/document-store/document-store-command.type";
import { DocumentStoreRoleCommand } from "../../commands/document-store/document-store-command.type";
import { getWalletOrSigner } from "../utils/wallet";
import { dryRunMode } from "../utils/dryRun";
import { TransactionReceipt } from "@ethersproject/providers";
import { getRoleString } from "./document-store-roles";

const { trace } = getLogger("document-store:transfer-ownership");

export const transferDocumentStoreOwnershipToWallet = async ({
export const grantDocumentStoreRole = async ({
address,
newOwner,
account,
network,
dryRun,
role,
...rest
}: DocumentStoreTransferOwnershipCommand): Promise<TransactionReceipt> => {
}: DocumentStoreRoleCommand): Promise<TransactionReceipt> => {
const wallet = await getWalletOrSigner({ network, ...rest });
const documentStore = await DocumentStoreFactory.connect(address, wallet);
const roleString = await getRoleString(documentStore, role);
if (dryRun) {
const documentStore = await DocumentStoreFactory.connect(address, wallet);
await dryRunMode({
estimatedGas: await documentStore.estimateGas.transferOwnership(newOwner),
estimatedGas: await documentStore.estimateGas.grantRole(roleString, account),
network,
});
process.exit(0);
}

signale.await(`Sending transaction to pool`);
const documentStore = await DocumentStoreFactory.connect(address, wallet);
await documentStore.callStatic.transferOwnership(newOwner);
const transaction = await documentStore.transferOwnership(newOwner);
await documentStore.callStatic.grantRole(roleString, account);
const transaction = await documentStore.grantRole(roleString, account);
trace(`Tx hash: ${transaction.hash}`);
trace(`Block Number: ${transaction.blockNumber}`);
signale.await(`Waiting for transaction ${transaction.hash} to be mined`);
Expand Down
Loading
Loading