check tem failures to preflight

This commit is contained in:
Denis Angell
2025-01-28 11:12:02 +01:00
parent f0222d9fa6
commit 0c4f5680e6
2 changed files with 102 additions and 88 deletions

View File

@@ -88,6 +88,69 @@ preflight0(PreflightContext const& ctx)
return tesSUCCESS;
}
NotTEC
checkServiceFee(PreflightContext const& ctx)
{
if (!ctx.tx.isFieldPresent(sfServiceFee))
return tesSUCCESS;
if (!ctx.rules.enabled(featureServiceFee))
{
JLOG(ctx.j.debug())
<< "ServiceFee: Service fee feature is not enabled.";
return temDISABLED;
}
STObject const& obj = const_cast<ripple::STTx&>(ctx.tx)
.getField(sfServiceFee)
.downcast<STObject>();
// This should be enforced by template but doesn't hurt to
// defensively check it here.
if (!obj.isFieldPresent(sfDestination) || !obj.isFieldPresent(sfAmount) ||
obj.getCount() != 2)
{
JLOG(ctx.j.debug()) << "ServiceFee: Malformed: Destination and Amount "
"fields are required.";
return temINVALID;
}
if (ctx.tx.getAccountID(sfAccount) == obj.getAccountID(sfDestination))
{
JLOG(ctx.j.debug()) << "ServiceFee: Malformed: Destination may not be "
"the same as the source account.";
return temDST_IS_SRC;
}
auto const amount = obj.getFieldAmount(sfAmount);
if (!isXRP(amount))
{
if (!isLegalNet(amount))
{
JLOG(ctx.j.debug())
<< "ServiceFee: Malformed: Amount must be a valid net amount.";
return temBAD_AMOUNT;
}
if (isBadCurrency(amount))
{
JLOG(ctx.j.debug())
<< "ServiceFee: Malformed: Currency is not allowed.";
return temBAD_CURRENCY;
}
}
if (amount <= beast::zero)
{
JLOG(ctx.j.debug())
<< "ServiceFee: Malformed: Amount must be a positive value.";
return temBAD_AMOUNT;
}
return tesSUCCESS;
}
/** Performs early sanity checks on the account and fee fields */
NotTEC
preflight1(PreflightContext const& ctx)
@@ -100,12 +163,6 @@ preflight1(PreflightContext const& ctx)
return temMALFORMED;
}
if (ctx.tx.isFieldPresent(sfServiceFee) &&
!ctx.rules.enabled(featureServiceFee))
{
return temMALFORMED;
}
auto const ret = preflight0(ctx);
if (!isTesSuccess(ret))
return ret;
@@ -171,6 +228,9 @@ preflight1(PreflightContext const& ctx)
ctx.tx.isFieldPresent(sfAccountTxnID))
return temINVALID;
if (auto const ret = checkServiceFee(ctx); !isTesSuccess(ret))
return ret;
return tesSUCCESS;
}
@@ -1914,43 +1974,16 @@ Transactor::operator()()
STObject const& obj = const_cast<ripple::STTx&>(ctx_.tx)
.getField(sfServiceFee)
.downcast<STObject>();
// This should be enforced by template but doesn't hurt to
// defensively check it here.
if (!obj.isFieldPresent(sfDestination) ||
!obj.isFieldPresent(sfAmount) || obj.getCount() != 2)
{
JLOG(j_.warn())
<< "service fee not applied - malformed inner object.";
break;
}
auto const src = ctx_.tx.getAccountID(sfAccount);
auto const dst = obj.getAccountID(sfDestination);
auto const amt = obj.getFieldAmount(sfAmount);
// sanity check fields
if (src == dst)
{
JLOG(j_.info())
<< "skipping self service-fee on " << src << ".";
break;
}
if (amt <= beast::zero)
{
JLOG(j_.info())
<< "skipping non-positive service-fee from " << src << ".";
break;
}
// check if the source exists
auto const& sleSrc = view().read(keylet::account(src));
if (!sleSrc)
{
// this can happen if the account was just deleted
JLOG(j_.info()) << "service fee not applied because source "
JLOG(j_.debug()) << "service fee not applied because source "
<< src << " does not exist.";
break;
}
@@ -1959,7 +1992,7 @@ Transactor::operator()()
// service fee cannot be used to create accounts.
if (!view().exists(keylet::account(dst)))
{
JLOG(j_.info())
JLOG(j_.debug())
<< "service fee not applied because destination " << dst
<< " does not exist.";
break;
@@ -1977,7 +2010,7 @@ Transactor::operator()()
if (after < view().fees().accountReserve(
sleSrc->getFieldU32(sfOwnerCount)))
{
JLOG(j_.info())
JLOG(j_.debug())
<< "service fee not applied because source " << src
<< " cannot pay it (native).";
break;
@@ -1985,14 +2018,14 @@ Transactor::operator()()
PaymentSandbox pv(&view());
auto res = accountSend(pv, src, dst, amt, j_, false);
if (res == tesSUCCESS)
if (isTesSuccess(res))
{
pv.apply(ctx_.rawView());
break;
}
JLOG(j_.warn()) << "service fee (native) not applied because "
"accountSend failed.";
"accountSend failed.";
break;
}
@@ -2003,22 +2036,9 @@ Transactor::operator()()
// be able to be xfer'd to it
auto const issuer = amt.getIssuer();
if (auto const result = requireAuth(view(), amt.issue(), dst);
!isTesSuccess(result))
{
JLOG(j_.info())
<< "service fee not applied because destination " << dst
<< " does not have a authorized trustline for "
"currency: "
<< amt.getCurrency() << " issued by: " << toBase58(issuer)
<< ".";
break;
}
if (issuer != src && !view().exists(keylet::line(src, amt.issue())))
{
JLOG(j_.info())
JLOG(j_.debug())
<< "service fee not applied because source " << src
<< " has no trustline for currency: " << amt.getCurrency()
<< " issued by: " << toBase58(issuer) << ".";
@@ -2027,22 +2047,26 @@ Transactor::operator()()
if (issuer != dst && !view().exists(keylet::line(dst, amt.issue())))
{
JLOG(j_.info())
JLOG(j_.debug())
<< "service fee not applied because destination " << dst
<< " has no trustline for currency: " << amt.getCurrency()
<< " issued by: " << toBase58(issuer) << ".";
break;
}
if (dst != issuer && src != issuer &&
!trustTransferAllowed(view(), {src, dst}, amt.issue(), j_))
if (dst != issuer && src != issuer)
{
JLOG(j_.info())
<< "service fee not applied because destination " << dst
<< " has frozen trustline for currency: "
<< amt.getCurrency() << " issued by: " << toBase58(issuer)
<< ".";
break;
TER const res =
trustTransferAllowed(view(), {src, dst}, amt.issue(), j_);
if (!isTesSuccess(res))
{
JLOG(j_.debug())
<< "service fee not applied because destination " << dst
<< " trust transfer not allowed for currency: "
<< amt.getCurrency()
<< " issued by: " << toBase58(issuer) << ".";
break;
}
}
if (src != issuer)
@@ -2056,7 +2080,7 @@ Transactor::operator()()
// TODO: xferRate
if (srcBalance < amt)
{
JLOG(j_.info())
JLOG(j_.debug())
<< "service fee not applied because source " << src
<< " has insufficient funds for currency: "
<< amt.getCurrency()
@@ -2076,7 +2100,7 @@ Transactor::operator()()
if (accountFunds(view(), dst, amt, fhZERO_IF_FROZEN, j_) + amt >
dstLimit)
{
JLOG(j_.info())
JLOG(j_.debug())
<< "service fee not applied because destination " << dst
<< " has insufficient trustline limit for "
"currency: "
@@ -2092,13 +2116,13 @@ Transactor::operator()()
STAmount saActual;
auto res = accountSend(pv, src, dst, amt, j_, false);
if (res == tesSUCCESS)
if (isTesSuccess(res))
{
pv.apply(ctx_.rawView());
break;
}
JLOG(j_.info())
JLOG(j_.warn())
<< "service fee not sent from " << src << " to " << dst
<< " for " << amt.getCurrency() << " issued by "
<< toBase58(issuer) << " because "

View File

@@ -63,16 +63,16 @@ struct ServiceFee_test : public beast::unit_test::suite
auto const preBob = env.balance(bob);
auto const preCarol = env.balance(carol);
auto const result = withSFee ? ter(tesSUCCESS) : ter(temDISABLED);
env(pay(alice, bob, XRP(10)),
fee(feeDrops),
sfee(XRP(1), carol),
ter(tesSUCCESS));
result);
env.close();
auto const postAlice = withSFee
? preAlice - feeDrops - XRP(10) - XRP(1)
: preAlice - feeDrops - XRP(10);
auto const postBob = preBob + XRP(10);
auto const postAlice =
withSFee ? preAlice - feeDrops - XRP(10) - XRP(1) : preAlice;
auto const postBob = withSFee ? preBob + XRP(10) : preBob;
auto const postCarol = withSFee ? preCarol + XRP(1) : preCarol;
BEAST_EXPECT(env.balance(alice) == postAlice);
BEAST_EXPECT(env.balance(bob) == postBob);
@@ -98,30 +98,24 @@ struct ServiceFee_test : public beast::unit_test::suite
// skipping self service-fee
{
Env env{*this, features};
auto const baseFee = env.current()->fees().base;
env.fund(XRP(1000), alice, bob);
env.close();
auto const preAlice = env.balance(alice);
auto const amt = XRP(10);
auto const sfeeAmt = XRP(1);
env(pay(alice, bob, amt), sfee(sfeeAmt, alice));
env(pay(alice, bob, amt), sfee(sfeeAmt, alice), ter(temDST_IS_SRC));
env.close();
BEAST_EXPECT(env.balance(alice) == preAlice - amt - baseFee);
}
// skipping non-positive service-fee
{
Env env{*this, features};
auto const baseFee = env.current()->fees().base;
env.fund(XRP(1000), alice, bob);
env.fund(XRP(1000), alice, bob, carol);
env.close();
auto const preAlice = env.balance(alice);
auto const amt = XRP(10);
auto const sfeeAmt = XRP(-1);
env(pay(alice, bob, amt), sfee(sfeeAmt, alice));
env(pay(alice, bob, amt), sfee(sfeeAmt, carol), ter(temBAD_AMOUNT));
env.close();
BEAST_EXPECT(env.balance(alice) == preAlice - amt - baseFee);
}
// source does not exist.
{
@@ -259,12 +253,7 @@ struct ServiceFee_test : public beast::unit_test::suite
for (auto const& t : tests)
{
//Env env{*this, features};
Env env{
*this, envconfig(), features, nullptr,
//beast::severities::kWarning
beast::severities::kInfo
};
Env env{*this, features};
auto const carol = Account("carol");
auto const USD = t.gw["USD"];
env.fund(XRP(5000), t.src, t.dst, t.gw, carol);
@@ -574,9 +563,10 @@ struct ServiceFee_test : public beast::unit_test::suite
auto const sfeeAmt = USD(1);
env(pay(alice, bob, XRP(100)), sfee(sfeeAmt, carol));
env.close();
BEAST_EXPECT(env.balance(alice, USD) == preAlice - sfeeAmt);
BEAST_EXPECT(
env.balance(alice, USD) == preAlice - sfeeAmt - USD(0.25));
BEAST_EXPECT(env.balance(carol, USD) == preCarol + sfeeAmt);
env.balance(carol, USD) == preCarol + sfeeAmt - USD(0.20));
}
// test issuer doesnt pay own rate
@@ -603,6 +593,7 @@ struct ServiceFee_test : public beast::unit_test::suite
void
testWithFeats(FeatureBitset features)
{
testEnabled(features);
testInvalid(features);
testRippleState(features);
testGateway(features);
@@ -617,7 +608,6 @@ public:
{
using namespace test::jtx;
auto const sa = supported_amendments();
testEnabled(sa);
testWithFeats(sa);
}
};