Skip to content

Commit

Permalink
remove fee from outer batch
Browse files Browse the repository at this point in the history
  • Loading branch information
@dangell7
dangell7 committed Dec 17, 2024
1 parent 175ad0b commit 38365b5
Show file tree
Hide file tree
Showing 8 changed files with 108 additions and 63 deletions.
116 changes: 93 additions & 23 deletions src/test/app/Batch_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -773,10 +773,10 @@ class Batch_test : public beast::unit_test::suite

auto const batchFee = XRP(1);
auto const seq = env.seq(alice);
// env(batch::batch(alice, seq, batchFee, tfAllOrNothing),
// batch::add(pay(alice, bob, XRP(10)), seq + 1),
// batch::add(pay(alice, bob, XRP(10)), seq + 2),
// ter(tecBATCH_FAILURE));
env(batch::batch(alice, seq, batchFee, tfAllOrNothing),
batch::add(pay(alice, bob, XRP(10)), seq + 1),
batch::add(pay(alice, bob, XRP(10)), seq + 2),
ter(tesSUCCESS));
auto const txIDs = env.tx()->getFieldV256(sfTransactionIDs);
std::vector<TestBatchData> testCases = {
{"tesSUCCESS", to_string(txIDs[0])},
Expand All @@ -798,6 +798,75 @@ class Batch_test : public beast::unit_test::suite
BEAST_EXPECT(env.balance(bob) == preBob);
}

void
testBadInnerFee(FeatureBitset features)
{
testcase("bad inner fee");

using namespace test::jtx;
using namespace std::literals;

test::jtx::Env env{*this, envconfig(), nullptr, beast::severities::kTrace};

auto const feeDrops = env.current()->fees().base;
auto const alice = Account("alice");
auto const bob = Account("bob");
auto const carol = Account("carol");
auto const gw = Account("gw");
auto const USD = gw["USD"];

env.fund(XRP(1000), alice, bob, carol, gw);
env.close();
env.trust(USD(1000), alice, bob);
env(pay(gw, alice, USD(100)));
env(pay(gw, bob, USD(100)));
env.close();

auto const preAlice = env.balance(alice);
auto const preBob = env.balance(bob);

auto const seq = env.seq(alice);
auto const ammCreate = [&env, &alice](STAmount const& amount, STAmount const& amount2) {
Json::Value jv;
jv[jss::Account] = alice.human();
jv[jss::Amount] = amount.getJson(JsonOptions::none);
jv[jss::Amount2] = amount2.getJson(JsonOptions::none);
jv[jss::TradingFee] = 0;
jv[jss::TransactionType] = jss::AMMCreate;
return jv;
};

auto const batchFee = ((0 + 2) * feeDrops) + feeDrops * 2;
env(batch::batch(alice, seq, batchFee, tfAllOrNothing),
batch::add(ammCreate(XRP(10), USD(10)), seq + 1),
batch::add(pay(alice, bob, XRP(10)), seq + 2),
ter(tesSUCCESS));
auto const txIDs = env.tx()->getFieldV256(sfTransactionIDs);
std::vector<TestBatchData> testCases = {
{"tesSUCCESS", to_string(txIDs[0])},
{"tecUNFUNDED_PAYMENT", to_string(txIDs[1])},
};
env.close();

Json::Value params;
params[jss::ledger_index] = env.current()->seq() - 1;
params[jss::transactions] = true;
params[jss::expand] = true;
auto const jrr = env.rpc("json", "ledger", to_string(params));
std::cout << "jrr: " << jrr << "\n";
auto const txn = getTxByIndex(jrr, 0);
validateBatchTxns(txn[jss::metaData], 2, testCases);
validateBatchPreMeta(txn[jss::metaData], preAlice, seq);

std::cout << "env.seq(alice): " << env.seq(alice) << "\n";
std::cout << "env.balance(alice): " << env.balance(alice) << "\n";
std::cout << "env.balance(bob): " << env.balance(bob) << "\n";

BEAST_EXPECT(env.seq(alice) == 8);
BEAST_EXPECT(env.balance(alice) == preAlice - batchFee);
BEAST_EXPECT(env.balance(bob) == preBob);
}

void
testAllOrNothing(FeatureBitset features)
{
Expand Down Expand Up @@ -1867,30 +1936,31 @@ class Batch_test : public beast::unit_test::suite
void
testWithFeats(FeatureBitset features)
{
testEnable(features);
testPreflight(features);
// testEnable(features);
// testPreflight(features);
// testNonTecInner(features);
// testBadSequence(features);
// testBadFeeNoSigner(features);
// testBadFeeSigner(features);
// testChangesBetweenViews(features);
testAllOrNothing(features);
testOnlyOne(features);
testUntilFailure(features);
testIndependent(features);
testMultiParty(features);
testMultisign(features);
testMultisignMultiParty(features);
testBatchType(features);
testSubmit(features);
testNoAccount(features);
testAccountSet(features);
testObjectCreateSequence(features);
testObjectCreateTicket(features);
testObjectCreate3rdParty(features);
testTicketsOuter(features);
testTicketsInner(features);
testTicketsOuterInner(features);
testBadInnerFee(features);
// testAllOrNothing(features);
// testOnlyOne(features);
// testUntilFailure(features);
// testIndependent(features);
// testMultiParty(features);
// testMultisign(features);
// testMultisignMultiParty(features);
// testBatchType(features);
// testSubmit(features);
// testNoAccount(features);
// testAccountSet(features);
// testObjectCreateSequence(features);
// testObjectCreateTicket(features);
// testObjectCreate3rdParty(features);
// testTicketsOuter(features);
// testTicketsInner(features);
// testTicketsOuterInner(features);
}

public:
Expand Down
6 changes: 4 additions & 2 deletions src/test/jtx/batch.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,14 @@ class add
Json::Value txn_;
std::uint32_t seq_;
std::optional<std::uint32_t> ticket_;
std::optional<std::uint32_t> fee_;

public:
add(Json::Value const& txn,
std::uint32_t const& sequence,
std::optional<std::uint32_t> const& ticket = std::nullopt)
: txn_(txn), seq_(sequence), ticket_(ticket)
std::optional<std::uint32_t> const& ticket = std::nullopt,
std::optional<std::uint32_t> const& fee = std::nullopt)
: txn_(txn), seq_(sequence), ticket_(ticket), fee_(fee)
{
}

