💬 hodlinator commented on pull request "qa: Avoid knock-on exception in assert_start_raises_init_error":
(https://github.com/bitcoin/bitcoin/pull/32929#issuecomment-3607011155)
Latest push (42ed679f0bb9807b08e6209528829f1e45f8765d):
* Rebases to resolve conflict with 42ed679f0bb9807b08e6209528829f1e45f8765d
* Adds 9481948e0d979dccedd7b4e6616ef4903bc1cdba - Reverts one line of 42ed679f0bb9807b08e6209528829f1e45f8765d to fix issue
* Adds 0ff74253858f0a9b6c1f0fecb815c8380d5661be - Minor documentation change to `assert_start_raises_init_error()` as requested by https://github.com/bitcoin/bitcoin/pull/32929#issuecomment-3453237274
* Modifies last commit 42ed679f0bb9807b
...

💬 hodlinator commented on pull request "contrib: Remove brittle, confusing and redundant UTF8 encoding from Python IO":
(https://github.com/bitcoin/bitcoin/pull/33702#discussion_r2585288311)
The change on this specific line is incorrect, see 9481948e0d979dccedd7b4e6616ef4903bc1cdba (#32929) for a fix.

Verified that the changes to test/lint/lint-spelling.py (where we are also printing output from an exception field) to the contrary *are correct* in this PR by passing in an invalid argument in the subprocess call.

🤔 darosior reviewed a pull request: "test: clarify timewarp grace period griefing attack"
(https://github.com/bitcoin/bitcoin/pull/31725#pullrequestreview-3535301159)
This is adding a test that demonstrates how Bitcoin Core would reject a block from a broken mining pool software that:
- ignores the block chain and set the block timestamp using wall clock time;
- ignores the `mintime` value provided by Bitcoin Core's block template;
- ignores the `curtime` value provided by Bitcoin Core's block template.

There is many, many ways a mining pool software that disregards consensus rules and values provided by Bitcoin Core's block template may create an invalid bl
...

✅ Sjors closed a pull request: "test: add logging to mock external signers"
(https://github.com/bitcoin/bitcoin/pull/32928)

💬 Sjors commented on pull request "test: add logging to mock external signers":
(https://github.com/bitcoin/bitcoin/pull/32928#issuecomment-3607170297)
Closing as up for grabs (rather than rebasing again) due to lack of review.

💬 laanwj commented on pull request "guix: use GCC 14.3.0 over 13.3.0":
(https://github.com/bitcoin/bitcoin/pull/33775#issuecomment-3607180028)
This is a minimum C++ file that will reproduce the issue:
```c++
#include <filesystem>
#include <string>
#include <fstream>
#include <vector>

#include <cstring> // Unused, but removing this makes it determinstic

namespace fs {

using namespace std::filesystem;

// Dummy class. Using "path = std::filesystem::path" makes it deterministic.
class path : public std::filesystem::path
{
public:
using std::filesystem::path::path;

path(std::filesystem::path path) : std::file
...

✅ Sjors closed a pull request: "test: clarify timewarp grace period griefing attack"
(https://github.com/bitcoin/bitcoin/pull/31725)

💬 Sjors commented on pull request "test: clarify timewarp grace period griefing attack":
(https://github.com/bitcoin/bitcoin/pull/31725#issuecomment-3607255055)
[BIP54](https://github.com/bitcoin/bips/blob/master/bip-0054.md) has been updated to use a 2 hour grace period rather than the `MAX_TIMEWARP = 600;` that we use.

If you change `MAX_TIMEWARP` to `7200` there's no way to make the test in this PR pass. I'll leave that as an exercise to the reader.

Note that the list of three bullet items boils down to _a single mistake_: using wall time for `nTime`. And as documented in the Delving post, that's not a hypothetical mistake. It's no longer a pro
...

💬 furszy commented on pull request "wallet, test: Ancient Wallet Migration from v0.14.3 (no-HD and Single Chain)":
(https://github.com/bitcoin/bitcoin/pull/33186#issuecomment-3607270186)
Concept ACK, will review soon

🤔 furszy reviewed a pull request: "qa: Avoid knock-on exception in assert_start_raises_init_error"
(https://github.com/bitcoin/bitcoin/pull/32929#pullrequestreview-3535491080)
Concept ACK, cool stuff

💬 pablomartin4btc commented on pull request "Update about logo icon (colour) to denote the chain type of the QT instance in About/ Help Message Window/ Dialog":
(https://github.com/bitcoin-core/gui/pull/762#issuecomment-3607354974)
_<ins>Updates:</ins>_
- Addressed @laanwj's feedback.
- Reworked the [error message window inconsistency](https://github.com/bitcoin-core/gui/pull/762#issuecomment-2276259183) fix detected by @hebasto [above](https://github.com/bitcoin-core/gui/pull/762#pullrequestreview-2219281894), applying it to most `QMessageBox` possible instances in `bitcoin.cpp` and separating it in the 2nd commit for clarity and in case it needs to be dropped because it's getting out of scope or extended to other files
...

💬 hebasto commented on pull request "depends: Propagate native C compiler to `sqlite` package":
(https://github.com/bitcoin/bitcoin/pull/33995#issuecomment-3607392698)
My Guiz build:
```
x86_64
5ea5588f1e2ee4e37f4b90313a8c32ec17474a39d1dff77d9d585ae9e106c761 guix-build-710031ebef83/output/aarch64-linux-gnu/SHA256SUMS.part
8d7b95ecb5950220f6d70c069d7fdf5add92f8135daee0d0acb9af753c9bab0c guix-build-710031ebef83/output/aarch64-linux-gnu/bitcoin-710031ebef83-aarch64-linux-gnu-debug.tar.gz
dd0f06c48c57e1243437298d02c219758ecd167c88d3a59be15a9051434a99cb guix-build-710031ebef83/output/aarch64-linux-gnu/bitcoin-710031ebef83-aarch64-linux-gnu.tar.gz
fe143b6be
...

✅ hebasto closed a pull request: "depends, doc: Add `tcl` as build dependency for `sqlite` package"
(https://github.com/bitcoin/bitcoin/pull/33975)

💬 hebasto commented on pull request "depends, doc: Add `tcl` as build dependency for `sqlite` package":
(https://github.com/bitcoin/bitcoin/pull/33975#issuecomment-3607395504)
> > Can we just pass the compiler through?
>
> See #33995.

Closing in favour of https://github.com/bitcoin/bitcoin/pull/33995.

💬 hebasto commented on pull request "depends: Switch from multilib to platform-specific toolchains":
(https://github.com/bitcoin/bitcoin/pull/32162#issuecomment-3607424994)
Rebased to resolve a conflict with merged bitcoin/bitcoin#33857.

💬 hebasto commented on pull request "guix: Use UCRT runtime for Windows release binaries":
(https://github.com/bitcoin/bitcoin/pull/33593#issuecomment-3607449106)
Rebased on top of the merged bitcoin/bitcoin#33857.

💬 ryanofsky commented on issue "Memory leak when using IPC mining interface":
(https://github.com/bitcoin/bitcoin/issues/33940#issuecomment-3607454869)
> yes, that's also my understanding... the puzzling thing is that we're still having the memory leaks... so we're either missing some detail on this analysis, or the cause is related to something other than templates

I think at this point we've ruled out the obvious possible causes of leaks, and need to start debugging the issue more directly.

It would help to have some steps to reproduce. If someone can let me know what code to check out and build, and what commands to run that will show the
...

💬 JeremyRubin commented on pull request "ci: remove `doc/release-notes.md` from lint-spelling.py":
(https://github.com/bitcoin/bitcoin/pull/33968#issuecomment-3607478808)
I guess it's _not_ a failure, as it only pops up when other issues flag, but it does get reported in CI as:

```
^---- ⚠️ Failure generated from lint-tests.py
doc/release-notes.md:379: Atack ==> Attack
```

which is sort of confusing

💬 fanquake commented on issue "build: use UCRT runtime for Windows (release) binaries":
(https://github.com/bitcoin/bitcoin/issues/30210#issuecomment-3607488174)
> Since https://github.com/bitcoin/bitcoin/pull/33764 has been merged, this checkbox can now be ticked.

As I mentioned in that PR, neither the headers, neither the compiler version, or headers used, match Guix, so its not exactly testing something similar to how we build releases. That will be improved by #33775.

💬 fanquake commented on pull request "guix: Use UCRT runtime for Windows release binaries":
(https://github.com/bitcoin/bitcoin/pull/33593#issuecomment-3607495972)
Given it's dropping the other CI, I think this PR should be completing #30210, dropping workarounds, updating any docs, and completing the migration.