fix: correct testnet P2SH address classification bug in bitcoin provider

.gitignore

@@ -1,4 +1,5 @@
 venv/
+.venv/
 allways-venv/
 .env
 *.env

allways/chain_providers/base.py

@@ -5,6 +5,10 @@
 from allways.chains import ChainDefinition
 
 
+class ProviderUnreachableError(Exception):
+    """Raised when a chain provider cannot reach its backend during verification."""
+
+
 @dataclass
 class TransactionInfo:
     tx_hash: str
@@ -41,10 +45,11 @@ def check_connection(self, **kwargs) -> None:
     def verify_transaction(
         self, tx_hash: str, expected_recipient: str, expected_amount: int, block_hint: int = 0
     ) -> Optional[TransactionInfo]:
-        """Verify a transaction. Uses >= for amount (overpayment is acceptable on-chain).
+        """Verify a transaction; returns TransactionInfo if found, None if not found,
+        raises ProviderUnreachableError on transient failures.
 
-        block_hint: If > 0, the block number where the tx is expected to be found.
-        Providers can use this for O(1) lookup instead of scanning.
+        Uses >= for amount (overpayment is acceptable on-chain).
+        block_hint: If > 0, providers can use this for O(1) lookup instead of scanning.
         """
         ...
 

allways/chain_providers/bitcoin.py

@@ -7,7 +7,7 @@
 import requests
 from bitcoin_message_tool.bmt import sign_message, verify_message
 
-from allways.chain_providers.base import ChainProvider, TransactionInfo
+from allways.chain_providers.base import ChainProvider, ProviderUnreachableError, TransactionInfo
 from allways.chains import CHAIN_BTC, ChainDefinition
 from allways.constants import BTC_TO_SAT
 
@@ -32,7 +32,9 @@ def detect_address_type(address: str) -> str:
         return ADDR_TYPE_P2WPKH
     if address.startswith('bcrt1p') or address.startswith('tb1p'):
         return ADDR_TYPE_P2TR
-    if address.startswith('2') or address.startswith('m') or address.startswith('n'):
+    if address.startswith('2'):
+        return ADDR_TYPE_P2SH_P2WPKH
+    if address.startswith('m') or address.startswith('n'):
         return ADDR_TYPE_P2PKH
     return 'unknown'
 
@@ -168,10 +170,8 @@ def _rpc_verify_transaction(
             if block_info:
                 block_number = block_info.get('height')
 
-        # Parse vout for matching output
         for vout in raw_tx.get('vout', []):
             addresses = vout.get('scriptPubKey', {}).get('addresses', [])
-            # Also check 'address' field (newer Bitcoin Core versions)
             if not addresses:
                 addr = vout.get('scriptPubKey', {}).get('address')
                 if addr:
@@ -180,23 +180,7 @@ def _rpc_verify_transaction(
             amount_sat = int(round(vout.get('value', 0) * BTC_TO_SAT))
 
             if expected_recipient in addresses and amount_sat >= expected_amount:
-                # Get sender from first vin
-                sender = ''
-                if raw_tx.get('vin'):
-                    vin = raw_tx['vin'][0]
-                    if 'txid' in vin:
-                        prev_tx = self._rpc_call('getrawtransaction', [vin['txid'], True])
-                        vout_idx = vin.get('vout', 0)
-                        if prev_tx and prev_tx.get('vout') and vout_idx < len(prev_tx['vout']):
-                            prev_vout = prev_tx['vout'][vout_idx]
-                            prev_addrs = prev_vout.get('scriptPubKey', {}).get('addresses', [])
-                            if not prev_addrs:
-                                prev_addr = prev_vout.get('scriptPubKey', {}).get('address')
-                                if prev_addr:
-                                    prev_addrs = [prev_addr]
-                            if prev_addrs:
-                                sender = prev_addrs[0]
-
+                sender = self._rpc_resolve_sender(raw_tx)
                 return TransactionInfo(
                     tx_hash=tx_hash,
                     confirmed=confirmed,
@@ -209,16 +193,50 @@ def _rpc_verify_transaction(
 
         return None
 
+    def _rpc_resolve_sender(self, raw_tx: dict) -> str:
+        """Extract sender address from the first vin of a raw transaction."""
+        if not raw_tx.get('vin'):
+            return ''
+        vin = raw_tx['vin'][0]
+        if 'txid' not in vin:
+            return ''
+        prev_tx = self._rpc_call('getrawtransaction', [vin['txid'], True])
+        if not prev_tx or not prev_tx.get('vout'):
+            return ''
+        vout_idx = vin.get('vout', 0)
+        if vout_idx >= len(prev_tx['vout']):
+            return ''
+        prev_vout = prev_tx['vout'][vout_idx]
+        prev_addrs = prev_vout.get('scriptPubKey', {}).get('addresses', [])
+        if not prev_addrs:
+            prev_addr = prev_vout.get('scriptPubKey', {}).get('address')
+            if prev_addr:
+                prev_addrs = [prev_addr]
+        return prev_addrs[0] if prev_addrs else ''
+
     # --- Blockstream API methods ---
     # Used as primary data source in lightweight mode, and as fallback in node mode.
 
+    def _blockstream_calc_confirmations(self, block_number: int) -> int:
+        """Fetch the chain tip from Blockstream and calculate confirmations for a block."""
+        try:
+            tip_resp = requests.get(f'{self._blockstream_api_url()}/blocks/tip/height', timeout=10)
+            if tip_resp.ok:
+                tip_height = int(tip_resp.text.strip())
+                return tip_height - block_number + 1
+        except Exception:
+            pass
+        return 0
+
     def _blockstream_verify_transaction(
         self, tx_hash: str, expected_recipient: str, expected_amount: int
     ) -> Optional[TransactionInfo]:
-        """Verify a Bitcoin transaction using Blockstream API."""
+        """Verify via Blockstream API; raises ProviderUnreachableError if unreachable."""
         try:
             url = f'{self._blockstream_api_url()}/tx/{tx_hash}'
             resp = requests.get(url, timeout=15)
+            if resp.status_code == 404:
+                return None
             resp.raise_for_status()
             data = resp.json()
 
@@ -227,11 +245,7 @@ def _blockstream_verify_transaction(
             confirmations = 0
 
             if confirmed and block_number:
-                # Get current tip to calculate confirmations
-                tip_resp = requests.get(f'{self._blockstream_api_url()}/blocks/tip/height', timeout=10)
-                if tip_resp.ok:
-                    tip_height = int(tip_resp.text.strip())
-                    confirmations = tip_height - block_number + 1
+                confirmations = self._blockstream_calc_confirmations(block_number)
 
             min_confs = self.get_chain().min_confirmations
             is_confirmed = confirmations >= min_confs if confirmed else False
@@ -256,6 +270,10 @@ def _blockstream_verify_transaction(
                     )
 
             return None
+        except (requests.ConnectionError, requests.Timeout) as e:
+            raise ProviderUnreachableError(f'Blockstream API unreachable: {e}') from e
+        except requests.HTTPError as e:
+            raise ProviderUnreachableError(f'Blockstream API error: {e}') from e
         except Exception as e:
             bt.logging.error(f'Blockstream tx lookup failed for {tx_hash}: {e}')
             return None
@@ -401,83 +419,24 @@ def send_amount_lightweight(
             pubkey = privkey.get_public_key()
             segwit_script = p2wpkh(pubkey)
 
-            # Derive address type: if from_address is known, match it directly.
-            # Otherwise probe all types to find where UTXOs exist.
             type_to_script = {
                 ADDR_TYPE_P2WPKH: ('p2wpkh', segwit_script, segwit_script.address(network)),
                 ADDR_TYPE_P2SH_P2WPKH: ('p2sh-p2wpkh', p2sh(segwit_script), p2sh(segwit_script).address(network)),
                 ADDR_TYPE_P2PKH: ('p2pkh', p2pkh(pubkey), p2pkh(pubkey).address(network)),
             }
 
-            if from_address:
-                detected = detect_address_type(from_address)
-                if detected in type_to_script:
-                    atype, script, addr = type_to_script[detected]
-                    if addr != from_address:
-                        bt.logging.error(
-                            f'WIF key derives {addr} but committed address is {from_address} — key mismatch'
-                        )
-                        return None
-                    utxo_url = f'{self._blockstream_api_url()}/address/{addr}/utxo'
-                    resp = requests.get(utxo_url, timeout=15)
-                    resp.raise_for_status()
-                    utxos = resp.json()
-                    my_script, my_address, addr_type = script, addr, atype
-                else:
-                    bt.logging.error(f'Unsupported address type for {from_address}: {detected}')
-                    return None
-            else:
-                # Probe all address types
-                candidates = [type_to_script[t] for t in (ADDR_TYPE_P2WPKH, ADDR_TYPE_P2SH_P2WPKH, ADDR_TYPE_P2PKH)]
-                my_script = None
-                my_address = None
-                utxos = None
-                addr_type = None
-                import time as _time
-
-                for idx, (atype, script, addr) in enumerate(candidates):
-                    try:
-                        if idx > 0:
-                            _time.sleep(1)  # avoid Blockstream rate limiting
-                        utxo_url = f'{self._blockstream_api_url()}/address/{addr}/utxo'
-                        resp = requests.get(utxo_url, timeout=15)
-                        resp.raise_for_status()
-                        candidate_utxos = resp.json()
-                        if candidate_utxos:
-                            my_script, my_address, utxos, addr_type = script, addr, candidate_utxos, atype
-                            bt.logging.debug(f'Found UTXOs on {atype} address: {addr}')
-                            break
-                    except Exception:
-                        continue
-
-            if not utxos:
-                bt.logging.error(f'No UTXOs found for {from_address or "any address type"}')
+            result = self._resolve_sender_utxos(from_address, type_to_script)
+            if result is None:
                 return None
+            my_script, my_address, utxos, addr_type = result
 
             is_segwit = addr_type in ('p2wpkh', 'p2sh-p2wpkh')
             bt.logging.info(f'Sending from {addr_type} address: {my_address}')
 
-            # Select UTXOs (simple greedy: accumulate until we cover amount + fee estimate)
-            fee_rate = self._estimate_fee_rate()
-            # vsize per input: ~68 for segwit, ~148 for legacy
-            input_vsize = 68 if is_segwit else 148
-            selected = []
-            total_in = 0
-            for utxo in sorted(utxos, key=lambda u: u['value'], reverse=True):
-                selected.append(utxo)
-                total_in += utxo['value']
-                est_vsize = 11 + len(selected) * input_vsize + 2 * 31
-                fee = est_vsize * fee_rate
-                if total_in >= amount + fee:
-                    break
-
-            # Final fee calculation
-            est_vsize = 11 + len(selected) * input_vsize + 2 * 31
-            fee = est_vsize * fee_rate
-            if total_in < amount + fee:
-                bt.logging.error(f'Insufficient funds: have {total_in} sat, need {amount} + {fee} fee')
+            coin_selection = self._select_utxos(utxos, amount, is_segwit)
+            if coin_selection is None:
                 return None
-
+            selected, total_in, fee = coin_selection
             change = total_in - amount - fee
 
             # Build transaction
@@ -530,21 +489,86 @@ def send_amount_lightweight(
                             bytes([len(sig_bytes)]) + sig_bytes + bytes([len(pub_bytes)]) + pub_bytes
                         )
 
-            # Broadcast
             raw_tx = final_tx.serialize().hex()
-            broadcast_url = f'{self._blockstream_api_url()}/tx'
-            broadcast_resp = requests.post(broadcast_url, data=raw_tx, timeout=15)
-            if broadcast_resp.status_code != 200:
-                bt.logging.error(f'Broadcast rejected ({broadcast_resp.status_code}): {broadcast_resp.text.strip()}')
+            tx_hash = self._broadcast_tx(raw_tx)
+            if tx_hash is None:
                 return None
-            tx_hash = broadcast_resp.text.strip()
 
             bt.logging.info(f'Sent {amount} sat to {to_address} via embit (tx: {tx_hash}, fee: {fee})')
             return (tx_hash, 0)
         except Exception as e:
             bt.logging.error(f'embit send failed: {e}')
             return None
 
+    def _resolve_sender_utxos(self, from_address, type_to_script):
+        """Match from_address to address type and fetch UTXOs, or probe all types."""
+        if from_address:
+            detected = detect_address_type(from_address)
+            if detected not in type_to_script:
+                bt.logging.error(f'Unsupported address type for {from_address}: {detected}')
+                return None
+            atype, script, addr = type_to_script[detected]
+            if addr != from_address:
+                bt.logging.error(f'WIF key derives {addr} but committed address is {from_address} — key mismatch')
+                return None
+            utxo_url = f'{self._blockstream_api_url()}/address/{addr}/utxo'
+            resp = requests.get(utxo_url, timeout=15)
+            resp.raise_for_status()
+            utxos = resp.json()
+            if not utxos:
+                bt.logging.error(f'No UTXOs found for {from_address}')
+                return None
+            return script, addr, utxos, atype
+
+        import time as _time
+
+        for idx, (atype, script, addr) in enumerate(type_to_script.values()):
+            try:
+                if idx > 0:
+                    _time.sleep(1)
+                utxo_url = f'{self._blockstream_api_url()}/address/{addr}/utxo'
+                resp = requests.get(utxo_url, timeout=15)
+                resp.raise_for_status()
+                candidate_utxos = resp.json()
+                if candidate_utxos:
+                    bt.logging.debug(f'Found UTXOs on {atype} address: {addr}')
+                    return script, addr, candidate_utxos, atype
+            except Exception:
+                continue
+
+        bt.logging.error('No UTXOs found for any address type')
+        return None
+
+    def _select_utxos(self, utxos, amount: int, is_segwit: bool):
+        """Greedy UTXO selection. Returns (selected, total_in, fee) or None."""
+        fee_rate = self._estimate_fee_rate()
+        input_vsize = 68 if is_segwit else 148
+        selected = []
+        total_in = 0
+        for utxo in sorted(utxos, key=lambda u: u['value'], reverse=True):
+            selected.append(utxo)
+            total_in += utxo['value']
+            est_vsize = 11 + len(selected) * input_vsize + 2 * 31
+            fee = est_vsize * fee_rate
+            if total_in >= amount + fee:
+                break
+
+        est_vsize = 11 + len(selected) * input_vsize + 2 * 31
+        fee = est_vsize * fee_rate
+        if total_in < amount + fee:
+            bt.logging.error(f'Insufficient funds: have {total_in} sat, need {amount} + {fee} fee')
+            return None
+        return selected, total_in, fee
+
+    def _broadcast_tx(self, raw_hex: str) -> Optional[str]:
+        """Broadcast a raw transaction via Blockstream. Returns tx_hash or None."""
+        url = f'{self._blockstream_api_url()}/tx'
+        resp = requests.post(url, data=raw_hex, timeout=15)
+        if resp.status_code != 200:
+            bt.logging.error(f'Broadcast rejected ({resp.status_code}): {resp.text.strip()}')
+            return None
+        return resp.text.strip()
+
     def _estimate_fee_rate(self) -> int:
         """Estimate fee rate (sat/vbyte) from Blockstream/mempool. Falls back to 5 sat/vb.