Skip to content

Major upgrade: RN 0.81 / React 19 / credo-ts 0.6.3, VC 2.0, RCE v3 + eddsa-rdfc-2022 Data Integrity, witnessed exchange#17

Open
albertoleon7794 wants to merge 33 commits into
mainfrom
upgrade/consolidated-vc2-di
Open

Major upgrade: RN 0.81 / React 19 / credo-ts 0.6.3, VC 2.0, RCE v3 + eddsa-rdfc-2022 Data Integrity, witnessed exchange#17
albertoleon7794 wants to merge 33 commits into
mainfrom
upgrade/consolidated-vc2-di

Conversation

@albertoleon7794

@albertoleon7794 albertoleon7794 commented Jul 18, 2026

Copy link
Copy Markdown
Collaborator

📖 Reviewing this PR? Start with the runbook: docs/REVIEWING_THE_UPGRADE.md — setup, static gates, the E2E ladder (and what each run proves), security invariants, and a suggested review order.

Consolidates the full upgrade effort into one branch for review. Supersedes
the stacked upgrade PRs #13 and #14 (closed in favor of this).

Scope (29 commits; see each ticket for design/details):

Proven on real devices: VC 2.0 credential, eddsa-rdfc-2022 DI-signed,
hardware-attested, and witnessed (both contact shields).

Companion bifold PR: berkmancenter/keyring-bifold#31

Known follow-up (non-blocking): DIDComm perf regression since 0.6 — #16.

Reviewer note: the app diff is the review target. The bifold submodule is a
fork-swap (old fork -> bifold-3.x), ~2,257 commits of fork history, not new code.

Replace hardcoded BC Gov mediator URL check with
MEDIATOR_USE_V2_BATCH_PICKUP. When true, agent startup initiates
Pickup V2 live mode and batch pickup for Credo-TS mediators.

Refs #4

Signed-off-by: Alberto L <aleon@law.harvard.edu>
WebdriverIO harness under e2e/: fresh install, onboarding, invitation
paste, bidirectional VRC offer acceptance, and Contacts-list assertion
on Android emulator + iOS simulator. Green end-to-end run recorded in
UPGRADE_PROGRESS.md. Silence dev-only IndyVdr LogBox toast that
overlaid the UI and broke taps.

Signed-off-by: Alberto L <aleon@law.harvard.edu>
Signed-off-by: Alberto L <aleon@law.harvard.edu>
App template updated per Upgrade Helper: Gradle 8.10.2 / AGP 8.7.3 /
Kotlin 2.0.21 / buildTools 35 / NDK 27.1.12297006, plugin-based
autolinking in settings.gradle, OpenSourceMergedSoMapping in
MainApplication, iOS deployment target 15.1, -bundleURL AppDelegate,
Flipper removed from Podfile.

Ecosystem bumps (gesture-handler 2.22, safe-area-context 5.1,
screens 4.6, svg 15.11, vision-camera 4.7.3, webview 13.13).

RN 0.76+ prefab fix: patch @hyperledger anoncreds/askar/indy-vdr
CMakeLists to link the merged ReactAndroid::reactnative target, plus
jniLibs pickFirsts for the duplicated prefab .so files.

bcsc-core codegen spec adjusted for RN 0.77's stricter parser
(UnsafeObject instead of any/indexer, no Omit<>).

Gate: app jest 23/88 PASS, bifold core jest 134/1242 PASS, Android
assembleDebug and iOS simulator build both succeed.

Signed-off-by: Alberto L <aleon@law.harvard.edu>
Signed-off-by: Alberto L <aleon@law.harvard.edu>
Removes the react-native-bcsc-core native module workspace
(packages/bcsc-core) and the entire BCSC surface in the app:
src/bcsc-theme/, the BCSC store slice (BCSCState, BCSCDispatchAction,
Mode enum), BCSCApiClientProvider, Developer screen theme/mode
toggles, useSetupSteps, and related tests. Root now always renders
the KeyRing stack and keyring-theme remains the only theme.

Also cleans up the @bcsc-theme babel/tsconfig/metro aliases and the
bcsc-core workspace/dependency entries; gradle locks and Podfile.lock
regenerated (RNBcscCore pod gone).

Gate: typecheck, eslint, jest (14 suites / 27 tests), Android
assembleDebug and iOS simulator build all pass.

Signed-off-by: Alberto L <aleon@law.harvard.edu>
…t regression

