refactor(wizardconnect): use inputPaths for key derivation instead of address scanning

[dagurval]

Description

Describe your changes in detail. Why is this change required? What problem does it solve? Include link or references to relevant issues or tasks (eg. Bug ID - 1).

WizardConnect now requires dapp to provide what paths and indexes were used for signature.
This removes the need to scan a large range of addresses to find a matching locking code.
This also fixes the issue for one of the derivation paths where we did not know the max and capped at 30 addresses.

Acceptance criteria for the issue or task could also be included here.

Fixes # (issue)

Screenshots (if applicable):

(Include screenshots for UI-related changes)

Type of Change

• UI improvements with no business logic changes
• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Documentation update

Test Notes

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so others can reproduce (e.g. npm install for new dependencies, quasar dev, etc.). Include relevant details for automated or manual testing.

Will the applied changes affect other areas of the code? Will the applied changes affect functionalities of other features?

@mentions

Mention the person or team responsible for reviewing the proposed changes.

[joemarct]

PR Review: refactor(wizardconnect): use inputPaths for key derivation instead of address scanning

Files changed: 3 (package.json, package-lock.json, src/wallet/wizardconnect/service.js)
Net change: +23 / -98 (significant code removal)

---

Summary

This PR upgrades @wizardconnect/core (0.1.4 -> 0.1.6) and @wizardconnect/wallet (0.1.3 -> 0.1.5). The key behavioral change is that dApps now provide explicit inputPaths (tuples of [inputIndex, pathName, addressIndex]) in sign requests, so the wallet no longer needs to scan/derive a large range of addresses to find the matching key for each input.

This removes the buildAddressKeyMap function (~40 lines) which previously pre-derived keys across receive, change, and defi chains up to heuristic index limits.

---

Positives

1. Significant simplification. The address scanning approach was complex — it imported the Vuex store to get lastAddressIndex, iterated through multiple chains with lookahead, built two maps (address-based and pubkey-hash-based), and had separate matching logic for P2PKH vs contract inputs. All of that is replaced by a clean 6-line loop.

2. Fixes a real bug. The old code capped defi path scanning at ADDRESS_SCAN_MIN = 30, which was an arbitrary limit. If a user had > 30 defi addresses, signing would silently fail. The new approach has no such limit.

3. Removes unused imports. The PR cleanly removes encodeCashAddress, CashAddressType, binToHex, lockingBytecodeToCashAddress, sha256, ripemd160 — all no longer needed.

4. prefix is now unused in resolveKey. The getPrefix() call at line 189 is still there but prefix is no longer used in the new resolveKey path. It's only passed through to signBchTransaction, where it's used for P2PKH compilation (so it's still needed there). Fine as-is.

---

Issues and Concerns

1. No validation of request.inputPaths (Medium severity)

At service.js:193, the code destructures request.inputPaths without any validation:

for (const [inputIndex, pathName, addressIndex] of request.inputPaths) {
  const childIndex = PATH_NAME_TO_CHILD[pathName]

If request.inputPaths is undefined, null, or not iterable, this will throw an unhelpful error. If pathName is not one of receive, change, defi, then childIndex will be undefined, and getHdChainForIndex will silently default to nodes.hdMain (the receive chain) — which would derive the wrong key without any error.

Recommendation: Add defensive validation:

if (!request.inputPaths || !Array.isArray(request.inputPaths)) {
  throw new Error('Sign request missing inputPaths')
}

And validate each pathName:

const childIndex = PATH_NAME_TO_CHILD[pathName]
if (childIndex === undefined) {
  throw new Error(`Unknown path name: ${pathName}`)
}

2. Trust boundary shift — security consideration (Medium severity)

Previously, the wallet independently determined which key to use by matching locking bytecodes against its own derived addresses. Now the dApp tells the wallet which key to use for each input. This is a trust boundary shift worth noting.

If a malicious dApp sends crafted inputPaths pointing to keys the wallet controls but for addresses the user didn't intend, the wallet would sign with those keys. The old approach had an implicit safeguard: the wallet only signed if it independently verified the locking bytecode matched a known address.

Question for the author: Is there any verification in the new @wizardconnect/wallet library (0.1.5) that validates inputPaths against the actual transaction inputs? Or is this considered acceptable because the user reviews the transaction details in the approval dialog?

3. prefix variable is assigned but only used downstream (Nitpick)

const prefix = getPrefix() at line 189 is still needed (passed to signBchTransaction), but the comment block and the removal of buildAddressKeyMap makes it look like it might be dead code. No action needed, just noting for clarity.

---

package-lock.json

The lock file also removes @capacitor/core@8.2.0 and @capacitor/preferences@8.0.1 from the @walletconnect/keyvaluestorage dependency tree. This looks like a transitive dependency change unrelated to the wizardconnect upgrade — likely a side effect of re-resolving the lock file. Worth confirming this doesn't break anything on mobile.

---

Verdict

Approve with minor suggestions. The refactoring is clean and correct in the happy path. The main recommendation is adding input validation for request.inputPaths and pathName to fail explicitly rather than silently deriving wrong keys. The trust boundary question should be addressed by the author.

[dagurval]

We check for request.inputPath already, but not for pathName. Will follow up with a commit.

This is a valid consern but not a security issue because the wallet uses SIGHASH_ALL | SIGHASH_FORKID | SIGHASH_UTXO, followed up with documenting SIGHASH_ALL requirement here:
https://gitlab.com/riftenlabs/lib/wizardconnect/-/merge_requests/10

[joemarct]

We check for request.inputPath already, but not for pathName. Will follow up with a commit.

I proceeded to add the validation. Minor change anyway. We are wrapping up the next release.

[dagurval]

Thanks!