From 4a89589ddf57d04998985c0846aa4ca7ee9c76d6 Mon Sep 17 00:00:00 2001 From: Rishabh Singh Date: Wed, 25 Feb 2026 13:46:43 +0530 Subject: [PATCH 1/3] fix: update document store functions --- src/document-store/document-store-roles.ts | 16 +++++++--------- src/document-store/index.ts | 1 + src/document-store/transferOwnership.ts | 22 +++++++++------------- src/index.ts | 4 ++++ 4 files changed, 21 insertions(+), 22 deletions(-) diff --git a/src/document-store/document-store-roles.ts b/src/document-store/document-store-roles.ts index 35eca0c..9ccbe4e 100644 --- a/src/document-store/document-store-roles.ts +++ b/src/document-store/document-store-roles.ts @@ -30,16 +30,14 @@ export const getRoleString = async ( // eslint-disable-next-line @typescript-eslint/no-explicit-any provider as any, ); - 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'); + + // Role should be the actual contract method name (e.g., 'DEFAULT_ADMIN_ROLE', 'ISSUER_ROLE', 'REVOKER_ROLE') + if (typeof documentStore[role as keyof typeof documentStore] !== 'function') { + throw new Error(`Invalid role: ${role}`); } + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + return await (documentStore as any)[role](); }; export const rolesList = ['admin', 'issuer', 'revoker']; diff --git a/src/document-store/index.ts b/src/document-store/index.ts index a84743f..7eeacd3 100644 --- a/src/document-store/index.ts +++ b/src/document-store/index.ts @@ -4,6 +4,7 @@ export { documentStoreRevokeRole } from './revoke-role'; export { documentStoreGrantRole } from './grant-role'; export { documentStoreTransferOwnership } from './transferOwnership'; export { deployDocumentStore } from '../deploy/document-store'; +export { getRoleString } from './document-store-roles'; export { supportInterfaceIds } from './supportInterfaceIds'; export { DocumentStore__factory, diff --git a/src/document-store/transferOwnership.ts b/src/document-store/transferOwnership.ts index c06e114..c2fc498 100644 --- a/src/document-store/transferOwnership.ts +++ b/src/document-store/transferOwnership.ts @@ -17,7 +17,7 @@ import { CommandOptions } from './types'; * @param {string} account - The account to transfer ownership to. * @param {SignerV5 | SignerV6} signer - Signer instance (Ethers v5 or v6) that authorizes the transfer ownership transaction. * @param {CommandOptions} options - Optional transaction metadata including gas values and chain ID. - * @returns {Promise<{grantTransaction: Promise; revokeTransaction: Promise}>} A promise resolving to the transaction result from the grant and revoke role calls. + * @returns {Promise<{grantTransaction: ContractTransactionV5 | ContractTransactionV6; revokeTransaction: ContractTransactionV5 | ContractTransactionV6}>} A promise resolving to the transaction result from the grant and revoke role calls. * @throws {Error} If the document store address or signer provider is not provided. * @throws {Error} If the role is invalid. * @throws {Error} If the `callStatic.revokeRole` fails as a pre-check. @@ -28,21 +28,21 @@ export const documentStoreTransferOwnership = async ( signer: SignerV5 | SignerV6, options: CommandOptions = {}, ): Promise<{ - grantTransaction: Promise; - revokeTransaction: Promise; + grantTransaction: ContractTransactionV5 | ContractTransactionV6; + revokeTransaction: ContractTransactionV5 | ContractTransactionV6; }> => { if (!documentStoreAddress) throw new Error('Document store address is required'); if (!signer.provider) throw new Error('Provider is required'); if (!account) throw new Error('Account is required'); const ownerAddress = await signer.getAddress(); - const roleString = await getRoleString(documentStoreAddress, 'admin', { + const roleString = await getRoleString(documentStoreAddress, 'DEFAULT_ADMIN_ROLE', { provider: signer.provider, }); //call the transferOwnership function of the document store contract //call grant and revoke function here - const grantTransaction = documentStoreGrantRole( + const grantTransaction = await documentStoreGrantRole( documentStoreAddress, roleString, account, @@ -50,14 +50,12 @@ export const documentStoreTransferOwnership = async ( options, ); //check if the grant transaction is successful - const grantTransactionResult = await grantTransaction; - if (!grantTransactionResult) { - //add custom error message not proceeding eith the revoke transaction + if (!grantTransaction) { + //add custom error message not proceeding with the revoke transaction throw new Error('Grant transaction failed, not proceeding with revoke transaction'); - //return the grant transaction result } //call revoke function here - const revokeTransaction = documentStoreRevokeRole( + const revokeTransaction = await documentStoreRevokeRole( documentStoreAddress, roleString, ownerAddress, @@ -65,10 +63,8 @@ export const documentStoreTransferOwnership = async ( options, ); //check if the revoke transaction is successful - const revokeTransactionResult = await revokeTransaction; - if (!revokeTransactionResult) { + if (!revokeTransaction) { throw new Error('Revoke transaction failed'); - //return the revoke transaction result } return { grantTransaction, revokeTransaction }; }; diff --git a/src/index.ts b/src/index.ts index 974c908..64da810 100644 --- a/src/index.ts +++ b/src/index.ts @@ -27,6 +27,8 @@ import { documentStoreRevoke, DocumentStore__factory, TransferableDocumentStore__factory, + getRoleString, + documentStoreTransferOwnership, } from './document-store'; export type { TypedContractMethod } from './token-registry-v5/typedContractMethod'; export * from './token-registry-functions'; @@ -61,6 +63,8 @@ export { documentStoreRevokeRole, documentStoreIssue, documentStoreRevoke, + getRoleString, + documentStoreTransferOwnership, DocumentStore__factory, TransferableDocumentStore__factory, }; From 54584a72cb4b635e63998de4e85c37969dbfae8b Mon Sep 17 00:00:00 2001 From: Rishabh Singh Date: Wed, 25 Feb 2026 14:35:27 +0530 Subject: [PATCH 2/3] fix: code rabbit comments --- src/document-store/document-store-roles.ts | 2 +- src/document-store/transferOwnership.ts | 111 +++++++++++++++++++-- 2 files changed, 102 insertions(+), 11 deletions(-) diff --git a/src/document-store/document-store-roles.ts b/src/document-store/document-store-roles.ts index 9ccbe4e..6053e7b 100644 --- a/src/document-store/document-store-roles.ts +++ b/src/document-store/document-store-roles.ts @@ -40,4 +40,4 @@ export const getRoleString = async ( return await (documentStore as any)[role](); }; -export const rolesList = ['admin', 'issuer', 'revoker']; +export const rolesList = ['DEFAULT_ADMIN_ROLE', 'ISSUER_ROLE', 'REVOKER_ROLE']; diff --git a/src/document-store/transferOwnership.ts b/src/document-store/transferOwnership.ts index c2fc498..d5fca4e 100644 --- a/src/document-store/transferOwnership.ts +++ b/src/document-store/transferOwnership.ts @@ -1,10 +1,25 @@ -import { Signer as SignerV6, ContractTransactionResponse as ContractTransactionV6 } from 'ethersV6'; -import { ContractTransaction as ContractTransactionV5, Signer as SignerV5 } from 'ethers'; +import { + Signer as SignerV6, + ContractTransactionResponse as ContractTransactionV6, + Contract as ContractV6, +} from 'ethersV6'; +import { + ContractTransaction as ContractTransactionV5, + Signer as SignerV5, + Contract as ContractV5, +} from 'ethers'; import { documentStoreRevokeRole } from './revoke-role'; - import { documentStoreGrantRole } from './grant-role'; import { getRoleString } from './document-store-roles'; import { CommandOptions } from './types'; +import { checkSupportsInterface } from '../core'; +import { supportInterfaceIds } from './supportInterfaceIds'; +import { TT_DOCUMENT_STORE_ABI } from './tt-document-store-abi'; +import { getEthersContractFromProvider, isV6EthersProvider } from '../utils/ethers'; +import { + DocumentStore__factory, + TransferableDocumentStore__factory, +} from '@trustvc/document-store'; /** * Transfers ownership of a DocumentStore contract to a new owner. @@ -20,7 +35,7 @@ import { CommandOptions } from './types'; * @returns {Promise<{grantTransaction: ContractTransactionV5 | ContractTransactionV6; revokeTransaction: ContractTransactionV5 | ContractTransactionV6}>} A promise resolving to the transaction result from the grant and revoke role calls. * @throws {Error} If the document store address or signer provider is not provided. * @throws {Error} If the role is invalid. - * @throws {Error} If the `callStatic.revokeRole` fails as a pre-check. + * @throws {Error} If either the `callStatic.grantRole` or `callStatic.revokeRole` pre-check fails. */ export const documentStoreTransferOwnership = async ( documentStoreAddress: string, @@ -39,9 +54,85 @@ export const documentStoreTransferOwnership = async ( const roleString = await getRoleString(documentStoreAddress, 'DEFAULT_ADMIN_ROLE', { provider: signer.provider, }); - //call the transferOwnership function of the document store contract - //call grant and revoke function here + // Get the contract instance for pre-checks + const Contract = getEthersContractFromProvider(signer.provider); + const isDocumentStore = await checkSupportsInterface( + documentStoreAddress, + supportInterfaceIds.IDocumentStore, + signer.provider, + ); + const isTransferableDocumentStore = await checkSupportsInterface( + documentStoreAddress, + supportInterfaceIds.ITransferableDocumentStore, + signer.provider, + ); + + let documentStoreAbi; + if (isDocumentStore || isTransferableDocumentStore) { + const DocumentStoreFactory = isTransferableDocumentStore + ? TransferableDocumentStore__factory + : DocumentStore__factory; + documentStoreAbi = DocumentStoreFactory.abi; + } else { + documentStoreAbi = TT_DOCUMENT_STORE_ABI; + } + + const documentStoreContract: ContractV5 | ContractV6 = new Contract( + documentStoreAddress, + documentStoreAbi, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + signer as any, + ); + + // CRITICAL: Perform BOTH static call pre-checks BEFORE executing any real transactions + // This prevents partial ownership transfer if revoke would fail after grant succeeds + const isV6 = isV6EthersProvider(signer.provider); + + try { + // Pre-check 1: Verify grant will succeed + if (isV6) { + await (documentStoreContract as ContractV6).grantRole.staticCall(roleString, account); + } else { + await (documentStoreContract as ContractV5).callStatic.grantRole(roleString, account); + } + } catch (e) { + const errorMessage = e instanceof Error ? e.message : String(e); + const reason = errorMessage.includes('AccessControl') + ? 'Caller does not have permission to grant this role' + : errorMessage.includes('already has role') + ? 'Account already has this role' + : 'Transaction simulation failed'; + + throw new Error( + `Pre-check (callStatic) for grant-role failed: ${reason}. Original error: ${errorMessage}`, + ); + } + + try { + // Pre-check 2: Verify revoke will succeed + if (isV6) { + await (documentStoreContract as ContractV6).revokeRole.staticCall(roleString, ownerAddress); + } else { + await (documentStoreContract as ContractV5).callStatic.revokeRole(roleString, ownerAddress); + } + } catch (e) { + const errorMessage = e instanceof Error ? e.message : String(e); + const reason = errorMessage.includes('AccessControl') + ? 'Caller does not have permission to revoke this role' + : errorMessage.includes('does not have role') + ? 'Account does not have this role' + : 'Transaction simulation failed'; + + throw new Error( + `Pre-check (callStatic) for revoke-role failed: ${reason}. Original error: ${errorMessage}`, + ); + } + + // Both pre-checks passed - now execute the real transactions + // Note: We skip the individual function's pre-checks since we already did them above + // by passing the contract instance directly or using a flag (implementation depends on refactoring grant/revoke functions) + const grantTransaction = await documentStoreGrantRole( documentStoreAddress, roleString, @@ -49,12 +140,11 @@ export const documentStoreTransferOwnership = async ( signer, options, ); - //check if the grant transaction is successful + if (!grantTransaction) { - //add custom error message not proceeding with the revoke transaction throw new Error('Grant transaction failed, not proceeding with revoke transaction'); } - //call revoke function here + const revokeTransaction = await documentStoreRevokeRole( documentStoreAddress, roleString, @@ -62,9 +152,10 @@ export const documentStoreTransferOwnership = async ( signer, options, ); - //check if the revoke transaction is successful + if (!revokeTransaction) { throw new Error('Revoke transaction failed'); } + return { grantTransaction, revokeTransaction }; }; From 9919cf7692b88a766aaaaec91c5fd83ed77e06d5 Mon Sep 17 00:00:00 2001 From: Rishabh Singh Date: Wed, 25 Feb 2026 14:45:00 +0530 Subject: [PATCH 3/3] fix: code rabbit comments --- src/document-store/transferOwnership.ts | 27 ++++--------------------- 1 file changed, 4 insertions(+), 23 deletions(-) diff --git a/src/document-store/transferOwnership.ts b/src/document-store/transferOwnership.ts index d5fca4e..2e2d796 100644 --- a/src/document-store/transferOwnership.ts +++ b/src/document-store/transferOwnership.ts @@ -97,16 +97,8 @@ export const documentStoreTransferOwnership = async ( await (documentStoreContract as ContractV5).callStatic.grantRole(roleString, account); } } catch (e) { - const errorMessage = e instanceof Error ? e.message : String(e); - const reason = errorMessage.includes('AccessControl') - ? 'Caller does not have permission to grant this role' - : errorMessage.includes('already has role') - ? 'Account already has this role' - : 'Transaction simulation failed'; - - throw new Error( - `Pre-check (callStatic) for grant-role failed: ${reason}. Original error: ${errorMessage}`, - ); + console.error('callStatic failed:', e); + throw new Error('Pre-check (callStatic) for grant-role failed'); } try { @@ -117,22 +109,11 @@ export const documentStoreTransferOwnership = async ( await (documentStoreContract as ContractV5).callStatic.revokeRole(roleString, ownerAddress); } } catch (e) { - const errorMessage = e instanceof Error ? e.message : String(e); - const reason = errorMessage.includes('AccessControl') - ? 'Caller does not have permission to revoke this role' - : errorMessage.includes('does not have role') - ? 'Account does not have this role' - : 'Transaction simulation failed'; - - throw new Error( - `Pre-check (callStatic) for revoke-role failed: ${reason}. Original error: ${errorMessage}`, - ); + console.error('callStatic failed:', e); + throw new Error('Pre-check (callStatic) for revoke-role failed'); } // Both pre-checks passed - now execute the real transactions - // Note: We skip the individual function's pre-checks since we already did them above - // by passing the contract instance directly or using a flag (implementation depends on refactoring grant/revoke functions) - const grantTransaction = await documentStoreGrantRole( documentStoreAddress, roleString,