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

Cleanup pointer serialization #4831

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

lehins
Copy link
Collaborator

@lehins lehins commented Jan 11, 2025

Description

Previously definition of a pointer used Word64 type for slot number, transaction and certificate indices. We can finally use narrower types and treat bogus pointers as invalid for all eras retroactively. There were a few stages to this that were necessary over the last few years:

  1. In Babbage era deserialization of transactions prevented pointers with values outside of Word32 for slot number and Word16 for transaction and certificate indices
  2. Upon entry to Conway when translating ledger state all pointers that didn't fit in those bounded types without overflowing were converted to an all zero pointer: Ptr 0 0 0
  3. Once in Conway (i.e. finally we are) we can pretend like all bogus pointers where always clamped to zero. The reason why we can do that retroactively because all those bogus pointers that exist on mainnet never affected reward calculation or anything else for that matter, so clamping them to zero permanently will not affect that in any way. In order to do that we need to do pointer normalization (i.e. clamping to zero of bogus pointers) during deserialization of transactions, instead of during translation into Conway.

Step 3 is implemented in this PR.

This was a long awaited cleanup that is finally here.

Checklist

  • Commits in meaningful sequence and with useful messages
  • Tests added or updated when needed
  • CHANGELOG.md files updated for packages with externally visible changes

    New section is never added with the code changes. (See RELEASING.md)
  • Versions updated in .cabal and CHANGELOG.md files when necessary, according to the
    versioning process.
  • Version bounds in .cabal files updated when necessary

    If you change the bounds in a cabal file, that package itself must have a version increase. (See RELEASING.md)
  • Code formatted (use scripts/fourmolize.sh)
  • Cabal files formatted (use scripts/cabal-format.sh)
  • hie.yaml updated (use scripts/gen-hie.sh)
  • Self-reviewed the diff

@lehins lehins force-pushed the lehins/cleanup-pointer-serialization branch 3 times, most recently from 87ff626 to bcb7fda Compare January 12, 2025 04:19
@lehins lehins force-pushed the lehins/cleanup-pointer-serialization branch from bcb7fda to 2f78d3f Compare January 12, 2025 15:50
@lehins lehins force-pushed the lehins/cleanup-pointer-serialization branch from 2f78d3f to fd2fa12 Compare January 12, 2025 16:01
@lehins lehins marked this pull request as ready for review January 12, 2025 16:01
@lehins lehins requested a review from a team as a code owner January 12, 2025 16:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant