💬 TheCharlatan commented on pull request "refactor: Make node_id a const& in RemoveBlockRequest":
(https://github.com/bitcoin/bitcoin/pull/31282#discussion_r1842129659)
Mmh, I was more concerned about losing the sanitizer, but I guess there is nothing really gained by having it turned on when running valgrind.

💬 maflcko commented on pull request "refactor: Make node_id a const& in RemoveBlockRequest":
(https://github.com/bitcoin/bitcoin/pull/31282#discussion_r1842132228)
The only benefit would be the symbolizer, but that's not really needed with valgrind.

👍 storopoli approved a pull request: "rpc: increase the defaults for -rpcthreads and -rpcworkqueue"
(https://github.com/bitcoin/bitcoin/pull/31215#pullrequestreview-2435944639)
ACK e56fc7ce6a92eae7e80657d9f57a148cc002358d

💬 laanwj commented on pull request "refactor: Avoid std::string format strings":
(https://github.com/bitcoin/bitcoin/pull/31287#discussion_r1842156687)
it seems unnecessary to use strprintf at all just to concatenate two strings and a space 😄

👍 TheCharlatan approved a pull request: "refactor: Make node_id a const& in RemoveBlockRequest"
(https://github.com/bitcoin/bitcoin/pull/31282#pullrequestreview-2436015897)
ACK fa21f83d2983d97006ec1e3c47634dc0fe0349dc

👋 Sjors's pull request is ready for review: "Add waitNext() to BlockTemplate interface"
(https://github.com/bitcoin/bitcoin/pull/31283)

💬 Sjors commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#issuecomment-2476309704)
I added test coverage for `waitNext()`.

💬 TheCharlatan commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#issuecomment-2476337636)
In commit f9200f6c65b5a97a4842cc1ec34e688ec13bffcd

"Use createNewBlock via the interface instead of calling Chainman's CreateNewBlock."

Do you mean "Use createNewBlock via the interface instead of calling CreateNewBlock through the BlockAssembler directly"? There is no `CreateNewBlock` in the chainman.

💬 laanwj commented on issue "RFC: Formal description of the RPC API":
(https://github.com/bitcoin/bitcoin/issues/29912#issuecomment-2476350501)
oh neat ! i've been working on a similar export functionality to get the RPCHelpMan data into json, but he beat me to it.
This isn't the format i was targeting, and i still think it's better to avoid introducing a bitcoin-core specific description format, if we can, but exporting it in the first place an important step.

💬 Sjors commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#discussion_r1842225267)
I dropped this line.

Additionally this PR does not make use of `last_mempool_update`, since it's not very useful in a typical production setting where something in the mempool updates all the time.

I think the race condition mentioned there is not an issue here because we don't pass in the current fees via argument, but instead derive them from the original template.

I did forget to update `now` at the end of the loop, which I fixed.

💬 Sjors commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#issuecomment-2476365392)
Made some adjustments based on https://github.com/bitcoin/bitcoin/pull/31283#discussion_r1842225267

💬 Sjors commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#issuecomment-2476371060)
@TheCharlatan I adjusted the commit message

✅ Sjors closed a pull request: "Add waitFeesChanged() to Mining interface"
(https://github.com/bitcoin/bitcoin/pull/31003)

💬 Sjors commented on pull request "Add waitFeesChanged() to Mining interface":
(https://github.com/bitcoin/bitcoin/pull/31003#issuecomment-2476375818)
Actually I'm convinced the `waitNext()` approach is better.

💬 fanquake commented on pull request "guix: scope pkg-config to Linux only":
(https://github.com/bitcoin/bitcoin/pull/31276#issuecomment-2476385674)
> https://github.com/TheCharlatan/bitcoin/commit/1a6724af305122973efcc8a329e98f41077314a2

Pulled that in. Added another commit for sqlite.

💬 hodlinator commented on pull request "descriptor: Add proper Clone function to miniscript::Node":
(https://github.com/bitcoin/bitcoin/pull/30866#issuecomment-2476390623)
> Do you have a particular test case that triggered there?

Worked through several iterations of the `shared_ptr`-removal over a couple of days, at some point I had:
```C++
ret.emplace_back(std::make_unique<MiniscriptDescriptor>(std::move(pubs), std::move(*node)));
```
'cause I wasn't paying attention to the statement being inside a loop. The second descriptor created in the loop ended up with already-moved-from values, which resulted in an this test failing:

https://github.com/bitcoin/
...

💬 josibake commented on pull request "wallet, desc spkm: Return SigningProvider only if we have the privkey":
(https://github.com/bitcoin/bitcoin/pull/31242#issuecomment-2476403763)
Concept ACK

📝 Sjors opened a pull request: "Add destroy to BlockTemplate schema"
(https://github.com/bitcoin/bitcoin/pull/31288)
This ensures that if a client no longer needs a block template, the node can clear its memory as soon as possible.

A block template may hold on to transactions that are no longer in the mempool, so this can be significant.

✅ hodlinator closed a pull request: "test: Fix RANDOM_CTX_SEED use with parallel tests"
(https://github.com/bitcoin/bitcoin/pull/30737)

💬 hodlinator commented on pull request "test: Fix RANDOM_CTX_SEED use with parallel tests":
(https://github.com/bitcoin/bitcoin/pull/30737#issuecomment-2476412113)
Closing since the main issue was fixed in a different way by the now merged #31000.

💬 Sjors commented on pull request "Add destroy to BlockTemplate schema":
(https://github.com/bitcoin/bitcoin/pull/31288#issuecomment-2476412321)
@ryanofsky can you further clarify what the difference in behavior is with an without a `destroy` method?