🚀 fanquake merged a pull request: "build, doc: Adjust comment after PR27254"
(https://github.com/bitcoin/bitcoin/pull/27656)
(https://github.com/bitcoin/bitcoin/pull/27656)
💬 fanquake commented on pull request "rpc: Add test-only RPC getaddrmaninfo for new/tried table address count":
(https://github.com/bitcoin/bitcoin/pull/27511#issuecomment-1547603057)
> I think that ideally we never should have exposed the terrible filter in the first place.
@mzumsande do you want to propose some changes to remove its exposure?
(https://github.com/bitcoin/bitcoin/pull/27511#issuecomment-1547603057)
> I think that ideally we never should have exposed the terrible filter in the first place.
@mzumsande do you want to propose some changes to remove its exposure?
👍 TheCharlatan approved a pull request: "init: Error if ignored bitcoin.conf file is found"
(https://github.com/bitcoin/bitcoin/pull/27302#pullrequestreview-1426261194)
ACK 0319de5cbedd1a8f8766cfec61151c58b3fb27ef
(https://github.com/bitcoin/bitcoin/pull/27302#pullrequestreview-1426261194)
ACK 0319de5cbedd1a8f8766cfec61151c58b3fb27ef
🚀 fanquake merged a pull request: "Introduce `MockableDatabase` for wallet unit tests"
(https://github.com/bitcoin/bitcoin/pull/26715)
(https://github.com/bitcoin/bitcoin/pull/26715)
💬 MarcoFalke commented on pull request "test: Drop `deadlock:libdb` TSan suppression":
(https://github.com/bitcoin/bitcoin/pull/27658#issuecomment-1547639621)
Steps to reproduce on a fresh install of OpenSuse Tumbleweed (`registry.opensuse.org/opensuse/tumbleweed:latest`):
```
zypper in -y libevent-devel boost-devel sqlite3-devel libqt5-qttools-devel libqt5-qtbase-devel libdb-4_8-devel find bison gcc-c++ libtool make autoconf automake python3 clang llvm lbzip2 patch xz curl wget htop git vim ccache && git clone https://github.com/bitcoin/bitcoin.git --depth=1 ./bitcoin-core && cd bitcoin-core && ./autogen.sh && ./configure CC=cla
...
(https://github.com/bitcoin/bitcoin/pull/27658#issuecomment-1547639621)
Steps to reproduce on a fresh install of OpenSuse Tumbleweed (`registry.opensuse.org/opensuse/tumbleweed:latest`):
```
zypper in -y libevent-devel boost-devel sqlite3-devel libqt5-qttools-devel libqt5-qtbase-devel libdb-4_8-devel find bison gcc-c++ libtool make autoconf automake python3 clang llvm lbzip2 patch xz curl wget htop git vim ccache && git clone https://github.com/bitcoin/bitcoin.git --depth=1 ./bitcoin-core && cd bitcoin-core && ./autogen.sh && ./configure CC=cla
...
💬 fanquake commented on pull request "test: Drop `deadlock:libdb` TSan suppression":
(https://github.com/bitcoin/bitcoin/pull/27658#issuecomment-1547642636)
Concept NACK given the above.
(https://github.com/bitcoin/bitcoin/pull/27658#issuecomment-1547642636)
Concept NACK given the above.
💬 fanquake commented on pull request "init: add MALLOC_ARENA_MAX=1 to systemd":
(https://github.com/bitcoin/bitcoin/pull/27642#issuecomment-1547643907)
Note that internally we already set [`M_ARENA_MAX` to 1](https://github.com/bitcoin/bitcoin/blob/d02df7db6b3651a725fe35be42e3489e2d6b53a1/src/util/system.cpp#L70), but that is currently restricted to 32-bit systems. I would assume everyone reporting issues (#24542) is using on 64-bit. We may be at the point where we could just drop that code entirely.
(https://github.com/bitcoin/bitcoin/pull/27642#issuecomment-1547643907)
Note that internally we already set [`M_ARENA_MAX` to 1](https://github.com/bitcoin/bitcoin/blob/d02df7db6b3651a725fe35be42e3489e2d6b53a1/src/util/system.cpp#L70), but that is currently restricted to 32-bit systems. I would assume everyone reporting issues (#24542) is using on 64-bit. We may be at the point where we could just drop that code entirely.
🤔 stickies-v reviewed a pull request: "test: Return dict in MiniWallet::send_to"
(https://github.com/bitcoin/bitcoin/pull/27640#pullrequestreview-1426307846)
Concept ACK for moving away from a tuple return value. I think I'd prefer something more struct-like though, for example a `typing.NamedTuple`? Also, I don't see the point in returning `tx`, `wtxid`, and `hex` if no one is accessing them yet - can easily be added later on when needed?
(https://github.com/bitcoin/bitcoin/pull/27640#pullrequestreview-1426307846)
Concept ACK for moving away from a tuple return value. I think I'd prefer something more struct-like though, for example a `typing.NamedTuple`? Also, I don't see the point in returning `tx`, `wtxid`, and `hex` if no one is accessing them yet - can easily be added later on when needed?
💬 fanquake commented on issue ""Create Unsigned" should not show the message: "The amount exceeds you balance" without suggesting alternatives":
(https://github.com/bitcoin/bitcoin/issues/27659#issuecomment-1547651940)
> 25.0.1 rc1
This version doesn't exist?
> There might have been similar issue reported: https://github.com/bitcoin-core/gui/issues/83
Given there is already what looks like a similar issue, might be better to continue the discussion there? GUI issues should be opened on the GUI repo: https://github.com/bitcoin-core/gui.
(https://github.com/bitcoin/bitcoin/issues/27659#issuecomment-1547651940)
> 25.0.1 rc1
This version doesn't exist?
> There might have been similar issue reported: https://github.com/bitcoin-core/gui/issues/83
Given there is already what looks like a similar issue, might be better to continue the discussion there? GUI issues should be opened on the GUI repo: https://github.com/bitcoin-core/gui.
💬 MarcoFalke commented on pull request "test: Return dict in MiniWallet::send_to":
(https://github.com/bitcoin/bitcoin/pull/27640#issuecomment-1547661500)
> I'd prefer something more struct-like
The idea is to use the same return type as the other MiniWallet methods. Not sure if it is worth it to use a named tuple. (rant: If you want compile-time type checks, you are better off using rust, instead of maiming python code with type annotations)
> I don't see the point in returning tx, wtxid, and hex if no one is accessing them yet
The idea was to use the same return values as the other MiniWallet methods, but I am happy to drop this.
(https://github.com/bitcoin/bitcoin/pull/27640#issuecomment-1547661500)
> I'd prefer something more struct-like
The idea is to use the same return type as the other MiniWallet methods. Not sure if it is worth it to use a named tuple. (rant: If you want compile-time type checks, you are better off using rust, instead of maiming python code with type annotations)
> I don't see the point in returning tx, wtxid, and hex if no one is accessing them yet
The idea was to use the same return values as the other MiniWallet methods, but I am happy to drop this.
✅ hebasto closed a pull request: "test: Drop `deadlock:libdb` TSan suppression"
(https://github.com/bitcoin/bitcoin/pull/27658)
(https://github.com/bitcoin/bitcoin/pull/27658)
💬 MarcoFalke commented on pull request "test: Drop `deadlock:libdb` TSan suppression":
(https://github.com/bitcoin/bitcoin/pull/27658#issuecomment-1547767101)
Could add a link with steps to reproduce as doc?
(https://github.com/bitcoin/bitcoin/pull/27658#issuecomment-1547767101)
Could add a link with steps to reproduce as doc?
📝 hebasto opened a pull request: "doc, test: Document steps to reproduce TSan warning for `libdb`"
(https://github.com/bitcoin/bitcoin/pull/27661)
Requested [here](https://github.com/bitcoin/bitcoin/pull/27658#issuecomment-1547767101).
(https://github.com/bitcoin/bitcoin/pull/27661)
Requested [here](https://github.com/bitcoin/bitcoin/pull/27658#issuecomment-1547767101).
💬 hebasto commented on pull request "test: Drop `deadlock:libdb` TSan suppression":
(https://github.com/bitcoin/bitcoin/pull/27658#issuecomment-1547773651)
@MarcoFalke
> Could add a link with steps to reproduce as doc?
https://github.com/bitcoin/bitcoin/pull/27661
(https://github.com/bitcoin/bitcoin/pull/27658#issuecomment-1547773651)
@MarcoFalke
> Could add a link with steps to reproduce as doc?
https://github.com/bitcoin/bitcoin/pull/27661
💬 brunoerg commented on pull request "fuzz: wallet, add target for `fees`":
(https://github.com/bitcoin/bitcoin/pull/27647#discussion_r1193770535)
Since I'm not using anything related to mempool, I don't think so, will remove it.
(https://github.com/bitcoin/bitcoin/pull/27647#discussion_r1193770535)
Since I'm not using anything related to mempool, I don't think so, will remove it.
💬 achow101 commented on issue "v25.0 testing":
(https://github.com/bitcoin/bitcoin/issues/27621#issuecomment-1547781966)
> What could be causing this behaviour?
There are a few new signers this time so you're probably missing their keys.
(https://github.com/bitcoin/bitcoin/issues/27621#issuecomment-1547781966)
> What could be causing this behaviour?
There are a few new signers this time so you're probably missing their keys.
💬 jonatack commented on pull request "rpc: Add test-only RPC getaddrmaninfo for new/tried table address count":
(https://github.com/bitcoin/bitcoin/pull/27511#issuecomment-1547789857)
The author and the numerous reviewers of #13152 were aware of the relationship to getaddr (see the PR description and review discussion), as well as mention of IsTerrible in https://github.com/bitcoin/bitcoin/pull/13152#pullrequestreview-121943454.
I don't see it as unfortunate; as a frequent user of getnodeaddresses and -addrinfo it seems useful that the data doesn't include terrible/non-recent (and banned/discouraged) peers.
But guessing whether it was coincidental is beside the point. W
...
(https://github.com/bitcoin/bitcoin/pull/27511#issuecomment-1547789857)
The author and the numerous reviewers of #13152 were aware of the relationship to getaddr (see the PR description and review discussion), as well as mention of IsTerrible in https://github.com/bitcoin/bitcoin/pull/13152#pullrequestreview-121943454.
I don't see it as unfortunate; as a frequent user of getnodeaddresses and -addrinfo it seems useful that the data doesn't include terrible/non-recent (and banned/discouraged) peers.
But guessing whether it was coincidental is beside the point. W
...
📝 MarcoFalke opened a pull request: "build: Drop support for g++-8 "
(https://github.com/bitcoin/bitcoin/pull/27662)
It is a bit frustrating to write valid C++ code only to realize that g++-8 fails to parse it later on.
The only non-EOL operating system still shipping with g++-8 is CentOS 8. I think it is reasonable for users of affected Linux distributions to:
* Upgrade their operating system, or compiler to a supported version.
* Alternatively, stay with a previous release of Bitcoin Core as long as it is supported.
Fixes https://github.com/bitcoin/bitcoin/issues/27537
(https://github.com/bitcoin/bitcoin/pull/27662)
It is a bit frustrating to write valid C++ code only to realize that g++-8 fails to parse it later on.
The only non-EOL operating system still shipping with g++-8 is CentOS 8. I think it is reasonable for users of affected Linux distributions to:
* Upgrade their operating system, or compiler to a supported version.
* Alternatively, stay with a previous release of Bitcoin Core as long as it is supported.
Fixes https://github.com/bitcoin/bitcoin/issues/27537
💬 hebasto commented on pull request "build: Drop support for g++-8":
(https://github.com/bitcoin/bitcoin/pull/27662#issuecomment-1547796152)
Does this [comment](https://github.com/bitcoin/bitcoin/issues/27537#issuecomment-1525048924) hold:
> Added 27.0 for now, which should be uncontroversial.
?
(https://github.com/bitcoin/bitcoin/pull/27662#issuecomment-1547796152)
Does this [comment](https://github.com/bitcoin/bitcoin/issues/27537#issuecomment-1525048924) hold:
> Added 27.0 for now, which should be uncontroversial.
?
💬 fanquake commented on pull request "build: Drop support for g++-8":
(https://github.com/bitcoin/bitcoin/pull/27662#issuecomment-1547796695)
Concept ACK - seems fine for 26.0.
(https://github.com/bitcoin/bitcoin/pull/27662#issuecomment-1547796695)
Concept ACK - seems fine for 26.0.