💬 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
(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
(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?
(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.
(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.
(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.
(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
(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)
(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)
(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
(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:
...
(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
...
(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 :)
(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.
(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.
(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.
💬 alfonsoromanz commented on pull request "Feature: Use different datadirs for different signets":
(https://github.com/bitcoin/bitcoin/pull/29838#issuecomment-2057231800)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/29838#issuecomment-2057231800)
Concept ACK
💬 instagibbs commented on pull request "fuzz: explicitly cap the vsize of RBFs for diagram checks":
(https://github.com/bitcoin/bitcoin/pull/29879#issuecomment-2057240637)
> there is no strict need to tie it to standard sizes.
Pretty sure we can overflow if we allow ~1MvB transactions, as `10,000 * 1,000,000 > 2^31`
Made a more direct limiting attempt. I will squash if it is preferred
(https://github.com/bitcoin/bitcoin/pull/29879#issuecomment-2057240637)
> there is no strict need to tie it to standard sizes.
Pretty sure we can overflow if we allow ~1MvB transactions, as `10,000 * 1,000,000 > 2^31`
Made a more direct limiting attempt. I will squash if it is preferred
🤔 BrandonOdiwuor reviewed a pull request: "Add Signet launch shortcut for Windows"
(https://github.com/bitcoin/bitcoin/pull/26334#pullrequestreview-2001547908)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/26334#pullrequestreview-2001547908)
Concept ACK
📝 fanquake opened a pull request: "depends: build FreeType with CMake"
(https://github.com/bitcoin/bitcoin/pull/29880)
Bumps FreeType from [`2.11.0`](https://sourceforge.net/projects/freetype/files/freetype2/2.11.0/) to [`2.13.2`](https://sourceforge.net/projects/freetype/files/freetype2/2.13.2/) (currently untested/reviewed).
Switches Freetype to be built with CMake.
(https://github.com/bitcoin/bitcoin/pull/29880)
Bumps FreeType from [`2.11.0`](https://sourceforge.net/projects/freetype/files/freetype2/2.11.0/) to [`2.13.2`](https://sourceforge.net/projects/freetype/files/freetype2/2.13.2/) (currently untested/reviewed).
Switches Freetype to be built with CMake.
🚀 fanquake merged a pull request: "[27.x] More backports and finalize"
(https://github.com/bitcoin/bitcoin/pull/29780)
(https://github.com/bitcoin/bitcoin/pull/29780)