vacuumlabs/cardano-hw-cli

Invalid asset group in multiasset token bundle - incorrect ordering of tokens

gitmachtl opened this issue · 18 comments

Soon we will have hex encoded assets available on the cli, you can see a demo right here. However, hw-cli/app-cardano complains about an incorrect token ordering...

image

The TxBody itself is:

{
    "type": "TxBodyMary",
    "description": "",
    "cborHex": "83a5008182582012d6891bc839c7cea7ad3c034c32265db80c0420a29d3d9843687709fa14d99200018182581d6088ff402b0522f27649ac742238c697c579beeb344eb723099d1f16ce821b000000e43bdb642aa3581c2e2f143b3ccbe339145183dc2a799a469e92ab56e0d5b0bd04f54f15a440186d46112233221100181e464c614c654c75187b4568616c6c6f183c581c4ef8525f365560af6b685169cac4c86acdd122d683d3400f6ca23467a743000000181e430001000f45546573743114455465737432181e4554657374330f4554657374360a4a616c6f6e7a6f746573741907d0581c6b79ff1505414bd7ebba7a9b3242ba02810f0b91ea9e391c6e0b7409a1466877746573740a021a0002dcf1031a004ce7eb09a1581c6b79ff1505414bd7ebba7a9b3242ba02810f0b91ea9e391c6e0b7409a1466877746573740a9f8200581c8d87ba19c400938453e86abf8c2f1f9a94c1acf7f495978e70ab0068fff6"
}

Its possible to mint hex encoded assets on the ledger, tested it, thats working. But somehow in that example something results in an error. I thought the correct ordering was something we got rid of a while ago?

Canonical ordering is back in the newest version, because HW wallets must ensure that the map entries are unique. We only temporarily suspended it a couple of months ago, not got rid of it entirely. See https://cips.cardano.org/cips/cip21/.

Canonical ordering is back in the newest version, because HW wallets must ensure that the map entries are unique. We only temporarily suspended it a couple of months ago, not got rid of it entirely. See https://cips.cardano.org/cips/cip21/.

So, its a cardano-cli issue again?

We are working on a js library for transforming non-compliant txs into compliant ones (if possible). The library will then be integrated as a command in hw-cli to deal with txs produced by cardano-cli (or any other tx producer). Hopefully, it will be finished in 1-2 weeks.

Ah thx, that sounds good. Because i am not sure if cardano-cli will adapt to the "right" order. We had that discussion a while ago.
But, wouldn't that reintroduce the issue with the different TxID? Or would this only be an internal reorder to feed the hw wallet.

@janmazak So, this sorting should be ok for hw-wallets? Or am i missing something here?

Unsorted:

[ 
  "6b79ff1505414bd7ebba7a9b3242ba02810f0b91ea9e391c6e0b7409.687774657374",
  "2e2f143b3ccbe339145183dc2a799a469e92ab56e0d5b0bd04f54f15",
  "2e2f143b3ccbe339145183dc2a799a469e92ab56e0d5b0bd04f54f15.112233221100",
  "2e2f143b3ccbe339145183dc2a799a469e92ab56e0d5b0bd04f54f15.4c614c654c75",
  "2e2f143b3ccbe339145183dc2a799a469e92ab56e0d5b0bd04f54f15.68616c6c6f",
  "4ef8525f365560af6b685169cac4c86acdd122d683d3400f6ca23467.000000",
  "4ef8525f365560af6b685169cac4c86acdd122d683d3400f6ca23467.000100",
  "4ef8525f365560af6b685169cac4c86acdd122d683d3400f6ca23467.5465737431",
  "4ef8525f365560af6b685169cac4c86acdd122d683d3400f6ca23467.5465737432",
  "4ef8525f365560af6b685169cac4c86acdd122d683d3400f6ca23467.5465737433",
  "4ef8525f365560af6b685169cac4c86acdd122d683d3400f6ca23467.5465737436",
  "4ef8525f365560af6b685169cac4c86acdd122d683d3400f6ca23467.616c6f6e7a6f74657374"
]

