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 version 2.3.1 #5259

Merged
merged 4 commits into from
Jan 29, 2025
Merged

Proposed version 2.3.1 #5259

merged 4 commits into from
Jan 29, 2025

Conversation

ximinez
Copy link
Collaborator

@ximinez ximinez commented Jan 29, 2025

High Level Overview of Change

This is a hotfix release. This release fixes several issues related to peer connectivity.

The base branch is master. This PR branch will be pushed directly to master (not squashed or rebased, and not using the GitHub UI), and reverse-merged to develop and release. Do not use the GitHub UI to merge this PR

Changelog

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • Chore (no impact to binary, e.g. .gitignore, formatting, dropping support for older tooling)
  • Release

vlntb and others added 2 commits January 27, 2025 19:41
- Fix an erroneous high fee penalty that peers could incur for sending
  older transactions.
- Update to the fees charged for imposing a load on the server.
- Prevent the relaying of internal pseudo-transactions.
  - Before: Pseudo-transactions received from a peer will fail the signature
    check, even if they were requested (using TMGetObjectByHash), because
    they have no signature. This causes the peer to be charge for an
    invalid signature.
  - After: Pseudo-transactions, are put into the global cache
    (TransactionMaster) only. If the transaction is not part of
    a TMTransactions batch, the peer is charged an unwanted data fee.
    These fees will not be a problem in the normal course of operations,
    but should dissuade peers from behaving badly by sending a bunch of
    junk.
- Improve logging: include the reason for fees charged to a peer.

Co-authored-by: Ed Hennis <ed@ripple.com>
@ximinez ximinez added the Bug label Jan 29, 2025
Copy link

codecov bot commented Jan 29, 2025

Codecov Report

Attention: Patch coverage is 24.29379% with 134 lines in your changes missing coverage. Please review.

Project coverage is 77.9%. Comparing base (f64cf91) to head (8458233).
Report is 4 commits behind head on master.

Files with missing lines Patch % Lines
src/xrpld/overlay/detail/PeerImp.cpp 1.0% 102 Missing ⚠️
src/xrpld/overlay/detail/OverlayImpl.cpp 60.0% 8 Missing ⚠️
src/xrpld/overlay/detail/PeerImp.h 0.0% 7 Missing ⚠️
src/xrpld/app/ledger/detail/InboundLedger.cpp 0.0% 5 Missing ⚠️
...rc/xrpld/app/ledger/detail/InboundTransactions.cpp 0.0% 4 Missing ⚠️
src/xrpld/app/ledger/detail/LedgerMaster.cpp 0.0% 4 Missing ⚠️
src/libxrpl/resource/Charge.cpp 0.0% 2 Missing ⚠️
include/xrpl/resource/detail/Logic.h 92.3% 1 Missing ⚠️
src/xrpld/rpc/detail/ServerHandler.cpp 88.9% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff            @@
##           master   #5259     +/-   ##
========================================
- Coverage    77.9%   77.9%   -0.0%     
========================================
  Files         784     784             
  Lines       66681   66743     +62     
  Branches     8162    8190     +28     
========================================
+ Hits        51950   51967     +17     
- Misses      14731   14776     +45     
Files with missing lines Coverage Δ
src/libxrpl/protocol/BuildInfo.cpp 98.2% <ø> (ø)
src/libxrpl/resource/Consumer.cpp 89.8% <100.0%> (ø)
src/xrpld/overlay/Overlay.h 70.0% <ø> (ø)
src/xrpld/overlay/Peer.h 100.0% <ø> (ø)
src/xrpld/overlay/detail/OverlayImpl.h 40.0% <ø> (ø)
src/xrpld/rpc/detail/RPCHelpers.cpp 82.8% <100.0%> (ø)
src/xrpld/rpc/handlers/GatewayBalances.cpp 89.8% <100.0%> (ø)
src/xrpld/rpc/handlers/LedgerHandler.cpp 22.6% <100.0%> (ø)
src/xrpld/rpc/handlers/PathFind.cpp 48.3% <100.0%> (ø)
src/xrpld/rpc/handlers/RipplePathFind.cpp 42.5% <100.0%> (ø)
... and 12 more

... and 3 files with indirect coverage changes

Impacted file tree graph

Copy link
Collaborator

@vlntb vlntb left a comment

Choose a reason for hiding this comment

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

LGTM

@ximinez ximinez merged commit 8458233 into master Jan 29, 2025
22 of 24 checks passed
@ximinez ximinez deleted the release-next branch January 29, 2025 22:56
@ximinez ximinez mentioned this pull request Jan 30, 2025
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants