From 9e346b05daeffc9a7ecfc40987abf2c53b7c4e39 Mon Sep 17 00:00:00 2001 From: Andrew Kozlik Date: Tue, 15 Mar 2022 19:32:56 +0100 Subject: [PATCH] chore(core): Don't check fairness of mining fees in CoinJoin. [no changelog] --- core/src/apps/bitcoin/sign_tx/approvers.py | 13 +-- .../bitcoin/test_authorize_coinjoin.py | 102 ------------------ tests/ui_tests/fixtures.json | 1 - 3 files changed, 2 insertions(+), 114 deletions(-) diff --git a/core/src/apps/bitcoin/sign_tx/approvers.py b/core/src/apps/bitcoin/sign_tx/approvers.py index bfff96377b..c21943d3ba 100644 --- a/core/src/apps/bitcoin/sign_tx/approvers.py +++ b/core/src/apps/bitcoin/sign_tx/approvers.py @@ -319,10 +319,6 @@ class BasicApprover(Approver): class CoinJoinApprover(Approver): - # Maximum weight of an output for standard scriptPubKeys P2PKH (25), P2SH (23), P2WPKH (22), - # P2WSH (34) and P2TR (34). - MAX_OUTPUT_WEIGHT = const(4 * (8 + 1 + 34)) - def __init__( self, tx: SignTx, coin: CoinInfo, authorization: CoinJoinAuthorization ) -> None: @@ -394,13 +390,8 @@ class CoinJoinApprover(Approver): if mining_fee > max_fee_per_vbyte * self.weight.get_total() / 4: raise wire.ProcessError("Mining fee over threshold") - # The maximum mining fee that the user should be paying assuming that participants share - # the fees for the coordinator's output. - our_max_mining_fee = ( - mining_fee - * self.our_weight.get_total() - / (self.weight.get_total() - self.MAX_OUTPUT_WEIGHT) - ) + # The maximum mining fee that the user should be paying. + our_max_mining_fee = max_fee_per_vbyte * self.our_weight.get_total() / 4 # The maximum coordination fee for the user's inputs. our_max_coordinator_fee = max_coordinator_fee_rate * ( diff --git a/tests/device_tests/bitcoin/test_authorize_coinjoin.py b/tests/device_tests/bitcoin/test_authorize_coinjoin.py index b1f804bbe1..cf3398697d 100644 --- a/tests/device_tests/bitcoin/test_authorize_coinjoin.py +++ b/tests/device_tests/bitcoin/test_authorize_coinjoin.py @@ -245,108 +245,6 @@ def test_sign_tx(client: Client): ) -def test_unfair_fee(client: Client): - # Test unfair mining fee distribution amongst participants. - - with client: - btc.authorize_coinjoin( - client, - coordinator="www.example.com", - max_rounds=1, - max_fee_rate=50_000_000, # 0.5 % - max_fee_per_kvbyte=3500, - n=parse_path("m/84h/1h/0h"), - coin_name="Testnet", - script_type=messages.InputScriptType.SPENDWITNESS, - ) - - inputs = [ - messages.TxInputType( - # seed "alcohol woman abuse must during monitor noble actual mixed trade anger aisle" - # 84'/1'/0'/0/0 - # tb1qnspxpr2xj9s2jt6qlhuvdnxw6q55jvygcf89r2 - amount=100_000, - prev_hash=TXHASH_e5b7e2, - prev_index=0, - script_type=messages.InputScriptType.EXTERNAL, - script_pubkey=bytes.fromhex("00149c02608d469160a92f40fdf8c6ccced029493088"), - ownership_proof=bytearray.fromhex( - "534c001901016b2055d8190244b2ed2d46513c40658a574d3bc2deb6969c0535bb818b44d2c40002483045022100a6c7d59b453efa7b4abc9bc724a94c5655ae986d5924dc29d28bcc2b859cbace022047d2bc4422a47f7b044bd6cdfbf63fe1a0ecbf11393f4c0bf8565f867a5ced16012103505f0d82bbdd251511591b34f36ad5eea37d3220c2b81a1189084431ddb3aa3d" - ), - commitment_data=b"\x0fwww.example.com" + (1).to_bytes(ROUND_ID_LEN, "big"), - ), - messages.TxInputType( - address_n=parse_path("m/84h/1h/0h/1/0"), - amount=7_289_000, - prev_hash=FAKE_TXHASH_f982c0, - prev_index=1, - script_type=messages.InputScriptType.SPENDWITNESS, - ), - ] - - outputs = [ - # Other's coinjoined output. - messages.TxOutputType( - address="tb1qk7j3ahs2v6hrv4v282cf0tvxh0vqq7rpt3zcml", - amount=50_000, - script_type=messages.OutputScriptType.PAYTOWITNESS, - payment_req_index=0, - ), - # Our coinjoined output. - messages.TxOutputType( - # tb1qze76uzqteg6un6jfcryrxhwvfvjj58ts0swg3d - address_n=parse_path("m/84h/1h/0h/1/1"), - amount=50_000, - script_type=messages.OutputScriptType.PAYTOWITNESS, - payment_req_index=0, - ), - # Our change output. - messages.TxOutputType( - # tb1qr5p6f5sk09sms57ket074vywfymuthlgud7xyx - address_n=parse_path("m/84h/1h/0h/1/2"), - amount=7_289_000 - 50_000 - 36_445 - 600, # unfair mining fee - script_type=messages.OutputScriptType.PAYTOWITNESS, - payment_req_index=0, - ), - # Other's change output. - messages.TxOutputType( - address="tb1q9cqhdr9ydetjzrct6tyeuccws9505hl96azwxk", - amount=100_000 - 50_000 - 500 - 400, - script_type=messages.OutputScriptType.PAYTOWITNESS, - payment_req_index=0, - ), - # Coordinator's output. - messages.TxOutputType( - address="mvbu1Gdy8SUjTenqerxUaZyYjmveZvt33q", - amount=36_945, - script_type=messages.OutputScriptType.PAYTOWITNESS, - payment_req_index=0, - ), - ] - - payment_req = make_payment_request( - client, - recipient_name="www.example.com", - outputs=outputs, - change_addresses=[ - "tb1qze76uzqteg6un6jfcryrxhwvfvjj58ts0swg3d", - "tb1qr5p6f5sk09sms57ket074vywfymuthlgud7xyx", - ], - ) - payment_req.amount = None - - with pytest.raises(TrezorFailure, match="fee over threshold"): - btc.sign_tx( - client, - "Testnet", - inputs, - outputs, - prev_txes=TX_CACHE_TESTNET, - payment_reqs=[payment_req], - preauthorized=True, - ) - - def test_wrong_coordinator(client: Client): # Ensure that a preauthorized GetOwnershipProof fails if the commitment_data doesn't match the coordinator. diff --git a/tests/ui_tests/fixtures.json b/tests/ui_tests/fixtures.json index 1b99d31d26..e493781a75 100644 --- a/tests/ui_tests/fixtures.json +++ b/tests/ui_tests/fixtures.json @@ -603,7 +603,6 @@ "TT_bitcoin-test_authorize_coinjoin.py::test_cancel_authorization": "2e5cffe7bd0dc6034852d21612fba8cf1ee3c45a14e76140a4c2786f360f54f0", "TT_bitcoin-test_authorize_coinjoin.py::test_multisession_authorization": "65c9435e10e3dede19169a9b02d373bd5acb22eba0ba3b31324271025d65d5a7", "TT_bitcoin-test_authorize_coinjoin.py::test_sign_tx": "87b81a29fe6e27fdfedfdbb1953b3d0178786749eadbb0fe01509c1af8075de5", -"TT_bitcoin-test_authorize_coinjoin.py::test_unfair_fee": "ab1aa516510b627b8ffc65391c1c113922ab08f48baf295861a9b597f27f8ea1", "TT_bitcoin-test_authorize_coinjoin.py::test_wrong_coordinator": "2e5cffe7bd0dc6034852d21612fba8cf1ee3c45a14e76140a4c2786f360f54f0", "TT_bitcoin-test_bcash.py::test_attack_change_input": "b824d3eb233f6ba2567dd052fa4b52e9a1f170fe4a39af55c1cc262683f188b9", "TT_bitcoin-test_bcash.py::test_send_bch_change": "b824d3eb233f6ba2567dd052fa4b52e9a1f170fe4a39af55c1cc262683f188b9",