Expand Down
8 changes: 7 additions & 1 deletion src/test/jtx/impl/batch.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,12 +65,18 @@ add::operator()(Env& env, JTx& jt) const
batchTransaction = Json::Value{};
batchTransaction[jss::RawTransaction] = txn_;
batchTransaction[jss::RawTransaction][jss::SigningPubKey] = "";
batchTransaction[jss::RawTransaction][sfFee.jsonName] = 0;
batchTransaction[jss::RawTransaction][jss::Sequence] = seq_;
batchTransaction[jss::RawTransaction][jss::Flags] =
batchTransaction[jss::RawTransaction][jss::Flags].asUInt() |
tfInnerBatchTxn;

// Optionally set new fee
if (fee_.has_value())
batchTransaction[jss::RawTransaction][jss::Fee] = to_string(*fee_);

if (!batchTransaction[jss::RawTransaction][jss::Fee] && !fee_.has_value())
batchTransaction[jss::RawTransaction][sfFee.jsonName] = to_string(env.current()->fees().base);

// Optionally set ticket sequence
if (ticket_.has_value())
{
Expand Down
23 changes: 1 addition & 22 deletions src/xrpld/app/tx/detail/Batch.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,33 +32,12 @@ namespace ripple {
XRPAmount
Batch::calculateBaseFee(ReadView const& view, STTx const& tx)
{
// Calculate the Inner Txn Fees
XRPAmount txnFees{0};

if (tx.isFieldPresent(sfRawTransactions))
{
XRPAmount txFees{0};
auto const& txns = tx.getFieldArray(sfRawTransactions);
for (STObject txn : txns)
{
// FIXME: THIS IS BROKEN! This will call the base class' version of
// calculateBaseFee, but many transactors customize what the
// base fee should (e.g. EscrowFinish). As written, it would
// be cheaper to submit some transactions as part of a batch
// or even as a single transaction batch than to submit them
// individually.
STTx const stx = STTx{std::move(txn)};
txFees += Transactor::calculateBaseFee(view, tx);
}
txnFees += txFees;
}

// Calculate the BatchSigners Fees
std::int32_t signerCount = tx.isFieldPresent(sfBatchSigners)
? tx.getFieldArray(sfBatchSigners).size()
: 0;

return ((signerCount + 2) * view.fees().base) + txnFees;
return ((signerCount + 2) * view.fees().base);
}

NotTEC
Expand Down
10 changes: 0 additions & 10 deletions src/xrpld/app/tx/detail/Transactor.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -186,16 +186,6 @@ Transactor::checkFee(PreclaimContext const& ctx, XRPAmount baseFee)
return temBAD_FEE;

auto const feePaid = ctx.tx[sfFee].xrp();

if (ctx.tx.isFlag(tfInnerBatchTxn))
{
if (feePaid == beast::zero)
return tesSUCCESS;

JLOG(ctx.j.warn()) << "Batch: sfFee must be zero.";
return temBAD_FEE;
}

if (!isLegalAmount(feePaid) || feePaid < beast::zero)
return temBAD_FEE;

Expand Down
3 changes: 1 addition & 2 deletions src/xrpld/ledger/detail/ApplyStateTable.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,7 @@ ApplyStateTable::apply(
TER ter,
std::optional<STAmount> const& deliver,
std::optional<uint256 const> const& batchId,
beast::Journal j,
int xxx)
beast::Journal j)
{
// Build metadata and insert
auto const sTx = std::make_shared<Serializer>();
Expand Down
3 changes: 1 addition & 2 deletions src/xrpld/ledger/detail/ApplyStateTable.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,7 @@ class ApplyStateTable
TER ter,
std::optional<STAmount> const& deliver,
std::optional<uint256 const> const& batchId,
beast::Journal j,
int xxx);
beast::Journal j);

bool
exists(ReadView const& base, Keylet const& k) const;
Expand Down
2 changes: 1 addition & 1 deletion src/xrpld/ledger/detail/ApplyViewImpl.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ ApplyViewImpl::apply(
std::optional<uint256> batchId,
beast::Journal j)
{
items_.apply(to, tx, ter, deliver_, batchId, j, 89);
items_.apply(to, tx, ter, deliver_, batchId, j);
}

std::size_t
Expand Down

0 comments on commit 38365b5

Please sign in to comment.