π AngusP approved a pull request: "test: Set mocktime in p2p_disconnect_ban.py to avoid intermittent test failure"
(https://github.com/bitcoin/bitcoin/pull/30174#pullrequestreview-2098492905)
re-ACK 4444de152f01368e603f2b089679a86eae02e34a
(https://github.com/bitcoin/bitcoin/pull/30174#pullrequestreview-2098492905)
re-ACK 4444de152f01368e603f2b089679a86eae02e34a
π fanquake opened a pull request: "doc: fixup deps doc after #30198"
(https://github.com/bitcoin/bitcoin/pull/30227)
Forgotten in #30198.
Addresses: https://github.com/bitcoin/bitcoin/pull/30198#issuecomment-2148087913.
(https://github.com/bitcoin/bitcoin/pull/30227)
Forgotten in #30198.
Addresses: https://github.com/bitcoin/bitcoin/pull/30198#issuecomment-2148087913.
π¬ maflcko commented on pull request "consensus: Store transaction nVersion as uint32_t":
(https://github.com/bitcoin/bitcoin/pull/29325#issuecomment-2149356751)
ACK 2431ecff2fb6f4b68d3de0ea36e3bcc4403e94ba π³
<details><summary>Show signature</summary>
Signature:
```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: ACK 2431ecff2fb6f4b68d3de0ea36
...
(https://github.com/bitcoin/bitcoin/pull/29325#issuecomment-2149356751)
ACK 2431ecff2fb6f4b68d3de0ea36e3bcc4403e94ba π³
<details><summary>Show signature</summary>
Signature:
```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: ACK 2431ecff2fb6f4b68d3de0ea36
...
π fanquake opened a pull request: "build: no-longer allow GCC-10 in C++20 check"
(https://github.com/bitcoin/bitcoin/pull/30228)
Reverts part of fa67f096bdea9db59dd20c470c9e32f3dac5be94, now that we require a minimum of GCC 11.
See also:
https://github.com/bitcoin/bitcoin/pull/28349#issuecomment-1745143612.
(https://github.com/bitcoin/bitcoin/pull/30228)
Reverts part of fa67f096bdea9db59dd20c470c9e32f3dac5be94, now that we require a minimum of GCC 11.
See also:
https://github.com/bitcoin/bitcoin/pull/28349#issuecomment-1745143612.
π¬ maflcko commented on pull request "build: no-longer allow GCC-10 in C++20 check":
(https://github.com/bitcoin/bitcoin/pull/30228#issuecomment-2149383116)
Is there any user facing difference? If so, it would be good to show it in a comment.
(https://github.com/bitcoin/bitcoin/pull/30228#issuecomment-2149383116)
Is there any user facing difference? If so, it would be good to show it in a comment.
π¬ fanquake commented on pull request "build: no-longer allow GCC-10 in C++20 check":
(https://github.com/bitcoin/bitcoin/pull/30228#issuecomment-2149392998)
> Is there any user facing difference? If so, it would be good to show it in a comment.
```bash
./configure CC=gcc-10 CXX=g++-10
<snip>
configure: error: *** A compiler with support for C++20 language features is required.
```
(https://github.com/bitcoin/bitcoin/pull/30228#issuecomment-2149392998)
> Is there any user facing difference? If so, it would be good to show it in a comment.
```bash
./configure CC=gcc-10 CXX=g++-10
<snip>
configure: error: *** A compiler with support for C++20 language features is required.
```
π¬ maflcko commented on pull request "build: no-longer allow GCC-10 in C++20 check":
(https://github.com/bitcoin/bitcoin/pull/30228#issuecomment-2149410956)
Otherwise it would continue to compile (and miscompile `RenameOver`)?
(https://github.com/bitcoin/bitcoin/pull/30228#issuecomment-2149410956)
Otherwise it would continue to compile (and miscompile `RenameOver`)?
π ChillerDragon opened a pull request: "wallet: Improve error log color in the console"
(https://github.com/bitcoin-core/gui/pull/823)
# before

# after

(https://github.com/bitcoin-core/gui/pull/823)
# before

# after

π¬ fanquake commented on pull request "build: no-longer allow GCC-10 in C++20 check":
(https://github.com/bitcoin/bitcoin/pull/30228#issuecomment-2149420217)
> Otherwise it would continue to compile (and miscompile RenameOver)?
I don't know where it would fail first, but I don't see the point of having it continue past configure in this case.
There are multiple devs using clang-14 who've been confused by clang-14 failing to compile code, and unsure what the issue was. So fixing the case here for GCC seems more useful than random compile errors.
(https://github.com/bitcoin/bitcoin/pull/30228#issuecomment-2149420217)
> Otherwise it would continue to compile (and miscompile RenameOver)?
I don't know where it would fail first, but I don't see the point of having it continue past configure in this case.
There are multiple devs using clang-14 who've been confused by clang-14 failing to compile code, and unsure what the issue was. So fixing the case here for GCC seems more useful than random compile errors.
π€ MarnixCroes reviewed a pull request: "wallet: Improve error log color in the console"
(https://github.com/bitcoin-core/gui/pull/823#pullrequestreview-2098729890)
The default GUI is in light-mode, `red` looks fine there for me.
Other warning messages throughout the GUI are also using `red`, they should all use the same color.
(https://github.com/bitcoin-core/gui/pull/823#pullrequestreview-2098729890)
The default GUI is in light-mode, `red` looks fine there for me.
Other warning messages throughout the GUI are also using `red`, they should all use the same color.
π fanquake merged a pull request: "test: Set mocktime in p2p_disconnect_ban.py to avoid intermittent test failure"
(https://github.com/bitcoin/bitcoin/pull/30174)
(https://github.com/bitcoin/bitcoin/pull/30174)
π fanquake merged a pull request: "guix: show `*_FLAGS` variables in pre-build output"
(https://github.com/bitcoin/bitcoin/pull/30185)
(https://github.com/bitcoin/bitcoin/pull/30185)
π¬ maflcko commented on pull request "build: no-longer allow GCC-10 in C++20 check":
(https://github.com/bitcoin/bitcoin/pull/30228#issuecomment-2149476756)
I am asking, because this may need backport, if compilation succeeds. I assumed it would fail, which is why I left it as-is. However, if this compiles for users using g++10, then it seems dangerous. It would expose them to https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98985 and possibly other bugs.
(https://github.com/bitcoin/bitcoin/pull/30228#issuecomment-2149476756)
I am asking, because this may need backport, if compilation succeeds. I assumed it would fail, which is why I left it as-is. However, if this compiles for users using g++10, then it seems dangerous. It would expose them to https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98985 and possibly other bugs.
π¬ fanquake commented on pull request "build: no-longer allow GCC-10 in C++20 check":
(https://github.com/bitcoin/bitcoin/pull/30228#issuecomment-2149496033)
The first compile failure I see is in `util/time.cpp`:
```bash
util/time.cpp: In function βstd::string FormatISO8601DateTime(int64_t)β:
util/time.cpp:50:24: error: βyear_month_dayβ in namespace βstd::chronoβ does not name a type
50 | const std::chrono::year_month_day ymd{days};
| ^~~~~~~~~~~~~~
util/time.cpp:51:24: error: βhh_mm_ssβ in namespace βstd::chronoβ does not name a type
51 | const std::chrono::hh_mm_ss hms{secs - days};
|
...
(https://github.com/bitcoin/bitcoin/pull/30228#issuecomment-2149496033)
The first compile failure I see is in `util/time.cpp`:
```bash
util/time.cpp: In function βstd::string FormatISO8601DateTime(int64_t)β:
util/time.cpp:50:24: error: βyear_month_dayβ in namespace βstd::chronoβ does not name a type
50 | const std::chrono::year_month_day ymd{days};
| ^~~~~~~~~~~~~~
util/time.cpp:51:24: error: βhh_mm_ssβ in namespace βstd::chronoβ does not name a type
51 | const std::chrono::hh_mm_ss hms{secs - days};
|
...
π¬ ChillerDragon commented on pull request "wallet: Improve error log color in the console":
(https://github.com/bitcoin-core/gui/pull/823#issuecomment-2149504423)
A light theme indeed is smoother on the eye

I still think the softer color is better. And it fits both dark and light themes.
(https://github.com/bitcoin-core/gui/pull/823#issuecomment-2149504423)
A light theme indeed is smoother on the eye

I still think the softer color is better. And it fits both dark and light themes.
π¬ paplorinc commented on pull request "refactor: Reduce memory copying operations in bech32 encoding":
(https://github.com/bitcoin/bitcoin/pull/29607#issuecomment-2149519045)
@josibake, now that the cherry-pick was merged, I've rebased and re-measured - the important part of this change was already included in the other PR, the remaining optimizations here are smaller, but also a lot simpler.
(https://github.com/bitcoin/bitcoin/pull/29607#issuecomment-2149519045)
@josibake, now that the cherry-pick was merged, I've rebased and re-measured - the important part of this change was already included in the other PR, the remaining optimizations here are smaller, but also a lot simpler.
π¬ hebasto commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#issuecomment-2149533131)
Ported to the CMake-based build system in https://github.com/hebasto/bitcoin/pull/220.
(https://github.com/bitcoin/bitcoin/pull/26606#issuecomment-2149533131)
Ported to the CMake-based build system in https://github.com/hebasto/bitcoin/pull/220.
π¬ hebasto commented on pull request "Update libsecp256k1 subtree to current master":
(https://github.com/bitcoin/bitcoin/pull/30120#issuecomment-2149534731)
Ported to the CMake-based build system in https://github.com/hebasto/bitcoin/pull/220.
(https://github.com/bitcoin/bitcoin/pull/30120#issuecomment-2149534731)
Ported to the CMake-based build system in https://github.com/hebasto/bitcoin/pull/220.
π¬ josibake commented on pull request "refactor: Reduce memory copying operations in bech32 encoding":
(https://github.com/bitcoin/bitcoin/pull/29607#discussion_r1627539860)
in https://github.com/bitcoin/bitcoin/pull/29607/commits/e7c55d4f4dea4c42b40ff326a17e6884de181ed6:
Is there a reason to prefer `auto` over explicitly mentioning the types? I find it to be more readable as
```suggestion
for (const char& c : hrp) assert(c < 'A' || c > 'Z');
std::string ret;
ret.reserve(hrp.size() + 1 + values.size() + CHECKSUM_SIZE);
ret += hrp;
ret += '1';
for (const uint8_t& i : values) ret += CHARSET[i];
for (const uint8_t& i : Create
...
(https://github.com/bitcoin/bitcoin/pull/29607#discussion_r1627539860)
in https://github.com/bitcoin/bitcoin/pull/29607/commits/e7c55d4f4dea4c42b40ff326a17e6884de181ed6:
Is there a reason to prefer `auto` over explicitly mentioning the types? I find it to be more readable as
```suggestion
for (const char& c : hrp) assert(c < 'A' || c > 'Z');
std::string ret;
ret.reserve(hrp.size() + 1 + values.size() + CHECKSUM_SIZE);
ret += hrp;
ret += '1';
for (const uint8_t& i : values) ret += CHARSET[i];
for (const uint8_t& i : Create
...
π¬ paplorinc commented on pull request "refactor: Reduce memory copying operations in bech32 encoding":
(https://github.com/bitcoin/bitcoin/pull/29607#discussion_r1627559640)
Done, thanks
(https://github.com/bitcoin/bitcoin/pull/29607#discussion_r1627559640)
Done, thanks