Closed refi93 closed 3 years ago
This is an urgent one! For example i was not able to move out a specific NativeAsset (NFT) from a Wallet today. I had to do some special CLI "tricks" shifting Tokens around to get that one out. Thats nothing the normal user can do. So, we need a fix for the CBOR sorting via cardano-cli ASAP please, thx.
Summary from a private slack conversation:
I'm happy to discuss it further, but my current view is that it is the cardano-hw-cli
that has the responsibility to pass the tx information to the hardware device in an order that is suitable for the constraints imposed by the hardware device. My view is that it should not be a universal constraint imposed on all tools (e.g. the cardano-cli
). The constraint arises from the hardware, it should be local to just the hardware and the hardware's host software.
Yes for multisig this means that cooperating wallets that involve hardware may need to agree on a canonical representation, since they follow the approach of serialising the tx in multiple locations and expecting it to come to the same txid. A CIP is appropriate for that. But the cardano-cli
would not be bound by that CIP.
Indeed, in any scheme where there is flexibility that is never used, things tend to ossify and assumptions creep in that everything does use the "normal" representation. So in that context a little arbitrary diversity is a good thing to stop unhelpful assumptions creeping into tools.
I disagree. Cbor has specs for a reason. Tools that are following the specs should not have to work around cardano-cli which is not. The fix should happen in the tool not following the spec.
No, there's no non-compliance with specs here. Cardano Shelley (and onwards) does not use canonical CBOR. The node accepts any valid CBOR fitting the CDDL (plus some extra restrictions not desribable in the CDDL). The cardano-cli
generates valid spec-compliant CBOR. There is no suggestion here that any tool is not following any spec (that it should actually be following).
The problem here is that hardware wallets have tighter constraints than that. Because hardware wallets are so constrained for memory space, they do not work by parsing the tx body CBOR. They work by being fed the intput data to make a transaction, and they produce the serialised representation as an output. Effectively this means that hardware wallets want to use a canonical representation (within all the permitted representations). This is fine, and hardware wallets can do that. It has implications for multi-sig, since it means all the wallets need to agree on that canonical form.
Furthermore, a CIP for wallets that did propose a canonical form is free to chose the details of that canonical form and still be fully compliant with the CBOR specification:
https://datatracker.ietf.org/doc/html/rfc7049#section-3.9
3.9. Canonical CBOR
Some protocols may want encoders to only emit CBOR in a particular canonical format; those protocols might also have the decoders check that their input is canonical. Those protocols are free to define what they mean by a canonical format and what encoders and decoders are expected to do. This section lists some suggestions for such protocols.
Emphasis added by me. So a CIP on this topic would be free to standardise as a canonical form the one we use now. The only difference from the suggestions in the CBOR spec is the sort order for map keys. The CBOR spec suggestion is (in my humble opinion) totally bonkers. Even Cardano Byron (which does use canonical CBOR) does not use that rule. The sane rule, which we use, is that the sort order is the natural sort order of the keys (e.g. the normal mathematical sort order for numbers, or the lexicographic order for byte strings).
the normal mathematical sort order for numbers, or the lexicographic order for byte strings)
@dcoutts sorry to circle back on this. This rule is clear to me as long as the keys in the map are of the same type, but I noticed in the CDDL the metadata (in case it is a map) can indeed have keys of different types (numbers/bytes/text) -
transaction_metadatum =
{ * transaction_metadatum => transaction_metadatum }
/ [ * transaction_metadatum ]
/ int
/ bytes .size (0..64)
/ text .size (0..64)
It's not clear to me yet how these keys of different types should be consistently sorted within a map following the rule above. Canonical CBOR addresses that by taking the cbor serialized representations of the keys and comparing these (but that would result in definite-length bytestrings of the same type being ordered by length as the length is being prepended in their CBOR representation) - could you clarify how to handle keys of different types? Or is the CDDL just too rigid to specify that keys of different types within the same map is an invalid case in Cardano?
Note that the cardano-cli
has no defined order for tx metadata. It allows user supplied metadata, and uses whatever order is given by the user input.
You might not need any defined order for these in the hw case either: just use the exact order in which the user chooses to present it. You don't even need to exclude duplicates.
If one needs a canonical order for these, the obvious thing to do is to define a specific order of the types. For example that might be the order they appear here in the CDDL, or it could be the order of the 3bits of the CBOR major type (which is what the spec's canonical CBOR suggestion amounts to).
@dcoutts i have to bring this topic up again, because the current implementation of the asset ordering within the cardano-cli generated txbody is again causing issues with hw-wallets. would it be possible to add an optional flag to the cli to generate the maps with the length before lexicographically order for the assetnames? Or a flag that would allow to add the assets in the order they are provided via the --tx-out parameters without reordering them internally? So they can be presorted externally.
Doing a reordering just for the hw-wallet-signing is not a good solution, because when you have multiple signers, the TxID would differ from the original TxBody. So we would need a presorted TxBody before starting the multiple signers process when it is a mixed setup of hw-keys and cli-keys.
@disassembler I wonder if properly dusting the IOG UTxOs with right native assets would make @dcoutts change his mind... by the time it is time to rotate the community delegations... aren't you guys using ledger devices?
@disassembler I wonder if properly dusting the IOG UTxOs with right native assets would make @dcoutts change his mind... by the time it is time to rotate the community delegations... aren't you guys using ledger devices?
hmm... 😄
Note that the
cardano-cli
has no defined order for tx metadata. It allows user supplied metadata, and uses whatever order is given by the user input.You might not need any defined order for these in the hw case either: just use the exact order in which the user chooses to present it. You don't even need to exclude duplicates.
If one needs a canonical order for these, the obvious thing to do is to define a specific order of the types. For example that might be the order they appear here in the CDDL, or it could be the order of the 3bits of the CBOR major type (which is what the spec's canonical CBOR suggestion amounts to).
@dcoutts can we please change the cardano-cli to not sort the given assets by its own strange order, just use the order that is provided via the given parameters on the call? also possible would be a flag as a parameter like --sort-assets if you wanna keep your current order. because currently even if you provide the assets in the order the hw wallets need them, cardano-cli reorders it in an own way.
also please read: https://github.com/Emurgo/cardano-serialization-lib/issues/240 and https://github.com/vacuumlabs/cardano-hw-cli/issues/104
thx!
Internal/External External otherwise.
Area Native Tokens Related to Native Tokens (Mary).
Summary Not a bug per se, but at least complication/blocker when it comes to interoperability with HW wallets.
The asset names within the serialized asset group are apparently ordered alphabetically, but this isn't fully compliant with the canonical serialization CBOR specs, namely:
Example command:
The resulting tx is:
Which when deserialized in https://cbor.me you can see
So the
SpaceBud312
map key comes afterSpaceBud2934
even though its shorter, violating the canonical CBOR specs.This has interoperability implications on HW wallets which serialize the transactions canonically (Trezor in-memory and Ledger requires a deterministic ordering of the token bundle items to be able to reject duplicate keys which can otherwise lead to transactions with unclear effect which bears security implications), therefore cardano-hw-cli can't sign such a transaction with hw wallets.
Expected behavior If we agree canonical CBOR is the way to serialize transactions across tools in the Cardano ecosystem (for the sake of interoperability and security of hw wallets), the asset group keys should be in the order
SpaceBud312, SpaceBud2934, SpaceBud3843
- i.e. ordered by length first and only then alphabetically (technically, by their CBOR representation length/alphabetical order but that shouldn't matter given the keys are of the same type).System info (please complete the following information):