Skip to content

Conversation

@f-f
Copy link
Contributor

@f-f f-f commented Jan 15, 2026

Description

Fix #607

Checklist

  • Commit sequence broadly makes sense and commits have useful messages
  • New tests are added if needed and existing tests are updated
  • All visible changes are prepended to the latest section of a CHANGELOG.md for the affected packages.
    New section is never added with the code changes. (See RELEASING.md)
  • When applicable, versions are updated in .cabal and CHANGELOG.md files according to the
    versioning process.
  • The version bounds in .cabal files for all affected packages are updated.
    If you change the bounds in a cabal file, that package itself must have a version increase. (See RELEASING.md)
  • Self-reviewed the diff

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.

It would be useful to export conversion functions that allows to wrap with forcing and unwrap the underlying types:

wrapIPv4 :: IP.IPv4 -> IPv4
wrapIPv4 = IPv4

unwrapIPv4 :: IPv4 -> IP.IPv4
unwrapIPv4 = unIPv4

wrapIPv6 :: IP.IPv6 -> IPv6
wrapIPv6 ipv6 = 
  case IP.toIPv6w ipv6 of
    (!w1, !w2, !w3, !w4) -> IPv6 ipv6 

unwrapIPv6 :: IPv6 -> IP.IPv6
unwrapIPv6 = unIPv6

If we did that, then this would be the new implementation of toIPv6w:

toIPv6w :: (Word32, Word32, Word32, Word32) -> IPv6
toIPv6w = wrapIPv6 . IP.toIPv6w

Instead of wrap prefix we could use mk or something else entirely, I don't have any strong preference about it.

We also need to add FromCBOR and ToCBOR instances for those two new types to the cardano-binary.

@f-f f-f requested a review from a team as a code owner January 19, 2026 14:36
@f-f
Copy link
Contributor Author

f-f commented Jan 19, 2026

This should be good for another look

Copy link
Contributor

@neilmayhew neilmayhew left a comment

Choose a reason for hiding this comment

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

Looks great!

Just a few minor suggestions.

Copy link
Contributor

@neilmayhew neilmayhew left a comment

Choose a reason for hiding this comment

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

Looking good!

@neilmayhew
Copy link
Contributor

Feel free to resolve my comments while I'm away

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.

Beautiful!
Thank you!

@lehins lehins enabled auto-merge January 22, 2026 18:52
@lehins lehins merged commit d78f2f3 into master Jan 22, 2026
22 checks passed
@lehins lehins deleted the f-f/607 branch January 22, 2026 19:00
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.

Move IPv4 and IPv6 from ledger

4 participants