chore(core): Don't check fairness of mining fees in CoinJoin.

[no changelog]
trezor · Mar 17, 2022 · 9e346b0 · 9e346b0
1 parent 94d1a49
commit 9e346b0
Show file tree

Hide file tree

Showing 3 changed files with 2 additions and 114 deletions.
diff --git a/core/src/apps/bitcoin/sign_tx/approvers.py b/core/src/apps/bitcoin/sign_tx/approvers.py
@@ -319,10 +319,6 @@ async def approve_tx(self, tx_info: TxInfo, orig_txs: list[OriginalTxInfo]) -> N
 
 
 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 @@ async def approve_tx(self, tx_info: TxInfo, orig_txs: list[OriginalTxInfo]) -> N
         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
@@ -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
@@ -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",