Reconstructing Transactions from CBOR does not preserve the structure

[nielstron]

Describe the bug
I am currently looking at implementing chain synchronization based on the building blocks provided by pycardano. For this it is crucial that the transaction id is preserved. There are some transactions for which the reconstructed transaction computes to a different hash than the original transaction.

To Reproduce
Example transaction: f2470890db9c93ed3a98329835a442e70a0b040c53b680a5284be9e80e9d6d89

from pycardano import Transaction

transaction = Transaction.from_cbor(bytes.fromhex("84a40081825820852ec7f8da4556214f45b166c346802dbe644bdbf16cd8245d431ccdd573fa3100018182581d604b03bd62f7e2d36d157620dd25d3960dc073fa71346a05cb29efbbc91b000000023be7fce3021a00029e61048182018200581cefc7915da7275cfcf0b33909f390d5a2e71e9f1ebc9deaeb503e95bba100828258202fe2a46673313473a680cd2d63993bbf1cd22f864d8d7caeca9c0dffa78e2d595840086ffb2c9adef2a03476f986cf09a50aa40416c007a5f89190ad3edaa8befb5ec7f9dc7a5c6d507705982479a0db4370240e08110b0422ad4de117629f6f7e00825820318e90845971166555968276047c4e3da1102d59b702ec8fc82f748f4072556e584036be05721e2dfbbe35957b0097e53eeffc57c295208bebb8a8063add4dd6978989abd286a8ab57cff4f93d938ab1c4a7ff7089f871b8ba79636c6c3d2fc3ac03f5f6"))
print(transaction.id.payload.hex())
assert transaction.id.payload.hex() == "f2470890db9c93ed3a98329835a442e70a0b040c53b680a5284be9e80e9d6d89", "Incorrect hash"

Expected behavior
The hash should match

Environment and software version (please complete the following information):

• OS: Ubuntu 21
• Ogmios 6.0.7
• PyCardano Version f2596d8

[nielstron]

Comparing the CBOR hexs one can actually see that there is a slight difference on how the list of certificates are encoded (likely unbounded list vs bounded list):

print(transaction.to_cbor().hex())
> 84a40081825820852ec7f8da4556214f45b166c346802dbe644bdbf16cd8245d431ccdd573fa3100018182581d604b03bd62f7e2d36d157620dd25d3960dc073fa71346a05cb29efbbc91b000000023be7fce3021a00029e61049f82018200581cefc7915da7275cfcf0b33909f390d5a2e71e9f1ebc9deaeb503e95bbffa100828258202fe2a46673313473a680cd2d63993bbf1cd22f864d8d7caeca9c0dffa78e2d595840086ffb2c9adef2a03476f986cf09a50aa40416c007a5f89190ad3edaa8befb5ec7f9dc7a5c6d507705982479a0db4370240e08110b0422ad4de117629f6f7e00825820318e90845971166555968276047c4e3da1102d59b702ec8fc82f748f4072556e584036be05721e2dfbbe35957b0097e53eeffc57c295208bebb8a8063add4dd6978989abd286a8ab57cff4f93d938ab1c4a7ff7089f871b8ba79636c6c3d2fc3ac03f5f6

[theeldermillenial]

Maybe related to #330

[theeldermillenial]

@nielstron I am reasonably certain our issues are related. From your example, I plugged this into the CBOR playground.

Original CBOR

Restored CBOR

In the original CBOR, there is a definite list that is getting converted to an indefinite list in deserialization. I believe this is the same issue I'm having with plutus data hashing in #330

The change in CBOR is nominally changing the cbor, which affects the hashing.

Working on a fix now.

[nielstron]

I also noticed that definite lists are missing from the Datum definition - this might help in the fix

[theeldermillenial]

Ugh, you did. I missed that comment 😅

Well, I'm going to track it down and send a PR shortly. I need this for cancelling dex transactions.

I'll make sure to include a test for your case.

[theeldermillenial]

So...this ended up being significantly more complex than I realized.

I thought worst case scenario was to create a custom decoder analogous to the custom encoder. It turns out that cbor2 implemented the decoder in such a way that you not only cannot do it, but you also cannot easily subclass their decoder to make the nominal change needed to support this fix.

I opened an issue. I have a nominally refactored version of their code that would at least allow us to subclass the decoder. I'll submit a PR for that and see how that goes.

agronholm/cbor2#224

[cffls]

Fix WIP: #331