From bcaa9444a04bca31a3f206e0c69b88af4ad2bb3b Mon Sep 17 00:00:00 2001 From: gabrielkerekes Date: Wed, 6 Dec 2023 17:04:45 +0100 Subject: [PATCH] fix(solana): fix Memo, multisig and polish instructions UI --- core/src/apps/solana/layout.py | 15 ++-- .../src/apps/solana/predefined_transaction.py | 4 + core/src/apps/solana/transaction/__init__.py | 2 +- .../apps/solana/transaction/instructions.py | 87 ++++++++++++------- .../src/apps/solana/transaction/programs.json | 78 ++++++++++------- 5 files changed, 112 insertions(+), 74 deletions(-) diff --git a/core/src/apps/solana/layout.py b/core/src/apps/solana/layout.py index c083923d1..ffe779c05 100644 --- a/core/src/apps/solana/layout.py +++ b/core/src/apps/solana/layout.py @@ -59,6 +59,14 @@ async def confirm_instruction( br_code=ButtonRequestType.Other, ) + if instruction.multisig_signers: + await confirm_metadata( + "confirm_multisig", + "Confirm multisig", + "The following instruction is a multisig instruction.", + br_code=ButtonRequestType.Other, + ) + for ui_property in instruction.ui_properties: if ui_property.parameter is not None: property_template = instruction.get_property_template(ui_property.parameter) @@ -124,13 +132,6 @@ async def confirm_instruction( raise ValueError # Invalid ui property if instruction.multisig_signers: - await confirm_metadata( - "confirm_multisig", - "Confirm multisig", - "The following instruction is a multisig instruction.", - br_code=ButtonRequestType.Other, - ) - signers: list[tuple[str, str]] = [] for i, multisig_signer in enumerate(instruction.multisig_signers, 1): multisig_signer_public_key = multisig_signer[0] diff --git a/core/src/apps/solana/predefined_transaction.py b/core/src/apps/solana/predefined_transaction.py index eb25c2e66..aeac58a67 100644 --- a/core/src/apps/solana/predefined_transaction.py +++ b/core/src/apps/solana/predefined_transaction.py @@ -181,6 +181,10 @@ async def try_confirm_predefined_transaction( instructions = transaction.instructions instructions_count = len(instructions) + for instruction in instructions: + if instruction.multisig_signers: + return False + if instructions_count == 1: if SystemProgramTransferInstruction.is_type_of(instructions[0]): await confirm_system_transfer(instructions[0], fee, signer_path, blockhash) diff --git a/core/src/apps/solana/transaction/__init__.py b/core/src/apps/solana/transaction/__init__.py index 932a1176a..7182d5e00 100644 --- a/core/src/apps/solana/transaction/__init__.py +++ b/core/src/apps/solana/transaction/__init__.py @@ -113,7 +113,7 @@ class Transaction: data_length = parse_var_int(serialized_tx_reader) instruction_id_length = get_instruction_id_length(program_id) - if instruction_id_length <= data_length: + if 0 < instruction_id_length <= data_length: instruction_id = int.from_bytes( serialized_tx_reader.read_memoryview(instruction_id_length), "little", diff --git a/core/src/apps/solana/transaction/instructions.py b/core/src/apps/solana/transaction/instructions.py index 3eb9226a7..07c4a07d3 100644 --- a/core/src/apps/solana/transaction/instructions.py +++ b/core/src/apps/solana/transaction/instructions.py @@ -918,14 +918,14 @@ def get_instruction( UIProperty( None, "assigned_account", - "Assigned account", + "Assign account", False, None, ), UIProperty( "owner", None, - "To program", + "Assign account to program", False, None, ), @@ -1411,7 +1411,7 @@ def get_instruction( UIProperty( None, "allocated_account", - "Allocate account", + "Allocate data for account", False, None, ), @@ -1474,14 +1474,14 @@ def get_instruction( UIProperty( None, "assigned_account", - "Assigned account", + "Assign account", False, None, ), UIProperty( "owner", None, - "To program", + "Assign account to program", False, None, ), @@ -1763,7 +1763,7 @@ def get_instruction( UIProperty( None, "stake_account", - "Set stake authority for", + "Set authority for", False, None, ), @@ -1989,7 +1989,7 @@ def get_instruction( UIProperty( "lamports", None, - "Stake withdraw", + "Withdraw stake", False, None, ), @@ -2049,7 +2049,7 @@ def get_instruction( UIProperty( None, "delegated_stake_account", - "Deactivate stake", + "Deactivate stake account", False, None, ), @@ -2189,7 +2189,7 @@ def get_instruction( UIProperty( None, "source_stake_account", - "Merge", + "Merge stake account", False, None, ), @@ -2276,14 +2276,21 @@ def get_instruction( UIProperty( None, "stake_account", - "Set stake auth", + "Set authority for", False, None, ), UIProperty( "new_authorized_pubkey", None, - "New (stake/withdraw) auth", + "New authority", + False, + None, + ), + UIProperty( + "stake_authorize", + None, + "Authority type", False, None, ), @@ -2412,21 +2419,35 @@ def get_instruction( UIProperty( None, "stake_account", - "Set stake auth", + "Set authority for", + False, + None, + ), + UIProperty( + None, + "new_stake_or_withdraw_authority", + "New authority", + False, + None, + ), + UIProperty( + "stake_authorize", + None, + "Authority type", False, None, ), UIProperty( None, "stake_or_withdraw_authority", - "Stake or withdraw authority", + "Authorized by", False, None, ), UIProperty( None, - "new_stake_or_withdraw_authority", - "New stake or withdraw authority", + "lockup_authority", + "Custodian", False, None, ), @@ -2497,7 +2518,7 @@ def get_instruction( UIProperty( None, "stake_account", - "Set authority for stake account", + "Set authority for", False, None, ), @@ -2772,7 +2793,7 @@ def get_instruction( UIProperty( None, "account_to_initialize", - "Init account", + "Initialize account", False, None, ), @@ -2786,7 +2807,7 @@ def get_instruction( UIProperty( None, "mint_account", - "Mint", + "For token", False, None, ), @@ -2833,7 +2854,7 @@ def get_instruction( UIProperty( None, "multisig_account", - "Init multisig", + "Initialize multisig", False, None, ), @@ -3003,7 +3024,7 @@ def get_instruction( UIProperty( None, "source_account", - "Rewoke delegate", + "Revoke delegate", False, None, ), @@ -3718,7 +3739,7 @@ def get_instruction( UIProperty( None, "account_to_initialize", - "Init account", + "Initialize account", False, None, ), @@ -3732,7 +3753,7 @@ def get_instruction( UIProperty( None, "mint_account", - "Mint", + "For token", False, None, ), @@ -3803,7 +3824,7 @@ def get_instruction( UIProperty( None, "account_to_initialize", - "Init account", + "Initialize account", False, None, ), @@ -3817,7 +3838,7 @@ def get_instruction( UIProperty( None, "mint_account", - "Mint", + "For token", False, None, ), @@ -3904,7 +3925,7 @@ def get_instruction( UIProperty( None, "account_to_initialize", - "Init account", + "Initialize account", False, None, ), @@ -3918,7 +3939,7 @@ def get_instruction( UIProperty( None, "mint_account", - "Mint", + "For token", False, None, ), @@ -4850,7 +4871,7 @@ def get_instruction( UIProperty( None, "account_to_initialize", - "Init account", + "Initialize account", False, None, ), @@ -4864,7 +4885,7 @@ def get_instruction( UIProperty( None, "mint_account", - "Mint", + "For token", False, None, ), @@ -4935,7 +4956,7 @@ def get_instruction( UIProperty( None, "account_to_initialize", - "Init account", + "Initialize account", False, None, ), @@ -4949,7 +4970,7 @@ def get_instruction( UIProperty( None, "mint_account", - "Mint", + "For token", False, None, ), @@ -4978,7 +4999,7 @@ def get_instruction( UIProperty( None, "account_to_initialize", - "Init account", + "Initialize immutable owner extension for account", False, None, ), @@ -5053,7 +5074,7 @@ def get_instruction( UIProperty( None, "token_mint", - "From mint", + "For token", False, None, ), @@ -5128,7 +5149,7 @@ def get_instruction( UIProperty( None, "token_mint", - "From mint", + "For token", False, None, ), diff --git a/core/src/apps/solana/transaction/programs.json b/core/src/apps/solana/transaction/programs.json index 974ee8508..867c928a3 100644 --- a/core/src/apps/solana/transaction/programs.json +++ b/core/src/apps/solana/transaction/programs.json @@ -74,11 +74,11 @@ "ui_properties": [ { "account": "assigned_account", - "display_name": "Assigned account" + "display_name": "Assign account" }, { "parameter": "owner", - "display_name": "To program" + "display_name": "Assign account to program" } ] }, @@ -420,7 +420,7 @@ "ui_properties": [ { "account": "allocated_account", - "display_name": "Allocate account" + "display_name": "Allocate data for account" }, { "parameter": "space", @@ -464,11 +464,11 @@ "ui_properties": [ { "account": "assigned_account", - "display_name": "Assigned account" + "display_name": "Assign account" }, { "parameter": "owner", - "display_name": "To program" + "display_name": "Assign account to program" } ] }, @@ -664,7 +664,7 @@ "ui_properties": [ { "account": "stake_account", - "display_name": "Set stake authority for" + "display_name": "Set authority for" }, { "parameter": "pubkey", @@ -829,7 +829,7 @@ "ui_properties": [ { "parameter": "lamports", - "display_name": "Stake withdraw" + "display_name": "Withdraw stake" }, { "account": "stake_account", @@ -870,7 +870,7 @@ "ui_properties": [ { "account": "delegated_stake_account", - "display_name": "Deactivate stake" + "display_name": "Deactivate stake account" }, { "account": "stake_authority", @@ -971,7 +971,7 @@ "ui_properties": [ { "account": "source_stake_account", - "display_name": "Merge" + "display_name": "Merge stake account" }, { "account": "destination_stake_account", @@ -1034,11 +1034,15 @@ "ui_properties": [ { "account": "stake_account", - "display_name": "Set stake auth" + "display_name": "Set authority for" }, { "parameter": "new_authorized_pubkey", - "display_name": "New (stake/withdraw) auth" + "display_name": "New authority" + }, + { + "parameter": "stake_authorize", + "display_name": "Authority type" }, { "account": "stake_or_withdraw_authority", @@ -1133,15 +1137,23 @@ "ui_properties": [ { "account": "stake_account", - "display_name": "Set stake auth" + "display_name": "Set authority for" + }, + { + "account": "new_stake_or_withdraw_authority", + "display_name": "New authority" + }, + { + "parameter": "stake_authorize", + "display_name": "Authority type" }, { "account": "stake_or_withdraw_authority", - "display_name": "Stake or withdraw authority" + "display_name": "Authorized by" }, { - "account": "new_stake_or_withdraw_authority", - "display_name": "New stake or withdraw authority" + "account": "lockup_authority", + "display_name": "Custodian" } ] }, @@ -1196,7 +1208,7 @@ "ui_properties": [ { "account": "stake_account", - "display_name": "Set authority for stake account" + "display_name": "Set authority for" }, { "account": "new_stake_or_withdraw_authority", @@ -1375,7 +1387,7 @@ "ui_properties": [ { "account": "account_to_initialize", - "display_name": "Init account" + "display_name": "Initialize account" }, { "account": "owner", @@ -1383,7 +1395,7 @@ }, { "account": "mint_account", - "display_name": "Mint" + "display_name": "For token" } ] }, @@ -1418,7 +1430,7 @@ "ui_properties": [ { "account": "multisig_account", - "display_name": "Init multisig" + "display_name": "Initialize multisig" }, { "account": "signer_accounts", @@ -1538,7 +1550,7 @@ "ui_properties": [ { "account": "source_account", - "display_name": "Rewoke delegate" + "display_name": "Revoke delegate" }, { "account": "owner", @@ -2041,7 +2053,7 @@ "ui_properties": [ { "account": "account_to_initialize", - "display_name": "Init account" + "display_name": "Initialize account" }, { "parameter": "owner", @@ -2049,7 +2061,7 @@ }, { "account": "mint_account", - "display_name": "Mint" + "display_name": "For token" } ] }, @@ -2098,7 +2110,7 @@ "ui_properties": [ { "account": "account_to_initialize", - "display_name": "Init account" + "display_name": "Initialize account" }, { "parameter": "owner", @@ -2106,7 +2118,7 @@ }, { "account": "mint_account", - "display_name": "Mint" + "display_name": "For token" } ] }, @@ -2166,7 +2178,7 @@ "ui_properties": [ { "account": "account_to_initialize", - "display_name": "Init account" + "display_name": "Initialize account" }, { "account": "owner", @@ -2174,7 +2186,7 @@ }, { "account": "mint_account", - "display_name": "Mint" + "display_name": "For token" } ] }, @@ -2832,7 +2844,7 @@ "ui_properties": [ { "account": "account_to_initialize", - "display_name": "Init account" + "display_name": "Initialize account" }, { "parameter": "owner", @@ -2840,7 +2852,7 @@ }, { "account": "mint_account", - "display_name": "Mint" + "display_name": "For token" } ] }, @@ -2889,7 +2901,7 @@ "ui_properties": [ { "account": "account_to_initialize", - "display_name": "Init account" + "display_name": "Initialize account" }, { "parameter": "owner", @@ -2897,7 +2909,7 @@ }, { "account": "mint_account", - "display_name": "Mint" + "display_name": "For token" } ] }, @@ -2916,7 +2928,7 @@ "ui_properties": [ { "account": "account_to_initialize", - "display_name": "Init account" + "display_name": "Initialize immutable owner extension for account" } ] } @@ -2971,7 +2983,7 @@ }, { "account": "token_mint", - "display_name": "From mint" + "display_name": "For token" }, { "account": "wallet_address", @@ -3027,7 +3039,7 @@ }, { "account": "token_mint", - "display_name": "From mint" + "display_name": "For token" }, { "account": "wallet_addr",