bug: Panic on too big Felt

[FabijanC]

Bug Report

types-rs version:

0.1.5

Current behavior:

When using Felt::from_hex or deserializing, if the input is too long, the code panics.

Expected behavior:

I'd prefer it to return Err rather than panic because I can do nothing about that except write my own wrapper, which beats the purpose of having a unified Felt type.

Steps to reproduce:

E.g.:

Felt::from_hex(&format!("0x{}", "f".repeat(65)))

But not just that, if you replace 65 with 63, it doesn't panic but gives this output:

0x7ffffffffffffeefffffffffffffffffffffffffffffffffffffffffffffffe

Other information:

[FabijanC]

I updated the steps to reproduce with more info.

[FabijanC]

Not opening a new issue because the following finding is relevant to this topic:

The expecting method of FeltVisitor is not implemented properly:

types-rs/crates/starknet-types-core/src/felt/mod.rs

Line 963 in a6bcf4a

formatter.write_str("Failed to deserialize hexadecimal string")

It results in errors like this:

Invalid block ID: invalid type: number, expected Failed to deserialize hexadecimal string at line 1 column 33'

[tdelabro]

This no longer panic for 65.
But, for 63 the value wraps. Which result in a felt with has a different value from the one that was given originally.

[tdelabro]

@0xLucqs @gabrielbosio what do you think?
Mathematically speaking, it is not necessarily an error, as numbers bigger than the field's max value do indeed wrap.
BUT, this method already prevents you from using a hex string longer that the field max num of chars once hex serialized.

Which means that you can only successfully parse numbers biggers than Felt::MAX that are lower or equal to 0xfff...repeat 64 time...ff. Which is really quirky and weird.

0x0 <= value <= 0x800000000000011000000000000000000000000000000000000000000000000: OK
0x800000000000011000000000000000000000000000000000000000000000000 < value <= 0xfffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff:
Ok (but wrap around)
0xfffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff < value: KO

I would add a check to prevent this but I think it will be better handled at the lambdawork level. Don't you think?

[0xLucqs]

maybe we should have 2 functions, one that errors and another one that wraps ?

[tdelabro]

@gabrielbosio any news from upstream?