NHK Mask Feature

[JanKuczma]

Summary by CodeRabbit

Release Notes

• Documentation

  • Updated integration guide with simplified account-key derivation workflow
  • Added security note regarding nullifier hiding key exposure and future masking improvements

• Refactor

  • Streamlined nullifier hiding key access API with simplified single-parameter method

[coderabbitai]

Walkthrough

This PR simplifies the NHK (nullifier hiding key) API across the Aztec state migration wallet layer by replacing the multi-parameter getMaskedNhk() and getNhkApp() methods with a single-argument getNhk() method. Changes span interface definitions, implementations, base wallet logic, tests, and documentation.

Changes

Cohort / File(s)	Summary
NHK API Refactoring ts/aztec-state-migration/wallet/migration-account.ts, ts/aztec-state-migration/wallet/migration-base-wallet.ts	Replaced getMaskedNhk(mask) and getNhkApp(contractAddress) with simplified getNhk() method in MigrationAccount interface and implementations (MigrationAccountWithSecretKey, SignerlessMigrationAccount). Removed computeAppNullifierHidingKey import and simplified method logic to return master NHK directly.
Documentation & Specification Updates docs/spec/mode-b-spec.md, docs/integration-guide.md, docs/security.md	Updated Mode B specification and integration guide to reflect new getNhk() API. Added security note about raw NHK exposure and future masking improvements planned.
E2E Test Updates e2e-tests/migration-mode-b.test.ts, e2e-tests/nft-migration-mode-b.test.ts, e2e-tests/token-migration-mode-b.test.ts	Updated all test files to call getNhk(oldUserManager.address) instead of getMaskedNhk() with multiple cross-address parameters.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• tests and some mac devcontainer fixes #41: Directly modifies the same NHK wallet APIs and e2e tests, replacing getMaskedNhk/getNhkApp with getNhk across the codebase.
• Integration guide #49: Updates docs/integration-guide.md with overlapping changes to the Mode B integration guide documentation.

Suggested reviewers

• DamianStraszak

Poem

🐰 The NHK API hops with grace,
From masked chaos to simpler space—
One method now, no hiding games,
Security notes call out the names.
A cleaner wallet, tests in line,
This refactor makes all aligned! ✨

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Title check	⚠️ Warning	The title 'NHK Mask Feature' is misleading—the PR actually removes masking functionality, replacing getMaskedNhk() with getNhk().	Update the title to accurately reflect the change, such as 'Simplify NHK access by removing masking' or 'Replace masked NHK API with direct NHK retrieval'.
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch jk-mask-feat

---

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

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@ts/aztec-state-migration/wallet/migration-account.ts`:
- Around line 92-95: getNhk currently returns the raw masterNullifierHidingKey
(from deriveKeys(getSecretKey())), which exposes a secret; replace this API so
the raw NHK never leaves the wallet boundary — e.g., remove/privatize getNhk and
add a wallet-side method (e.g., performWithNhk or deriveMigrationArtifact) that
calls deriveKeys(this.getSecretKey()), uses masterNullifierHidingKey only
internally to compute and return a safe derived artifact (HMAC/hash/blinded
token/commitment) required by the migration, or accepts a callback to operate on
the NHK inside the wallet and return only non-secret results; update callers to
use that new method instead of expecting the raw NHK from getNhk.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 3fc98d62-508c-4f9c-b125-855a82d2ffe3

📥 Commits

Reviewing files that changed from the base of the PR and between 42a96f1 and bba993a.

📒 Files selected for processing (8)

• docs/integration-guide.md
• docs/security.md
• docs/spec/mode-b-spec.md
• e2e-tests/migration-mode-b.test.ts
• e2e-tests/nft-migration-mode-b.test.ts
• e2e-tests/token-migration-mode-b.test.ts
• ts/aztec-state-migration/wallet/migration-account.ts
• ts/aztec-state-migration/wallet/migration-base-wallet.ts