👍 ryanofsky approved a pull request: "refactor: Replace g_genesis_wait_cv with m_tip_block_cv"
(https://github.com/bitcoin/bitcoin/pull/30967#pullrequestreview-2332196385)
Code review ACK fa24d8dbddebd61b9c418bd6773c333ad72bed67. Fixed wait_for overflow bug since last review

💬 ryanofsky commented on pull request "refactor: Replace g_genesis_wait_cv with m_tip_block_cv":
(https://github.com/bitcoin/bitcoin/pull/30967#discussion_r1777641007)
In commit "refactor: Add missing GUARDED_BY(m_tip_block_mutex)" (fa47d893acdd18132204810dd95880ab21abf415)

It seems like it would be a little better to make this change in the next commit "refactor: Use wait_for predicate to check for interrupt" (faa3f411f6842bf1201b25d6391b1913e1b72439) instead of this one so this code is not changing more than once.

💬 ryanofsky commented on pull request "refactor: Replace g_genesis_wait_cv with m_tip_block_cv":
(https://github.com/bitcoin/bitcoin/pull/30967#discussion_r1777642365)
In commit "refactor: Use wait_for predicate to check for interrupt" (faa3f411f6842bf1201b25d6391b1913e1b72439)

Might be better to avoid the 100 year hack by switching to wait_until

```diff
--- a/src/node/interfaces.cpp
+++ b/src/node/interfaces.cpp
@@ -940,10 +940,10 @@ public:

BlockRef waitTipChanged(uint256 current_tip, MillisecondsDouble timeout) override
{
- if (timeout > std::chrono::years{100}) timeout = std::chrono::years{100}; // Upper bound to avoid UB in
...

💬 ryanofsky commented on pull request "Add multiprocess binaries to release build":
(https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2377800457)
> For my part, I need to fix the windows build and also open an issue listing different ways it would be possible to make releases including multiprocess support in short or long term.

Update: currently working on getting mingw32 build working, and I created issue #30983 to lay out different multiprocess release options.

💬 maflcko commented on issue "Test p2p_headers_presync fuzz target on macOS/Windows":
(https://github.com/bitcoin/bitcoin/issues/30950#issuecomment-2377843249)
> Wouldn't a developer looking to fuzz test set `-DBUILD_FOR_FUZZING=ON`?

Most likely yes, but it isn't enforced. For example, it is possible to compile the fuzz binary without `DBUILD_FOR_FUZZING` and execute it. Support for that is also added to the `fuzz/test_runner.py` script. (This is also what the failing CIs were using to call the fuzz binary).

> Would future harnesses relying on FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION also do an early return?

I'd say it depends on the fuzz targ
...

💬 mzumsande commented on pull request "kernel: Move block tree db open to block manager":
(https://github.com/bitcoin/bitcoin/pull/30965#discussion_r1777703574)
This changes the error handling if the block tree db is corrupted a bit worse as a side effect. Before, we'd get the advice "Please restart with -reindex or -reindex-chainstate to recover." on the console, and, if using the GUI, the option to reindex with by clicking a button. Now we just get a "Error opening block database" error on both without any advice how to fix it.

I'm not sure if the GUI one-click reindex option is totally essential, but in my opinion we should at least keep the advic
...

🤔 mzumsande reviewed a pull request: "kernel: Move block tree db open to block manager"
(https://github.com/bitcoin/bitcoin/pull/30965#pullrequestreview-2332298896)
Concept ACK

💬 instagibbs commented on pull request "Disable RBF Rule 2":
(https://github.com/bitcoin/bitcoin/pull/30964#issuecomment-2377936088)
@sdaftuar I think the same reasoning I gave is just being applied recursively, which is fine :+1:

🤔 hodlinator reviewed a pull request: "rpc: add getorphantxs"
(https://github.com/bitcoin/bitcoin/pull/30793#pullrequestreview-2332231261)
Concept ACK

Feels like *p2p_orphan_handling.py*/`test_same_txid_orphan()` might benefit from some augmented checking using this new `getorphantxs`. It could verify that same-txids-differing-wtxid orphans are handled in the expected way, with the bogus one being removed after the one with the valid witness gets adopted.

💬 hodlinator commented on pull request "rpc: add getorphantxs":
(https://github.com/bitcoin/bitcoin/pull/30793#discussion_r1777666525)
My guess is that this weird indentation style is an artifact of refactorings not wanting to re-indent old lines. Should not apply to new code.

💬 hodlinator commented on pull request "rpc: add getorphantxs":
(https://github.com/bitcoin/bitcoin/pull/30793#discussion_r1777682322)
Feels like `TestNode` itself shouldn't have tx-level code. Maybe `/test/functional/test_framework/blocktools.py` would be slightly better? It has `create_witness_tx(node, ...)` and `send_to_witness(use_p2wsh, node, utxo, ...)`.

💬 hodlinator commented on pull request "rpc: add getorphantxs":
(https://github.com/bitcoin/bitcoin/pull/30793#discussion_r1777688749)
Could be applied to `getblock` in *src/rpc/blockchain.cpp* as well.

💬 hodlinator commented on pull request "rpc: add getorphantxs":
(https://github.com/bitcoin/bitcoin/pull/30793#discussion_r1777697752)
nit: The other 15 comments in the file do not occupy the same ratio of vertical space.
```suggestion
/** Allows providing orphan information externally */
```

💬 hodlinator commented on pull request "rpc: add getorphantxs":
(https://github.com/bitcoin/bitcoin/pull/30793#discussion_r1777729324)
nit: Might be nice to check the mempool size is zero as a post-condition, even though it should be covered by other tests.

🤔 jarolrod reviewed a pull request: "contrib: Update asmap link in seeds readme"
(https://github.com/bitcoin/bitcoin/pull/30979#pullrequestreview-2332366220)
ACK f158993fd5575d3cbf991b6df7bcd2af7cf96c9a

📝 brunoerg opened a pull request: "p2p: rpc: add `tx_reconciliation` to `getpeerinfo`"
(https://github.com/bitcoin/bitcoin/pull/30984)
This PR adds a new field in `getpeerinfo` RPC to know whether a peer is registered for transaction reconciliation (we register it when receiving a `SENDTXRCNCL` msg).

- The field `tx_reconciliation` is optional for in case you do not support tx reconciliation.
- With this field, we can improve `p2p_sendtxrcncl` tests by checking whether a peer is registered for transaction reconciliation instead of simply rely on logs message.
- d660df07d756ba5e06b74e4b51a7287620ae306b - Just remove unnec
...

💬 brunoerg commented on pull request "p2p: rpc: add `tx_reconciliation` to `getpeerinfo`":
(https://github.com/bitcoin/bitcoin/pull/30984#issuecomment-2377976569)
cc: @sr-gi per our convo.

👍 brunoerg approved a pull request: "contrib: Update asmap link in seeds readme"
(https://github.com/bitcoin/bitcoin/pull/30979#pullrequestreview-2332382359)
ACK f158993fd5575d3cbf991b6df7bcd2af7cf96c9a

⚠️ andremralves opened an issue: "gen-manpages.py should skip nonexistent binaries and still work for the existent binaries"
(https://github.com/bitcoin/bitcoin/issues/30985)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

By executing `BUILDDIR=$PWD/build contrib/devtools/gen-manpages.py`, if one binary is not found, it will stop the generation of all manpages.

For example: Executing gen-manpages.py without bitcoin-qt. The command exits after detecting that bitcoin-qt does not exist, and no manpage is generated, even though only bitcoin-qt is missing.

```bash
➜ bitcoin git:(master) ✗ BUILDDIR=$PWD/b
...

💬 glozow commented on pull request "Disable RBF Rule 2":
(https://github.com/bitcoin/bitcoin/pull/30964#issuecomment-2377982154)
Tend to agree with the "let's wait for cluster mempool, which is currently our only proper solution to the problem" approach.

My reasoning for #23121 was also that rule 2 doesn't actually stop people from adding "new" unconfirmed inputs, and (2) it doesn't do its job within the RBF rules of helping us check that replacements have a better mining score / would confirm faster. I think reason (2) is actually much more important and complicated. I'm not convinced that the removal would be an impr
...