ARCHIVE/rippled
1
0
Fork 0
mirror of https://github.com/XRPLF/rippled.git synced 2025-12-01 08:25:51 +00:00
Code Issues Packages Projects Releases Wiki Activity

Ensure that an account is not deleted with an XRP balance

Browse Source 
- Updates the AccountRootsDeletedClean invariant
This commit is contained in:
Ed Hennis
2025-04-17 17:36:29 -04:00
parent 56ff345a99
commit 1798be0ba9
3 changed files with 59 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files

42
src/test/ledger/Invariants_test.cpp
View File

@@ -159,9 +159,10 @@ class Invariants_test : public beast::unit_test::suite
{{"an account root was deleted"}},
[](Account const& A1, Account const&, ApplyContext& ac) {
// remove an account from the view
auto const sle = ac.view().peek(keylet::account(A1.id()));
auto sle = ac.view().peek(keylet::account(A1.id()));
if (!sle)
return false;
sle->at(sfBalance) = beast::zero;
ac.view().erase(sle);
return true;
});
@@ -185,10 +186,12 @@ class Invariants_test : public beast::unit_test::suite
{{"account deletion succeeded but deleted multiple accounts"}},
[](Account const& A1, Account const& A2, ApplyContext& ac) {
// remove two accounts from the view
auto const sleA1 = ac.view().peek(keylet::account(A1.id()));
auto const sleA2 = ac.view().peek(keylet::account(A2.id()));
auto sleA1 = ac.view().peek(keylet::account(A1.id()));
auto sleA2 = ac.view().peek(keylet::account(A2.id()));
if (!sleA1 || !sleA2)
return false;
sleA1->at(sfBalance) = beast::zero;
sleA2->at(sfBalance) = beast::zero;
ac.view().erase(sleA1);
ac.view().erase(sleA2);
return true;
@@ -203,6 +206,23 @@ class Invariants_test : public beast::unit_test::suite
using namespace test::jtx;
testcase << "account root deletion left artifact";
doInvariantCheck(
{{"account deletion left behind a non-zero balance"}},
[&](Account const& A1, Account const& A2, ApplyContext& ac) {
// Add an object to the ledger for account A1, then delete
// A1
auto const a1 = A1.id();
auto const sleA1 = ac.view().peek(keylet::account(a1));
if (!sleA1)
return false;
ac.view().erase(sleA1);
return true;
},
XRPAmount{},
STTx{ttACCOUNT_DELETE, [](STObject& tx) {}});
for (auto const& keyletInfo : directAccountKeylets)
{
// TODO: Use structured binding once LLVM 16 is the minimum
@@ -223,29 +243,31 @@ class Invariants_test : public beast::unit_test::suite
// Add an object to the ledger for account A1, then delete
// A1
auto const a1 = A1.id();
auto const sleA1 = ac.view().peek(keylet::account(a1));
auto sleA1 = ac.view().peek(keylet::account(a1));
if (!sleA1)
return false;
auto const key = std::invoke(keyletfunc, a1);
auto const newSLE = std::make_shared<SLE>(key);
ac.view().insert(newSLE);
sleA1->at(sfBalance) = beast::zero;
ac.view().erase(sleA1);
return true;
},
XRPAmount{},
STTx{ttACCOUNT_DELETE, [](STObject& tx) {}});
};
}
// NFT special case
doInvariantCheck(
{{"account deletion left behind a NFTokenPage object"}},
[&](Account const& A1, Account const&, ApplyContext& ac) {
// remove an account from the view
auto const sle = ac.view().peek(keylet::account(A1.id()));
auto sle = ac.view().peek(keylet::account(A1.id()));
if (!sle)
return false;
sle->at(sfBalance) = beast::zero;
ac.view().erase(sle);
return true;
},
@@ -269,13 +291,14 @@ class Invariants_test : public beast::unit_test::suite
[&](Account const& A1, Account const& A2, ApplyContext& ac) {
// Delete the AMM account without cleaning up the directory or
// deleting the AMM object
auto const sle = ac.view().peek(keylet::account(ammAcctID));
auto sle = ac.view().peek(keylet::account(ammAcctID));
if (!sle)
return false;
BEAST_EXPECT(sle->at(~sfAMMID));
BEAST_EXPECT(sle->at(~sfAMMID) == ammKey);
sle->at(sfBalance) = beast::zero;
ac.view().erase(sle);
return true;
@@ -298,7 +321,7 @@ class Invariants_test : public beast::unit_test::suite
// Delete all the AMM's trust lines, remove the AMM from the AMM
// account's directory (this deletes the directory), and delete
// the AMM account. Do not delete the AMM object.
auto const sle = ac.view().peek(keylet::account(ammAcctID));
auto sle = ac.view().peek(keylet::account(ammAcctID));
if (!sle)
return false;
@@ -338,6 +361,7 @@ class Invariants_test : public beast::unit_test::suite
!ac.view().exists(ownerDirKeylet) ||
ac.view().emptyDirDelete(ownerDirKeylet));
sle->at(sfBalance) = beast::zero;
ac.view().erase(sle);
return true;
@@ -1628,7 +1652,7 @@ class Invariants_test : public beast::unit_test::suite
doInvariantCheck(
{{"Loan Broker with zero OwnerCount has multiple directory "
"pages"}},
[&loanBrokerKeylet, &setupTest, this](
[&setupTest, this](
Account const& A1, Account const& A2, ApplyContext& ac) {
auto test = setupTest(A1, A2, ac);
if (!test || !test->first || !test->second)

24
src/xrpld/app/tx/detail/InvariantCheck.cpp
View File

@@ -415,10 +415,10 @@ void
AccountRootsDeletedClean::visitEntry(
bool isDelete,
std::shared_ptr<SLE const> const& before,
std::shared_ptr<SLE const> const&)
std::shared_ptr<SLE const> const& after)
{
if (isDelete && before && before->getType() == ltACCOUNT_ROOT)
accountsDeleted_.emplace_back(before);
accountsDeleted_.emplace_back(before, after);
}
bool
@@ -462,9 +462,21 @@ AccountRootsDeletedClean::finalize(
return false;
};
for (auto const& accountSLE : accountsDeleted_)
for (auto const& [before, after] : accountsDeleted_)
{
auto const accountID = accountSLE->getAccountID(sfAccount);
auto const accountID = before->getAccountID(sfAccount);
// An account should not be deleted with a balance
if (after->at(sfBalance) != beast::zero)
{
JLOG(j.fatal()) << "Invariant failed: account deletion left "
"behind a non-zero balance";
XRPL_ASSERT(
enforce,
"ripple::AccountRootsDeletedClean::finalize : "
"account deletion left behind a non-zero balance");
if (enforce)
return false;
}
// Simple types
for (auto const& [keyletfunc, _, __] : directAccountKeylets)
{
@@ -490,9 +502,9 @@ AccountRootsDeletedClean::finalize(
// Keys directly stored in the AccountRoot object
for (auto const& field : getPseudoAccountFields())
{
if (accountSLE->isFieldPresent(*field))
if (before->isFieldPresent(*field))
{
auto const key = accountSLE->getFieldH256(*field);
auto const key = before->getFieldH256(*field);
if (objectExists(keylet::unchecked(key)) && enforce)
return false;
}

9
src/xrpld/app/tx/detail/InvariantCheck.h
View File

@@ -174,7 +174,14 @@ public:
*/
class AccountRootsDeletedClean
{
std::vector<std::shared_ptr<SLE const>> accountsDeleted_;
// Pair is <before, after>. Before is used for most of the checks, so that
// if, for example, and object ID field is cleared, but the object is not
// deleted, it can still be found. After is used specifically for any checks
// that are expected as part of the deletion, such as zeroing out the
// balance.
std::vector<
std::pair<std::shared_ptr<SLE const>, std::shared_ptr<SLE const>>>
accountsDeleted_;
public:
void