From 556e8a147a149e9218aa07cd02d3cfdfdf409408 Mon Sep 17 00:00:00 2001 From: Andrew Kozlik Date: Fri, 25 Feb 2022 16:15:37 +0100 Subject: [PATCH] chore(core): Make CoinJoin checks more lenient. [no changelog] --- core/src/apps/bitcoin/sign_tx/approvers.py | 15 +-- .../bitcoin/test_authorize_coinjoin.py | 117 ------------------ tests/ui_tests/fixtures.json | 1 - 3 files changed, 3 insertions(+), 130 deletions(-) diff --git a/core/src/apps/bitcoin/sign_tx/approvers.py b/core/src/apps/bitcoin/sign_tx/approvers.py index df3c723a0..0882a6e99 100644 --- a/core/src/apps/bitcoin/sign_tx/approvers.py +++ b/core/src/apps/bitcoin/sign_tx/approvers.py @@ -319,7 +319,9 @@ class BasicApprover(Approver): class CoinJoinApprover(Approver): - MAX_OUTPUT_WEIGHT = const(4 * 43) + # 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 @@ -345,9 +347,6 @@ class CoinJoinApprover(Approver): # amount of each output in the current group self.group_amount = 0 - # flag indicating whether our outputs are gaining any anonymity - self.anonymity = False - async def add_internal_input(self, txi: TxInput) -> None: self.our_weight.add_input(txi) if not self.authorization.check_sign_tx_input(txi, self.coin): @@ -420,14 +419,6 @@ class CoinJoinApprover(Approver): if our_fees > our_coordinator_fee + our_max_mining_fee: raise wire.ProcessError("Total fee over threshold") - # Ensure that at least one of the user's outputs is in a group with an external output. - # Note: _get_coordinator_fee() needs to be called before checking this. - if not self.anonymity: - raise wire.ProcessError("No anonymity gain") - - if tx_info.tx.lock_time > 0: - raise wire.ProcessError("nLockTime not allowed in CoinJoin") - if not self.authorization.approve_sign_tx(tx_info.tx, our_fees): raise wire.ProcessError("Fees exceed authorized limit") diff --git a/tests/device_tests/bitcoin/test_authorize_coinjoin.py b/tests/device_tests/bitcoin/test_authorize_coinjoin.py index fe7d7cd88..b730d1496 100644 --- a/tests/device_tests/bitcoin/test_authorize_coinjoin.py +++ b/tests/device_tests/bitcoin/test_authorize_coinjoin.py @@ -345,123 +345,6 @@ def test_unfair_fee(client: Client): ) -def test_no_anonymity(client: Client): - # Test CoinJoin transaction giving the user's outputs no gain in anonymity. - - with client: - btc.authorize_coinjoin( - client, - coordinator="www.example.com", - max_total_fee=5_005, - fee_per_anonymity=5_000_000, # 0.005 % - 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=30_000, - script_type=messages.OutputScriptType.PAYTOWITNESS, - payment_req_index=0, - ), - # Other's coinjoined output. - messages.TxOutputType( - address="tb1q9cqhdr9ydetjzrct6tyeuccws9505hl96azwxk", - amount=30_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 coinjoined output. - messages.TxOutputType( - # tb1qr5p6f5sk09sms57ket074vywfymuthlgud7xyx - address_n=parse_path("m/84h/1h/0h/1/2"), - amount=50_000, - script_type=messages.OutputScriptType.PAYTOWITNESS, - payment_req_index=0, - ), - # Our change output. - messages.TxOutputType( - # tb1qwn0s88t9r39g72m78mcaxj72sy3ct4m404xsmq - address_n=parse_path("m/84h/1h/0h/1/3"), - amount=7_289_000 - 50_000 - 50_000 - 10 - 5_000, - script_type=messages.OutputScriptType.PAYTOWITNESS, - payment_req_index=0, - ), - # Other's change output. - messages.TxOutputType( - address="tb1q9cqhdr9ydetjzrct6tyeuccws9505hl96azwxk", - amount=100_000 - 30_000 - 30_000 - 6 - 5_000, - script_type=messages.OutputScriptType.PAYTOWITNESS, - payment_req_index=0, - ), - # Coordinator's output. - messages.TxOutputType( - address="mvbu1Gdy8SUjTenqerxUaZyYjmveZvt33q", - amount=16, - 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", - "tb1qwn0s88t9r39g72m78mcaxj72sy3ct4m404xsmq", - ], - ) - payment_req.amount = None - - with pytest.raises(TrezorFailure, match="No anonymity gain"): - 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 2c5311820..e5c885452 100644 --- a/tests/ui_tests/fixtures.json +++ b/tests/ui_tests/fixtures.json @@ -602,7 +602,6 @@ "TT_binance-test_sign_tx.py::test_binance_sign_message[message2-expected_response2]": "323e0a474e71ede187ee1332e42952aeca501b42da95f88b2bad5445a3db858c", "TT_bitcoin-test_authorize_coinjoin.py::test_cancel_authorization": "9887e0f4da5c7800f832396e50391beb03229c8edcb5b0e078433703cac6e0d3", "TT_bitcoin-test_authorize_coinjoin.py::test_multisession_authorization": "fd412d086cf4ff677f6ae266e88de725549505d9a2abc1c2ba36f8f854461694", -"TT_bitcoin-test_authorize_coinjoin.py::test_no_anonymity": "f0c8167a4a6aa05751c3eab3d8d944044acec497ca0bf05d2e2fc1577a36c5c4", "TT_bitcoin-test_authorize_coinjoin.py::test_sign_tx": "c48bbbaf032eacb42567e49f5b4b82ed51fe97bb613e165dca2c51207199f236", "TT_bitcoin-test_authorize_coinjoin.py::test_unfair_fee": "ab1aa516510b627b8ffc65391c1c113922ab08f48baf295861a9b597f27f8ea1", "TT_bitcoin-test_authorize_coinjoin.py::test_wrong_coordinator": "9887e0f4da5c7800f832396e50391beb03229c8edcb5b0e078433703cac6e0d3",