fix: update key retrieval methods to return nullable types on failure

[sergei-boiko-trustwallet]

This pull request improves the robustness of the HD wallet API by updating several methods to return nullable values when key or address derivation fails (such as when the mnemonic entropy is empty). The changes also update documentation to clarify these behaviors and adjust Swift test and example code to handle the possibility of nil returns. Additionally, the Cardano key derivation logic is hardened against invalid entropy.

API changes for error handling and nullability:

• Updated several HD wallet API methods in TWHDWallet.h to return nullable pointers (e.g., TWPrivateKey* _Nullable, TWString* _Nullable) and clarified documentation to indicate that null is returned if derivation fails, such as with empty mnemonic entropy.
• Modified the C++ and C interface implementations to use try-catch blocks and return nullptr on failure for key and address derivation methods.

Cardano derivation safety:

• Added a check in Cardano master node derivation to throw an exception if the mnemonic entropy is empty, preventing the generation of constant, insecure secrets.

Swift example and test code updates:

• Updated Swift example and test code to force unwrap results from key and address derivation methods, reflecting the new nullable return types and ensuring test correctness.

[octane-security-app]

Summary by Octane

New Contracts

No new contracts were added.

Updated Contracts

• HDWallet.cpp: Added a validation to prevent Cardano key derivation if mnemonic entropy is empty.

---

🔗 Commit Hash: c580cda

[github-actions]

⚠️ Backward-compatibility check needed

This PR touches persistence-sensitive files: include/TrustWalletCore/TWHDWallet.h, src/HDWallet.cpp.

Post a comment with one of:

• [bc-check: Pass] — audit run, Verdict: SAFE. Must include audit output.
• [bc-check: Mitigated] — audit found RISK or BLOCKER; you fixed it in code. Must include the post-fix audit output.
• [bc-check: Risk-Accepted] — audit found RISK or BLOCKER; investigation confirms blast radius is effectively zero (no user data in the wild can trigger it). Must include audit output + explicit evidence (who confirmed it, what data or reasoning).
• [bc-check: N/A] — scanner fired on a file with zero BC relevance (comment edit, test fixture, renamed variable). Invalid if the audit found a real risk.

Each token must be accompanied by >=60 chars of reasoning. The reasoning must be fresh — posted or edited at or after the HEAD commit.

Why audit evidence is required for Pass / Mitigated: humans don't reliably ask the right BC question on every PR. AI being in the loop is the whole point of this gate. The bot does not judge whether your reasoning is correct — reviewers do, like any other code review.

Things worth thinking about: could a previous version have written data this PR's new check would now reject? Was there a partial migration ("regenerate on next user action") that may not have completed for all users? Does this format live in iCloud / Google Drive backup, exported files, or sync payloads? See docs/bc-footguns.md for known cases.

Changed files (2)

• include/TrustWalletCore/TWHDWallet.h
• src/HDWallet.cpp

Copy this audit prompt into Claude Code on this branch (or run the /bc-check skill) for a structured analysis you can paste into your sign-off

You are auditing a Trust Wallet Core PR against origin/master
for **backward-compatibility risk against persisted user data and wire formats**.
Find cases where this PR will reject, mis-parse, or mishandle inputs that older
versions of our software already wrote to disk, to backups, or onto the network.

**Trust Wallet Core domain context:**
- src/Keystore/ — JSON keystore files (user encrypted keys/mnemonics).
  These live in iCloud, Google Drive, and manual exports.
  A parse failure = wallet inaccessible = user cannot access funds.
- src/proto/*.proto — Protobuf SigningInput/SigningOutput wire formats.
  Field numbers are permanent; reuse or removal silently corrupts binary data.
- include/TrustWalletCore/TW*.h — Public C ABI.
  Removing/renaming TW* functions or changing enum values breaks compiled bindings.
- registry.json — coin metadata (SLIP44, derivation path, curve, address encoding).
  Changing any of these re-derives different addresses for all existing wallets.

Do NOT trust the PR description's framing.
'Just a security fix' / 'stricter validation' is exactly the framing that hides this class of bug.
Read docs/bc-footguns.md first.

Walk these 5 steps. Cite file:line and commit SHA everywhere:

1. Classify: tightening validation? Parsing change? Exception type change?
   Removing 'if missing use default'? Moving validation into a constructor?
2. Historical baseline. For each tightened rule:
   - Could a prior version have PRODUCED data that violates the new rule? (cite SHA)
   - Was there a partial migration that may not have completed for all users?
   - Does the format ever leave the device (backup, export, sync)?
3. Concrete failure scenarios: old version -> action -> where stored -> code path
   (file:line) -> user symptom -> blast radius. No hand-waving.
4. Red-flag checklist (yes/no + file:line evidence):
   - New throw on a read/load/decode/import path?
   - Removed an 'if missing use default' branch?
   - New length/range/enum check on a >1-year-old field?
   - Constructor changed from lenient parse to parse+validate?
   - Changed which exception type a public API throws?
   - Format ever in backup/export/sync?
   - Prior PR shipped a 'regenerate on next user action' partial migration? (cite SHA)
   - Proto: field number reused or removed? Enum value renumbered/removed?
   - Keystore: JSON key renamed/removed/made required without default fallback?
   - Registry: slip44, curve, or address-encoding field changed?
5. Mitigations: accept legacy at read + normalize on write (preferred);
   gate strict check behind 'newly created' flag; one-time migration with clear UX;
   or apply tightening to write paths only.

Output a markdown report: Verdict (SAFE/RISK/BLOCKER) at top, then steps 1-5,
then a 'Suggested PR comment' block ([bc-check: Pass|Mitigated|N/A] + reasoning),
then a 'Suggested addition to docs/bc-footguns.md' block (or 'none').

Merge is blocked by verify-bc-check-comment until a tagged comment is posted.

[github-actions]

Binary size comparison

➡️ aarch64-apple-ios: 14.31 MB

➡️ aarch64-apple-ios-sim: 14.31 MB

➡️ aarch64-linux-android: 18.73 MB

➡️ armv7-linux-androideabi: 16.17 MB

➡️ wasm32-unknown-emscripten: 13.66 MB

[octane-security-app]

Overview

Warnings found:	1

Warnings

src/HDWallet.cpp

• Unhandled exception across C/Swift FFI in HDWallet extended-key getters for Cardano with empty mnemonic entropy causes process crash (DoS). See more

---

🔗 Commit Hash: c580cda
🛡️ Octane Dashboard: All vulnerabilities

[sonarqubecloud]

Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 80%)
MAJOR Code Smells Severity on New Code (required < MAJOR)

See analysis details on SonarQube Cloud