IntersectMBO / plutus

The Plutus language implementation and tools
Apache License 2.0
1.57k stars 479 forks source link

`PlutusTx.Ratio.reduce` allows negative integer in the denominator #5722

Closed SeungheonOh closed 8 months ago

SeungheonOh commented 9 months ago

Summary

Documentation on PlutusTx.Ratio.denominator says that the "[denominator] will always be greater than, or equal to, 1, although the type does not describe this." However, using PlutusTx.Ratio.reduce, it is possible to build negative rational with negative integer on the denominator.

Steps to reproduce the behavior

λ> import PlutusTx.Ratio
λ> denominator $ reduce 1 (-1)

Actual Result

-1

Expected Result

1

Describe the approach you would take to fix this

PlutusTx.Ratio.reduce can be deprecated entirely since PlutusTx.Ratio.unsafeRatio serves identical purpose while correctly taking care of negative integer on denominator input.

Also, adding invariant notes to PlutusTx.Ratio.Rational's haddock documentation would be helpful. As it is currently not included in haddock: https://github.com/IntersectMBO/plutus/blob/master/plutus-tx/src/PlutusTx/Ratio.hs#L69-L72

System info

Linux Mars 6.1.41 #1-NixOS SMP PREEMPT_DYNAMIC Mon Jul 24 16:55:35 UTC 2023 x86_64 GNU/Linux
NixOS 23.11.20230727.db8672b (Tapir) x86_64
plutus-tx: 1.7.0.0
zliu41 commented 8 months ago

Thanks for reporting the bug @SeungheonOh