Skip to content

Implement proper to_bigint and rename the current one to something appropriate #1074

@pefontana

Description

@pefontana

During development of the cairo-felt, we chose to name the routine converting to the [-P/2..P/2] signed representation of field elements Felt252::to_bigint(&self).
At the same time, relatively often we need to get the equivalent to felt.to_biguint().to_bigint() to get the exact same representation typed as a signed BigInt. This choice of name leads to developers often using .to_bigint() expecting the latter behavior, which leads to bugs. Because in the range [0..P/2] the value is the same, it's also often not caught by tests, and relies on developer folklore during the code reviews (luckily, most times it was caught, but no guarantees there).

The task is making .to_bigint() behave as everyone would expect and to rename the old one to something that makes sense (possibly .to_signed_felt() or something similar), and carefully audit uses to see whether they need updating to the new name or the original code was actually buggy.

It could be done in two steps if that makes it easier, as currently correct code does .to_biguint().to_bigint() when it just wants the regular type conversions, so the developer could do the renaming first and the conversion routine later.

Metadata

Metadata

Assignees

Labels

enhancementNew feature or request

Type

No type

Projects

Status

Done

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions