cardano-hw-cli transaction transform seems to change the metadata

[HeptaSean]

When trying to submit a transaction assembled from witnesses generated by cardano-hw-cli, I am getting a ConflictingMetadataHash error.

Before being able to generate the witnesses, I had to do (as expected):

$ cardano-hw-cli transaction transform --tx-file transaction.raw --out-file transaction.canonical

When comparing, what cardano-cli transaction view says about both, I get:

$ diff -u <(cardano-cli transaction view --tx-file transaction.raw) <(cardano-cli transaction view --tx-file transaction.canonical)
--- /dev/fd/63  2022-08-12 21:20:48.780862638 +0200
+++ /dev/fd/62  2022-08-12 21:20:48.780862638 +0200
@@ -9,10 +9,10 @@
   '721':
   - - 43c4b46c9635b3a57c9731de817a20b3d099ee2c7f44a0e2723c4bb6
     - - - HeptaCard
-        - - - files
-            - - - - mediaType
-                  - text/html
-                - - src
+        - - - name
+            - HeptaSean's Card
+          - - files
+            - - - - src
                   - - data:text/html;base64,PCFkb2N0eXBlIGh0bWw+PGh0bWwgbGFuZz1lbj48bW
                     - V0YSBjaGFyc2V0PXV0Zi04Pjx0aXRsZT5IZXB0YVNlYW4ncyBDYXJkPC90aXRsZT
                     - 48c3R5bGU+QGZvbnQtZmFjZXtmb250LWZhbWlseTonU2VuJztzcmM6dXJsKGRhdG
@@ -120,6 +120,8 @@
                     - IxSGtvci9ZNnhRUUVEVVJQLzdvNFdBN1o4Qndpd0xpQ1luOFJjL0RSTFVUR0VORl
                     - RCSWhvYklQQWVHSlg0aDRNY2diK05MOUFBQUFBRWxGVGtTdVFtQ0MgYWx0PjwvYT
                     - 48L2Rpdj4=
+                - - mediaType
+                  - text/html
           - - image
             - - data:image/svg+xml;base64,PD94bWwgdmVyc2lvbj0iMS4wIj8+PHN2ZyB3aW
               - R0aD0iMTYwMCIgaGVpZ2h0PSIxNjAwIiB4bWxucz0iaHR0cDovL3d3dy53My5vcm
@@ -138,8 +140,6 @@
               - Y3eiIvPjwvc3ZnPgo=
           - - mediaType
             - image/svg+xml
-          - - name
-            - HeptaSean's Card
 mint:
   policy 43c4b46c9635b3a57c9731de817a20b3d099ee2c7f44a0e2723c4bb6:
     asset 486570746143617264 (HeptaCard): 1

So, my theory would be that transaction transform changed the order of some elements in the metadata and that is the reason that the hash does not match anymore.

[HeptaSean]

For what it's worth: This reordering was the cause of the error on submit.

I have fed the CBOR as reordered by cardano-hw-cli directly into cardno-cli (using --metadata-cbor-file) and then cardano-hw-cli did not reorder them differently and the transaction could be successfully submitted.

https://forum.cardano.org/t/minting-an-on-chain-nft-with-a-hardware-wallet/105922#bummer-9

[HeptaSean]

After digging a bit deeper:

