From 86fbddaa2e6f15a7e874c662117e1749cb2b2fa9 Mon Sep 17 00:00:00 2001 From: Denis Angell Date: Thu, 25 Apr 2024 09:34:00 +0200 Subject: [PATCH] update fields --- src/ripple/app/hook/impl/applyHook.cpp | 2 +- src/ripple/app/ledger/impl/OpenLedger.cpp | 6 +- src/ripple/app/misc/NetworkOPs.cpp | 73 ++++++++++++----------- src/ripple/app/tx/impl/ApplyContext.h | 2 +- src/ripple/app/tx/impl/Batch.cpp | 29 ++++++--- src/ripple/app/tx/impl/Transactor.cpp | 20 +++++-- src/ripple/app/tx/impl/apply.cpp | 3 + src/ripple/app/tx/impl/applySteps.cpp | 8 +++ src/ripple/ledger/impl/ApplyViewImpl.cpp | 2 +- src/ripple/protocol/impl/STTx.cpp | 15 +++-- src/test/app/Batch_test.cpp | 15 +++-- 11 files changed, 106 insertions(+), 69 deletions(-) diff --git a/src/ripple/app/hook/impl/applyHook.cpp b/src/ripple/app/hook/impl/applyHook.cpp index b0aa621f2..72ed638fc 100644 --- a/src/ripple/app/hook/impl/applyHook.cpp +++ b/src/ripple/app/hook/impl/applyHook.cpp @@ -899,7 +899,7 @@ hook::isEmittedTxn(ripple::STTx const& tx) bool hook::isBatchTxn(ripple::STTx const& tx) { - return tx.isFieldPresent(ripple::sfBatchIndex); + return tx.isFieldPresent(ripple::sfBatchTxn); } int64_t diff --git a/src/ripple/app/ledger/impl/OpenLedger.cpp b/src/ripple/app/ledger/impl/OpenLedger.cpp index ec18781bc..0024e31c6 100644 --- a/src/ripple/app/ledger/impl/OpenLedger.cpp +++ b/src/ripple/app/ledger/impl/OpenLedger.cpp @@ -135,9 +135,9 @@ OpenLedger::accept( if (tx->isFieldPresent(sfEmitDetails)) continue; - // skip batch txns - if (tx->isFieldPresent(sfBatchIndex)) - continue; + // // skip batch txns + // if (tx->isFieldPresent(sfBatchTxn)) + // continue; if (auto const toSkip = app.getHashRouter().shouldRelay(txId)) { diff --git a/src/ripple/app/misc/NetworkOPs.cpp b/src/ripple/app/misc/NetworkOPs.cpp index f1ecdf0f1..5c67b663d 100644 --- a/src/ripple/app/misc/NetworkOPs.cpp +++ b/src/ripple/app/misc/NetworkOPs.cpp @@ -1218,20 +1218,20 @@ NetworkOPsImp::processTransaction( return; } - // This function is called by several different parts of the codebase - // under no circumstances will we ever accept an emitted txn from the - // network. Emitted txns are *always* and *only* inserted by TxQ::accept, - // and only arise from processing ltEMITTED_TXN out of the EMITTED_DIR. This - // isn't always an error because a fetch pack etc might include an emitted - // txn and if this is a deliberate attempt to send an emitted txn over the - // network it was already billed in PeerImp - if (view->rules().enabled(featureBatch) && - hook::isBatchTxn(*transaction->getSTransaction())) - { - // RH NOTE: cannot set SF_BAD because if the tx will be generated by a - // hook we are about to execute - return; - } + // // This function is called by several different parts of the codebase + // // under no circumstances will we ever accept an emitted txn from the + // // network. Emitted txns are *always* and *only* inserted by TxQ::accept, + // // and only arise from processing ltEMITTED_TXN out of the EMITTED_DIR. This + // // isn't always an error because a fetch pack etc might include an emitted + // // txn and if this is a deliberate attempt to send an emitted txn over the + // // network it was already billed in PeerImp + // if (view->rules().enabled(featureBatch) && + // hook::isBatchTxn(*transaction->getSTransaction())) + // { + // // RH NOTE: cannot set SF_BAD because if the tx will be generated by a + // // hook we are about to execute + // return; + // } auto const newFlags = app_.getHashRouter().getFlags(transaction->getID()); @@ -1295,17 +1295,17 @@ NetworkOPsImp::doTransactionAsync( return; } - // Enforce Network bar for batch txn - if (view->rules().enabled(featureBatch) && - hook::isBatchTxn(*transaction->getSTransaction())) - { - JLOG(m_journal.info()) - << "Transaction received over network has BatchIndex, discarding."; - // RH NOTE: cannot set SF_BAD because if the tx will be generated by a - // hook we are about to execute then this would poison consensus for - // that emitted tx - return; - } + // // Enforce Network bar for batch txn + // if (view->rules().enabled(featureBatch) && + // hook::isBatchTxn(*transaction->getSTransaction())) + // { + // JLOG(m_journal.info()) + // << "Transaction received over network has BatchTxn, discarding."; + // // RH NOTE: cannot set SF_BAD because if the tx will be generated by a + // // hook we are about to execute then this would poison consensus for + // // that emitted tx + // return; + // } if (transaction->getApplying()) return; @@ -1346,14 +1346,14 @@ NetworkOPsImp::doTransactionSync( return; } - // Enforce Network bar for batch txn - if (view->rules().enabled(featureBatch) && - hook::isBatchTxn(*transaction->getSTransaction())) - { - JLOG(m_journal.info()) - << "Transaction received over network has BatchIndex, discarding."; - return; - } + // // Enforce Network bar for batch txn + // if (view->rules().enabled(featureBatch) && + // hook::isBatchTxn(*transaction->getSTransaction())) + // { + // JLOG(m_journal.info()) + // << "Transaction received over network has BatchTxn, discarding."; + // return; + // } if (!transaction->getApplying()) { @@ -1560,10 +1560,11 @@ NetworkOPsImp::apply(std::unique_lock& batchLock) bool const isEmitted = hook::isEmittedTxn(*(e.transaction->getSTransaction())); - bool const isBatch = - hook::isBatchTxn(*(e.transaction->getSTransaction())); + // bool const isBatch = + // hook::isBatchTxn(*(e.transaction->getSTransaction())); - if (toSkip && !isEmitted || !isBatch) + // if (toSkip && !isEmitted || !isBatch) + if (toSkip && !isEmitted) { protocol::TMTransaction tx; Serializer s; diff --git a/src/ripple/app/tx/impl/ApplyContext.h b/src/ripple/app/tx/impl/ApplyContext.h index 2a81ab0ed..6aad83b25 100644 --- a/src/ripple/app/tx/impl/ApplyContext.h +++ b/src/ripple/app/tx/impl/ApplyContext.h @@ -126,7 +126,7 @@ public: bool isBatchTxn() { - return tx.isFieldPresent(sfBatchIndex); + return tx.isFieldPresent(sfBatchTxn); } ApplyFlags const& diff --git a/src/ripple/app/tx/impl/Batch.cpp b/src/ripple/app/tx/impl/Batch.cpp index 75bee0d49..ff9f411eb 100644 --- a/src/ripple/app/tx/impl/Batch.cpp +++ b/src/ripple/app/tx/impl/Batch.cpp @@ -217,20 +217,22 @@ invoke_preclaim(PreclaimContext const& ctx) if (id != beast::zero) { - TER result = T::checkSeqProxy(ctx.view, ctx.tx, ctx.j); + // TER result = T::checkSeqProxy(ctx.view, ctx.tx, ctx.j); - if (result != tesSUCCESS) - return result; + // if (result != tesSUCCESS) + // return result; + + TER result = tesSUCCESS; result = T::checkPriorTxAndLastLedger(ctx); if (result != tesSUCCESS) return result; - result = T::checkFee(ctx, calculateBaseFee(ctx.view, ctx.tx)); + // result = T::checkFee(ctx, calculateBaseFee(ctx.view, ctx.tx)); - if (result != tesSUCCESS) - return result; + // if (result != tesSUCCESS) + // return result; result = T::checkSign(ctx); @@ -364,6 +366,15 @@ Batch::preclaim(PreclaimContext const& ctx) preclaimResponses.push_back(response); } + for (auto const& response : preclaimResponses) + { + if (response != tesSUCCESS) + { + return response; + } + } + + return tesSUCCESS; } @@ -477,8 +488,7 @@ Batch::doApply() meta.setFieldU8(sfTransactionResult, TERtoInt(_result.first)); meta.setFieldU16(sfTransactionType, stx.getTxnType()); - if(_result.first == tesSUCCESS) - meta.setFieldH256(sfTransactionHash, stx.getTransactionID()); + meta.setFieldH256(sfTransactionHash, stx.getTransactionID()); avi.addBatchExecutionMetaData(std::move(meta)); @@ -524,7 +534,8 @@ Batch::doApply() // std::cout << "ACCOUNT BALANCE: " << sleSrcAcc->getFieldAmount(sfBalance) << "\n"; auto const feePaid = ctx_.tx[sfFee].xrp(); - // sleSrcAcc->setFieldU32(sfSequence, ctx_.tx.getFieldU32(sfSequence) + 1); + // auto const& txns = ctx_.tx.getFieldArray(sfRawTransactions); + sleSrcAcc->setFieldU32(sfSequence, ctx_.tx.getFieldU32(sfSequence) + txns.size() + 1); sleSrcAcc->setFieldAmount(sfBalance, sleBase->getFieldAmount(sfBalance).xrp() - feePaid); sb.update(sleSrcAcc); sb.apply(ctx_.rawView()); diff --git a/src/ripple/app/tx/impl/Transactor.cpp b/src/ripple/app/tx/impl/Transactor.cpp index e46108f40..36e4eb8c9 100644 --- a/src/ripple/app/tx/impl/Transactor.cpp +++ b/src/ripple/app/tx/impl/Transactor.cpp @@ -596,6 +596,12 @@ Transactor::checkSeqProxy( return tesSUCCESS; } + // // // pass all emitted tx provided their seq is 0 + // if (view.rules().enabled(featureBatch) && hook::isBatchTxn(tx)) + // { + // return tesSUCCESS; + // } + // reserved for emitted tx only at this time if (tx.isFieldPresent(sfFirstLedgerSequence)) return tefINTERNAL; @@ -734,14 +740,14 @@ Transactor::consumeSeqProxy(SLE::pointer const& sleAccount) // do not update sequence of sfAccountTxnID for emitted tx if (ctx_.isEmittedTxn()) return tesSUCCESS; - - // do not update sequence of sfAccountTxnID for emitted tx - if (ctx_.isBatchTxn()) - return tesSUCCESS; SeqProxy const seqProx = ctx_.tx.getSeqProxy(); if (seqProx.isSeq()) { + // do not update sequence of sfAccountTxnID for batch tx + if (ctx_.isBatchTxn()) + return tesSUCCESS; + // Note that if this transaction is a TicketCreate, then // the transaction will modify the account root sfSequence // yet again. @@ -1762,6 +1768,12 @@ Transactor::operator()() } #endif + // if (ctx_.isBatchTxn()) + // { + // JLOG(j_.trace()) << "BAD BATCH: " << ctx_.tx.getTransactionID(); + // return {tecINTERNAL, false}; + // } + // Enforce an absolute bar to applying emitted transactions which are either // explicitly in preflight test mode, or somehow managed to make their way // here despite not being emitted here by a hook here. diff --git a/src/ripple/app/tx/impl/apply.cpp b/src/ripple/app/tx/impl/apply.cpp index 0c7d13616..e01459a6d 100644 --- a/src/ripple/app/tx/impl/apply.cpp +++ b/src/ripple/app/tx/impl/apply.cpp @@ -161,6 +161,9 @@ apply( STAmountSO stAmountSO{view.rules().enabled(fixSTAmountCanonicalize)}; NumberSO stNumberSO{view.rules().enabled(fixUniversalNumber)}; + if (tx.isFieldPresent(sfBatchTxn)) + return {tesSUCCESS, false}; + auto pfresult = preflight(app, view.rules(), tx, flags, j); auto pcresult = preclaim(pfresult, app, view); return doApply(pcresult, app, view); diff --git a/src/ripple/app/tx/impl/applySteps.cpp b/src/ripple/app/tx/impl/applySteps.cpp index c40d03f13..d97838615 100644 --- a/src/ripple/app/tx/impl/applySteps.cpp +++ b/src/ripple/app/tx/impl/applySteps.cpp @@ -193,6 +193,11 @@ invoke_preclaim(PreclaimContext const& ctx) // list one, preflight will have already a flagged a failure. auto const id = ctx.tx.getAccountID(sfAccount); + std::cout << "invoke_preclaim: " << ctx.tx.getTxnType() << "\n"; + + // if (ctx.tx.isFieldPresent(sfBatchTxn)) + // return tesSUCCESS; + if (id != beast::zero) { TER result = T::checkSeqProxy(ctx.view, ctx.tx, ctx.j); @@ -682,6 +687,9 @@ doApply(PreclaimResult const& preclaimResult, Application& app, OpenView& view) if (!preclaimResult.likelyToClaimFee) return {preclaimResult.ter, false}; + // if (preclaimResult.tx.isFieldPresent(sfBatchTxn)) + // return tesSUCCESS; + ApplyContext ctx( app, view, diff --git a/src/ripple/ledger/impl/ApplyViewImpl.cpp b/src/ripple/ledger/impl/ApplyViewImpl.cpp index 8811b003b..5ce82d53e 100644 --- a/src/ripple/ledger/impl/ApplyViewImpl.cpp +++ b/src/ripple/ledger/impl/ApplyViewImpl.cpp @@ -31,7 +31,7 @@ ApplyViewImpl::ApplyViewImpl(ReadView const* base, ApplyFlags flags) void ApplyViewImpl::apply(OpenView& to, STTx const& tx, TER ter, beast::Journal j) { - std::cout << "ApplyViewImpl::apply" << "\n"; + std::cout << "ApplyViewImpl::apply " << "\n"; items_.apply(to, tx, ter, deliver_, batchExecution_, hookExecution_, hookEmission_, j); } diff --git a/src/ripple/protocol/impl/STTx.cpp b/src/ripple/protocol/impl/STTx.cpp index 64f07b69a..fea4cbd67 100644 --- a/src/ripple/protocol/impl/STTx.cpp +++ b/src/ripple/protocol/impl/STTx.cpp @@ -187,17 +187,16 @@ STTx::getSeqProxy() const std::uint32_t const seq{getFieldU32(sfSequence)}; if (seq != 0) { - // if (getTxnType() == ttBATCH) - // { - // auto const& txns = ctx_.tx.getFieldArray(sfRawTransactions); - // return SeqProxy::sequence(seq + txns.size()); - // } return SeqProxy::sequence(seq); } - std::uint32_t const batchIndex{getFieldU32(sfBatchIndex)}; - if (batchIndex != 0) - return SeqProxy::sequence(batchIndex); + if (isFieldPresent(sfBatchTxn)) + { + STObject const batchTxn = const_cast(*this).getField(sfBatchTxn).downcast(); + std::uint32_t const startSequence{batchTxn.getFieldU32(sfOuterSequence)}; + std::uint32_t const batchIndex{batchTxn.getFieldU8(sfBatchIndex)}; + return SeqProxy::sequence(startSequence + batchIndex + 1); + } std::optional const ticketSeq{operator[](~sfTicketSequence)}; if (!ticketSeq) diff --git a/src/test/app/Batch_test.cpp b/src/test/app/Batch_test.cpp index f92da8ee7..dc19e01dc 100644 --- a/src/test/app/Batch_test.cpp +++ b/src/test/app/Batch_test.cpp @@ -82,15 +82,18 @@ class Batch_test : public beast::unit_test::suite Json::Value const& tx, jtx::Account const& account, XRPAmount feeDrops, - std::uint32_t index, - std::uint32_t next) + std::uint8_t index, + std::uint32_t outerSequence) { jv[sfRawTransactions.jsonName][index] = Json::Value{}; jv[sfRawTransactions.jsonName][index][jss::RawTransaction] = tx; jv[sfRawTransactions.jsonName][index][jss::RawTransaction][jss::SigningPubKey] = strHex(account.pk()); jv[sfRawTransactions.jsonName][index][jss::RawTransaction][sfFee.jsonName] = 0; jv[sfRawTransactions.jsonName][index][jss::RawTransaction][jss::Sequence] = 0; - jv[sfRawTransactions.jsonName][index][jss::RawTransaction][sfBatchIndex.jsonName] = next; + jv[sfRawTransactions.jsonName][index][jss::RawTransaction][sfBatchTxn.jsonName] = Json::Value{}; + jv[sfRawTransactions.jsonName][index][jss::RawTransaction][sfBatchTxn.jsonName][jss::Account] = account.human(); + jv[sfRawTransactions.jsonName][index][jss::RawTransaction][sfBatchTxn.jsonName][sfOuterSequence.jsonName] = outerSequence; + jv[sfRawTransactions.jsonName][index][jss::RawTransaction][sfBatchTxn.jsonName][sfBatchIndex.jsonName] = index; return jv; } @@ -254,11 +257,11 @@ class Batch_test : public beast::unit_test::suite // Tx 1 Json::Value const tx1 = pay(alice, bob, XRP(1)); - jv = addBatchTx(jv, tx1, alice, feeDrops, 0, seq + 1); + jv = addBatchTx(jv, tx1, alice, feeDrops, 0, seq); // Tx 2 Json::Value const tx2 = pay(alice, bob, XRP(1)); - jv = addBatchTx(jv, tx2, alice, feeDrops, 1, seq + 2); + jv = addBatchTx(jv, tx2, alice, feeDrops, 1, seq); env(jv, fee(feeDrops * 2), ter(tesSUCCESS)); env.close(); @@ -286,7 +289,7 @@ class Batch_test : public beast::unit_test::suite std::cout << "alice: " << env.balance(alice) << "\n"; std::cout << "bob: " << env.balance(bob) << "\n"; - BEAST_EXPECT(env.seq(alice) == 2); + BEAST_EXPECT(env.seq(alice) == 4); BEAST_EXPECT(env.balance(alice) == XRP(1000) - XRP(2) - (feeDrops * 2)); BEAST_EXPECT(env.balance(bob) == XRP(1000) + XRP(2)); }