👍 hebasto approved a pull request: "ci: Roll clang in test-each-commit task"
(https://github.com/bitcoin/bitcoin/pull/30060#pullrequestreview-2057221891)
re-ACK fa90ad23c0cb99bde305af156c978c066f7bacb8.
(https://github.com/bitcoin/bitcoin/pull/30060#pullrequestreview-2057221891)
re-ACK fa90ad23c0cb99bde305af156c978c066f7bacb8.
💬 maflcko commented on pull request "build: Enable `thread_local` for MinGW-w64 builds":
(https://github.com/bitcoin/bitcoin/pull/30099#issuecomment-2111844845)
utACK df879e5a91134a67ada3167ebff4e87f163b81a9
Seems reasonable to assume that this was a wine issue, or another issue that is now fixed.
(https://github.com/bitcoin/bitcoin/pull/30099#issuecomment-2111844845)
utACK df879e5a91134a67ada3167ebff4e87f163b81a9
Seems reasonable to assume that this was a wine issue, or another issue that is now fixed.
✅ fanquake closed a pull request: "init: Fixes for file descriptor accounting"
(https://github.com/bitcoin/bitcoin/pull/27539)
(https://github.com/bitcoin/bitcoin/pull/27539)
💬 fanquake commented on pull request "init: Fixes for file descriptor accounting":
(https://github.com/bitcoin/bitcoin/pull/27539#issuecomment-2111853223)
Closing for now, re #30065.
(https://github.com/bitcoin/bitcoin/pull/27539#issuecomment-2111853223)
Closing for now, re #30065.
💬 maflcko commented on issue "Restore wallet taking forever to load":
(https://github.com/bitcoin/bitcoin/issues/30108#issuecomment-2111855234)
Is it busy with IO or is the CPU busy? It could also be useful to attach gdb to get a stacktrace.
(https://github.com/bitcoin/bitcoin/issues/30108#issuecomment-2111855234)
Is it busy with IO or is the CPU busy? It could also be useful to attach gdb to get a stacktrace.
💬 fanquake commented on pull request "init: fixes file descriptor accounting":
(https://github.com/bitcoin/bitcoin/pull/30065#discussion_r1601187564)
I think now could also be the time to remove this FreeBSD workaround. It was needed because older versions of FreeBSD, used to ship with an old Clang (3.x). However we now require Clang 15+, and the effected version of FreeBSD 10.x, is long EOL.
(https://github.com/bitcoin/bitcoin/pull/30065#discussion_r1601187564)
I think now could also be the time to remove this FreeBSD workaround. It was needed because older versions of FreeBSD, used to ship with an old Clang (3.x). However we now require Clang 15+, and the effected version of FreeBSD 10.x, is long EOL.
🚀 fanquake merged a pull request: "ci: Roll clang in test-each-commit task"
(https://github.com/bitcoin/bitcoin/pull/30060)
(https://github.com/bitcoin/bitcoin/pull/30060)
💬 maflcko commented on pull request "[DO NOT MERGE] cmake: Migrate CI scripts to CMake-based build system -- WIP":
(https://github.com/bitcoin/bitcoin/pull/29790#issuecomment-2112000903)
If the CI uses the same patterns that end-users will use, that is ideal. If it does not, then it is fine, too.
(https://github.com/bitcoin/bitcoin/pull/29790#issuecomment-2112000903)
If the CI uses the same patterns that end-users will use, that is ideal. If it does not, then it is fine, too.
💬 TheCharlatan commented on pull request "[refactor] Check CTxMemPool options in ctor":
(https://github.com/bitcoin/bitcoin/pull/28830#issuecomment-2112006683)
Rebased d447bdcfb0e38353940e4a7fc89d09482d8d39c3 -> dacdb7962c5fef8db26f6fa31facb606165d1d1e ([mempoolArgs_6](https://github.com/TheCharlatan/bitcoin/tree/mempoolArgs_6) -> [mempoolArgs_7](https://github.com/TheCharlatan/bitcoin/tree/mempoolArgs_7), [compare](https://github.com/TheCharlatan/bitcoin/compare/mempoolArgs_6..mempoolArgs_7))
* Fixed conflict with https://github.com/bitcoin/bitcoin/pull/29086
(https://github.com/bitcoin/bitcoin/pull/28830#issuecomment-2112006683)
Rebased d447bdcfb0e38353940e4a7fc89d09482d8d39c3 -> dacdb7962c5fef8db26f6fa31facb606165d1d1e ([mempoolArgs_6](https://github.com/TheCharlatan/bitcoin/tree/mempoolArgs_6) -> [mempoolArgs_7](https://github.com/TheCharlatan/bitcoin/tree/mempoolArgs_7), [compare](https://github.com/TheCharlatan/bitcoin/compare/mempoolArgs_6..mempoolArgs_7))
* Fixed conflict with https://github.com/bitcoin/bitcoin/pull/29086
👍 brunoerg approved a pull request: "p2p: detect addnode cjdns peers in GetAddedNodeInfo()"
(https://github.com/bitcoin/bitcoin/pull/30085#pullrequestreview-2057456926)
crACK d0b047494c28381942c09d0cca45baa323bfcffc
(https://github.com/bitcoin/bitcoin/pull/30085#pullrequestreview-2057456926)
crACK d0b047494c28381942c09d0cca45baa323bfcffc
💬 josibake commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#discussion_r1601309470)
Looks like this warning is still failing the CI, perhaps just disabling the `unreachable-code` warning here is the simplest?
(https://github.com/bitcoin/bitcoin/pull/26606#discussion_r1601309470)
Looks like this warning is still failing the CI, perhaps just disabling the `unreachable-code` warning here is the simplest?
💬 TheCharlatan commented on pull request "kernel: De-globalize fReindex":
(https://github.com/bitcoin/bitcoin/pull/29817#issuecomment-2112044166)
Rebased b0594f6cf157e9bd71b2062cdf0aa65936fd2282 -> 46b9ec10dd238c5c1191bcdfcf06d1361cde15f1 ([noGlobalReindex_2](https://github.com/TheCharlatan/bitcoin/tree/noGlobalReindex_2) -> [noGlobalReindex_3](https://github.com/TheCharlatan/bitcoin/tree/noGlobalReindex_3), [compare](https://github.com/TheCharlatan/bitcoin/compare/noGlobalReindex_2..noGlobalReindex_3))
* Fixed conflict with #30083
(https://github.com/bitcoin/bitcoin/pull/29817#issuecomment-2112044166)
Rebased b0594f6cf157e9bd71b2062cdf0aa65936fd2282 -> 46b9ec10dd238c5c1191bcdfcf06d1361cde15f1 ([noGlobalReindex_2](https://github.com/TheCharlatan/bitcoin/tree/noGlobalReindex_2) -> [noGlobalReindex_3](https://github.com/TheCharlatan/bitcoin/tree/noGlobalReindex_3), [compare](https://github.com/TheCharlatan/bitcoin/compare/noGlobalReindex_2..noGlobalReindex_3))
* Fixed conflict with #30083
💬 maflcko commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#discussion_r1601323608)
Ok, it is a bit stupid to fight this compile warning, but apparently moving it out of the function scope fixes it :shrug:
It only happens with `-stdlib=libc++`.
See https://godbolt.org/z/vjEM7zjzb
(https://github.com/bitcoin/bitcoin/pull/26606#discussion_r1601323608)
Ok, it is a bit stupid to fight this compile warning, but apparently moving it out of the function scope fixes it :shrug:
It only happens with `-stdlib=libc++`.
See https://godbolt.org/z/vjEM7zjzb
📝 glozow opened a pull request: "refactor: TxDownloadManager"
(https://github.com/bitcoin/bitcoin/pull/30110)
Part of #27463.
Draft while #30000 is not merged, and because I am still smoothing out the interface, making sure the commits are clean, and writing more tests.
This modularizes transaction download logic into a `TxDownloadManager`. Transaction download logic refers to the process of deciding what transactions to request, download, and validate.[1]
All of the changes here should result in no behavior change.
There are several benefits to this interface, such as:
- Unit test coverage a
...
(https://github.com/bitcoin/bitcoin/pull/30110)
Part of #27463.
Draft while #30000 is not merged, and because I am still smoothing out the interface, making sure the commits are clean, and writing more tests.
This modularizes transaction download logic into a `TxDownloadManager`. Transaction download logic refers to the process of deciding what transactions to request, download, and validate.[1]
All of the changes here should result in no behavior change.
There are several benefits to this interface, such as:
- Unit test coverage a
...
💬 laanwj commented on pull request "util: avoid using thread_local variable that has a destructor":
(https://github.com/bitcoin/bitcoin/pull/30095#issuecomment-2112065633)
Concept ACK as i said here: https://github.com/bitcoin/bitcoin/issues/29952#issuecomment-2091354481
> I'd rather thread_local be safe to use (in all circumstance) on a platform, or we just not use it.
Sure, that'd be ideal, but handling destructors with TLS is *very hard* and easy to get wrong. It's not something we want to rely on.
Ideally we'd get rid of thread-local usage completely. But we've considered alternative solutions, and tracking some data until a thread disappears is hard
...
(https://github.com/bitcoin/bitcoin/pull/30095#issuecomment-2112065633)
Concept ACK as i said here: https://github.com/bitcoin/bitcoin/issues/29952#issuecomment-2091354481
> I'd rather thread_local be safe to use (in all circumstance) on a platform, or we just not use it.
Sure, that'd be ideal, but handling destructors with TLS is *very hard* and easy to get wrong. It's not something we want to rely on.
Ideally we'd get rid of thread-local usage completely. But we've considered alternative solutions, and tracking some data until a thread disappears is hard
...
💬 glozow commented on pull request "p2p: index TxOrphanage by wtxid, allow entries with same txid":
(https://github.com/bitcoin/bitcoin/pull/30000#discussion_r1601351165)
You're referring to the 1p1c logic, which isn't executed here (notice that all of the feerates are "normal" feerates).
Here, the orphans are reconsidered with `ProcessOrphanTx`, which is called at the top of `ProcessMessages`.
https://github.com/bitcoin/bitcoin/blob/dbb3113082a75035b14d20021036d2166171976e/src/net_processing.cpp#L5355-L5369
`ProcessMessages` is called for each peer in a random order
https://github.com/bitcoin/bitcoin/blob/dbb3113082a75035b14d20021036d2166171976e/src/net.cp
...
(https://github.com/bitcoin/bitcoin/pull/30000#discussion_r1601351165)
You're referring to the 1p1c logic, which isn't executed here (notice that all of the feerates are "normal" feerates).
Here, the orphans are reconsidered with `ProcessOrphanTx`, which is called at the top of `ProcessMessages`.
https://github.com/bitcoin/bitcoin/blob/dbb3113082a75035b14d20021036d2166171976e/src/net_processing.cpp#L5355-L5369
`ProcessMessages` is called for each peer in a random order
https://github.com/bitcoin/bitcoin/blob/dbb3113082a75035b14d20021036d2166171976e/src/net.cp
...
💬 glozow commented on pull request "p2p: index TxOrphanage by wtxid, allow entries with same txid":
(https://github.com/bitcoin/bitcoin/pull/30000#discussion_r1601354578)
It is rejected. The acceptance of the parent triggers the node to reconsider all children in orphanage (grep `AddChildrenToWorkset`. If the failure isn't `TX_MISSING_INPUTS`, it is removed from orphanage.
https://github.com/bitcoin/bitcoin/blob/dbb3113082a75035b14d20021036d2166171976e/src/net_processing.cpp#L3402-L3414
(https://github.com/bitcoin/bitcoin/pull/30000#discussion_r1601354578)
It is rejected. The acceptance of the parent triggers the node to reconsider all children in orphanage (grep `AddChildrenToWorkset`. If the failure isn't `TX_MISSING_INPUTS`, it is removed from orphanage.
https://github.com/bitcoin/bitcoin/blob/dbb3113082a75035b14d20021036d2166171976e/src/net_processing.cpp#L3402-L3414
🤔 willcl-ark reviewed a pull request: "contrib: use ENV flags in get_arch"
(https://github.com/bitcoin/bitcoin/pull/30074#pullrequestreview-2057646945)
post-merge ACK b59a027d957a4cffd225a681e6c85f9ae7fd77f3
My hashes match, but don't include the `output/*/SHA256SUMS.part` output lines:
```
15310e3aff2a9ef71bad06315fcd425e56611d5bed7f6c394f3fe2248140e17b bitcoin-b59a027d957a-aarch64-linux-gnu-debug.tar.gz
8e60146d39a47e9c332e3b842e239f1ca7773ca2e6788876155377f4568ab1fa bitcoin-b59a027d957a-aarch64-linux-gnu.tar.gz
96c131400a038434f5338926d72c62d7fa260daf3dde9966c7d7880a182b0b6e bitcoin-b59a027d957a-arm-linux-gnueabihf-debug.tar.gz
...
(https://github.com/bitcoin/bitcoin/pull/30074#pullrequestreview-2057646945)
post-merge ACK b59a027d957a4cffd225a681e6c85f9ae7fd77f3
My hashes match, but don't include the `output/*/SHA256SUMS.part` output lines:
```
15310e3aff2a9ef71bad06315fcd425e56611d5bed7f6c394f3fe2248140e17b bitcoin-b59a027d957a-aarch64-linux-gnu-debug.tar.gz
8e60146d39a47e9c332e3b842e239f1ca7773ca2e6788876155377f4568ab1fa bitcoin-b59a027d957a-aarch64-linux-gnu.tar.gz
96c131400a038434f5338926d72c62d7fa260daf3dde9966c7d7880a182b0b6e bitcoin-b59a027d957a-arm-linux-gnueabihf-debug.tar.gz
...
👍 TheCharlatan approved a pull request: "blockstorage: Separate reindexing from saving new blocks"
(https://github.com/bitcoin/bitcoin/pull/29975#pullrequestreview-2057650383)
Re-ACK e41667b720372dae8438ea86e9819027e62b54e0
(https://github.com/bitcoin/bitcoin/pull/29975#pullrequestreview-2057650383)
Re-ACK e41667b720372dae8438ea86e9819027e62b54e0
👍 stickies-v approved a pull request: "[refactor] Check CTxMemPool options in ctor"
(https://github.com/bitcoin/bitcoin/pull/28830#pullrequestreview-2057592650)
ACK dacdb7962c5fef8db26f6fa31facb606165d1d1e
nit: some IWYU fixes, mostly because of the newly introduced `bilingual_str` and `Assert` usage:
<details>
<summary>git diff on dacdb7962c</summary>
```diff
diff --git a/src/test/fuzz/mini_miner.cpp b/src/test/fuzz/mini_miner.cpp
index a52c566076..3cc28a2b14 100644
--- a/src/test/fuzz/mini_miner.cpp
+++ b/src/test/fuzz/mini_miner.cpp
@@ -12,6 +12,8 @@
#include <primitives/transaction.h>
#include <random.h>
#include <txmempool.h>
...
(https://github.com/bitcoin/bitcoin/pull/28830#pullrequestreview-2057592650)
ACK dacdb7962c5fef8db26f6fa31facb606165d1d1e
nit: some IWYU fixes, mostly because of the newly introduced `bilingual_str` and `Assert` usage:
<details>
<summary>git diff on dacdb7962c</summary>
```diff
diff --git a/src/test/fuzz/mini_miner.cpp b/src/test/fuzz/mini_miner.cpp
index a52c566076..3cc28a2b14 100644
--- a/src/test/fuzz/mini_miner.cpp
+++ b/src/test/fuzz/mini_miner.cpp
@@ -12,6 +12,8 @@
#include <primitives/transaction.h>
#include <random.h>
#include <txmempool.h>
...