8000 Acquire previously failed transaction set from by mtrippled · Pull Request #5318 · XRPLF/rippled · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Acquire previously failed transaction set from #5318

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

Merged
merged 1 commit into from
Feb 26, 2025

Conversation

mtrippled
Copy link
Collaborator

network as new proposal arrives.

High Level Overview of Change

Clear failure flag to ensure that previously-failed inbound ledger gets acquired if a new trusted proposal arrives.

Context of Change

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Refactor (non-breaking change that only restructures code)
  • Performance (increase or change in throughput and/or latency)
  • Tests (you added tests for code that already exists, or your new feature included in this PR)
  • Documentation update
  • Chore (no impact to binary, e.g. .gitignore, formatting, dropping support for older tooling)
  • Release

API Impact

  • Public API: New feature (new methods and/or new fields)
  • Public API: Breaking change (in general, breaking changes should only impact the next api_version)
  • libxrpl change (any change that may affect libxrpl or dependents of libxrpl)
  • Peer protocol change (must be backward compatible or bump the peer protocol version)

network as new proposal arrives.
Copy link
codecov bot commented Feb 25, 2025

Codecov Report

Attention: Patch coverage is 0% with 1 line in your changes missing coverage. Please review.

Project coverage is 78.2%. Comparing base (37d06bc) to head (87a96a0).
Report is 1 commits behind head on develop.

Files with missing lines Patch % Lines
src/xrpld/app/ledger/detail/TransactionAcquire.cpp 0.0% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##           develop   #5318   +/-   ##
=======================================
  Coverage     78.2%   78.2%           
=======================================
  Files          790     790           
  Lines        67740   67741    +1     
  Branches      8177    8173    -4     
=======================================
+ Hits         52956   52965    +9     
+ Misses       14784   14776    -8     
Files with missing lines Coverage Δ
src/xrpld/app/ledger/detail/TransactionAcquire.cpp 0.0% <0.0%> (ø)

... and 4 files with indirect coverage changes

Impacted file tree graph

Copy link
Collaborator
@ximinez ximinez left a comment

Choose a reason for hiding this comment

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

There are a couple of places in trigger where failed_ can be set if the received data is somehow incorrect. It might be worth clearing that data out if that happens. But I suspect that is very unlikely to be the case if we get a trusted proposal with it. Extra checking can be added later. This is a good start as-is.

@mtrippled mtrippled added the Ready to merge *PR author* thinks it's ready to merge. Has passed code review. Perf sign-off may still be required. label Feb 26, 2025
@bthomee bthomee merged commit cd7c628 into XRPLF:develop Feb 26, 2025
21 of 22 checks passed
@legleux legleux mentioned this pull request Feb 26, 2025
5 tasks
ximinez pushed a commit that referenced this pull request Feb 28, 2025
Tapanito pushed a commit to Tapanito/rippled that referenced this pull request Mar 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ready to merge *PR author* thinks it's ready to merge. Has passed code review. Perf sign-off may still be required.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
0