namada icon indicating copy to clipboard operation
namada copied to clipboard

Review Namada Ledger app transaction formatting

Open murisi opened this issue 1 year ago • 2 comments

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

murisi avatar Jul 10 '24 10:07 murisi

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

cwgoes avatar Aug 27 '24 07:08 cwgoes

@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

murisi avatar Aug 29 '24 16:08 murisi