Skip to content

Commit

Permalink
Test the new invariant
Browse files Browse the repository at this point in the history
* Resolves XRPLF#4638
  • Loading branch information
ximinez committed Sep 1, 2023
1 parent 19a76ae commit da3692c
Show file tree
Hide file tree
Showing 3 changed files with 111 additions and 4 deletions.
1 change: 1 addition & 0 deletions Builds/CMake/RippledCore.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -1117,5 +1117,6 @@ if (tests)
# these two seem to produce conflicts in beast teardown template methods
src/test/rpc/ValidatorRPC_test.cpp
src/test/rpc/ShardArchiveHandler_test.cpp
src/test/ledger/Invariants_test.cpp
PROPERTIES SKIP_UNITY_BUILD_INCLUSION TRUE)
endif () #tests
2 changes: 1 addition & 1 deletion src/ripple/app/tx/impl/InvariantCheck.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,7 @@ AccountRootsDeletedClean::finalize(
&keylet::nftpage_min,
&keylet::nftpage_max};

static auto const noObject = [&](auto const& keylet) {
auto const noObject = [&](auto const& keylet) {
// It may be a little unintuitive, but this function follows the same
// return paradigm as the `finalize` function. True if the check passes
// (object is NOT found). False if it fails (object IS found).
Expand Down
112 changes: 109 additions & 3 deletions src/test/ledger/Invariants_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include <ripple/protocol/STLedgerEntry.h>
#include <boost/algorithm/string/predicate.hpp>
#include <test/jtx.h>
#include <test/jtx/AMM.h>
#include <test/jtx/Env.h>

namespace ripple {
Expand All @@ -38,22 +39,32 @@ class Invariants_test : public beast::unit_test::suite
test::jtx::Account const& b,
ApplyContext& ac)>;

// this is an uncommon setup for additional setup beyond creating two
// accounts. The preclose function is used to run additional transactions on
// the Env needed to set up the test.
using Preclose = std::function<bool(
test::jtx::Account const& a,
test::jtx::Account const& b,
test::jtx::Env& env)>;

void
doInvariantCheck(
std::vector<std::string> const& expect_logs,
Precheck const& precheck,
XRPAmount fee = XRPAmount{},
STTx tx = STTx{ttACCOUNT_SET, [](STObject&) {}},
std::initializer_list<TER> ters = {
tecINVARIANT_FAILED,
tefINVARIANT_FAILED})
std::initializer_list<TER> ters =
{tecINVARIANT_FAILED, tefINVARIANT_FAILED},
Preclose const& preclose = {})
{
using namespace test::jtx;
Env env{*this};

Account A1{"A1"};
Account A2{"A2"};
env.fund(XRP(1000), A1, A2);
if (preclose)
BEAST_EXPECT(preclose(A1, A2, env));
env.close();

OpenView ov{*env.current()};
Expand Down Expand Up @@ -162,6 +173,100 @@ class Invariants_test : public beast::unit_test::suite
STTx{ttACCOUNT_DELETE, [](STObject& tx) {}});
}

void
testAccountRootsDeletedClean()
{
using namespace test::jtx;
testcase << "account root deletion left artifact";

// This list should include all of the keylet functions that take a
// single AccountID parameter
using fType = std::function<Keylet(AccountID const& id)>;
static std::map<std::string, fType> const keyletfuncs{
// Skip the account root, since that's what's being deleted.
//{"AccountRoot", &keylet::account},
{"DirectoryNode", &keylet::ownerDir},
{"SignerList", &keylet::signers},
{"NFTokenPage", &keylet::nftpage_min},
{"NFTokenPage", &keylet::nftpage_max}};

for (auto const& item : keyletfuncs)
{
auto const& type = item.first;
fType const& keyletfunc = item.second;
doInvariantCheck(
{{"account deletion left behind a " + type + " object"}},
[&](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;

auto const key = std::invoke(keyletfunc, a1);
auto const newSLE = std::make_shared<SLE>(key);
ac.view().erase(sleA1);
ac.view().insert(newSLE);

return true;
},
XRPAmount{},
STTx{ttACCOUNT_DELETE, [](STObject& tx) {}});
};

AccountID ammAcctID;
uint256 ammKey;
Issue ammIssue;
doInvariantCheck(
{{"account deletion left behind a AMM object"}},
[&](Account const& A1, Account const& A2, ApplyContext& ac) {
// Delete the AMM account without deleting the AMM object
auto const sle = ac.view().peek(keylet::account(ammAcctID));
if (!sle)
return false;

BEAST_EXPECT(sle->at(~sfAMMID));
BEAST_EXPECT(sle->at(~sfAMMID) == ammKey);

for (auto const trustKeylet :
{keylet::line(ammAcctID, A1["USD"]),
keylet::line(A1, ammIssue)})
{
if (auto const line = ac.view().peek(trustKeylet); !line)
{
return false;
}
else
{
STAmount lowLimit = line->at(sfLowLimit);
STAmount highLimit = line->at(sfHighLimit);
BEAST_EXPECT(
trustDelete(
ac.view(),
line,
lowLimit.getIssuer(),
highLimit.getIssuer(),
ac.journal) == tesSUCCESS);
}
}

ac.view().erase(sle);

return true;
},
XRPAmount{},
STTx{ttAMM_WITHDRAW, [](STObject& tx) {}},
{tecINVARIANT_FAILED, tefINVARIANT_FAILED},
[&](Account const& A1, Account const& A2, Env& env) {
AMM amm(env, A1, XRP(100), A1["USD"](50));
ammAcctID = amm.ammAccount();
ammKey = amm.ammID();
ammIssue = amm.lptIssue();
return true;
});
}

void
testTypesMatch()
{
Expand Down Expand Up @@ -467,6 +572,7 @@ class Invariants_test : public beast::unit_test::suite
{
testXRPNotCreated();
testAccountRootsNotRemoved();
testAccountRootsDeletedClean();
testTypesMatch();
testNoXRPTrustLine();
testXRPBalanceCheck();
Expand Down

0 comments on commit da3692c

Please sign in to comment.