- Upgrade @react-native-firebase app/messaging 14.12.0 -> 21.14.0 (v14 is
  incompatible with RN 0.77; iOS crashed at startup with "No Firebase App
  '[DEFAULT]' has been created").
- Set RCTAppDependencyProvider in AppDelegate.mm, required by RN 0.77 for
  third-party Fabric component/module resolution (fixes
  setSheetLargestUndimmedDetent unrecognized-selector crash from
  react-native-screens).
- Apply flex:1 SafeAreaView/ScrollView fix to PersonCredentialLoading
  (same safe-area-context 5.x layout regression fixed in bifold).
- Bump bifold submodule (layout fixes in modal screens).
- Mark Phase 2 complete in UPGRADE_PROGRESS.md: jest green, both platforms
  build, two-device E2E VRC exchange green on Android and iOS.
The quality workflow still built packages/bcsc-core, which was removed in
the Phase 2 BCSC cleanup, so the Lint+Format+Types job failed on a missing
directory. Remove the build/cache steps and re-enable the TypeScript check
that was disabled pending bcsc theme type errors (now gone; passes locally).
… 0.6.3

Wire the app workspace to the upgraded bifold 3.x submodule:

- deps: react-native 0.81.5 / react 19.1.0, @bifold/* 3.0.16 portals (incl.
  new @bifold/react-hooks), @credo-ts/* 0.6.3 with @credo-ts/didcomm,
  askar -> @openwallet-foundation/askar-react-native 0.6.0, expo modules,
  reanimated + keyboard-controller; adopt upstream patch set, drop 0.5.17
  patches; remove Storybook entirely
- bc-agent-modules: credo 0.6 module graph (DidCommModule config, KMS with
  askar + expo secure-environment, explicit JSON-LD/AnonCreds DIDComm format
  services, VRC document loader)
- useBCAgentSetup: wallet id/key move into AskarModule store config (no
  agent.wallet.open()), didcomm transport registration, guarded indy-vdr
  pool warm-up
- attestation: remove DRPC-based BC attestation flow (@credo-ts/drpc has no
  stable 0.6.x and Keyring does not use BC's attestation service); delete
  utils/drpc.ts; keep bifold AttestationMonitor interface
- credo 0.6 API renames across hooks/utils (DidComm* types, proofExchangeRecordId,
  metadata keys from @credo-ts/anoncreds root)
- jest/babel: sync transformIgnorePatterns with bifold core, map @bifold/* to
  sources, RN 0.81 Keyboard/BackHandler/reanimated mocks, drop module-resolver
  root '.' (rewrote node_modules '.' imports to app index.js), add
  export-namespace-from + reanimated babel plugins
- snapshots updated for RN 0.81 style flattening; theme gains new bifold 3.x
  IBrandColors fields

App gate (JS): typecheck 0 errors, eslint clean, jest 14 suites / 27 tests pass.
Native Android/iOS project upgrade is the next step.
Android: compileSdk/targetSdk 36, kotlin 2.1.20, gradle 8.14.3, AGP from RN
gradle plugin, newArchEnabled=true, expo modules autolinking (settings.gradle
useExpoModules + ReactNativeHostWrapper/loadReactNative in MainApplication),
react.internal.disableJavaVersionAlignment + subprojects Java 17 alignment
(mirrors bifold samples/app), gradle lockfiles regenerated. assembleDebug green.

iOS: Podfile gains expo autolinking (use_expo_modules! +
expo_patch_react_imports!), pods reinstalled on RN 0.81.5. xcodebuild Debug
(simulator) green. AppDelegate stays on RCTAppDelegate for now.

Bundling (metro 0.83): exclusionList import moved to metro-config/private,
package exports enabled with react-native/browser/require conditions,
bifold/node_modules added to watchFolders/nodeModulesPaths,
react-native-svg-transformer 0.14 -> 1.5.1. Release bundles green for both
platforms.

Deps: add zustand (bifold core lists it only as devDep) and
@expo/app-integrity (used by bifold attestation hooks). Patch
react-native-document-picker 9.3.1 to drop GuardedResultAsyncTask
(removed in RN 0.81).

Gates re-run after changes: typecheck 0 errors, eslint clean, jest 14/27 pass.
- metro: re-resolve react/react-native/react-dom/@credo-ts singletons from the
  app root when imported from outside app/ (bifold sources were pulling a second
  react copy from bifold/node_modules in dev bundles, crashing the hooks
  dispatcher); add @bifold/react-hooks to the dev source-resolution list
- remove react-native-date-picker: unused, and 5.0.13 fails RN 0.81's
  RCTModuleProvider conformance check on iOS

Verified: Android emulator reaches onboarding, iOS simulator reaches PIN
unlock (existing askar store opens). Both native builds and release bundles
re-verified after the changes; typecheck clean.
- attestation service: credo 0.6 renamed credential event payload
  credentialRecord -> credentialExchangeRecord (handler threw "Cannot read
  property 'id' of undefined" on every credential event)
- index.js: LogBox.ignoreAllLogs() in dev — notification banners overlay
  bottom buttons (Done) and eat taps during E2E; everything still logs to
  metro/logcat/syslog
- e2e: iOS caps get enforceAppInstall (stale simulator builds caused RN
  version-mismatch redbox), document ad-hoc signing requirement for keychain
  entitlements; acceptCredentialOfferFromChat retries the YES tap until the
  offer screen opens
- resolutions: pin @hyperledger/indy-vdr-shared 0.2.4 (duplicate instance
  broke pool init)
- bifold submodule: E2E fixes (event names, gesture-handler chat touchables,
  react-hooks memo loop fix, issuanceDate clock-skew backdate)
- UPGRADE_PROGRESS.md: Phase 3 E2E gate green, fixes documented
… present

The RN Modal hosting the camera-use disclosure occasionally fails to present
when it mounts mid push-animation on iOS, leaving a blank Scan screen with no
Continue or PasteUrlButton. Back out and re-enter the screen to re-present it.

Also gitignore app/android/app/.cxx/ (CMake build cache).
run-store-migration.js installs the baseline apk (credo 0.5.17/askar
0.2.3, release build so its own JS is bundled), populates the old-format
store with a real VRC exchange against the new iOS build, upgrades in
place with `adb install -r`, and verifies the wallet unlocks and the
contact + VRC survive.

- deliver the invitation to the old android build via deep link (its QR
  bottom-sheet ignores appium synthetic taps; deep link is the real-user
  path anyway)
- grant runtime permissions via `adb shell pm grant` (manual installs
  skip appium's autoGrantPermissions)
- iOS: `simctl privacy grant camera` kills the app; terminate + wait +
  activate avoids a black-screen race on relaunch
- app: run migrateRCardTemplateProofs() after agent.initialize() so
  legacy R-Card template records get a proof before any provider reads
  W3C credential records (bifold 3dd6a786)

Signed-off-by: Alberto L <aleon@law.harvard.edu>
Signed-off-by: Alberto L <aleon@law.harvard.edu>
The baseline is one worktree of the root repo (submodule init for bifold),
not two separate worktrees. Worktrees under /tmp were removed; the
upgrade-baseline-p0 tags are the source of truth.

Signed-off-by: Alberto L <aleon@law.harvard.edu>
…ive-mode pickup

Port the generic app-layer improvements from upstream bc-wallet-mobile
(BCSC-specific work intentionally skipped):

- errors framework (src/errors/): AppError + trimmed ErrorRegistry +
  handler utils, ErrorInfoCard/AppErrorModal/ErrorBoundary components,
  ErrorAlertProvider/useErrorAlert context. Snowplow analytics coupling
  dropped; "Report this problem" ships the report to Loki via
  reportProblem() and surfaces a copyable reference code. App.tsx now
  uses our ErrorBoundaryWrapper and mounts ErrorAlertProvider.
- PressableOpacity component (New-Architecture-reliable Pressable
  wrapper mimicking TouchableOpacity).
- hooks: usePreventGestureBack, useDeclineCredentialOffer,
  useDeclineProofRequest, useAutoRequestPermission.
- mediator pickup simplified to unconditional PickUpV2LiveMode
  (WebSocket push); legacy batch-pickup + 5s trust-ping polling loop
  removed and MEDIATOR_USE_V2_BATCH_PICKUP flag retired. Verified
  against the hosted mediator via the two-device VRC E2E.
- i18n: en/fr/pt-br strings for the error modal reference-code UI.

Gate: app jest 16 suites/70 tests, bifold core 155 suites/1398 tests,
typecheck, and Android<->iOS VRC exchange E2E all green.

Signed-off-by: Alberto L <aleon@law.harvard.edu>
…edo patch)

Bump the bifold submodule to the Phase 5 commit (RCard/VRC separation, JCS
witness digest, VCDM 2.0 verification + negotiated issuance with RCE v2).
Apply the @credo-ts/core 0.6.3 yarn patch in the app tree as well (accept
the VC 2.0 context as first @context entry, make issuanceDate optional) so
the wallet's own dependency graph resolves the same patched credo.

Gates green 2026-07-08: app + bifold jest/typecheck/lint, two-device E2E
VRC exchange on the VC 2.0 flow, and a backward-compat E2E where a Phase-4
baseline build exchanges with the new app (legacy 1.1 VRC with issuer
object, no RCard) and then upgrades in place with its store intact.
…y-pick)

Pull in bifold 88f80fe8: embed Google's three published attestation roots
(legacy RSA, re-signed RSA, RKP ECDSA) and validate Android hardware
attestation chains against them only, instead of accepting the chain's own
root as its anchor. Needed for real-device attestation with biometrics.

Verified on the upgraded stack: 7/7 attestation JUnit tests, 29/29
BiometricSignatureVerifier TS tests, app assembleDebug green.
Rewrite HARDWARE_ATTESTATION_FLOW for native verifyHardwareEvidence and
preference-gated Secure Exchange. Add CRYPTO_SUITE_FOLLOWUP so
Ed25519Signature2018 stays documented as intentional until a clean stack
can take DataIntegrityProof (eddsa-rdfc-2022) as a separate project.
Ship dual real-device VRC harness, RN Hermes SIGBUS yarn patch, and
bifold bump for modal/a11y/credential-dump fixes.

Signed-off-by: Alberto L <aleon@law.harvard.edu>
…ual-verify

Installs the @credo-ts/didcomm cryptosuite patch (cryptosuite in jsonld
options + dual-field proof matching), gates VRC/RCard offers on RCE v3
(DataIntegrityProof/eddsa-rdfc-2022 for capable peers, Ed25519Signature2018
otherwise), registers the DI suite + dev self-test probe in the app, adds
witness/reference dual-verify, and fixes the witnessed-flow VP submission
for VCDM 2.0. Plan and findings: docs/CRYPTO_SUITE_FOLLOWUP.md.

Signed-off-by: Alberto L <aleon@law.harvard.edu>
The witnessed exchange now carries Data Integrity end-to-end for RCE v3
pairs: the app signs the VP-submitted VRC with eddsa-rdfc-2022 when the
counterparty announced v3, and the witness mirrors the observed proof
family when issuing VWCs (DI VRC observed -> DI VWC issued). Includes the
witnessed-exchange context fix required for safe-mode JSON-LD signing.
See docs/CRYPTO_SUITE_FOLLOWUP.md.

Signed-off-by: Alberto L <aleon@law.harvard.edu>
…, 0.6 API migration, did re-import keys fix)

Signed-off-by: Alberto L <aleon@law.harvard.edu>
The @digitalcredentials/jsonld-signatures 9.4.0 patch targeted a version
no longer installed anywhere (both trees resolve to 12.0.1 via credo
0.6.3), and v12 ships its own React Native SHA-256 shim built on
expo-crypto (already an app dependency) — the patch has been dead since
the credo upgrade. patch-package postinstall verified clean after
removal (rdf-canonize 5.0.0 still applies; that one is load-bearing for
RDFC on RN).

Metro comments updated: the js-sha256 alias/interceptor now serves the
patched rdf-canonize only. See docs/CRYPTO_SUITE_FOLLOWUP.md carry-over
checklist.

Signed-off-by: Alberto L <aleon@law.harvard.edu>
The unlock helper keyed off the Enter button, but the inactivity-lock
screen ("You were locked out after N minutes") auto-submits on the final
PIN digit and has no Enter button — only the cold-start unlock screen has
one. So a wallet that auto-locked while the harness waited (e.g. for the
slower attested iOS credential offer during the real-device attestation
gate) was never detected, and the wait died staring at the PIN screen.

Now detect by the PIN field and treat Enter as optional. This unblocked
the real-device e2e:vrc:devices attestation gate.

Signed-off-by: Alberto L <aleon@law.harvard.edu>
…devices)

Adds an attended two-phone witnessed exchange: starts a local witness
server (LAN-reachable), both wallets connect to it, then the existing
hardware-attested VRC exchange auto-routes through the witness and each
wallet stores a VWC. With the DI work this proves the witness issues a
DataIntegrityProof/eddsa-rdfc-2022 VWC end to end — the last uncovered
cell of the DI matrix. Also the witnessed-device re-run owed after the
witnessed-flow VP fixes.

- e2e/lib/witness.js: witness-server lifecycle (LAN-IP detect, spawn
  transpile-only, free-ports, ready-on-invitation-file, teardown)
- flows.js: connectToWitness, openContactDetail, assertWitnessCredential
- run-vrc-exchange-witnessed-devices.js: witness bootstrap + two
  witness-connects + VWC assertions around the proven device exchange
- bifold submodule bump (testIDs + witness askar import-order fix)

Spec: docs/spikes/witnessed-e2e-spec.md. Reachability: phones + Mac on one
Wi-Fi (WITNESS_HOST_IP overrides auto-detect).

Signed-off-by: Alberto L <aleon@law.harvard.edu>
The witnessed e2e needs both wallets connected to a locally-run witness,
but the app blocks cleartext http (usesCleartextTraffic=false), so a
plain-LAN witness is unreachable by design. Two approaches tried:

- Routing the witness through the app's mediator (wss) reached it, but the
  witness's mediator pickup websocket was unstable (askar DidCommConnection
  record read failures) and connections never completed on the witness side
  — no witness-announcement, so no witnessed exchange.
- A cloudflared quick tunnel (no account needed) gives the witness a public
  https:// endpoint while it runs in its normal DIRECT mode. This is how a
  production witness is reached (real mediators have SSL) and is stable:
  verified end to end — NEW PARTICIPANT CONNECTED + witness-announcement
  sent to a real device.

Also corrects the connect signal: there is NO "connected to witness" banner
in the app (WitnessStatusBanner is mounted nowhere; witness participation
only surfaces as a VWC after an exchange). connectToWitness now just pastes
the invitation and the caller confirms completion server-side via the
witness log (witness.waitForParticipants).

The full two-phone witnessed exchange (VWC on both) is the remaining
attended validation. debug-witness-connect.js isolates the connect step
unattended for future debugging.

Signed-off-by: Alberto L <aleon@law.harvard.edu>
…s, nav)

Finishes the witnessed VRC exchange e2e — proven on real devices to produce
a VC 2.0 / eddsa-rdfc-2022 credential that is hardware-attested AND
witnessed (both contact shields):

- witness.js: WITNESS_LOCALITY_REQUIRED=false — the witness otherwise
  rejects the VP with a locality (BLE co-location) error and the exchange
  falls back to unwitnessed; Appium phones can't produce a locality proof
  (production keeps it enforced).
- flows.js: assertContactShields requires BOTH "Secure Exchange"
  (attestation) and Witness Records (VWC) on a contact; connectToWitness
  returns to the Contacts tab afterward (connecting lands on the witness
  chat, which hid the QR opener); iOS shield poll bumped to 240s for the
  slower post-0.6 exchange (see keyring-wallet#16).
- runner: waitForParticipants(2) confirms both witness connects server-side,
  then assertContactShields on both.
- bifold: submodule bump for the static-import + badge-testID fixes.

Signed-off-by: Alberto L <aleon@law.harvard.edu>
Two Linux-CI-only failures (both pass on case-insensitive macOS / were
pre-existing from the upgrade):

- jestSetup.js mocked '.../openid/refresh/RefreshOrchestrator' but the file
  is 'refreshOrchestrator.ts' (lowercase) — module-not-found on Linux CI
  failed every test suite. Corrected the case.
- prettier formatting in three Phase-3 upgrade files (notifications.ts,
  attestation.ts, credentials.ts).

- bifold: submodule bump for the React 19 act() test fixes.

Signed-off-by: Alberto L <aleon@law.harvard.edu>
Picks up bifold 88b4e2c7, which types the witness-server interval fields
as ReturnType<typeof setInterval> so the topological bifold build no
longer aborts at witness-server. That abort had skipped remote-logs,
leaving @bifold/remote-logs without type declarations and failing the
app "Check types" CI step. App typecheck is green with this bump.

Signed-off-by: Alberto L <aleon@law.harvard.edu>
@albertoleon7794
albertoleon7794 force-pushed the upgrade/consolidated-vc2-di branch from 2b9a935 to 1c29525 Compare July 19, 2026 00:32
@albertoleon7794
albertoleon7794 marked this pull request as ready for review July 20, 2026 13:17
Signed-off-by: Alberto L <alberto_leon@seas.harvard.edu>
Adds docs/REVIEWING_THE_UPGRADE.md — a command-driven guide for the
principal engineer and future reviewers of upgrade/consolidated-vc2-di:
setup, static gates, the E2E ladder (and what each run does/doesn't
prove), feature verification, security invariants, known limitations,
and a suggested review order. Cross-references the paired bifold PR.

Signed-off-by: Alberto L <aleon@law.harvard.edu>
@albertoleon7794
albertoleon7794 force-pushed the upgrade/consolidated-vc2-di branch from 50b65c6 to b00f438 Compare July 21, 2026 19:49
@bmiller59

Copy link
Copy Markdown
Collaborator

DRY follow-ups in the VRC module (non-blocking)

While reviewing the VRC exchange flow (bifold/packages/core/src/modules/vrc/**), a few duplications are worth cleaning up in a follow-up — none affect correctness, they're maintainability/DRY notes:

  1. counterpartySpeaksVc20 / counterpartySpeaksDi in vrc-manager.ts are near-identical: same RelationshipDidRepository lookup, same ?? 1 default, differing only in the version threshold (>= 2 vs >= 3) and both swallowing errors to false. Suggest collapsing into a single counterpartyRceVersionAtLeast(agent, counterpartyRelationshipDid, minVersion: number) helper.

  2. buildVrcCredential (vrc-manager.ts) and buildRCardCredential (services/rCardCredential.ts) duplicate the same useVc20/useDi@context array selection logic (VCDM2+DI vs VCDM2+Ed25519-suite-context vs legacy VCDM1.1), each with its own copy of the "why" comment. A shared selectCredentialContexts({ useVc20, useDi }, ...) helper would remove the duplication and the risk of the two credential builders drifting on proof-context rules as the DI/VC2.0 rollout continues.

  3. credential.encoded raw-JSON extraction (getting the credential JSON out of a W3cCredentialRecord, tolerating both plain objects and class instances with .toJSON()) is implemented independently four times: utils/rcardDisplayUtils.ts (toRawCredential — the most careful version, added in this PR), utils/witnessCredentialUtils.ts (six inline copies, no .toJSON() handling), utils/vrcNameHelper.ts (two more inline copies), and hooks/useDeleteContact.ts (one more). Since the robust toRawCredential helper is new code in this very PR, it'd be worth exporting it and pointing the other three files at it rather than keeping their own inline credential.encoded reads.

Happy to file these as a follow-up issue if that's preferred over addressing inline.

@bmiller59

Copy link
Copy Markdown
Collaborator

vrc-reference doesn't exercise the app's new capability-negotiation surface

Comparing bifold/packages/core/src/modules/vrc/** (the app) against bifold/packages/vrc-reference/** (the reference/conformance package) across this PR (main@74ac281d → this branch@1755fbb):

Aligned: the credo-ts 0.6 migration (agent.connectionsagent.modules.didcomm.connections, CredentialEventTypesDidCommCredentialEventTypes, etc.), relationship-DID creation (PeerDidNumAlgo.InceptionKeyWithoutDoc + the same credo-0.6 KMS key-lookup re-import fix), the VCDM 2.0 shape swap (issuanceDatevalidFrom + v2 context), and the witnessed-flow proof-family mirroring (Witness.getMirroredJsonLdProofOptions mirrors whatever proof type it's shown, same design as the app's getVrcJsonLdProofOptions) are all consistently applied in both packages. These parts look like a safe, faithful upgrade relative to vrc-reference's pre-PR behavior — no leftover old-API calls, no shape mismatches found.

Gap — the app's new headline feature isn't represented in vrc-reference at all:

  1. No RCE version negotiation. The app announces/stores a capability version (vrc:rceVersion:<n>) per counterparty and gates VC2.0/DataIntegrity issuance on it (counterpartySpeaksVc20/counterpartySpeaksDi in vrc-manager.ts). Participant.ts has no equivalent — it unconditionally builds VCDM 2.0 and unconditionally signs with 'Ed25519Signature2018' at all three call sites. It never issues a v1-legacy credential and never issues a DataIntegrityProof credential from the peer-to-peer issuer path.
  2. No RCard. RelationshipCard/buildRCardCredential — the contact-info credential the app now exchanges separately from the VRC — doesn't exist anywhere under packages/vrc-reference (zero grep hits outside the app module).
  3. Handshake format diverged further. The app's basic-message handshake gained an rceVersion field this PR; Participant.ts's JSON.stringify({ rDid: did }) handshake has no field to carry it, so vrc-reference can't even represent the new capability announcement, let alone negotiate on it.
  4. DI issuance is only proven in isolation. The new diConformance.test.ts is a single in-memory-agent crypto round-trip (sign → verify → tamper-check), not an end-to-end DIDComm exchange. Because Participant.ts never sends a DI-signed VRC, the Witness's vrcUsesDi branch is unreachable from any of the real integration tests (connectionFlow, witnessedFlow, credentialIssuance, etc.) — only from that synthetic single-agent test.

Why this matters: vrc-reference tests passing is good evidence the credo 0.6 migration and the DI cryptosuite adapter are correct in isolation, but it is not evidence that the app's actual new feature — RCE v1/v2/v3 capability negotiation driving dual-stack proof selection over a real DIDComm exchange, plus the RCard split — works end-to-end, since vrc-reference doesn't implement or test that surface. Given vrc-reference is likely to become the foundation of a conformance test suite, this seems worth closing before relying on it for that purpose.

Suggested follow-up: update vrc-reference's Participant to negotiate rceVersion and issue DI-proof VRCs the same way the app does, and add an RCard-equivalent exchange — bringing it to full parity with the app's negotiated capability surface, while keeping the already-confirmed-safe parts (0.6 migration, VC2.0 shape, proof mirroring) as-is. If vrc-reference is meant to deliberately stay v1/v2-only for now, that scope limitation should be stated explicitly in BIFOLD_FLOW_ALIGNMENT.md so reviewers don't read "vrc-reference tests pass" as end-to-end conformance proof for the RCE v3 / RCard rollout.

@bmiller59

Copy link
Copy Markdown
Collaborator

Credential display architecture: the RCard split needs a cross-credential composition path

Looking at how VRC/RCard content is displayed (ContactDetails.tsx, ListContacts.tsx, CredentialOffer.tsx, ListCredentials.tsx), the original intent — that a credential type's display data is assembled into one object regardless of which underlying credential(s) it comes from — has only partially survived the RCard split, and the gap has already produced a concrete inconsistency worth fixing.

What exists today

Two separate, non-communicating systems answer "what should this contact/credential look like":

  1. CredentialDisplayHandler registry (display/displayRegistry.ts, RelationshipCredentialHandler, WitnessCredentialHandler) — a clean per-type strategy pattern, but canHandle/extractFields operate on exactly one credential JSON at a time. There's no way for a handler to look at sibling credentials.
  2. resolveContactDisplayInfo (utils/rcardDisplayUtils.ts) — a hand-rolled function that scans all w3cCredentialRecords, re-implements its own DTGCredential/RelationshipCredential/RCard type-matching (duplicating isDTGCredential/isRelationshipCredential from the registry file), and merges name/email/organization from whichever credential (RCard or legacy embedded issuer object) has them. It's called ad hoc from four separate screens (ContactDetails.tsx, ListContacts.tsx, CredentialOffer.tsx, chat-messages.tsx).

The RCard split introduced a genuinely new requirement — one contact's display identity now legitimately spans two W3C credentials (the VRC + the RCard) — but instead of extending the registry to handle that, a parallel resolver was bolted on beside it. Concretely, this already causes drift: in CredentialOffer.tsx, the header name is resolved via resolveContactDisplayInfo (RCard-aware) while the field list below it is resolved via RelationshipCredentialHandler.extractFields (not RCard-aware — reads the embedded issuer object only). For a new-style (v2/v3) offer, the header shows the right name while the field list's "Issuer Name/Email/Organization" rows simply don't render. Not a data-correctness bug, but a symptom of the same information being computed two different ways in the same screen.

Recommended fix

Extend the existing handler architecture rather than working around it, so there's one doorway instead of two:

1. Add cross-credential context and a subject output to the handler interface (display/types.ts):

export interface CredentialDisplayContext {
  /** Sibling W3C records in the wallet, for handlers that need to cross-reference
   *  (e.g. RelationshipCredentialHandler resolving a contact's RCard). */
  relatedRecords: W3cCredentialRecord[]
}

