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.0-rc1 #4910

Merged
merged 8 commits into from
Feb 8, 2024
Merged

Proposed 2.1.0-rc1 #4910

merged 8 commits into from
Feb 8, 2024

Conversation

intelliot
Copy link
Collaborator

High Level Overview of Change

This is a release candidate for the 2.1.0 release, which introduces two amendments:

  • fixNFTokenReserve
  • fixInnerObjTemplate

Additionally, there are test improvements, improvements to libxrpl, and relaxed validation for port_grpc in rippled.cfg.

The base branch is release. All releases (including betas) go in release. This PR will be merged with --ff-only (not squashed or rebased, and not using the GitHub UI).

Type of Change

  • Release

API Impact

Improvements to libxrpl help dependents like validator-keys-tool and xbridge_witness.

If/when amendments activate, changes to transaction processing - as implemented by those amendments - will take effect.

Bronek and others added 8 commits January 30, 2024 22:14
If the payee and payer are the same account, then the transaction fails
in preflight with temREDUNDANT.
* Disable the Windows CI unit tests "allowed to fail" workaround which
  was previously introduced in XRPLF#4596.
* The runner hardware was upgraded, and the unit tests have been passing
  since then.
Use consistent platform-agnostic library names on all platforms.

Fix an issue that prevents dependents like validator-keys-tool from
linking to libxrpl on Windows.

It is bad practice to change the binary base name depending on the
platform. CMake already manipulates the base name into a final name that
fits the conventions of the platform. Linkers accept base names on the
command line and then look for conventional names on disk.
…LF#4767)

Without this amendment, an NFTokenAcceptOffer transaction can succeed
even when the NFToken recipient does not have sufficient reserves for
the new NFTokenPage. This allowed accounts to accept NFT sell offers
without having a sufficient reserve. (However, there was no issue in
brokered mode or when a buy offer is involved.)

Instead, the transaction should fail with `tecINSUFFICIENT_RESERVE` as
appropriate. The `fixNFTokenReserve` amendment adds checks in the
NFTokenAcceptOffer transactor to check if the OwnerCount changed. If it
did, then it checks the new reserve requirement.

Fix XRPLF#4679
These headers are required in the xrpl Conan package in order for
xbridge witness server (xbwd) to build. This change to libxrpl may help
any dependents of libxrpl. This addition does not change any C++ code.
)

Prior to this commit, `port_grpc` could not be added to the [server]
stanza. Instead of validating gRPC IP/Port/Protocol information in
ServerHandler, validate grpc port info in GRPCServer constructor. This
should not break backwards compatibility.

gRPC-related config info must be in a section (stanza) called
[port_gprc].

* Close XRPLF#4015 - That was an alternate solution. It was decided that with
  relaxed validation, it is not necessary to rename port_grpc.
* Fix XRPLF#4557
Add `STObject` constructor to explicitly set the inner object template.
This allows certain AMM transactions to apply in the same ledger:

There is no issue if the trading fee is greater than or equal to 0.01%.
If the trading fee is less than 0.01%, then:
- After AMM create, AMM transactions must wait for one ledger to close
  (3-5 seconds).
- After one ledger is validated, all AMM transactions succeed, as
  appropriate, except for AMMVote.
- The first AMMVote which votes for a 0 trading fee in a ledger will
  succeed. Subsequent AMMVote transactions which vote for a 0 trading
  fee will wait for the next ledger (3-5 seconds). This behavior repeats
  for each ledger.

This has no effect on the ultimate correctness of AMM. This amendment
will allow the transactions described above to succeed as expected, even
if the trading fee is 0 and the transactions are applied within one
ledger (block).
@intelliot intelliot added this to the 2.1.0 (Fix) milestone Feb 7, 2024
@codecov-commenter
Copy link

Codecov Report

Attention: 18 lines in your changes are missing coverage. Please review.

Comparison is base (22cdb57) 61.50% compared to head (da68651) 61.49%.

Files Patch % Lines
src/ripple/app/tx/impl/AMMBid.cpp 14.28% 3 Missing and 3 partials ⚠️
src/ripple/app/tx/impl/NFTokenAcceptOffer.cpp 76.92% 3 Missing and 3 partials ⚠️
src/ripple/rpc/impl/ServerHandler.cpp 0.00% 1 Missing and 1 partial ⚠️
src/ripple/app/misc/impl/AMMUtils.cpp 88.88% 0 Missing and 1 partial ⚠️
src/ripple/app/tx/impl/AMMVote.cpp 75.00% 0 Missing and 1 partial ⚠️
src/ripple/protocol/impl/STObject.cpp 92.30% 0 Missing and 1 partial ⚠️
src/ripple/rpc/handlers/AMMInfo.cpp 0.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           release    #4910      +/-   ##
===========================================
- Coverage    61.50%   61.49%   -0.02%     
===========================================
  Files          797      797              
  Lines        70122    70157      +35     
  Branches     36238    36261      +23     
===========================================
+ Hits         43132    43146      +14     
- Misses       19757    19763       +6     
- Partials      7233     7248      +15     

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

Copy link
Collaborator

@gregtatcam gregtatcam left a comment

Choose a reason for hiding this comment

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

I reviewed #4860 and #4906. build on OSX with unity on/off looks fine. all unit-tests pass.

@intelliot intelliot merged commit da68651 into XRPLF:release Feb 8, 2024
17 checks passed
@intelliot intelliot deleted the develop-next branch February 8, 2024 15:24
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.

9 participants