Skip to content

Conversation

Soupstraw
Copy link
Contributor

Description

close #5181

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.
    NOTE: 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.
    NOTE: If bounds change 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).
  • CDDL files are up to date (use scripts/gen-cddl.sh)
  • hie.yaml updated (use scripts/gen-hie.sh).
  • Self-reviewed the diff.

@Soupstraw Soupstraw force-pushed the jj/tx-size-word32 branch 2 times, most recently from e8df1b2 to 9745172 Compare August 27, 2025 12:09
@Soupstraw Soupstraw marked this pull request as ready for review August 27, 2025 12:09
@Soupstraw Soupstraw requested a review from a team as a code owner August 27, 2025 12:09
Copy link
Collaborator

@lehins lehins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good overall, but we need to guard against overflows and underflows.
In this particular PR there would be no danger for mainnet, since there are other checks in place that would prevent it from happen, however, it does not mean we should not do our due diligence.

@Soupstraw Soupstraw requested a review from lehins August 28, 2025 11:32
@Soupstraw Soupstraw force-pushed the jj/tx-size-word32 branch 2 times, most recently from 8167b20 to 5d690ff Compare August 28, 2025 12:14
Copy link
Collaborator

@lehins lehins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just noticed that the length function of LazyByteString actually returns an Int64, not a Word64, so it should be fine to use fromIntegral @int64 @word32 here since maxBound of Int64 is representable with a Word32.

This is totally incorrect.

@Soupstraw Soupstraw force-pushed the jj/tx-size-word32 branch 2 times, most recently from 50b58d3 to 5513026 Compare September 1, 2025 10:12
@Soupstraw Soupstraw requested a review from lehins September 1, 2025 10:13
@Soupstraw Soupstraw force-pushed the jj/tx-size-word32 branch 2 times, most recently from 04b2c18 to c31b4a4 Compare September 1, 2025 12:24
Copy link
Collaborator

@lehins lehins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect! Thank you!

Co-authored-by: Alexey Kuleshevich <[email protected]>
@Soupstraw Soupstraw merged commit 403eb94 into master Sep 3, 2025
121 of 122 checks passed
@Soupstraw Soupstraw deleted the jj/tx-size-word32 branch September 3, 2025 14:36
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.

Change type of sizeTxF to Word32
2 participants