ARCHIVE/rippled
1
0
Fork 0
mirror of https://github.com/XRPLF/rippled.git synced 2026-08-22 14:50:54 +00:00
Code Issues Packages Projects Releases Wiki Activity

feat: Add an invariant to ensure object deletion also deletes its pseudo-account (#7445)

Browse Source 
Co-authored-by: xrplf-ai-reviewer[bot] <266832837+xrplf-ai-reviewer[bot]@users.noreply.github.com>
This commit is contained in:
Vito Tumas
2026-07-01 15:21:23 +02:00
committed by GitHub
parent 86d8b244d6
commit ea13be81b7
3 changed files with 242 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files

26
include/xrpl/tx/invariants/InvariantCheck.h
View File

@@ -375,16 +375,35 @@ public:
*/
class ValidAmounts
{
std::vector<std::shared_ptr<SLE const>> afterEntries_;
std::vector<SLE::const_pointer> afterEntries_;
public:
void
visitEntry(bool, std::shared_ptr<SLE const> const&, std::shared_ptr<SLE const> const&);
visitEntry(bool, SLE::const_ref, SLE::const_ref);
[[nodiscard]] bool
finalize(STTx const&, TER const, XRPAmount const, ReadView const&, beast::Journal const&) const;
};
/*
* Verify that when an object with an associated pseudo-account is deleted,
* its pseudo-account is also deleted.
*
* The reverse (pseudo-account deleted → object deleted) is enforced by
* AccountRootsDeletedClean via getPseudoAccountFields().
*/
class ObjectHasPseudoAccount
{
public:
void
visitEntry(bool, SLE::const_ref, SLE::const_ref);
[[nodiscard]] bool
finalize(STTx const&, TER const, XRPAmount const, ReadView const&, beast::Journal const&) const;
private:
std::vector<SLE::const_pointer> deletedObjSles_;
};
// additional invariant checks can be declared above and then added to this
// tuple
using InvariantChecks = std::tuple<
@@ -416,7 +435,8 @@ using InvariantChecks = std::tuple<
ValidConfidentialMPToken,
ValidMPTPayment,
ValidAmounts,
ValidMPTTransfer>;
ValidMPTTransfer,
ObjectHasPseudoAccount>;
/**
* @brief get a tuple of all invariant checks

95
src/libxrpl/tx/invariants/InvariantCheck.cpp
View File

@@ -35,6 +35,7 @@
#include <memory>
#include <optional>
#include <sstream>
#include <string>
#include <vector>
namespace xrpl {
@@ -63,6 +64,23 @@ hasPrivilege(STTx const& tx, Privilege priv)
#undef TRANSACTION
#pragma pop_macro("TRANSACTION")
// Returns the human-readable name of a ledger entry's type, falling back to
// the numeric type if the format is somehow unknown.
static std::string
ledgerEntryTypeName(SLE const& sle)
{
auto const item = LedgerFormats::getInstance().findByType(sle.getType());
if (item == nullptr)
{
// LCOV_EXCL_START
UNREACHABLE("xrpl::ledgerEntryTypeName : ledger entry has no known ledger format");
return std::to_string(sle.getType());
// LCOV_EXCL_STOP
}
return item->getName();
}
void
TransactionFeeCheck::visitEntry(bool, SLE::const_ref, SLE::const_ref)
{
@@ -457,16 +475,8 @@ AccountRootsDeletedClean::finalize(
if (auto const sle = view.read(keylet))
{
// Finding the object is bad
auto const typeName = [&sle]() {
auto item = LedgerFormats::getInstance().findByType(sle->getType());
if (item != nullptr)
return item->getName();
return std::to_string(sle->getType());
}();
JLOG(j.fatal()) << "Invariant failed: account deletion left behind a " << typeName
<< " object";
JLOG(j.fatal()) << "Invariant failed: account deletion left behind a "
<< ledgerEntryTypeName(*sle) << " object";
// The comment above starting with "assert(enforce)" explains this
// assert.
XRPL_ASSERT(
@@ -1070,4 +1080,69 @@ ValidAmounts::finalize(
return true;
}
void
ObjectHasPseudoAccount::visitEntry(bool isDelete, SLE::const_ref before, SLE::const_ref after)
{
if (!isDelete)
return;
// Before should never be null when isDelete = true
if (!before)
{
// LCOV_EXCL_START
UNREACHABLE(
"xrpl::ObjectHasPseudoAccount::visitEntry : deleted ledger entry missing before state");
return;
// LCOV_EXCL_STOP
}
switch (before->getType())
{
case ltAMM:
case ltVAULT:
case ltLOAN_BROKER:
deletedObjSles_.push_back(before);
break;
default:
return;
}
}
[[nodiscard]] bool
ObjectHasPseudoAccount::finalize(
STTx const&,
TER const,
XRPAmount const,
ReadView const& view,
beast::Journal const& j) const
{
if (!view.rules().enabled(fixCleanup3_3_0))
return true;
if (deletedObjSles_.empty())
return true;
bool failed = false;
for (auto const& sle : deletedObjSles_)
{
if (!sle->isFieldPresent(sfAccount))
{
JLOG(j.fatal()) << "Invariant failed: deleted " << ledgerEntryTypeName(*sle)
<< " is missing pseudo-account field";
failed = true;
continue;
}
// The pseudo-account must NOT exist on the ledger after the object is deleted.
if (view.exists(keylet::account(sle->getAccountID(sfAccount))))
{
JLOG(j.fatal()) << "Invariant failed: deleted " << ledgerEntryTypeName(*sle)
<< " without deleting its pseudo-account";
failed = true;
}
}
return !failed;
}
} // namespace xrpl

142
src/test/app/Invariants_test.cpp
View File

@@ -2829,7 +2829,8 @@ class Invariants_test : public beast::unit_test::Suite
});
doInvariantCheck(
{"vault updated by a wrong transaction type"},
{"vault updated by a wrong transaction type",
"deleted Vault without deleting its pseudo-account"},
[&](Account const& a1, Account const& a2, ApplyContext& ac) {
auto const keylet = keylet::vault(a1.id(), ac.view().seq());
auto sleVault = ac.view().peek(keylet);
@@ -2840,7 +2841,7 @@ class Invariants_test : public beast::unit_test::Suite
},
XRPAmount{},
STTx{ttPAYMENT, [](STObject&) {}},
{tecINVARIANT_FAILED, tecINVARIANT_FAILED},
{tecINVARIANT_FAILED, tefINVARIANT_FAILED},
[&](Account const& a1, Account const& a2, Env& env) {
Vault const vault{env};
auto [tx, _] = vault.create({.owner = a1, .asset = xrpIssue()});
@@ -2877,6 +2878,7 @@ class Invariants_test : public beast::unit_test::Suite
auto const vaultPage = ac.view().dirInsert(
keylet::ownerDir(a1.id()), sleVault->key(), describeOwnerDir(a1.id()));
sleVault->setFieldU64(sfOwnerNode, *vaultPage);
sleVault->setAccountID(sfAccount, a1.id());
ac.view().insert(sleVault);
return true;
},
@@ -2885,7 +2887,8 @@ class Invariants_test : public beast::unit_test::Suite
{tecINVARIANT_FAILED, tecINVARIANT_FAILED});
doInvariantCheck(
{"vault deleted by a wrong transaction type"},
{"vault deleted by a wrong transaction type",
"deleted Vault without deleting its pseudo-account"},
[&](Account const& a1, Account const& a2, ApplyContext& ac) {
auto const keylet = keylet::vault(a1.id(), ac.view().seq());
auto sleVault = ac.view().peek(keylet);
@@ -2896,7 +2899,7 @@ class Invariants_test : public beast::unit_test::Suite
},
XRPAmount{},
STTx{ttVAULT_SET, [](STObject&) {}},
{tecINVARIANT_FAILED, tecINVARIANT_FAILED},
{tecINVARIANT_FAILED, tefINVARIANT_FAILED},
[&](Account const& a1, Account const& a2, Env& env) {
Vault const vault{env};
auto [tx, _] = vault.create({.owner = a1, .asset = xrpIssue()});
@@ -2905,7 +2908,8 @@ class Invariants_test : public beast::unit_test::Suite
});
doInvariantCheck(
{"vault operation updated more than single vault"},
{"vault operation updated more than single vault",
"deleted Vault without deleting its pseudo-account"},
[&](Account const& a1, Account const& a2, ApplyContext& ac) {
{
auto const keylet = keylet::vault(a1.id(), ac.view().seq());
@@ -2925,7 +2929,7 @@ class Invariants_test : public beast::unit_test::Suite
},
XRPAmount{},
STTx{ttVAULT_DELETE, [](STObject&) {}},
{tecINVARIANT_FAILED, tecINVARIANT_FAILED},
{tecINVARIANT_FAILED, tefINVARIANT_FAILED},
[&](Account const& a1, Account const& a2, Env& env) {
Vault const vault{env};
{
@@ -2949,6 +2953,7 @@ class Invariants_test : public beast::unit_test::Suite
auto const vaultPage = ac.view().dirInsert(
keylet::ownerDir(a.id()), sleVault->key(), describeOwnerDir(a.id()));
sleVault->setFieldU64(sfOwnerNode, *vaultPage);
sleVault->setAccountID(sfAccount, a.id());
ac.view().insert(sleVault);
};
insertVault(a1);
@@ -2960,7 +2965,8 @@ class Invariants_test : public beast::unit_test::Suite
{tecINVARIANT_FAILED, tecINVARIANT_FAILED});
doInvariantCheck(
{"deleted vault must also delete shares"},
{"deleted vault must also delete shares",
"deleted Vault without deleting its pseudo-account"},
[&](Account const& a1, Account const& a2, ApplyContext& ac) {
auto const keylet = keylet::vault(a1.id(), ac.view().seq());
auto sleVault = ac.view().peek(keylet);
@@ -2971,7 +2977,7 @@ class Invariants_test : public beast::unit_test::Suite
},
XRPAmount{},
STTx{ttVAULT_DELETE, [](STObject&) {}},
{tecINVARIANT_FAILED, tecINVARIANT_FAILED},
{tecINVARIANT_FAILED, tefINVARIANT_FAILED},
[&](Account const& a1, Account const& a2, Env& env) {
Vault const vault{env};
auto [tx, _] = vault.create({.owner = a1, .asset = xrpIssue()});
@@ -5176,6 +5182,125 @@ class Invariants_test : public beast::unit_test::Suite
}
}
void
testObjectHasPseudoAccount()
{
testcase << "object has pseudo-account";
using namespace jtx;
auto const amendments = defaultAmendments() | fixCleanup3_3_0;
// Vault: object deleted without its pseudo-account
{
Keylet vaultKeylet = keylet::amendments();
doInvariantCheck(
Env{*this, amendments},
{{"deleted Vault without deleting its pseudo-account"}},
[&vaultKeylet](Account const&, Account const&, ApplyContext& ac) {
auto sle = ac.view().peek(vaultKeylet);
if (!sle)
return false;
ac.view().erase(sle);
return true;
},
XRPAmount{},
STTx{ttVAULT_DELETE, [](STObject&) {}},
{tecINVARIANT_FAILED, tefINVARIANT_FAILED},
[&vaultKeylet](Account const& a1, Account const&, Env& env) {
Vault const vault{env};
auto [tx, keylet] = vault.create({.owner = a1, .asset = xrpIssue()});
env(tx);
vaultKeylet = keylet;
return true;
});
}
// AMM: object deleted without its pseudo-account
{
uint256 ammID{};
Account const gw{"gw"};
doInvariantCheck(
Env{*this, amendments},
{{"deleted AMM without deleting its pseudo-account"}},
[&ammID](Account const&, Account const&, ApplyContext& ac) {
auto sle = ac.view().peek(keylet::amm(ammID));
if (!sle)
return false;
ac.view().erase(sle);
return true;
},
XRPAmount{},
STTx{ttAMM_DELETE, [](STObject&) {}},
{tecINVARIANT_FAILED, tefINVARIANT_FAILED},
[&ammID, &gw](Account const&, Account const&, Env& env) {
env.fund(XRP(1'000), gw);
AMM const amm(env, gw, XRP(100), gw["USD"](100));
ammID = amm.ammID();
return true;
});
}
// LoanBroker: object deleted without its pseudo-account
{
Keylet loanBrokerKeylet = keylet::amendments();
doInvariantCheck(
Env{*this, amendments},
{{"deleted LoanBroker without deleting its pseudo-account"}},
[&loanBrokerKeylet](Account const&, Account const&, ApplyContext& ac) {
auto sle = ac.view().peek(loanBrokerKeylet);
if (!sle)
return false;
ac.view().erase(sle);
return true;
},
XRPAmount{},
STTx{ttLOAN_BROKER_DELETE, [](STObject&) {}},
{tecINVARIANT_FAILED, tefINVARIANT_FAILED},
[&loanBrokerKeylet, this](Account const& a1, Account const&, Env& env) {
PrettyAsset const xrpAsset{xrpIssue(), 1'000'000};
loanBrokerKeylet = this->createLoanBroker(a1, env, xrpAsset);
return BEAST_EXPECT(env.le(loanBrokerKeylet));
});
}
// Deleted object missing sfAccount field (defensive check).
// Manually construct the view to place a vault SLE without
// sfAccount into the base ledger, then erase it.
{
Env env{*this, amendments};
Account const a1{"A1"};
Account const a2{"A2"};
env.fund(XRP(1000), a1, a2);
env.close();
OpenView ov{*env.current()};
auto const vaultKeylet = keylet::vault(a1.id(), ov.seq());
auto sleVault = std::make_shared<SLE>(vaultKeylet);
sleVault->makeFieldAbsent(sfAccount);
ov.rawInsert(sleVault);
STTx const tx{ttVAULT_DELETE, [](STObject&) {}};
test::StreamSink sink{beast::Severity::Warning};
beast::Journal const jlog{sink};
ApplyContext ac{
env.app(), ov, tx, tesSUCCESS, env.current()->fees().base, TapNone, jlog};
CurrentTransactionRulesGuard const rulesGuard(ov.rules());
auto sle = ac.view().peek(vaultKeylet);
if (!BEAST_EXPECT(sle))
return;
ac.view().erase(sle);
auto transactor = makeTransactor(ac);
if (!BEAST_EXPECT(transactor))
return;
TER const result = transactor->checkInvariants(tesSUCCESS, XRPAmount{});
BEAST_EXPECT(result == tecINVARIANT_FAILED);
BEAST_EXPECT(sink.messages().str().contains("is missing pseudo-account field"));
}
}
void
testConfidentialMPTTransfer()
{
@@ -5458,6 +5583,7 @@ public:
testInvariantOverwrite(defaultAmendments() - fixCleanup3_1_3);
testVaultComputeCoarsestScale();
testAMM();
testObjectHasPseudoAccount();
}
};