Skip to content

Fix HardForkInitiationAction protocol_version: (major, minor) pair, not Fraction - #510

Open
Kshot3000 wants to merge 1 commit into
Python-Cardano:mainfrom
Kshot3000:fix/hardfork-initiation-protocol-version
Open

Kshot3000 wants to merge 1 commit into
Python-Cardano:mainfrom
Kshot3000:fix/hardfork-initiation-protocol-version

Conversation

@Kshot3000

Copy link
Copy Markdown

Problem

HardForkInitiationAction is unusable end to end. Its protocol_version field is declared as Fraction, but the Conway CDDL encodes the field as a two-element array:

protocol_version       = [major_protocol_version, uint .size 4]
major_protocol_version = 0 .. 12

That one wrong type breaks every direction:

  1. Construction with the declared type fails — __post_init__ unpacks the value as a pair, so HardForkInitiationAction(None, Fraction(11, 1)) raises TypeError: cannot unpack non-iterable Fraction object.
  2. Serialisation fails — constructing with the pair the docstring describes, (10, 0), then calling to_cbor() raises TypeError: Field 'protocol_version' should be of type <class 'fractions.Fraction'>.
  3. Parsing fails — a ProposalProcedure carrying a real wire-form action [1, <gov_action_id>, [10, 0]] raises DeserializeException; no hard-fork initiation proposal on chain can be decoded into this class.
  4. The version bound is stale — __post_init__ enforces major 1..10, rejecting 11, the protocol version live on mainnet today, and 0, both inside the CDDL's 0..12 range. (The existing test even enshrines this, asserting (11, 0) raises.)

I ran into this while building a Conway governance-proposals decoder against the CDDL: every other gov action round-tripped through pycardano, this one could not be represented at all.

Fix

  • governance.py: type protocol_version as Tuple[int, int] (the same convention pycardano.utils uses for version pairs) and validate the major version against the CDDL range 0..12.
  • serialization.py: teach _restore_typed_primitive to restore Tuple[...] fields, mirroring the existing List branch (including Tuple[X, ...]), so a parsed [major, minor] array comes back as a tuple. Previously any tuple-typed field fell through to DeserializeException, so this cannot change the behaviour of any field that deserialises today.
  • test_governance.py: update the stale-bound test to the CDDL range (13 now raises) and add CBOR round-trip tests — including the exact wire bytes for an initiation to protocol version 11 (8301f6820b00) and one carrying a previous-action id.

Verification

  • Before the fix, all three failure modes above reproduce on main; after it, construction, serialisation and parsing all work, and a CDDL-built ProposalProcedure with a [10, 0] hard-fork action parses and re-serialises byte-for-byte.
  • Full test suite: 643 passed, 5 xfailed.

Submitted by Kyle Cox — @Kshot9000 (https://x.com/kshot9000) · github.com/Kshot3000
Support: ADA addr1q8hnl6vl5a6k3rw3n5g3jtte696zcl76kfatzv7gpswa9r0dj7fma6klq55y4ffm7tf0em09udnyhuk4ah92pl5x9jpqjae44v

… minor) pair the ledger encodes

protocol_version was declared as Fraction, which made the action
unusable end to end: __post_init__ unpacks it as a (major, minor)
pair (a Fraction raises TypeError), serialising a pair raises a
field-type error, and parsing a real [major, minor] array raises
DeserializeException. The Conway CDDL encodes protocol_version as
[major_protocol_version, uint .size 4] with major in 0..12, while
__post_init__ also enforced a stale 1..10 bound that rejects the
protocol version live on mainnet today (11).

- governance.py: type protocol_version as Tuple[int, int] and
  validate major against the CDDL range 0..12.
- serialization.py: teach _restore_typed_primitive to restore
  Tuple[...] fields, mirroring the existing List branch, so a
  parsed [major, minor] array comes back as a tuple.
- test_governance.py: update the stale-bound test to the CDDL
  range and add CBOR round-trip tests asserting the exact wire
  bytes for a hard-fork initiation to protocol version 11.
@Kshot3000

Copy link
Copy Markdown
Author

One extra data point from verifying this branch: the Tuple[...] restore added here also fixes NewConstitution, whose constitution field is Tuple[Anchor, Optional[ScriptHash]]. On current main, a NewConstitution serialises fine but its own from_cbor raises DeserializeException on the tuple restore; on this branch the same action round-trips with equality. I ran into both failures while building governance-proposal test vectors from the Conway CDDL — the hard-fork action was unrepresentable at all three stages, and the constitution action was emit-only. No code change needed beyond what's already here; noting it so the restore's coverage is on record.

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