Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Review Namada Ledger app transaction formatting #3500

Open
murisi opened this issue Jul 10, 2024 · 2 comments
Open

Review Namada Ledger app transaction formatting #3500

murisi opened this issue Jul 10, 2024 · 2 comments

Comments

@murisi
Copy link
Contributor

murisi commented Jul 10, 2024

Review scope

Could we review the following details pertaining to the Namada Ledger app test vectors generated by #3494?:

  • Transaction coverage: are all the desired transaction types generated by the test vector generator?
  • Variation coverage: are all possible variations (enum variants) of each transaction generated by the vector generator?
  • Display coverage: are all fields of a given transaction displayed in expert mode without any omission?
  • User-friendliness of the normal mode:
    • Is too much information being displayed for the average user?
    • Are the relevant details of each transaction being printed in normal mode?
    • Are the outputs in normal mode being ordered sensibly?
    • Are there better ways of grouping/organizing the normal mode outputs? (Especially for Transfers)

Current transaction coverage

The following list enumerates the transactions currently covered by the test vector generator. Is anything missing?

  • TX_CHANGE_METADATA_WASM - pos::MetaDataChange
  • TX_REVEAL_PK - common::PublicKey
  • TX_CHANGE_CONSENSUS_KEY_WASM - ConsensusKeyChange
  • TX_CHANGE_COMMISSION_WASM - pos::CommissionChange
  • TX_UPDATE_STEWARD_COMMISSION - UpdateStewardCommission
  • TX_UNJAIL_VALIDATOR_WASM - Address
  • TX_DEACTIVATE_VALIDATOR_WASM - Address
  • TX_REACTIVATE_VALIDATOR_WASM - Address
  • TX_RESIGN_STEWARD - Address
  • TX_REDELEGATE_WASM - pos::Redelegation
  • TX_WITHDRAW_WASM - pos::Withdraw
  • TX_CLAIM_REWARDS_WASM - pos::ClaimRewards
  • TX_UNBOND_WASM - pos::Unbond
  • TX_BOND_WASM - pos::Bond
  • TX_INIT_PROPOSAL - InitProposalData
  • TX_VOTE_PROPOSAL - VoteProposalData
  • TX_BECOME_VALIDATOR_WASM - BecomeValidator
  • TX_INIT_ACCOUNT_WASM - InitAccount
  • TX_UPDATE_ACCOUNT_WASM - UpdateAccount
  • TX_TRANSFER_WASM - Transfer
  • TX_IBC_WASM - MsgTransfer
  • TX_IBC_WASM - MsgNftTransfer
  • TX_BRIDGE_POOL_WASM - PendingTransfer

Test vectors

Please find attached the generated test vectors in the JSON file. I've also attached the debug prints and transaction schemas in case this is relevant.
vectors.json
schema.txt
debugs.txt.gz

@cwgoes
Copy link
Contributor

cwgoes commented Aug 27, 2024

@murisi Is this list of review instructions still up-to-date?

@murisi
Copy link
Contributor Author

murisi commented Aug 29, 2024

@murisi Is this list of review instructions still up-to-date?

@cwgoes Yes the instructions are still up to date, but the attachments are now out of date. See attached the latest test vectors. Also, please focus on the output and output_expert fields of the test vectors because these represent the lines that users will actually see on the hardware wallet as they press the left and right buttons. The rest of the fields outside these two are just metadata. Thanks! Also, one additional note: I opened this PR thinking that it would be more efficient to review the test vectors than to manually be running namadac commands with the hardware wallet (or simulator) connected and reviewing the outputs one at a time.

schema43.txt
vectors43.json
debugs43.txt.gz

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants