Python-Cardano / pycardano

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

Creating an invalid tx when trying to mint too large amount of tokens #336

Closed nielstron closed 5 months ago

nielstron commented 6 months ago

Describe the bug Trying to mint too many tokens produces and invalid transaction (basically minting 1231234145232534000000 here for example) This results in ogmios failing to evaluate/submit the transaction with a very obscure error message.

To Reproduce TBD

Logs

ogmios.errors.ResponseError: Ogmios responded with error: {'jsonrpc': '2.0', 'method': 'evaluateTransaction', 'error': {'code': -32602, 'message': "Invalid transaction; It looks like the given transaction wasn't well-formed. Note that I try to decode the transaction in every possible era and it was malformed in ALL eras. Yet, I can't pinpoint the exact issue for I do not know in which era / format you intended the transaction to be. The 'data' field, therefore, contains errors for each era.", 'data': {'babbage': "invalid or incomplete value of type 'Transaction': expected word", 'conway': "invalid or incomplete value of type 'Transaction': expected word", 'alonzo': "invalid or incomplete value of type 'Transaction': expected list len or indef", 'mary': "invalid or incomplete value of type 'Transaction': Size mismatch when decoding Object / Array. Expected 4, but found 3.", 'allegra': "invalid or incomplete value of type 'Transaction': Size mismatch when decoding Object / Array. Expected 4, but found 3.", 'shelley': "invalid or incomplete value of type 'Transaction': Size mismatch when decoding Object / Array. Expected 4, but found 3."}}, 'id': None}

Expected behavior Ogmios could report that the integer is breaking/incorrectly serialized. PyCardano should abort the building or raise an error/warning this.

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

Additional context Looping in @KtorZ for ogmios, but the error is really with PyCardano here.

KtorZ commented 6 months ago

Indeed, the mint value must fit into a word which is a u64 or u32 depending on OS arch, but most likely u64 (so 18_446_744_073_709_551_615 max).

KtorZ commented 6 months ago

Note that: this is as good as it gets when it comes to deserialization errors here. I cannot get anything better without re-writing the CBOR parsers which I am slightly reluctant to at the moment 😅 ...

nielstron commented 6 months ago

So it should be possible to just check that mint is smaller than sys.maxsize.

cffls commented 6 months ago

From the ledger spec, it looks like mint field should be int64, and uint should be used in all other cases, which doesn't have an upper bound (at least I couldn't find its definition in the spec). Maybe we should only limit mint field to int64. What do you think?

nielstron commented 6 months ago

If the spec sais i64 I agree we should restrict to that, not sys.maxsize