From bffad33b743f00513ed37628db1449e632d02bce Mon Sep 17 00:00:00 2001 From: Adam Gibson Date: Thu, 5 Jan 2023 19:05:36 +0000 Subject: [PATCH 1/8] Tx size estimation allows varied in, out types Prior to this commit, p2wsh inputs from fidelity bonds resulted in miscalculation of transaction fees, even in cases where the exact set of inputs were known (such as a direct send). In this commit we change the estimation to a model in which the caller of jmbitcoin.secp256k1_transaction.estimate_tx_size must specify a list of types, one for each input to the transaction, and the same for outputs. In some cases, the caller of the function uses the default script type of the wallet, but in other cases where the caller can know the exact types of each utxo used as input, and each destination used as output, they are specified explicitly. In particular, the use of fidelity bond outputs as input to transactions can be accounted for. Currently this is only done in direct send payments; coinjoins still fall back to assuming all inputs the same type (note that it is not possible to use fidelity bond utxos as inputs to coinjoins). Note also that the burn destination calculation in direct send is removed, since it is not used, so the maintenance burden is best avoided. --- jmbitcoin/jmbitcoin/secp256k1_transaction.py | 143 +++++++++++-------- jmclient/jmclient/taker_utils.py | 62 ++++---- jmclient/jmclient/wallet.py | 44 +++++- jmclient/test/test_taker.py | 12 +- 4 files changed, 154 insertions(+), 107 deletions(-) diff --git a/jmbitcoin/jmbitcoin/secp256k1_transaction.py b/jmbitcoin/jmbitcoin/secp256k1_transaction.py index a42b364..3d4b33b 100644 --- a/jmbitcoin/jmbitcoin/secp256k1_transaction.py +++ b/jmbitcoin/jmbitcoin/secp256k1_transaction.py @@ -106,70 +106,89 @@ def human_readable_output(txoutput): pass # non standard script return outdict -def estimate_tx_size(ins, outs, txtype='p2pkh', outtype=None): +def there_is_one_segwit_input(x): + # note that we need separate input types for + # any distinct types of scripthash inputs supported, + # since each may have a different size of witness; in + # that case, the internal list in this list comprehension + # will need updating. + return any([y in ["p2sh-p2wpkh", "p2wpkh", "p2wsh"] for y in x]) + +def estimate_tx_size(ins, outs): '''Estimate transaction size. - The txtype field as detailed below is used to distinguish - the type, but there is at least one source of meaningful roughness: - we assume that the scriptPubKey type of all the outputs are the same as - the input, unless `outtype` is specified, in which case *one* of - the outputs is assumed to be that other type, with all of the other - outputs being of the same type as before. - This, combined with a few bytes variation in signature sizes means - we will sometimes see small inaccuracies in this estimate. - - Assuming p2pkh: - out: 8+1+3+20+2=34, in: 32+4+1+1+~72+1+33+4=148, - ver: 4, locktime:4, +2 (len in,out) - total = 34*len_out + 148*len_in + 10 (sig sizes vary slightly) - - Assuming p2sh M of N multisig: - "ins" must contain M, N so ins= (numins, M, N) (crude assuming all same) - 73*M + 34*N + 45 per input, so total ins ~ len_ins * (45+73M+34N) - so total ~ 32*len_out + (45+73M+34N)*len_in + 10 - - Assuming p2sh-p2wpkh: - witness are roughly 1+1+~72+1+33 for each input - (txid, vin, 4+20 for witness program encoded as scriptsig, 4 for sequence) - non-witness input fields are roughly 32+4+4+20+4=64, so total becomes - n_in * 64 + 4(ver) + 2(marker, flag) + 2(n_in, n_out) + 4(locktime) + n_out*32 - - Assuming p2wpkh native: - witness as previous case - non-witness loses the 24 witnessprogram, replaced with 1 zero, - in the scriptSig, so becomes: - 4 + 1 + 1 + (n_in) + (vin) + (n_out) + (vout) + (witness) + (locktime) - non-witness: 4(ver) +2 (marker, flag) + n_in*41 + 4(locktime) +2 (len in, out) + n_out*31 - witness: 1 + 1 + 72 + 1 + 33 + Both arguments `ins` and `outs` must be lists of script types, + and they must be present in the keys of the dicts `inmults`, + `outmults` defined here. + Note that variation in ECDSA signature sizes means + we will sometimes see small inaccuracies in this estimate, but + that this is ameliorated by the existence of the witness discount, + in actually estimating fees. + The value '72' is used for the most-likely size of these ECDSA + signatures, due to 30[1 byte] + len(rest)[1 byte] + type:02 [1 byte] + len(r)[1] + r[32 or 33] + type:02[1] + len(s)[1] + s[32] + sighash_all [1] + ... though as can be seen, 71 is also likely: + r length 33 occurs when the value is 'negative' (>N/2) and a byte x80 is prepended, + but shorter values for r are possible if rare. + Returns: + Either a single integer, if the transaction will be non-segwit, + or a tuple (int, int) for witness and non-witness bytes respectively). ''' - if txtype == 'p2pkh': - return 4 + 4 + 2 + ins*148 + 34*outs + ( - OUTPUT_EXTRA_BYTES[txtype][outtype] - if outtype and outtype in OUTPUT_EXTRA_BYTES[txtype] else 0) - elif txtype == 'p2sh-p2wpkh': - #return the estimate for the witness and non-witness - #portions of the transaction, assuming that all the inputs - #are of segwit type p2sh-p2wpkh - # Note as of Jan19: this misses 2 bytes (trivial) for len in, out - # and also overestimates output size by 2 bytes. - witness_estimate = ins*108 - non_witness_estimate = 4 + 4 + 4 + outs*32 + ins*64 + ( - OUTPUT_EXTRA_BYTES[txtype][outtype] - if outtype and outtype in OUTPUT_EXTRA_BYTES[txtype] else 0) - return (witness_estimate, non_witness_estimate) - elif txtype == 'p2wpkh': - witness_estimate = ins*108 - non_witness_estimate = 4 + 4 + 4 + outs*31 + ins*41 + ( - OUTPUT_EXTRA_BYTES[txtype][outtype] - if outtype and outtype in OUTPUT_EXTRA_BYTES[txtype] else 0) - return (witness_estimate, non_witness_estimate) - elif txtype == 'p2shMofN': - ins, M, N = ins - return 4 + 4 + 2 + (45 + 73*M + 34*N)*ins + outs*32 + ( - OUTPUT_EXTRA_BYTES['p2sh-p2wpkh'][outtype] - if outtype and outtype in OUTPUT_EXTRA_BYTES['p2sh-p2wpkh'] else 0) - else: - raise NotImplementedError("Transaction size estimation not" + - "yet implemented for type: " + txtype) + + # All non-witness input sizes include: txid, index, sequence, + # which is 32, 4 and 4; the remaining is scriptSig which is 1 + # at minimum, for native segwit (the byte x00). Hence 41 is the minimum. + # The witness field for p2wpkh consists of sig, pub so 72 + 33 + 1 byte + # for the number of witness elements and 2 bytes for the size of each element, + # hence 108. + # For p2pkh, 148 comes from 32+4+1+1+~72+1+33+4 + # For p2sh-p2wpkh there is an additional 23 bytes of witness for the redeemscript. + # + # Note that p2wsh here is specific to the script + # we use for fidelity bonds; 43 is the bytes required for that + # script's redeemscript field in the witness, but for arbitrary scripts, + # the witness portion could be any other size. + # Hence, we may need to modify this later. + inmults = {"p2wsh": {"w": 1 + 72 + 43, "nw": 41}, + "p2wpkh": {"w": 108, "nw": 41}, + "p2sh-p2wpkh": {"w": 108, "nw": 64}, + "p2pkh": {"w": 0, "nw": 148}} + + # Notes: in outputs, there is only 1 'scripthash' + # type for either segwit/nonsegwit. + # p2wsh has structure 8 bytes output, then: + # x22,x00,x20,(32 byte hash), so 32 + 3 + 8 + # note also there is no need to distinguish witness + # here, outputs are always entirely nonwitness. + outmults = {"p2wsh": 43, + "p2wpkh": 31, + "p2sh-p2wpkh": 64, + "p2pkh": 34} + + # nVersion, nLockTime, nins, nouts: + nwsize = 4 + 4 + 2 + wsize = 0 + tx_is_segwit = there_is_one_segwit_input(ins) + if tx_is_segwit: + # flag and marker bytes + nwsize += 2 + + for i in ins: + if i not in inmults: + raise NotImplementedError( + "Script type not supported for transaction size " + "estimation: {}".format(i)) + inmult = inmults[i] + nwsize += inmult["nw"] + wsize += inmult["w"] + for o in outs: + if o not in outmults: + raise NotImplementedError( + "Script type not supported for transaction size " + "estimation: {}".format(o)) + nwsize += outmults[o] + + if not tx_is_segwit: + return nwsize + return (wsize, nwsize) def pubkey_to_p2pkh_script(pub, require_compressed=False): """ diff --git a/jmclient/jmclient/taker_utils.py b/jmclient/jmclient/taker_utils.py index a89fc60..97e4547 100644 --- a/jmclient/jmclient/taker_utils.py +++ b/jmclient/jmclient/taker_utils.py @@ -10,9 +10,9 @@ from .schedule import human_readable_schedule_entry, tweak_tumble_schedule,\ schedule_to_text from .wallet import BaseWallet, estimate_tx_fee, compute_tx_locktime, \ FidelityBondMixin -from jmbitcoin import make_shuffled_tx, amount_to_str, mk_burn_script,\ +from jmbitcoin import make_shuffled_tx, amount_to_str, \ PartiallySignedTransaction, CMutableTxOut,\ - human_readable_transaction, Hash160 + human_readable_transaction from jmbase.support import EXIT_SUCCESS log = get_log() @@ -21,6 +21,15 @@ Utility functions for tumbler-style takers; Currently re-used by CLI script tumbler.py and joinmarket-qt """ +def get_utxo_scripts(wallet: BaseWallet, utxos): + # given a Joinmarket wallet and a set of utxos + # as passed from `get_utxos_by_mixdepth` at one mixdepth, + # return the list of script types for each utxo + script_types = [] + for k, v in utxos.items(): + script_types.append(wallet.get_outtype(v["address"])) + return script_types + def direct_send(wallet_service, amount, mixdepth, destination, answeryes=False, accept_callback=None, info_callback=None, error_callback=None, return_transaction=False, with_final_psbt=False, @@ -97,38 +106,27 @@ def direct_send(wallet_service, amount, mixdepth, destination, answeryes=False, "There are no available utxos in mixdepth: " + str(mixdepth) + ", quitting.") return total_inputs_val = sum([va['value'] for u, va in utxos.items()]) - - if is_burn_destination(destination): - if len(utxos) > 1: - log.error("Only one input allowed when burning coins, to keep " - + "the tx small. Tip: use the coin control feature to freeze utxos") - return - address_type = FidelityBondMixin.BIP32_BURN_ID - index = wallet_service.wallet.get_next_unused_index(mixdepth, address_type) - path = wallet_service.wallet.get_path(mixdepth, address_type, index) - privkey, engine = wallet_service.wallet._get_key_from_path(path) - pubkey = engine.privkey_to_pubkey(privkey) - pubkeyhash = Hash160(pubkey) - - #size of burn output is slightly different from regular outputs - burn_script = mk_burn_script(pubkeyhash) - fee_est = estimate_tx_fee(len(utxos), 0, txtype=txtype, extra_bytes=len(burn_script)/2) - - outs = [{"script": burn_script, "value": total_inputs_val - fee_est}] - destination = "BURNER OUTPUT embedding pubkey at " \ - + wallet_service.wallet.get_path_repr(path) \ - + "\n\nWARNING: This transaction if broadcasted will PERMANENTLY DESTROY your bitcoins\n" - else: - #regular sweep (non-burn) - fee_est = estimate_tx_fee(len(utxos), 1, txtype=txtype, outtype=outtype) - outs = [{"address": destination, "value": total_inputs_val - fee_est}] + script_types = get_utxo_scripts(wallet_service.wallet, utxos) + fee_est = estimate_tx_fee(len(utxos), 1, txtype=script_types, outtype=outtype) + outs = [{"address": destination, "value": total_inputs_val - fee_est}] else: - #not doing a sweep; we will have change - #8 inputs to be conservative - initial_fee_est = estimate_tx_fee(8,2, txtype=txtype, outtype=outtype) - utxos = wallet_service.select_utxos(mixdepth, amount + initial_fee_est) + change_type = wallet_service.get_txtype() + if custom_change_addr: + change_type = wallet_service.get_outtype(custom_change_addr) + if change_type is None: + # we don't recognize this type; best we can do is revert to default, + # even though it may be inaccurate: + change_type = wallet_service.get_txtype() + outtypes = [change_type, outtype] + # not doing a sweep; we will have change. + # 8 inputs to be conservative; note we cannot account for the possibility + # of non-standard input types at this point. + initial_fee_est = estimate_tx_fee(8,2, txtype=txtype, outtype=outtypes) + utxos = wallet_service.select_utxos(mixdepth, amount + initial_fee_est, + includeaddr=True) + script_types = get_utxo_scripts(wallet_service.wallet, utxos) if len(utxos) < 8: - fee_est = estimate_tx_fee(len(utxos), 2, txtype=txtype, outtype=outtype) + fee_est = estimate_tx_fee(len(utxos), 2, txtype=script_types, outtype=outtypes) else: fee_est = initial_fee_est total_inputs_val = sum([va['value'] for u, va in utxos.items()]) diff --git a/jmclient/jmclient/wallet.py b/jmclient/jmclient/wallet.py index 2f373fb..237f5f7 100644 --- a/jmclient/jmclient/wallet.py +++ b/jmclient/jmclient/wallet.py @@ -57,6 +57,21 @@ def estimate_tx_fee(ins, outs, txtype='p2pkh', outtype=None, extra_bytes=0): '''Returns an estimate of the number of satoshis required for a transaction with the given number of inputs and outputs, based on information from the blockchain interface. + + Arguments: + ins: int, number of inputs + outs: int, number of outputs + txtype: either a single string, or a list of strings + outtype: either None or a list of strings + extra_bytes: an int + These arguments are intended to allow a kind of 'default', where + all the inputs and outputs match a predefined type (that of the wallet), + but also allow customization for heterogeneous input and output types. + For supported input and output types, see the keys of the dicts + `inmults` and `outmults` in jmbitcoin.secp256k1_transaction.estimate_tx_size`. + + Returns: + a single integer number of satoshis as estimate. ''' if jm_single().bc_interface is None: raise RuntimeError("Cannot estimate transaction fee " + @@ -73,18 +88,33 @@ def estimate_tx_fee(ins, outs, txtype='p2pkh', outtype=None, extra_bytes=0): btc.fee_per_kb_to_str(fee_per_kb) + " greater than absurd value " + btc.fee_per_kb_to_str(absurd_fee) + ", quitting.") - if txtype in ['p2pkh', 'p2shMofN']: - tx_estimated_bytes = btc.estimate_tx_size(ins, outs, txtype, outtype) + extra_bytes + + # See docstring for explanation: + if isinstance(txtype, str): + ins = [txtype]* ins + else: + assert isinstance(txtype, list) + ins = txtype + if outtype is None: + outs = [txtype] * outs + elif isinstance(outtype, str): + outs = [outtype] * outs + else: + assert isinstance(outtype, list) + outs = outtype + + # Note: the calls to `estimate_tx_size` in this code + # block can raise `NotImplementedError` if any of the + # strings in (ins, outs) are not known script types. + if not btc.there_is_one_segwit_input(ins): + tx_estimated_bytes = btc.estimate_tx_size(ins, outs) + extra_bytes return int((tx_estimated_bytes * fee_per_kb)/Decimal(1000.0)) - elif txtype in ['p2wpkh', 'p2sh-p2wpkh']: + else: witness_estimate, non_witness_estimate = btc.estimate_tx_size( - ins, outs, txtype, outtype) + ins, outs) non_witness_estimate += extra_bytes return int(int(( non_witness_estimate + 0.25*witness_estimate)*fee_per_kb)/Decimal(1000.0)) - else: - raise NotImplementedError("Txtype: " + txtype + " not implemented.") - def compute_tx_locktime(): # set locktime for best anonset (Core, Electrum) diff --git a/jmclient/test/test_taker.py b/jmclient/test/test_taker.py index 47db342..5b96b33 100644 --- a/jmclient/test/test_taker.py +++ b/jmclient/test/test_taker.py @@ -513,15 +513,15 @@ def test_custom_change(setup_taker): for out in taker.latest_tx.vout: # input utxo is 200M; amount is 20M; as per logs: # totalin=200000000 - # my_txfee=13050 + # my_txfee=13680 <- this estimate ignores address type # makers_txfee=3000 - # cjfee_total=12000 => changevalue=179974950 + # cjfee_total=12000 => changevalue=179974320 # note that there is a small variation in the size of # the transaction (a few bytes) for the different scriptPubKey - # type, but this is currently ignored by the Taker, who makes - # fee estimate purely based on the number of ins and outs; - # this will never be too far off anyway. - if out.scriptPubKey == script and out.nValue == 179974950: + # type, but this is currently ignored in coinjoins by the + # Taker (not true for direct send operations), hence we get + # the same value for each different output type. + if out.scriptPubKey == script and out.nValue == 179974320: # must be only one assert not custom_change_found custom_change_found = True From 7bf6696228c181460c3439917cd21ef148603658 Mon Sep 17 00:00:00 2001 From: Adam Gibson Date: Thu, 5 Jan 2023 22:27:47 +0000 Subject: [PATCH 2/8] Add tests of size estimator in jmbitcoin --- jmbitcoin/jmbitcoin/secp256k1_transaction.py | 2 +- jmbitcoin/test/test_tx_signing.py | 22 ++++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/jmbitcoin/jmbitcoin/secp256k1_transaction.py b/jmbitcoin/jmbitcoin/secp256k1_transaction.py index 3d4b33b..a628279 100644 --- a/jmbitcoin/jmbitcoin/secp256k1_transaction.py +++ b/jmbitcoin/jmbitcoin/secp256k1_transaction.py @@ -160,7 +160,7 @@ def estimate_tx_size(ins, outs): # here, outputs are always entirely nonwitness. outmults = {"p2wsh": 43, "p2wpkh": 31, - "p2sh-p2wpkh": 64, + "p2sh-p2wpkh": 32, "p2pkh": 34} # nVersion, nLockTime, nins, nouts: diff --git a/jmbitcoin/test/test_tx_signing.py b/jmbitcoin/test/test_tx_signing.py index 8178228..4b9e5ba 100644 --- a/jmbitcoin/test/test_tx_signing.py +++ b/jmbitcoin/test/test_tx_signing.py @@ -4,6 +4,28 @@ import hashlib from jmbase import bintohex import jmbitcoin as btc + +# Cases copied from: +# https://github.com/kristapsk/bitcoin-scripts/blob/0b847bec016638e60313ecec2b81f2e8accd311b/tests/tx-vsize.bats +@pytest.mark.parametrize( + "inaddrtypes, outaddrtypes, size_expected", + [(["p2pkh"], ["p2pkh"], 192), + (["p2pkh"], ["p2pkh", "p2pkh"], 226), + (["p2pkh"], ["p2sh-p2wpkh", "p2sh-p2wpkh"], 222), + (["p2pkh"], ["p2pkh", "p2sh-p2wpkh"], 224), + (["p2sh-p2wpkh"], ["p2sh-p2wpkh"], 135), + (["p2wpkh"], ["p2wpkh"], 111), + ]) +def test_tx_size_estimate(inaddrtypes, outaddrtypes, size_expected): + # non-sw only inputs result in a single integer return, + # segwit inputs return (witness size, non-witness size) + x = btc.estimate_tx_size(inaddrtypes, outaddrtypes) + if btc.there_is_one_segwit_input(inaddrtypes): + s = int(x[0]/4 + x[1]) + else: + s = x + assert s == size_expected + @pytest.mark.parametrize( "addrtype", [("p2wpkh"), From 4921d01b9277d1b00eccb2b1ae1c3b129f426dee Mon Sep 17 00:00:00 2001 From: Adam Gibson Date: Fri, 6 Jan 2023 15:30:21 +0000 Subject: [PATCH 3/8] add test cases, move marker, flag to witness --- jmbitcoin/jmbitcoin/secp256k1_transaction.py | 4 +-- jmbitcoin/test/test_tx_signing.py | 31 ++++++++++++++++---- jmclient/jmclient/wallet.py | 4 +-- jmclient/test/test_taker.py | 6 ++-- 4 files changed, 33 insertions(+), 12 deletions(-) diff --git a/jmbitcoin/jmbitcoin/secp256k1_transaction.py b/jmbitcoin/jmbitcoin/secp256k1_transaction.py index a628279..c0b3c41 100644 --- a/jmbitcoin/jmbitcoin/secp256k1_transaction.py +++ b/jmbitcoin/jmbitcoin/secp256k1_transaction.py @@ -168,8 +168,8 @@ def estimate_tx_size(ins, outs): wsize = 0 tx_is_segwit = there_is_one_segwit_input(ins) if tx_is_segwit: - # flag and marker bytes - nwsize += 2 + # flag and marker bytes are included in witness + wsize += 2 for i in ins: if i not in inmults: diff --git a/jmbitcoin/test/test_tx_signing.py b/jmbitcoin/test/test_tx_signing.py index 4b9e5ba..bfdb946 100644 --- a/jmbitcoin/test/test_tx_signing.py +++ b/jmbitcoin/test/test_tx_signing.py @@ -3,25 +3,46 @@ import pytest import hashlib from jmbase import bintohex import jmbitcoin as btc +from math import ceil -# Cases copied from: +# Case of spending into a FB, one p2wpkh input, one FB output, one p2wpkh output: +# +# "020000000001019b3a5c6ec9712bd6b1aa1e07ed12a677eb21215430fb84a689664fb0d4fa175a0000000000feffffff0290099c0a000000001600144d32ca4673822334531a2941bb85bff075c384d180b14f0100000000220020dae515a98f31542dd6b21bb0b0e31fccc4ebcdc9ba3e225798bc981ccbb8a21d024830450221009bc5fb8d077b32304c02a886926a68110bc7c5195a92b55977bd4affd23ab2d50220774e88828cf80cba4bb2e277a69a7c693c3566b5b72a0c0cf25cba89df0646d30121036d1baed4008f0d7f03ac41bbdbe46e02eeef6f040f7bdbc41adac30c2cf8831886020000" +# +# case of spending from a FB, single input, to 2 p2wpkh outputs +# "020000000001013499e10a8035a3cb928f5a894b2a3feed7d46ab579f0a2211a17ed1df0e2d9660100000000feffffff02405dc6000000000016001486551870825ef64d7fda89cc2a6fda497ab71a02954d8900000000001600147c24b9a3ddf5eb26142fb3f05a3746a0a582f81a0247304402206ca2049154939b083a5eb22713d3cb78f6673f43828fa4a7ef0f03275584da6c0220770fe7d50ba5e0a5f8039f28c5deaaf1e8b2030008bf51cc8721cf088eab5586012a0480c22e61b175210376e5964ee2c328e85b88a50f02953b1cddb1490825140331a3948023cc19946bac81c22e61" +# +# case of spending from an FB plus one more p2wpkh input, spending to a p2sh output and a p2wpkh output +# "02000000000102117efb788cee9644d5493fc9d1a120598f4f5200bb6909c0ebdac66cf88da80a0100000000feffffffb0f3ce583987d08080684e2c75d2d6871cb2d30f610327671440d7121c14b7ab0000000000feffffff0240a5ae020000000017a914c579342c2c4c9220205e2cdc285617040c924a0a8797027a00000000001600146fad63e6420ec3eb423a105b05c6df6cc8dab92902473044022040f1db3289d8c6e0dd94c0f55254f10512486094e36fd92f4423abc95320418902206718670b3f332d84cf8294ad389155594ebe125988f2f535c58ff3b077471ce9012102f9306fdc84a366f21fed44fbdd9149a046829f26fb52e7a316b19b536c18d2df0247304402207d0fde11ce32f48107ac246a01efad1c22b9466cd9ff4d5683030790bcb34ce5022020014fcf1b1b0606db5ef363568f10476524f7102644691a8a435faa17cbbe88012a04804f5661b17521037a9502304b2810706adef15b884ac7ca0c48c2e5d03cf93934487b44feb7c276ac814f5661" +# +# case of spending from one FB input to one FB output +# +# "0200000000010150c8e3786d357cbe61d8e27de7439e1b32d75d0a0ad596c5ff2863134cbd3ead0100000000feffffff01db84f701000000002200208b8ed0bc565e419dd956c3841b7bb25f7c197e2699002bac58a68f47206e1f340247304402202a639209aa9a2883ad75210edce2165260167435f56cede83e8c74095944f355022050fde591f1fefb615a072a797ace3c332c678e0f9161e58d79efa1705f9ab17c012a04002e7f61b1752103d4d747d0dca80c129c017ec1cdc658945013e04ff3d6946f15ccc9df52c323f0ac012e7f61" +# +# Virtual sizes can be calculated from bitcointx.core.CTransaction.deserialize(unhexlify(txhex)).get_virtual_size() +# +# More cases copied from: # https://github.com/kristapsk/bitcoin-scripts/blob/0b847bec016638e60313ecec2b81f2e8accd311b/tests/tx-vsize.bats @pytest.mark.parametrize( "inaddrtypes, outaddrtypes, size_expected", - [(["p2pkh"], ["p2pkh"], 192), + [(["p2wpkh"], ["p2wsh", "p2wpkh"], 153), + (["p2wsh"], ["p2wpkh", "p2wpkh"], 143), + (["p2wsh", "p2wpkh"], ["p2sh-p2wpkh", "p2wpkh"], 212), + (["p2wsh"], ["p2wsh"], 124), + (["p2pkh"], ["p2pkh"], 192), (["p2pkh"], ["p2pkh", "p2pkh"], 226), (["p2pkh"], ["p2sh-p2wpkh", "p2sh-p2wpkh"], 222), (["p2pkh"], ["p2pkh", "p2sh-p2wpkh"], 224), - (["p2sh-p2wpkh"], ["p2sh-p2wpkh"], 135), - (["p2wpkh"], ["p2wpkh"], 111), + (["p2sh-p2wpkh"], ["p2sh-p2wpkh"], 134), + (["p2wpkh"], ["p2wpkh"], 110), ]) def test_tx_size_estimate(inaddrtypes, outaddrtypes, size_expected): # non-sw only inputs result in a single integer return, # segwit inputs return (witness size, non-witness size) x = btc.estimate_tx_size(inaddrtypes, outaddrtypes) if btc.there_is_one_segwit_input(inaddrtypes): - s = int(x[0]/4 + x[1]) + s = ceil((x[0] + x[1] * 4)/4.0) else: s = x assert s == size_expected diff --git a/jmclient/jmclient/wallet.py b/jmclient/jmclient/wallet.py index 237f5f7..f2892d9 100644 --- a/jmclient/jmclient/wallet.py +++ b/jmclient/jmclient/wallet.py @@ -8,6 +8,7 @@ import random import copy import base64 import json +from math import ceil from binascii import hexlify, unhexlify from datetime import datetime, timedelta from calendar import timegm @@ -113,8 +114,7 @@ def estimate_tx_fee(ins, outs, txtype='p2pkh', outtype=None, extra_bytes=0): witness_estimate, non_witness_estimate = btc.estimate_tx_size( ins, outs) non_witness_estimate += extra_bytes - return int(int(( - non_witness_estimate + 0.25*witness_estimate)*fee_per_kb)/Decimal(1000.0)) + return int(int(ceil(non_witness_estimate + 0.25*witness_estimate)*fee_per_kb)/Decimal(1000.0)) def compute_tx_locktime(): # set locktime for best anonset (Core, Electrum) diff --git a/jmclient/test/test_taker.py b/jmclient/test/test_taker.py index 5b96b33..aa7a51b 100644 --- a/jmclient/test/test_taker.py +++ b/jmclient/test/test_taker.py @@ -513,15 +513,15 @@ def test_custom_change(setup_taker): for out in taker.latest_tx.vout: # input utxo is 200M; amount is 20M; as per logs: # totalin=200000000 - # my_txfee=13680 <- this estimate ignores address type + # my_txfee=13650 <- this estimate ignores address type # makers_txfee=3000 - # cjfee_total=12000 => changevalue=179974320 + # cjfee_total=12000 => changevalue=179974350 # note that there is a small variation in the size of # the transaction (a few bytes) for the different scriptPubKey # type, but this is currently ignored in coinjoins by the # Taker (not true for direct send operations), hence we get # the same value for each different output type. - if out.scriptPubKey == script and out.nValue == 179974320: + if out.scriptPubKey == script and out.nValue == 179974350: # must be only one assert not custom_change_found custom_change_found = True From db71d30b80380fed44e331e288ee4020df02035c Mon Sep 17 00:00:00 2001 From: Adam Gibson Date: Sat, 7 Jan 2023 13:55:18 +0000 Subject: [PATCH 4/8] address review of @PulpCattel --- jmbitcoin/jmbitcoin/secp256k1_transaction.py | 10 ++++------ jmclient/jmclient/taker_utils.py | 8 ++++---- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/jmbitcoin/jmbitcoin/secp256k1_transaction.py b/jmbitcoin/jmbitcoin/secp256k1_transaction.py index c0b3c41..1d61d25 100644 --- a/jmbitcoin/jmbitcoin/secp256k1_transaction.py +++ b/jmbitcoin/jmbitcoin/secp256k1_transaction.py @@ -106,13 +106,13 @@ def human_readable_output(txoutput): pass # non standard script return outdict -def there_is_one_segwit_input(x): +def there_is_one_segwit_input(input_types): # note that we need separate input types for # any distinct types of scripthash inputs supported, # since each may have a different size of witness; in # that case, the internal list in this list comprehension # will need updating. - return any([y in ["p2sh-p2wpkh", "p2wpkh", "p2wsh"] for y in x]) + return any(y in ["p2sh-p2wpkh", "p2wpkh", "p2wsh"] for y in input_types) def estimate_tx_size(ins, outs): '''Estimate transaction size. @@ -174,16 +174,14 @@ def estimate_tx_size(ins, outs): for i in ins: if i not in inmults: raise NotImplementedError( - "Script type not supported for transaction size " - "estimation: {}".format(i)) + f"Script type not supported for transaction size estimation: {i}") inmult = inmults[i] nwsize += inmult["nw"] wsize += inmult["w"] for o in outs: if o not in outmults: raise NotImplementedError( - "Script type not supported for transaction size " - "estimation: {}".format(o)) + f"Script type not supported for transaction size estimation: {o}") nwsize += outmults[o] if not tx_is_segwit: diff --git a/jmclient/jmclient/taker_utils.py b/jmclient/jmclient/taker_utils.py index 97e4547..c166d00 100644 --- a/jmclient/jmclient/taker_utils.py +++ b/jmclient/jmclient/taker_utils.py @@ -26,8 +26,8 @@ def get_utxo_scripts(wallet: BaseWallet, utxos): # as passed from `get_utxos_by_mixdepth` at one mixdepth, # return the list of script types for each utxo script_types = [] - for k, v in utxos.items(): - script_types.append(wallet.get_outtype(v["address"])) + for utxo in utxos.values(): + script_types.append(wallet.get_outtype(utxo["address"])) return script_types def direct_send(wallet_service, amount, mixdepth, destination, answeryes=False, @@ -110,13 +110,13 @@ def direct_send(wallet_service, amount, mixdepth, destination, answeryes=False, fee_est = estimate_tx_fee(len(utxos), 1, txtype=script_types, outtype=outtype) outs = [{"address": destination, "value": total_inputs_val - fee_est}] else: - change_type = wallet_service.get_txtype() + change_type = txtype if custom_change_addr: change_type = wallet_service.get_outtype(custom_change_addr) if change_type is None: # we don't recognize this type; best we can do is revert to default, # even though it may be inaccurate: - change_type = wallet_service.get_txtype() + change_type = txtype outtypes = [change_type, outtype] # not doing a sweep; we will have change. # 8 inputs to be conservative; note we cannot account for the possibility From 6250d243df9469b0e20a01499b0e5d91b4206800 Mon Sep 17 00:00:00 2001 From: Adam Gibson Date: Thu, 12 Jan 2023 11:27:46 +0000 Subject: [PATCH 5/8] add typing hints --- jmbitcoin/jmbitcoin/secp256k1_transaction.py | 5 +++-- jmclient/jmclient/taker_utils.py | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/jmbitcoin/jmbitcoin/secp256k1_transaction.py b/jmbitcoin/jmbitcoin/secp256k1_transaction.py index 1d61d25..a4d7b0f 100644 --- a/jmbitcoin/jmbitcoin/secp256k1_transaction.py +++ b/jmbitcoin/jmbitcoin/secp256k1_transaction.py @@ -1,6 +1,7 @@ # note, only used for non-cryptographic randomness: import random import json +from typing import List, Union, Tuple # needed for single sha256 evaluation, which is used # in bitcoin (p2wsh) but not exposed in python-bitcointx: import hashlib @@ -106,7 +107,7 @@ def human_readable_output(txoutput): pass # non standard script return outdict -def there_is_one_segwit_input(input_types): +def there_is_one_segwit_input(input_types: List[str]) -> bool: # note that we need separate input types for # any distinct types of scripthash inputs supported, # since each may have a different size of witness; in @@ -114,7 +115,7 @@ def there_is_one_segwit_input(input_types): # will need updating. return any(y in ["p2sh-p2wpkh", "p2wpkh", "p2wsh"] for y in input_types) -def estimate_tx_size(ins, outs): +def estimate_tx_size(ins: List[str], outs: List[str]) -> Union[int, Tuple[int]]: '''Estimate transaction size. Both arguments `ins` and `outs` must be lists of script types, and they must be present in the keys of the dicts `inmults`, diff --git a/jmclient/jmclient/taker_utils.py b/jmclient/jmclient/taker_utils.py index c166d00..a81563b 100644 --- a/jmclient/jmclient/taker_utils.py +++ b/jmclient/jmclient/taker_utils.py @@ -21,7 +21,7 @@ Utility functions for tumbler-style takers; Currently re-used by CLI script tumbler.py and joinmarket-qt """ -def get_utxo_scripts(wallet: BaseWallet, utxos): +def get_utxo_scripts(wallet: BaseWallet, utxos: dict) -> list: # given a Joinmarket wallet and a set of utxos # as passed from `get_utxos_by_mixdepth` at one mixdepth, # return the list of script types for each utxo From e281c14fa30a9a2430d2c8947170ed4869755de4 Mon Sep 17 00:00:00 2001 From: Adam Gibson Date: Sat, 14 Jan 2023 12:25:27 +0000 Subject: [PATCH 6/8] account for unrecognized destination type --- jmclient/jmclient/taker_utils.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/jmclient/jmclient/taker_utils.py b/jmclient/jmclient/taker_utils.py index a81563b..a500fd2 100644 --- a/jmclient/jmclient/taker_utils.py +++ b/jmclient/jmclient/taker_utils.py @@ -117,6 +117,11 @@ def direct_send(wallet_service, amount, mixdepth, destination, answeryes=False, # we don't recognize this type; best we can do is revert to default, # even though it may be inaccurate: change_type = txtype + if outtype is None: + # we don't recognize the destination script type, + # so set it as the same as the change (which will usually + # be the same as the spending wallet, but see above for custom) + outtype = change_type outtypes = [change_type, outtype] # not doing a sweep; we will have change. # 8 inputs to be conservative; note we cannot account for the possibility From c1d7f023d0d0d2dd03d2ff0c4e790f4b983217ce Mon Sep 17 00:00:00 2001 From: Adam Gibson Date: Sat, 14 Jan 2023 12:28:01 +0000 Subject: [PATCH 7/8] remove now unused OUTPUT_EXTRA_BYTES --- jmbitcoin/jmbitcoin/secp256k1_transaction.py | 25 -------------------- 1 file changed, 25 deletions(-) diff --git a/jmbitcoin/jmbitcoin/secp256k1_transaction.py b/jmbitcoin/jmbitcoin/secp256k1_transaction.py index a4d7b0f..8dfc3c5 100644 --- a/jmbitcoin/jmbitcoin/secp256k1_transaction.py +++ b/jmbitcoin/jmbitcoin/secp256k1_transaction.py @@ -19,31 +19,6 @@ from bitcointx.core.scripteval import (VerifyScript, SCRIPT_VERIFY_WITNESS, SCRIPT_VERIFY_STRICTENC, SIGVERSION_WITNESS_V0) -# for each transaction type, different output script pubkeys may result in -# a difference in the number of bytes accounted for while estimating the -# transaction size, this variable stores the difference and is factored in -# when calculating the correct transaction size. For example, for a p2pkh -# transaction, if one of the outputs is a p2wsh pubkey, then the transaction -# would need 9 extra bytes to account for the difference in script pubkey -# sizes -OUTPUT_EXTRA_BYTES = { - 'p2pkh': { - 'p2wpkh': -3, - 'p2sh-p2wpkh': -2, - 'p2wsh': 9 - }, - 'p2wpkh': { - 'p2pkh': 3, - 'p2sh-p2wpkh': 1, - 'p2wsh': 12 - }, - 'p2sh-p2wpkh': { - 'p2pkh': 2, - 'p2wpkh': -1, - 'p2wsh': 11 - } -} - def human_readable_transaction(tx, jsonified=True): """ Given a CTransaction object, output a human readable json-formatted string (suitable for terminal From 357b61165551ac1e096527bc9f372c2c03377404 Mon Sep 17 00:00:00 2001 From: Adam Gibson Date: Sat, 14 Jan 2023 16:41:51 +0000 Subject: [PATCH 8/8] support p2tr outputs in size estimation --- jmbitcoin/jmbitcoin/secp256k1_transaction.py | 14 ++++++++++++-- jmbitcoin/test/test_tx_signing.py | 3 ++- jmclient/jmclient/cryptoengine.py | 13 +++++++++++-- jmclient/jmclient/taker_utils.py | 4 +++- jmclient/jmclient/wallet.py | 8 +++++--- 5 files changed, 33 insertions(+), 9 deletions(-) diff --git a/jmbitcoin/jmbitcoin/secp256k1_transaction.py b/jmbitcoin/jmbitcoin/secp256k1_transaction.py index 8dfc3c5..d58deb7 100644 --- a/jmbitcoin/jmbitcoin/secp256k1_transaction.py +++ b/jmbitcoin/jmbitcoin/secp256k1_transaction.py @@ -88,6 +88,7 @@ def there_is_one_segwit_input(input_types: List[str]) -> bool: # since each may have a different size of witness; in # that case, the internal list in this list comprehension # will need updating. + # note that there is no support yet for paying *from* p2tr. return any(y in ["p2sh-p2wpkh", "p2wpkh", "p2wsh"] for y in input_types) def estimate_tx_size(ins: List[str], outs: List[str]) -> Union[int, Tuple[int]]: @@ -123,21 +124,30 @@ def estimate_tx_size(ins: List[str], outs: List[str]) -> Union[int, Tuple[int]]: # script's redeemscript field in the witness, but for arbitrary scripts, # the witness portion could be any other size. # Hence, we may need to modify this later. + # + # Note that there is no support yet for spending *from* p2tr: + # we should fix this soon, since it is desirable to be able to support + # coinjoins with counterparties sending taproot, but note, JM coinjoins + # do not allow non-standard (usually v0 segwit) inputs, anyway. inmults = {"p2wsh": {"w": 1 + 72 + 43, "nw": 41}, "p2wpkh": {"w": 108, "nw": 41}, "p2sh-p2wpkh": {"w": 108, "nw": 64}, "p2pkh": {"w": 0, "nw": 148}} # Notes: in outputs, there is only 1 'scripthash' - # type for either segwit/nonsegwit. + # type for either segwit/nonsegwit (hence "p2sh-p2wpkh" + # is a bit misleading, but is kept to the same as inputs, + # for simplicity. See notes on inputs above). # p2wsh has structure 8 bytes output, then: # x22,x00,x20,(32 byte hash), so 32 + 3 + 8 # note also there is no need to distinguish witness # here, outputs are always entirely nonwitness. + # p2tr is also 32 byte hash with x01 instead of x00 version. outmults = {"p2wsh": 43, "p2wpkh": 31, "p2sh-p2wpkh": 32, - "p2pkh": 34} + "p2pkh": 34, + "p2tr": 43} # nVersion, nLockTime, nins, nouts: nwsize = 4 + 4 + 2 diff --git a/jmbitcoin/test/test_tx_signing.py b/jmbitcoin/test/test_tx_signing.py index bfdb946..ebe73ef 100644 --- a/jmbitcoin/test/test_tx_signing.py +++ b/jmbitcoin/test/test_tx_signing.py @@ -36,13 +36,14 @@ from math import ceil (["p2pkh"], ["p2pkh", "p2sh-p2wpkh"], 224), (["p2sh-p2wpkh"], ["p2sh-p2wpkh"], 134), (["p2wpkh"], ["p2wpkh"], 110), + (["p2wpkh"], ["p2wpkh", "p2tr"], 153), ]) def test_tx_size_estimate(inaddrtypes, outaddrtypes, size_expected): # non-sw only inputs result in a single integer return, # segwit inputs return (witness size, non-witness size) x = btc.estimate_tx_size(inaddrtypes, outaddrtypes) if btc.there_is_one_segwit_input(inaddrtypes): - s = ceil((x[0] + x[1] * 4)/4.0) + s = ceil((x[0] + x[1] * 4) / 4.0) else: s = x assert s == size_expected diff --git a/jmclient/jmclient/cryptoengine.py b/jmclient/jmclient/cryptoengine.py index 70f35f7..065e3ee 100644 --- a/jmclient/jmclient/cryptoengine.py +++ b/jmclient/jmclient/cryptoengine.py @@ -16,7 +16,7 @@ from .configure import get_network, jm_single # make existing wallets unsable. TYPE_P2PKH, TYPE_P2SH_P2WPKH, TYPE_P2WPKH, TYPE_P2SH_M_N, TYPE_TIMELOCK_P2WSH, \ TYPE_SEGWIT_WALLET_FIDELITY_BONDS, TYPE_WATCHONLY_FIDELITY_BONDS, \ - TYPE_WATCHONLY_TIMELOCK_P2WSH, TYPE_WATCHONLY_P2WPKH, TYPE_P2WSH = range(10) + TYPE_WATCHONLY_TIMELOCK_P2WSH, TYPE_WATCHONLY_P2WPKH, TYPE_P2WSH, TYPE_P2TR = range(11) NET_MAINNET, NET_TESTNET, NET_SIGNET = range(3) NET_MAP = {'mainnet': NET_MAINNET, 'testnet': NET_TESTNET, 'signet': NET_SIGNET} @@ -52,6 +52,8 @@ def detect_script_type(script_str): return TYPE_P2WPKH elif script.is_witness_v0_scripthash(): return TYPE_P2WSH + elif script.is_witness_v1_taproot(): + return TYPE_P2TR raise EngineError("Unknown script type for script '{}'" .format(bintohex(script_str))) @@ -224,6 +226,12 @@ class BTCEngine(object): stype = detect_script_type(script) assert stype in ENGINES engine = ENGINES[stype] + # TODO though taproot is currently a returnable + # type from detect_script_type, there is not yet + # a corresponding ENGINE, thus a None return is possible. + # Callers recognize this as EngineError. + if engine is None: + raise EngineError pscript = engine.pubkey_to_script(pubkey) return script == pscript @@ -457,5 +465,6 @@ ENGINES = { TYPE_TIMELOCK_P2WSH: BTC_Timelocked_P2WSH, TYPE_WATCHONLY_TIMELOCK_P2WSH: BTC_Watchonly_Timelocked_P2WSH, TYPE_WATCHONLY_P2WPKH: BTC_Watchonly_P2WPKH, - TYPE_SEGWIT_WALLET_FIDELITY_BONDS: BTC_P2WPKH + TYPE_SEGWIT_WALLET_FIDELITY_BONDS: BTC_P2WPKH, + TYPE_P2TR: None # TODO } diff --git a/jmclient/jmclient/taker_utils.py b/jmclient/jmclient/taker_utils.py index a500fd2..7ed235e 100644 --- a/jmclient/jmclient/taker_utils.py +++ b/jmclient/jmclient/taker_utils.py @@ -121,12 +121,14 @@ def direct_send(wallet_service, amount, mixdepth, destination, answeryes=False, # we don't recognize the destination script type, # so set it as the same as the change (which will usually # be the same as the spending wallet, but see above for custom) + # Notice that this is handled differently to the sweep case above, + # because we must use a list - there is more than one output outtype = change_type outtypes = [change_type, outtype] # not doing a sweep; we will have change. # 8 inputs to be conservative; note we cannot account for the possibility # of non-standard input types at this point. - initial_fee_est = estimate_tx_fee(8,2, txtype=txtype, outtype=outtypes) + initial_fee_est = estimate_tx_fee(8, 2, txtype=txtype, outtype=outtypes) utxos = wallet_service.select_utxos(mixdepth, amount + initial_fee_est, includeaddr=True) script_types = get_utxo_scripts(wallet_service.wallet, utxos) diff --git a/jmclient/jmclient/wallet.py b/jmclient/jmclient/wallet.py index f2892d9..094a326 100644 --- a/jmclient/jmclient/wallet.py +++ b/jmclient/jmclient/wallet.py @@ -27,8 +27,8 @@ from .support import select_gradual, select_greedy, select_greediest, \ select, NotEnoughFundsException from .cryptoengine import TYPE_P2PKH, TYPE_P2SH_P2WPKH, TYPE_P2WSH,\ TYPE_P2WPKH, TYPE_TIMELOCK_P2WSH, TYPE_SEGWIT_WALLET_FIDELITY_BONDS,\ - TYPE_WATCHONLY_FIDELITY_BONDS, TYPE_WATCHONLY_TIMELOCK_P2WSH, TYPE_WATCHONLY_P2WPKH,\ - ENGINES, detect_script_type, EngineError + TYPE_WATCHONLY_FIDELITY_BONDS, TYPE_WATCHONLY_TIMELOCK_P2WSH, \ + TYPE_WATCHONLY_P2WPKH, TYPE_P2TR, ENGINES, detect_script_type, EngineError from .support import get_random_bytes from . import mn_encode, mn_decode import jmbitcoin as btc @@ -92,7 +92,7 @@ def estimate_tx_fee(ins, outs, txtype='p2pkh', outtype=None, extra_bytes=0): # See docstring for explanation: if isinstance(txtype, str): - ins = [txtype]* ins + ins = [txtype] * ins else: assert isinstance(txtype, list) ins = txtype @@ -505,6 +505,8 @@ class BaseWallet(object): return 'p2sh-p2wpkh' elif script_type == TYPE_P2WSH: return 'p2wsh' + elif script_type == TYPE_P2TR: + return 'p2tr' # should be unreachable; all possible returns # from detect_script_type are covered. assert False