💬 ismaelsadeeq commented on pull request "Detect and ignore transactions that were CPFP'd in the fee estimator":
(https://github.com/bitcoin/bitcoin/pull/25380#discussion_r1252956237)
Thank you
If the ancestors are confirmed in the previous block, and the child (with fee rate higher than ancestors score) is in the block we are processing, is that also considered CPFP? if yes then we are not removing those ancestors here right?

Also why is `/* inBlock = */true` ?

💬 MarcoFalke commented on issue "Intermittent failures in interface_usdt_mempool.py":
(https://github.com/bitcoin/bitcoin/issues/27380#issuecomment-1621555962)
> neither tx nor result go out of scope before it's called

I don't think the lifetime of `result` is relevant here. `GetRejectReason()` gives birth to a fresh temporary copy of the string.

> All that said, it's obviously still going wrong somehow...

Yeah, I am just guessing that this is a lifetime issue. It would be good to rule this out.

👍 TheCharlatan approved a pull request: "test: bugfix, synchronize indexes synchronously"
(https://github.com/bitcoin/bitcoin/pull/28026#pullrequestreview-1514306173)
ACK f01cb3a8dcc12954d5b6e075ebeb94c00fd37779

💬 MarcoFalke commented on pull request "test: bugfix, synchronize indexes synchronously":
(https://github.com/bitcoin/bitcoin/pull/28026#issuecomment-1621578293)
(An alternative, functionally equivalent to this, might be to just make the timeout infinite, see https://github.com/bitcoin/bitcoin/pull/27988#issuecomment-1619218007, but I think this is fine as well.)

💬 MarcoFalke commented on pull request "test: Fix intermittent issue in mining_getblocktemplate_longpoll.py":
(https://github.com/bitcoin/bitcoin/pull/27941#issuecomment-1621604286)
In any case, if the Bitcoin Core behavior is changed, it should be accompanied with deterministic tests. I don't think non-deterministic tests are useful, especially if they lead to a failure every time they hit the "other side" of the race.

Going to reopen for now to fix the intermittent issue, and make the tests deterministic. Happy to review a future or conflicting pull that instead changes the non-test code, or if there is documentation I missed.

📝 MarcoFalke reopened a pull request: "test: Fix intermittent issue in mining_getblocktemplate_longpoll.py"
(https://github.com/bitcoin/bitcoin/pull/27941)
Fixes https://github.com/bitcoin/bitcoin/issues/26962

Wait for the thread to have started and the RPC to have reached the node before continuing. Otherwise the test may run into a race.

For example:

```
test 2023-06-23T13:10:29.245000Z TestFramework (INFO): Test that introducing a new transaction into the mempool will terminate the longpoll
node0 2023-06-23T13:10:29.245712Z [http] [httpserver.cpp:254] [http_request_cb] [http] Received a POST request for / from 127.0.0.1:43568
n
...

💬 Ayush170-Future commented on pull request "init: adding check for : for -torcontrol flag":
(https://github.com/bitcoin/bitcoin/pull/28018#issuecomment-1621635810)
A gentle ping to @luke-jr, what about this issue then? [23589](https://github.com/bitcoin/bitcoin/issues/23589)

💬 Xekyo commented on pull request "Detect and ignore transactions that were CPFP'd in the fee estimator":
(https://github.com/bitcoin/bitcoin/pull/25380#discussion_r1253156046)
CPFP refers to a couple of unconfirmed transactions where the descendant transaction pays a higher feerate than the parent transaction. If the parent transaction is already confirmed, it’s no longer a CPFP situation.

🤔 stickies-v reviewed a pull request: "fuzz: Generate rpc fuzz targets individually"
(https://github.com/bitcoin/bitcoin/pull/28015#pullrequestreview-1514639350)
From an empty corpus, was able to find this bug (on) in approx. 2 hours with `LIMIT_TO_RPC_COMMAND` (on my pretty old i5-6500T CPU @ 2.50GHz / 16GB RAM machine):

```
FUZZ=rpc FUZZ=rpc LIMIT_TO_RPC_COMMAND=analyzepsbt UBSAN_OPTIONS="suppressions=$(pwd)/test/sanitizer_suppressions/ubsan:print_stacktrace=1:halt_on_error=1:report_error_type=1" ./src/test/fuzz/fuzz ~/fuzz/rpc -fork=4
```

I'm letting it run again overnight without `LIMIT_TO_RPC_COMMAND` (from an empty corpus) to see if I can f
...

💬 stickies-v commented on pull request "fuzz: Generate rpc fuzz targets individually":
(https://github.com/bitcoin/bitcoin/pull/28015#discussion_r1253179575)
doc nit:
```suggestion
targets = [(t, {}) for t in targets] # expand to add dictionary for target-specific env variables
```

💬 stickies-v commented on pull request "fuzz: Generate rpc fuzz targets individually":
(https://github.com/bitcoin/bitcoin/pull/28015#discussion_r1253178781)
nit: Would add a small docstring here:

```suggestion

# Instead of a single rpc target, replace it with a a target for each rpc function"""
rpc_target = "rpc"
```

💬 MarcoFalke commented on pull request "fuzz: Generate rpc fuzz targets individually":
(https://github.com/bitcoin/bitcoin/pull/28015#discussion_r1253208733)
Thanks, there will be follow-ups either way, so I'll save the nit for later, if that is ok.

💬 MarcoFalke commented on pull request "fuzz: Generate rpc fuzz targets individually":
(https://github.com/bitcoin/bitcoin/pull/28015#discussion_r1253208865)
Thanks, there will be follow-ups either way, so I'll save the nit for later, if that is ok.

💬 MarcoFalke commented on pull request "fuzz: Generate rpc fuzz targets individually":
(https://github.com/bitcoin/bitcoin/pull/28015#issuecomment-1621899833)
> Fuzzing is still a bit too magical to me to comment on whether or not this approach is the best we can take.

For reference, within the suggested solutions (either compile into a separate binary for each fuzz target, or select via env var), all should be identical in the eyes of the fuzz engine. Obviously there are other improvements that can be done, but they are orthogonal to this pull.

💬 ismaelsadeeq commented on pull request "Detect and ignore transactions that were CPFP'd in the fee estimator":
(https://github.com/bitcoin/bitcoin/pull/25380#discussion_r1253237996)
Thank you @Xekyo

🤔 Xekyo reviewed a pull request: "fuzz: improve `coinselection`"
(https://github.com/bitcoin/bitcoin/pull/27585#pullrequestreview-1514739299)
reACK bd6315a2a75d2891a8d3476c492d6b53309b7296

💬 sipa commented on pull request "Add support for "partial" fuzzers that indicate usefulness":
(https://github.com/bitcoin/bitcoin/pull/27552#discussion_r1253248607)
I've added a FuzzResult enum as suggested by @MarcoFalke, and added some explanation there.

💬 sipa commented on pull request "Add support for "partial" fuzzers that indicate usefulness":
(https://github.com/bitcoin/bitcoin/pull/27552#discussion_r1253249225)
Done!

💬 sipa commented on pull request "Add support for "partial" fuzzers that indicate usefulness":
(https://github.com/bitcoin/bitcoin/pull/27552#issuecomment-1621949444)
Rebased and switched from `bool` to an `enum class FuzzResult` which has values `MAYBE_INTERESTING` and `UNINTERESTING`, making it hopefully clearer what the return values correspond to.

📝 glozow opened a pull request: "Package Relay 1/3: Introduce TxPackageTracker as Orphan Resolution Module"
(https://github.com/bitcoin/bitcoin/pull/28031)
Milestone 1 of package relay p2p changes. See #27463 for full project tracking.

Please see #27742 for how this PR fits into the big picture. I strongly suggest that reviewers look at that PR first to decide if they are comfortable with the overall approach.

This PR is mainly refactors, with a few behavior changes and improvements:
- Introduces `TxPackageTracker`, which starts out as a wrapper for the `TxOrphanage`. This will be the main "package relay" module as well.
- Adds a new log ca
...