-
Notifications
You must be signed in to change notification settings - Fork 29
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
Changes from all commits
45db37f
6fb907d
20cca7e
2e5424c
364dc96
bc3ff1f
56735aa
2518a7d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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)); | ||
} | ||
}; |
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)); | ||
} | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,9 +2,9 @@ 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 { transferDocumentStoreOwnership } from "../../implementations/document-store/transfer-ownership"; | ||
|
||
const { trace } = getLogger("document-store:transfer-ownership"); | ||
|
||
|
@@ -23,7 +23,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, | ||
|
@@ -35,13 +35,27 @@ 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 { address, newOwner } = args; | ||
const { grantTransaction, revokeTransaction } = await transferDocumentStoreOwnership({ | ||
...args, | ||
// add 0x automatically in front of the hash if it's not provided | ||
newOwner: addAddressPrefix(args.newOwner), | ||
newOwner: addAddressPrefix(newOwner), | ||
address: addAddressPrefix(address), | ||
}); | ||
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}`); | ||
|
||
const grantTransactionHash = (await grantTransaction).transactionHash; | ||
const revokeTransactionHash = (await revokeTransaction).transactionHash; | ||
|
||
success(`Ownership of document store ${args.address} has been transferred to wallet ${args.newOwner}`); | ||
|
||
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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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)); | ||
|
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 |
---|---|---|
@@ -0,0 +1,109 @@ | ||
import { grantDocumentStoreRole } from "./grant-role"; | ||
|
||
import { Wallet } from "ethers"; | ||
import { DocumentStoreFactory } from "@govtechsg/document-store"; | ||
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: DocumentStoreRoleCommand = { | ||
account: "0xabcd", | ||
role: "issuer", | ||
address: "0x1234", | ||
network: "sepolia", | ||
key: "0000000000000000000000000000000000000000000000000000000000000001", | ||
dryRun: false, | ||
}; | ||
|
||
// TODO the following test is very fragile and might break on every interface change of DocumentStoreFactory | ||
// ideally must setup ganache, and run the function over it | ||
describe("document-store", () => { | ||
// increase timeout because ethers is throttling | ||
jest.setTimeout(30000); | ||
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 mockedGrantRole = jest.fn(); | ||
const mockedCallStaticGrantRole = jest.fn().mockResolvedValue(undefined); | ||
|
||
beforeEach(() => { | ||
delete process.env.OA_PRIVATE_KEY; | ||
mockedDocumentStoreFactory.mockReset(); | ||
mockedConnect.mockReset(); | ||
mockedCallStaticGrantRole.mockClear(); | ||
mockedConnect.mockReturnValue({ | ||
grantRole: mockedGrantRole, | ||
DEFAULT_ADMIN_ROLE: jest.fn().mockResolvedValue("ADMIN"), | ||
ISSUER_ROLE: jest.fn().mockResolvedValue("ISSUER"), | ||
REVOKER_ROLE: jest.fn().mockResolvedValue("REVOKER"), | ||
callStatic: { | ||
grantRole: mockedCallStaticGrantRole, | ||
}, | ||
}); | ||
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 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(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 account without 0x prefix and return deployed instance", async () => { | ||
const instance = await grantDocumentStoreRole({ | ||
...deployParams, | ||
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(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 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 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]; | ||
expect(passedSigner.privateKey).toBe(`0x0000000000000000000000000000000000000000000000000000000000000003`); | ||
}); | ||
}); | ||
}); |
There was a problem hiding this comment.
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?There was a problem hiding this comment.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, got it.