Conversation
7efe564 to
3aace0d
Compare
cwgoes
left a comment
There was a problem hiding this comment.
The changes mostly look fine. Some of the SDK code needs to be cleaned up, but we can do that separately. My main request here is that we make sure that all fixed strings are constants defined somewhere instead of strings copied around.
| tv.output_expert | ||
| .push(format!("Code hash : {}", HEXLOWER.encode(&code_hash.0))); | ||
| })?; | ||
| tv.output_expert.push(format!( |
There was a problem hiding this comment.
This code is super super messy and I don't understand what exactly it's trying to do, but most of the messiness seems to be leftover from before. What exactly is the aim of this function?
There was a problem hiding this comment.
This code is there to produce the JSON test vectors for Ledger integration. See an example output of this function (called with some Tx argument) below:
{
"blob":"1d0000006c6f63616c6e65742e6664633665356661643365356535326433662d300023000000323032332d31312d31365431343a33313a31322e3030383437393437392b30303a303029e3fd2d0a8c786d5318be88f0be06629152ac26628396e28350f7c5b81b1d58f09f9bf315fe3b244703f3695cafff63b67156f799dc5c0742d1612cdd4897be0101000000000000000000000000000000000000000000000000000000000000000032fdd4e57f56519541491312d4e9089032244eca0048998ffa0340c473b72dad3604abd76581e71e4a334d0708ef754a0adcec66d80300000000000000a861000000000000000400000002b3078bd88b010000007c7a739c83e943d4a56a0fd4e4c52a9edc0d66d9105324bcc909619857a6683b010c00000074785f626f6e642e7761736d00b3078bd88b0100004b00000000f2d1fbf5a690f8ab12cfa6166425bec4d7569bb400e9a435000000000000000000000000000000000000000000000000000000000100ba4c9645a23343896227110a902af84e7b4a4bb30301000000c7fec5279e22792a9cad6346f8933c1b2249043e1a03c835030d4e71dfbac3e00000ba4c9645a23343896227110a902af84e7b4a4bb301000000000087d6e5a4617cce4c93120504a5f5db8c9ce1af0416e260c3fbe9066df3f3fdb2abfda0cac21b97b3e89b3c29013db345bd22548e8baf2df4e682bb4e1a041f0f03040000005b693f86a6a8053b79effacd031e2367a1d35cc64988795768920b296501374229e3fd2d0a8c786d5318be88f0be06629152ac26628396e28350f7c5b81b1d58f09f9bf315fe3b244703f3695cafff63b67156f799dc5c0742d1612cdd4897bed4bfd3e247c0ef6e2ab23983a793412fd94a78d9a08efaa94a3d6a977e3c601c01010000000048998ffa0340c473b72dad3604abd76581e71e4a334d0708ef754a0adcec66d8010000000000cfcc82f327627fed72368dd168663db755478675d812365b9c8b92c36acaaebf1fe9a0494aaf9e675d4b4f041ffebc5234d9da012721b1bd5d1bbc819ed56f04",
"index":0,
"name":"Bond_0",
"output":[
"0 | Type : Bond",
"1 | Source [1/2] : tnam1qxaye9j95ge58ztzyugs4yp2lp88kjjtk",
"1 | Source [2/2] : vrwawgc",
"2 | Validator [1/2] : tnam1q8edr7l456g032cje7npvep9hmzdw45mk",
"2 | Validator [2/2] : sktpy8f",
"3 | Amount : NAM 900.0"
],
"output_expert":[
"0 | Code hash [1/2] : 7c7a739c83e943d4a56a0fd4e4c52a9edc0d66",
"0 | Code hash [2/2] : d9105324bcc909619857a6683b",
"1 | Source [1/2] : tnam1qxaye9j95ge58ztzyugs4yp2lp88kjjtk",
"1 | Source [2/2] : vrwawgc",
"2 | Validator [1/2] : tnam1q8edr7l456g032cje7npvep9hmzdw45mk",
"2 | Validator [2/2] : sktpy8f",
"3 | Amount : NAM 900.0",
"4 | Timestamp : 2023-11-16 14:31:12.008479479 UTC",
"5 | Pubkey [1/2] : tpknam1qpyfnrl6qdqvguah9kknvp9t6ajcrec",
"5 | Pubkey [2/2] : 7fge56pcgaa655zkua3nds48x83t",
"6 | Epoch : 3",
"7 | Gas limit : 0.025",
"8 | Fees/gas unit : NAM 0.000001"
],
"valid":true
},
This JSON output format and its pagination is mostly determined by Zondax. The fact that the outputs are user-facing necessitates some amount of text formatting code. Also this code is structured to simulate how a hardware wallet might decode a transaction blob in order to ensure that the blob has sufficient information to decode, say, a MASP transfer from it. That being said, I'm responsible for essentially all of the messiness and will think of a way to fix it.
…ector generation.
3aace0d to
5b24e82
Compare
06d8876 to
ece850b
Compare
cwgoes
left a comment
There was a problem hiding this comment.
Changes look alright to me. Can you point me to where in the transaction execution code we check that the tag and hash correspond to each other correctly?
Hi. This is where we check that the transaction type tag and hash correspond in the transaction execution code: https://github.com/anoma/namada/blob/385714ec43b26fff394def26c23a44221cb1f17b/shared/src/vm/wasm/run.rs#L113 . In the transactions that contain VP code hashes like |
Describe your changes
Added transaction type tag field in order to aid hardware wallets in decoding transactions. More specifically the following changes were made:
tag: Option<String>field to theCodetype that is used in the code and extra data sections - this is now where a transaction's tag can be placed. This field is also included in the section hashes.tagis present in theCodetype, then the protocol will ensure that the hash corresponding towasm/hash/<tag>equals the hash in the commitment.tagfield with the appropriate WASM path whenever possible (essentially whenever the WASM code hash was obtained withquery_wasm_code_hash)WrapperTxheader now that the Ledger app has been modified.InitValidatortransaction, namely: email, description, Discord handle, and website.The corresponding changes to the Ledger app can be found here: Zondax/ledger-namada#16 .
Indicate on which release or other PRs this topic is based on
Namada v0.26.0.
Checklist before merging to
draft