💬 Sjors commented on pull request "Have createNewBlock() return a BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/30440#issuecomment-2242552923)
I split `getCoinbaseMerklePath()` and `submitSolution() out of this PR and moved them to [TODO].

Since it's astronomically unlikely multiprocess will be in the upcoming v28.0 release, we can add these functions later. They both use the new `BlockMerkleRoot` which I need to study in more detail myself.

Rebased and addressed comments above: https://github.com/bitcoin/bitcoin/pull/30440#pullrequestreview-2186655119

Marked as no longer draft.

👋 Sjors's pull request is ready for review: "Have createNewBlock() return a BlockTemplate interface"
(https://github.com/bitcoin/bitcoin/pull/30440)

💬 paplorinc commented on pull request "crypto, refactor: add new KeyPair class":
(https://github.com/bitcoin/bitcoin/pull/30051#issuecomment-2242574270)
CI failure seems unrelated, please restart or rebase so we can ACK - and don't worry about our reacks :)

> Update https://github.com/bitcoin/bitcoin/commit/b8b3a9f18670ec7bf246a57950cdae7e034a264d -> https://github.com/bitcoin/bitcoin/commit/56267cf84281789186035f09898cce3d749748ec ([apply-taptweak-method-04](https://github.com/josibake/bitcoin/tree/apply-taptweak-method-04) -> [apply-taptweak-method-05](https://github.com/josibake/bitcoin/tree/apply-taptweak-method-05) ([compare](https://git
...

💬 hebasto commented on pull request "locks: introduce mutex for tx download, flush rejection filters once per tip change":
(https://github.com/bitcoin/bitcoin/pull/30111#discussion_r1686303768)
The `PeerManagerImpl::ProcessMessages` implementation has multiple early `return` statements. In such a case, the Developer Notes [suggest](https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#threads-and-synchronization) to:
> Combine annotations in function declarations with run-time asserts in function definitions

i.e., `AssertLockNotHeld(m_tx_download_mutex);`

💬 darosior commented on pull request "descriptors: Be able to specify change and receiving in a single descriptor string":
(https://github.com/bitcoin/bitcoin/pull/22838#issuecomment-2242578323)
ACK a512245e217199208807214b09bdb256362d08af

💬 paplorinc commented on pull request "rest: Reject truncated hex txid early in getutxos parsing":
(https://github.com/bitcoin/bitcoin/pull/30482#discussion_r1686306726)
Sure, but you did change it, so unless there's any particular reason, I think the original order makes more sense

💬 paplorinc commented on pull request "rest: Reject truncated hex txid early in getutxos parsing":
(https://github.com/bitcoin/bitcoin/pull/30482#discussion_r1686309888)
I understood, that's why I recommended the name change. We don't say `trim` is fragile, just because more values become valid after they're trimmed, we call it lenient, right?

💬 paplorinc commented on pull request "rest: Reject truncated hex txid early in getutxos parsing":
(https://github.com/bitcoin/bitcoin/pull/30482#discussion_r1686312388)
The reason is that it's not a setter, but a complex method that changes the internal state.

💬 stickies-v commented on pull request "rest: Reject truncated hex txid early in getutxos parsing":
(https://github.com/bitcoin/bitcoin/pull/30482#discussion_r1686313096)
I don't see the point of bloating the build logs with this, a tracking issue seems more appropriate if we're worried it'll be forgotten.

💬 hebasto commented on pull request "locks: introduce mutex for tx download, flush rejection filters once per tip change":
(https://github.com/bitcoin/bitcoin/pull/30111#discussion_r1686314984)
Do we need to hold `m_tx_download_mutex` for this call: https://github.com/bitcoin/bitcoin/blob/c85accecafc20f6a6ae94bdf6cdd3ba9747218fd/src/net_processing.cpp#L6327-L6328 a few lines down?

💬 darosior commented on pull request "fuzz: Deglobalize signature cache in sigcache test":
(https://github.com/bitcoin/bitcoin/pull/30447#discussion_r1686316001)
nit: this would invalidate existing seeds.

⚠️ Sjors opened an issue: "Spurious markdown linter failure"
(https://github.com/bitcoin/bitcoin/issues/30496)
The linter job failed for https://github.com/bitcoin/bitcoin/pull/30440

```
One or more markdown links are broken.

Relative links are preferred (but not required) as jumping to file works natively within Emacs.

Markdown link errors found:
[Err ] ./pyenv/README.md (12, 1) => /terminal_output.png - Target filename not found.


^---- ⚠️ Failure generated from lint check 'markdown'!
Check that markdown links resolve
```

The PR does not touch any markdown files.

...

💬 maflcko commented on issue "Spurious markdown linter failure":
(https://github.com/bitcoin/bitcoin/issues/30496#issuecomment-2242612557)
I guess the folder is dirty after building python (after the cache got evicted randomly).

So possibly a solution would be to clean the folder after building python.

💬 stickies-v commented on pull request "fix: Make TxidFromString() respect string_view length":
(https://github.com/bitcoin/bitcoin/pull/30436#issuecomment-2242614404)
re-ACK 788fe9cc9ab979c5e14f544ae05dc46436892b81

No changes except for commit message update.

📝 maflcko opened a pull request: " rpc: Return precise loadtxoutset error messages "
(https://github.com/bitcoin/bitcoin/pull/30497)
The error messages should never happen in normal operation. However, if
they do, they are helpful to return to the user to debug the issue. For
example, to notice a truncated file.

This fixes https://github.com/bitcoin/bitcoin/issues/28621

Also includes a minor refactor commit.

💬 fanquake commented on pull request "depends: build zeromq with CMake":
(https://github.com/bitcoin/bitcoin/pull/29723#discussion_r1686334519)
> My question was about a change that introduced such non-determinism, given that the source code is the same.

The change is CMake. It's passing the full/path/to/the/source/files during compilation, rather than something like src/file which autotools does, which in turn causes the `__FILE__` macros in the zmq error handling code expand to non-deterministic paths.

💬 vasild commented on pull request "util: explicitly close all AutoFiles that have been written":
(https://github.com/bitcoin/bitcoin/pull/29307#issuecomment-2242633991)
`ea89a6e368...4533445fd7`: rebase due to conflicts

💬 Sjors commented on pull request "multiprocess: add bitcoin-mine test program":
(https://github.com/bitcoin/bitcoin/pull/30437#issuecomment-2242645460)
If you want to simplify this PR a bit further, you can drop `waitFeesChanged`, as well as `getCoinbaseMerklePath()` and `submitSolution` from the `BlockTemplate` interface.

I moved those out of the PRs on the repo and into my own fork (https://github.com/Sjors/bitcoin/pull/52 and https://github.com/Sjors/bitcoin/pull/53). I can apply the required changes to `ipc/capnp/mining.capnp` myself in https://github.com/Sjors/bitcoin/pull/48; that's much easier than I expected.

That should remove an
...

🚀 fanquake merged a pull request: "fuzz: Limit parse_univalue input length"
(https://github.com/bitcoin/bitcoin/pull/30473)

💬 paplorinc commented on pull request "rest: Reject truncated hex txid early in getutxos parsing":
(https://github.com/bitcoin/bitcoin/pull/30482#discussion_r1686350738)
I don't see why silencing warnings and putting them to dead comments that cannot bite back is better than "bloating the logs"