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

`fixAMMv1_1`: improve the quality of the generated AMM offer in certain cases #4983

Closed gregtatcam closed 1 month ago

gregtatcam commented 2 months ago

High Level Overview of Change

Introduces fixAMMOfferRounding (subsequently combined with #5016 and renamed to fixAMMv1_1) amendment to improve quality of the generated AMM offer for a single path payment when one side of the offer is XRP and there is a Limit Order Book (LOB) offer for the same asset pair on the DEX.

Context of Change

A single-path AMM offer with account offer on DEX, is always generated starting with the takerPays first, which is rounded up, and then the takerGets, which is rounded down. This rounding ensures that the pool's product invariant is maintained. However, when one of the offer's side is XRP, this rounding can result in the AMM offer having a lower quality, potentially causing offer generation to fail if the quality is lower than the account's offer quality.

To address this issue, the proposed fix adjusts the offer generation process to start with the XRP side first and always rounds it down. This results in a smaller offer size, improving the offer's quality. This change still ensures the product invariant, as the other generated side is the exact result of the swap-in or swap-out equations.

Type of Change

API Impact

Test Plan

A unit-test testFixAMMOfferRounding() is added to AMM_test to verify this amendment

codecov-commenter commented 2 months ago

Codecov Report

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

Project coverage is 71.0%. Comparing base (244ac5e) to head (7b76a0a).

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/XRPLF/rippled/pull/4983/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/4983?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XRPLF) ```diff @@ Coverage Diff @@ ## develop #4983 +/- ## ========================================= + Coverage 71.0% 71.0% +0.1% ========================================= Files 796 796 Lines 66793 66912 +119 Branches 10987 10988 +1 ========================================= + Hits 47420 47539 +119 Misses 19373 19373 ``` | [Files](https://app.codecov.io/gh/XRPLF/rippled/pull/4983?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/4983?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.1% <100.0%> (+0.3%)` | :arrow_up: | | [src/ripple/app/paths/impl/BookStep.cpp](https://app.codecov.io/gh/XRPLF/rippled/pull/4983?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=) | `92.0% <100.0%> (+0.3%)` | :arrow_up: | | [src/ripple/app/tx/impl/AMMCreate.cpp](https://app.codecov.io/gh/XRPLF/rippled/pull/4983?src=pr&el=tree&filepath=src%2Fripple%2Fapp%2Ftx%2Fimpl%2FAMMCreate.cpp&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XRPLF#diff-c3JjL3JpcHBsZS9hcHAvdHgvaW1wbC9BTU1DcmVhdGUuY3Bw) | `90.2% <ø> (ø)` | | | [src/ripple/basics/Number.h](https://app.codecov.io/gh/XRPLF/rippled/pull/4983?src=pr&el=tree&filepath=src%2Fripple%2Fbasics%2FNumber.h&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XRPLF#diff-c3JjL3JpcHBsZS9iYXNpY3MvTnVtYmVyLmg=) | `100.0% <100.0%> (ø)` | | | [src/ripple/ledger/impl/View.cpp](https://app.codecov.io/gh/XRPLF/rippled/pull/4983?src=pr&el=tree&filepath=src%2Fripple%2Fledger%2Fimpl%2FView.cpp&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XRPLF#diff-c3JjL3JpcHBsZS9sZWRnZXIvaW1wbC9WaWV3LmNwcA==) | `91.6% <100.0%> (ø)` | | | [src/ripple/protocol/Feature.h](https://app.codecov.io/gh/XRPLF/rippled/pull/4983?src=pr&el=tree&filepath=src%2Fripple%2Fprotocol%2FFeature.h&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XRPLF#diff-c3JjL3JpcHBsZS9wcm90b2NvbC9GZWF0dXJlLmg=) | `100.0% <ø> (ø)` | | | [src/ripple/protocol/impl/Feature.cpp](https://app.codecov.io/gh/XRPLF/rippled/pull/4983?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/protocol/impl/STAmount.cpp](https://app.codecov.io/gh/XRPLF/rippled/pull/4983?src=pr&el=tree&filepath=src%2Fripple%2Fprotocol%2Fimpl%2FSTAmount.cpp&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XRPLF#diff-c3JjL3JpcHBsZS9wcm90b2NvbC9pbXBsL1NUQW1vdW50LmNwcA==) | `90.0% <ø> (+0.1%)` | :arrow_up: | | [src/ripple/app/misc/AMMHelpers.h](https://app.codecov.io/gh/XRPLF/rippled/pull/4983?src=pr&el=tree&filepath=src%2Fripple%2Fapp%2Fmisc%2FAMMHelpers.h&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XRPLF#diff-c3JjL3JpcHBsZS9hcHAvbWlzYy9BTU1IZWxwZXJzLmg=) | `85.7% <95.8%> (+16.7%)` | :arrow_up: | | [src/ripple/app/paths/impl/AMMLiquidity.cpp](https://app.codecov.io/gh/XRPLF/rippled/pull/4983?src=pr&el=tree&filepath=src%2Fripple%2Fapp%2Fpaths%2Fimpl%2FAMMLiquidity.cpp&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XRPLF#diff-c3JjL3JpcHBsZS9hcHAvcGF0aHMvaW1wbC9BTU1MaXF1aWRpdHkuY3Bw) | `84.6% <25.0%> (-3.3%)` | :arrow_down: | ... and [4 files with indirect coverage changes](https://app.codecov.io/gh/XRPLF/rippled/pull/4983/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/4983/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/4983?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XRPLF)
gregtatcam commented 1 month ago

I pushed commit, which should address all feedback. The commit message details all the updates, but the main change is that the rounding is changed to nearest when the AMM offer matching target quality is generated.