Feat(solana): Hidden flag functionality for instructions #89
Conversation
|
|
There was a problem hiding this comment.
Minor nitpicks, otherwise LGTM.
Before creating a PR to Trezor repo please:
- sync our fork's master branch to Trezor's and rebase this PR on top of that
- rename branch to
feat/solana...(I.e. drop the TRSOL) - commits should follow the conventional commits spec (e.g.
feat(solana): hide compute budget instructions)
When creating the Trezor PR please also link this issue comment and perhaps provide more context in the PR's description something like "Until now compute budget instructions (SetComputeUnitLimit and SetComputeUnitPrice) were being displayed although the user doesn't really care about them since they affect only the final fee which is displayed on Trezor as well. Including Compute Budget instructions also broke the "Predefined transactions" logic. This wasn't a huge problem before since wallets didn't use priority fees - but they started to use them now. Priority fees support will be added to Trezor Suite as well and so not fixing the predefined transactions would break the UX on Trezor with Suite since predefined transactions would not work....".
| @@ -30,6 +30,7 @@ class Instruction: | |||
|
|
|||
| is_program_supported: bool | |||
| is_instruction_supported: bool | |||
| is_hidden: bool | |||
There was a problem hiding this comment.
Maybe we could name it more precisely e.g. can_be_hidden_from_ui? That way the displaying logic can decide to display such an instruction if it really wants to and it's clearer what "hidden" means.
| @@ -123,12 +123,12 @@ async def try_confirm_token_transfer_transaction( | |||
| from .token_account import try_get_token_account_base_address | |||
|
|
|||
| if not is_predefined_token_transfer( | |||
| transaction.instructions, | |||
| transaction.get_visible_instructions(), | |||
There was a problem hiding this comment.
Visible instructions can be stored in a variable and reused below.
|
Please cherry-pick and include in the final PR also e315160 which fixes a partially related issue. |
|
Oh, also please run |
RostarMarek
changed the title
RostarMarek
changed the title
RostarMarek
force-pushed
the
feat/TRSOL-57/solana-hide-instructions
branch
from
1cb19db
to
112b89f
Compare
Added hidden falg for instructions and added the functionality hide instructions in transactions. Currently applied to compute budget prices.