💬 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.
💬 MarcoFalke commented on pull request "doc, test: Document steps to reproduce TSan warning for `libdb`":
(https://github.com/bitcoin/bitcoin/pull/27661#issuecomment-1547800982)
lgtm ACK f03a708c1190852862c2e3ade5ee01797f291dd4
(https://github.com/bitcoin/bitcoin/pull/27661#issuecomment-1547800982)
lgtm ACK f03a708c1190852862c2e3ade5ee01797f291dd4
💬 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-1547801330)
@stratospher do you plan to repush after rebase to current master for the CI and perhaps with the totals?
(https://github.com/bitcoin/bitcoin/pull/27511#issuecomment-1547801330)
@stratospher do you plan to repush after rebase to current master for the CI and perhaps with the totals?
💬 hebasto commented on pull request "build: Drop support for g++-8":
(https://github.com/bitcoin/bitcoin/pull/27662#issuecomment-1547801748)
> The only non-EOL operating system still shipping with g++-8 is CentOS 8.
> seems fine for 26.0.
Considering https://www.centos.org/centos-linux-eol/:
> CentOS Linux 8 will reach End Of Life (EOL) on December 31st, 2021.
I agree.
Concept ACK.
(https://github.com/bitcoin/bitcoin/pull/27662#issuecomment-1547801748)
> The only non-EOL operating system still shipping with g++-8 is CentOS 8.
> seems fine for 26.0.
Considering https://www.centos.org/centos-linux-eol/:
> CentOS Linux 8 will reach End Of Life (EOL) on December 31st, 2021.
I agree.
Concept ACK.
🚀 fanquake merged a pull request: "doc, test: Document steps to reproduce TSan warning for `libdb`"
(https://github.com/bitcoin/bitcoin/pull/27661)
(https://github.com/bitcoin/bitcoin/pull/27661)
💬 MarcoFalke commented on issue "Drop support for g++-8?":
(https://github.com/bitcoin/bitcoin/issues/27537#issuecomment-1547831477)
Looks like one bug I presumed to be fixed in g++9 is affecting all versions of g++, even 13.1. Maybe someone should report that upstream, because it is valid C++ code and works in clang: https://godbolt.org/z/fM39z38nc
(https://github.com/bitcoin/bitcoin/issues/27537#issuecomment-1547831477)
Looks like one bug I presumed to be fixed in g++9 is affecting all versions of g++, even 13.1. Maybe someone should report that upstream, because it is valid C++ code and works in clang: https://godbolt.org/z/fM39z38nc
💬 brunoerg commented on pull request "fuzz: wallet, add target for `fees`":
(https://github.com/bitcoin/bitcoin/pull/27647#discussion_r1193839090)
Seems good, thanks
(https://github.com/bitcoin/bitcoin/pull/27647#discussion_r1193839090)
Seems good, thanks
💬 brunoerg commented on pull request "fuzz: wallet, add target for `fees`":
(https://github.com/bitcoin/bitcoin/pull/27647#discussion_r1193840138)
Nice, make sense having it in `initialize_setup`, if I have to retouch the code again I can do it.
(https://github.com/bitcoin/bitcoin/pull/27647#discussion_r1193840138)
Nice, make sense having it in `initialize_setup`, if I have to retouch the code again I can do it.
💬 brunoerg commented on pull request "fuzz: wallet, add target for `fees`":
(https://github.com/bitcoin/bitcoin/pull/27647#issuecomment-1547866703)
Force-pushed addressing @MarcoFalke's review.
- Remove cast from `chainstate` - https://github.com/bitcoin/bitcoin/pull/27647#discussion_r1193099662
- Code around `FeeCalculation` has been simplified - https://github.com/bitcoin/bitcoin/pull/27647#discussion_r1193100074
(https://github.com/bitcoin/bitcoin/pull/27647#issuecomment-1547866703)
Force-pushed addressing @MarcoFalke's review.
- Remove cast from `chainstate` - https://github.com/bitcoin/bitcoin/pull/27647#discussion_r1193099662
- Code around `FeeCalculation` has been simplified - https://github.com/bitcoin/bitcoin/pull/27647#discussion_r1193100074