XRPLF / rippled

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

Fix offer crossing with transfer fee over AMM and other fixes #5016

Closed gregtatcam closed 1 month ago

gregtatcam commented 1 month ago

High Level Overview of Change

This PR addresses three issues:

Type of Change

Before / After

  1. qualityUpperBound() and QualityFunction calculation doesn't factor in the transfer fee on offer crossing. This is correct behavior when crossing an account offer. Single-path with AMM limits the out value based on the limitQuality. If the transfer fee is not factored in then the out value is going to be such that the generated AMM offer quality matches the limitQuality. But the effective quality is smaller because takerPays amount factors in the transfer fee when the strand's quality is calculated. The fix factors in the transfer fee when the quality is adjusted for the fees on offer crossing.
  2. adjustAmountsByLPTokens() function is not returning adjusted tokens and amounts in some cases. The fix returns the adjusted LPToken and adjusted amounts.
  3. Liquidity Provider's (LP) LPToken trustline balances might not add up to the total LPTokenBalance due to the rounding. This results in a dust amount left in the balances on the last LP withdrawal or the withdrawal transaction failing. The fix sets the LPTokenBalance to the trustline balance if LP is the only remaining LP on withdrawal transaction.

Test Plan

Updated AMM_test and AMMExtended unit-tests.

codecov-commenter commented 1 month ago

Codecov Report

Attention: Patch coverage is 91.57895% with 8 lines in your changes are missing coverage. Please review.

Project coverage is 71.1%. Comparing base (2705109) to head (15390be).

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/XRPLF/rippled/pull/5016/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/5016?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XRPLF) ```diff @@ Coverage Diff @@ ## develop #5016 +/- ## ========================================= + Coverage 71.0% 71.1% +0.1% ========================================= Files 796 796 Lines 66912 66997 +85 Branches 10988 10979 -9 ========================================= + Hits 47540 47635 +95 + Misses 19372 19362 -10 ``` | [Files](https://app.codecov.io/gh/XRPLF/rippled/pull/5016?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XRPLF) | Coverage Δ | | |---|---|---| | [src/ripple/app/misc/impl/AMMHelpers.cpp](https://app.codecov.io/gh/XRPLF/rippled/pull/5016?src=pr&el=tree&filepath=src%2Fripple%2Fapp%2Fmisc%2Fimpl%2FAMMHelpers.cpp&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XRPLF#diff-c3JjL3JpcHBsZS9hcHAvbWlzYy9pbXBsL0FNTUhlbHBlcnMuY3Bw) | `97.6% <100.0%> (+0.4%)` | :arrow_up: | | [src/ripple/app/misc/impl/AMMUtils.cpp](https://app.codecov.io/gh/XRPLF/rippled/pull/5016?src=pr&el=tree&filepath=src%2Fripple%2Fapp%2Fmisc%2Fimpl%2FAMMUtils.cpp&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XRPLF#diff-c3JjL3JpcHBsZS9hcHAvbWlzYy9pbXBsL0FNTVV0aWxzLmNwcA==) | `99.4% <100.0%> (+0.2%)` | :arrow_up: | | [src/ripple/protocol/impl/Feature.cpp](https://app.codecov.io/gh/XRPLF/rippled/pull/5016?src=pr&el=tree&filepath=src%2Fripple%2Fprotocol%2Fimpl%2FFeature.cpp&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XRPLF#diff-c3JjL3JpcHBsZS9wcm90b2NvbC9pbXBsL0ZlYXR1cmUuY3Bw) | `93.9% <ø> (ø)` | | | [src/ripple/app/paths/impl/BookStep.cpp](https://app.codecov.io/gh/XRPLF/rippled/pull/5016?src=pr&el=tree&filepath=src%2Fripple%2Fapp%2Fpaths%2Fimpl%2FBookStep.cpp&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XRPLF#diff-c3JjL3JpcHBsZS9hcHAvcGF0aHMvaW1wbC9Cb29rU3RlcC5jcHA=) | `93.1% <95.0%> (+1.1%)` | :arrow_up: | | [src/ripple/app/tx/impl/AMMWithdraw.cpp](https://app.codecov.io/gh/XRPLF/rippled/pull/5016?src=pr&el=tree&filepath=src%2Fripple%2Fapp%2Ftx%2Fimpl%2FAMMWithdraw.cpp&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XRPLF#diff-c3JjL3JpcHBsZS9hcHAvdHgvaW1wbC9BTU1XaXRoZHJhdy5jcHA=) | `91.2% <66.7%> (+0.8%)` | :arrow_up: | ... and [2 files with indirect coverage changes](https://app.codecov.io/gh/XRPLF/rippled/pull/5016/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/5016/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/5016?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XRPLF)