From 1f20c4953624997e6e3d98e1fa8c9201b43204ac Mon Sep 17 00:00:00 2001 From: David Misiak Date: Tue, 15 Feb 2022 15:14:28 +0100 Subject: [PATCH] feat(cardano): allow device-owned outputs in plutus txs --- common/tests/fixtures/cardano/sign_tx.json | 51 +++++- .../cardano/sign_tx.plutus.failed.json | 8 +- .../fixtures/cardano/sign_tx.plutus.json | 150 ++++++++++++++++++ core/src/apps/cardano/address.py | 13 +- core/src/apps/cardano/get_address.py | 4 +- core/src/apps/cardano/layout.py | 42 +++-- core/src/apps/cardano/sign_tx.py | 58 ++++--- 7 files changed, 282 insertions(+), 44 deletions(-) diff --git a/common/tests/fixtures/cardano/sign_tx.json b/common/tests/fixtures/cardano/sign_tx.json index 8a5753d39..34e5458b0 100644 --- a/common/tests/fixtures/cardano/sign_tx.json +++ b/common/tests/fixtures/cardano/sign_tx.json @@ -221,7 +221,7 @@ } }, { - "description": "simple transaction with base script address change output", + "description": "simple transaction with base script address and change output", "parameters": { "protocol_magic": 764824073, "network_id": 1, @@ -418,6 +418,55 @@ ] } }, + { + "description": "simple transaction with base address change output with staking script hash", + "parameters": { + "protocol_magic": 764824073, + "network_id": 1, + "fee": 42, + "ttl": 10, + "certificates": [], + "withdrawals": [], + "auxiliary_data": null, + "inputs": [ + { + "path": "m/1852'/1815'/0'/0/0", + "prev_hash": "3b40265111d8bb3c3c608d95b3a0bf83461ace32d79336579a1939b3aad1c0b7", + "prev_index": 0 + } + ], + "outputs": [ + { + "address": "addr1q84sh2j72ux0l03fxndjnhctdg7hcppsaejafsa84vh7lwgmcs5wgus8qt4atk45lvt4xfxpjtwfhdmvchdf2m3u3hlsd5tq5r", + "amount": "1" + }, + { + "addressType": 2, + "path": "m/1852'/1815'/0'/0/0", + "scriptStakingHash": "8d7bebc7a58f1c7b5fb7c9391071ecd3b51b032695522f8c555343a9", + "amount": "7120787" + } + ], + "mint": [], + "script_data_hash": null, + "collateral_inputs": [], + "required_signers": [], + "signing_mode": "ORDINARY_TRANSACTION", + "additional_witness_requests": [], + "include_network_id": false + }, + "result": { + "tx_hash": "1d6fc2044d54d4af5b44e4be4c1ce3670fe4d2e37523a945d087252c27b215f2", + "witnesses": [ + { + "type": 1, + "pub_key": "5d010cf16fdeff40955633d6c565f3844a288a24967cf6b76acbeb271b4f13c1", + "signature": "d301e21dccbc0cf5122629476795f993eb14160ca22a590d5e1ac3bf6996f3bd35c83646c1c8eacbb2dac3367f27e057a340aa19e77e008851274fdb19ab000f", + "chain_code": null + } + ] + } + }, { "description": "simple transaction with pointer address change output", "parameters": { diff --git a/common/tests/fixtures/cardano/sign_tx.plutus.failed.json b/common/tests/fixtures/cardano/sign_tx.plutus.failed.json index df42a175f..929fe0107 100644 --- a/common/tests/fixtures/cardano/sign_tx.plutus.failed.json +++ b/common/tests/fixtures/cardano/sign_tx.plutus.failed.json @@ -46,7 +46,7 @@ } }, { - "description": "Plutus transaction with output containing address parameters", + "description": "Plutus transaction with output containing forbidden address parameters", "parameters": { "protocol_magic": 764824073, "network_id": 1, @@ -63,8 +63,8 @@ ], "outputs": [ { - "addressType": 0, - "path": "m/1852'/1815'/0'/0/0", + "addressType": 1, + "scriptPaymentHash": "8d7bebc7a58f1c7b5fb7c9391071ecd3b51b032695522f8c555343a9", "stakingPath": "m/1852'/1815'/0'/2/0", "amount": "7120787" } @@ -84,7 +84,7 @@ "include_network_id": false }, "result": { - "error_message": "Invalid output" + "error_message": "Invalid address parameters" } }, { diff --git a/common/tests/fixtures/cardano/sign_tx.plutus.json b/common/tests/fixtures/cardano/sign_tx.plutus.json index e1b127e93..e20ef6e21 100644 --- a/common/tests/fixtures/cardano/sign_tx.plutus.json +++ b/common/tests/fixtures/cardano/sign_tx.plutus.json @@ -943,6 +943,156 @@ } ] } + }, + { + "description": "Plutus transaction with base address device-owned output", + "parameters": { + "protocol_magic": 764824073, + "network_id": 1, + "fee": 42, + "ttl": 10, + "certificates": [], + "withdrawals": [], + "auxiliary_data": null, + "inputs": [ + { + "prev_hash": "3b40265111d8bb3c3c608d95b3a0bf83461ace32d79336579a1939b3aad1c0b7", + "prev_index": 0 + } + ], + "outputs": [ + { + "addressType": 0, + "path": "m/1852'/1815'/0'/0/0", + "stakingPath": "m/1852'/1815'/0'/2/0", + "amount": "7120787" + } + ], + "mint": [], + "script_data_hash": "d593fd793c377ac50a3169bb8378ffc257c944da31aa8f355dfa5a4f6ff89e02", + "collateral_inputs": [ + { + "path": "m/1852'/1815'/0'/0/0", + "prev_hash": "1af8fa0b754ff99253d983894e63a2b09cbb56c833ba18c3384210163f63dcfc", + "prev_index": 0 + } + ], + "required_signers": [], + "signing_mode": "PLUTUS_TRANSACTION", + "additional_witness_requests": [], + "include_network_id": false + }, + "result": { + "tx_hash": "5581fb7a99b8e0ebaf552a5d7157cebb37c9624602d78d86337dfbf838fb2e13", + "witnesses": [ + { + "type": 1, + "pub_key": "5d010cf16fdeff40955633d6c565f3844a288a24967cf6b76acbeb271b4f13c1", + "signature": "9f5432c85adaf7191e77388a75ff4ee57ce86b382e99b561e1e2dbcb263732bfcc5a726f268d3a5833e37468e1c7baa64ffd88044b59b3f7b67d59502108be0c", + "chain_code": null + } + ] + } + }, + { + "description": "Plutus transaction with base address device-owned output with path mismatch", + "parameters": { + "protocol_magic": 764824073, + "network_id": 1, + "fee": 42, + "ttl": 10, + "certificates": [], + "withdrawals": [], + "auxiliary_data": null, + "inputs": [ + { + "prev_hash": "3b40265111d8bb3c3c608d95b3a0bf83461ace32d79336579a1939b3aad1c0b7", + "prev_index": 0 + } + ], + "outputs": [ + { + "addressType": 0, + "path": "m/1852'/1815'/0'/0/0", + "stakingPath": "m/1852'/1815'/1'/2/0", + "amount": "7120787" + } + ], + "mint": [], + "script_data_hash": "d593fd793c377ac50a3169bb8378ffc257c944da31aa8f355dfa5a4f6ff89e02", + "collateral_inputs": [ + { + "path": "m/1852'/1815'/0'/0/0", + "prev_hash": "1af8fa0b754ff99253d983894e63a2b09cbb56c833ba18c3384210163f63dcfc", + "prev_index": 0 + } + ], + "required_signers": [], + "signing_mode": "PLUTUS_TRANSACTION", + "additional_witness_requests": [], + "include_network_id": false + }, + "result": { + "tx_hash": "e5beff2153154ab3dba4ff5c638f40ae8c1594c67aee3310bd19997f5013e47b", + "witnesses": [ + { + "type": 1, + "pub_key": "5d010cf16fdeff40955633d6c565f3844a288a24967cf6b76acbeb271b4f13c1", + "signature": "29032600142f199c144a6270fb64632468e6ca4ed217e6b5d5b459e8fa41c3f91a6fc73d3cf14a609e9edd863a485ceab549200daa8f50926cf16e1526dc220f", + "chain_code": null + } + ] + } + }, + { + "description": "Plutus transaction with BASE_KEY_SCRIPT address device-owned output", + "parameters": { + "protocol_magic": 764824073, + "network_id": 1, + "fee": 42, + "ttl": 10, + "certificates": [], + "withdrawals": [], + "auxiliary_data": null, + "inputs": [ + { + "prev_hash": "3b40265111d8bb3c3c608d95b3a0bf83461ace32d79336579a1939b3aad1c0b7", + "prev_index": 0 + } + ], + "outputs": [ + { + "addressType": 2, + "path": "m/1852'/1815'/0'/0/0", + "scriptStakingHash": "8d7bebc7a58f1c7b5fb7c9391071ecd3b51b032695522f8c555343a9", + "amount": "7120787" + } + ], + "mint": [], + "script_data_hash": "d593fd793c377ac50a3169bb8378ffc257c944da31aa8f355dfa5a4f6ff89e02", + "collateral_inputs": [ + { + "path": "m/1852'/1815'/0'/0/0", + "prev_hash": "1af8fa0b754ff99253d983894e63a2b09cbb56c833ba18c3384210163f63dcfc", + "prev_index": 0 + } + ], + "required_signers": [], + "signing_mode": "PLUTUS_TRANSACTION", + "additional_witness_requests": [], + "include_network_id": false + }, + "result": { + "tx_hash": "8c131e38f378d141deb05d4bf2eb2d13bb75d6363dd470a1f524ba027eff566a", + "witnesses": [ + { + "type": 1, + "pub_key": "5d010cf16fdeff40955633d6c565f3844a288a24967cf6b76acbeb271b4f13c1", + "signature": "e82967f5030f8d2c8b986f75e5744fa92e77444f0eff4aa440492c6aad0b0639ada1501fcbb01db5cfb4b39333aff96362adec4620db6f7f9771039600b24300", + "chain_code": null + } + ] + } } ] } diff --git a/core/src/apps/cardano/address.py b/core/src/apps/cardano/address.py index c278d1d0a..a3733bb46 100644 --- a/core/src/apps/cardano/address.py +++ b/core/src/apps/cardano/address.py @@ -223,13 +223,12 @@ def validate_output_address_parameters( ) -> None: validate_address_parameters(parameters) - if parameters.address_type in ( - CardanoAddressType.BASE_SCRIPT_KEY, - CardanoAddressType.BASE_SCRIPT_SCRIPT, - CardanoAddressType.POINTER_SCRIPT, - CardanoAddressType.ENTERPRISE_SCRIPT, - CardanoAddressType.REWARD, - CardanoAddressType.REWARD_SCRIPT, + if parameters.address_type not in ( + CardanoAddressType.BASE, + CardanoAddressType.BASE_KEY_SCRIPT, + CardanoAddressType.POINTER, + CardanoAddressType.ENTERPRISE, + CardanoAddressType.BYRON, ): # Change outputs with script payment part are forbidden. # Reward addresses are forbidden as outputs in general, see also validate_output_address diff --git a/core/src/apps/cardano/get_address.py b/core/src/apps/cardano/get_address.py index 14e12b165..641c35b01 100644 --- a/core/src/apps/cardano/get_address.py +++ b/core/src/apps/cardano/get_address.py @@ -6,7 +6,7 @@ from trezor.messages import CardanoAddress from . import seed from .address import derive_human_readable_address, validate_address_parameters from .helpers.credential import Credential, should_show_address_credentials -from .layout import show_cardano_address, show_credentials +from .layout import show_address_credentials, show_cardano_address from .sign_tx import validate_network_info if TYPE_CHECKING: @@ -47,7 +47,7 @@ async def _display_address( protocol_magic: int, ) -> None: if should_show_address_credentials(address_parameters): - await show_credentials( + await show_address_credentials( ctx, Credential.payment_credential(address_parameters), Credential.stake_credential(address_parameters), diff --git a/core/src/apps/cardano/layout.py b/core/src/apps/cardano/layout.py index b03b93a24..66d759f56 100644 --- a/core/src/apps/cardano/layout.py +++ b/core/src/apps/cardano/layout.py @@ -271,22 +271,45 @@ async def confirm_sending_token( ) -async def show_credentials( +async def show_address_credentials( ctx: wire.Context, payment_credential: Credential, stake_credential: Credential, - is_change_output: bool = False, ) -> None: - await _show_credential(ctx, payment_credential, is_change_output) - await _show_credential(ctx, stake_credential, is_change_output) + intro_text = "Address" + await _show_credential(ctx, payment_credential, intro_text, is_output=False) + await _show_credential(ctx, stake_credential, intro_text, is_output=False) + + +async def show_change_output_credentials( + ctx: wire.Context, + payment_credential: Credential, + stake_credential: Credential, +) -> None: + intro_text = "The following address is a change address. Its" + await _show_credential(ctx, payment_credential, intro_text, is_output=True) + await _show_credential(ctx, stake_credential, intro_text, is_output=True) + + +async def show_device_owned_output_credentials( + ctx: wire.Context, + payment_credential: Credential, + stake_credential: Credential, + show_both_credentials: bool, +) -> None: + intro_text = "The following address is owned by this device. Its" + await _show_credential(ctx, payment_credential, intro_text, is_output=True) + if show_both_credentials: + await _show_credential(ctx, stake_credential, intro_text, is_output=True) async def _show_credential( ctx: wire.Context, credential: Credential, - is_change_output: bool = False, + intro_text: str, + is_output: bool, ) -> None: - if is_change_output: + if is_output: title = "Confirm transaction" else: title = f"{ADDRESS_TYPE_NAMES[credential.address_type]} address" @@ -297,15 +320,10 @@ async def _show_credential( # and reward address payment credential. In that case we don't want to # show some of the "props". if credential.is_set(): - if is_change_output: - address_usage = "Change address" - else: - address_usage = "Address" - credential_title = credential.get_title() props.append( ( - f"{address_usage} {credential.type_name} credential is a {credential_title}:", + f"{intro_text} {credential.type_name} credential is a {credential_title}:", None, ) ) diff --git a/core/src/apps/cardano/sign_tx.py b/core/src/apps/cardano/sign_tx.py index 9525fa1af..51494ef24 100644 --- a/core/src/apps/cardano/sign_tx.py +++ b/core/src/apps/cardano/sign_tx.py @@ -118,7 +118,8 @@ from .layout import ( confirm_transaction, confirm_withdrawal, confirm_witness_request, - show_credentials, + show_change_output_credentials, + show_device_owned_output_credentials, show_transaction_signing_mode, show_warning_no_collateral_inputs, show_warning_no_script_data_hash, @@ -444,6 +445,7 @@ async def _process_outputs( output, protocol_magic, network_id, + signing_mode, ) output_address = _get_output_address( @@ -886,8 +888,12 @@ def _validate_output( raise INVALID_OUTPUT if address_parameters := output.address_parameters: - if signing_mode != CardanoTxSigningMode.ORDINARY_TRANSACTION: - # change outputs are allowed only in ORDINARY_TRANSACTION + if signing_mode not in ( + CardanoTxSigningMode.ORDINARY_TRANSACTION, + CardanoTxSigningMode.PLUTUS_TRANSACTION, + ): + # Change outputs are allowed only in ORDINARY_TRANSACTION. + # In PLUTUS_TRANSACTION, we display device-owned outputs similarly to change outputs. raise INVALID_OUTPUT validate_output_address_parameters(address_parameters) @@ -927,6 +933,11 @@ def _should_show_output( # Plutus script address without a datum hash is unspendable, we must show a warning return True + if signing_mode == CardanoTxSigningMode.PLUTUS_TRANSACTION: + # In Plutus transactions, all outputs need to be shown (even device-owned), because they + # might influence the script evaluation. + return True + if signing_mode == CardanoTxSigningMode.POOL_REGISTRATION_AS_OWNER: # In a pool registration transaction, there are no inputs belonging to the user # and no spending witnesses. It is thus safe to not show the outputs. @@ -946,34 +957,45 @@ async def _show_output( output: CardanoTxOutput, protocol_magic: int, network_id: int, + signing_mode: CardanoTxSigningMode, ) -> None: if output.datum_hash: await show_warning_tx_output_contains_datum_hash(ctx, output.datum_hash) - if output.asset_groups_count > 0: - await show_warning_tx_output_contains_tokens(ctx) - address_type = _get_output_address_type(output) if output.datum_hash is None and address_type in ADDRESS_TYPES_PAYMENT_SCRIPT: await show_warning_tx_output_no_datum_hash(ctx) - is_change_output: bool - if address_parameters := output.address_parameters: - is_change_output = True - - await show_credentials( - ctx, - Credential.payment_credential(address_parameters), - Credential.stake_credential(address_parameters), - is_change_output=True, - ) + if output.asset_groups_count > 0: + await show_warning_tx_output_contains_tokens(ctx) + is_change_output = False + if address_parameters := output.address_parameters: address = derive_human_readable_address( keychain, address_parameters, protocol_magic, network_id ) - else: - is_change_output = False + payment_credential = Credential.payment_credential(address_parameters) + stake_credential = Credential.stake_credential(address_parameters) + if signing_mode == CardanoTxSigningMode.PLUTUS_TRANSACTION: + show_both_credentials = should_show_address_credentials(address_parameters) + # In ORDINARY_TRANSACTION, change outputs with matching payment and staking paths can + # be hidden, but we need to show them in PLUTUS_TRANSACTION because of the script + # evaluation. We at least hide the staking path if it matches the payment path. + await show_device_owned_output_credentials( + ctx, + payment_credential, + stake_credential, + show_both_credentials, + ) + else: + is_change_output = True + await show_change_output_credentials( + ctx, + payment_credential, + stake_credential, + ) + else: assert output.address is not None # _validate_output address = output.address