export interface CredentialDisplaySubject {
  id: string
  name?: string
  email?: string
  organization?: string
}

export interface CredentialDisplayHandler {
  extractFields(credential: W3cCredentialJson, context: CredentialDisplayContext): Field[]
  /** Optional: canonical display identity for this credential's issuer/subject —
   *  used for headers, chat messages, contact screens. Handlers with no notion
   *  of "subject" (e.g. WitnessCredentialHandler) simply omit it. */
  extractSubject?(credential: W3cCredentialJson, context: CredentialDisplayContext): CredentialDisplaySubject
  // ...unchanged
}

2. Thread it through CredentialDisplayResult / getDisplayInfo (display/displayRegistry.ts):

export interface CredentialDisplayResult {
  fields: Field[]
  buttonText: CredentialButtonText
  matched: boolean
  credentialTypeName?: string
  subject?: CredentialDisplaySubject   // NEW
}

getDisplayInfo(credential: W3cCredentialJson, context: CredentialDisplayContext): CredentialDisplayResult {
  const handler = this.findHandler(credential)
  if (!handler) return { fields: [], buttonText: DEFAULT_BUTTON_TEXT, matched: false }
  return {
    fields: handler.extractFields(credential, context),
    subject: handler.extractSubject?.(credential, context),
    buttonText: handler.getButtonText(),
    matched: true,
    credentialTypeName: handler.getCredentialTypeName?.(),
  }
}

