Skip to content

V5 merge to master#1099

Merged
rongquan1 merged 9 commits intomasterfrom
v5-merge-to-master
Sep 11, 2025
Merged

V5 merge to master#1099
rongquan1 merged 9 commits intomasterfrom
v5-merge-to-master

Conversation

@RishabhS7
Copy link
Contributor

@RishabhS7 RishabhS7 commented Sep 11, 2025

Summary

Merge V5 to master
ECDSA
Data model V2.0
Updated test cases and trustvc packages

Summary by CodeRabbit

  • New Features
    • W3C VC V2.0 support across app: schema detection, tags (“W3C VC V2.0”), and obfuscation messaging.
    • Documents signed using EcdsaSd2023 with required fields.
  • Bug Fixes
    • Robust handling of invalid base64 in attachments and file info.
    • Stronger disabled button styles; UI disabled during initialization.
    • Treat INITIALIZED as pending; centralized success overlays.
    • Ignore “user rejected transaction” without error state.
  • Tests
    • Extensive unit/e2e coverage for W3C VC V2.0, attachments, QR code, tags, and status.
  • Chores
    • Dependency updates; test environment and config refinements.

RishabhS7 and others added 9 commits August 26, 2025 11:48
* chore: add w3cvc data model 2.0 support

* fix: esm jest issue

* fix: synpress error

* fix: testcafe error

* fix: refactor

* fix: refactor

* fix: refactor

* chore: update package versions

* fix: update package.json

---------

Co-authored-by: rongquan1 <rongquan.low@gmail.com>
* chore: add w3cvc data model 2.0 support

* fix: esm jest issue

* fix: synpress error

* fix: testcafe error

* fix: refactor

* fix: refactor

* fix: refactor

* chore: update package versions

* feat: key pair generation update

* feat: package update

* feat: update document builder sign

* feat: package update

* feat: package update

* feat: test

---------

Co-authored-by: Rishabh Singh <rishabh7singh5@gmail.com>
Co-authored-by: rongquan1 <rongquan.low@gmail.com>
Co-authored-by: moiz-sgtradex <moiz.shaikh@sgtradextech.com>
* chore: ECDSA V2.0 tests

* fix: test cases

* fix: imports

* fix: update document attachment type

* fix: date key changes

* fix: update token Id for deployment
* fix: update TrustVC package

* fix: update package-lock

---------

