💬 fanquake commented on pull request "depends: build expat with CMake":
(https://github.com/bitcoin/bitcoin/pull/29878#issuecomment-2057124280)
Looks like there could be issues with expat, CMake and 32-bit builds:
* https://cirrus-ci.com/task/6695396838211584
* https://cirrus-ci.com/task/5850971908079616

💬 achow101 commented on issue "IBD performance regression in 27.0rc1 on Windows":
(https://github.com/bitcoin/bitcoin/issues/29785#issuecomment-2057136484)
> Did someone with Windows try to enable the benchmarks in guix and compile them, and run them?

Yes, I did that. I could not find any benchmark that had a significant difference.

💬 fjahr commented on pull request "Safegcd-based modular inverses in MuHash3072":
(https://github.com/bitcoin/bitcoin/pull/21590#issuecomment-2057150785)
@sipa The description still lists "Add more tests" as an open todo. Did you still want to address this before this could be merged? I think we have ok-ish coverage of MuHash but if you think something should be added could you tell us what you had in mind? Are there maybe test vectors we can port over?

💬 sipa commented on pull request "Safegcd-based modular inverses in MuHash3072":
(https://github.com/bitcoin/bitcoin/pull/21590#issuecomment-2057153942)
@fjahr I've dropped the TODO. Feel free to contribute tests of course if you feel that's helpful.

💬 instagibbs commented on pull request "fuzz: explicitly cap the vsize of RBFs for diagram checks":
(https://github.com/bitcoin/bitcoin/pull/29879#discussion_r1566001308)
this is hopefully a conservative over-estimate since clusters should only be 101kvB total each

💬 instagibbs commented on pull request "test: p2p: add test for rejected tx request logic (`m_recent_rejects` filter)":
(https://github.com/bitcoin/bitcoin/pull/29827#discussion_r1565931380)
is this strictly necessary since the mempool is empty?

fine to me to add as belt and suspenders either way

👍 instagibbs approved a pull request: "test: p2p: add test for rejected tx request logic (`m_recent_rejects` filter)"
(https://github.com/bitcoin/bitcoin/pull/29827#pullrequestreview-2001340821)
ACK 01b17b62cbeb168629d17eb2a54d547628e16759

💬 instagibbs commented on pull request "test: p2p: add test for rejected tx request logic (`m_recent_rejects` filter)":
(https://github.com/bitcoin/bitcoin/pull/29827#discussion_r1565964264)
I'll punt to future work(TM) but I was wondering if we could just generate an "ephemeral" miniwallet inside `fill_mempool` if the resulting utxos from filling the mempool aren't necessary for the rest of the test?

💬 achow101 commented on pull request "doc: update release-process.md":
(https://github.com/bitcoin/bitcoin/pull/29645#discussion_r1566005991)
I think we've decided to start uploading all build artifacts, including the debug files, so this sentence and section below can be removed.

💬 achow101 commented on pull request "doc: update release-process.md":
(https://github.com/bitcoin/bitcoin/pull/29645#discussion_r1566010280)
If we're going to mention torrent generation, maybe also mention the OpenTimestamps file too? Both are generated automatically on the server now.

💬 maflcko commented on issue "IBD performance regression in 27.0rc1 on Windows":
(https://github.com/bitcoin/bitcoin/issues/29785#issuecomment-2057185867)
Thanks for confirming. If the benchmarks can not show a difference for someone who could reproduce, bisecting guix builds may be the only option left to debug this, but that will probably take some time.

💬 achow101 commented on pull request "[27.x] More backports and finalize":
(https://github.com/bitcoin/bitcoin/pull/29780#issuecomment-2057187851)
ACK 910e3e8728b258a38d38f8f9ddf6b23406e8d5ce

💬 sr-gi commented on pull request "Simplify network-adjusted time warning logic":
(https://github.com/bitcoin/bitcoin/pull/29623#discussion_r1566015813)
I don't think this was covered (or it may have been regressed)

💬 sr-gi commented on pull request "Simplify network-adjusted time warning logic":
(https://github.com/bitcoin/bitcoin/pull/29623#issuecomment-2057192875)
Re-ACK [c6be144](https://github.com/bitcoin/bitcoin/pull/29623/commits/c6be144c4b774a03a8bcab5a165768cf81e9b06b)

👍 alfonsoromanz approved a pull request: "test: remove duplicated ban test"
(https://github.com/bitcoin/bitcoin/pull/29688#pullrequestreview-2001500817)
ACK e30e8625bbc42045b8b757a8d7e80c20cc61cebf

💬 laanwj commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#issuecomment-2057202451)
i tried some old wallets, and there was a problem with parsing the headers of overflow pages (which have btree level 0), the following makes the check pass and adds a further check that overflow records correspond to overflow pages:
```patch
diff --git a/src/wallet/migrate.cpp b/src/wallet/migrate.cpp
index be54b58f67c9bc961f8df306fe4bc0bf810b41f3..1c4ce1c1ed8d44221626613df2cd0d4712fc5c85 100644
--- a/src/wallet/migrate.cpp
+++ b/src/wallet/migrate.cpp
@@ -361,7 +361,7 @@ public:

...

💬 sipa commented on pull request "fuzz: explicitly cap the vsize of RBFs for diagram checks":
(https://github.com/bitcoin/bitcoin/pull/29879#issuecomment-2057203936)
I'm not convinced about this approach. A nice thing about fuzz test is that they can be written to exercise scenarios which go beyond what matters in practice, as more extreme values may trigger bugs more easily (and those bugs may affect realistic cases too, but be harder to trigger there).

More specifically, in this case it seems the issue is the code under test not working if the sum of sizes exceeds 2^31. The standard transaction size shouldn't matter for this code, so I'd prefer to restr
...

💬 brunoerg commented on pull request "test: check disconnection when sending sendaddrv2 after verack":
(https://github.com/bitcoin/bitcoin/pull/29699#discussion_r1566030227)
Yes :)

💬 theStack commented on pull request "test: p2p: add test for rejected tx request logic (`m_recent_rejects` filter)":
(https://github.com/bitcoin/bitcoin/pull/29827#discussion_r1566039065)
> is this strictly necessary since the mempool is empty?

Good point. Without that change the test fails, but not for the reason I thought it does. I think the problem is that MiniWallet is not aware of restarts between nodes and still holds mempool UTXOs that it tries to spend. Will force-push in a bit with a fix that does a `self.wallet.rescan_utxos()` after restarts, which should tackle the problem as the root.

💬 stickies-v commented on pull request "Simplify network-adjusted time warning logic":
(https://github.com/bitcoin/bitcoin/pull/29623#discussion_r1566042673)
Sorry, I think it must have regressed with [this update in the next line](https://github.com/bitcoin/bitcoin/compare/2ef71c73582be554e565ada3f8a6ca77c2cd79f1..5362db10ab6d1d6697807c8f8494a2fc5bf41ff8#diff-6875de769e90cec84d2e8a9c1b962cdbcda44d870d42e4215827e599e11e90e3R3687) - will fix if I have to retouch.