Python-Cardano / pycardano

A lightweight Cardano library in Python
https://pycardano.readthedocs.io
MIT License
214 stars 66 forks source link

Staking certificate to_cbor -> from_cbor gives incorrect result #379

Open gszabo opened 2 days ago

gszabo commented 2 days ago

Describe the bug Serializing a list of Certificate into CBOR and deserializing them back gives incorrect result.

To Reproduce The following pytest test demonstrates the issue:

import os

from pycardano import (
    DeserializeException,
    PoolKeyHash,
    StakeCredential,
    StakeDelegation,
    StakeRegistration,
    StakeSigningKey,
    TransactionBody,
)

def test_staking_certificate_serdes():
    staking_key = StakeSigningKey.generate()
    stake_pool_key_hash = os.urandom(28)

    transaction_body = TransactionBody(
        certificates=[
            StakeRegistration(stake_credential=StakeCredential(staking_key.to_verification_key().hash())),
            StakeDelegation(
                stake_credential=StakeCredential(staking_key.to_verification_key().hash()),
                pool_keyhash=PoolKeyHash(stake_pool_key_hash),
            ),
        ]
    )

    after_serdes = TransactionBody.from_cbor(transaction_body.to_cbor())

    assert after_serdes == transaction_body

Logs The error:

Differing attributes:
['certificates']

Drill down into differing attribute certificates:
  certificates: [{\n  '_CODE': 0,\n  'stake_credential': {\n    '_CODE': 0,\n    'credential': VerificationKeyHash(hex='f550e0ac4369d67e685aba52830fb63f15e1bbcbb253ac46283a6e0c'),\n  },\n}, {\n  '_CODE': 0,\n  'stake_credential': {\n    '_CODE': 0,\n    'credential': VerificationKeyHash(hex='f550e0ac4369d67e685aba52830fb63f15e1bbcbb253ac46283a6e0c'),\n  },\n}] != [{\n  '_CODE': 0,\n  'stake_credential': {\n    '_CODE': 0,\n    'credential': VerificationKeyHash(hex='f550e0ac4369d67e685aba52830fb63f15e1bbcbb253ac46283a6e0c'),\n  },\n}, {\n  '_CODE': 2,\n  'pool_keyhash': PoolKeyHash(hex='f771b70fb009511ea81d965faeb030abfa9033b48bb1ba2a0d2276e0'),\n  'stake_credential': {\n    '_CODE': 0,\n    'credential': VerificationKeyHash(hex='f550e0ac4369d67e685aba52830fb63f15e1bbcbb253ac46283a6e0c'),\n  },\n}]
  At index 1 diff: {\n  '_CODE': 0,\n  'stake_credential': {\n    '_CODE': 0,\n    'credential': VerificationKeyHash(hex='f550e0ac4369d67e685aba52830fb63f15e1bbcbb253ac46283a6e0c'),\n  },\n} != {\n  '_CODE': 2,\n  'pool_keyhash': PoolKeyHash(hex='f771b70fb009511ea81d965faeb030abfa9033b48bb1ba2a0d2276e0'),\n  'stake_credential': {\n    '_CODE': 0,\n    'credential': VerificationKeyHash(hex='f550e0ac4369d67e685aba52830fb63f15e1bbcbb253ac46283a6e0c'),\n  },\n}

  Full diff:
    [
        {
      '_CODE': 0,
      'stake_credential': {
        '_CODE': 0,
        'credential': VerificationKeyHash(hex='f550e0ac4369d67e685aba52830fb63f15e1bbcbb253ac46283a6e0c'),
      },
    },
        {
  -   '_CODE': 2,
  ?            ^
  +   '_CODE': 0,
  ?            ^
  -   'pool_keyhash': PoolKeyHash(hex='f771b70fb009511ea81d965faeb030abfa9033b48bb1ba2a0d2276e0'),
      'stake_credential': {
        '_CODE': 0,
        'credential': VerificationKeyHash(hex='f550e0ac4369d67e685aba52830fb63f15e1bbcbb253ac46283a6e0c'),
      },
    },
    ]

Expected behavior The StakeDelegation object is serialized and deserialized back into a StakeDelegation object and not into a StakeRegistration object (CODE=2 instead of CODE=0).

Environment and software version (please complete the following information):

Additional context I believe the StakeRegistration.from_primitive method lacks the expected code/type check that other Certificate classes have. This implementation fixes the test:

    @classmethod
    @limit_primitive_type(list)
    def from_primitive(
        cls: Type[StakeRegistration], values: Union[list, tuple]
    ) -> StakeRegistration:
        if values[0] == 0:
            return cls(stake_credential=StakeCredential.from_primitive(values[1]))
        else:
            raise DeserializeException(f"Invalid StakeRegistration type {values[0]}")
nielstron commented 2 days ago

Sounds about correct. I looked into this a while ago and noticed that a lot of classes are missing... because no on got bothered enough by their absence to add them :)