3. Move (not delete) the RCard-resolution logic into RelationshipCredentialHandler:

extractSubject(credential: W3cCredentialJson, { relatedRecords }: CredentialDisplayContext): CredentialDisplaySubject {
  const issuer = extractIssuerObject(credential)
  const rcardInfo = resolveContactDisplayInfo(relatedRecords, issuer.id) // existing, tested logic — just relocated
  return {
    id: issuer.id,
    name: rcardInfo.name ?? issuer.name,
    email: rcardInfo.email ?? issuer.email,
    organization: rcardInfo.organization ?? issuer.organization,
  }
}

rcardDisplayUtils.ts's jCard parsing and its existing test suite stay as-is — only the entry point moves.

4. Update the four call sites (CredentialOffer.tsx header, ContactDetails.tsx, ListContacts.tsx, chat-messages.tsx) to read .subject off the one getDisplayInfo() call instead of separately calling resolveContactDisplayInfo. Header and field list become structurally guaranteed to agree, since they're now sourced from the same function call.

Why this shape

  • Additive, not a breaking rewriteextractSubject/context are optional/new parameters; handlers that don't need cross-referencing are unaffected.
  • Preserves the registry's real strength (the core/VRC decoupling via ICredentialDisplayRegistry) rather than introducing a second architecture alongside it.
  • Closes the existing duplicationisDTGCredential/isRelationshipCredential vs. the type-matching re-implemented in resolveContactDisplayInfo collapse into one call path.
  • Regression-proofs itself — a test asserting getDisplayInfo(vrcCredential, { relatedRecords }).subject.name matches what a screen renders as its header would fail immediately if a future change reintroduces a second, drifting path.

