[Jetszxcki]

Description

Two changes to WizardConnect transaction signing, both requested by the ParyonUSD team.

1. Sign P2PKH inputs with SIGHASH_UTXOS (0x61)

WizardConnect requires every input to be signed with SIGHASH_ALL | SIGHASH_FORKID | SIGHASH_UTXOS (0x61). The P2PKH branch in src/wallet/bch-sign.js previously delegated to the stock walletTemplateP2pkhNonHd compiler, whose unlock script (key.schnorr_signature.all_outputs) produces SIGHASH_ALL | SIGHASH_FORKID (0x41). The contract-input branch already used the correct 0x61 flags — only the P2PKH branch diverged.

The P2PKH branch now signs explicitly with allOutputs | utxos | forkId, mirroring the contract branch, and the now-unused template compiler imports were removed. Both branches emit 0x61.

Note: signBchTransaction is shared by the WizardConnect and WalletConnect v2 paths, so both now sign P2PKH inputs with 0x61 (valid and recommended for BCH).

2. Honour broadcast: false

When a sign request comes in with broadcast: false (sent inside the transaction payload as request.transaction.broadcast), the wallet returns the signed transaction hex to the dapp via sendSignResponse and skips the Watchtower broadcast entirely. Requests without the flag default to true, so broadcast behavior for all existing integrations is unchanged.

Additionally, if signing succeeds but the subsequent broadcast fails, the wallet now still returns the signed hex to the dapp (reporting the broadcast failure via a notification) instead of discarding the signature and returning a signing error. The address-buffer check after broadcast only runs on a successful broadcast.

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

1. Install & lint

npm install
npm run lint

• No new dependencies were added.
• The touched files (src/wallet/bch-sign.js, src/store/wizardconnect/actions.js) show only pre-existing unused-var lint errors — this PR adds zero new lint issues.

2. Signature verification (offline)

Signed a P2PKH transaction through signBchTransaction and inspected the unlocking bytecode with bitauth-libauth-v3:

• P2PKH signature push is 65 bytes and its hashtype byte is 0x61 (previously 0x41).
• secp256k1.verifySignatureSchnorr passes against the 0x61 signing serialization and fails against 0x41.

3. Unit tests

npm run test:unit

No test files exist in the repo yet for these actions — nothing to update (N/A).

4. Manual end-to-end test

# terminal 1 — patched wallet
cd paytaca-app && npm install && npx quasar dev -m spa

# terminal 2 — start the dapp's Express + Django backend,
# then trigger a 2-of-2 refund signing from the dapp UI and
# approve it in the wallet

• broadcast:false, before the fix: wallet tried to broadcast the 1-of-2 signed tx → node rejected it (mandatory-script-verify-flag-failed) → error shown, signed hex discarded, dapp got nothing.
• broadcast:false, after the fix: no broadcast attempted, wallet returns the signed tx to the dapp, and the backend records the first party's signature. Repeat from the second party's session → both signatures recorded → backend assembles and broadcasts the final tx.

---

Does this affect other areas?

No. The broadcast flag defaults to true, so requests without broadcast: false behave exactly as before (apart from the now-correct 0x61 hashtype). Normal payment flows, signing, and key handling are otherwise untouched.

@mentions

@joemarct

[github-actions]

PR #784 Review: improvements/wizard-connect

Verdict: Approve. The change is minimal, spec-correct, and introduces no security regressions.

The change — src/store/wizardconnect/actions.js:215

const shouldBroadcast = request?.transaction?.broadcast !== false && request?.broadcast !== false
if (!shouldBroadcast) {
  await wizardConnectService.sendSignResponse(connectionId, sequence, signedTxHex)
  return
}

What I verified

