rippled
rippled copied to clipboard
amendment fix to add `sfPreviousTxnID`/`sfPreviousTxnLgrSequence` to all ledger objects
High Level Overview of Change
This PR adds sfPreviousTxnID
/sfPreviousTxnLgrSequence
as fields to all ledger objects that don't already have them (except LedgerHashes
, because that seems a bit redundant and also those objects aren't modified via transaction).
The affected objects are:
-
DirectoryNode
-
Amendments
-
FeeSettings
-
NegativeUNL
-
AMM
Context of Change
It makes it a lot easier to go through the history of a ledger object. Related to #4602.
Type of Change
- [x] Bug fix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds functionality)
- [x] Breaking change (fix or feature that would cause existing functionality to not work as expected)
Test Plan
Tests have been added to ensure that everything behaves as-is prior to the amendment and transitions seamlessly when the amendment is activated.
@Silkjaer @RichardAH @nixer89 @dangell7 this is ready for review now.
Codecov Report
Attention: Patch coverage is 90.19608%
with 5 lines
in your changes are missing coverage. Please review.
Project coverage is 76.97%. Comparing base (
c88166e
) to head (c8af202
).
Files | Patch % | Lines |
---|---|---|
src/test/ledger/Directory_test.cpp | 92.68% | 0 Missing and 3 partials :warning: |
src/ripple/ledger/impl/ApplyStateTable.cpp | 33.33% | 0 Missing and 2 partials :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## develop #4751 +/- ##
========================================
Coverage 76.97% 76.97%
========================================
Files 1129 1129
Lines 131914 131959 +45
Branches 39629 39687 +58
========================================
+ Hits 101539 101574 +35
- Misses 24459 24505 +46
+ Partials 5916 5880 -36
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Proposed commit message:
fix amendment to add `PreviousTxnID`/`PreviousTxnLgrSequence`
This amendment, `fixPreviousTxnID`, adds `PreviousTxnID` and
`PreviousTxnLgrSequence` as fields to all ledger objects that did
not already have them included (`DirectoryNode`, `Amendments`,
`FeeSettings`, `NegativeUNL`, and `AMM`). This makes it much easier
to go through the history of these ledger objects.
I'm not sure who to tag to indicate that this is ready to go (maybe @seelabs) - the Mac test failure looks like the known flakiness, not an actual failure.
I see you proposed a commit message (https://github.com/XRPLF/rippled/pull/4751#issuecomment-2007734702), and I just added the "Passed" label. One of us will merge it soon.
API Change
: this adds fields to objects. I don't think it's a breaking change. But it would still make sense to update the models in the client libraries (@mvadari has noted this).