cbor changed after deserialization due to sorted Map keys

[uac-nhan-nguyen]

Hi,

The order of cbor Map items in Assets get changed during decode. This causes the transaction body message that is signed by wallets using this lib to be different from what the client sends.

This is probably caused by a default sort. In my opinion, cbor input should be the same as cbor output because the wallets that uses serialization-lib should not change the sent by dapp.

This also happens to TransactionBody Map items.

Test case:

/// Input 
a54443484f431a00989680444d494e541b000000012a05f200465242455252591a442490d4465342455252591a7d30dff84556414e494c1a00e4e1c0

/// Output
a54443484f431a00989680444d494e541b000000012a05f2004556414e494c1a00e4e1c0465242455252591a442490d4465342455252591a7d30dff8

Full example:

import * as cardano from '@emurgo/cardano-serialization-lib-nodejs';
import {bytesToHex, hexToBytes} from "./src/utils.js";

const originalAssetHex = 'a54443484f431a00989680444d494e541b000000012a05f200465242455252591a442490d4465342455252591a7d30dff84556414e494c1a00e4e1c0'
const parsedAsset = cardano.Assets.from_bytes(hexToBytes(originalAssetHex));
const resultAssetHex = bytesToHex(parsedAsset.to_bytes());

console.log(originalAssetHex);
console.log(resultAssetHex);
console.log('EXPECT', originalAssetHex === resultAssetHex);

[rooooooooob]

@uac-nhan-nguyen see #246 we used to respect insertion order but we later made things be ordered canonically as several places require this ordering in order to work properly on Cardano. Otherwise the cardano-node/cli will compute different hashes as the order won't be canonical. It was causing issues e.g. #247 before.

It's possible that this ordering should only be applied during the computation of hashes, but I haven't looked into the validity of assets in non-canonical form.

[uac-nhan-nguyen]

Thank you. I understand from draft CIP 27 that this is an attempt to reduce size of the transaction message by removing the duplicate keys. However this should only be applicable when we use serialization lib on dapps, to prepare transaction message.

For wallets implementation on the other hand, the wallets should respect the cbor message input during deserialization, and leave the optimization to dapps.

Perhaps could you implement the canonical ordering to be only in the TransactionBuilder. And keep the same insertion order when uses from_bytes()? I believe there is no hash computation needed for deserialization either.

This is not an issue for us anymore since we also sort the Map keys canonically. But I hope that we can be sure the signature produced is for the same message to we send to the wallets.

[rooooooooob]

I understand from draft CIP 27 that this is an attempt to reduce size of the transaction message by removing the duplicate keys.

It's not about the size. It's about maintaining a canonical (unambiguous) byte format for hashing so that we don't get different hashes in different implementations.

Is this causing a specific issue integrating with other tooling that is blocking you? Which hash is coming back different, and using which other tools? Generally, regardless of the actual CBOR bytes, any hashing being done (by us or by cardano-node) where this matters should be doing some kind of canonical form but we've had an issue with plutus datums where that doesn't seem to be the case. I'll update this issue when I figure out if that should have been the case or not. We've had CBOR differences messing up hashes vs IOHK software a few times, some of which turned out to be issues on our end, and others ended up being issues in cardano-node, so it's good to figure out where this is causing issues and what the correct behavior according to the Cardano specs is since otherwise we don't know which library/tool is doing it right/wrong.

[lisicky]

CSL does not guarantee that an input CBOR will remain the same after a serialization roundtrip, except for the PlutusData type. For signing purposes, we have the FixedTransaction type, which preserves the original transaction body types to ensure the same transaction hash during the serialization roundtrip.