💬 MarcoFalke commented on pull request "net processing: avoid serving non-announced txs as a result of a MEMPOOL message":
(https://github.com/bitcoin/bitcoin/pull/27602#discussion_r1193582239)
Side note: The 50k max inv size also seems to overflow, which is probably another source for false negatives, depending on the use case.
<img src=https://user-images.githubusercontent.com/19157360/235448790-6a1448bd-64b3-4c41-89d3-1ca5bf0cf76d.png></img>
Image stolen from the comment https://github.com/bitcoin/bitcoin/pull/27426#issuecomment-1529678174 by 0xB10C .
(https://github.com/bitcoin/bitcoin/pull/27602#discussion_r1193582239)
Side note: The 50k max inv size also seems to overflow, which is probably another source for false negatives, depending on the use case.
<img src=https://user-images.githubusercontent.com/19157360/235448790-6a1448bd-64b3-4c41-89d3-1ca5bf0cf76d.png></img>
Image stolen from the comment https://github.com/bitcoin/bitcoin/pull/27426#issuecomment-1529678174 by 0xB10C .
📝 fanquake opened a pull request: "[24.1] Final Changes"
(https://github.com/bitcoin/bitcoin/pull/27660)
Final changes for `v24.1`.
PR for bitcoincore.org is here: https://github.com/bitcoin-core/bitcoincore.org/pull/968.
(https://github.com/bitcoin/bitcoin/pull/27660)
Final changes for `v24.1`.
PR for bitcoincore.org is here: https://github.com/bitcoin-core/bitcoincore.org/pull/968.
👍 hebasto approved a pull request: "[24.1] Final Changes"
(https://github.com/bitcoin/bitcoin/pull/27660#pullrequestreview-1426164333)
ACK 89a5a416deac060fed8c21d4381c8f59da4f4187, I have reviewed the code and it looks OK.
(https://github.com/bitcoin/bitcoin/pull/27660#pullrequestreview-1426164333)
ACK 89a5a416deac060fed8c21d4381c8f59da4f4187, I have reviewed the code and it looks OK.
👍 TheCharlatan approved a pull request: "ci: Remove CI_EXEC bloat"
(https://github.com/bitcoin/bitcoin/pull/27616#pullrequestreview-1426207043)
ACK fa01c3c59cbe28be0751c2956609907ecfbcbe49
(https://github.com/bitcoin/bitcoin/pull/27616#pullrequestreview-1426207043)
ACK fa01c3c59cbe28be0751c2956609907ecfbcbe49
🚀 fanquake merged a pull request: "ci: Remove CI_EXEC bloat"
(https://github.com/bitcoin/bitcoin/pull/27616)
(https://github.com/bitcoin/bitcoin/pull/27616)
👍 stickies-v approved a pull request: "[24.1] Final Changes"
(https://github.com/bitcoin/bitcoin/pull/27660#pullrequestreview-1426234284)
ACK 89a5a416deac060fed8c21d4381c8f59da4f4187
(https://github.com/bitcoin/bitcoin/pull/27660#pullrequestreview-1426234284)
ACK 89a5a416deac060fed8c21d4381c8f59da4f4187
🚀 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