Sorted (into policyIDs >> into assetname length >> into assetnames):

[
  "2e2f143b3ccbe339145183dc2a799a469e92ab56e0d5b0bd04f54f15",
  "2e2f143b3ccbe339145183dc2a799a469e92ab56e0d5b0bd04f54f15.68616c6c6f",
  "2e2f143b3ccbe339145183dc2a799a469e92ab56e0d5b0bd04f54f15.112233221100",
  "2e2f143b3ccbe339145183dc2a799a469e92ab56e0d5b0bd04f54f15.4c614c654c75",
  "4ef8525f365560af6b685169cac4c86acdd122d683d3400f6ca23467.000000",
  "4ef8525f365560af6b685169cac4c86acdd122d683d3400f6ca23467.000100",
  "4ef8525f365560af6b685169cac4c86acdd122d683d3400f6ca23467.5465737431",
  "4ef8525f365560af6b685169cac4c86acdd122d683d3400f6ca23467.5465737432",
  "4ef8525f365560af6b685169cac4c86acdd122d683d3400f6ca23467.5465737433",
  "4ef8525f365560af6b685169cac4c86acdd122d683d3400f6ca23467.5465737436",
  "4ef8525f365560af6b685169cac4c86acdd122d683d3400f6ca23467.616c6f6e7a6f74657374",
  "6b79ff1505414bd7ebba7a9b3242ba02810f0b91ea9e391c6e0b7409.687774657374"
]

Cardano-CLI is sorting it like:

2e2f143b3ccbe339145183dc2a799a469e92ab56e0d5b0bd04f54f15
2e2f143b3ccbe339145183dc2a799a469e92ab56e0d5b0bd04f54f15.112233221100  <- wrong
2e2f143b3ccbe339145183dc2a799a469e92ab56e0d5b0bd04f54f15.4c614c654c75  <- wrong
2e2f143b3ccbe339145183dc2a799a469e92ab56e0d5b0bd04f54f15.68616c6c6f
4ef8525f365560af6b685169cac4c86acdd122d683d3400f6ca23467.000000
4ef8525f365560af6b685169cac4c86acdd122d683d3400f6ca23467.000100
4ef8525f365560af6b685169cac4c86acdd122d683d3400f6ca23467.5465737431
4ef8525f365560af6b685169cac4c86acdd122d683d3400f6ca23467.5465737432
4ef8525f365560af6b685169cac4c86acdd122d683d3400f6ca23467.5465737433
4ef8525f365560af6b685169cac4c86acdd122d683d3400f6ca23467.5465737436
4ef8525f365560af6b685169cac4c86acdd122d683d3400f6ca23467.616c6f6e7a6f74657374
6b79ff1505414bd7ebba7a9b3242ba02810f0b91ea9e391c6e0b7409.687774657374

so the problem lies in the alnum sorting of the assetNames before the length sorting, right?

Because i am not sure if cardano-cli will adapt to the "right" order.

Yes, sadly it won't that is why we're working on the "transformation" library.

But, wouldn't that reintroduce the issue with the different TxID? Or would this only be an internal reorder to feed the hw wallet.

Yes it would. If there are multiple signers and one of them uses a HW wallet, for example, the initial transaction then has to be created by cardano-hw-cli (or "transformed" by the new command) in order to be 100% compliant with HW wallets.

so the problem lies in the alnum sorting of the assetNames before the length sorting, right?

Yes. The sort should first happen by policyIDs. Inside of a policyID assetNames should be sorted by length and if they are the same length then they should be sorted lexicographically.

This order should be ok (the sorted one you also posted):

