Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposed 2.1.1 #4968

Merged
merged 2 commits into from
Mar 27, 2024
Merged

Proposed 2.1.1 #4968

merged 2 commits into from
Mar 27, 2024

Conversation

seelabs
Copy link
Collaborator

@seelabs seelabs commented Mar 26, 2024

High Level Overview of Change

This is a hotfix release that fixes a critical bug when handling of a large synthetic AMM offers in the payment engine.

Type of Change

  • [ x] Bug fix (non-breaking change which fixes an issue)
  • [ x] Release

A large synthetic offer was not handled correctly in the payment engine.
This patch fixes that issue and introduces a new invariant check while
processing synthetic offers.
@bachase bachase changed the base branch from develop to master March 26, 2024 23:36
@seelabs seelabs force-pushed the master-next branch 2 times, most recently from 01f4987 to 42e41cf Compare March 27, 2024 00:51
@codecov-commenter
Copy link

codecov-commenter commented Mar 27, 2024

Codecov Report

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

Project coverage is 61.49%. Comparing base (d7d15a9) to head (42e41cf).

❗ Current head 42e41cf differs from pull request most recent head 561bc7a. Consider uploading reports for the commit 561bc7a to get more accurate results

Files Patch % Lines
src/ripple/app/paths/impl/AMMOffer.cpp 37.03% 15 Missing and 2 partials ⚠️
src/ripple/app/paths/impl/AMMLiquidity.cpp 45.00% 8 Missing and 3 partials ⚠️
src/ripple/app/paths/impl/BookStep.cpp 0.00% 3 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4968      +/-   ##
==========================================
- Coverage   61.53%   61.49%   -0.04%     
==========================================
  Files         797      797              
  Lines       70158    70199      +41     
  Branches    36261    36287      +26     
==========================================
+ Hits        43170    43171       +1     
- Misses      19753    19779      +26     
- Partials     7235     7249      +14     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@scottschurr scottschurr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 LGTM.

Copy link
Contributor

@tequdev tequdev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed the code reproducing this bug works correctly in 2.1.1.

RELEASENOTES.md Outdated

### Amendments

- **fixAMMOverflowOffer**: Fix improper handling of a large synthetic AMM offers in the payment engine. Due to the importance of this fix, the default vote in the source code has been set to YES. For information on how to configure your validator's amendment voting, see [Configure Amendment Voting](https://xrpl.org/docs/infrastructure/configuration/configure-amendment-voting).

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit:

  1. Remove "a": "... handling of a large synthetic AMM ..." -> "... handling of large synthetic AMM ..."

or

  1. Remove plural: "... AMM offers ... " -> "... AMM offer ... "

@WietseWind WietseWind merged commit 2d1854f into XRPLF:master Mar 27, 2024
16 of 17 checks passed
ximinez added a commit to ximinez/rippled that referenced this pull request Mar 28, 2024
Merging changes for 2.1.1 from master into develop
@ximinez ximinez mentioned this pull request Mar 28, 2024
2 tasks
@seelabs seelabs deleted the master-next branch June 4, 2024 01:51
@thejohnfreeman thejohnfreeman mentioned this pull request Jun 6, 2024
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants