Repository: bitcoin/bitcoin
File: src/validation.cpp (C++)

Code before:
        m_replaced_transactions.push_back(it->GetSharedTx());
    }
    m_pool.RemoveStaged(m_all_conflicts, false, MemPoolRemovalReason::REPLACED);
    m_all_conflicts.clear();
    // Store transaction in memory
    m_pool.addUnchecked(*entry, ws.m_ancestors);

    // trim mempool and check if tx was trimmed
    // If we are validating a package, don't trim here because we could evict a previous transaction
    // in the package. LimitMempoolSize() should be called at the very end to make sure the mempool
    // is still within limits and package submission happens atomically.
    if (!args.m_package_submission && !bypass_limits) {
        LimitMempoolSize(m_pool, m_active_chainstate.CoinsTip());
        if (!m_pool.exists(GenTxid::Txid(hash)))
            // The tx no longer meets our (new) mempool minimum feerate but could be reconsidered in a package.
            return state.Invalid(TxValidationResult::TX_RECONSIDERABLE, "mempool full");
    }
    return true;
}

bool MemPoolAccept::SubmitPackage(const ATMPArgs& args, std::vector<Workspace>& workspaces,
                                  PackageValidationState& package_state,
                                  std::map<uint256, MempoolAcceptResult>& results)
{
    AssertLockHeld(cs_main);
    AssertLockHeld(m_pool.cs);
    // Sanity check: none of the transactions should be in the mempool, and none of the transactions
    // should have a same-txid-different-witness equivalent in the mempool.
    assert(std::all_of(workspaces.cbegin(), workspaces.cend(), [this](const auto& ws){
        return !m_pool.exists(GenTxid::Txid(ws.m_ptx->GetHash())); }));

    bool all_submitted = true;
    // ConsensusScriptChecks adds to the script cache and is therefore consensus-critical;
    // CheckInputsFromMempoolAndCache asserts that transactions only spend coins available from the
    // mempool or UTXO set. Submit each transaction to the mempool immediately after calling
    // ConsensusScriptChecks to make the outputs available for subsequent transactions.
    for (Workspace& ws : workspaces) {
        if (!ConsensusScriptChecks(args, ws)) {
            results.emplace(ws.m_ptx->GetWitnessHash(), MempoolAcceptResult::Failure(ws.m_state));
            // Since PolicyScriptChecks() passed, this should never fail.
            Assume(false);
            all_submitted = false;
            package_state.Invalid(PackageValidationResult::PCKG_MEMPOOL_ERROR,
                                  strprintf("BUG! PolicyScriptChecks succeeded but ConsensusScriptChecks failed: %s",
                                            ws.m_ptx->GetHash().ToString()));
        }

        // Re-calculate mempool ancestors to call addUnchecked(). They may have changed since the
        // last calculation done in PreChecks, since package ancestors have already been submitted.
        {
            auto ancestors{m_pool.CalculateMemPoolAncestors(*ws.m_entry, m_pool.m_limits)};

Review comment (suggestion):
Worth commenting that even though clearing happens in `ClearSubPackageState`, this is cleared so that we don't process the vector over and over again for each tx in the package.

fwiw here's a test for it, doing an RBF (but not package RBF) within package eval: https://github.com/glozow/bitcoin/blob/2024-05-review-30072/src/test/txpackage_tests.cpp#L940-L996

Code after:
    {
        LogPrint(BCLog::MEMPOOL, "replacing tx %s (wtxid=%s) with %s (wtxid=%s) for %s additional fees, %d delta bytes\n",
                it->GetTx().GetHash().ToString(),
                it->GetTx().GetWitnessHash().ToString(),
                hash.ToString(),
                tx.GetWitnessHash().ToString(),
                FormatMoney(ws.m_modified_fees - m_subpackage.m_conflicting_fees),
                (int)entry->GetTxSize() - (int)m_subpackage.m_conflicting_size);
        TRACE7(mempool, replaced,
                it->GetTx().GetHash().data(),
                it->GetTxSize(),
                it->GetFee(),
                std::chrono::duration_cast<std::chrono::duration<std::uint64_t>>(it->GetTime()).count(),
                hash.data(),
                entry->GetTxSize(),
                entry->GetFee()
        );
        m_subpackage.m_replaced_transactions.push_back(it->GetSharedTx());
    }
    m_pool.RemoveStaged(m_subpackage.m_all_conflicts, false, MemPoolRemovalReason::REPLACED);
    // Don't attempt to process the same conflicts repeatedly during subpackage evaluation:
    // they no longer exist on subsequent calls to Finalize() post-RemoveStaged
    m_subpackage.m_all_conflicts.clear();
    // Store transaction in memory
    m_pool.addUnchecked(*entry, ws.m_ancestors);

    // trim mempool and check if tx was trimmed
    // If we are validating a package, don't trim here because we could evict a previous transaction
    // in the package. LimitMempoolSize() should be called at the very end to make sure the mempool
    // is still within limits and package submission happens atomically.
    if (!args.m_package_submission && !bypass_limits) {
        LimitMempoolSize(m_pool, m_active_chainstate.CoinsTip());
        if (!m_pool.exists(GenTxid::Txid(hash)))
            // The tx no longer meets our (new) mempool minimum feerate but could be reconsidered in a package.
            return state.Invalid(TxValidationResult::TX_RECONSIDERABLE, "mempool full");
    }
    return true;
}

bool MemPoolAccept::SubmitPackage(const ATMPArgs& args, std::vector<Workspace>& workspaces,
                                  PackageValidationState& package_state,
                                  std::map<uint256, MempoolAcceptResult>& results)
{
    AssertLockHeld(cs_main);
    AssertLockHeld(m_pool.cs);
    // Sanity check: none of the transactions should be in the mempool, and none of the transactions
    // should have a same-txid-different-witness equivalent in the mempool.
    assert(std::all_of(workspaces.cbegin(), workspaces.cend(), [this](const auto& ws){
        return !m_pool.exists(GenTxid::Txid(ws.m_ptx->GetHash())); }));

    bool all_submitted = true;