✅ willcl-ark closed an issue: "bitcoin core v.26 shuts down without warning - Doesnt save blocks downloaded"
(https://github.com/bitcoin/bitcoin/issues/29348)

💬 willcl-ark commented on issue "bitcoin core v.26 shuts down without warning - Doesnt save blocks downloaded":
(https://github.com/bitcoin/bitcoin/issues/29348#issuecomment-1923557734)
Hey @Rucade, I'm going to close this now as I don't think there's anything to be done on the Bitcoin Core side.

Feel free to open a new issue if this re-occurs fo you using v26.0

💬 maflcko commented on issue "test: Write assumeutxo tests":
(https://github.com/bitcoin/bitcoin/issues/28648#issuecomment-1923585233)
Another test idea would be to check what happens when downgrading to a previous release of Bitcoin Core during the background sync.

💬 fanquake commented on pull request "test: Drop `x` modifier in `fsbridge::fopen` call for MinGW builds":
(https://github.com/bitcoin/bitcoin/pull/29357#discussion_r1475960179)
I think this comment could just be something like `Our usage of mingw-w64 and the msvcrt runtime does not support the x modifier for the _wfopen()`. The other info seems a bit overkiil, I'm also not sure if it's correct. From a quick look, if you choose the runtime you want, when you compile mingw-w64, there's no need for any GCC flag.

💬 Sjors commented on pull request "Stratum v2 Noise Protocol":
(https://github.com/bitcoin/bitcoin/pull/29346#issuecomment-1923648187)
Alright, code is back in working state. I dropped a bunch of spurious `Make*ByteSpan` (mostly by switching remaining uses of `uint8_t` to `std::byte`).

Also switched to the new logging convention, mostly `LogTrace()`. Also expanded the fuzzer to mess with bytes during the handshake.

💬 fanquake commented on pull request "test: Drop `x` modifier in `fsbridge::fopen` call for MinGW builds":
(https://github.com/bitcoin/bitcoin/pull/29357#issuecomment-1923649409)
> FWIW, the [_wfopen_s](https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/fopen-s-wfopen-s?view=msvc-170) function reports an error for the x modifier even in Wine.

So it reports an error but the unit tests don't fail? Is this another (different bug)?

💬 hebasto commented on pull request "test: Drop `x` modifier in `fsbridge::fopen` call for MinGW builds":
(https://github.com/bitcoin/bitcoin/pull/29357#issuecomment-1923653115)
> > FWIW, the [_wfopen_s](https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/fopen-s-wfopen-s?view=msvc-170) function reports an error for the x modifier even in Wine.
>
> So it reports an error but the unit tests don't fail? Is this another (different bug)?

We use the `_wfopen`. Microsoft docs [suggests](https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/fopen-wfopen?view=msvc-170):
> More-secure versions of these functions that perform more parameter valida
...

💬 maflcko commented on pull request "test: Drop `x` modifier in `fsbridge::fopen` call for MinGW builds":
(https://github.com/bitcoin/bitcoin/pull/29357#discussion_r1475964652)
> The other info seems a bit overkiil, I'm also not sure if it's correct. From a quick look, if you choose the runtime you want, when you compile mingw-w64, there's no need for any GCC flag.

If the new runtime is available in guix, I'd say it would be better to just use it, instead of patching the code. Or is there some downside I am missing?

💬 fanquake commented on pull request "test: Drop `x` modifier in `fsbridge::fopen` call for MinGW builds":
(https://github.com/bitcoin/bitcoin/pull/29357#discussion_r1475966162)
> If the new runtime is available in guix, I'd say it would be better to just use it, instead of patching the code. Or is there some downside I am missing?

I agree that we should move to the new runtime. I assume the downside is possibly dropping support for Windows 7, but I don't see why that would be a problem.

💬 maflcko commented on pull request "test: Drop `x` modifier in `fsbridge::fopen` call for MinGW builds":
(https://github.com/bitcoin/bitcoin/pull/29357#discussion_r1475966908)
> > The other info seems a bit overkiil, I'm also not sure if it's correct. From a quick look, if you choose the runtime you want, when you compile mingw-w64, there's no need for any GCC flag.
>
> If the new runtime can be linked in guix builds, I'd say it would be better to use it, instead of patching the code. Or is there some downside I am missing?

💬 maflcko commented on pull request "test: Drop `x` modifier in `fsbridge::fopen` call for MinGW builds":
(https://github.com/bitcoin/bitcoin/pull/29357#discussion_r1475969263)
According to the docs, if it is missing, it can be installed via "Windows Update": https://devblogs.microsoft.com/cppblog/introducing-the-universal-crt/ . IIUC it is shipped by default on Windows 10.

👍 stickies-v approved a pull request: "refactor: Fix timedata includes"
(https://github.com/bitcoin/bitcoin/pull/29361#pullrequestreview-1859084735)
ACK fad0fafd5aca699cfab7673f8eb18211139aeb18

💬 stickies-v commented on pull request "refactor: Fix timedata includes":
(https://github.com/bitcoin/bitcoin/pull/29361#discussion_r1475972375)
nit: iwyu suggests

```
chain.h should add these lines:
namespace Consensus { struct Params; }

chain.h should remove these lines:
- #include <consensus/params.h> // lines 10-10
```

💬 hebasto commented on pull request "test: Drop `x` modifier in `fsbridge::fopen` call for MinGW builds":
(https://github.com/bitcoin/bitcoin/pull/29357#discussion_r1475973211)
> if you choose the runtime you want, when you compile mingw-w64

Yes, there is `--with-default-msvcrt=ucrt` configuration option.

💬 dooglus commented on issue "New crash in v26.0":
(https://github.com/bitcoin-core/gui/issues/785#issuecomment-1923669190)
It crashed again. This crash looks the same as the last one:

```
[Thread 0x7fffea7fb6c0 (LWP 2748) exited]
[Thread 0x7fffeaffc6c0 (LWP 2747) exited]

Thread 1 "bitcoin-qt-v26." received signal SIGSEGV, Segmentation fault.
QSortFilterProxyModelPrivate::build_source_to_proxy_mapping (
proxy_to_source=..., source_to_proxy=..., this=this@entry=0x555557a7d520)
at ../../include/QtCore/../../src/corelib/tools/qarraydata.h:61
61 ../../include/QtCore/../../src/corelib/tools/qarray
...

💬 theStack commented on pull request "rpc: parse legacy pubkeys consistently with specific error messages":
(https://github.com/bitcoin/bitcoin/pull/28336#discussion_r1475987895)
Sorry for the late reply. I think the exact order of the checks doesn't really matter here from a performance perspective, since in the typical case (if the passed pubkey is valid) all three of them are executed anyway. Also, if users pass in something that has a different format like e.g. a Bitcoin address, the error message "pubkey has to be in hex" seems to be more useful than "pubkey has the wrong length".

💬 delta1 commented on pull request "refactor: Allow CScript construction from any std::input_iterator":
(https://github.com/bitcoin/bitcoin/pull/29369#issuecomment-1923691732)
Concept ACK dddd56c6eecfb0e0f53874355ee821f5bd6af556

Looks like a nice code change to me. "previous releases" CI is turning up an issue in script/interpreter

```
script/interpreter.cpp: In function ‘bool EvalChecksigPreTapscript(const valtype&, const valtype&, prevector<28, unsigned char>::const_iterator, prevector<28, unsigned char>::const_iterator, unsigned int, const BaseSignatureChecker&, SigVersion, ScriptError*, bool&)’:
script/interpreter.cpp:325:44: error: no matching function f
...

💬 delta1 commented on pull request "refactor: Allow CScript construction from any std::input_iterator":
(https://github.com/bitcoin/bitcoin/pull/29369#issuecomment-1923698793)
> CI is turning up an issue in script/interpreter

looks like a few issues in that file

💬 maflcko commented on pull request "refactor: Fix timedata includes":
(https://github.com/bitcoin/bitcoin/pull/29361#discussion_r1475998235)
Going to leave removal of further includes to a follow-up

💬 fanquake commented on pull request "test: Drop `x` modifier in `fsbridge::fopen` call for MinGW builds":
(https://github.com/bitcoin/bitcoin/pull/29357#discussion_r1476007817)
Yea. So I don't see why anything GCC related is required, or needs to be mentioned here.