fix bip69_sort, nsequence

Author: ecdsa

electrum/coinchooser.py

@@ -225,14 +225,17 @@ def _change_outputs(self, tx: PartialTransaction, change_addrs, fee_estimator_nu
             c.is_change = True
         return change
 
-    def _construct_tx_from_selected_buckets(self, *, buckets: Sequence[Bucket],
-                                            base_tx: PartialTransaction, change_addrs,
-                                            fee_estimator_w, dust_threshold,
-                                            base_weight) -> Tuple[PartialTransaction, List[PartialTxOutput]]:
+    def _construct_tx_from_selected_buckets(
+            self, *, buckets: Sequence[Bucket],
+            base_tx: PartialTransaction, change_addrs,
+            fee_estimator_w, dust_threshold,
+            base_weight,
+            BIP69_sort: bool,
+    ) -> Tuple[PartialTransaction, List[PartialTxOutput]]:
         # make a copy of base_tx so it won't get mutated
-        tx = PartialTransaction.from_io(base_tx.inputs()[:], base_tx.outputs()[:])
+        tx = PartialTransaction.from_io(base_tx.inputs()[:], base_tx.outputs()[:], BIP69_sort=BIP69_sort)
 
-        tx.add_inputs([coin for b in buckets for coin in b.coins])
+        tx.add_inputs([coin for b in buckets for coin in b.coins], BIP69_sort=BIP69_sort)
         tx_weight = self._get_tx_weight(buckets, base_weight=base_weight)
 
         # change is sent back to sending address unless specified
@@ -246,7 +249,7 @@ def _construct_tx_from_selected_buckets(self, *, buckets: Sequence[Bucket],
         output_weight = 4 * Transaction.estimated_output_size_for_address(change_addrs[0])
         fee_estimator_numchange = lambda count: fee_estimator_w(tx_weight + count * output_weight)
         change = self._change_outputs(tx, change_addrs, fee_estimator_numchange, dust_threshold)
-        tx.add_outputs(change)
+        tx.add_outputs(change, BIP69_sort=BIP69_sort)
 
         return tx, change
 
@@ -296,7 +299,7 @@ def make_tx(
         self.p = PRNG(b''.join(sorted(utxos)))
 
         # Copy the outputs so when adding change we don't modify "outputs"
-        base_tx = PartialTransaction.from_io(inputs[:], outputs[:])
+        base_tx = PartialTransaction.from_io(inputs[:], outputs[:], BIP69_sort=BIP69_sort)
         input_value = base_tx.input_value()
 
         # Weight of the transaction with no inputs and no change
@@ -327,13 +330,15 @@ def sufficient_funds(buckets, *, bucket_value_sum):
             return total_input >= spent_amount + fee_estimator_w(total_weight)
 
         def tx_from_buckets(buckets):
-            return self._construct_tx_from_selected_buckets(buckets=buckets,
-                                                            base_tx=base_tx,
-                                                            change_addrs=change_addrs,
-                                                            fee_estimator_w=fee_estimator_w,
-                                                            dust_threshold=dust_threshold,
-                                                            base_weight=base_weight)
-
+            return self._construct_tx_from_selected_buckets(
+                buckets=buckets,
+                base_tx=base_tx,
+                change_addrs=change_addrs,
+                fee_estimator_w=fee_estimator_w,
+                dust_threshold=dust_threshold,
+                base_weight=base_weight,
+                BIP69_sort=BIP69_sort,
+            )
         # Collect the coins into buckets
         all_buckets = self.bucketize_coins(coins, fee_estimator_vb=fee_estimator_vb)
         # Filter some buckets out. Only keep those that have positive effective value.
@@ -349,8 +354,6 @@ def tx_from_buckets(buckets):
         self.logger.info(f"using {len(tx.inputs())} inputs")
         self.logger.info(f"using buckets: {[bucket.desc for bucket in scored_candidate.buckets]}")
 
-        if BIP69_sort:
-            tx.BIP69_sort()
         return tx
 
     def choose_buckets(self, buckets: List[Bucket],

electrum/transaction.py

@@ -2164,16 +2164,18 @@ def inputs(self) -> Sequence[PartialTxInput]:
     def outputs(self) -> Sequence[PartialTxOutput]:
         return self._outputs
 
-    def add_inputs(self, inputs: List[PartialTxInput]) -> None:
+    def add_inputs(self, inputs: List[PartialTxInput], BIP69_sort=True) -> None:
         self._inputs.extend(inputs)
-        self.BIP69_sort(outputs=False)
+        if BIP69_sort:
+            self.BIP69_sort(outputs=False)
         self.invalidate_ser_cache()
 
-    def add_outputs(self, outputs: List[PartialTxOutput], *, merge_duplicates: bool = False) -> None:
+    def add_outputs(self, outputs: List[PartialTxOutput], *, merge_duplicates: bool = False, BIP69_sort: bool = True) -> None:
         self._outputs.extend(outputs)
         if merge_duplicates:
             self._outputs = merge_duplicate_tx_outputs(self._outputs)
-        self.BIP69_sort(inputs=False)
+        if BIP69_sort:
+            self.BIP69_sort(inputs=False)
         self.invalidate_ser_cache()
 
     def set_rbf(self, rbf: bool) -> None:

electrum/wallet.py

@@ -1852,8 +1852,10 @@ def make_unsigned_transaction(
         if fee is None and self.config.fee_per_kb() is None:
             raise NoDynamicFeeEstimates()
 
-        for item in coins:
-            self.add_input_info(item)
+        for txin in coins:
+            self.add_input_info(txin)
+            nSequence = 0xffffffff - (2 if rbf else 1)
+            txin.nsequence = nSequence
 
         # Fee estimator
         if fee is None:
@@ -1944,7 +1946,6 @@ def fee_estimator(size: Union[int, float, Decimal]) -> int:
         # Timelock tx to current height.
         tx.locktime = get_locktime_for_new_transaction(self.network)
         tx.rbf_merge_txid = rbf_merge_txid
-        #tx.set_rbf(rbf) # this overwrites nsequence
         tx.add_info_from_wallet(self)
         run_hook('make_unsigned_transaction', self, tx)
         return tx
@@ -3390,7 +3391,7 @@ async def manage_batch_payments(self):
                 self.start_new_batch(tx)
 
             base_tx = self.batch_txs[-1] if self.batch_txs else None
-            # group by locktime
+            # todo: group by locktime
             htlcs = self.htlcs_after(base_tx) if base_tx else self.batch_htlcs
             inputs = [copy.deepcopy(h[0]) for h in htlcs.values()]
             to_pay = [h[1] for h in htlcs.values()]

tests/test_commands.py

@@ -342,6 +342,7 @@ async def test_bumpfee(self, mock_save_db):
         orig_rawtx = "02000000000101b9723dfc69af058ef6613539a000d2cd098a2c8a74e802b6d8739db708ba8c9a0100000000fdffffff02a00f00000000000016001429e1fd187f0cac845946ae1b11dc136c536bfc0fe8b2000000000000160014100611bcb3aee7aad176936cf4ed56ade03027aa02473044022063c05e2347f16251922830ccc757231247b3c2970c225f988e9204844a1ab7b802204652d2c4816707e3d3bea2609b83b079001a435bad2a99cc2e730f276d07070c012102ee3f00141178006c78b0b458aab21588388335078c655459afe544211f15aee050721f00"
         orig_tx = tx_from_any(orig_rawtx)
         orig_txid = orig_tx.txid()
+        self.maxDiff = 1000
         self.assertEqual("02000000000101b9723dfc69af058ef6613539a000d2cd098a2c8a74e802b6d8739db708ba8c9a0100000000fdffffff02a00f00000000000016001429e1fd187f0cac845946ae1b11dc136c536bfc0f84b2000000000000160014100611bcb3aee7aad176936cf4ed56ade03027aa0247304402203aa63539b673a3bd70a76482b17f35f8843974fab28f84143a00450789010bc40220779c2ce2d0217f973f1f6c9f718e19fc7ebd14dd8821a962f002437cda3082ec012102ee3f00141178006c78b0b458aab21588388335078c655459afe544211f15aee000000000",
                          await cmds.bumpfee(tx=orig_rawtx, new_fee_rate='1.6', wallet=wallet))
         # test txid as first arg