Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Datum round-trip corrupts datum bytes #1214

Closed
Quantumplation opened this issue Aug 1, 2022 · 5 comments · Fixed by #1216
Closed

Datum round-trip corrupts datum bytes #1214

Quantumplation opened this issue Aug 1, 2022 · 5 comments · Fixed by #1216
Assignees
Labels
bug Something isn't working

Comments

@Quantumplation
Copy link

Problem Report
The cardano-ledger code provides no guarantees about round-tripping the serialization; For example, if a datum uses fixed-length arrays in it's CBOR encoding, the cardano-ledger will re-serialize it with indefinite-length arrays.

WIth cardano-db-sync, this manifests as storing the wrong bytes for a datum. For example, consider this datum:
https://testnet.cexplorer.io/datum/06269f665176dc65b334d685b2f64826c092875ca77e15007b5f690ecc9db1af

(cexplorer.io is powered by cardano-db-sync)

If you grab the bytes seen at the bottom of the page, they will not hash to 06269... as expected. It turns out, this is because the bytes on the wire are different:

D8798441FFD87982D87982D87982D87981581CC279A3FB3B4E62BBC78E288783B58045D4AE82A18867D8352D02775AD87981D87981D87981581C121FD22E0B57AC206FEFC763F8BFA0771919F5218B40691EEA4514D0D87A80D87A801A002625A0D87983D879801A000F4240D879811A000FA92E

This can cause issues with down-stream applications that depend on this data, such as making the UTXO un-spendable, since the app has the incorrect datum as a witness.

We originally encountered this with Ogmios, and the lovely @KtorZ helped me figure out what was going on, and fixed it in Ogmios. For an example of this fix, and/or a more detailed writeup, see:
CardanoSolutions/ogmios@3f614c3

I also opened a ticket for discussion with the Ledger team on this, seen here:
IntersectMBO/cardano-ledger#2943

@Quantumplation Quantumplation added the bug Something isn't working label Aug 1, 2022
@kderme
Copy link
Contributor

kderme commented Aug 1, 2022

Thank you for reporting. It seems reasonable to use the memoized bytes instead of redoing the serialisation both in terms of hash correctness and performance.

@dcoutts
Copy link
Contributor

dcoutts commented Aug 3, 2022

@kderme yes indeed. The principle that the ledger follows is to remember the original bytes and to never re-serialise, and db-sync should do the same.

@ArturWieczorek
Copy link
Contributor

Datum test failed when run on DBSYNC_REV="13.0.4-rc1-fix-datum" :

>       assert (
            db_cbor_hex == orig_cbor_hex
        ), "Datum bytes in db-sync doesn't correspond to the original datum"
E       AssertionError: Datum bytes in db-sync doesn't correspond to the original datum
E       assert 'd8799f41ffd8799fd8799fd8799fd8799f581cc279a3fb3b4e62bbc78e288783b58045d4ae82a18867d8352d02775affd8799fd8799fd8799f581c121fd22e0b57ac206fefc763f8bfa0771919f5218b40691eea4514d0ffffffffd87a80ffd87a80ff1a002625a0d8799fd879801a000f4240d8799f1a000fa92effffff' == 'd8798441ffd87982d87982d87982d87981581cc279a3fb3b4e62bbc78e288783b58045d4ae82a18867d8352d02775ad87981d87981d87981581c121fd22e0b57ac206fefc763f8bfa0771919f5218b40691eea4514d0d87a80d87a801a002625a0d87983d879801a000f4240d879811a000fa92e'

@kderme
Copy link
Contributor

kderme commented Sep 13, 2022

I believe this has been fixed and the observed issue is actually in CLI IntersectMBO/cardano-node#4433. Feel free to reopen if not

@JaredCorduan
Copy link

For example, if a datum uses fixed-length arrays in it's CBOR encoding, the cardano-ledger will re-serialize it with indefinite-length arrays.

Just to clarify for posterity, the ledger does not re-serialize any user-submitted data!

sorki added a commit to blockfrost/blockfrost-backend-ryo that referenced this issue Dec 15, 2022
Re-serialization error caused some of these to not match the original
bytes. This is fixed in `cardano-db-sync >= 13.0.5`, see
also IntersectMBO/cardano-db-sync#1214
sorki added a commit to blockfrost/blockfrost-backend-ryo that referenced this issue Dec 15, 2022
Re-serialization error caused some of these to not match the original
bytes. This is fixed in `cardano-db-sync >= 13.0.5`, see
also IntersectMBO/cardano-db-sync#1214
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants