Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

impl Arbitrary for BigDecimal #122

Open
Ekleog opened this issue Jan 10, 2024 · 3 comments
Open

impl Arbitrary for BigDecimal #122

Ekleog opened this issue Jan 10, 2024 · 3 comments

Comments

@Ekleog
Copy link

Ekleog commented Jan 10, 2024

Hey!

I'm trying to fuzz some code that uses BigDecimal, and am hitting the issue that it does not implement Arbitrary. Currently, my solution is the following code to generate one:

BigDecimal::new(u.arbitrary()?, u.arbitrary::<i64>()? % 1000000)

I originally did not have the % 1000000, but it resulted in memory allocation failures.

What's the recommended way to use BigDecimal from a fuzzer? Relatedly, maybe it'd make sense to add an arbitrary feature to BigDecimal, allowing to auto-generate them?

Anyway, thank you for BigDecimal! I'm currently stuck on 0.3 because sqlx didn't update probably due to #117 ; but it's really coming in handy to not have to care about floating points and integers too much :)

@akubera
Copy link
Owner

akubera commented Jan 15, 2024

If you check out my property tests I'm using proptest to generate f32 and pairs of (i128, i8) to use as (mantissa, exponent), and using those to build random BigDecimals on which I can test properties like (a - b) == -(b - a).

Doing something more robust would be nice, but it's pretty low on the todo-list.

What's the standard procedure for Arbitrary? Is it add a feature and then just:

#[cfg_attr(feature='arbitrary', derive(Arbitrary)]
struct BigDecimal { ... }

and as long as BigInt implements it too, it works? or would the mod 1000000 be required and it needs something more configurable by the user? Something like ArbitraryBigDecimalBuilder::max(decimal_places)...?

I originally did not have the % 1000000, but it resulted in memory allocation failures.

Yeah I've hit that too :-(.

@Ekleog
Copy link
Author

Ekleog commented Jan 17, 2024

Arbitrary impls usually don't have anything more configurable by the user. Considering the scale issue, I think an impl Arbitrary for BigDecimal { ... } where the contents of the function is basically what I put in my first message should work out fine, and be much more usable for the user than the statu quo :)

That said, if you hit it too, TBH I'm not entirely sure why a too big scale leads to memory allocation issues, so this might actually be some bug inside bigdecimal itself? I thought that for me it was just due to serializing to string, and thus allocating too long a string.

(For full disclosure, I eventually noticed I don't actually need arbitrary decimals, and switched to the finite-length rust_decimal crate since opening this issue, so it went down quite a bit in my priority list too)

@akubera
Copy link
Owner

akubera commented Jan 17, 2024

I think the bug is in the implementation of Debug/Display, where the decimal is printed in non-exponential form, even when it'll write exabytes of leading/trailing zeros. I think that's what #121 is addressing but I haven't reviewed it, yet.

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

No branches or pull requests

2 participants