The whole transformation seems to be done mainly in cardano-hw-interop-lib (https://github.com/vacuumlabs/cardano-hw-cli/blob/develop/src/transaction/transactionValidation.ts#L110-L133). And you do not seem to intentionally touch the auxiliaryData anywhere, as far as I can see (https://github.com/vacuumlabs/cardano-hw-interop-lib/blob/develop/src/txParsers.ts#L360-L363).

But you do select {canonical: true}, when encoding to CBOR again (https://github.com/vacuumlabs/cardano-hw-interop-lib/blob/develop/src/utils.ts#L26). And canonical means that the keys in all maps are sorted by their byte representation (https://github.com/hildjj/node-cbor/issues/26).

It seems that cardano-cli does not respect that part of the specification (or does it wrong) and, hence, your choice of canonical encoding changes things it should not change. (So, this should maybe also be an issue at cardano-cli.)

[gitmachtl]

This is a real issue.

I have done a test with a very simple metadata.json file:

{
  "721" :{
        "ipfs": "some text",
        "assetHash": "0931ae2de9aa46212e50adc1798f9071e7c6368b78ae7eb434c2ca0ed9d05370"
  }
}

Even without any Native Assets included in the transaction process, this metadata will be transformed.

cardano-cli itself produces an output with a wrong order of the keys, which cannot be recovered to the original: ({0: {721: {"assetHash": "0931ae2de9aa46212e50adc1798f9071e7c6368b78ae7eb434c2ca0ed9d05370", "ipfs": "some text"}}}) example on testnet

hw-cli corrects it - because hw wallets needs it in the canonical order - back to: ({0: {721: {"ipfs": "some text", "assetHash": "0931ae2de9aa46212e50adc1798f9071e7c6368b78ae7eb434c2ca0ed9d05370"}}})

which will result in an error like: Error: Error while submitting tx: ShelleyTxValidationError ShelleyBasedEraAlonzo (ApplyTxError [UtxowFailure (WrappedShelleyEraFailure (ConflictingMetadataHash (AuxiliaryDataHash {unsafeAuxiliaryDataHash = SafeHash "fb7099a47afd6efb4f9cccf9d0f8745331a19eb8b3f50548ffadae9de8551743"}) (AuxiliaryDataHash {unsafeAuxiliaryDataHash = SafeHash "ef10842a1a55e79310cba366c48b5bd8538039fbbadb01d9a69da74f0a5310f0"})))]) during a submit via cardano-cli.

@gabrielKerekes is it possible to just generate a new datahash for the corrected output?

The transaction before is like:

[{0: [[h'26AA58EB74752A4D69B2F19180F174B28C4BB47305FC29E3BD3CA84E2AD48EE7', 0]], 1: [[h'0074976C54AFAF444F7CD499BD8519AAC6592B13B22B9D5817F0DA5C5203D205532089AD2F7816892E2EF42849B7B52788E41B3FD43A6E01CF', 1000000], [h'0074976C54AFAF444F7CD499BD8519AAC6592B13B22B9D5817F0DA5C5203D205532089AD2F7816892E2EF42849B7B52788E41B3FD43A6E01CF', 7891054]], 2: 183013, 3: 4812944, 7: h'FB7099A47AFD6EFB4F9CCCF9D0F8745331A19EB8B3F50548FFADAE9DE8551743'}, {}, true, 259({0: {721: {"assetHash": "0931ae2de9aa46212e50adc1798f9071e7c6368b78ae7eb434c2ca0ed9d05370", "ipfs": "some text"}}})]

and after the transform its:

[{0: [[h'26AA58EB74752A4D69B2F19180F174B28C4BB47305FC29E3BD3CA84E2AD48EE7', 0]], 1: [[h'0074976C54AFAF444F7CD499BD8519AAC6592B13B22B9D5817F0DA5C5203D205532089AD2F7816892E2EF42849B7B52788E41B3FD43A6E01CF', 1000000], [h'0074976C54AFAF444F7CD499BD8519AAC6592B13B22B9D5817F0DA5C5203D205532089AD2F7816892E2EF42849B7B52788E41B3FD43A6E01CF', 7891054]], 2: 183013, 3: 4812944, 7: h'FB7099A47AFD6EFB4F9CCCF9D0F8745331A19EB8B3F50548FFADAE9DE8551743'}, {}, true, 259({0: {721: {"ipfs": "some text", "assetHash": "0931ae2de9aa46212e50adc1798f9071e7c6368b78ae7eb434c2ca0ed9d05370"}}})]

the auxiliary datahash in the key 7 babbage-cddl before the auxiliary data itself was not updated to match the new one. so it should be able to update this value along with the auxiliary data canonical order as a solution for now. @janmazak

[HeptaSean]

Thank you!

I was just preparing for answering to your original comment, but saw that you also have done a lot of testing and acknowledge the issue.

With IOG doing nothing, I'd also say that detecting if canonicalisation modified the auxiliary data and then updating the auxiliary data hash would be the preferred way of fixing it. Seems to require a lot of new code on your side, though, since you just pass through the auxiliary data, right now.

The very least should be a warning to the user that there was a modification, so that they do not only run into it, when getting the error on submission and do not have to do the bug tracing that we have done again.

If IOG stands by “We do not want canonical CBOR.”, but at least fixes that users can influence the order by the provided JSON (see https://github.com/input-output-hk/cardano-node/issues/4335), it would also be an option to tell the user exactly, in which order they have to feed the metadata to cardano-cli. … But maybe repairing the auxiliary data hash is the preferable solution even in that case.

[janmazak]

The auxiliary data / metadata are not fed into HW wallets with the exception of Catalyst voting registration (HW wallets only expect hash of the aux data that is part of tx body and expect the client to put together the whole transaction). So transformation of aux data by hw-cli is undesirable.

We will take a look at what can be done; cardano-hw-cli relies on https://github.com/vacuumlabs/cardano-hw-interop-lib for parsing and transforming txs (which Vacuumlabs develop and have full control over). Perhaps just using transformTxBody instead of transformTx will do what we need in hw-cli, but we'll have to investigate the implications.

[Scitz0]

I would argue that its not needed to transform the metadata at all by cardano-hw-cli as its only the hash of the metadata thats passed on to the Ledger, not the actual metadata.

Edit I saw that Jan beat me to it 😄

[gitmachtl]

We will take a look at what can be done; cardano-hw-cli relies on https://github.com/vacuumlabs/cardano-hw-interop-lib for parsing and transforming txs (which Vacuumlabs develop and have full control over). Perhaps just using transformTxBody instead of transformTx will do what we need in hw-cli, but we'll have to investigate the implications.

A fast solution is to just recalculate the new aux data hash for txbody key 7. In the example above the output of cardano-cli for the auxdata was: ({0: {721: {"assetHash": "0931ae2de9aa46212e50adc1798f9071e7c6368b78ae7eb434c2ca0ed9d05370", "ipfs": "some text"}}}) in cbor thats: D90103A100A11902D1A269617373657448617368784030393331616532646539616134363231326535306164633137393866393037316537633633363862373861653765623433346332636130656439643035333730646970667369736F6D652074657874 hashed via blake2b 256bits (32bytes) this gets us the aux data hash fb7099a47afd6efb4f9cccf9d0f8745331a19eb8b3f50548ffadae9de8551743 which was the original one.

The output of the auxdata after the canonical correction was: ({0: {721: {"ipfs": "some text", "assetHash": "0931ae2de9aa46212e50adc1798f9071e7c6368b78ae7eb434c2ca0ed9d05370"}}}) in cbor thats: D90103A100A11902D1A2646970667369736F6D65207465787469617373657448617368784030393331616532646539616134363231326535306164633137393866393037316537633633363862373861653765623433346332636130656439643035333730 hashed via blake2b 256bits (32bytes) gets us the correct new aux data hash of ef10842a1a55e79310cba366c48b5bd8538039fbbadb01d9a69da74f0a5310f0

Replace that in the tx too and should be good to go. I just did a simple search&replace in the txfile and it worked. proof on testnet

[gitmachtl]

erhaps just using transformTxBody instead of transformTx will do what we need in hw-cli

transformTx supports the cddl-format, which is the format to go with in the future. i also converted all my scripts to cddl-format output. transformTxBody does not support it - as far as i know. but no matter what, the generated output after a "transform" command should have a correct aux data hash 😄

catalyst voting registration is working, because the keys are named 0,1,2,3 in the metadata. that might be changed in the future but ok for now i guess.

[HeptaSean]

transformTx supports the cddl-format, which is the format to go with in the future. i also converted all my scripts to cddl-format output. transformTxBody does not support it - as far as i know.

Looking at https://github.com/vacuumlabs/cardano-hw-interop-lib/blob/develop/src/txTransformers.ts#L78-L97, the differences shouldn't be too large.

If canonical format is not needed for the metadata, it would probably be most user-friendly to leave it as it is.

If IOG also stops fiddling with the order, users could then determine it by the JSON.

(I'm actually quite surprised that I haven't read complaints anywhere that the NFT metadata fields appear in such a strange order.)

[gitmachtl]

I'm actually quite surprised that I haven't read complaints anywhere that the NFT metadata fields appear in such a strange order.

Not only NFT stuff, like my little example here it happens everytime the order is changed. But looks like not that many are directly minting on HW-Wallets and attaching 721-Metadata.

Yes, one solution - most likely the easiest one - is to not touch the auxdata at all during the transform process.

The other solution is to correct the auxHash. I have it in my scripts now as a proof of concept, and it works:

Either way it should be fixed, and hopefully within this next release to clean this up.

[janmazak]

The problem is that hw-interop-lib (and thus cardano-hw-cli) use a generic library for CBOR. Such libraries are unaware of what exactly they are parsing and AFAIK do not support things like "parse everything except metadata", nor do they give you the original CBOR bytestring for each parsed element (so that it would be easy to swap the one for medatada). Without some custom parser/encoder (at least a custom fork of some existing lib), we probably can't treat different parts of the tx (i.e. body vs. metadata) in different ways.

It thus seems much easier to just recalculate the aux data hash (and it is apparently acceptable to all participants in this discussion, and the aux data hash is meaningless, so hopefully no one will object if we change it --- we will check with IOHK). Since the issue basically makes cardano-hw-cli unusable for txs with metadata, it is a very high priority to fix (but might take a bit longer due to the combination of covid and vacations in the VL team).

[HeptaSean]

the aux data hash is meaningless, so hopefully no one will object if we change it --- we will check with IOHK

If it does not match the aux data anymore, the transaction will be rejected, anyway. I don't think there can be any case for not adjusting it.

Yes, sounds okay to fix it that way.

Leaving it untouched would be nicer, but if it is a prohibitive effort it shouldn't stop an okay fix. (And as long as cardano-cli fiddles with the order, it doesn't help much if cardano-hw-cli does not, anyway.)

[gitmachtl]

I also opened this issue on the cardano-node repo, because currently we have no command before the actual submit phase that is checking a txfile/txbody about its complete correctness which should include hashes of all kind. https://github.com/input-output-hk/cardano-node/issues/4399