change partial txn serialization format for imported addresses txins

offline signing with segwit WIF keys now works.
offline seed + online address signing now works.

gui/qt/transaction_dialog.py

@@ -72,7 +72,7 @@ class TxDialog(QDialog, MessageBoxMixin):
         # Take a copy; it might get updated in the main window by
         # e.g. the FX plugin.  If this happens during or after a long
         # sign operation the signatures are lost.
-        self.tx = copy.deepcopy(tx)
+        self.tx = tx = copy.deepcopy(tx)
         try:
             self.tx.deserialize()
         except BaseException as e:
@@ -83,6 +83,11 @@ class TxDialog(QDialog, MessageBoxMixin):
         self.saved = False
         self.desc = desc
 
+        # if the wallet can populate the inputs with more info, do it now.
+        # as a result, e.g. we might learn an imported address tx is segwit,
+        # in which case it's ok to display txid
+        self.wallet.add_input_info_to_all_inputs(tx)
+
         self.setMinimumWidth(950)
         self.setWindowTitle(_("Transaction"))
 

lib/coinchooser.py

@@ -99,7 +99,7 @@ class CoinChooserBase(PrintError):
             buckets[key].append(coin)
 
         def make_Bucket(desc, coins):
-            witness = any(Transaction.is_segwit_input(coin) for coin in coins)
+            witness = any(Transaction.is_segwit_input(coin, guess_for_address=True) for coin in coins)
             # note that we're guessing whether the tx uses segwit based
             # on this single bucket
             weight = sum(Transaction.estimated_input_weight(coin, witness)

lib/tests/test_wallet_vertical.py

@@ -1112,14 +1112,12 @@ class TestWalletOfflineSigning(TestCaseForTestnet):
         self.assertEqual(tx.txid(), tx_copy.txid())
 
         # sign tx
-        #wallet_offline.can_sign(tx_copy)  # FIXME
         tx = wallet_offline.sign_transaction(tx_copy, password=None)
         self.assertTrue(tx.is_complete())
         self.assertFalse(tx.is_segwit())
         self.assertEqual('e56da664631b8c666c6df38ec80c954c4ac3c4f56f040faf0070e4681e937fc4', tx.txid())
         self.assertEqual('e56da664631b8c666c6df38ec80c954c4ac3c4f56f040faf0070e4681e937fc4', tx.wtxid())
 
-    @unittest.skip("not implemented yet")
     @needs_test_with_all_ecc_implementations
     @mock.patch.object(storage.WalletStorage, '_write')
     def test_sending_offline_wif_online_addr_p2wpkh_p2sh(self, mock_write):
@@ -1145,17 +1143,15 @@ class TestWalletOfflineSigning(TestCaseForTestnet):
         tx_copy = Transaction(tx.serialize())
         self.assertTrue(wallet_online.is_mine(wallet_online.get_txin_address(tx_copy.inputs()[0])))
 
-        #self.assertEqual(tx.txid(), tx_copy.txid())  # FIXME
+        self.assertEqual(tx.txid(), tx_copy.txid())
 
         # sign tx
-        wallet_offline.can_sign(tx_copy)  # FIXME
         tx = wallet_offline.sign_transaction(tx_copy, password=None)
         self.assertTrue(tx.is_complete())
         self.assertTrue(tx.is_segwit())
         self.assertEqual('7642816d051aa3b333b6564bb6e44fe3a5885bfe7db9860dfbc9973a5c9a6562', tx.txid())
         self.assertEqual('9bb9949974954613945756c48ca5525cd5cba1b667ccb10c7a53e1ed076a1117', tx.wtxid())
 
-    @unittest.skip("not implemented yet")
     @needs_test_with_all_ecc_implementations
     @mock.patch.object(storage.WalletStorage, '_write')
     def test_sending_offline_wif_online_addr_p2wpkh(self, mock_write):
@@ -1181,17 +1177,15 @@ class TestWalletOfflineSigning(TestCaseForTestnet):
         tx_copy = Transaction(tx.serialize())
         self.assertTrue(wallet_online.is_mine(wallet_online.get_txin_address(tx_copy.inputs()[0])))
 
-        #self.assertEqual(tx.txid(), tx_copy.txid())  # FIXME
+        self.assertEqual(tx.txid(), tx_copy.txid())
 
         # sign tx
-        wallet_offline.can_sign(tx_copy)  # FIXME
         tx = wallet_offline.sign_transaction(tx_copy, password=None)
         self.assertTrue(tx.is_complete())
         self.assertTrue(tx.is_segwit())
         self.assertEqual('f8039bd85279f2b5698f15d47f2e338d067d09af391bd8a19467aa94d03f280c', tx.txid())
         self.assertEqual('3b7cc3c3352bbb43ddc086487ac696e09f2863c3d9e8636721851b8008a83ffa', tx.wtxid())
 
-    @unittest.skip("not implemented yet")
     @needs_test_with_all_ecc_implementations
     @mock.patch.object(storage.WalletStorage, '_write')
     def test_sending_offline_xprv_online_addr_p2pkh(self, mock_write):  # compressed pubkey
@@ -1223,14 +1217,12 @@ class TestWalletOfflineSigning(TestCaseForTestnet):
         self.assertEqual(tx.txid(), tx_copy.txid())
 
         # sign tx
-        wallet_offline.can_sign(tx_copy)  # FIXME
         tx = wallet_offline.sign_transaction(tx_copy, password=None)
         self.assertTrue(tx.is_complete())
         self.assertFalse(tx.is_segwit())
         self.assertEqual('e56da664631b8c666c6df38ec80c954c4ac3c4f56f040faf0070e4681e937fc4', tx.txid())
         self.assertEqual('e56da664631b8c666c6df38ec80c954c4ac3c4f56f040faf0070e4681e937fc4', tx.wtxid())
 
-    @unittest.skip("not implemented yet")
     @needs_test_with_all_ecc_implementations
     @mock.patch.object(storage.WalletStorage, '_write')
     def test_sending_offline_xprv_online_addr_p2wpkh_p2sh(self, mock_write):
@@ -1259,17 +1251,15 @@ class TestWalletOfflineSigning(TestCaseForTestnet):
         tx_copy = Transaction(tx.serialize())
         self.assertTrue(wallet_online.is_mine(wallet_online.get_txin_address(tx_copy.inputs()[0])))
 
-        #self.assertEqual(tx.txid(), tx_copy.txid())  # FIXME
+        self.assertEqual(tx.txid(), tx_copy.txid())
 
         # sign tx
-        wallet_offline.can_sign(tx_copy)  # FIXME
         tx = wallet_offline.sign_transaction(tx_copy, password=None)
         self.assertTrue(tx.is_complete())
         self.assertTrue(tx.is_segwit())
         self.assertEqual('7642816d051aa3b333b6564bb6e44fe3a5885bfe7db9860dfbc9973a5c9a6562', tx.txid())
         self.assertEqual('9bb9949974954613945756c48ca5525cd5cba1b667ccb10c7a53e1ed076a1117', tx.wtxid())
 
-    @unittest.skip("not implemented yet")
     @needs_test_with_all_ecc_implementations
     @mock.patch.object(storage.WalletStorage, '_write')
     def test_sending_offline_xprv_online_addr_p2wpkh(self, mock_write):
@@ -1298,10 +1288,9 @@ class TestWalletOfflineSigning(TestCaseForTestnet):
         tx_copy = Transaction(tx.serialize())
         self.assertTrue(wallet_online.is_mine(wallet_online.get_txin_address(tx_copy.inputs()[0])))
 
-        #self.assertEqual(tx.txid(), tx_copy.txid())  # FIXME
+        self.assertEqual(tx.txid(), tx_copy.txid())
 
         # sign tx
-        wallet_offline.can_sign(tx_copy)  # FIXME
         tx = wallet_offline.sign_transaction(tx_copy, password=None)
         self.assertTrue(tx.is_complete())
         self.assertTrue(tx.is_segwit())

lib/transaction.py

@@ -385,6 +385,19 @@ def parse_scriptSig(d, _bytes):
         d['address'] = hash160_to_p2sh(hash_160(bfh(redeem_script)))
         return
 
+    # custom partial format for imported addresses
+    match = [ opcodes.OP_INVALIDOPCODE, opcodes.OP_0, opcodes.OP_PUSHDATA4 ]
+    if match_decoded(decoded, match):
+        x_pubkey = bh2u(decoded[2][1])
+        pubkey, address = xpubkey_to_address(x_pubkey)
+        d['type'] = 'address'
+        d['address'] = address
+        d['num_sig'] = 1
+        d['x_pubkeys'] = [x_pubkey]
+        d['pubkeys'] = None  # get_sorted_pubkeys will populate this
+        d['signatures'] = [None]
+        return
+
     print_error("parse_scriptSig: cannot find address in input script (unknown)",
                 bh2u(_bytes))
 
@@ -499,6 +512,8 @@ def parse_witness(vds, txin, full_parse: bool):
             raise UnknownTxinType()
         if txin['type'] == 'coinbase':
             pass
+        elif txin['type'] == 'address':
+            pass
         elif txin['type'] == 'p2wsh-p2sh' or n > 2:
             witness_script_unsanitized = w[-1]  # for partial multisig txn, this has x_pubkeys
             try:
@@ -784,21 +799,25 @@ class Transaction:
 
     @classmethod
     def serialize_witness(self, txin, estimate_size=False):
-        if not self.is_segwit_input(txin):
+        _type = txin['type']
+        if not self.is_segwit_input(txin) and not self.is_input_value_needed(txin):
             return '00'
-        if txin['type'] == 'coinbase':
+        if _type == 'coinbase':
             return txin['witness']
 
         witness = txin.get('witness', None)
-        if witness is None:
+        if witness is None or estimate_size:
+            if _type == 'address' and estimate_size:
+                _type = self.guess_txintype_from_address(txin['address'])
             pubkeys, sig_list = self.get_siglist(txin, estimate_size)
-            if txin['type'] in ['p2wpkh', 'p2wpkh-p2sh']:
+            if _type in ['p2wpkh', 'p2wpkh-p2sh']:
                 witness = construct_witness([sig_list[0], pubkeys[0]])
-            elif txin['type'] in ['p2wsh', 'p2wsh-p2sh']:
+            elif _type in ['p2wsh', 'p2wsh-p2sh']:
                 witness_script = multisig_script(pubkeys, txin['num_sig'])
                 witness = construct_witness([0] + sig_list + [witness_script])
             else:
-                raise Exception('wrong txin type:', txin['type'])
+                witness = txin.get('witness', '00')
+
         if self.is_txin_complete(txin) or estimate_size:
             partial_format_witness_prefix = ''
         else:
@@ -808,14 +827,32 @@ class Transaction:
         return partial_format_witness_prefix + witness
 
     @classmethod
-    def is_segwit_input(cls, txin):
+    def is_segwit_input(cls, txin, guess_for_address=False):
+        _type = txin['type']
+        if _type == 'address' and guess_for_address:
+            _type = cls.guess_txintype_from_address(txin['address'])
         has_nonzero_witness = txin.get('witness', '00') not in ('00', None)
-        return cls.is_segwit_inputtype(txin['type']) or has_nonzero_witness
+        return cls.is_segwit_inputtype(_type) or has_nonzero_witness
 
     @classmethod
     def is_segwit_inputtype(cls, txin_type):
         return txin_type in ('p2wpkh', 'p2wpkh-p2sh', 'p2wsh', 'p2wsh-p2sh')
 
+    @classmethod
+    def is_input_value_needed(cls, txin):
+        return cls.is_segwit_input(txin) or txin['type'] == 'address'
+
+    @classmethod
+    def guess_txintype_from_address(cls, addr):
+        witver, witprog = segwit_addr.decode(constants.net.SEGWIT_HRP, addr)
+        if witprog is not None:
+            return 'p2wpkh'
+        addrtype, hash_160 = b58_address_to_hash160(addr)
+        if addrtype == constants.net.ADDRTYPE_P2PKH:
+            return 'p2pkh'
+        elif addrtype == constants.net.ADDRTYPE_P2SH:
+            return 'p2wpkh-p2sh'
+
     @classmethod
     def input_script(self, txin, estimate_size=False):
         _type = txin['type']
@@ -832,6 +869,8 @@ class Transaction:
 
         pubkeys, sig_list = self.get_siglist(txin, estimate_size)
         script = ''.join(push_script(x) for x in sig_list)
+        if _type == 'address' and estimate_size:
+            _type = self.guess_txintype_from_address(txin['address'])
         if _type == 'p2pk':
             pass
         elif _type == 'p2sh':
@@ -855,7 +894,7 @@ class Transaction:
             scriptSig = bitcoin.p2wsh_nested_script(witness_script)
             return push_script(scriptSig)
         elif _type == 'address':
-            script += push_script(pubkeys[0])
+            return 'ff00' + push_script(pubkeys[0])  # fd extended pubkey
         elif _type == 'unknown':
             return txin['scriptSig']
         return script
@@ -956,10 +995,10 @@ class Transaction:
             preimage = nVersion + txins + txouts + nLocktime + nHashType
         return preimage
 
-    def is_segwit(self):
+    def is_segwit(self, guess_for_address=False):
         if not self.is_partial_originally:
             return self._segwit_ser
-        return any(self.is_segwit_input(x) for x in self.inputs())
+        return any(self.is_segwit_input(x, guess_for_address=guess_for_address) for x in self.inputs())
 
     def serialize(self, estimate_size=False, witness=True):
         network_ser = self.serialize_to_network(estimate_size, witness)
@@ -978,7 +1017,11 @@ class Transaction:
         outputs = self.outputs()
         txins = var_int(len(inputs)) + ''.join(self.serialize_input(txin, self.input_script(txin, estimate_size)) for txin in inputs)
         txouts = var_int(len(outputs)) + ''.join(self.serialize_output(o) for o in outputs)
-        if witness and self.is_segwit():
+        use_segwit_ser_for_estimate_size = estimate_size and self.is_segwit(guess_for_address=True)
+        use_segwit_ser_for_actual_use = not estimate_size and \
+                                        (self.is_segwit() or any(txin['type'] == 'address' for txin in inputs))
+        use_segwit_ser = use_segwit_ser_for_estimate_size or use_segwit_ser_for_actual_use
+        if witness and use_segwit_ser:
             marker = '00'
             flag = '01'
             witness = ''.join(self.serialize_witness(x, estimate_size) for x in inputs)
@@ -1038,8 +1081,7 @@ class Transaction:
         script = cls.input_script(txin, True)
         input_size = len(cls.serialize_input(txin, script)) // 2
 
-        if cls.is_segwit_input(txin):
+        if cls.is_segwit_input(txin, guess_for_address=True):
-            assert is_segwit_tx
             witness_size = len(cls.serialize_witness(txin, True)) // 2
         else:
             witness_size = 1 if is_segwit_tx else 0
@@ -1063,10 +1105,10 @@ class Transaction:
 
     def estimated_witness_size(self):
         """Return an estimate of witness size in bytes."""
-        if not self.is_segwit():
+        estimate = not self.is_complete()
+        if not self.is_segwit(guess_for_address=estimate):
             return 0
         inputs = self.inputs()
-        estimate = not self.is_complete()
         witness = ''.join(self.serialize_witness(x, estimate) for x in inputs)
         witness_size = len(witness) // 2 + 2  # include marker and flag
         return witness_size

lib/wallet.py

@@ -1440,21 +1440,32 @@ class Abstract_Wallet(PrintError):
         # note: no need to call tx.BIP_LI01_sort() here - single input/output
         return Transaction.from_io(inputs, outputs, locktime=locktime)
 
+    def add_input_sig_info(self, txin, address):
+        raise NotImplementedError()  # implemented by subclasses
+
     def add_input_info(self, txin):
         address = txin['address']
         if self.is_mine(address):
             txin['type'] = self.get_txin_type(address)
             # segwit needs value to sign
-            if txin.get('value') is None and Transaction.is_segwit_input(txin):
+            if txin.get('value') is None and Transaction.is_input_value_needed(txin):
                 received, spent = self.get_addr_io(address)
                 item = received.get(txin['prevout_hash']+':%d'%txin['prevout_n'])
                 tx_height, value, is_cb = item
                 txin['value'] = value
             self.add_input_sig_info(txin, address)
 
+    def add_input_info_to_all_inputs(self, tx):
+        if tx.is_complete():
+            return
+        for txin in tx.inputs():
+            self.add_input_info(txin)
+
     def can_sign(self, tx):
         if tx.is_complete():
             return False
+        # add info to inputs if we can; otherwise we might return a false negative:
+        self.add_input_info_to_all_inputs(tx)  # though note that this is a side-effect
         for k in self.get_keystores():
             if k.can_sign(tx):
                 return True
@@ -1497,6 +1508,7 @@ class Abstract_Wallet(PrintError):
     def sign_transaction(self, tx, password):
         if self.is_watching_only():
             return
+        self.add_input_info_to_all_inputs(tx)
         # hardware wallets require extra info
         if any([(isinstance(k, Hardware_KeyStore) and k.can_sign(tx)) for k in self.get_keystores()]):
             self.add_hw_info(tx)
@@ -2315,7 +2327,13 @@ class Multisig_Wallet(Deterministic_Wallet):
         # they are sorted in transaction.get_sorted_pubkeys
         # pubkeys is set to None to signal that x_pubkeys are unsorted
         derivation = self.get_address_index(address)
-        txin['x_pubkeys'] = [k.get_xpubkey(*derivation) for k in self.get_keystores()]
+        x_pubkeys_expected = [k.get_xpubkey(*derivation) for k in self.get_keystores()]
+        x_pubkeys_actual = txin.get('x_pubkeys')
+        # if 'x_pubkeys' is already set correctly (ignoring order, as above), leave it.
+        # otherwise we might delete signatures
+        if x_pubkeys_actual and set(x_pubkeys_actual) == set(x_pubkeys_expected):
+            return
+        txin['x_pubkeys'] = x_pubkeys_expected
         txin['pubkeys'] = None
         # we need n place holders
         txin['signatures'] = [None] * self.n