Scope: display/types.ts, display/displayRegistry.ts, RelationshipCredentialHandler.ts, and the four call sites above, plus tests. A moderate, well-scoped follow-up — not something that should block this PR, but worth filing before more fields or another related-credential-type need the same cross-referencing.

@bmiller59

Copy link
Copy Markdown
Collaborator

witness-server: dual-stack design is sound, but the test suite doesn't exercise the changed behavior

Compared bifold/packages/witness-server/** between main@74ac281d and this branch@1755fbb (7 commits, WitnessService.ts is the bulk of it at 462 changed lines).

The good news: backward compatibility holds

Dual-verify for Ed25519Signature2018 vs DataIntegrityProof/eddsa-rdfc-2022 was implemented by registering a new signature suite into Credo's existing suite-resolution machinery (DataIntegritySuiteModule), not by rewriting verification logic — so the verification path for old-style VRCs is untouched. Issuance mirrors the same way: getMirroredJsonLdProofOptions explicitly defaults to the old hardcoded Ed25519Signature2018/VC1.1 options for anything that isn't a recognized DI proof, so an old client's VRC still produces a byte-identical old-shape VWC (WitnessService.ts VC2.0 branch is gated on vrcContexts[0] === CREDENTIALS_V2_CONTEXT_URL; legacy VRCs fall through to the pre-existing @context/issuanceDate/expirationDate shape unchanged). The freshness-window widening (5→10 min) is a coordinated fix matching a backdating allowance already present on the app side (vrc-manager.ts), not an arbitrary loosening. This is genuine dual-stack support, not a bolt-on.

Finding 1 (moderate-high): the test suite doesn't test the changed code

WitnessService.test.ts (2524 lines) wasn't touched in this PR despite the 462-line source diff. On inspection, it's not just stale — it's structurally decoupled from the real implementation:

  • The "Freshness Check" tests hardcode the old 5-minute window and assert a 6-minute-old credential is rejected. That assertion is now false against the shipped code (which accepts up to 10 minutes) — but the test still passes because it only checks its own local arithmetic, never handlePresentationSubmission's actual freshness logic.
  • The "VWC Building" tests call a local buildMockVWC() helper that still canonicalizes with the old JSON.stringify(vrcJson, Object.keys(vrcJson).sort()) (pre-jcsCanonicalize) and builds issuer as a bare string, while the real buildWitnessCredential now emits issuer: { id, name }. Neither a digest-algorithm regression nor an issuer-shape regression would be caught by this test.
  • Nothing in the file references getMirroredJsonLdProofOptions, CREDENTIALS_V2_CONTEXT_URL, vrcIsVc20, or DI at all.

The mirroring helper itself is well-covered elsewhere (vrc-reference/__tests__/unit/mirroredProofOptions.test.ts, imported directly from @bifold/vrc-shared), and DI sign/verify round-trips are covered in isolation by diConformance.test.ts — but neither touches WitnessService.ts's two call sites (offer-side proof options, VWC-build data-model branch). Net: a real behavioral change to the core witness flow shipped with a test file that looks like coverage (parallel "VWC Building"/"Freshness Check" describe blocks) but wouldn't fail if this exact diff were wrong.

Suggestion: either add tests that call WitnessService's real methods directly for the mirroring/freshness/digest changes, or at minimum fix the now-incorrect 5-minute freshness assertion so it doesn't quietly assert stale behavior.

Finding 2 (minor): stale, self-contradicting comment

WitnessService.ts (added in the DI-verify commit) still has a comment reading "Witness ISSUANCE stays 2018 until the peer matrix is proven" — directly contradicted two commits later in the same PR when DI issuance via mirroring was added. Low functional risk (it's a comment), but likely to mislead the next reader into assuming issuance never emits a DI proof.

Everything else checked out clean: TlsManager/LocalityService diffs are mechanical type/lint fixes with no TLS-behavior change, index.ts's askar-nodejs import-order fix is a well-explained narrow correctness fix, and CredentialRegistry.ts (unchanged) doesn't need anything the new mirroring code doesn't already provide.

Non-blocking overall — the design itself is sound — but the test-coverage gap in particular seems worth closing before relying on this suite as a regression guard for the witnessed-exchange flow going forward.

@bmiller59

Copy link
Copy Markdown
Collaborator

app/src/services/attestation.ts: unrelated to hardware attestation — but the DRPC removal leaves a silent dead-end

Worth clarifying scope first: despite the name and directory, this file has nothing to do with hardware/biometric attestation (Secure Enclave, StrongBox, App Attest, useHardwareAttestation) — all of that lives in bifold/packages/core/src/modules/vrc/* and has already been covered in earlier review passes. This file is a separate, older subsystem inherited from the upstream BC Wallet fork lineage: AttestationMonitor auto-detects and auto-handles proof requests asking for a remote attestation credential from BC Government's Traction service, gated by hard-coded credDefIDs/invitation URLs per environment in app/src/constants.ts:14-30 (pointing at traction-acapy-*.apps.silver.devops.gov.bc.ca). It's live-wired, not dead code: registered in the DI container (app/container-imp.ts:111), started/stopped on every agent lifecycle transition (app/src/hooks/useBCAgentSetup.ts:63-69), and subscribed to by Connection.tsx/ProofRequest.tsx for loading/toast UI.

Finding (medium): notification filter and failure path are now out of sync

Diffed against main: this PR deliberately strips the entire remote-attestation fetch/generate flow (connectToAttestationAgent, fetchNonceForAttestation, generateAttestation, requestAttestation, plus the associated @bifold/react-native-attestation and react-native-device-info imports) — a reasonable simplification, with a comment explaining there's no stable @credo-ts/drpc for Credo 0.6 and that Keyring doesn't run the BC service. requestAttestationCredential (attestation.ts:188-204) now unconditionally logs a warning and fails.

The problem: useNotifications (app/src/hooks/notifications.ts:110-120) still unconditionally filters proof requests matching isProofRequestingAttestation out of the notification list, regardless of whether they can be resolved. Combined with the now-guaranteed failure, any live proof request matching those credDefIDs becomes invisible to the user and permanently unresolvable — not shown as a notification, not auto-completed. The user only learns something went wrong if they happen to have Connection.tsx or ProofRequest.tsx mounted at the exact moment the FailedRequestCredential event fires; otherwise it's a silent dead end.

Whether this is actually reachable depends on whether Keyring still talks to real BC Government/Traction verifiers gated on these specific credDefIds — the repo still carries BC "Person credential" flow code (keyring-theme/features/person-flow, BCIDHelper.ts, CRED_HELP_ACTION_OVERRIDES in app/container-imp.ts), so this isn't provably dead, just unclear.

Suggested resolution — pick one:

  1. If Keyring still needs this flow, restore a real handler (or an explicit "not supported, tell the user" UI) rather than a silent failure.
  2. If Keyring no longer talks to BC's Traction instance at all, remove AttestationMonitor/AttestationRestrictions and the notification-hiding filter wholesale, rather than leaving a stubbed, dead-ended version wired live in the container.

Everything else checked out clean

The Credo 0.6/@credo-ts/didcomm migration in this file itself is mechanically solid: agent.proofs.*agent.modules.didcomm.proofs.*, proofRecordId/credentialRecordIdproofExchangeRecordId/credentialExchangeRecordId, ProofState/CredentialStateDidCommProofState/DidCommCredentialState, event payload key rename — all consistent with the same pattern used elsewhere (app/src/utils/credentials.ts). Lint is clean, no leftover unused imports from the stripped code path. Two low-priority items not worth a blocking discussion: six of nine AttestationErrorCodes are now unused dead code left over from the removed flow, and a pre-existing (not introduced by this PR) theoretical stuck-spinner edge case if handleProofRequest ever returns false with an undefined agent — practically unreachable today, just worth a follow-up ticket if anyone's doing cleanup in this area.

Non-blocking, but the notification/failure mismatch in particular seems worth a deliberate decision rather than leaving as-is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment