more fixes

This commit is contained in:
Mayukha Vadari
2026-05-20 15:23:09 -04:00
parent bf68edb541
commit 9cf2c0c3ae
3 changed files with 20 additions and 28 deletions

View File

@@ -15,6 +15,7 @@
// Add new amendments to the top of this list.
// Keep it sorted in reverse chronological order.
XRPL_FIX (ConstantInvariant, Supported::No, VoteBehavior::DefaultNo)
XRPL_FIX (Cleanup3_2_0, Supported::No, VoteBehavior::DefaultNo)
XRPL_FEATURE(MPTokensV2, Supported::No, VoteBehavior::DefaultNo)
XRPL_FIX (Cleanup3_1_3, Supported::Yes, VoteBehavior::DefaultYes)

View File

@@ -984,8 +984,8 @@ NoModifiedUnmodifiableFields::visitEntry(
}
// Check whether any constant (or unannotated) fields in the given template
// have been modified between before and after. Recurses into STObject and
// STArray fields using InnerObjectFormats.
// have been modified between before and after.
// TODO(future): recurse into STObject and STArray fields using InnerObjectFormats
static bool
hasConstantFieldChanged(STObject const& before, STObject const& after, SOTemplate const& tmpl)
{
@@ -1001,21 +1001,10 @@ hasConstantFieldChanged(STObject const& before, STObject const& after, SOTemplat
if (constant == SoeImmutable)
{
if (elem.style() == SoeOptional)
{
// Optional constant fields may be added or removed,
// but their value must not change once present.
if (bPresent && aPresent && *bField != *aField)
return true;
}
else
{
// Required and default constant fields must not
// change at all — including transitions between
// default (not-present) and explicit values.
if (bPresent != aPresent || (bPresent && aPresent && *bField != *aField))
return true;
}
// Field must not change at all — including transitions between
// default (not-present) and explicit values.
if (bPresent != aPresent || (bPresent && aPresent && *bField != *aField))
return true;
}
// SoeMutable fields may change freely — no recursion
// into inner objects/arrays is needed because the parent
@@ -1038,7 +1027,7 @@ NoModifiedUnmodifiableFields::finalize(
return beforeField != afterField || (afterField && before->at(field) != after->at(field));
};
bool const useTemplate = view.rules().enabled(featureInvariantsV1_1);
bool const useTemplate = view.rules().enabled(fixConstantInvariant);
for (auto const& slePair : changedEntries_)
{
@@ -1057,12 +1046,12 @@ NoModifiedUnmodifiableFields::finalize(
// that is not reliably present via peekAtPField, so we
// check it explicitly.
if (!bad)
bad = kFIELD_CHANGED(before, after, sfLedgerIndex);
bad = kFieldChanged(before, after, sfLedgerIndex);
}
// Old hardcoded check
bool badOld = false;
[[maybe_unused]] bool enforceOld = false;
[[maybe_unused]] bool enforceOld = view.rules().enabled(featureLendingProtocol);
switch (type)
{
case ltLOAN_BROKER:
@@ -1071,7 +1060,6 @@ NoModifiedUnmodifiableFields::finalize(
* amendment status, allowing for detection and logging of
* potential issues even when the amendment is disabled.
*/
enforce = view.rules().enabled(featureLendingProtocol);
bad = kFieldChanged(before, after, sfLedgerEntryType) ||
kFieldChanged(before, after, sfLedgerIndex) ||
kFieldChanged(before, after, sfSequence) ||
@@ -1090,7 +1078,6 @@ NoModifiedUnmodifiableFields::finalize(
* amendment status, allowing for detection and logging of
* potential issues even when the amendment is disabled.
*/
enforce = view.rules().enabled(featureLendingProtocol);
bad = kFieldChanged(before, after, sfLedgerEntryType) ||
kFieldChanged(before, after, sfLedgerIndex) ||
kFieldChanged(before, after, sfSequence) ||
@@ -1122,7 +1109,6 @@ NoModifiedUnmodifiableFields::finalize(
* all transactions are affected because that's when it
* was added.
*/
enforce = view.rules().enabled(featureLendingProtocol);
bad = kFieldChanged(before, after, sfLedgerEntryType) ||
kFieldChanged(before, after, sfLedgerIndex);
}
@@ -1143,7 +1129,12 @@ NoModifiedUnmodifiableFields::finalize(
{
JLOG(j.fatal()) << "Invariant failed: changed an unchangeable field for "
<< tx.getTransactionID();
if (!useTemplate && enforceOld)
// If the new check passes when the old check fails, it's a bug in the new check.
XRPL_ASSERT(
!useTemplate,
"xrpl::NoModifiedUnmodifiableFields::finalize : new invariant version "
"passes when old version fails");
if (enforceOld)
return false;
}
}

View File

@@ -2154,7 +2154,7 @@ class Invariants_test : public beast::unit_test::Suite
{tesSUCCESS, tesSUCCESS});
}
// Without featureInvariantsV1_1, old hardcoded path should
// Without fixConstantInvariant, old hardcoded path should
// still catch sfLedgerEntryType/sfLedgerIndex changes
{
auto const mods = std::to_array<std::function<void(SLE::pointer&)>>({
@@ -2165,7 +2165,7 @@ class Invariants_test : public beast::unit_test::Suite
for (auto const& mod : mods)
{
doInvariantCheck(
Env(*this, defaultAmendments() - featureInvariantsV1_1),
Env(*this, defaultAmendments() - fixConstantInvariant),
{{"changed an unchangeable field"}},
[&](Account const& a1, Account const&, ApplyContext& ac) {
auto sle = ac.view().peek(keylet::account(a1.id()));
@@ -2178,12 +2178,12 @@ class Invariants_test : public beast::unit_test::Suite
}
}
// Without featureInvariantsV1_1, modifying a SoeImmutable field
// Without fixConstantInvariant, modifying a SoeImmutable field
// that is NOT sfLedgerEntryType/sfLedgerIndex on a non-loan
// type should NOT fail (old code doesn't check it)
{
doInvariantCheck(
Env(*this, defaultAmendments() - featureInvariantsV1_1),
Env(*this, defaultAmendments() - fixConstantInvariant),
{},
[&](Account const& a1, Account const& a2, ApplyContext& ac) {
auto sle = ac.view().peek(keylet::account(a1.id()));