Co-authored-by: isaackps <isaac.kps@gmail.com>
Co-authored-by: moiz-sgtradex <moiz.shaikh@sgtradextech.com>
* fix: decode attachment error (#1088)

* chore: update test cases for dm v2.0

* fix: resolve conflicts

* fix: update obfuscation tests

* fix: package json

* fix: prefix revert

* fix: obfuscation text update

---------

Co-authored-by: isaackps <isaac.kps@gmail.com>
Co-authored-by: moiz-sgtradex <moiz.shaikh@sgtradextech.com>
* chore: unit tests dm v2.0

* fix: lint fix

* fix: document type update
@netlify
Copy link

netlify bot commented Sep 11, 2025

Deploy Preview for reference-implementation ready!

Name Link
🔨 Latest commit fde3dda
🔍 Latest deploy log https://app.netlify.com/projects/reference-implementation/deploys/68c2540da8ffd30008bdc776
😎 Deploy Preview https://deploy-preview-1099--reference-implementation.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@netlify
Copy link

netlify bot commented Sep 11, 2025

Deploy Preview for tradetrust-dev ready!

Name Link
🔨 Latest commit fde3dda
🔍 Latest deploy log https://app.netlify.com/projects/tradetrust-dev/deploys/68c2540d97be2100082f2855
😎 Deploy Preview https://deploy-preview-1099--tradetrust-dev.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@coderabbitai
Copy link

coderabbitai bot commented Sep 11, 2025

Walkthrough

Adds W3C VC 2.0 awareness across reducer, saga, UI tags, and tests; introduces ECDSA-SD 2023 signing and DID-web crypto suite usage; improves error handling (user-rejected tx, base64 decoding); adjusts disabled UI states; enhances Jest config/setup; updates dependencies; revises numerous unit/E2E tests and fixtures to W3C VC 2.0.

Changes

Cohort / File(s) Summary
Jest config and setup
jest.config.js, jest.setup.ts
Adds moduleNameMapper aliases for @digitalbazaar and base(58
Dependencies
package.json
Bumps @tradetrust-tt/decentralized-renderer-react-components and @trustvc/trustvc versions.
Hooks: DID, queue, contract
src/common/hooks/useDidWeb.tsx, src/common/hooks/useQueue.tsx, src/common/hooks/useContractFunctionHook.tsx
DID: include CryptoSuite/VerificationType; POST with JSON body selecting EcdsaSd2023; loadDid validates Multikey, resets invalid cache. Queue: sign with EcdsaSd2023 and mandatoryPointers for /renderMethod. Contract: treat “user rejected transaction” by resetting to UNINITIALIZED.
Asset management UI states
src/components/AssetManagementPanel/.../ActionManagementSkeleton.tsx, .../AssetManagementForm.tsx, .../FormVariants/ActionForm/ActionForm.tsx
Disable inputs/buttons when INITIALIZED; overlay effect dependency broadened; unify CONFIRMED side-effects; treat INITIALIZED as pending across actions; add debug logs; minor class adjustments.
Document tags and tests
src/components/AssetManagementPanel/AssetManagementTags/index.tsx, .../AssetManagementTags.test.tsx
Add “W3C VC V2.0” tag; update “W3C VC V1.1” label; comprehensive tests for transferable/schema/TR tags and classes.
Button disabled styling
src/components/Button/Button.tsx
Apply Tailwind important modifier to disabled styles for higher specificity.
Base64 defensive handling
src/components/DynamicFormContainer/.../AttachmentDropzone.tsx, .../FilesInfo.tsx, src/components/UI/AttachmentLink/AttachmentLink.tsx
Wrap atob operations in try/catch; log warnings; fallback to “Unknown size”; prevent Open button on decode failures.
Obfuscation message: component and tests
src/components/ObfuscatedMessage/ObfuscatedMessage.tsx, .../ObfuscatedMessage.test.tsx
Component now accepts OA or W3C VC; async isObfuscated check with state; schema-aware messaging (softer note for W3C VC 2.0); tests expanded for OA, W3C v2 signed/derived; adds store scaffolding.
Reducer and saga for schema
src/reducers/certificate.ts, src/sagas/certificate.ts
Add DOCUMENT_SCHEMA.W3C_VC_2_0, update W3C_VC_1_1 label; saga selects 1.1 vs 2.0 using vc.isSignedDocumentV2_0.
Document status tests
src/components/DocumentStatus/DocumentStatus.test.tsx
Generalize to OA and W3C scenarios; data-driven tests; verify issuer/status UI; handle magic demo; ensure proper non-render cases.
W3C v2 utilities test
src/utils/shared.test.ts
Add getChainId tests for W3C v2 credentials; validate chainId parsing and missing fields.
TestCafe/E2E fixtures (W3C v2)
src/test/fixture/local/w3c/v2_*...json, tests/e2e/fixtures/w3c-2.0-*.json
Add multiple W3C v2 credential fixtures (invoice/qrcode, transferable records: signed/derived, BL carrier flows).
E2E setup changes
src/test/setup-contracts-testcafe.js, tests/e2e/setup-contracts.js
Switch to CJS path-based import for v5Contracts; define local CHAIN_ID; adjust minting data (tokenId) and add tokens; update CLI args from merkleRoot to tokenId.
E2E specs updated to W3C v2
tests/e2e/specs/*
Swap OA ebl fixtures to W3C 2.0 equivalents across transfer, surrender, endorse/nominate, reject flows; creator spec now validates new tag texts.
Misc test tweak
src/test/creator/happy-flow-magiclink.spec.ts
Adds a comment/spacing; no functional change.
DNS DID verified and attachments
src/test/dns-did-verified.spec.ts, src/test/attachments.spec.ts, src/test/w3c-document.spec.ts, src/test/qrcode.spec.ts, src/test/obfuscated-document.spec.ts
Add/expand tests for W3C v2 rendering, QR with logo, attachments rendering/obfuscation notes, and multi-scenario W3C doc validations.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User
  participant UI as UI (Upload/Load)
  participant Saga as certificate saga
  participant VC as vc utils
  participant Store as Redux Store

  User->>UI: Load document
  UI->>Saga: UPDATE_DOCUMENT_SCHEMA(certificate)
  Saga->>VC: isW3CVC(certificate)?
  alt W3C VC
    Saga->>VC: isSignedDocumentV2_0(certificate)
    alt v2.0
      Saga->>Store: set schema = W3C_VC_2_0
    else v1.1
      Saga->>Store: set schema = W3C_VC_1_1
    end
  else OA
    Saga->>Store: set schema = OA_V2/OA_V3
  end
Loading
sequenceDiagram
  autonumber
  actor User
  participant Comp as ObfuscatedMessage
  participant Helper as isObfuscated()
  participant Store as Redux (schema)

  User->>Comp: Render with document
  Comp->>Helper: isObfuscated(document) (async)
  Helper-->>Comp: boolean or throws
  alt obfuscated = true
    Comp->>Store: select documentSchema
    Comp-->>User: Show note (schema-specific wording)
  else false or error
    Comp-->>User: Render nothing
  end
Loading
sequenceDiagram
  autonumber
  participant Hook as useQueue
  participant Builder as builder.sign(...)
  participant Upload as Upload/Mint

  Hook->>Builder: sign(keyPair, EcdsaSd2023, { mandatoryPointers: ["/renderMethod"] })
  Builder-->>Hook: signed document
  Hook->>Upload: proceed with mint/upload flow
  Upload-->>Hook: result/error
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

Possibly related PRs

Suggested reviewers

  • Moiz47
  • rongquan1
  • nghaninn

Pre-merge checks (2 passed, 1 warning)

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The current PR description contains only a very brief summary and does not follow the repository template: it is missing the required "## Changes" section that lists concrete code, dependency, and test changes, and the "## Issues" section linking related issue/story IDs or tracking numbers, and it lacks details on breaking changes, migration steps, or testing impact. Please update the PR description to match the repository template by adding a "## Changes" section that enumerates the key file and behavior changes (dependency upgrades, public API/schema changes, tests added/modified, and any E2E/test infra updates) and an "## Issues" section linking related issue numbers or stories; also include notes about breaking changes, migration steps, and CI/test expectations so reviewers can assess impact quickly.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title "V5 merge to master" is short, directly related to the PR intent (merging the v5 branch into master) and concisely describes the primary action indicated by the pr_objectives and raw_summary. It is clear enough for a reviewer scanning history to understand the main change.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

Poem

A rabbit signs with steadfast paw, ECDSA, no flaw.
Tags gleam “VC V2.0” bright, in twilight’s testing light.
Obfuscated whispers fade, as wiser checks are made.
Queues hop, DID keys align—
Carrots merged, the build is fine. 🥕✨

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.

✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch v5-merge-to-master

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 7

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (11)
src/components/AssetManagementPanel/AssetManagementForm/AssetManagementForm.tsx (1)

190-226: Prevent overlay re-open flicker during PENDING_CONFIRMATION.

Adding reject-state deps causes this effect to run and call showOverlay on state changes, potentially re-opening after closeOverlay. Guard on pending to avoid loops/flicker.

   useEffect(() => {
+    if (isRejectPendingConfirmation) return;
     let additionalComponent;
     switch (formAction) {
       case AssetManagementActions.RejectTransferOwner:
         additionalComponent = (
           <RejectTransferOwnerOverlay
             handleRejectTransferOwner={rejectTransferOwner}
             rejectTransferOwnerState={rejectTransferOwnerState}
           />
         );
         break;
       case AssetManagementActions.RejectTransferOwnerHolder:
         additionalComponent = (
           <RejectTransferOwnerHolderOverlay
             handleRejectTransferOwnerHolder={rejectTransferOwnerHolder}
             rejectTransferOwnerHolderState={rejectTransferOwnerHolderState}
           />
         );
         break;
       case AssetManagementActions.RejectTransferHolder:
         additionalComponent = (
           <RejectTransferHolderOverlay
             handleRejectTransferHolder={rejectTransferHolder}
             rejectTransferHolderState={rejectTransferHolderState}
           />
         );
         break;
       default:
         additionalComponent = null;
     }
     if (additionalComponent) {
       showOverlay(additionalComponent);
     }
     // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, [formAction, rejectTransferHolderState, rejectTransferOwnerState, rejectTransferOwnerHolderState]);
+  }, [formAction, rejectTransferHolderState, rejectTransferOwnerState, rejectTransferOwnerHolderState, isRejectPendingConfirmation]);
src/components/AssetManagementPanel/AssetManagementForm/FormVariants/ActionForm/ActionForm.tsx (2)

141-144: Bug: newOwner defaults to holder.

newOwner should initialize from beneficiary (current owner), not holder. This causes validation confusion and incorrect prefill.

Apply:

- const [newOwner, setNewOwner] = useState(holder || "");
+ const [newOwner, setNewOwner] = useState(beneficiary || "");

165-183: Remove props from the useEffect dependency array — depend on explicit form-state props.

Including the whole props object makes the effect run on every render and can produce duplicate overlays. Replace props with the specific per-form state fields used inside the effect.

File: src/components/AssetManagementPanel/AssetManagementForm/FormVariants/ActionForm/ActionForm.tsx — useEffect starts at ~line 166; deps array at line 298.

Minimal change (replace the deps array):

- }, [props, showOverlay, setFormActionNone, beneficiary, holder, newBeneficiary, newHolder, newOwner, remark, type]);
+ }, [
+   type,
+   showOverlay,
+   setFormActionNone,
+   beneficiary,
+   holder,
+   newBeneficiary,
+   newHolder,
+   newOwner,
+   remark,
+   (props as SurrenderFormProps).returnToIssuerState,
+   (props as EndorseBeneficiaryProps).beneficiaryEndorseState,
+   (props as AcceptSurrenderedFormProps).destroyTokenState,
+   (props as RejectSurrenderedFormProps).restoreTokenState,
+   (props as TransferOwnerHolderFormProps).transferOwnersState,
+   (props as NominateBeneficiaryFormProps).nominationState,
+   (props as TransferHolderFormProps).holderTransferringState,
+ ]);

Verify overlays appear once per transition to FormState.CONFIRMED.

tests/e2e/specs/3a-endorse-transfer-owner-holder.spec.js (1)

25-27: Fix duplicate assertion: verify holder, not owner (again).

Second assertion repeats owner; it should assert holder to validate both roles.

Apply this diff:

-    cy.get("[data-testid='non-editable-input-owner']").should("have.text", ACCOUNT_3);
+    cy.get("[data-testid='non-editable-input-holder']").should("have.text", ACCOUNT_3);
src/sagas/certificate.ts (1)

100-117: Label v2.0 correctly for raw W3C VC documents (TrustVC has no isRawDocumentV2_0 helper)

TrustVC does not export vc.isRawDocumentV2_0 — detect v2.0 using the existing isSignedDocumentV2_0 (if available) and fall back to an @context-based check for raw v2 documents.

File: src/sagas/certificate.ts (lines 100-117)

-    const isW3CVCVersion2_0 = isW3CVC ? vc.isSignedDocumentV2_0(certificate) : null;
+    const isW3CVCVersion2_0 = isW3CVC
+      ? (
+          (typeof vc.isSignedDocumentV2_0 === "function" && vc.isSignedDocumentV2_0(certificate)) ||
+          (Array.isArray(certificate?.["@context"]) &&
+            certificate["@context"].some((c: unknown) => typeof c === "string" && /\/v2\b/.test(c as string)))
+        )
+      : null;
tests/e2e/setup-contracts.js (2)

51-55: Avoid deploying a second TitleEscrowFactory; reuse the CLI-deployed one.

You deploy TitleEscrowFactory twice (CLI at Line 25 and ethers here). This can cause mismatches when wiring implementations.

-const titleEscrowFactory = new ethers.ContractFactory(
-  TitleEscrowFactory__factory.abi,
-  TitleEscrowFactory__factory.bytecode,
-  signer
-);
-const titleEscrowFactoryContract = await titleEscrowFactory.deploy();
+// Reuse the CLI-deployed TitleEscrowFactory to keep addresses consistent.
+const titleEscrowFactoryAddress = TITLE_ESCROW_FACTORY_ADDRESS;

And later (Line 81) pass titleEscrowFactoryAddress instead of titleEscrowFactoryContract.address.

-  const addImplementationTx = await tDocDeployerThroughProxy.addImplementation(
-    TOKEN_IMPLEMENTATION_ADDRESS,
-    titleEscrowFactoryContract.address
-  );
+  const addImplementationTx = await tDocDeployerThroughProxy.addImplementation(
+    TOKEN_IMPLEMENTATION_ADDRESS,
+    titleEscrowFactoryAddress
+  );

Also applies to: 67-67


70-74: Hardcoded addresses will desync from the freshly deployed contracts.

Using fixed addresses instead of the just-deployed instances is fragile and likely incorrect for addImplementation. Use the live deployment addresses.

-const TOKEN_IMPLEMENTATION_ADDRESS = "0x0952a6817E00fc2455418a5303395760A9c4EE71"; //tokenImplementationContract.address
-const TITLE_ESCROW_FACTORY_ADDRESS2 = "0x547Ca63C8fB3Ccb856DEb7040D327dBfe4e7d20F"; //titleEscrowFactoryContract.address;
-const TDOC_DEPLOYER_ADDRESS = "0xfE442b75786c67E1e7a7146DAeD8943F0f2c23d2"; //tDocDeployerFactoryContract.address
-const ERC1967_PROXY_ADDRESS2 = "0x3488EAA1bF4f606f758b24F5ef6eb2a1E32335be"; //ERC1967ProxyFactoryContract.address
+const TOKEN_IMPLEMENTATION_ADDRESS = tokenImplementationContract.address;
+const ERC1967_PROXY_ADDRESS2 = ERC1967ProxyFactoryContract.address;
+const TDOC_DEPLOYER_ADDRESS = tDocDeployerFactoryContract.address;
+// If you keep the CLI TitleEscrowFactory, prefer TITLE_ESCROW_FACTORY_ADDRESS (above).

Also applies to: 81-83

src/test/setup-contracts-testcafe.js (4)

51-55: Avoid double deployment of TitleEscrowFactory; reuse CLI deployment.

Duplicate deployment can cause wiring inconsistencies.

-const titleEscrowFactory = new ethers.ContractFactory(
-  TitleEscrowFactory__factory.abi,
-  TitleEscrowFactory__factory.bytecode,
-  signer
-);
-const titleEscrowFactoryContract = await titleEscrowFactory.deploy();
+const titleEscrowFactoryAddress = TITLE_ESCROW_FACTORY_ADDRESS;

And update any downstream usage accordingly.

Also applies to: 67-67


70-74: Replace fixed addresses with live deployment addresses.

Same issue as the e2e script—use freshly deployed addresses.

-const TOKEN_IMPLEMENTATION_ADDRESS = "0x0952a6817E00fc2455418a5303395760A9c4EE71"; //tokenImplementationContract.address
-const TITLE_ESCROW_FACTORY_ADDRESS2 = "0x547Ca63C8fB3Ccb856DEb7040D327dBfe4e7d20F"; //titleEscrowFactoryContract.address;
-const TDOC_DEPLOYER_ADDRESS = "0xfE442b75786c67E1e7a7146DAeD8943F0f2c23d2"; //tDocDeployerFactoryContract.address
-const ERC1967_PROXY_ADDRESS2 = "0x3488EAA1bF4f606f758b24F5ef6eb2a1E32335be"; //ERC1967ProxyFactoryContract.address
+const TOKEN_IMPLEMENTATION_ADDRESS = tokenImplementationContract.address;
+const ERC1967_PROXY_ADDRESS2 = ERC1967ProxyFactoryContract.address;
+const TDOC_DEPLOYER_ADDRESS = tDocDeployerFactoryContract.address;

Also applies to: 81-83


125-129: Check CLI return codes when issuing.

Surface failures to avoid silent test flakiness.

-  merkleRootToMint.tokenRegistry.forEach((element) => {
-    shell.exec(
-      `${oaCLI_PATH} token-registry issue --beneficiary ${element.owner} --holder ${element.holder} --address ${element.tokenRegistryAddress} --tokenId ${element.merkleRoot} -n local -k ${element.accountKey}`
-    );
-  });
+  const run = (cmd) => { const r = shell.exec(cmd); if (r.code !== 0) throw new Error(`Command failed: ${cmd}`); };
+  merkleRootToMint.tokenRegistry.forEach((el) => {
+    run(`${oaCLI_PATH} token-registry issue --beneficiary ${el.owner} --holder ${el.holder} --address ${el.tokenRegistryAddress} --tokenId ${el.merkleRoot} -n local -k ${el.accountKey}`);
+  });

112-121: Remove hardcoded private keys; use environment variables

  • Critical: src/test/setup-contracts-testcafe.js contains hardcoded private keys ACCOUNT_KEY and ACCOUNT_KEY_2 (lines 19–20). Move them to env vars (process.env) and update .env.sample/.env.test; rotate any keys already pushed.
  • The 0x... 64-hex values in the merkle-root array (lines 112–121) are test tokenIds (not secrets) — add a short comment and/or adjust Gitleaks rules to avoid false positives.
🧹 Nitpick comments (53)
src/test/creator/happy-flow-magiclink.spec.ts (1)

39-42: Make the email field selector resilient to placeholder changes.

Relying on exact placeholder text is brittle. Match by attribute regex (case-insensitive) or input[type=email].

-// placeholder text Email address
-const emailInput = magicSigninModel.find('[placeholder="Email address"]');
+// match case-insensitively to reduce brittleness
+const emailInput = magicSigninModel.find("input").withAttribute("placeholder", /email/i);
jest.setup.ts (1)

3-9: Expose CryptoKey on window as well for libs accessing window.CryptoKey.

Some browser-oriented libs reference window.CryptoKey. Mirror it to reduce surprises.

 import { CryptoKey } from "@peculiar/webcrypto";
 
 Object.assign(global, {
   TextDecoder,
   TextEncoder,
   CryptoKey,
 });
+Object.assign(window as any, { CryptoKey });
src/common/hooks/useQueue.tsx (1)

22-23: ECDSA-SD 2023 signing with mandatory pointer—LGTM.

Using CryptoSuite.EcdsaSd2023 and mandating "/renderMethod" is consistent with VC 2.0; builder.renderMethod is set prior, so this should pass.

Optional: Mirror “user rejected” handling during mint to reset queueState instead of erroring, for a smoother UX.

Also applies to: 180-183

src/components/DynamicFormContainer/DynamicForm/AttachmentDropzone/FilesInfo/FilesInfo.tsx (2)

23-31: Avoid decoding base64 just to compute size (faster, safer).

Derive byte length from the base64 string length instead of calling atob. This avoids exceptions and reduces work on large files.

-        let decodedData = "";
-        let size = "0 B";
-        try {
-          decodedData = atob(data);
-          size = prettyBytes(decodedData.length);
-        } catch (e) {
-          console.warn("Invalid base64 data in file info:", e);
-          size = "Unknown size";
-        }
+        // Faster and safer: compute byte length from base64 without decoding
+        const base64 = (data || "").trim();
+        const bytes = Math.floor((base64.replace(/=+$/, "").length * 3) / 4);
+        const size = Number.isFinite(bytes) ? prettyBytes(bytes) : "Unknown size";

18-18: Use the ProcessedFiles type instead of any.

Tighten typing for readability and safety.

-      {filesInfo.map((file: any, key: number) => {
+      {filesInfo.map((file: ProcessedFiles, key: number) => {
src/components/DynamicFormContainer/DynamicForm/AttachmentDropzone/AttachmentDropzone.tsx (1)

27-34: Compute existing attachments’ size without decoding base64.

Replace atob in the reducer with byte-length math to avoid exceptions and improve performance.

-        ? uploadedFiles.reduce((acc: number, current: any) => {
-            try {
-              return acc + atob(current.data).length;
-            } catch (e) {
-              console.warn("Invalid base64 data in attachment:", e);
-              return acc; // Skip invalid data
-            }
-          }, 0)
+        ? uploadedFiles.reduce((acc: number, current: any) => {
+            const base64 = (current?.data || "").trim();
+            if (!base64) return acc;
+            const bytes = Math.floor((base64.replace(/=+$/, "").length * 3) / 4);
+            return Number.isFinite(bytes) ? acc + bytes : acc;
+          }, 0)
src/components/UI/AttachmentLink/AttachmentLink.tsx (1)

87-95: Guard decoding behind hasBase64 to prevent spurious warnings and work.

Currently the try/catch runs even when data/path isn’t base64, causing avoidable console noise.

-  try {
-    const decodedData = atob(data.startsWith(prefix) ? data.slice(prefix.length).trim() : data);
-    canOpenFile = isOpenAttestationFile(decodedData);
-    filesize = prettyBytes(decodedData.length);
-  } catch (e) {
-    console.warn("Invalid base64 data in attachment link:", e);
-    canOpenFile = false;
-    filesize = "Unknown size";
-  }
+  if (hasBase64) {
+    try {
+      const payload = data.startsWith(prefix) ? data.slice(prefix.length).trim() : data;
+      const decodedData = atob(payload);
+      canOpenFile = isOpenAttestationFile(decodedData);
+      filesize = prettyBytes(decodedData.length);
+    } catch (e) {
+      console.warn("Invalid base64 data in attachment link:", e);
+      canOpenFile = false;
+      filesize = "Unknown size";
+    }
+  }
src/components/AssetManagementPanel/AssetManagementActionOverlay/ActionManagementSkeleton.tsx (1)

29-29: Fix typo: isInitialized (not isInitalized).

Minor readability polish and avoids propagating misspelling.

-  const isInitalized = actionState === FormState.INITIALIZED;
+  const isInitialized = actionState === FormState.INITIALIZED;
@@
-              disabled={isInitalized}
+              disabled={isInitialized}
@@
-            disabled={isInitalized}
+            disabled={isInitialized}
@@
-            disabled={isInitalized}
+            disabled={isInitialized}
@@
-            {isInitalized ? <LoaderSpinner data-testid={"loader"} /> : <>Confirm</>}
+            {isInitialized ? <LoaderSpinner data-testid={"loader"} /> : <>Confirm</>}

Also applies to: 63-63, 81-81, 84-84, 89-89, 92-92

src/components/Button/Button.tsx (1)

82-84: Align disabled styling with Button for consistency.

ButtonIcon uses a different disabled palette and no important modifiers; consider reusing the same pattern to prevent override by parent styles.

-        disabled && "cursor-not-allowed bg-gray-50 text-gray-300 hover:bg-gray-200"
+        disabled ? "!cursor-not-allowed !bg-gray-200 !text-white hover:!bg-gray-200" : ""
src/common/hooks/useDidWeb.tsx (1)

68-70: Duplicate state update; keep a single setDidKeyPair.

setDidKeyPair is called twice back-to-back with the same value. Remove the first call.

Apply:

-      setDidKeyPair(JSON.parse(localStorageDid));
-      setDidKeyPair(parsedDid);
+      setDidKeyPair(parsedDid);
src/test/fixture/local/w3c/v2_tr_er_ECDSA_Signed.json (2)

12-16: Gitleaks false positives on tokenRegistry/tokenId — add a path allowlist for test fixtures.

Addresses and token IDs in fixtures are not secrets but may trigger generic key heuristics. Suppress scanning for this fixture path instead of weakening global rules.

Add or extend .gitleaks.toml:

+[allowlist]
+  description = "Allow test fixtures with non-secret addresses/hashes"
+  paths = [
+    "src/test/fixture/local/w3c/.*\\.json"
+  ]

Then re-run gitleaks locally to confirm no real leaks are masked.


1-10: Optional: vendor JSON-LD contexts for deterministic tests.

Remote contexts under https://trustvc.io/... can introduce network flakiness. Consider vendoring these contexts into tests and using a custom documentLoader in Jest to resolve them locally.

src/test/fixture/local/w3c/v2_invoice_qrcode.json (1)

2-8: Optional: consider pinning or caching remote contexts for CI stability.

Relying on multiple external @context URLs can introduce flakiness in CI. If feasible, add a local cache/mirror for tests.

src/test/qrcode.spec.ts (2)

14-15: Avoid awaiting Selector to reduce stale element risk.

Using await here returns a snapshot; prefer passing the Selector directly to t.click.

Apply this diff:

-  const qrcodeButtonElement = await Selector("button").withAttribute("aria-label", "document-utility-qr-button");
-  await t.click(qrcodeButtonElement); // asserts that button exists and can be clicked
+  const qrcodeButton = Selector("button").withAttribute("aria-label", "document-utility-qr-button");
+  await t.click(qrcodeButton); // asserts that button exists and can be clicked

5-6: Make logo assertion resilient to SVG usage.

Some QR implementations embed logos via SVG <image> (href/xlink:href) rather than <img src>. Support both to avoid false negatives.

Apply this diff:

-const logo = qrcode.child("img");
+const logoImg = qrcode.find("img");
+const logoSvg = qrcode.find("image"); // SVG variant

-  await t.expect(logo.count).eql(1); // asserts that qr contains logo
-  await t.expect(logo.withAttribute("src", "/static/images/logo-qrcode.png").exists).ok();
+  // asserts that qr contains logo (either HTML img or SVG image)
+  await t.expect(logoImg.count.add(logoSvg.count)).gt(0);
+  await t
+    .expect(
+      logoImg.withAttribute("src", "/static/images/logo-qrcode.png").exists
+        .or(logoSvg.withAttribute("href", "/static/images/logo-qrcode.png").exists)
+    )
+    .ok();

Also applies to: 17-20

src/test/fixture/local/w3c/v2_tr_er_ECDSA_Derived.json (2)

23-27: Normalize chainId type across fixtures.

Here chainId is a string "1337", while other new fixtures use the number 1337. Pick one type to reduce schema ambiguity in consumers.


26-27: Gitleaks false positive: address/token identifiers.

tokenRegistry and tokenId are not secrets but get flagged. Add a repo-level allowlist rule for test fixture paths/patterns to silence this class of false positives.

Example (adjust to your gitleaks config):

+[rules.allowlist]
+  description = "Allow known fixture TR identifiers"
+  paths = [
+    "src/test/fixture/.*\\.json",
+    "tests/e2e/fixtures/.*\\.json"
+  ]
src/test/attachments.spec.ts (1)

66-104: Reduce duplication between legacy and W3C attachment tests.

Both tests repeat the same assertions. Extract shared steps into a helper to simplify maintenance and cut flakiness surface.

Apply this diff:

+const assertAttachmentsUI = async (t) => {
+  // tabs number should tally
+  await TabDefault.with({ visibilityCheck: true })();
+  await t.expect(TabsItems.count).eql(4);
+  await t.expect(TabAttachment.textContent).contains("Attachments");
+  await t.expect(AttachmentNumber.textContent).contains("5");
+
+  // non-pdf tabs should not render
+  await t.expect(TabWordDoc.count).eql(0);
+  await t.expect(TabExcel.count).eql(0);
+  await t.expect(TabJpeg.count).eql(0);
+
+  // pdf tabs content should render
+  await t.click(TabPdf1);
+  await t.switchToIframe(Iframe);
+  await Pdf1Span.with({ visibilityCheck: true })();
+  await validateTextContent(t, SampleTemplate, ["UNCITRAL Model Law on Electronic Transferable Records"]);
+  await t.switchToMainWindow();
+
+  await t.click(TabPdf2);
+  await t.switchToIframe(Iframe);
+  await Pdf2Span.with({ visibilityCheck: true })();
+  await validateTextContent(t, SampleTemplate, ["Dummy PDF file"]);
+  await t.switchToMainWindow();
+
+  // attachment tab should render with correct attachment files count
+  await t.click(TabAttachment);
+  await t.expect(AttachmentLink.count).eql(5);
+};
@@
 test("W3C VC 2.0 Document with Attachments - Attachment Tab and Panel rendered correctly", async (t) => {
   await navigateToVerify();
   await uploadDocument("./fixture/local/w3c/v2_tr_er_attachment_ECDSA_Derived.json");
 
   // Validate document content
   await validateIframeTexts(["BILL OF LADING", "20250107"]);
 
-  // tabs number should tally
-  await TabDefault.with({ visibilityCheck: true })();
-  await t.expect(TabsItems.count).eql(4);
-  await t.expect(TabAttachment.textContent).contains("Attachments");
-  await t.expect(AttachmentNumber.textContent).contains("5");
-  // non-pdf tabs should not render
-  await t.expect(TabWordDoc.count).eql(0);
-  await t.expect(TabExcel.count).eql(0);
-  await t.expect(TabJpeg.count).eql(0);
-  // pdf tabs content should render ...
-  ...
-  await t.expect(AttachmentLink.count).eql(5);
+  await assertAttachmentsUI(t);
 }).timeouts({
tests/e2e/fixtures/w3c-2.0-bl-nominate-owner.json (2)

23-31: Confirm chainId and registry correctness for your test chain.

chainId uses number 1337 here (vs. string in another fixture). Align types and ensure tokenRegistry is the expected local/test contract.


29-30: Silence Gitleaks for fixture identifiers.

The address and tokenId trip “generic API key” rules. Add an allowlist for test fixtures to avoid noisy scans.

tests/e2e/fixtures/w3c-2.0-bl-surrender.json (2)

23-31: Type consistency on chainId and contract metadata.

Same note as other fixtures: keep chainId type consistent across all samples; verify tokenRegistry/tokenId align with the local devnet used by the E2E suite.


29-30: Address Gitleaks false positives for fixture data.

Whitelist fixture paths to prevent CI noise on hex-like identifiers.

src/components/AssetManagementPanel/AssetManagementForm/FormVariants/ActionForm/ActionForm.tsx (3)

304-306: INITIALIZED treated as pending: align editability for consistency.

You’ve disabled buttons/remarks on INITIALIZED, but inputs remain editable for several forms (isEditable ignores INITIALIZED). Consider disabling edits during INITIALIZED to avoid mid-init changes.

Example pattern:

- const isEditable = state !== FormState.PENDING_CONFIRMATION && state !== FormState.CONFIRMED;
+ const isEditable = ![FormState.PENDING_CONFIRMATION, FormState.INITIALIZED, FormState.CONFIRMED].includes(state);

Also applies to: 359-361, 433-435, 507-509, 578-581, 653-654, 729-730, 805-806


580-581: Remove console logs before merge.

Debug logs will leak to production and noisy tests.

- console.log("🚀 ~ beneficiaryEndorseState:", beneficiaryEndorseState);
...
- console.log("🚀 ~ holderTransferringState:", holderTransferringState);

Also applies to: 651-653


809-816: Validate owner change as well (avoid no-op transfers).

TransferOwnerHolder currently rejects unchanged holder but allows unchanged owner. Consider also requiring newOwner differ from beneficiary.

- if (!newHolder || !newOwner) return false;
- if (newHolder === holder) return false;
+ if (!newHolder || !newOwner) return false;
+ if (newHolder?.toLowerCase() === holder?.toLowerCase()) return false;
+ if (newOwner?.toLowerCase() === beneficiary?.toLowerCase()) return false;
src/components/DocumentStatus/DocumentStatus.test.tsx (2)

38-64: Deduplicate verification fragments via shared helpers.

These builders appear to exist under src/test/helpers/documentStatusVerification.ts per PR context. Import them here to avoid drift across files.

- const getOpenAttestationVerificationFragments = ...
- const getW3CVerificationFragments = ...
+ import {
+   getOpenAttestationVerificationFragments,
+   getW3CVerificationFragments
+ } from "../../test/helpers/documentStatusVerification";

Also applies to: 66-121


212-228: Avoid hardcoding explorer base URL in assertion.

Use chain utils to build the expected href so tests don’t break with network config changes.

const base = getSupportedChainInfo().find((c) => c.chainId === 1337)!.explorerAddressLink; // e.g., https://localhost/address/
expect(screen.getByText("View NFT Registry").closest("a")?.getAttribute("href")).toBe(`${base}${tokenAddress}`);
src/reducers/certificate.ts (1)

17-19: Consider decoupling schema state from display labels.

State stores display strings (e.g., "W3C VC 1.1"), which couples UI copy to logic and complicates future relabeling/localization. Prefer storing stable keys and mapping to labels in the view layer.

Example approach (outside this hunk):

  • Store: "W3C_VC_1_1" | "W3C_VC_2_0" | "OA_V2" | "OA_V3" | null
  • Map to labels in the component via a LABELS object.
tests/e2e/specs/5a-creator.spec.js (2)

9-14: Keep tag expectations resilient to environment.

Hard-coding "TR V5" can flake if a V4 registry is encountered. Make TR tag check accept either V4 or V5.

Apply this diff:

-const tags = [
+const tags = [
     "Transferable",
     "Negotiable",
     "W3C VC V2.0",
-    "TR V5"
 ];
+const possibleTRTags = ["TR V4", "TR V5"];

And below, assert one-of for TR:

-              [...verificationText, ...tags].forEach((text) => {
-                  cy.get("#document-status").should("contain", text);
-              });
+              [...verificationText, ...tags].forEach((text) => {
+                cy.get("#document-status").should("contain", text);
+              });
+              cy.get("#document-status")
+                .invoke("text")
+                .should((txt) => {
+                  expect(possibleTRTags.some((t) => txt.includes(t))).to.eq(true);
+                });

66-71: Avoid fixed sleeps for downloads; wait on readiness.

Replace the static wait(5000) with a polling check to reduce flakiness.

If you use cypress-wait-until, consider:

-        cy.get("[data-testid='download-all-button']").click();
-        cy.wait(5000);
+        cy.get("[data-testid='download-all-button']").click();
+        cy.waitUntil(
+          () => cy.task("getFilePath", { tempDirectory, fileName }).then(Boolean),
+          { timeout: 15000, interval: 500 }
+        );

Do you have cypress-wait-until installed in this repo? If not, I can propose a small custom retry with recursive cy.wrap.

src/components/AssetManagementPanel/AssetManagementTags/index.tsx (2)

19-21: Minor naming consistency nit.

tagCSOrange diverges from tagCSSBlue/tagCSSGrey. Consider tagCSSOrange for consistency.

-  const tagCSOrange = "bg-tangerine-500/[24%] text-tangerine-500 rounded-full font-gilroy-bold";
+  const tagCSSOrange = "bg-tangerine-500/[24%] text-tangerine-500 rounded-full font-gilroy-bold";

And update usages accordingly.


34-38: Confirm intent: OA tag shows for both OA_V2 and OA_V3.

Since both map to "OA", this condition will be true for either version. If you intended to show OA for both, consider checking documentSchema === "OA" or storing schema keys instead of labels.

src/utils/shared.test.ts (1)

121-181: Solid coverage for W3C VC 2.0; add one negative case for invalid tokenNetwork.chain.

You cover valid/invalid chainId and missing fields. Consider 1 extra test where tokenNetwork.chain is invalid (e.g., "AMOY123") to mirror OA v2/v3 tests and assert the same “Invalid Document…” path.

 it("should throw an error when W3C v2.0 document has invalid chain in tokenNetwork", () => {
   const doc = {
     ...w3cV2Document,
     credentialStatus: {
       ...w3cV2Document.credentialStatus,
-      tokenNetwork: { chain: "MATIC", chainId: "80002" },
+      tokenNetwork: { chain: "AMOY123", chainId: "80002" },
     },
   };
   expect(() => getChainId(doc as SignedVerifiableCredential)).toThrow(
     "Invalid Document, please use a valid document."
   );
 });
tests/e2e/setup-contracts.js (3)

7-12: Make the v5 contracts import resilient across CJS/ESM and install paths.

Hardcoding the node_modules CJS path is brittle. Provide a fallback strategy that tries the package entry, then the CJS path, and finally the resolved path.

-const v5ContractsPath = path.resolve(
-  __dirname,
-  "../../node_modules/@trustvc/trustvc/dist/cjs/token-registry-v5/contracts.js"
-);
-const v5Contracts = require(v5ContractsPath);
+let v5Contracts;
+try {
+  v5Contracts = require("@trustvc/trustvc/dist/cjs/token-registry-v5/contracts.js");
+} catch {
+  v5Contracts = require(
+    path.resolve(__dirname, "../../node_modules/@trustvc/trustvc/dist/cjs/token-registry-v5/contracts.js")
+  );
+}

14-14: Parameterize local chain id.

Many stacks default to 31337. Allow override via env to reduce local friction.

-const CHAIN_ID = { local: 1337 };
+const CHAIN_ID = { local: Number(process.env.LOCAL_CHAIN_ID ?? 1337) };

63-66: Confirm initializer calldata is correct for TDocDeployer.

You pass only 0x8129fc1c to the proxy constructor. If initialize has params, this will misconfigure the proxy.

-const ERC1967ProxyFactoryContract = await ERC1967ProxyFactory.deploy(
-  tDocDeployerFactoryContract.address,
-  "0x8129fc1c"
-);
+const initData = TDocDeployer__factory.interface.encodeFunctionData("initialize", []); // add args if needed
+const ERC1967ProxyFactoryContract = await ERC1967ProxyFactory.deploy(
+  tDocDeployerFactoryContract.address,
+  initData
+);
src/test/setup-contracts-testcafe.js (2)

7-12: Harden import path for v5 contracts (same as e2e script).

Mirror the resilient CJS import fallback to avoid path brittleness.

-const v5ContractsPath = path.resolve(
-  __dirname,
-  "../../node_modules/@trustvc/trustvc/dist/cjs/token-registry-v5/contracts.js"
-);
-const v5Contracts = require(v5ContractsPath);
+let v5Contracts;
+try {
+  v5Contracts = require("@trustvc/trustvc/dist/cjs/token-registry-v5/contracts.js");
+} catch {
+  v5Contracts = require(
+    path.resolve(__dirname, "../../node_modules/@trustvc/trustvc/dist/cjs/token-registry-v5/contracts.js")
+  );
+}

14-15: Parameterize local chain id.

Same rationale as e2e script.

-const CHAIN_ID = { local: 1337 };
+const CHAIN_ID = { local: Number(process.env.LOCAL_CHAIN_ID ?? 1337) };
src/test/w3c-document.spec.ts (3)

28-51: Remove unused scenario fields.

dateField and dateValue are never used. Drop them to avoid confusion/noise in test data.

   {
     version: "v1",
     credentialFile: "./fixture/local/w3c/v1_tr_er.json",
-    dateField: "issuanceDate",
-    dateValue: "2021-12-03T12:19:52Z",
     description: "basic W3C VC document",
   },
   {
     version: "v2.0",
     credentialFile: "./fixture/local/w3c/v2_tr_er_ECDSA_Signed.json",
-    dateField: "validFrom",
-    dateValue: "2024-04-01T12:19:52Z",
     description: "ECDSA V2.0 Signed W3C VC document",
   },
   {
     version: "v2.0",
     credentialFile: "./fixture/local/w3c/v2_tr_er_ECDSA_Derived.json",
-    dateField: "validFrom",
-    dateValue: "2024-04-01T12:19:52Z",
     description: "ECDSA V2.0 Derived W3C VC document",
   },

53-76: Remove unused scenario fields (attachments).

Same here—dateField and dateValue unused.

   {
     version: "v1",
     credentialFile: "./fixture/local/w3c/v1_tr_er_attachment.json",
-    dateField: "issuanceDate",
-    dateValue: "2021-12-03T12:19:52Z",
     description: "W3C VC document with attachments",
   },
   {
     version: "v2.0",
     credentialFile: "./fixture/local/w3c/v2_tr_er_attachment_ECDSA_Signed.json",
-    dateField: "validFrom",
-    dateValue: "2024-04-01T12:19:52Z",
     description: "ECDSA V2.0 Signed W3C VC document with attachments",
   },
   {
     version: "v2.0",
     credentialFile: "./fixture/local/w3c/v2_tr_er_attachment_ECDSA_Derived.json",
-    dateField: "validFrom",
-    dateValue: "2024-04-01T12:19:52Z",
     description: "ECDSA V2.0 Derived W3C VC document with attachments",
   },

92-131: Reduce flakiness: wait for iframe visibility and assert attachment count dynamically.

  • Add a visibility wait before switching to the iframe for PDFs.
  • Assert attachment badge number matches actual rendered attachments instead of hardcoding “5”.
   await t.expect(TabsItems.count).eql(4);
   await t.expect(TabAttachment.textContent).contains("Attachments");
-  await t.expect(AttachmentNumber.textContent).contains("5");
+  // check badge later after attachments tab renders, using dynamic count

   // non-pdf tabs should not render
   await t.expect(TabWordDoc.count).eql(0);
   await t.expect(TabExcel.count).eql(0);
   await t.expect(TabJpeg.count).eql(0);

   // pdf tabs content should render
   await t.click(TabPdf1);
-  await t.switchToIframe(Iframe);
+  await Iframe.with({ visibilityCheck: true })();
+  await t.switchToIframe(Iframe);
   await Pdf1Span.with({ visibilityCheck: true })();
   await validateTextContent(t, SampleTemplate, ["UNCITRAL Model Law on Electronic Transferable Records"]);
   await t.switchToMainWindow();

   await t.click(TabPdf2);
-  await t.switchToIframe(Iframe);
+  await Iframe.with({ visibilityCheck: true })();
+  await t.switchToIframe(Iframe);
   await Pdf2Span.with({ visibilityCheck: true })();
   await validateTextContent(t, SampleTemplate, ["Dummy PDF file"]);
   await t.switchToMainWindow();

   // attachment tab should render with correct attachment files count
   await t.click(TabAttachment);
-  await t.expect(AttachmentLink.count).eql(5);
+  const count = await AttachmentLink.count;
+  await t.expect(AttachmentLink.count).eql(count);
+  await t.expect(AttachmentNumber.textContent).contains(String(count));
tests/e2e/fixtures/w3c-2.0-bl-endorse-owner.json (2)

23-31: Gitleaks false positives: allowlist test fixture fields.

tokenRegistry (ETH address) and tokenId (hash) are flagged as generic API keys but are non-secret fixture values. Add a Gitleaks allowlist for test fixtures.

Example .gitleaks.toml additions:

[[rules]]
description = "Ignore fixture crypto-like values"
id = "fixtures-eth-hash-allow"
regex = '''(?i)^(0x[a-f0-9]{40}|[a-f0-9]{64})$'''
paths = ["tests/e2e/fixtures/"]

[[allowlist]]
paths = ["tests/e2e/fixtures/"]

2-9: Network dependencies in @context.

Multiple remote contexts can slow or flake E2E in CI. Consider vendoring/pinning contexts for tests.

src/components/ObfuscatedMessage/ObfuscatedMessage.tsx (2)

17-29: Guard against setState on unmounted/stale effects.

Add a cancellation flag to avoid updating state after unmount or when document changes rapidly.

   useEffect(() => {
-    const checkObfuscation = async () => {
+    let active = true;
+    const checkObfuscation = async () => {
       try {
         const result = await isObfuscated(document);
-        setIsDocumentObfuscated(result);
+        if (active) setIsDocumentObfuscated(result);
       } catch (error) {
         console.warn("Error checking if document is obfuscated:", error);
-        setIsDocumentObfuscated(false);
+        if (active) setIsDocumentObfuscated(false);
       }
     };
-
-    checkObfuscation();
-  }, [document]);
+    checkObfuscation();
+    return () => {
+      active = false;
+    };
+  }, [document]);

36-40: Consider externalizing user-facing strings.

Move the obfuscation messages to i18n to keep UI copy centralized.

src/components/ObfuscatedMessage/ObfuscatedMessage.test.tsx (2)

38-50: Rename variable for clarity.

container is a RenderResult, not an HTML container. Rename or destructure queries for readability.

-const container = render(
+const renderResult = render(
   <Provider store={store}>
     <ObfuscatedMessage document={ObfuscatedDocument as WrappedOrSignedOpenAttestationDocument} />
   </Provider>
 );
-// ...
-await waitFor(() => {
-  expect(container.queryByText("Note: There are fields/data obfuscated in this document.")).not.toBeNull();
-});
+await waitFor(() => {
+  expect(renderResult.queryByText("Note: There are fields/data obfuscated in this document.")).not.toBeNull();
+});

60-67: Prefer findBy over waitFor + queryByText for presence/absence.*

findByTestId/findByText reduces manual waiting. Optional but cleaner.

-await waitFor(
-  () => {
-    expect(container.queryByText("Note: There are fields/data obfuscated in this document.")).toBeNull();
-  },
-  { timeout: 1000 }
-);
+await expect(renderResult.queryByText("Note: There are fields/data obfuscated in this document.")).toBeNull();
src/components/AssetManagementPanel/AssetManagementTags/AssetManagementTags.test.tsx (6)

11-16: Stabilize hook mocking and typing.

Prefer an explicit mock factory and jest.mocked for safer typing and fewer surprises across Jest/TS configs.

-jest.mock("../../../common/hooks/useTokenRegistryVersion");
-const mockUseTokenRegistryVersion = tokenRegistryVersion.useTokenRegistryVersion as jest.MockedFunction<
-  typeof tokenRegistryVersion.useTokenRegistryVersion
->;
+jest.mock("../../../common/hooks/useTokenRegistryVersion", () => ({
+  useTokenRegistryVersion: jest.fn(),
+}));
+const mockUseTokenRegistryVersion = jest.mocked(
+  tokenRegistryVersion.useTokenRegistryVersion
+);

18-40: Derive slice initial state to avoid drift with reducer.

Hardcoding many fields makes tests brittle when the slice evolves. Derive from the reducer’s initial state and override only what’s needed; also tighten the documentSchema type.

-  const createMockStore = (documentSchema: any = null) => {
-    return configureStore({
+  type DocumentSchemaValue = (typeof DOCUMENT_SCHEMA)[keyof typeof DOCUMENT_SCHEMA] | null;
+  const createMockStore = (documentSchema: DocumentSchemaValue = null) => {
+    const base = certificateReducer(undefined, { type: "@@INIT" } as any);
+    return configureStore({
       reducer: {
         certificate: certificateReducer,
       },
       preloadedState: {
-        certificate: {
-          raw: null,
-          rawModified: null,
-          filename: "",
-          providerOrSigner: null,
-          tokenRegistryVersion: null,
-          documentSchema,
-          verificationPending: false,
-          verificationStatus: null,
-          verificationError: null,
-          retrieveCertificateByActionState: "INITIAL" as const,
-          retrieveCertificateByActionError: null,
-          keyId: null,
-        },
+        certificate: { ...base, documentSchema },
       },
     });
   };

42-49: Tighten helper typings for clarity.

Typing the helper reduces any spread and improves IDE help.

-const renderWithProvider = (store: any, props: any = {}) => {
+const renderWithProvider = (
+  store: ReturnType<typeof configureStore>,
+  props: React.ComponentProps<typeof AssetManagementTags> = {}
+) => {

112-118: Rename test for intent clarity.

Current name reads as if an OA_V2 tag exists. Make it explicit that OA_V2 maps to “OA”.

-it("should not display OA_V2 tag as it uses same value as OA_V3", () => {
+it("should display OA tag for OA_V2 due to identical mapping with OA_V3", () => {

188-191: Avoid querying by CSS selector; use a test id to reduce brittleness.

Class names change frequently with Tailwind refactors. Prefer a stable data-testid.

-      const tagsContainer = container.querySelector(".flex.flex-wrap.py-2.gap-2");
-      expect(tagsContainer).toBeInTheDocument();
-      expect(tagsContainer?.children).toHaveLength(0);
+      const tagsContainer = screen.getByTestId("asset-management-tags");
+      expect(tagsContainer.children).toHaveLength(0);

Add data-testid to the component container (outside this file):

// src/components/AssetManagementPanel/AssetManagementTags/index.tsx
<div data-testid="asset-management-tags" className="flex flex-wrap py-2 gap-2">

194-249: Reduce coupling to exact color classes.

Color tokens often change; asserting all classes can cause noisy failures. Consider asserting shape/intent (e.g., presence of rounded-full and a role/test id), or snapshot the tag container once per scenario.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5c0779b and fde3dda.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (46)
  • jest.config.js (1 hunks)
  • jest.setup.ts (1 hunks)
  • package.json (1 hunks)
  • src/common/hooks/useContractFunctionHook.tsx (1 hunks)
  • src/common/hooks/useDidWeb.tsx (4 hunks)
  • src/common/hooks/useQueue.tsx (2 hunks)
  • src/components/AssetManagementPanel/AssetManagementActionOverlay/ActionManagementSkeleton.tsx (2 hunks)
  • src/components/AssetManagementPanel/AssetManagementForm/AssetManagementForm.tsx (1 hunks)
  • src/components/AssetManagementPanel/AssetManagementForm/FormVariants/ActionForm/ActionForm.tsx (8 hunks)
  • src/components/AssetManagementPanel/AssetManagementTags/AssetManagementTags.test.tsx (1 hunks)
  • src/components/AssetManagementPanel/AssetManagementTags/index.tsx (1 hunks)
  • src/components/Button/Button.tsx (1 hunks)
  • src/components/DocumentStatus/DocumentStatus.test.tsx (4 hunks)
  • src/components/DynamicFormContainer/DynamicForm/AttachmentDropzone/AttachmentDropzone.tsx (1 hunks)
  • src/components/DynamicFormContainer/DynamicForm/AttachmentDropzone/FilesInfo/FilesInfo.tsx (1 hunks)
  • src/components/ObfuscatedMessage/ObfuscatedMessage.test.tsx (1 hunks)
  • src/components/ObfuscatedMessage/ObfuscatedMessage.tsx (1 hunks)
  • src/components/UI/AttachmentLink/AttachmentLink.tsx (1 hunks)
  • src/reducers/certificate.ts (1 hunks)
  • src/sagas/certificate.ts (2 hunks)
  • src/test/attachments.spec.ts (1 hunks)
  • src/test/creator/happy-flow-magiclink.spec.ts (1 hunks)
  • src/test/dns-did-verified.spec.ts (1 hunks)
  • src/test/fixture/local/w3c/v2_invoice_qrcode.json (1 hunks)
  • src/test/fixture/local/w3c/v2_tr_er_ECDSA_Derived.json (1 hunks)
  • src/test/fixture/local/w3c/v2_tr_er_ECDSA_Signed.json (1 hunks)
  • src/test/obfuscated-document.spec.ts (1 hunks)
  • src/test/qrcode.spec.ts (1 hunks)
  • src/test/setup-contracts-testcafe.js (2 hunks)
  • src/test/w3c-document.spec.ts (1 hunks)
  • src/utils/shared.test.ts (2 hunks)
  • tests/e2e/fixtures/w3c-2.0-bl-endorse-owner.json (1 hunks)
  • tests/e2e/fixtures/w3c-2.0-bl-nominate-owner.json (1 hunks)
  • tests/e2e/fixtures/w3c-2.0-bl-surrender.json (1 hunks)
  • tests/e2e/fixtures/w3c-2.0-bl-transfer-holder.json (1 hunks)
  • tests/e2e/setup-contracts.js (2 hunks)
  • tests/e2e/specs/1a-transfer-holder.spec.js (1 hunks)
  • tests/e2e/specs/2a-surrender-reject.spec.js (2 hunks)
  • tests/e2e/specs/2b-surrender-accept.spec.js (2 hunks)
  • tests/e2e/specs/3a-endorse-transfer-owner-holder.spec.js (1 hunks)
  • tests/e2e/specs/3b-endorse-nominate-owner.spec.js (1 hunks)
  • tests/e2e/specs/3c-endorse-nominate-owner-accept.spec.js (1 hunks)
  • tests/e2e/specs/4a-reject-transfer-holder.spec.js (1 hunks)
  • tests/e2e/specs/4b-reject-transfer-owner.spec.js (1 hunks)
  • tests/e2e/specs/4c-reject-transfer-owner-holder.spec.js (1 hunks)
  • tests/e2e/specs/5a-creator.spec.js (2 hunks)
🧰 Additional context used
🧬 Code graph analysis (17)
src/test/dns-did-verified.spec.ts (1)
src/test/helper.js (8)
  • navigateToVerify (28-37)
  • navigateToVerify (28-37)
  • uploadDocument (50-53)
  • uploadDocument (50-53)
  • validateIssuerTexts (79-82)
  • validateIssuerTexts (79-82)
  • validateIframeTexts (55-59)
  • validateIframeTexts (55-59)
src/sagas/certificate.ts (1)
src/reducers/certificate.ts (1)
  • DOCUMENT_SCHEMA (14-19)
src/components/UI/AttachmentLink/AttachmentLink.tsx (1)
src/components/Demo/DemoCreate/DemoCreateForm/data.tsx (1)
  • data (4-44)
src/components/AssetManagementPanel/AssetManagementActionOverlay/ActionManagementSkeleton.tsx (1)
src/components/Button/Button.tsx (1)
  • Button (42-64)
src/components/AssetManagementPanel/AssetManagementTags/AssetManagementTags.test.tsx (2)
src/components/AssetManagementPanel/AssetManagementTags/index.tsx (1)
  • AssetManagementTags (13-61)
src/reducers/certificate.ts (1)
  • DOCUMENT_SCHEMA (14-19)
src/components/AssetManagementPanel/AssetManagementTags/index.tsx (2)
src/components/UI/Tag/Tag.tsx (1)
  • Tag (10-16)
src/reducers/certificate.ts (1)
  • DOCUMENT_SCHEMA (14-19)
src/test/qrcode.spec.ts (1)
src/test/helper.js (8)
  • navigateToVerify (28-37)
  • navigateToVerify (28-37)
  • uploadDocument (50-53)
  • uploadDocument (50-53)
  • validateIssuerTexts (79-82)
  • validateIssuerTexts (79-82)
  • validateIframeTexts (55-59)
  • validateIframeTexts (55-59)
src/test/obfuscated-document.spec.ts (1)
src/test/helper.js (6)
  • navigateToVerify (28-37)
  • navigateToVerify (28-37)
  • uploadDocument (50-53)
  • uploadDocument (50-53)
  • validateIssuerTexts (79-82)
  • validateIssuerTexts (79-82)
src/test/attachments.spec.ts (2)
src/test/helper.js (12)
  • navigateToVerify (28-37)
  • navigateToVerify (28-37)
  • uploadDocument (50-53)
  • uploadDocument (50-53)
  • validateIframeTexts (55-59)
  • validateIframeTexts (55-59)
  • Iframe (6-6)
  • Iframe (6-6)
  • validateTextContent (15-19)
  • validateTextContent (15-19)
  • SampleTemplate (8-8)
  • SampleTemplate (8-8)
src/components/UI/AttachmentLink/AttachmentLink.tsx (1)
  • AttachmentLink (80-137)
src/components/DynamicFormContainer/DynamicForm/AttachmentDropzone/FilesInfo/FilesInfo.tsx (1)
src/components/Demo/DemoCreate/DemoCreateForm/data.tsx (1)
  • data (4-44)
src/components/ObfuscatedMessage/ObfuscatedMessage.tsx (3)
src/utils/shared.ts (1)
  • WrappedOrSignedOpenAttestationDocument (27-27)
src/reducers/index.ts (1)
  • RootState (14-14)
src/reducers/certificate.ts (1)
  • DOCUMENT_SCHEMA (14-19)
tests/e2e/setup-contracts.js (1)
src/test/setup-contracts-testcafe.js (3)
  • defaultToken (89-94)
  • ADDRESS_EXAMPLE_2 (22-22)
  • oaCLI_PATH (24-24)
src/utils/shared.test.ts (1)
src/utils/shared.ts (2)
  • WrappedOrSignedOpenAttestationDocument (27-27)
  • getChainId (117-165)
src/components/DocumentStatus/DocumentStatus.test.tsx (4)
src/utils/shared.ts (1)
  • WrappedOrSignedOpenAttestationDocument (27-27)
src/store.ts (1)
  • configureStore (11-15)
src/common/utils/chain-utils.ts (1)
  • getSupportedChainInfo (48-51)
src/components/DocumentStatus/DocumentStatus.tsx (1)
  • DocumentStatus (16-56)
src/test/w3c-document.spec.ts (2)
src/test/helper.js (14)
  • navigateToVerify (28-37)
  • navigateToVerify (28-37)
  • uploadDocument (50-53)
  • uploadDocument (50-53)
  • validateIssuerTexts (79-82)
  • validateIssuerTexts (79-82)
  • validateIframeTexts (55-59)
  • validateIframeTexts (55-59)
  • Iframe (6-6)
  • Iframe (6-6)
  • validateTextContent (15-19)
  • validateTextContent (15-19)
  • SampleTemplate (8-8)
  • SampleTemplate (8-8)
src/components/UI/AttachmentLink/AttachmentLink.tsx (1)
  • AttachmentLink (80-137)
src/test/setup-contracts-testcafe.js (1)
tests/e2e/setup-contracts.js (2)
  • require (3-3)
  • ERC1967Proxy_artifact (4-4)
src/components/ObfuscatedMessage/ObfuscatedMessage.test.tsx (3)
src/reducers/certificate.ts (2)
  • DocumentSchemaType (21-21)
  • DOCUMENT_SCHEMA (14-19)
src/components/ObfuscatedMessage/ObfuscatedMessage.tsx (1)
  • ObfuscatedMessage (13-44)
src/utils/shared.ts (1)
  • WrappedOrSignedOpenAttestationDocument (27-27)
🪛 Gitleaks (8.27.2)
tests/e2e/fixtures/w3c-2.0-bl-surrender.json

[high] 29-29: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


[high] 30-30: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

tests/e2e/fixtures/w3c-2.0-bl-nominate-owner.json

[high] 29-29: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


[high] 30-30: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

tests/e2e/fixtures/w3c-2.0-bl-endorse-owner.json

[high] 29-29: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


[high] 30-30: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

src/test/fixture/local/w3c/v2_tr_er_ECDSA_Signed.json

[high] 14-14: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


[high] 15-15: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

tests/e2e/fixtures/w3c-2.0-bl-transfer-holder.json

[high] 29-29: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


[high] 30-30: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

tests/e2e/setup-contracts.js

[high] 100-100: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


[high] 106-106: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


[high] 111-111: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


[high] 116-116: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

src/components/DocumentStatus/DocumentStatus.test.tsx

[high] 101-101: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
  • GitHub Check: Redirect rules - tradetrust-dev
  • GitHub Check: Redirect rules - reference-implementation
  • GitHub Check: Header rules - tradetrust-dev
  • GitHub Check: Header rules - reference-implementation
  • GitHub Check: Pages changed - tradetrust-dev
  • GitHub Check: Pages changed - reference-implementation
  • GitHub Check: Synpress e2e Test
  • GitHub Check: Lint & Test
  • GitHub Check: Testcafe e2e Test
  • GitHub Check: Lighthouse Desktop

Comment on lines +29 to +31
"^@digitalbazaar/security-context$": "<rootDir>/node_modules/@digitalbazaar/security-context/js/index.js",
"^@digitalbazaar/(.*)$": "<rootDir>/node_modules/@digitalbazaar/$1/lib/index.js",
"^base(58|64url)-universal$": "<rootDir>/node_modules/base$1-universal/lib/index.js"
Copy link

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Be cautious with broad @digitalbazaar and base-universal Jest mappings.*

Some packages under @digitalbazaar are ESM-only or have non-uniform entry points. Mapping everything to lib/index.js (and security-context to js/index.js) can break resolution if the file doesn’t exist or is ESM in a CJS pipeline. Prefer verifying actual entry files for only the packages you import, or enable ESM transforms (babel-jest/ts-jest) and drop custom mappings for those modules.

Run this to confirm each mapped package has a resolvable entry (and to spot any ESM-only packages that your mapping can’t load):


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Discover @digitalbazaar packages imported in repo
pkgs=$(rg -nP "@digitalbazaar/([A-Za-z0-9._-]+)" -o --no-filename | sed -E 's/.*@digitalbazaar\///' | sort -u)

echo "Checking @digitalbazaar package entrypoints..."
missing=0
for p in $pkgs; do
  ok=""
  for path in \
    "node_modules/@digitalbazaar/$p/lib/index.js" \
    "node_modules/@digitalbazaar/$p/js/index.js" \
    "node_modules/@digitalbazaar/$p/dist/index.js"
  do
    if [ -f "$path" ]; then
      echo "OK  $p -> $path"
      ok=1; break
    fi
  done
  if [ -z "$ok" ]; then
    echo "MISSING entry for $p (no lib/js/dist index)."
    missing=1
  fi
done

echo "Checking base*-universal entrypoints..."
for p in base58-universal base64url-universal; do
  if [ -f "node_modules/$p/lib/index.js" ]; then
    echo "OK  $p -> lib/index.js"
  else
    echo "MISSING entry for $p (no lib/index.js)."
    missing=1
  fi
done

exit $missing

Length of output: 669


Broken Jest moduleNameMapper — mapped targets missing (tests will fail)

Verification found these mapped packages lack the targeted files: @digitalbazaar/{bbs-signatures, bls12-381-multikey, data-integrity, di-sd-primitives, ecdsa-multikey, ecdsa-sd-2023-cryptosuite, http-client, security-context}; base58-universal; base64url-universal.

Location: jest.config.js lines 29-31 — current mappings:

    "^@digitalbazaar/security-context$": "<rootDir>/node_modules/@digitalbazaar/security-context/js/index.js",
    "^@digitalbazaar/(.*)$": "<rootDir>/node_modules/@digitalbazaar/$1/lib/index.js",
    "^base(58|64url)-universal$": "<rootDir>/node_modules/base$1-universal/lib/index.js"

Action: either correct/remove these moduleNameMapper entries to point at each package's real entry (check package.json main/exports) or enable ESM transforms (babel-jest/ts-jest) and drop custom mappings for ESM-only packages.

🤖 Prompt for AI Agents
In jest.config.js around lines 29-31 the moduleNameMapper entries point to
non-existent files for several @digitalbazaar packages and base58/base64url
universal packages causing tests to fail; fix by either (A) replacing each
mapped path with the actual file exported by that package (open each package in
node_modules/@digitalbazaar/<pkg>/package.json and use its "main" or "exports"
target, and update "^@digitalbazaar/(.*)$" to explicit per-package mappings
rather than a generic /lib path; for base58/base64url point to the package's
real entry), or (B) remove these custom moduleNameMapper entries and enable ESM
handling in Jest (install and configure babel-jest or ts-jest, set
extensionsToTreatAsEsm and appropriate transform for .js/.mjs files) so the
ESM-only packages resolve without manual mapping; choose one approach and update
jest.config.js accordingly, then run tests to verify resolution.

Comment on lines +115 to 121
if ((e as Error).message?.includes("user rejected transaction")) {
setState("UNINITIALIZED");
return;
}
setError(e as Error);
setState("ERROR");
}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Handle user rejection robustly across wallets/providers.

Match on standard EIP-1193 code (4001), ethers reason ACTION_REJECTED, and generic denial text; also reset state cleanly.

-      if ((e as Error).message?.includes("user rejected transaction")) {
-        setState("UNINITIALIZED");
-        return;
-      }
+      const err: any = e;
+      const msg = String(err?.message || err?.error?.message || "");
+      const code = err?.code;
+      if (code === 4001 || err?.reason === "ACTION_REJECTED" || /user.*(reject|denied)/i.test(msg)) {
+        setState("UNINITIALIZED");
+        setError(undefined);
+        return;
+      }
       setError(e as Error);
       setState("ERROR");

Additionally, remove the empty id when absent (align with comment “Only include id if not null”):

-      const options = { id: keyId ?? "" };
+      const options = keyId ? { id: keyId } : {};
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if ((e as Error).message?.includes("user rejected transaction")) {
setState("UNINITIALIZED");
return;
}
setError(e as Error);
setState("ERROR");
}
const err: any = e;
const msg = String(err?.message || err?.error?.message || "");
const code = err?.code;
if (code === 4001 || err?.reason === "ACTION_REJECTED" || /user.*(reject|denied)/i.test(msg)) {
setState("UNINITIALIZED");
setError(undefined);
return;
}
setError(e as Error);
setState("ERROR");
}
Suggested change
if ((e as Error).message?.includes("user rejected transaction")) {
setState("UNINITIALIZED");
return;
}
setError(e as Error);
setState("ERROR");
}
const options = keyId ? { id: keyId } : {};
🤖 Prompt for AI Agents
In src/common/hooks/useContractFunctionHook.tsx around lines 115-121, the error
handling should treat wallet/provider user rejections more robustly: detect
EIP-1193 rejection via error.code === 4001, detect ethers-style rejections via
(error as any).reason === 'ACTION_REJECTED', and also match generic messages
like "user rejected" or "denied" (case-insensitive); when any of these matches,
clear any error state (setError(null) or undefined) and reset state to
"UNINITIALIZED" (do not leave an ERROR state). Also update the response
construction to omit the id field entirely when it is null/empty (only include
id if non-null/non-empty) so no empty id is returned.

Comment on lines +31 to 37
headers: {
"Content-Type": "application/json",
},
body: JSON.stringify({
type: CryptoSuite.EcdsaSd2023,
}),
});
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Harden the createDidWeb request (status handling).

If the server returns non-2xx, the caller proceeds with an invalid JSON. Check response.ok and surface a clear error.

Apply:

-    const response: Response = await fetch(`${didCreateServer}/didweb`, {
+    const response: Response = await fetch(`${didCreateServer}/didweb`, {
       method: "POST",
       headers: {
         "Content-Type": "application/json",
+        "Accept": "application/json",
       },
       body: JSON.stringify({
         type: CryptoSuite.EcdsaSd2023,
       }),
     });
-    return await response.json();
+    if (!response.ok) {
+      throw new Error(`DID create failed (${response.status} ${response.statusText})`);
+    }
+    return await response.json();
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
headers: {
"Content-Type": "application/json",
},
body: JSON.stringify({
type: CryptoSuite.EcdsaSd2023,
}),
});
const response: Response = await fetch(`${didCreateServer}/didweb`, {
method: "POST",
headers: {
"Content-Type": "application/json",
"Accept": "application/json",
},
body: JSON.stringify({
type: CryptoSuite.EcdsaSd2023,
}),
});
if (!response.ok) {
throw new Error(`DID create failed (${response.status} ${response.statusText})`);
}
return await response.json();
🤖 Prompt for AI Agents
In src/common/hooks/useDidWeb.tsx around lines 31 to 37, the fetch call that
creates the DIDWeb does not check response.ok and proceeds to parse JSON even on
non-2xx responses; modify the request flow to first await the response, check
response.ok, and if false throw a new Error including response.status and await
response.text() (or statusText) so callers get a clear failure; only call
response.json() when response.ok is true, and propagate or surface the thrown
error to the caller.

Comment on lines +50 to 56

if (parsedDid.type !== VerificationType.Multikey) {
localStorage.removeItem("did");
localStorage.removeItem("didId");
return false;
}

Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Avoid parsing before presence checks; clear invalid cache on parse/shape errors.

Currently JSON.parse runs even when localStorageDid is null/invalid, relying on exceptions for control flow and skipping cache cleanup on parse failure.

Apply:

-      const parsedDid = JSON.parse(localStorageDid);
-
-      if (parsedDid.type !== VerificationType.Multikey) {
-        localStorage.removeItem("did");
-        localStorage.removeItem("didId");
-        return false;
-      }
+      if (!localStorageDid || !localStorageId) {
+        return false;
+      }
+      let parsedDid: any;
+      try {
+        parsedDid = JSON.parse(localStorageDid);
+      } catch {
+        localStorage.removeItem("did");
+        localStorage.removeItem("didId");
+        return false;
+      }
+      if (parsedDid?.type !== VerificationType.Multikey) {
+        localStorage.removeItem("did");
+        localStorage.removeItem("didId");
+        return false;
+      }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const parsedDid = JSON.parse(localStorageDid);
if (parsedDid.type !== VerificationType.Multikey) {
localStorage.removeItem("did");
localStorage.removeItem("didId");
return false;
}
if (!localStorageDid || !localStorageId) {
return false;
}
let parsedDid: any;
try {
parsedDid = JSON.parse(localStorageDid);
} catch {
localStorage.removeItem("did");
localStorage.removeItem("didId");
return false;
}
if (parsedDid?.type !== VerificationType.Multikey) {
localStorage.removeItem("did");
localStorage.removeItem("didId");
return false;
}
🤖 Prompt for AI Agents
In src/common/hooks/useDidWeb.tsx around lines 50 to 56, avoid calling
JSON.parse before confirming the value exists and ensure invalid JSON or
unexpected shapes clear the cached entries; first check whether localStorageDid
is truthy and return false if not, then wrap JSON.parse in a try/catch, and in
the catch (or if parsed object lacks the expected type field or has a type !==
VerificationType.Multikey) remove "did" and "didId" from localStorage and return
false so malformed or absent cache is cleaned and parsing errors are handled
gracefully.

<button
className={`${shared} ${className} ${
disabled ? "cursor-not-allowed bg-gray-200 text-white hover:bg-gray-200" : ""
disabled ? "!cursor-not-allowed !bg-gray-200 !text-white !hover:bg-gray-200" : ""
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Tailwind important modifier is misplaced for hover.

Use hover:!bg-gray-200 instead of !hover:bg-gray-200; the latter won’t be generated/applied.

-        disabled ? "!cursor-not-allowed !bg-gray-200 !text-white !hover:bg-gray-200" : ""
+        disabled ? "!cursor-not-allowed !bg-gray-200 !text-white hover:!bg-gray-200" : ""
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
disabled ? "!cursor-not-allowed !bg-gray-200 !text-white !hover:bg-gray-200" : ""
disabled ? "!cursor-not-allowed !bg-gray-200 !text-white hover:!bg-gray-200" : ""
🤖 Prompt for AI Agents
In src/components/Button/Button.tsx around line 55, the Tailwind important
modifier is incorrectly placed as !hover:bg-gray-200 which prevents the hover
utility from being generated; change it to hover:!bg-gray-200 in the conditional
class string (i.e., replace "!hover:bg-gray-200" with "hover:!bg-gray-200") so
the hover rule is applied with !important while keeping the other disabled
modifiers intact.

Comment on lines +96 to 121
const tokensToMint = {
tokenRegistry: [
{
// Endorse Owner
merkleRoot: "0x69f86e95549d3bd5768fb6bbbea5ed7232856a43fe8ae96df4d000d1a5125637",
tokenId: "0x1a32e030f8ebef2a6a00c3513086b27b1233095df6cae6a47e6a36ecc09b7cf9",
...defaultToken,
},
{
// Nominate Owner
...defaultToken,
merkleRoot: "0xd352ab5e4a8a736ecce02d37842ff45721e138b08789e5231cca1e1b6794b3f4",
tokenId: "0x06d7151cfd58bff997b599a742e77e0bbef694d8e502824b5157517698c02577",
holder: ADDRESS_EXAMPLE_2,
},
{
// Surrender
merkleRoot: "0x84be0d43bb6e6e36c3dff96b0619737e18b116bf2a9ed229ba266057516c3bfa",
tokenId: "0x300e1a6fcecfd40fad5f0e3167bd3be6c6fcd3e7ec3f48b157e29b4ab1d32755",
...defaultToken,
},
{
// Transfer Holder
merkleRoot: "0x55f73a6a83243410b45929e53cc84327c2d24cb78f96200748590d3ec54fa099",
tokenId: "0xf22f8487eced43cf4b0afb2043f83877690f05ed8ecea5177305b945525b4e60",
...defaultToken,
},
],
};

Copy link

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Gitleaks false positives on tokenId values; real risk is hardcoded private keys.

The flagged lines are tokenIds, not secrets. However, ACCOUNT_KEY/ACCOUNT_KEY_2 (Lines 19-21) are hardcoded; move to env with safe defaults.

-const ACCOUNT_KEY = "0x…";
-const ACCOUNT_KEY_2 = "0x…";
+const ACCOUNT_KEY = process.env.LOCAL_PRIVATE_KEY ?? "0x…"; // dev-only default
+const ACCOUNT_KEY_2 = process.env.LOCAL_PRIVATE_KEY_2 ?? "0x…";

Run this to confirm ethers major version (v5 vs v6) so provider/wallet APIs are correct:


🏁 Script executed:

#!/bin/bash
rg -n '"ethers"\s*:\s*"' package.json -C1 || rg -n --no-heading '\bethers\b' -g 'package*.json' -C1

Length of output: 239


Critical: remove hardcoded private keys

Hardcoded ACCOUNT_KEY and ACCOUNT_KEY_2 found in tests/e2e/setup-contracts.js (lines 19–21) — move them to environment variables (dev-only defaults), remove committed keys from the repo, and rotate any exposed keys.

-const ACCOUNT_KEY = "0x…";
-const ACCOUNT_KEY_2 = "0x…";
+const ACCOUNT_KEY = process.env.LOCAL_PRIVATE_KEY ?? "0x…"; // dev-only default
+const ACCOUNT_KEY_2 = process.env.LOCAL_PRIVATE_KEY_2 ?? "0x…";

Ethers confirmed as ^5.8.0 in package.json (v5 APIs apply).

Committable suggestion skipped: line range outside the PR's diff.

🧰 Tools
🪛 Gitleaks (8.27.2)

[high] 100-100: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


[high] 106-106: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


[high] 111-111: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


[high] 116-116: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

Comment on lines +122 to 126
tokensToMint.tokenRegistry.forEach((element) => {
shell.exec(
`${oaCLI_PATH} token-registry issue --beneficiary ${element.owner} --holder ${element.holder} --address ${element.tokenRegistryAddress} --tokenId ${element.merkleRoot} -n local -k ${element.accountKey}`
`${oaCLI_PATH} token-registry issue --beneficiary ${element.owner} --holder ${element.holder} --address ${element.tokenRegistryAddress} --tokenId ${element.tokenId} -n local -k ${element.accountKey}`
);
});
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Fail fast on CLI errors.

shell.exec return codes are ignored; add minimal checks to surface failures early.

-  tokensToMint.tokenRegistry.forEach((element) => {
-    shell.exec(
-      `${oaCLI_PATH} token-registry issue --beneficiary ${element.owner} --holder ${element.holder} --address ${element.tokenRegistryAddress} --tokenId ${element.tokenId} -n local -k ${element.accountKey}`
-    );
-  });
+  const run = (cmd) => { const r = shell.exec(cmd); if (r.code !== 0) throw new Error(`Command failed: ${cmd}`); };
+  tokensToMint.tokenRegistry.forEach((el) => {
+    run(`${oaCLI_PATH} token-registry issue --beneficiary ${el.owner} --holder ${el.holder} --address ${el.tokenRegistryAddress} --tokenId ${el.tokenId} -n local -k ${el.accountKey}`);
+  });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
tokensToMint.tokenRegistry.forEach((element) => {
shell.exec(
`${oaCLI_PATH} token-registry issue --beneficiary ${element.owner} --holder ${element.holder} --address ${element.tokenRegistryAddress} --tokenId ${element.merkleRoot} -n local -k ${element.accountKey}`
`${oaCLI_PATH} token-registry issue --beneficiary ${element.owner} --holder ${element.holder} --address ${element.tokenRegistryAddress} --tokenId ${element.tokenId} -n local -k ${element.accountKey}`
);
});
const run = (cmd) => { const r = shell.exec(cmd); if (r.code !== 0) throw new Error(`Command failed: ${cmd}`); };
tokensToMint.tokenRegistry.forEach((el) => {
run(`${oaCLI_PATH} token-registry issue --beneficiary ${el.owner} --holder ${el.holder} --address ${el.tokenRegistryAddress} --tokenId ${el.tokenId} -n local -k ${el.accountKey}`);
});
🤖 Prompt for AI Agents
In tests/e2e/setup-contracts.js around lines 122 to 126, the shell.exec calls
that invoke the OA CLI ignore return codes so CLI failures are swallowed; modify
the loop to capture the result of shell.exec, check result.code (or
result.status) after each invocation, and if non-zero log the command,
stderr/exit code and throw an Error or call process.exit(1) to fail fast; keep
the shell.exec invocation otherwise the same and ensure the test setup stops
immediately on any CLI error.

@rongquan1 rongquan1 merged commit 0cffb01 into master Sep 11, 2025
21 of 22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants