fuzz: Fix assert bug in txorphan target #25624

Fix https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=48914.

It is possible to construct big tx that got rejected in AddTx, so we cannot assume tx will be added successfully. We can only guarantee tx will not be added if orphanage already has it.

include-what-you-use told me to add these lines, is it correct?

❯ iwyu-tool src/test/fuzz/txorphan.cpp -p compile_commands.json

test/fuzz/txorphan.cpp should add these lines:
#include "consensus/validation.h"          // for GetTransactionWeight
#include "node/eviction.h"                 // for NodeId
#include "policy/policy.h"                 // for MAX_STANDARD_TX_WEIGHT
#include "tinyformat.h"                    // for format_error

test/fuzz/txorphan.cpp should remove these lines:
- #include <net.h>  // lines 6-6

The full include-list for test/fuzz/txorphan.cpp:
#include <consensus/amount.h>              // for CAmount
#include <net_processing.h>                // for DEFAULT_MAX_ORPHAN_TRANSAC...
#include <primitives/transaction.h>        // for COutPoint, CTxIn, CTxOut
#include <script/script.h>                 // for CScript
#include <sync.h>                          // for LOCK, WITH_LOCK
#include <test/fuzz/FuzzedDataProvider.h>  // for FuzzedDataProvider
#include <test/fuzz/fuzz.h>                // for FuzzBufferType, LIMITED_WHILE
#include <test/fuzz/util.h>                // for ConsumeTime, CallOneOf
#include <test/util/setup_common.h>        // for MakeNoLogFileContext
#include <txorphanage.h>                   // for TxOrphanage, g_cs_orphans
#include <uint256.h>                       // for uint256
#include <util/check.h>                    // for inline_assertion_check
#include <util/time.h>                     // for SetMockTime
#include <algorithm>                       // for max
#include <cstdint>                         // for uint32_t, uint8_t
#include <memory>                          // for __shared_ptr_access, opera...
#include <set>                             // for set
#include <utility>                         // for swap, pair
#include <vector>                          // for vector
#include "consensus/validation.h"          // for GetTransactionWeight
#include "node/eviction.h"                 // for NodeId
#include "policy/policy.h"                 // for MAX_STANDARD_TX_WEIGHT
#include "tinyformat.h"                    // for format_error

Concept ACK

• The logic of code changes seems correct, and I agree with the reasoning that:

We can only guarantee tx will not be added if the orphanage already has it.

Though I wonder why this following assumption was initially made?

tx should already be added since it will not be too big in the test

• I looked through the file to confirm that the instances that should be added according to the iwyu-tool, and I was able to confirm there addition except for tinyformat.h, as I could not find any use of of format_error in this file.
• I have to further check whether the removal of net.h is correct or not.
• It would help if you could share the link to the iwyu-tool you are currently using to do the testing.

@shaavan I made the wrong assumption that the tx will not be too big since I expect each tx will only have a few inputs/outputs. Before the PR got merged, I ran the fuzz target locally for 1 day and there was no crash. But OSS-Fuzz can produce a test case that can construct overweight tx with only a few bytes.

For iwyu-tool, I installed it from AUR, which should be the same as the latest upstream. I generated compile_commands.json using bear.

iwyu is run by the CI (https://cirrus-ci.com/task/5279630734131200?logs=ci#L7283):

test/fuzz/txorphan.cpp should add these lines:
#include "consensus/validation.h"          // for GetTransactionWeight
#include "node/eviction.h"                 // for NodeId
#include "policy/policy.h"                 // for MAX_STANDARD_TX_WEIGHT
test/fuzz/txorphan.cpp should remove these lines:
- #include <net.h>  // lines 6-6

Which looks correct.

<!--e57a25ab6845829454e8d69fc972939a-->

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

<!--174a7506f384e20aa4161008e828411d-->

Conflicts

Reviewers, this pull request conflicts with the following ones:

• #25564 (Move DEFAULT_MAX_ORPHAN_TRANSACTIONS to node/txorphanage.h by MarcoFalke)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

lgtm ACK 2315830491b2cfa6b6e3e277700238e5ac92a8e0