💬 pinheadmz commented on pull request "init: Error if ignored bitcoin.conf file is found":
(https://github.com/bitcoin/bitcoin/pull/27302#discussion_r1147851308)
Ok I understand now, I tried removing it but now I see the environment variables are important for setting the initial "where's the conf" directory without spoiling `-datadir`. I also notice `get_temp_default_datadir()` has a windows branch but windows will skip this test anyway! Not a bad idea to leave it in there anyway though I guess.
(https://github.com/bitcoin/bitcoin/pull/27302#discussion_r1147851308)
Ok I understand now, I tried removing it but now I see the environment variables are important for setting the initial "where's the conf" directory without spoiling `-datadir`. I also notice `get_temp_default_datadir()` has a windows branch but windows will skip this test anyway! Not a bad idea to leave it in there anyway though I guess.
💬 pinheadmz commented on pull request "init: Error if ignored bitcoin.conf file is found":
(https://github.com/bitcoin/bitcoin/pull/27302#discussion_r1147851971)
> allowignoredconf
yeah
(https://github.com/bitcoin/bitcoin/pull/27302#discussion_r1147851971)
> allowignoredconf
yeah
💬 jonatack commented on pull request "Add pool based memory resource":
(https://github.com/bitcoin/bitcoin/pull/25325#discussion_r1147852238)
> The code generation is a bit different when you use unsigned types, then the `+ (bytes == 0)` version is the shortedst for me. In my microbenchmark the `+ (bytes == 0)` is also fastest, but in practice its most likely irrelevant
Yes, it seems to be one instruction shorter after updating @LarryRuane's examples (thanks for doing them) to `size_t` and checking with gcc 12.2 and clang 16.
(https://github.com/bitcoin/bitcoin/pull/25325#discussion_r1147852238)
> The code generation is a bit different when you use unsigned types, then the `+ (bytes == 0)` version is the shortedst for me. In my microbenchmark the `+ (bytes == 0)` is also fastest, but in practice its most likely irrelevant
Yes, it seems to be one instruction shorter after updating @LarryRuane's examples (thanks for doing them) to `size_t` and checking with gcc 12.2 and clang 16.
💬 jonatack commented on pull request "Add pool based memory resource":
(https://github.com/bitcoin/bitcoin/pull/25325#discussion_r1147852733)
(Only if you have to retouch, or maybe in a follow-up), perhaps add this comment:
```cpp
BOOST_TEST(b != block); // as `b` has to come from `::operator new` and not from the freelist
(https://github.com/bitcoin/bitcoin/pull/25325#discussion_r1147852733)
(Only if you have to retouch, or maybe in a follow-up), perhaps add this comment:
```cpp
BOOST_TEST(b != block); // as `b` has to come from `::operator new` and not from the freelist
👍 jonatack approved a pull request: "Add pool based memory resource"
(https://github.com/bitcoin/bitcoin/pull/25325)
Nice review work, @LarryRuane.
re-ACK 9f947fc3d4b779f017332135323b34e8f216f613
(https://github.com/bitcoin/bitcoin/pull/25325)
Nice review work, @LarryRuane.
re-ACK 9f947fc3d4b779f017332135323b34e8f216f613
💬 fanquake commented on pull request "ci: Use TSan new runtime (llvm-16, take 3)":
(https://github.com/bitcoin/bitcoin/pull/27298#issuecomment-1483134573)
> Does this happen on master as well?
A different failure with master (e352f5ab6b60ec1cc549997275e945238508cdee):
```bash
4.0K /home/fedora/bitcoin/releases/aarch64-unknown-linux-gnu
Making check in src
make[1]: Entering directory '/home/fedora/bitcoin/ci/scratch/build/bitcoin-aarch64-unknown-linux-gnu/src'
make[2]: Entering directory '/home/fedora/bitcoin/ci/scratch/build/bitcoin-aarch64-unknown-linux-gnu/src'
make minisketch/test
make[3]: Entering directory '/home/fedora/bitcoin/ci/
...
(https://github.com/bitcoin/bitcoin/pull/27298#issuecomment-1483134573)
> Does this happen on master as well?
A different failure with master (e352f5ab6b60ec1cc549997275e945238508cdee):
```bash
4.0K /home/fedora/bitcoin/releases/aarch64-unknown-linux-gnu
Making check in src
make[1]: Entering directory '/home/fedora/bitcoin/ci/scratch/build/bitcoin-aarch64-unknown-linux-gnu/src'
make[2]: Entering directory '/home/fedora/bitcoin/ci/scratch/build/bitcoin-aarch64-unknown-linux-gnu/src'
make minisketch/test
make[3]: Entering directory '/home/fedora/bitcoin/ci/
...
📝 fanquake opened a pull request: "guix: combine and document `enable_werror`"
(https://github.com/bitcoin/bitcoin/pull/27326)
Combine into `hardened-glibc`.
Document why we don't use `--disable-werror` directly.
https://www.gnu.org/software/libc/manual/html_node/Configuring-and-compiling.html
> By default, the GNU C Library is built with -Werror. If you wish
> to build without this option (for example, if building with a
> newer version of GCC than this version of the GNU C Library was
> tested with, so new warnings cause the build with -Werror to fail),
> you can configure with --disable-werror.
(https://github.com/bitcoin/bitcoin/pull/27326)
Combine into `hardened-glibc`.
Document why we don't use `--disable-werror` directly.
https://www.gnu.org/software/libc/manual/html_node/Configuring-and-compiling.html
> By default, the GNU C Library is built with -Werror. If you wish
> to build without this option (for example, if building with a
> newer version of GCC than this version of the GNU C Library was
> tested with, so new warnings cause the build with -Werror to fail),
> you can configure with --disable-werror.
💬 Ayms commented on pull request "Ignore datacarrier limits for dataless OP_RETURN outputs":
(https://github.com/bitcoin/bitcoin/pull/27261#issuecomment-1483171462)
Well, reading this 10 times, I am not sur to see what it has to do with https://github.com/bitcoin/bitcoin/issues/27043
(https://github.com/bitcoin/bitcoin/pull/27261#issuecomment-1483171462)
Well, reading this 10 times, I am not sur to see what it has to do with https://github.com/bitcoin/bitcoin/issues/27043
💬 LarryRuane commented on pull request "Add pool based memory resource":
(https://github.com/bitcoin/bitcoin/pull/25325#issuecomment-1483180518)
ACK 9f947fc3d4b779f017332135323b34e8f216f613
(https://github.com/bitcoin/bitcoin/pull/25325#issuecomment-1483180518)
ACK 9f947fc3d4b779f017332135323b34e8f216f613
💬 hebasto commented on pull request "guix: combine and document `enable_werror`":
(https://github.com/bitcoin/bitcoin/pull/27326#issuecomment-1483200464)
> Document why we don't use `--disable-werror` directly.
Concept ACK on that.
> Combine into `hardened-glibc`.
Not against it, just clarifying. What are benefits of doing that?
(https://github.com/bitcoin/bitcoin/pull/27326#issuecomment-1483200464)
> Document why we don't use `--disable-werror` directly.
Concept ACK on that.
> Combine into `hardened-glibc`.
Not against it, just clarifying. What are benefits of doing that?
📝 SValentyn opened a pull request: "I want more bitcoins! Haven't had time to buy at $0.2 😒"
(https://github.com/bitcoin/bitcoin/pull/27327)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
(https://github.com/bitcoin/bitcoin/pull/27327)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
✅ pinheadmz closed a pull request: "I want more bitcoins! Haven't had time to buy at $0.2 😒"
(https://github.com/bitcoin/bitcoin/pull/27327)
(https://github.com/bitcoin/bitcoin/pull/27327)
📝 fanquake locked a pull request: "I want more bitcoins! Haven't had time to buy at $0.2 😒"
(https://github.com/bitcoin/bitcoin/pull/27327)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
(https://github.com/bitcoin/bitcoin/pull/27327)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
💬 jnewbery commented on pull request "net: #27257 follow-ups":
(https://github.com/bitcoin/bitcoin/pull/27324#discussion_r1147917681)
Perhaps add a comment on why we don't want to copy `CNetMessage` objects.
(https://github.com/bitcoin/bitcoin/pull/27324#discussion_r1147917681)
Perhaps add a comment on why we don't want to copy `CNetMessage` objects.
💬 fanquake commented on pull request "guix: combine and document `enable_werror`":
(https://github.com/bitcoin/bitcoin/pull/27326#issuecomment-1483233917)
I don't see a reason to keep them separated, need multiple funcs etc. I doubt we'll ever turn -Werror back on here, so `hardened-glibc` is really just how we build glibc.
(https://github.com/bitcoin/bitcoin/pull/27326#issuecomment-1483233917)
I don't see a reason to keep them separated, need multiple funcs etc. I doubt we'll ever turn -Werror back on here, so `hardened-glibc` is really just how we build glibc.
📝 pinheadmz opened a pull request: "Flag addresses as "active" (or not)"
(https://github.com/bitcoin-core/gui/pull/723)
Closes https://github.com/bitcoin/bitcoin/issues/3314
Requires https://github.com/bitcoin/bitcoin/pull/27216
description WIP
<img width="1322" alt="Screen Shot 2023-03-24 at 2 31 16 PM" src="https://user-images.githubusercontent.com/2084648/227613293-3ad976ba-3b90-4ab5-8cd8-441455e2c6a5.png">
(https://github.com/bitcoin-core/gui/pull/723)
Closes https://github.com/bitcoin/bitcoin/issues/3314
Requires https://github.com/bitcoin/bitcoin/pull/27216
description WIP
<img width="1322" alt="Screen Shot 2023-03-24 at 2 31 16 PM" src="https://user-images.githubusercontent.com/2084648/227613293-3ad976ba-3b90-4ab5-8cd8-441455e2c6a5.png">
💬 1440000bytes commented on pull request "Ignore datacarrier limits for dataless OP_RETURN outputs":
(https://github.com/bitcoin/bitcoin/pull/27261#issuecomment-1483310633)
> Well, reading this 10 times, I am not sur to see what it has to do with #27043
I assume this was a test run for real pull pull request which will be controversial for sure, dont get mad else some people from this repo might write things about you. Getting mad is only reserved for a few people here.
(https://github.com/bitcoin/bitcoin/pull/27261#issuecomment-1483310633)
> Well, reading this 10 times, I am not sur to see what it has to do with #27043
I assume this was a test run for real pull pull request which will be controversial for sure, dont get mad else some people from this repo might write things about you. Getting mad is only reserved for a few people here.
💬 hebasto commented on pull request "guix: combine and document `enable_werror`":
(https://github.com/bitcoin/bitcoin/pull/27326#issuecomment-1483359577)
> I don't see a reason to keep them separated, need multiple funcs etc.
In the future, if we will have multiple glibc versions again, as it was when you [introduced](https://github.com/bitcoin/bitcoin/pull/25076/commits/c9c5b3060d2edb47ebfa7974fdde3154036717c2) those "multiple funcs", they will be useful again.
I mean, while this change improves code readability, it hurts maintainability a bit, doesn't it?
(https://github.com/bitcoin/bitcoin/pull/27326#issuecomment-1483359577)
> I don't see a reason to keep them separated, need multiple funcs etc.
In the future, if we will have multiple glibc versions again, as it was when you [introduced](https://github.com/bitcoin/bitcoin/pull/25076/commits/c9c5b3060d2edb47ebfa7974fdde3154036717c2) those "multiple funcs", they will be useful again.
I mean, while this change improves code readability, it hurts maintainability a bit, doesn't it?
📝 theuni opened a pull request: "depends: fix osx build with clang 16"
(https://github.com/bitcoin/bitcoin/pull/27328)
Current build (using forced system clang as a test) results in:
> error: unknown argument: '-internal-externc-isystem/opt/clang+llvm-16.0.0-x86_64-linux-gnu-ubuntu-18.04/lib/clang/16/include'
For some reason the previous syntax worked with clang 15 and below, but clang 16 requires that the option and value are properly separated.
See [here for an example of upstream using this syntax](https://github.com/llvm/llvm-project/blob/main/clang/test/Driver/crash-report-with-asserts.c#L9).
Th
...
(https://github.com/bitcoin/bitcoin/pull/27328)
Current build (using forced system clang as a test) results in:
> error: unknown argument: '-internal-externc-isystem/opt/clang+llvm-16.0.0-x86_64-linux-gnu-ubuntu-18.04/lib/clang/16/include'
For some reason the previous syntax worked with clang 15 and below, but clang 16 requires that the option and value are properly separated.
See [here for an example of upstream using this syntax](https://github.com/llvm/llvm-project/blob/main/clang/test/Driver/crash-report-with-asserts.c#L9).
Th
...
💬 hebasto commented on pull request "depends: fix osx build with clang 16":
(https://github.com/bitcoin/bitcoin/pull/27328#issuecomment-1483390448)
#27314?
(https://github.com/bitcoin/bitcoin/pull/27328#issuecomment-1483390448)
#27314?