[
  "2e2f143b3ccbe339145183dc2a799a469e92ab56e0d5b0bd04f54f15",
  "2e2f143b3ccbe339145183dc2a799a469e92ab56e0d5b0bd04f54f15.68616c6c6f",
  "2e2f143b3ccbe339145183dc2a799a469e92ab56e0d5b0bd04f54f15.112233221100",
  "2e2f143b3ccbe339145183dc2a799a469e92ab56e0d5b0bd04f54f15.4c614c654c75",
  "4ef8525f365560af6b685169cac4c86acdd122d683d3400f6ca23467.000000",
  "4ef8525f365560af6b685169cac4c86acdd122d683d3400f6ca23467.000100",
  "4ef8525f365560af6b685169cac4c86acdd122d683d3400f6ca23467.5465737431",
  "4ef8525f365560af6b685169cac4c86acdd122d683d3400f6ca23467.5465737432",
  "4ef8525f365560af6b685169cac4c86acdd122d683d3400f6ca23467.5465737433",
  "4ef8525f365560af6b685169cac4c86acdd122d683d3400f6ca23467.5465737436",
  "4ef8525f365560af6b685169cac4c86acdd122d683d3400f6ca23467.616c6f6e7a6f74657374",
  "6b79ff1505414bd7ebba7a9b3242ba02810f0b91ea9e391c6e0b7409.687774657374"
]

It seems that we should perhaps include some examples in CIP-0021.

You made a "transformation" library for the trezor hw-wallet, can it be used to "fix" it also in this case?

To get you right on "the new command", will it be possible to feed cardano-hw-cli with an unsigned tx-body and it will resort it and would again output an unsigned tx-body? If so, that would be super versatile.

You included a "transformation" library for the trezor hw-wallet, can it be used to "fix" it also in this case?

I'm not sure what exactly you mean, but I think we removed that. If you mean that Trezor was at some point sorting the policies and assets then that's already removed and it behaves the same as Ledger in this regard.

To get you right on "the new command", will it be possible to feed cardano-hw-cli with an unsigned tx-body and it will resort it and would again output an unsigned tx-body?

Yes. There will be a validation command, which will only tell you if the transaction is compliant. There will also be a command which will do exactly what you describe. And perhaps also an option will be added to tx signing which would silently transform the unsigned tx-body during signing. But I have to ask @KubqoA to please verify my statements in this paragraph.

A solution to feed hw-cli with a txbody and get back the txbody with the right order would be the best option imo. So also for multi-signing transaction, you could normally generate the txbody with cardano-cli, feed it thru hw-cli and use the resulting txbody for further processing.

P.S.: To be clear, the best solution would be to fix cardano-cli. Or let the user decide by just adding the assets in the order that you provide it with the --txout parameters. But looks like boths of them will not be implemented soon.

when the updated libs are out in the wild, we will see many stuck funds on users that are using hw-wallets via the cli because of that ordering issue, so @KubqoA and @gabrielKerekes i am shy to ask, but how close are we currently to that added "txbody assets reordering" command we talked about earlier?

The library for the CIP-0021 compliance is currently being worked on, I am afraid I cannot tell you exactly when it will be finished, but we are doing our best to have it ready ASAP as we know it's quite a big blocker. If you need to follow the development, the library is being developed here: vacuumlabs/cardano-hw-interop-lib#1

The library for the CIP-0021 compliance is currently being worked on, I am afraid I cannot tell you exactly when it will be finished, but we are doing our best to have it ready ASAP as we know it's quite a big blocker. If you need to follow the development, the library is being developed here: vacuumlabs/cardano-hw-interop-lib#1

thx, looks like you're making pretty fast progress on that front. thank you for your work!

so this issue is getting urgent, i receive pictures like that:
image

Where does this come from? AFAIK neither Ledger nor Trezor released HW wallet updates that would enforce canonical ordering, and all SW wallets we know of have been updated (the last one was Daedalus on 18th Nov).

We are pretty close to finishing the hw-cli update, too, but we got stuck a bit on the non-standard internal serialization format of cardano-cli for raw transactions (to be signed) which does not conform to CDDL for full tx (native script witnesses are serialized differently).

Where does this come from? AFAIK neither Ledger nor Trezor released HW wallet updates that would enforce canonical ordering, and all SW wallets we know of have been updated (the last one was Daedalus on 18th Nov).

a user sent this to me, he used yoroi and i asked three times about the cardano-app version and the reply was version 2.4.1

i am closing this issue now, the asset ordering can now be resolved with the new added feature to sort a given tx in canonical order. 👍