💬 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?

💬 MarcoFalke commented on pull request "Add support for "partial" fuzzers that indicate usefulness":
(https://github.com/bitcoin/bitcoin/pull/27552#discussion_r1182981008)
Also, might be good to make the return type type-safe with a C++ `enum class` to avoid confusion due to under-documentation and accidental typo bugs?

💬 MarcoFalke commented on pull request "fuzz: BIP 42, BIP 30, CVE-2018-17144":
(https://github.com/bitcoin/bitcoin/pull/17860#issuecomment-1532052827)
Rebased and partly rewritten

🤔 TheCharlatan requested changes to a pull request: "contrib: add ELF OS ABI check to symbol-check.py"
(https://github.com/bitcoin/bitcoin/pull/26953#pullrequestreview-1409806683)
Concept ACK

Seems worthwhile to check these in case we accidentally increment the ABI version.

💬 TheCharlatan commented on pull request "contrib: add ELF OS ABI check to symbol-check.py":
(https://github.com/bitcoin/bitcoin/pull/26953#discussion_r1182998138)
I tested all these versions manually first by reading them with `readelf --notes bitcoin-cli` and then running this script against the 24.0.1 release binaries. All the others matched, but the power pc little endian version gave me `3.2.0`. Against which binaries did you check these?

💬 fanquake commented on pull request "contrib: add ELF OS ABI check to symbol-check.py":
(https://github.com/bitcoin/bitcoin/pull/26953#discussion_r1183002436)
This is against master (binaries produced with a Guix build). The symbol/security scripts are (only) expected to work with the version of the source they are shipped with. In this case, we are using a newer version of glibc to produce the binaries in master, hence the difference. Think I also alluded to this in a comment above.

💬 TheCharlatan commented on pull request "contrib: add ELF OS ABI check to symbol-check.py":
(https://github.com/bitcoin/bitcoin/pull/26953#discussion_r1183005546)
Ah right, all the better that we catch these now. I found a recent guix build and it checks out there.

👍 TheCharlatan approved a pull request: "contrib: add ELF OS ABI check to symbol-check.py"
(https://github.com/bitcoin/bitcoin/pull/26953#pullrequestreview-1409819053)
ACK 65ba8a79a2919a0bd89f2f2d981e072d4f2f549d

👍 theStack approved a pull request: "test: Simplify feature_fastprune.py"
(https://github.com/bitcoin/bitcoin/pull/27553#pullrequestreview-1409826743)
ACK fa17767154e21e9ed00782a9e4cf9a3d1d66f5d1

Checked that the size of the generated blocks in master and PR are about equal by adding
`print(len(bytes.fromhex(self.nodes[0].getblock(self.nodes[0].getbestblockhash(), 0))))`
at the end of the test (interestingly, in the PR the block is exactly 1 byte larger [65904 vs. 65903] but that doesn't matter of course).