Compare commits

...

5 Commits

Author SHA1 Message Date
Gregory Tsipenyuk
d166c8dd18 Address reviewer's feedback. 2026-06-06 09:07:22 -04:00
Gregory Tsipenyuk
981caa344f Merge branch 'develop' into gregtatcam/mpt/audit-clawback-invariant-fix 2026-06-01 13:45:31 -04:00
Gregory Tsipenyuk
2e662bc55d Merge branch 'develop' into gregtatcam/mpt/audit-clawback-invariant-fix 2026-05-26 09:39:14 -04:00
Gregory Tsipenyuk
90565e6450 Merge branch 'develop' into gregtatcam/mpt/audit-clawback-invariant-fix 2026-05-20 09:50:56 -04:00
Gregory Tsipenyuk
7481430413 Strengthen Clawback invariant checks for MPT balances 2026-05-17 14:46:58 -04:00
5 changed files with 349 additions and 25 deletions

View File

@@ -310,17 +310,26 @@ public:
};
/**
* @brief Invariant: Token holder's trustline balance cannot be negative after
* Clawback.
* @brief Invariant: Token holder's trustline/MPT balance cannot be invalid
* after Clawback.
*
* We iterate all the trust lines affected by this transaction and ensure
* that no more than one trustline is modified, and also holder's balance is
* non-negative.
* non-negative. When featureMPTokensV2 is enabled, also verify the holder's
* raw trustline/MPToken balance decreased by the clawed amount.
*/
class ValidClawback
{
struct EntryChange
{
SLE::const_pointer before;
SLE::const_pointer after;
};
std::uint32_t trustlinesChanged_ = 0;
std::uint32_t mptokensChanged_ = 0;
EntryChange iou_;
EntryChange mpt_;
public:
void
@@ -413,7 +422,7 @@ using InvariantChecks = std::tuple<
ValidLoanBroker,
ValidLoan,
ValidVault,
ValidMPTPayment,
ValidMPTBalanceChanges,
ValidAmounts,
ValidMPTTransfer>;

View File

@@ -48,7 +48,7 @@ public:
* - OutstandingAmount after = OutstandingAmount before +
* sum (MPT after - MPT before) - this is total MPT credit/debit
*/
class ValidMPTPayment
class ValidMPTBalanceChanges
{
enum class Order { Before = 0, After = 1 };
struct MPTData

View File

@@ -755,14 +755,49 @@ ValidNewAccountRoot::finalize(
//------------------------------------------------------------------------------
static std::optional<STAmount>
clawbackTrustLineBalanceInHolderTerms(
SLE::const_pointer const& sle,
AccountID const& holder,
AccountID const& issuer,
Currency const& currency)
{
if (!sle)
return STAmount{Issue{currency, issuer}};
if (sle->getType() != ltRIPPLE_STATE ||
sle->key() != keylet::line(holder, issuer, currency).key)
{
return std::nullopt;
}
STAmount balance = sle->getFieldAmount(sfBalance);
if (holder > issuer)
balance.negate();
balance.get<Issue>().account = issuer;
return balance;
}
void
ValidClawback::visitEntry(bool, SLE::const_ref before, SLE::const_ref)
ValidClawback::visitEntry(bool isDelete, SLE::const_ref before, SLE::const_ref after)
{
if (before && before->getType() == ltRIPPLE_STATE)
{
trustlinesChanged_++;
iou_.before = before;
}
if (!isDelete && after && after->getType() == ltRIPPLE_STATE)
iou_.after = after;
if (before && before->getType() == ltMPTOKEN)
{
mptokensChanged_++;
mpt_.before = before;
}
if (!isDelete && after && after->getType() == ltMPTOKEN)
mpt_.after = after;
}
bool
@@ -791,31 +826,109 @@ ValidClawback::finalize(
}
bool const mptV2Enabled = view.rules().enabled(featureMPTokensV2);
if (trustlinesChanged_ != 0 && mptokensChanged_ != 0)
{
JLOG(j.fatal()) << "Invariant failed: trustline and MPToken both changed.";
if (mptV2Enabled)
return false;
}
if (trustlinesChanged_ == 1 || (mptV2Enabled && mptokensChanged_ == 1))
{
AccountID const issuer = tx.getAccountID(sfAccount);
STAmount const& amount = tx.getFieldAmount(sfAmount);
AccountID const& holder = amount.getIssuer();
STAmount const holderBalance = amount.asset().visit(
return amount.asset().visit(
[&](Issue const& issue) {
return accountHolds(
AccountID const issuer = tx.getAccountID(sfAccount);
AccountID const& holder = amount.getIssuer();
STAmount const holderBalance = accountHolds(
view, holder, issue.currency, issuer, FreezeHandling::IgnoreFreeze, j);
if (holderBalance.signum() < 0)
{
JLOG(j.fatal()) << "Invariant failed: trustline or MPT balance is negative";
return false;
}
if (!iou_.before)
{
JLOG(j.fatal())
<< "Invariant failed: trustline clawback changed the wrong line";
return !mptV2Enabled;
}
auto const beforeBalance = clawbackTrustLineBalanceInHolderTerms(
iou_.before, holder, issuer, issue.currency);
auto const afterBalance = clawbackTrustLineBalanceInHolderTerms(
iou_.after, holder, issuer, issue.currency);
if (!beforeBalance || !afterBalance)
{
JLOG(j.fatal())
<< "Invariant failed: trustline clawback changed the wrong line";
return !mptV2Enabled;
}
STAmount clawAmount = amount;
clawAmount.get<Issue>().account = issuer;
if (clawAmount <= beast::kZero)
{
JLOG(j.fatal()) << "Invariant failed: trustline clawback amount is invalid";
return !mptV2Enabled;
}
if (*afterBalance > *beforeBalance ||
(*beforeBalance - *afterBalance) != std::min(*beforeBalance, clawAmount))
{
JLOG(j.fatal())
<< "Invariant failed: trustline clawback balance change is invalid";
return !mptV2Enabled;
}
return true;
},
[&](MPTIssue const& issue) {
return accountHolds(
view,
holder,
issue,
FreezeHandling::IgnoreFreeze,
AuthHandling::IgnoreAuth,
j);
});
auto const holder = tx[~sfHolder];
if (!holder)
{
JLOG(j.fatal()) << "Invariant failed: MPT clawback missing holder";
return !mptV2Enabled;
}
if (holderBalance.signum() < 0)
{
JLOG(j.fatal()) << "Invariant failed: trustline or MPT balance is negative";
return false;
}
if (!mpt_.before || !mpt_.after)
{
JLOG(j.fatal()) << "Invariant failed: MPT clawback token is missing";
return !mptV2Enabled;
}
if (mpt_.before->getAccountID(sfAccount) != *holder ||
mpt_.after->getAccountID(sfAccount) != *holder ||
(*mpt_.before)[sfMPTokenIssuanceID] != issue.getMptID() ||
(*mpt_.after)[sfMPTokenIssuanceID] != issue.getMptID())
{
JLOG(j.fatal()) << "Invariant failed: MPT clawback changed the wrong token";
return !mptV2Enabled;
}
auto const before = mpt_.before->getFieldU64(sfMPTAmount);
auto const after = mpt_.after->getFieldU64(sfMPTAmount);
if (amount.negative() || amount.mantissa() == 0)
{
JLOG(j.fatal()) << "Invariant failed: MPT clawback amount is invalid";
return !mptV2Enabled;
}
auto const clawAmount = amount.mantissa();
// MPT balances are unsigned, so validate the raw holder
// debit instead of routing through accountHolds().
if (after > before || (before - after) != std::min(before, clawAmount))
{
JLOG(j.fatal())
<< "Invariant failed: MPT clawback balance change is invalid";
return !mptV2Enabled;
}
return true;
});
}
}
else

View File

@@ -365,7 +365,7 @@ ValidMPTIssuance::finalize(
}
void
ValidMPTPayment::visitEntry(bool, SLE::const_ref before, SLE::const_ref after)
ValidMPTBalanceChanges::visitEntry(bool, SLE::const_ref before, SLE::const_ref after)
{
if (overflow_)
return;
@@ -427,7 +427,7 @@ ValidMPTPayment::visitEntry(bool, SLE::const_ref before, SLE::const_ref after)
}
bool
ValidMPTPayment::finalize(
ValidMPTBalanceChanges::finalize(
STTx const& tx,
TER const result,
XRPAmount const,

View File

@@ -4265,6 +4265,208 @@ class Invariants_test : public beast::unit_test::Suite
return true;
});
// Invalid IOU clawback delta must fail once MPTokensV2 enforces before/after validation.
{
Env env(*this, defaultAmendments());
Account const issuer{"issuer"};
Account const holder{"holder"};
Account const other{"other"};
env.fund(XRP(1'000), issuer, holder, other);
auto const usd = issuer["USD"];
env.trust(usd(100), holder);
env(pay(issuer, holder, usd(100)));
env.close();
doInvariantCheck(
std::move(env),
holder,
other,
{{"Invariant failed: trustline clawback balance change is invalid"}},
[issuer, usd](Account const& holder, Account const&, ApplyContext& ac) {
auto sle = ac.view().peek(keylet::line(holder.id(), issuer.id(), usd.currency));
if (!sle)
return false;
STAmount balance{Issue{usd.currency, issuer.id()}, 80};
if (holder.id() > issuer.id())
balance.negate();
sle->setFieldAmount(sfBalance, balance);
ac.view().update(sle);
return true;
},
XRPAmount{},
STTx{
ttCLAWBACK,
[&](STObject& tx) {
tx[sfAccount] = issuer.id();
tx[sfAmount] = STAmount{Issue{usd.currency, holder.id()}, 10};
}},
{tecINVARIANT_FAILED, tefINVARIANT_FAILED});
}
// Full IOU clawback may delete the trustline; missing after-SLE represents zero balance.
{
Env env(*this, defaultAmendments());
Account const issuer{"issuer"};
Account const holder{"holder"};
Account const other{"other"};
env.fund(XRP(1'000), issuer, holder, other);
auto const usd = issuer["USD"];
env.trust(usd(100), holder);
env(pay(issuer, holder, usd(100)));
env.close();
doInvariantCheck(
std::move(env),
holder,
other,
{},
[issuer, usd](Account const& holder, Account const&, ApplyContext& ac) {
auto const sle =
ac.view().peek(keylet::line(holder.id(), issuer.id(), usd.currency));
if (!sle)
return false;
ac.view().erase(sle);
return true;
},
XRPAmount{},
STTx{
ttCLAWBACK,
[&](STObject& tx) {
tx[sfAccount] = issuer.id();
tx[sfAmount] = STAmount{Issue{usd.currency, holder.id()}, 100};
}},
{tesSUCCESS, tesSUCCESS});
}
// Pre-MPTokensV2 invalid IOU clawback delta logs but remains non-enforcing.
{
Env env(*this, defaultAmendments() - featureMPTokensV2);
Account const issuer{"issuer"};
Account const holder{"holder"};
Account const other{"other"};
env.fund(XRP(1'000), issuer, holder, other);
auto const usd = issuer["USD"];
env.trust(usd(100), holder);
env(pay(issuer, holder, usd(100)));
env.close();
doInvariantCheck(
std::move(env),
holder,
other,
{{"Invariant failed: trustline clawback balance change is invalid"}},
[issuer, usd](Account const& holder, Account const&, ApplyContext& ac) {
auto sle = ac.view().peek(keylet::line(holder.id(), issuer.id(), usd.currency));
if (!sle)
return false;
STAmount balance{Issue{usd.currency, issuer.id()}, 80};
if (holder.id() > issuer.id())
balance.negate();
sle->setFieldAmount(sfBalance, balance);
ac.view().update(sle);
return true;
},
XRPAmount{},
STTx{
ttCLAWBACK,
[&](STObject& tx) {
tx[sfAccount] = issuer.id();
tx[sfAmount] = STAmount{Issue{usd.currency, holder.id()}, 10};
}},
{tesSUCCESS, tesSUCCESS});
}
// Invalid MPT clawback delta must fail when raw MPToken debit mismatches sfAmount.
{
Env env(*this, defaultAmendments());
Account const issuer{"issuer"};
Account const holder{"holder"};
Account const other{"other"};
env.fund(XRP(1'000), issuer, holder, other);
MPTTester const mpt(
{.env = env, .issuer = issuer, .holders = {holder}, .pay = 100, .maxAmt = 100});
auto const id = mpt.issuanceID();
doInvariantCheck(
std::move(env),
holder,
other,
{{"Invariant failed: MPT clawback balance change is invalid"}},
[id](Account const& holder, Account const&, ApplyContext& ac) {
auto const sleToken = ac.view().peek(keylet::mptoken(id, holder));
auto const sleIssuance = ac.view().peek(keylet::mptIssuance(id));
if (!sleToken || !sleIssuance)
return false;
sleToken->setFieldU64(sfMPTAmount, 80);
sleIssuance->setFieldU64(sfOutstandingAmount, 80);
ac.view().update(sleToken);
ac.view().update(sleIssuance);
return true;
},
XRPAmount{},
STTx{
ttCLAWBACK,
[&](STObject& tx) {
tx[sfAccount] = issuer.id();
tx[sfHolder] = holder.id();
tx[sfAmount] = STAmount{MPTIssue{id}, 10};
}},
{tecINVARIANT_FAILED, tefINVARIANT_FAILED});
}
// A clawback that mutates both IOU and MPT entries must fail under MPTokensV2.
{
Env env(*this, defaultAmendments());
Account const issuer{"issuer"};
Account const holder{"holder"};
Account const other{"other"};
env.fund(XRP(1'000), issuer, holder, other);
auto const usd = issuer["USD"];
env.trust(usd(100), holder);
env(pay(issuer, holder, usd(100)));
MPTTester const mpt(
{.env = env, .issuer = issuer, .holders = {holder}, .pay = 100, .maxAmt = 100});
auto const id = mpt.issuanceID();
doInvariantCheck(
std::move(env),
holder,
other,
{{"Invariant failed: trustline and MPToken both changed"}},
[issuer, usd, id](Account const& holder, Account const&, ApplyContext& ac) {
auto const sleLine =
ac.view().peek(keylet::line(holder.id(), issuer.id(), usd.currency));
auto const sleToken = ac.view().peek(keylet::mptoken(id, holder.id()));
auto const sleIssuance = ac.view().peek(keylet::mptIssuance(id));
if (!sleLine || !sleToken || !sleIssuance)
return false;
STAmount balance{Issue{usd.currency, issuer.id()}, 90};
if (holder.id() > issuer.id())
balance.negate();
sleLine->setFieldAmount(sfBalance, balance);
sleToken->setFieldU64(sfMPTAmount, 90);
sleIssuance->setFieldU64(sfOutstandingAmount, 90);
ac.view().update(sleLine);
ac.view().update(sleToken);
ac.view().update(sleIssuance);
return true;
},
XRPAmount{},
STTx{
ttCLAWBACK,
[&](STObject& tx) {
tx[sfAccount] = issuer.id();
tx[sfHolder] = holder.id();
tx[sfAmount] = STAmount{MPTIssue{id}, 10};
}},
{tecINVARIANT_FAILED, tefINVARIANT_FAILED});
}
// More MPTokens created than expected
std::array<std::pair<xrpl::TxType, std::uint8_t>, 4> const tests = {
std::make_pair(ttAMM_WITHDRAW, 2),