💬 sipa commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r2015474923)
Right. The difference between OPTIMAL and ACCEPTABLE is not observable as far as the interface is concerned, because it only promises a best effort.
There is an operational difference, though: `DoWork()` will not bother putting more effort into OPTIMAL clusters.
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r2015474923)
Right. The difference between OPTIMAL and ACCEPTABLE is not observable as far as the interface is concerned, because it only promises a best effort.
There is an operational difference, though: `DoWork()` will not bother putting more effort into OPTIMAL clusters.
💬 sipa commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r2015477724)
That finds **a** connected component, but not the one that this particular transaction is in. I guess it could be refactored to support that too.
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r2015477724)
That finds **a** connected component, but not the one that this particular transaction is in. I guess it could be refactored to support that too.
💬 sipa commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r2015485894)
Well, `pick_fn` picks based on fuzz input, which is arbitrary, but definitely not random. There is a point to be made that we should leave it up to the fuzzer entirely here, but I felt like adding actual random ordering might more easily cover strange orderings. Probably doesn't add much, but also does not hurt much.
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r2015485894)
Well, `pick_fn` picks based on fuzz input, which is arbitrary, but definitely not random. There is a point to be made that we should leave it up to the fuzzer entirely here, but I felt like adding actual random ordering might more easily cover strange orderings. Probably doesn't add much, but also does not hurt much.
💬 sipa commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r2015486460)
Fixed in #32151
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r2015486460)
Fixed in #32151
💬 sipa commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r2015494404)
Where does the `MAX_TRANSACTIONS` come from? It should limit the size of `removed` to 100. This was added when the fuzzer ran for much longer (up to 1000 iterations, I think), and I wanted to avoid it just creating and removing transactions. It currently doesn't do much anymore with the lower iteration count.
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r2015494404)
Where does the `MAX_TRANSACTIONS` come from? It should limit the size of `removed` to 100. This was added when the fuzzer ran for much longer (up to 1000 iterations, I think), and I wanted to avoid it just creating and removing transactions. It currently doesn't do much anymore with the lower iteration count.
💬 sipa commented on pull request "cluster mempool: add txgraph diagrams/mining/eviction":
(https://github.com/bitcoin/bitcoin/pull/31444#issuecomment-2756541543)
Rebased after merge of #31363, and on top of #32151.
(https://github.com/bitcoin/bitcoin/pull/31444#issuecomment-2756541543)
Rebased after merge of #31363, and on top of #32151.
💬 sipa commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#issuecomment-2756541999)
Rebased after merge of #31363, and on top of #32151.
(https://github.com/bitcoin/bitcoin/pull/31553#issuecomment-2756541999)
Rebased after merge of #31363, and on top of #32151.
💬 theStack commented on pull request "test: avoid treating hash results as integers":
(https://github.com/bitcoin/bitcoin/pull/32050#discussion_r2015670851)
Note that both the `rehash` and `getwtxid` methods return a hex string. The former returns `self.hash`, which is set in `.calc_sha256` as follows:
https://github.com/bitcoin/bitcoin/blob/f1d129d96340503ec5f6b347c2fdf6a6901b1f7e/test/functional/test_framework/messages.py#L682
Another example where both of these methods are called and directly compared:
https://github.com/bitcoin/bitcoin/blob/f1d129d96340503ec5f6b347c2fdf6a6901b1f7e/test/functional/mempool_accept.py#L416
(Something for a d
...
(https://github.com/bitcoin/bitcoin/pull/32050#discussion_r2015670851)
Note that both the `rehash` and `getwtxid` methods return a hex string. The former returns `self.hash`, which is set in `.calc_sha256` as follows:
https://github.com/bitcoin/bitcoin/blob/f1d129d96340503ec5f6b347c2fdf6a6901b1f7e/test/functional/test_framework/messages.py#L682
Another example where both of these methods are called and directly compared:
https://github.com/bitcoin/bitcoin/blob/f1d129d96340503ec5f6b347c2fdf6a6901b1f7e/test/functional/mempool_accept.py#L416
(Something for a d
...
💬 theStack commented on pull request "test: avoid treating hash results as integers":
(https://github.com/bitcoin/bitcoin/pull/32050#discussion_r2015671169)
Will do if I have to retouch.
(https://github.com/bitcoin/bitcoin/pull/32050#discussion_r2015671169)
Will do if I have to retouch.
💬 Eunovo commented on pull request "[IBD] flush UTXOs in bigger batches":
(https://github.com/bitcoin/bitcoin/pull/31645#issuecomment-2756833986)
> The memory difference is measurable and non-negligible (~200MiB, seems the buffer may be copied 4 times), especially for small memory usecases.
> I have opened [google/leveldb#1259](https://github.com/google/leveldb/pull/1259) to enable pre-sizing the memory, which might help us in reducing the spikes slightly.
@l0rinc you can also check `leveldb.approximate-memory-usage` [use db.GetProperty](https://github.com/google/leveldb/blob/main/include/leveldb/db.h#L122) before and after batch wri
...
(https://github.com/bitcoin/bitcoin/pull/31645#issuecomment-2756833986)
> The memory difference is measurable and non-negligible (~200MiB, seems the buffer may be copied 4 times), especially for small memory usecases.
> I have opened [google/leveldb#1259](https://github.com/google/leveldb/pull/1259) to enable pre-sizing the memory, which might help us in reducing the spikes slightly.
@l0rinc you can also check `leveldb.approximate-memory-usage` [use db.GetProperty](https://github.com/google/leveldb/blob/main/include/leveldb/db.h#L122) before and after batch wri
...
💬 theStack commented on pull request "contrib: refactor: dedup deserialization routines in utxo-to-sqlite script":
(https://github.com/bitcoin/bitcoin/pull/32116#issuecomment-2756841655)
@laanwj: Fair points! My reasoning to consider this (somewhat hacky) "import code from test framework" approach as acceptable was the fact that we already do it in other contrib scripts as well (e.g. the [signet miner](https://github.com/bitcoin/bitcoin/blob/f1d129d96340503ec5f6b347c2fdf6a6901b1f7e/contrib/signet/miner#L17-L19)), but yeah, that doesn't necessarily mean it's a very good idea.
>> My preference would be to have them present once only instead of seeing the implementation in multi
...
(https://github.com/bitcoin/bitcoin/pull/32116#issuecomment-2756841655)
@laanwj: Fair points! My reasoning to consider this (somewhat hacky) "import code from test framework" approach as acceptable was the fact that we already do it in other contrib scripts as well (e.g. the [signet miner](https://github.com/bitcoin/bitcoin/blob/f1d129d96340503ec5f6b347c2fdf6a6901b1f7e/contrib/signet/miner#L17-L19)), but yeah, that doesn't necessarily mean it's a very good idea.
>> My preference would be to have them present once only instead of seeing the implementation in multi
...
💬 theStack commented on pull request "contrib: refactor: dedup deserialization routines in utxo-to-sqlite script":
(https://github.com/bitcoin/bitcoin/pull/32116#discussion_r2015733904)
Good find, will do if we end up still following the current approach of the PR (now less likely in light of the discussion triggered by https://github.com/bitcoin/bitcoin/pull/32116#issuecomment-2754401102).
(https://github.com/bitcoin/bitcoin/pull/32116#discussion_r2015733904)
Good find, will do if we end up still following the current approach of the PR (now less likely in light of the discussion triggered by https://github.com/bitcoin/bitcoin/pull/32116#issuecomment-2754401102).
💬 maflcko commented on pull request "Follow-ups for txgraph #31363":
(https://github.com/bitcoin/bitcoin/pull/32151#discussion_r2015764185)
If you want to fix typos in the touched files, there may be a bit more:
```
[03:14:31.857] src/txgraph.cpp:342: Entrys ==> Entries, Entry
[03:14:31.857] src/txgraph.cpp:415: Entrys ==> Entries, Entry
[03:14:31.857] src/txgraph.cpp:512: Entrys ==> Entries, Entry
[03:14:31.857] src/txgraph.cpp:1944: occured ==> occurred
[03:14:31.857] src/txgraph.h:102: commiting ==> committing
(https://github.com/bitcoin/bitcoin/pull/32151#discussion_r2015764185)
If you want to fix typos in the touched files, there may be a bit more:
```
[03:14:31.857] src/txgraph.cpp:342: Entrys ==> Entries, Entry
[03:14:31.857] src/txgraph.cpp:415: Entrys ==> Entries, Entry
[03:14:31.857] src/txgraph.cpp:512: Entrys ==> Entries, Entry
[03:14:31.857] src/txgraph.cpp:1944: occured ==> occurred
[03:14:31.857] src/txgraph.h:102: commiting ==> committing
🚀 fanquake merged a pull request: "lint: Remove needless borrow to fix Clippy warning"
(https://github.com/bitcoin/bitcoin/pull/32144)
(https://github.com/bitcoin/bitcoin/pull/32144)
💬 maflcko commented on pull request "test: fix intermittent timeout in p2p_ibd_stalling.py":
(https://github.com/bitcoin/bitcoin/pull/32148#issuecomment-2756969293)
lgtm ACK 9f35d4d070b2097259c0bfada391d8ef847e256e
(https://github.com/bitcoin/bitcoin/pull/32148#issuecomment-2756969293)
lgtm ACK 9f35d4d070b2097259c0bfada391d8ef847e256e
💬 maflcko commented on pull request "test: fix intermittent timeout in p2p_ibd_stalling.py":
(https://github.com/bitcoin/bitcoin/pull/32148#issuecomment-2756973294)
(This is not a clean backport and should be rare enough to not matter, so can probably skip the backport here)
(https://github.com/bitcoin/bitcoin/pull/32148#issuecomment-2756973294)
(This is not a clean backport and should be rare enough to not matter, so can probably skip the backport here)
💬 maflcko commented on pull request "fuzz: extract unsequenced operations with side-effects":
(https://github.com/bitcoin/bitcoin/pull/32141#issuecomment-2756976842)
lgtm ACK b1de59e8965354fff5a149bc0fe61ed0704aea7a
(https://github.com/bitcoin/bitcoin/pull/32141#issuecomment-2756976842)
lgtm ACK b1de59e8965354fff5a149bc0fe61ed0704aea7a
🚀 fanquake merged a pull request: "build: Remove bitness suffix from Windows installer"
(https://github.com/bitcoin/bitcoin/pull/32132)
(https://github.com/bitcoin/bitcoin/pull/32132)
💬 maflcko commented on issue "Failure to run Fuzz tests when running with corpus":
(https://github.com/bitcoin/bitcoin/issues/32089#issuecomment-2756991853)
Does the issue happen with all clang versions from brew? (clang-16 to clang-20)?
Does the issue happen when compiling clang from source?
(https://github.com/bitcoin/bitcoin/issues/32089#issuecomment-2756991853)
Does the issue happen with all clang versions from brew? (clang-16 to clang-20)?
Does the issue happen when compiling clang from source?
💬 fanquake commented on pull request "depends: set `CMAKE_*_COMPILER_TARGET` in toolchain":
(https://github.com/bitcoin/bitcoin/pull/31849#issuecomment-2757006492)
Guix Build (x86_64):
```bash
02c610ee1471cc2671588a999e5ef914547cd55352ddc084faf7a9a55d79098b guix-build-963355037fe7/output/aarch64-linux-gnu/SHA256SUMS.part
11c1779525e0ebfbb55badd50c818cc02ac85a113077cc6074e6d11d4bd98677 guix-build-963355037fe7/output/aarch64-linux-gnu/bitcoin-963355037fe7-aarch64-linux-gnu-debug.tar.gz
5ef64bbfadca863a09fada8da1b6cf4543c318bb4208f67c7473718c5dec6f55 guix-build-963355037fe7/output/aarch64-linux-gnu/bitcoin-963355037fe7-aarch64-linux-gnu.tar.gz
80beeb5
...
(https://github.com/bitcoin/bitcoin/pull/31849#issuecomment-2757006492)
Guix Build (x86_64):
```bash
02c610ee1471cc2671588a999e5ef914547cd55352ddc084faf7a9a55d79098b guix-build-963355037fe7/output/aarch64-linux-gnu/SHA256SUMS.part
11c1779525e0ebfbb55badd50c818cc02ac85a113077cc6074e6d11d4bd98677 guix-build-963355037fe7/output/aarch64-linux-gnu/bitcoin-963355037fe7-aarch64-linux-gnu-debug.tar.gz
5ef64bbfadca863a09fada8da1b6cf4543c318bb4208f67c7473718c5dec6f55 guix-build-963355037fe7/output/aarch64-linux-gnu/bitcoin-963355037fe7-aarch64-linux-gnu.tar.gz
80beeb5
...