From 09f18bb4449cbb1917149c8845923cc8dff28f51 Mon Sep 17 00:00:00 2001 From: Tomas Susanka Date: Thu, 23 Nov 2017 18:40:27 +0100 Subject: [PATCH] wallet: tx weight calculator is used to determine fee --- src/apps/wallet/sign_tx/signing.py | 12 +- ...apps.wallet.segwit.signtx.native_p2wpkh.py | 2 + ...pps.wallet.segwit.signtx.p2wpkh_in_p2sh.py | 4 + .../test_apps.wallet.signtx.fee_threshold.py | 156 ++++++++++++++++++ tests/test_apps.wallet.signtx.py | 1 + 5 files changed, 169 insertions(+), 6 deletions(-) create mode 100644 tests/test_apps.wallet.signtx.fee_threshold.py diff --git a/src/apps/wallet/sign_tx/signing.py b/src/apps/wallet/sign_tx/signing.py index 1f7c5863f..fcfd41fc7 100644 --- a/src/apps/wallet/sign_tx/signing.py +++ b/src/apps/wallet/sign_tx/signing.py @@ -15,6 +15,7 @@ from apps.wallet.sign_tx.helpers import * from apps.wallet.sign_tx.segwit_bip143 import * from apps.wallet.sign_tx.scripts import * from apps.wallet.sign_tx.writers import * +from apps.wallet.sign_tx.tx_weight_calculator import * # the number of bip32 levels used in a wallet (chain and address) _BIP32_WALLET_DEPTH = const(2) @@ -51,6 +52,7 @@ async def check_tx_fee(tx: SignTx, root): # tx, as the SignTx info is streamed only once h_first = HashWriter(sha256) # not a real tx hash bip143 = Bip143() + weight = TxWeightCalculator(tx.inputs_count, tx.outputs_count) txo_bin = TxOutputBinType() tx_req = TxRequest() @@ -67,6 +69,7 @@ async def check_tx_fee(tx: SignTx, root): txi = await request_tx_input(tx_req, i) wallet_path = input_extract_wallet_path(txi, wallet_path) write_tx_input_check(h_first, txi) + weight.add_input(txi) bip143.add_prevouts(txi) bip143.add_sequence(txi) is_segwit = (txi.script_type == InputScriptType.SPENDWITNESS or @@ -93,6 +96,7 @@ async def check_tx_fee(tx: SignTx, root): txo = await request_tx_output(tx_req, o) txo_bin.amount = txo.amount txo_bin.script_pubkey = output_derive_script(txo, coin, root) + weight.add_output(txo_bin.script_pubkey) if output_is_change(txo, wallet_path): if change_out != 0: raise SigningError(FailureType.ProcessError, @@ -110,8 +114,8 @@ async def check_tx_fee(tx: SignTx, root): raise SigningError(FailureType.NotEnoughFunds, 'Not enough funds') - tx_size_b = estimate_tx_size(tx.inputs_count, tx.outputs_count) - if fee > coin.maxfee_kb * ((tx_size_b + 999) // 1000): + # fee > (coin.maxfee per byte * tx size) + if fee > (coin.maxfee_kb / 1000) * (weight.get_total() / 4): if not await confirm_feeoverthreshold(fee, coin): raise SigningError(FailureType.ActionCancelled, 'Signing cancelled') @@ -320,10 +324,6 @@ async def get_prevtx_output_value(tx_req: TxRequest, prev_hash: bytes, prev_inde return total_out -def estimate_tx_size(inputs: int, outputs: int) -> int: - return 10 + inputs * 149 + outputs * 35 - - # TX Helpers # === diff --git a/tests/test_apps.wallet.segwit.signtx.native_p2wpkh.py b/tests/test_apps.wallet.segwit.signtx.native_p2wpkh.py index a833844d8..b28507e45 100644 --- a/tests/test_apps.wallet.segwit.signtx.native_p2wpkh.py +++ b/tests/test_apps.wallet.segwit.signtx.native_p2wpkh.py @@ -36,6 +36,7 @@ class TestSignSegwitTxNativeP2WPKH(unittest.TestCase): prev_index=0, script_type=InputScriptType.SPENDWITNESS, sequence=0xffffffff, + multisig=None, ) out1 = TxOutputType( address='2N4Q5FhU2497BryFfUgbqkAJE87aKHUhXMp', @@ -128,6 +129,7 @@ class TestSignSegwitTxNativeP2WPKH(unittest.TestCase): prev_index=0, script_type=InputScriptType.SPENDWITNESS, sequence=0xffffffff, + multisig=None, ) out1 = TxOutputType( address='2N4Q5FhU2497BryFfUgbqkAJE87aKHUhXMp', diff --git a/tests/test_apps.wallet.segwit.signtx.p2wpkh_in_p2sh.py b/tests/test_apps.wallet.segwit.signtx.p2wpkh_in_p2sh.py index 83fe499e3..f61bd39db 100644 --- a/tests/test_apps.wallet.segwit.signtx.p2wpkh_in_p2sh.py +++ b/tests/test_apps.wallet.segwit.signtx.p2wpkh_in_p2sh.py @@ -36,6 +36,7 @@ class TestSignSegwitTxP2WPKHInP2SH(unittest.TestCase): prev_index=0, script_type=InputScriptType.SPENDP2SHWITNESS, sequence=0xffffffff, + multisig=None, ) out1 = TxOutputType( address='mhRx1CeVfaayqRwq5zgRQmD7W5aWBfD5mC', @@ -128,6 +129,7 @@ class TestSignSegwitTxP2WPKHInP2SH(unittest.TestCase): prev_index=0, script_type=InputScriptType.SPENDP2SHWITNESS, sequence=0xffffffff, + multisig=None, ) out1 = TxOutputType( address='mhRx1CeVfaayqRwq5zgRQmD7W5aWBfD5mC', @@ -230,6 +232,7 @@ class TestSignSegwitTxP2WPKHInP2SH(unittest.TestCase): prev_index=0, script_type=InputScriptType.SPENDP2SHWITNESS, sequence=0xffffffff, + multisig=None, ) inpattack = TxInputType( # 49'/1'/0'/1/0" - 2N1LGaGg836mqSQqiuUBLfcyGBhyZbremDX @@ -239,6 +242,7 @@ class TestSignSegwitTxP2WPKHInP2SH(unittest.TestCase): prev_index=0, script_type=InputScriptType.SPENDP2SHWITNESS, sequence=0xffffffff, + multisig=None, ) out1 = TxOutputType( address='mhRx1CeVfaayqRwq5zgRQmD7W5aWBfD5mC', diff --git a/tests/test_apps.wallet.signtx.fee_threshold.py b/tests/test_apps.wallet.signtx.fee_threshold.py new file mode 100644 index 000000000..511ba6ed8 --- /dev/null +++ b/tests/test_apps.wallet.signtx.fee_threshold.py @@ -0,0 +1,156 @@ +from common import * + +from trezor.utils import chunks +from trezor.crypto import bip32, bip39 +from trezor.messages.SignTx import SignTx +from trezor.messages.TxInputType import TxInputType +from trezor.messages.TxOutputType import TxOutputType +from trezor.messages.TxOutputBinType import TxOutputBinType +from trezor.messages.TxRequest import TxRequest +from trezor.messages.TxAck import TxAck +from trezor.messages.TransactionType import TransactionType +from trezor.messages.RequestType import TXINPUT, TXOUTPUT, TXMETA, TXFINISHED +from trezor.messages.TxRequestDetailsType import TxRequestDetailsType +from trezor.messages import OutputScriptType + +from apps.common import coins +from apps.wallet.sign_tx import signing + + +class TestSignTxFeeThreshold(unittest.TestCase): + # pylint: disable=C0301 + + def test_over_fee_threshold(self): + coin_bitcoin = coins.by_name('Bitcoin') + + ptx1 = TransactionType(version=1, lock_time=0, inputs_cnt=2, outputs_cnt=1) + pinp1 = TxInputType(script_sig=unhexlify('483045022072ba61305fe7cb542d142b8f3299a7b10f9ea61f6ffaab5dca8142601869d53c0221009a8027ed79eb3b9bc13577ac2853269323434558528c6b6a7e542be46e7e9a820141047a2d177c0f3626fc68c53610b0270fa6156181f46586c679ba6a88b34c6f4874686390b4d92e5769fbb89c8050b984f4ec0b257a0e5c4ff8bd3b035a51709503'), + prev_hash=unhexlify('c16a03f1cf8f99f6b5297ab614586cacec784c2d259af245909dedb0e39eddcf'), + prev_index=1, + script_type=None, + sequence=None) + pinp2 = TxInputType(script_sig=unhexlify('48304502200fd63adc8f6cb34359dc6cca9e5458d7ea50376cbd0a74514880735e6d1b8a4c0221008b6ead7fe5fbdab7319d6dfede3a0bc8e2a7c5b5a9301636d1de4aa31a3ee9b101410486ad608470d796236b003635718dfc07c0cac0cfc3bfc3079e4f491b0426f0676e6643a39198e8e7bdaffb94f4b49ea21baa107ec2e237368872836073668214'), + prev_hash=unhexlify('1ae39a2f8d59670c8fc61179148a8e61e039d0d9e8ab08610cb69b4a19453eaf'), + prev_index=1, + script_type=None, + sequence=None) + pout1 = TxOutputBinType(script_pubkey=unhexlify('76a91424a56db43cf6f2b02e838ea493f95d8d6047423188ac'), + amount=390000, + address_n=None) + + inp1 = TxInputType(address_n=[0], # 14LmW5k4ssUrtbAB4255zdqv3b4w1TuX9e + # amount=390000, + prev_hash=unhexlify('d5f65ee80147b4bcc70b75e4bbf2d7382021b871bd8867ef8fa525ef50864882'), + prev_index=0, + amount=None, + script_type=None, + multisig=None, + sequence=None) + out1 = TxOutputType(address='1MJ2tj2ThBE62zXbBYA5ZaN3fdve5CPAz1', + amount=390000 - 100000, # fee increased to 100000 => too high + script_type=OutputScriptType.PAYTOADDRESS, + address_n=None) + tx = SignTx(coin_name=None, version=None, lock_time=None, inputs_count=1, outputs_count=1) + + messages = [ + None, + TxRequest(request_type=TXINPUT, details=TxRequestDetailsType(request_index=0, tx_hash=None)), + TxAck(tx=TransactionType(inputs=[inp1])), + TxRequest(request_type=TXMETA, details=TxRequestDetailsType(request_index=None, tx_hash=unhexlify('d5f65ee80147b4bcc70b75e4bbf2d7382021b871bd8867ef8fa525ef50864882')), serialized=None), + TxAck(tx=ptx1), + TxRequest(request_type=TXINPUT, details=TxRequestDetailsType(request_index=0, tx_hash=unhexlify('d5f65ee80147b4bcc70b75e4bbf2d7382021b871bd8867ef8fa525ef50864882')), serialized=None), + TxAck(tx=TransactionType(inputs=[pinp1])), + TxRequest(request_type=TXINPUT, details=TxRequestDetailsType(request_index=1, tx_hash=unhexlify('d5f65ee80147b4bcc70b75e4bbf2d7382021b871bd8867ef8fa525ef50864882')), serialized=None), + TxAck(tx=TransactionType(inputs=[pinp2])), + TxRequest(request_type=TXOUTPUT, details=TxRequestDetailsType(request_index=0, tx_hash=unhexlify('d5f65ee80147b4bcc70b75e4bbf2d7382021b871bd8867ef8fa525ef50864882')), serialized=None), + TxAck(tx=TransactionType(bin_outputs=[pout1])), + TxRequest(request_type=TXOUTPUT, details=TxRequestDetailsType(request_index=0, tx_hash=None), serialized=None), + TxAck(tx=TransactionType(outputs=[out1])), + signing.UiConfirmOutput(out1, coin_bitcoin), + True, + signing.UiConfirmFeeOverThreshold(100000, coin_bitcoin), + True, + signing.UiConfirmTotal(290000, 100000, coin_bitcoin), + True, + TxRequest(request_type=TXINPUT, details=TxRequestDetailsType(request_index=0, tx_hash=None), serialized=None), + ] + + seed = bip39.seed('alcohol woman abuse must during monitor noble actual mixed trade anger aisle', '') + root = bip32.from_seed(seed, 'secp256k1') + + signer = signing.sign_tx(tx, root) + for request, response in chunks(messages, 2): + self.assertEqualEx(signer.send(request), response) + + def test_under_threshold(self): + coin_bitcoin = coins.by_name('Bitcoin') + + ptx1 = TransactionType(version=1, lock_time=0, inputs_cnt=2, outputs_cnt=1) + pinp1 = TxInputType(script_sig=unhexlify('483045022072ba61305fe7cb542d142b8f3299a7b10f9ea61f6ffaab5dca8142601869d53c0221009a8027ed79eb3b9bc13577ac2853269323434558528c6b6a7e542be46e7e9a820141047a2d177c0f3626fc68c53610b0270fa6156181f46586c679ba6a88b34c6f4874686390b4d92e5769fbb89c8050b984f4ec0b257a0e5c4ff8bd3b035a51709503'), + prev_hash=unhexlify('c16a03f1cf8f99f6b5297ab614586cacec784c2d259af245909dedb0e39eddcf'), + prev_index=1, + script_type=None, + sequence=None) + pinp2 = TxInputType(script_sig=unhexlify('48304502200fd63adc8f6cb34359dc6cca9e5458d7ea50376cbd0a74514880735e6d1b8a4c0221008b6ead7fe5fbdab7319d6dfede3a0bc8e2a7c5b5a9301636d1de4aa31a3ee9b101410486ad608470d796236b003635718dfc07c0cac0cfc3bfc3079e4f491b0426f0676e6643a39198e8e7bdaffb94f4b49ea21baa107ec2e237368872836073668214'), + prev_hash=unhexlify('1ae39a2f8d59670c8fc61179148a8e61e039d0d9e8ab08610cb69b4a19453eaf'), + prev_index=1, + script_type=None, + sequence=None) + pout1 = TxOutputBinType(script_pubkey=unhexlify('76a91424a56db43cf6f2b02e838ea493f95d8d6047423188ac'), + amount=390000, + address_n=None) + + inp1 = TxInputType(address_n=[0], # 14LmW5k4ssUrtbAB4255zdqv3b4w1TuX9e + # amount=390000, + prev_hash=unhexlify('d5f65ee80147b4bcc70b75e4bbf2d7382021b871bd8867ef8fa525ef50864882'), + prev_index=0, + amount=None, + multisig=None, + script_type=None, + sequence=None) + out1 = TxOutputType(address='1MJ2tj2ThBE62zXbBYA5ZaN3fdve5CPAz1', + amount=390000 - 90000, # fee increased to 90000, slightly less than the threshold + script_type=OutputScriptType.PAYTOADDRESS, + address_n=None) + tx = SignTx(coin_name=None, version=None, lock_time=None, inputs_count=1, outputs_count=1) + + messages = [ + None, + TxRequest(request_type=TXINPUT, details=TxRequestDetailsType(request_index=0, tx_hash=None)), + TxAck(tx=TransactionType(inputs=[inp1])), + TxRequest(request_type=TXMETA, details=TxRequestDetailsType(request_index=None, tx_hash=unhexlify('d5f65ee80147b4bcc70b75e4bbf2d7382021b871bd8867ef8fa525ef50864882')), serialized=None), + TxAck(tx=ptx1), + TxRequest(request_type=TXINPUT, details=TxRequestDetailsType(request_index=0, tx_hash=unhexlify('d5f65ee80147b4bcc70b75e4bbf2d7382021b871bd8867ef8fa525ef50864882')), serialized=None), + TxAck(tx=TransactionType(inputs=[pinp1])), + TxRequest(request_type=TXINPUT, details=TxRequestDetailsType(request_index=1, tx_hash=unhexlify('d5f65ee80147b4bcc70b75e4bbf2d7382021b871bd8867ef8fa525ef50864882')), serialized=None), + TxAck(tx=TransactionType(inputs=[pinp2])), + TxRequest(request_type=TXOUTPUT, details=TxRequestDetailsType(request_index=0, tx_hash=unhexlify('d5f65ee80147b4bcc70b75e4bbf2d7382021b871bd8867ef8fa525ef50864882')), serialized=None), + TxAck(tx=TransactionType(bin_outputs=[pout1])), + TxRequest(request_type=TXOUTPUT, details=TxRequestDetailsType(request_index=0, tx_hash=None), serialized=None), + TxAck(tx=TransactionType(outputs=[out1])), + signing.UiConfirmOutput(out1, coin_bitcoin), + True, + signing.UiConfirmTotal(300000, 90000, coin_bitcoin), + True, + TxRequest(request_type=TXINPUT, details=TxRequestDetailsType(request_index=0, tx_hash=None), serialized=None), + ] + + seed = bip39.seed('alcohol woman abuse must during monitor noble actual mixed trade anger aisle', '') + root = bip32.from_seed(seed, 'secp256k1') + + signer = signing.sign_tx(tx, root) + for request, response in chunks(messages, 2): + self.assertEqualEx(signer.send(request), response) + + def assertEqualEx(self, a, b): + # hack to avoid adding __eq__ to signing.Ui* classes + if ((isinstance(a, signing.UiConfirmOutput) and isinstance(b, signing.UiConfirmOutput)) or + (isinstance(a, signing.UiConfirmTotal) and isinstance(b, signing.UiConfirmTotal)) or + (isinstance(a, signing.UiConfirmFeeOverThreshold) and isinstance(b, signing.UiConfirmFeeOverThreshold))): + return self.assertEqual(a.__dict__, b.__dict__) + else: + return self.assertEqual(a, b) + + +if __name__ == '__main__': + unittest.main() diff --git a/tests/test_apps.wallet.signtx.py b/tests/test_apps.wallet.signtx.py index b5957f9da..4f080ea42 100644 --- a/tests/test_apps.wallet.signtx.py +++ b/tests/test_apps.wallet.signtx.py @@ -47,6 +47,7 @@ class TestSignTx(unittest.TestCase): prev_index=0, amount=None, script_type=None, + multisig=None, sequence=None) out1 = TxOutputType(address='1MJ2tj2ThBE62zXbBYA5ZaN3fdve5CPAz1', amount=390000 - 10000,