Harden non-sensitive validation paths

[mswilkison]

Summary

This PR addresses the non-sensitive hardening/correctness findings from the security review. The changes are intentionally small and localized so they can be reviewed independently from the private operational/security-exposure fixes.

Findings Addressed

• Finding 4: pkg/bitcoin/transaction_builder.go

  • Issue: Electrum-provided UTXO data could reference an output index that does not exist in the fetched previous transaction, causing transaction assembly to panic.
  • Fix: Validate the output index before reading transaction.Outputs[...] and return a descriptive error instead.
  • Regression: Added coverage for an out-of-range UTXO output index.

• Finding 5: pkg/maintainer/spv/spv.go

  • Issue: SPV proof maintenance used an input-controlled previous-output index without checking the fetched transaction output count.
  • Fix: Return an error when the funding output index is outside the previous transaction outputs.
  • Regression: Added coverage for an out-of-range funding output index.

• Finding 6: pkg/tecdsa/retry/retry.go

  • Issue: Retry triplet eligibility counted the middle operator twice instead of counting the right operator, which could incorrectly exclude valid triplets.
  • Fix: Count the right-hand operator at index k.
  • Regression: Added a triplet eligibility test where the right operator's seat count controls the result.

• Finding 12: solidity/ecdsa/deploy/16_initialize_allowlist_weights.ts

  • Issue: The deployment script logged an ownership-transfer failure but still returned success.
  • Fix: Re-throw the failure so deployments cannot silently complete with incorrect ownership.

• Finding 13: solidity/ecdsa/contracts/WalletRegistryGovernance.sol

  • Issue: finalizeAuthorizationDecreaseDelayUpdate destructured the authorization tuple incorrectly and wrote the wrong value back as the change period.
  • Fix: Preserve the existing authorization decrease change period when updating the delay.
  • Regression: Added coverage proving the change period survives a delay update.

• Finding 14: pkg/tbtc/wallet.go

  • Issue: Wallet sync assumed an Electrum-returned transaction always has at least one input.
  • Fix: Return a descriptive error for zero-input transactions instead of indexing Inputs[0].
  • Regression: Added coverage for a returned transaction with no inputs.

• Finding 15: pkg/firewall/firewall.go

  • Issue: A positive recognition cache could accept a peer after application recognition was revoked.
  • Fix: Recheck positive recognition on each validation while keeping the negative cache for rate-limiting misses.
  • Regression: Updated the cache test to assert revoked peers are rejected.

• Finding 16: pkg/net/retransmission/retransmission.go

  • Issue: The retransmission deduplication map grew without any eviction.
  • Fix: Bound the message-ID cache with FIFO eviction.
  • Regression: Added coverage proving old retransmission entries are evicted.

• Finding 18: scripts/initialize.sh

  • Issue: The script built a command from arguments and executed it with eval.
  • Fix: Use a Bash argument array and quote path changes.

Operational Impact

The firewall change intentionally removes the positive recognition cache. That means recognized, non-allowlisted peers are rechecked against the configured applications on each validation instead of being accepted from a stale positive cache entry. This trades lower stale-admission risk for additional application recognition/RPC checks on reconnects. The negative cache still rate-limits repeated misses for the same peer key, but it does not replace network-level connection controls for many distinct peer keys; operators should continue to rely on network/firewall controls and monitor RPC load.

Validation

• go test ./pkg/bitcoin ./pkg/maintainer/spv ./pkg/tecdsa/retry ./pkg/tbtc ./pkg/firewall ./pkg/net/retransmission
• go test ./pkg/firewall
• go test ./pkg/tbtc -run TestEnsureWalletSyncedBetweenChains_TransactionWithoutInputs
• bash -n scripts/initialize.sh
• git diff --check
• npm ci --legacy-peer-deps in solidity/ecdsa
• npx prettier --check contracts/WalletRegistryGovernance.sol deploy/16_initialize_allowlist_weights.ts test/WalletRegistryGovernance.test.ts
• npx hardhat compile

Validation Note

A focused Hardhat test run for WalletRegistryGovernance compiled successfully but did not reach the test cases because the local hardhat-deploy fixture could not find a deployed ReimbursementPool record from the dependency package.

Follow-up issue filed for that fixture gap: #3950