💬 brunoerg commented on pull request "fuzz: addrman, add coverage for `network` field in `Select()`, `Size()` and `GetAddr()`":
(https://github.com/bitcoin/bitcoin/pull/27549#issuecomment-1531731272)
Force-pushed addressing https://github.com/bitcoin/bitcoin/pull/27549#pullrequestreview-1408940288

Added coverage for `Size()` and `GetAddr()`, since I got code from https://github.com/bitcoin/bitcoin/commit/8f91e5898b3571e0802f2197981c54dda9ee71fa, I added @mzumsande and @amitiuttarwar as co-authors. PR description has been updated.

💬 real-or-random commented on pull request "Add ASM optimizations for MuHash3072":
(https://github.com/bitcoin/bitcoin/pull/19181#issuecomment-1531736632)
> but I would also be curious if there is another way to tell GCC to do the same optimizations clang seems to be doing.


I played around with this a bit, and I don't see any obvious trick to make that work. If someone else wants to give it a try, https://gcc.godbolt.org/z/hhGfeEoKq could be a nice starting point.

💬 hebasto commented on pull request "test: Treat `bitcoin-wallet` binary in the same way as others":
(https://github.com/bitcoin/bitcoin/pull/27554#discussion_r1182788522)
> What about making this a function to reduce code bloat while touching this?

Done.

💬 laanwj commented on pull request "contrib: add ELF OS ABI check to symbol-check.py":
(https://github.com/bitcoin/bitcoin/pull/26953#issuecomment-1531794631)
Code review ACK 65ba8a79a2919a0bd89f2f2d981e072d4f2f549d

👍 MarcoFalke approved a pull request: "test: Treat `bitcoin-wallet` binary in the same way as others"
(https://github.com/bitcoin/bitcoin/pull/27554#pullrequestreview-1409478431)
lgtm

💬 MarcoFalke commented on pull request "test: Treat `bitcoin-wallet` binary in the same way as others":
(https://github.com/bitcoin/bitcoin/pull/27554#discussion_r1182791775)
```suggestion
attribute_name, env_variable_name = binaries[binary]
```

nit

🤔 instagibbs reviewed a pull request: "validate package transactions with their in-package ancestor sets"
(https://github.com/bitcoin/bitcoin/pull/26711#pullrequestreview-1262442548)
test issue causing failure

💬 instagibbs commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1182761583)
this should assert that package is non-empty

💬 instagibbs commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1182797698)
ordering between `package[i]` and `packageified.Txns()[i]` doesn't seem to be stable? The above shuffle is only "undone" by sorting by number of in-package ancestors when constructing the `AncestorPackage`

💬 instagibbs commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1182768590)
```suggestion
for (int parent_idx{0}; parent_idx < 99; ++parent_idx) {
```

💬 pablomartin4btc commented on pull request "blockstorage: do not flush block to disk if it is already there":
(https://github.com/bitcoin/bitcoin/pull/27039#issuecomment-1531882383)
re-ACK 46cca252e30f54b06df011d268ef1b8a48b076e7.

💬 hebasto commented on pull request "test: Treat `bitcoin-wallet` binary in the same way as others":
(https://github.com/bitcoin/bitcoin/pull/27554#discussion_r1182883849)
Thanks! Updated.

📝 furszy opened a pull request: "wallet: fix deadlock in bdb read write operation"
(https://github.com/bitcoin/bitcoin/pull/27556)
Decoupled from #26644 so it can closed in favor of #26715.

Basically, with bdb, we can't make a write operation while we are traversing the db, because these two operations are performed in different txn contexts. Causing a deadlock.

Added coverage by using `EraseRecords()` which is the simplest function that executes this process.

To replicate it, need bdb support and drop the first commit. The test will run forever.

💬 jamesob commented on pull request "fuzz: BIP 42, BIP 30, CVE-2018-17144":
(https://github.com/bitcoin/bitcoin/pull/17860#issuecomment-1531946556)
Concept ACK, will review this week

📝 pinheadmz opened a pull request: "net: call getaddrinfo() in detachable thread to prevent stalling"
(https://github.com/bitcoin/bitcoin/pull/27557)
Closes https://github.com/bitcoin/bitcoin/issues/16778
Replaces https://github.com/bitcoin/bitcoin/pull/27505

The library call `getaddrinfo()` has no internal timeout and depending on the user's system may wait indefinitely for a response when looking up a hostname in the DNS. By making that call in a separate thread, we can abandon it completely after some timeout (currently in this PR, 2 seconds).

TODO:

- [ ] We could make the polling loop interruptible but I'm not sure the best appr
...

💬 pinheadmz commented on pull request "net: use interruptible async getaddrinfo wrapper from libevent for DNS":
(https://github.com/bitcoin/bitcoin/pull/27505#issuecomment-1531951962)
Closing this now for alternative in https://github.com/bitcoin/bitcoin/pull/27557 which just calls `getaddrinfo()` from a detachable thread. No libevent at all and, yeeeeeesh it is way simpler.

✅ pinheadmz closed a pull request: "net: use interruptible async getaddrinfo wrapper from libevent for DNS"
(https://github.com/bitcoin/bitcoin/pull/27505)

⚠️ vostrnad opened an issue: "Coinstats index corrupted after invalidateblock and clean shutdown"
(https://github.com/bitcoin/bitcoin/issues/27558)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

I tried running `invalidateblock` on a block a few hundred blocks behind the chain tip. While this command was running, I also ran `stop` and the node apparently shut down cleanly:

<details>
<summary>Last logs before shutdown</summary>

```
2023-05-02T18:31:17Z UpdateTip: new best=00000000000000000003518599da068e68cca93b3d74bcc25e240229f7a86293 height=787853 version=0x23608000 log2_
...

💬 achow101 commented on pull request "wallet: bugfix, 'wallet_load_ckey' unit test fails with bdb":
(https://github.com/bitcoin/bitcoin/pull/26644#issuecomment-1532015825)
Would it be easier to do this on top of #26715?

💬 instagibbs commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1182968218)
under what situations does the "representative tx" not get filled, and why is `TX_MISSING_INPUTS` the right value to set?