diff --git a/src/ripple/app/tx/impl/Batch.cpp b/src/ripple/app/tx/impl/Batch.cpp index 6148eb3818b..688ca8b015b 100644 --- a/src/ripple/app/tx/impl/Batch.cpp +++ b/src/ripple/app/tx/impl/Batch.cpp @@ -338,7 +338,6 @@ Batch::preflight(PreflightContext const& ctx) auto const response = invoke_preflight(preflightContext); preflightResponses.push_back(response.first); } - return preflight2(ctx); } @@ -419,18 +418,20 @@ XRPAmount Batch::calculateBaseFee(ReadView const& view, STTx const& tx) { XRPAmount extraFee{0}; - // if (tx.isFieldPresent(sfTransactions)) - // { - // XRPAmount txFees{0}; - // auto const& txns = tx.getFieldArray(sfTransactions); - // for (auto const& txn : txns) - // { - // txFees += txn.isFieldPresent(sfFee) ? txn.getFieldAmount(sfFee) : - // XRPAmount{0}; - // } - // extraFee += txFees; - // } - return Transactor::calculateBaseFee(view, tx) + extraFee; + if (tx.isFieldPresent(sfTransactions)) + { + XRPAmount txFees{0}; + auto const& txns = tx.getFieldArray(sfTransactions); + for (auto const& txn : txns) + { + auto const tt = txn.getFieldU16(sfTransactionType); + auto const txtype = safe_cast(tt); + auto const stx = STTx(txtype, [&txn](STObject& obj) { obj = std::move(txn); }); + txFees += Transactor::calculateBaseFee(view, tx); + } + extraFee += txFees; + } + return (Transactor::calculateBaseFee(view, tx) * 2) + extraFee; } } // namespace ripple diff --git a/src/ripple/app/tx/impl/InvariantCheck.cpp b/src/ripple/app/tx/impl/InvariantCheck.cpp index 523f9b568cd..c717777f88f 100644 --- a/src/ripple/app/tx/impl/InvariantCheck.cpp +++ b/src/ripple/app/tx/impl/InvariantCheck.cpp @@ -143,9 +143,6 @@ XRPNotCreated::finalize( ReadView const&, beast::Journal const& j) { - if (tx.getTxnType() == ttBATCH) - return true; - // The net change should never be positive, as this would mean that the // transaction created XRP out of thin air. That's not possible. if (drops_ > 0) diff --git a/src/ripple/app/tx/impl/Transactor.cpp b/src/ripple/app/tx/impl/Transactor.cpp index 5ae88233a64..e211ad93d33 100644 --- a/src/ripple/app/tx/impl/Transactor.cpp +++ b/src/ripple/app/tx/impl/Transactor.cpp @@ -100,23 +100,29 @@ preflight1(PreflightContext const& ctx) } // No point in going any further if the transaction fee is malformed. - if (ctx.flags == tapPREFLIGHT_BATCH) - { - if (ctx.tx.isFieldPresent(sfFee)) - { - JLOG(ctx.j.debug()) << "preflight1: batch tx contains sfFee"; - return temMALFORMED; - } - } - else + auto const fee = ctx.tx.getFieldAmount(sfFee); + if (!fee.native() || fee.negative() || !isLegalAmount(fee.xrp())) { - auto const fee = ctx.tx.getFieldAmount(sfFee); - if (!fee.native() || fee.negative() || !isLegalAmount(fee.xrp())) - { - JLOG(ctx.j.debug()) << "preflight1: invalid fee"; - return temBAD_FEE; - } + JLOG(ctx.j.debug()) << "preflight1: invalid fee"; + return temBAD_FEE; } + // if (ctx.flags == tapPREFLIGHT_BATCH) + // { + // if (ctx.tx.isFieldPresent(sfFee) && ctx.tx.getFieldAmount(sfFee) != 0) + // { + // JLOG(ctx.j.debug()) << "preflight1: batch tx contains invalid sfFee"; + // return temMALFORMED; + // } + // } + // else + // { + // auto const fee = ctx.tx.getFieldAmount(sfFee); + // if (!fee.native() || fee.negative() || !isLegalAmount(fee.xrp())) + // { + // JLOG(ctx.j.debug()) << "preflight1: invalid fee"; + // return temBAD_FEE; + // } + // } // check public key validity if (ctx.flags == tapPREFLIGHT_BATCH) @@ -224,17 +230,41 @@ Transactor::checkFee(PreclaimContext const& ctx, XRPAmount baseFee) return temBAD_FEE; auto const feePaid = ctx.tx[sfFee].xrp(); - std::cout << "feePaid: " << feePaid << "\n"; if (!isLegalAmount(feePaid) || feePaid < beast::zero) return temBAD_FEE; // Only check fee is sufficient when the ledger is open. - if (ctx.view.open()) + if (ctx.view.open() && ctx.tx.getTxnType() == ttBATCH) + { + XRPAmount feeDue = XRPAmount{ctx.view.fees().base * 2}; + auto const& txns = ctx.tx.getFieldArray(sfTransactions); + for (std::size_t i = 0; i < txns.size(); ++i) + { + auto const& txn = txns[i]; + if (!txn.isFieldPresent(sfFee)) + { + JLOG(ctx.j.warn()) + << "Batch: sfFee missing in array entry."; + return telINSUF_FEE_P; + } + + auto const _fee = txn.getFieldAmount(sfFee); + feeDue += _fee.xrp(); + } + + if (feePaid < feeDue) + { + JLOG(ctx.j.trace()) + << "Insufficient fee paid: " << to_string(feePaid) << "/" + << to_string(feeDue); + return telINSUF_FEE_P; + } + } + if (ctx.view.open() && ctx.tx.getTxnType() != ttBATCH) { auto const feeDue = minimumFee(ctx.app, baseFee, ctx.view.fees(), ctx.flags); - std::cout << "feeDue: " << feeDue << "\n"; if (feePaid < feeDue) { JLOG(ctx.j.trace()) diff --git a/src/test/app/Batch_test.cpp b/src/test/app/Batch_test.cpp index 810968d2986..8355939a45a 100644 --- a/src/test/app/Batch_test.cpp +++ b/src/test/app/Batch_test.cpp @@ -35,14 +35,15 @@ class Batch_test : public beast::unit_test::suite using namespace test::jtx; using namespace std::literals; - // test::jtx::Env env{*this, network::makeNetworkConfig(21337)}; - Env env{ - *this, - envconfig(), - features, - nullptr, - // beast::severities::kWarning - beast::severities::kTrace}; + test::jtx::Env env{*this, envconfig()}; + auto const feeDrops = env.current()->fees().base; + // Env env{ + // *this, + // envconfig(), + // features, + // nullptr, + // // beast::severities::kWarning + // beast::severities::kTrace}; auto const alice = Account("alice"); auto const bob = Account("bob"); @@ -51,68 +52,44 @@ class Batch_test : public beast::unit_test::suite env.close(); auto const seq = env.seq("alice"); + + // ttBATCH Json::Value jv; jv[jss::TransactionType] = jss::Batch; jv[jss::Account] = alice.human(); + + // Batch Transactions jv[sfTransactions.jsonName] = Json::Value{Json::arrayValue}; + + // Tx 1 jv[sfTransactions.jsonName][0U] = Json::Value{}; jv[sfTransactions.jsonName][0U][jss::BatchTransaction] = Json::Value{}; jv[sfTransactions.jsonName][0U][jss::BatchTransaction] [jss::TransactionType] = jss::AccountSet; jv[sfTransactions.jsonName][0U][jss::BatchTransaction] [sfAccount.jsonName] = alice.human(); - jv[sfTransactions.jsonName][0U][jss::BatchTransaction] - [sfDestination.jsonName] = bob.human(); jv[sfTransactions.jsonName][0U][jss::BatchTransaction][sfFee.jsonName] = - "1000000"; + to_string(feeDrops); jv[sfTransactions.jsonName][0U][jss::BatchTransaction][jss::Sequence] = seq + 1; jv[sfTransactions.jsonName][0U][jss::BatchTransaction] [jss::SigningPubKey] = strHex(alice.pk()); + + // Tx 2 jv[sfTransactions.jsonName][1U] = Json::Value{}; jv[sfTransactions.jsonName][1U][jss::BatchTransaction] = Json::Value{}; jv[sfTransactions.jsonName][1U][jss::BatchTransaction] [jss::TransactionType] = jss::AccountSet; jv[sfTransactions.jsonName][1U][jss::BatchTransaction] [sfAccount.jsonName] = alice.human(); - jv[sfTransactions.jsonName][1U][jss::BatchTransaction] - [sfDestination.jsonName] = carol.human(); jv[sfTransactions.jsonName][1U][jss::BatchTransaction][sfFee.jsonName] = - "1000000"; + to_string(feeDrops); jv[sfTransactions.jsonName][1U][jss::BatchTransaction][jss::Sequence] = seq + 2; jv[sfTransactions.jsonName][1U][jss::BatchTransaction] [jss::SigningPubKey] = strHex(alice.pk()); - env(jv, fee(XRP(3)), ter(tesSUCCESS)); - jv[sfTransactions.jsonName][0U][jss::BatchTransaction] - [jss::TransactionType] = jss::AccountSet; - jv[sfTransactions.jsonName][0U][jss::BatchTransaction] - [sfAccount.jsonName] = alice.human(); - jv[sfTransactions.jsonName][0U][jss::BatchTransaction] - [sfDestination.jsonName] = bob.human(); - jv[sfTransactions.jsonName][0U][jss::BatchTransaction][sfFee.jsonName] = - "10"; - jv[sfTransactions.jsonName][0U][jss::BatchTransaction][jss::Sequence] = - seq; - jv[sfTransactions.jsonName][0U][jss::BatchTransaction] - [jss::SigningPubKey] = strHex(alice.pk()); - jv[sfTransactions.jsonName][1U] = Json::Value{}; - jv[sfTransactions.jsonName][1U][jss::BatchTransaction] = Json::Value{}; - jv[sfTransactions.jsonName][1U][jss::BatchTransaction] - [jss::TransactionType] = jss::Payment; - jv[sfTransactions.jsonName][1U][jss::BatchTransaction] - [sfAccount.jsonName] = alice.human(); - jv[sfTransactions.jsonName][1U][jss::BatchTransaction] - [sfDestination.jsonName] = carol.human(); - jv[sfTransactions.jsonName][1U][jss::BatchTransaction] - [sfAmount.jsonName] = "1000000"; - jv[sfTransactions.jsonName][1U][jss::BatchTransaction][sfFee.jsonName] = - "10"; - jv[sfTransactions.jsonName][1U][jss::BatchTransaction][jss::Sequence] = - seq; - jv[sfTransactions.jsonName][1U][jss::BatchTransaction] - [jss::SigningPubKey] = strHex(alice.pk()); - env(jv, fee(drops(10)), ter(tesSUCCESS)); + + env(jv, fee(drops((2 * feeDrops) + (2 * feeDrops))), ter(tesSUCCESS)); env.close(); }