fix: Enforce MPT balance invariants under fixCleanup3_4_0 - #7889
Open
gregtatcam wants to merge 1 commit into
Open
fix: Enforce MPT balance invariants under fixCleanup3_4_0#7889gregtatcam wants to merge 1 commit into
gregtatcam wants to merge 1 commit into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
High Level Overview of Change
Makes the
ValidMPTBalanceChangesandValidMPTTransferinvariant checksenforce under the
fixCleanup3_4_0amendment, and extends both to hold evenwhen a transaction fails. Previously these invariants only enforced under
featureMPTokensV2; because that amendment is unshipped, they were effectivelyadvisory (log-only). This PR makes them active protections on the next cleanup
amendment.
Context of Change
The MPT balance invariants (
ValidMPTBalanceChanges,ValidMPTTransfer) weregated solely on
featureMPTokensV2. SincefeatureMPTokensV2is not yetsupported/shipped, the checks logged but never escalated to
tecINVARIANT_FAILED/tefINVARIANT_FAILED, so they provided no realprotection on the live network. This change adds
fixCleanup3_4_0to the gateso enforcement turns on with the next cleanup amendment, independently of the
MPTokensV2 rollout. When both amendments are disabled the checks remain log-only
to preserve consensus with older nodes.
Separately, the invariants only ran on the success path. Per the requirement
that invariants must hold even for failed transactions, both checks now also
verify that a non-success result did not move
OutstandingAmountor anyholder's MPT balance.
tecINCOMPLETEandtecKILLEDare exempt because sometransactors legitimately commit MPT changes while returning those codes
(
AMMWithdrawcommits the pool ontecINCOMPLETE; fill-or-killOfferCreateand lending can return
tecKILLEDafter applying MPT changes). Any newtransactor that persists MPT state on a non-
tesSUCCESSresult must be reviewedagainst these checks.
API Impact
libxrplchange (any change that may affectlibxrplor dependents oflibxrpl)No public API,
libxrpl, or peer protocol impact. The behavior change is gatedbehind the
fixCleanup3_4_0amendment.Test Plan
Invariants_testwas updated and run to cover the new behavior:doInvariantCheckgains aninitialResultseed so a check can be exercisedstarting from a non-
tesSUCCESSinput, and it now asserts only when aninvariant actually fires on a pass (
terActual != terInput).transaction that trips an invariant now escalates to
tefINVARIANT_FAILEDonthe second pass (rather than staying
tecINVARIANT_FAILED).tecINCOMPLETE/tecKILLEDexemptions(invariant must not fire) and for the on-failure balance/transfer checks
(invariant must fire on other failure codes such as
tecEXPIRED).