💬 sdaftuar commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#issuecomment-2476081352)
> Instead I was worried about evictions (i can't think of other side effects) of add/remove invalid stuff. Doesn't this allow to clear the victims mempool for free?

Just to be clear, in case anyone has lost track: we're specifically talking about a codepath we believe is not possible to trigger, namely one where the `PolicyScriptChecks` pass yet the `ConsensusScriptChecks` fail for some transaction, and someone is able to submit such a transaction as part of a package to our node.

In this
...

💬 TheCharlatan commented on pull request "guix: scope pkg-config to Linux only":
(https://github.com/bitcoin/bitcoin/pull/31276#issuecomment-2476093725)
Looks like we could could also get rid of the pkgconfig hints in the toolchain file: https://github.com/TheCharlatan/bitcoin/commit/1a6724af305122973efcc8a329e98f41077314a2 (did a guix build with this patch too).

📝 maflcko opened a pull request: "refactor: Avoid std::string format strings"
(https://github.com/bitcoin/bitcoin/pull/31287)
This changes some unchecked `std::string` format strings to use string literals, which are `consteval` checked at compile-time.

Split out, because it is used in several pull requests.

💬 TheCharlatan commented on pull request "refactor: Make node_id a const& in RemoveBlockRequest":
(https://github.com/bitcoin/bitcoin/pull/31282#discussion_r1842093313)
Why not keep using clang for this job?

💬 maflcko commented on pull request "refactor: Make node_id a const& in RemoveBlockRequest":
(https://github.com/bitcoin/bitcoin/pull/31282#discussion_r1842104265)
It requires adding a suppression, or some other workaround at some point, see https://github.com/bitcoin/bitcoin/issues/29635#issuecomment-2047861599

Given that GCC does not require suppressions, it may be preferable for now.

Though, I am happy to drop the commit, and leave it for a follow-up. I just thought, it would be nice to include here, so that testing is easy for those that want.

💬 laanwj commented on issue "Fatal LevelDB error: Corruption: block checksum mismatch on Linux ext4 SATA SSDs":
(https://github.com/bitcoin/bitcoin/issues/30692#issuecomment-2476182650)
> @laanwj suggested to post the utxoset when it gets corrupted again, will do that when it does.

It might (or might not) give some insight into the kind of corruption happening, or whether it's a result of a software bug. Mind that leveldb's writing behavior is very predictable: it only ever appends to files, it doesn't do random writes.

💬 laanwj commented on pull request "rpc: increase the defaults for -rpcthreads and -rpcworkqueue":
(https://github.com/bitcoin/bitcoin/pull/31215#issuecomment-2476206670)
> The higher limits won't increase memory unless they are being used. And these defaults can be set lower on memory constrained systems to disable using them.

Not entirely true, every thread has some memory and overhead, even if it's not doing anything. On 32-bit systems the virtual memory space for the stack space of each thread is also significant.

💬 Sjors commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#discussion_r1842122891)
Done

💬 Sjors commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#issuecomment-2476210165)
I made the test code alternate calls between Chainman's `ProcessNewBlock` and `submitSolution` via the Mining interface.

💬 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