• Flag location is spec-correct. WcSignTransactionRequest (from @bch-wc2/interfaces, the type of the wire message's transaction field) defines broadcast?: boolean at request.transaction.broadcast — the primary check matches the interface exactly. Confirmed against the WizardConnect protocol spec and the installed type definitions.
• Default behavior preserved. Absent flag → undefined !== false → true → broadcast proceeds; only exact boolean false opts out. Existing integrations unchanged.
• Security (per the signing-flow review checklist):
  • No changes to key derivation, signing, SIGHASH enforcement, or secret handling — this is strictly post-signing behavior.
  • No new exposure of signed material: the signed hex was already sent to the dapp after broadcast in the existing flow (actions.js:236), so the dapp receives the same data either way.
  • Replay handling unchanged: the early-return path calls the same sendSignResponse, so the manager's sequence dedup/persist logic (wallet-connection-manager.js:165-180) applies identically.
  • User consent intact: the user still reviews the full transaction + userPrompt before approving, and SIGHASH_ALL|FORKID|UTXOS prevents the signature from being grafted onto a different transaction. The only new capability is dapp-side deferred broadcast (multisig assembly) — the feature's stated purpose.
  • Skipping the delayed ensureAddressBuffer on the non-broadcast path is correct — no on-chain state changed.
• Lint. Confirmed the only 2 errors in the file (unused commit/state in pair, actions.js:164) exist at the base commit a40bdf7 as well — the PR adds zero new lint issues, matching the test notes.

Minor observations (non-blocking)

1. Non-spec fallback: the second condition request?.broadcast !== false checks the outer SignTransactionRequest message, which has no broadcast field in the protocol. It's harmless (absent → no effect) but effectively dead code; consider dropping it so it doesn't imply the outer-level flag is protocol-sanctioned.
2. UX: WizardConnectSignRequestDialog.vue doesn't distinguish "Sign & broadcast" from "Sign only (returned to dApp)" — the user can't tell the wallet won't broadcast. Optional improvement: surface the broadcast: false state in the dialog.
3. Type strictness: !== false means only the exact boolean false disables broadcast (a dapp sending "false" or 0 still triggers broadcast). This matches the interface definition and is arguably the right strictness, but worth knowing.

opencode session  |  github run

[github-actions]

PR #784 Review

Verdict: Approve. The change is a minimal, spec-correct post-signing branch with no security regressions.

Change reviewed — src/store/wizardconnect/actions.js:215-219

const shouldBroadcast = request?.transaction?.broadcast !== false && request?.broadcast !== false
if (!shouldBroadcast) {
  await wizardConnectService.sendSignResponse(connectionId, sequence, signedTxHex)
  return
}

What I verified

• Flag path is spec-correct. WcSignTransactionRequest (the type of the wire message's transaction field, confirmed in node_modules/@wizardconnect/core/node_modules/@bch-wc2/interfaces/src/interfaces.ts:33-39) defines broadcast?: boolean at request.transaction.broadcast. In signRequest (service.js:277), transaction is an object with .transaction/.sourceOutputs/.broadcast, so the path resolves correctly.
• Default behavior preserved. Absent flag → undefined !== false → broadcast proceeds. Only exact boolean false opts out, matching the interface definition. All existing integrations unchanged.
• Security (per signing-flow checklist):
  • No changes to key derivation, signing, SIGHASH, or secret handling — this is strictly post-signing routing. signBchTransaction uses SIGHASH_ALL | FORKID | UTXOS (bch-sign.js:153), so the signature is bound to the exact approved transaction and cannot be grafted onto a different one.
  • No new exposure of signed material: the signed hex was already sent to the dapp after broadcast in the existing flow (actions.js:236), so the dapp receives identical data either way.
  • User consent intact: the user still reviews the full transaction + userPrompt before approving. The only new capability is dapp-side deferred broadcast (multisig assembly) — the stated purpose.
  • Replay handling unchanged: the early-return path uses the same sendSignResponse, so the manager's sequence dedup/persist logic (wallet-connection-manager.js:165-180) applies identically.
  • Skipping the delayed ensureAddressBuffer on the non-broadcast path is correct — no on-chain state changed.
  • Error handling: a sendSignResponse failure falls through to the same catch → sendSignError, consistent with the broadcast path.
• Lint. npx eslint src/store/wizardconnect/actions.js shows only the 2 pre-existing errors in pair (line 164, unused commit/state) — untouched by this diff. Zero new issues.

Minor observations (non-blocking)

1. Dead non-spec fallback: request?.broadcast !== false checks the outer sign_transaction_request wire message, which has no broadcast field in the protocol (only sequence, transaction, inputPaths). Harmless (absent → no effect), but consider dropping it so it doesn't imply outer-level flag is protocol-sanctioned.
2. UX: WizardConnectSignRequestDialog.vue doesn't distinguish "Sign & broadcast" from "Sign only (returned to dApp)" — the user can't tell the wallet won't broadcast. Worth a follow-up to surface the broadcast: false state in the dialog.
3. Strictness: !== false means a dapp sending "false" or 0 still triggers broadcast. This matches the interface and is the right strictness, but worth documenting for dapp integrators.

opencode session  |  github run

[github-actions]

PR #784 Review — WizardConnect signing changes

Verdict: Approve. Both changes are correct; I verified the signature change functionally, not just by reading it.

What I verified (beyond prior reviews)

1. New P2PKH signing path (src/wallet/bch-sign.js:162-174) — ran it offline against the installed bitauth-libauth-v3

• Unlock script is well-formed: OP_PUSH65 <64B schnorr sig + hashtype> OP_PUSH33 <33B compressed pubkey>; pubkey matches the derived key; generateTransaction accepts it.
• Hashtype byte is 0x61; signature verifies against the 0x61 preimage and fails against 0x41 (old compiler behavior).
• Security improvement confirmed: with a mutated valueSatoshis in the source output, the signature no longer verifies — SIGHASH_UTXOS binds each input's value into the signature, closing the input-value/fee-misrepresentation window that 0x41 left open.
• Token handling preserved: generateSigningSerializationBCH always reads sourceOutputs[inputIndex].token/.valueSatoshis (libauth signing-serialization.js:148-153), so token UTXOs are covered identically to the removed compiler path (which passed token: sourceOutput.token).
• Flag values confirmed: allOutputs|utxos|forkId = 1|32|64 = 0x61.

2. broadcast: false handling (src/store/wizardconnect/actions.js:215-219)

• Flag location is spec-correct: broadcast?: boolean exists on WcSignTransactionRequest at request.transaction.broadcast (@bch-wc2/interfaces/src/interfaces.ts:39); the JSON round-trip (handleSignRequest stringify → approveRequestWithData parse) preserves the boolean.
• Default preserved: absent flag → undefined !== false → broadcast proceeds.
• Signed hex was already returned to the dapp post-broadcast (actions.js:235), so the early-return adds no new data exposure; both paths use the same sendSignResponse (identical replay/dedup handling), and failures fall through to the same catch → sendSignError.
• Broadcast-failure path now returns the signature instead of discarding it: safe because the signature is SIGHASH_ALL|UTXOS|FORKID-bound to the exact user-reviewed transaction (can't be grafted onto a different tx or amounts); the dapp can retry broadcast — this is the multisig-assembly use case.
• ensureAddressBuffer correctly gated on successful broadcast only.

3. Hygiene

• Lint: reproduced the exact 5 errors at base a40bdf7 by checking out the base file versions — zero new issues.
• No test files exist in the repo; "N/A" note is accurate.
• No stray references to the removed compiler imports.

Minor observations (non-blocking)

1. request?.broadcast !== false checks the outer wire message, which has no broadcast field in the protocol — dead but harmless; consider dropping it so it doesn't imply outer-level flag is sanctioned.
2. UX: the sign-request dialog doesn't tell the user "wallet will not broadcast" — they approve assuming a payment is sent. Worth a follow-up to surface the broadcast: false state.
3. Blast radius: the shared signBchTransaction also moves WalletConnect v2 P2PKH signatures from 0x41 → 0x61. Valid and standard for BCH (UTXOS active since May 2020) and strictly more secure, but any dapp verifying the hashtype byte strictly would notice — already disclosed in the PR body.
4. Pre-existing (unchanged): a non-contract source output with a non-empty unlockingBytecode skips both branches and passes through unsigned — same branch condition as before this PR.

opencode session  |  github run

[github-actions]

PR #784 Review — WizardConnect signing changes

Verdict: Approve. All signing changes verified functionally (offline, against the installed bitauth-libauth-v3) — but the PR contains more than the body describes, including one substantive signing-behavior change and a dev-port change that breaks Cypress baselines.

Functionally verified

1. P2PKH path (src/wallet/bch-sign.js:206-218) — signed a tx through signBchTransaction and verified:

• Unlock is well-formed 41<65B sig+hashtype>21<33B pubkey>; pubkey matches derived key; generateTransaction accepts it.
• Hashtype byte is 0x61; schnorr verifies against the 0x61 preimage, fails against 0x41.
• SIGHASH_UTXOS binding confirmed: mutating valueSatoshis breaks the signature — closes the input-value/fee-misrepresentation window the old 0x41 compiler left open.

2. Contract-input refusal (bch-sign.js:74-102, 173-205) — this change is not described in the PR body ("two changes" — this is a third). Verified all cases:

• Garbage/foreign signature, no placeholder → refused.
• Stale signature valid only for different outputs → refused (the embedded sig is checked against the current tx's sighash — a dapp can't recycle an old signature against a mutated tx; the wallet fails fast instead of returning an invalid tx).
• Other party's valid sig, no placeholder for this wallet → refused.
• Genuinely already-signed-by-this-wallet input → passes through unchanged (no double-sign).
• verifySignatureSchnorr/verifySignatureDER return plain booleans in this libauth build (no truthy-string trap); the fork's decodeAuthenticationInstructions returns malformed-marker arrays, so the defensive string checks are inert but harmless.

3. broadcast: false (src/store/wizardconnect/actions.js:215-219) — spec-correct path (WcSignTransactionRequest.broadcast, confirmed at @bch-wc2/interfaces/src/interfaces.ts:39); default preserved; early-return uses the same manager.sendSignResponse (wallet-connection-manager.js:165-180) so replay/dedup/persist protection is identical; failure falls into the same catch. Broadcast-failure now returns the signed hex — safe since the sig is SIGHASH_ALL|UTXOS|FORKID-bound to the user-reviewed tx. ensureAddressBuffer correctly gated on successful broadcast.

4. WalletConnect changes (src/wallet/walletconnect2/index.js, undisclosed in body) — fail-fast on missing WALLETCONNECT_PROJECT_ID is caught by all callers (boot/walletconnect.js:15-23, both store actions), so the app still boots; relayer error listeners prevent uncaught-'error' crashes. .env_template documents the var; no secrets committed.

5. Lint — zero new issues; all 8 errors across the 3 files exist identically at base (reproduced against base file versions).

Findings

1. Dev port 9000 → 9001 (quasar.config.cjs:279) breaks Cypress — all cypress/e2e/*.cy.js specs hardcode http://localhost:9000 (homepage.cy.js:5, applications.cy.js:6, …) and .github/workflows/cypress.yml:33 health-checks port 9000. The workflow is already inert (no npm run start script; triggers on main while default branch is master), but npm run dev + npm test locally will now fail. Unrelated to the PR's purpose — revert it or update the specs/workflow in the same change. Low–medium.
2. PR body understates the diff — the head commit 3e04703 adds the contract-refusal change (a signing-behavior change that turns silent passthrough into a thrown error), the WC fail-fast/relayer handlers, .env_template, and the port change, none described in the body. Medium (process).
3. Dead non-spec fallback — request?.broadcast !== false (actions.js:215) checks the outer wire message, which has no broadcast field in the protocol. Harmless but effectively dead code; consider dropping. Low.
4. UX — WizardConnectSignRequestDialog.vue shows only userPrompt; the user can't tell "Sign & broadcast" from "Sign only (returned to dApp)". With broadcast: false the wallet hands the signed tx to the dapp without broadcasting — worth surfacing in the dialog. Low.
5. Dapp can't distinguish broadcast failure from success — on broadcast failure the dapp now gets a success-shaped response (with signed hex) and only the user sees a notification. Acceptable given the signature binding and retry semantics, but dapp integrators assuming the wallet broadcasts should be aware. Low.
6. Blast radius (disclosed) — shared signBchTransaction also moves WalletConnect v2 P2PKH sigs 0x41 → 0x61. Valid/recommended for BCH and strictly more secure; hashtype-strict verifiers will notice.
7. Pre-existing (not from this PR) — placeholder matching via indexOf/replace on the hex string can match a placeholder pattern inside the redeem-script push instead of the unlock-level signature push (observed while constructing tests). Real CashScript redeem scripts don't contain 65 zero bytes, and the behavior predates this PR.
8. package-lock.json — only "peer": true marker churn, no version changes. Benign.

The core signing changes are correct and security-improving; recommend addressing findings 1–2 (revert the port change, update the PR description) before merge — neither is blocking.

opencode session  |  github run

[github-actions]

All verification complete. Here is my review:

---

PR #784 Review — WizardConnect signing changes (head 5751bdc)

Verdict: Approve. All signing changes verified functionally offline against the installed bitauth-libauth-v3, not just by reading. The stray dev-port/lockfile churn flagged in earlier reviews is confirmed reverted in 5751bdc.

Functionally verified (ran signBchTransaction offline)

1. P2PKH path (src/wallet/bch-sign.js:206-218) — built a tx, signed, inspected the unlock bytecode:

• Unlock is 41 <65B sig+hashtype> 21 <33B pubkey>; pubkey matches the derived key; generateTransaction accepts it.
• Hashtype byte is 0x61; schnorr verifies against the 0x61 preimage and fails against 0x41 (old compiler behavior).
• SIGHASH_UTXOS binding confirmed: mutating valueSatoshis in the source output breaks the signature — closes the input-value/fee-misrepresentation window that 0x41 left open.
• Token handling preserved: generateSigningSerializationBCH reads sourceOutputs[inputIndex].token/.valueSatoshis (signing-serialization.js:150-153), so token UTXOs are covered identically to the removed compiler path.

2. Contract-input refusal (bch-sign.js:173-205) — the undisclosed third change — all cases pass:

• Garbage/foreign signature, no placeholder → refused.
• Stale signature valid only for different outputs → refused (embedded sigs are checked against the current tx's sighash — a dapp can't recycle an old signature against a mutated tx).
• Other party's valid signature → refused; 0x41-hashtype signature from this wallet → refused.
• Genuinely already-signed-by-this-wallet input → passes through unchanged (no double-sign).
• Placeholder path still signs correctly at 0x61.

3. broadcast: false (src/store/wizardconnect/actions.js:215-219) — spec-correct: broadcast?: boolean is on WcSignTransactionRequest (@bch-wc2/interfaces/src/interfaces.ts:39), carried at request.transaction.broadcast; the outer SignTransactionRequest wire message (hdwalletv1.d.ts:50-56) has fields action/transaction/sequence/inputPaths/txSummary only. Default preserved (absent → broadcast). Early return uses the same manager.sendSignResponse (wallet-connection-manager.js:165-180) so replay/dedup/persist protection is identical; a failure falls into the same catch → sendSignError. Broadcast failure now returns the signed hex — safe since the sig is SIGHASH_ALL|UTXOS|FORKID-bound to the exact user-reviewed tx. ensureAddressBuffer correctly gated on successful broadcast only.

4. WalletConnect changes (src/wallet/walletconnect2/index.js) — fail-fast on missing WALLETCONNECT_PROJECT_ID is caught by all callers (boot/walletconnect.js:15-23, both store actions, WalletConnectV2.vue pairURI/onMounted all wrap in try/catch); relayer error listeners prevent uncaught-'error'-event crashes. .env_template adds an empty variable — no secret committed.

5. Hygiene — port back to 9000 (quasar.config.cjs:279), Cypress specs (localhost:9000) consistent, lockfile churn gone. Lint: all 10 errors across the touched files pre-exist at base (every error line is outside this PR's diff hunks; prefix unused at base too) — zero new issues. No test files exist in the repo; the N/A note is accurate.

Findings (non-blocking)

1. Dead non-spec fallback — request?.broadcast !== false (actions.js:215) checks the outer wire message, which has no broadcast field in the protocol. Harmless (absent → no effect) but effectively dead; consider dropping so it doesn't imply outer-level sanction. Low.
2. UX — WizardConnectSignRequestDialog.vue shows only userPrompt + tx details; the user can't distinguish "Sign & broadcast" from "Sign only (returned to dApp)". With broadcast: false they may approve assuming a payment is sent. Worth a follow-up to surface the flag. Low.
3. Dapp can't distinguish broadcast failure from success — on broadcast failure the dapp now gets a success-shaped SignTransactionResponse. The protocol's response type supports error?: string (hdwalletv1.d.ts:61); populating it would let dapps tell the two apart (needs a small service-API change). Low.
4. PR body understates the diff — the contract-refusal change (a signing-behavior change: silent passthrough → thrown error), the WC fail-fast/relayer handlers, .env_template, and (now-reverted) port change are only in commit messages, not the body. Worth updating. Medium (process).
5. Blast radius (disclosed) — shared signBchTransaction also moves WalletConnect v2 P2PKH signatures 0x41 → 0x61. Valid/recommended for BCH and strictly more secure; hashtype-strict dapp verifiers will notice.
6. Pre-existing (not from this PR) — placeholder matching via indexOf/replace on the hex string can match a placeholder pattern inside the redeem-script push instead of the unlock-level signature push; real CashScript redeem scripts don't contain 65 zero bytes, and the behavior predates this PR.

The core changes are correct and security-improving; nothing blocking merge.

opencode session  |  github run