Bitcoin Core Github
44 subscribers
121K links
Download Telegram
hebasto closed a pull request: "test: Drop `deadlock:libdb` TSan suppression"
(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?
📝 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).
💬 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
💬 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.
💬 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.
💬 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
...
📝 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
💬 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.

?
💬 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.
💬 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
💬 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?
💬 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.
🚀 fanquake merged a pull request: "doc, test: Document steps to reproduce TSan warning for `libdb`"
(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
💬 brunoerg commented on pull request "fuzz: wallet, add target for `fees`":
(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.
💬 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
💬 MarcoFalke commented on pull request "fuzz: wallet, add target for `fees`":
(https://github.com/bitcoin/bitcoin/pull/27647#discussion_r1193861398)
Looks like you forgot to address this?
📝 fanquake opened a pull request: "[23.2] Final Changes"
(https://github.com/bitcoin/bitcoin/pull/27663)
Final changes for v23.2. I don't expect any futher backports, or the need to prolong the rc, as the changes here are fairly minimal.

PR for bitcoincore.org is here: https://github.com/bitcoin-core/bitcoincore.org/pull/969