XRPLF / rippled

Decentralized cryptocurrency blockchain daemon implementing the XRP Ledger protocol in C++
https://xrpl.org
ISC License
4.48k stars 1.45k forks source link

AMM Unit tests: rounding down of equal asset deposit LPToken calculation #4982

Closed ckeshava closed 2 months ago

ckeshava commented 2 months ago

High Level Overview of Change

This PR exercises a specific portion of the AMMHelpers.cpp file. It triggers a downward rounding of LPTokens in adjustLPTokens function in the AMMHelpers.cpp file.

Context of Change

Type of Change

API Impact

codecov-commenter commented 2 months ago

Codecov Report

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

Project coverage is 70.9%. Comparing base (24a275b) to head (8f7dbfb).

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/XRPLF/rippled/pull/4982/graphs/tree.svg?width=650&height=150&src=pr&token=i2RPGI5xGF&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XRPLF)](https://app.codecov.io/gh/XRPLF/rippled/pull/4982?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XRPLF) ```diff @@ Coverage Diff @@ ## develop #4982 +/- ## ======================================= Coverage 70.9% 70.9% ======================================= Files 796 796 Lines 66727 66727 Branches 10981 10976 -5 ======================================= + Hits 47333 47338 +5 + Misses 19394 19389 -5 ``` [see 2 files with indirect coverage changes](https://app.codecov.io/gh/XRPLF/rippled/pull/4982/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XRPLF) [![Impacted file tree graph](https://app.codecov.io/gh/XRPLF/rippled/pull/4982/graphs/tree.svg?width=650&height=150&src=pr&token=i2RPGI5xGF&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XRPLF)](https://app.codecov.io/gh/XRPLF/rippled/pull/4982?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XRPLF)
HowardHinnant commented 2 months ago

This looks good to me. But I was hoping it would hit these lines: https://app.codecov.io/gh/XRPLF/rippled/pull/4982/blob/src/ripple/app/misc/impl/AMMHelpers.cpp#L171. And I don't have a good explanation as to why it didn't. Nevertheless, this looks like a good addition to me.

ckeshava commented 2 months ago

@HowardHinnant I have updated the numbers so that the execution hits the necessary lines.

I expected 10'000.1 to be rounded down to 10'000 and consequently hit the said line. I think my debugger is malfunctioning.

ckeshava commented 2 months ago

Hello, the commit message could be: Unit test to examine the rounding behavior of equal-asset deposits with tfLPToken flag

@ximinez