⚠️ craigraw opened an issue: "Feature Request: Broadcast Pool"
(https://github.com/bitcoin/bitcoin/issues/30471)
### Please describe the feature you'd like to see added.

This is a feature request for a broadcast pool. A broadcast pool is a cache local to the node which contains transactions which have been initially broadcast from that node, either from a local wallet or via an RPC like `sendrawtransaction`. Broadcast transactions are retained in this cache until they are included in a block, or expire after the configured `mempoolexpiry` period. The transactions in the broadcast pool are included in view
...

💬 hebasto commented on pull request "depends: build zeromq with CMake":
(https://github.com/bitcoin/bitcoin/pull/29723#issuecomment-2233129404)
> > From [zeromq/libzmq#4667 (comment)](https://github.com/zeromq/libzmq/pull/4667#issue-2197755398):
>
> Can you elaborate / suggest something concrete?

Please see https://github.com/zeromq/libzmq/pull/4706.

👍 hodlinator approved a pull request: "rest: Reject negative outpoint index early in getutxos parsing"
(https://github.com/bitcoin/bitcoin/pull/30444#pullrequestreview-2182729770)
ut-ACK fac932bf93d9bd8cb7bef93f04ecf48ea2ccf1d1

Nice that the added tests managed to trigger the sanitizer! :+1:

💬 vasild commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1680951137)
Would all-zeros work as well for IPv6 / pinholing? If yes, then this can probably be simplified to always pass all-zeros.

💬 stratospher commented on pull request "test: bump mocktime only after node has received and sent bytes":
(https://github.com/bitcoin/bitcoin/pull/30468#discussion_r1680951432)
I couldn't reproduce it locally.

since peer timeout is `3`, `InactivityCheck()`(and hence disconnection) will happen when our time is` > 3`. so disconnection will only happen during the next `bumpmocktime(1)` I think.

(Also see https://github.com/bitcoin/bitcoin/pull/30420#discussion_r1680773490)

💬 maflcko commented on issue "Feature Request: Broadcast Pool":
(https://github.com/bitcoin/bitcoin/issues/30471#issuecomment-2233185138)
> making this existing functionality available to all transactions broadcasted through the node and not just those created by a local wallet.

Would it not be possible to implement this by creating a wallet called `broadcast_pool` and then write simple wrapper functions to implement the broadcast pool features?

> Future-dated transactions are also included in views on the mempool, but are marked as unbroadcast: true in mempool RPC responses

I don't think it is possible to include somethi
...

💬 Sjors commented on pull request "multiprocess: add bitcoin-mine test program":
(https://github.com/bitcoin/bitcoin/pull/30437#discussion_r1680953879)
8a20be71f07cfe0129107cb8effb2490b79639bf: `#include <logging.h>`

💬 hodlinator commented on pull request "refactor: Make uint256S(const char*) and ParseHex(const char*) consteval":
(https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1680955841)
Thanks for taking a closer look!

I want to enforce `consteval` for existing string literals. Ideally the `string_view` overload should be `constexpr` but from my research on MSVC assembly output in the PR summary it seems to handle lookup tables poorly. One possibility would be to remove the lookup table and use `if (c >= '0' && c <= '9')`, sacrificing some performance, but we don't often parse ASCII hex strings in runtime anyway, right?

The main purpose of overloading `uint256S()` with a
...

💬 maflcko commented on pull request "test: bump mocktime only after node has received and sent bytes":
(https://github.com/bitcoin/bitcoin/pull/30468#discussion_r1680958506)
Ah, I see the check is `<`, not `<=`.

However, what is the purpose of two immediately following `bumpmocktime` on the same node? Wouldn't it be easier to just combine them into one (and use the sum of their arguments)?

💬 maflcko commented on pull request "test: bump mocktime only after node has received and sent bytes":
(https://github.com/bitcoin/bitcoin/pull/30468#issuecomment-2233196862)
review ACK e6f4614ea57dbd3d8690fdbed55ecf2612619070

Looks good, feel free to ignore the nit.

💬 hodlinator commented on pull request "refactor: Make uint256S(const char*) and ParseHex(const char*) consteval":
(https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1680963050)
My first stab at this was much more strict but when I added tests I went for full compatibility with the runtime overload. I'm certainly open to making it less permissive though.

💬 hodlinator commented on pull request "refactor: Make uint256S(const char*) and ParseHex(const char*) consteval":
(https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1680963523)
Same (https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1680963050)

💬 stratospher commented on pull request "test: bump mocktime only after node has received and sent bytes":
(https://github.com/bitcoin/bitcoin/pull/30468#discussion_r1680964397)
true, done.

💬 maflcko commented on pull request "refactor: Make uint256S(const char*) and ParseHex(const char*) consteval":
(https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1680966315)
> `uint256(uint64_t,uint64_t,uint64_t,uint64_t)` constructor. That way it would be the compiler parsing hexadecimal integer literals directly.

Not sure. That'd make it impossible to grep for a (let's say) block hash. Also, it would be harder to copy-paste one, if the developer has to manually split it into 4 parts and add `0x` prefixes. Finally, truncation checks can't be done, see https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1680422462

> was to avoid changing all the call-sit
...

💬 maflcko commented on pull request "refactor: Make uint256S(const char*) and ParseHex(const char*) consteval":
(https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1680967973)
> I want to enforce `consteval` for existing string literals. Ideally the `string_view` overload should be `constexpr` but from my research on MSVC assembly output

I think `string_view` can be used in a consteval context, no?

💬 maflcko commented on pull request "test: bump mocktime only after node has received and sent bytes":
(https://github.com/bitcoin/bitcoin/pull/30468#issuecomment-2233211540)
reACK c322bddd08ed1f1f7f0c39768b659dd62d5e2dd5

💬 hodlinator commented on pull request "refactor: Make uint256S(const char*) and ParseHex(const char*) consteval":
(https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1680976118)
There is no `uint160S()` outside of **uint256_tests.cpp** and only `uint256` and `uint160` use `base_blob` so it felt alright having it here. If we abandon the `uint256S()`-overload approach I agree to change this.

📝 Igor258 opened a pull request: "btc"
(https://github.com/bitcoin/bitcoin/pull/30472)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...

✅ fanquake closed a pull request: "btc"
(https://github.com/bitcoin/bitcoin/pull/30472)

📝 fanquake locked a pull request: "btc"
(https://github.com/bitcoin/bitcoin/pull/30472)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...