AMM Unit test: Create AMM Pools with tiny pool balances

ckeshava commented 2 months ago

High Level Overview of Change

This PR introduces new unit tests to test small AMM Pool creation and tiny Payment transactions against AMMs. AMM pool sizes are in order of 1e-6 and payment transactions are in order of 1e-7. Please let me know if I should test smaller ranges.

The behavior of the code matches our understanding for both XRP/IOU and IOU/IOU AMM pools. Many thanks to @gregtatcam for helping with these tests.

Context of Change

Type of Change

[ ] Bug fix (non-breaking change which fixes an issue)
[ ] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
[ ] Refactor (non-breaking change that only restructures code)
[ ] Performance (increase or change in throughput and/or latency)
[x] Tests (you added tests for code that already exists, or your new feature included in this PR)
[ ] Documentation update
[ ] Chore (no impact to binary, e.g. .gitignore, formatting, dropping support for older tooling)
[ ] Release

API Impact

[ ] Public API: New feature (new methods and/or new fields)
[ ] Public API: Breaking change (in general, breaking changes should only impact the next api_version)
[ ] libxrpl change (any change that may affect libxrpl or dependents of libxrpl)
[ ] Peer protocol change (must be backward compatible or bump the peer protocol version)

No impact on performance. No changes to Public APIs either.

ckeshava commented 2 months ago

@seelabs can you please review this PR? I'm not able to add reviewers through the Github UI

codecov-commenter commented 2 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 76.97%. Comparing base (c88166e) to head (1e2cb43).

:exclamation: Current head 1e2cb43 differs from pull request most recent head 8b6382b. Consider uploading reports for the commit 8b6382b to get more accurate results

Additional details and impacted files

```diff @@ Coverage Diff @@ ## develop #4981 +/- ## =========================================== - Coverage 76.97% 76.97% -0.01% =========================================== Files 1129 1129 Lines 131914 131940 +26 Branches 39629 39693 +64 =========================================== + Hits 101539 101556 +17 - Misses 24459 24497 +38 + Partials 5916 5887 -29 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

seelabs commented 2 months ago

I think I'd separate these tests out as a separate test case - I'm not sure they belong in "basic" tests. Maybe call it "extreme values test" or somesuch. I'd also tests every combination of the following amounts. The amounts should be used for both AMM pool balances and for transaction amounts (deposit/withdrawal/payments)

max allowed IOU
10^-9 IOU
min allowed IOU
1 drop
2 drops
1 XRP
10^7 XRP
1 IOU
10^7 IOU

I'd expect that you'd want to create these combinations programmatically.

ckeshava commented 2 months ago

@seelabs what would be the appropriate values for minimum and maximum allowed IOUs? STAmount represents numbers with a mantissa and an exponent. I believe these numbers can have 16 significant digits. So the max and min values would be 1e16 and 1e-16 ?

ckeshava commented 2 months ago

FYI: When using IOUs with AMMDeposit, Payment and AMMWithdraw transactions, it is not feasible to use less than 1e-3 values. The individual transactions have differing limits, but this is the common denominator. 1e-3 appears to be the minimum value of IOUs

XRPLF / rippled