diff --git a/src/ripple/app/hook/applyHook.h b/src/ripple/app/hook/applyHook.h index c81514fe4..ced7eae6f 100644 --- a/src/ripple/app/hook/applyHook.h +++ b/src/ripple/app/hook/applyHook.h @@ -22,6 +22,8 @@ struct HookContext; struct HookResult; bool isEmittedTxn(ripple::STTx const& tx); +bool +isBatchTxn(ripple::STTx const& tx); // This map type acts as both a read and write cache for hook execution // and is preserved across the execution of the set of hook chains diff --git a/src/ripple/app/hook/impl/applyHook.cpp b/src/ripple/app/hook/impl/applyHook.cpp index bf5c9f160..b0aa621f2 100644 --- a/src/ripple/app/hook/impl/applyHook.cpp +++ b/src/ripple/app/hook/impl/applyHook.cpp @@ -896,6 +896,12 @@ hook::isEmittedTxn(ripple::STTx const& tx) return tx.isFieldPresent(ripple::sfEmitDetails); } +bool +hook::isBatchTxn(ripple::STTx const& tx) +{ + return tx.isFieldPresent(ripple::sfBatchIndex); +} + int64_t hook::computeExecutionFee(uint64_t instructionCount) { diff --git a/src/ripple/app/ledger/impl/OpenLedger.cpp b/src/ripple/app/ledger/impl/OpenLedger.cpp index e13d80438..ec18781bc 100644 --- a/src/ripple/app/ledger/impl/OpenLedger.cpp +++ b/src/ripple/app/ledger/impl/OpenLedger.cpp @@ -135,7 +135,7 @@ OpenLedger::accept( if (tx->isFieldPresent(sfEmitDetails)) continue; - // skip emitted txns + // skip batch txns if (tx->isFieldPresent(sfBatchIndex)) continue; diff --git a/src/ripple/app/misc/NetworkOPs.cpp b/src/ripple/app/misc/NetworkOPs.cpp index c04ca05bc..f1ecdf0f1 100644 --- a/src/ripple/app/misc/NetworkOPs.cpp +++ b/src/ripple/app/misc/NetworkOPs.cpp @@ -1218,6 +1218,21 @@ 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; + } + auto const newFlags = app_.getHashRouter().getFlags(transaction->getID()); if ((newFlags & SF_BAD) != 0) @@ -1280,6 +1295,18 @@ 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; + } + if (transaction->getApplying()) return; @@ -1318,6 +1345,15 @@ NetworkOPsImp::doTransactionSync( // 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 BatchIndex, discarding."; + return; + } if (!transaction->getApplying()) { @@ -1524,8 +1560,10 @@ NetworkOPsImp::apply(std::unique_lock& batchLock) bool const isEmitted = hook::isEmittedTxn(*(e.transaction->getSTransaction())); + bool const isBatch = + hook::isBatchTxn(*(e.transaction->getSTransaction())); - if (toSkip && !isEmitted) + if (toSkip && !isEmitted || !isBatch) { protocol::TMTransaction tx; Serializer s; @@ -2754,6 +2792,10 @@ NetworkOPsImp::pubProposedTransaction( if (hook::isEmittedTxn(*transaction)) return; + // never publish emitted txns + if (hook::isBatchTxn(*transaction)) + return; + Json::Value jvObj = transJson(*transaction, result, false, ledger); { diff --git a/src/ripple/app/tx/impl/ApplyContext.h b/src/ripple/app/tx/impl/ApplyContext.h index eed91eb50..2a81ab0ed 100644 --- a/src/ripple/app/tx/impl/ApplyContext.h +++ b/src/ripple/app/tx/impl/ApplyContext.h @@ -123,6 +123,12 @@ public: return tx.isFieldPresent(sfEmitDetails); } + bool + isBatchTxn() + { + return tx.isFieldPresent(sfBatchIndex); + } + ApplyFlags const& flags() { diff --git a/src/ripple/app/tx/impl/Batch.cpp b/src/ripple/app/tx/impl/Batch.cpp index ad8410227..a59e75182 100644 --- a/src/ripple/app/tx/impl/Batch.cpp +++ b/src/ripple/app/tx/impl/Batch.cpp @@ -480,7 +480,7 @@ Batch::doApply() meta.setFieldU8(sfTransactionResult, TERtoInt(_result.first)); meta.setFieldU16(sfTransactionType, stx.getTxnType()); - if(_result.first != tesSUCCESS) + if(_result.first == tesSUCCESS) meta.setFieldH256(sfTransactionHash, stx.getTransactionID()); avi.addBatchExecutionMetaData(std::move(meta)); @@ -525,7 +525,7 @@ Batch::doApply() // std::cout << "ACCOUNT BALANCE: " << sleSrcAcc->getFieldAmount(sfBalance) << "\n"; auto const feePaid = ctx_.tx[sfFee].xrp(); - sleSrcAcc->setFieldU32(sfSequence, sleBase->getFieldU32(sfSequence)); + // sleSrcAcc->setFieldU32(sfSequence, ctx_.tx.getFieldU32(sfSequence) + 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 a66284dc4..e46108f40 100644 --- a/src/ripple/app/tx/impl/Transactor.cpp +++ b/src/ripple/app/tx/impl/Transactor.cpp @@ -734,6 +734,10 @@ 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()) diff --git a/src/ripple/protocol/impl/STTx.cpp b/src/ripple/protocol/impl/STTx.cpp index 8429278e4..64f07b69a 100644 --- a/src/ripple/protocol/impl/STTx.cpp +++ b/src/ripple/protocol/impl/STTx.cpp @@ -186,11 +186,18 @@ 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); + std::uint32_t const batchIndex{getFieldU32(sfBatchIndex)}; + if (batchIndex != 0) + return SeqProxy::sequence(batchIndex); 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 0153170fd..0b4087847 100644 --- a/src/test/app/Batch_test.cpp +++ b/src/test/app/Batch_test.cpp @@ -89,7 +89,7 @@ class Batch_test : public beast::unit_test::suite 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] = next; + jv[sfRawTransactions.jsonName][index][jss::RawTransaction][jss::Sequence] = 0; jv[sfRawTransactions.jsonName][index][jss::RawTransaction][sfBatchIndex.jsonName] = next; return jv; }