💬 maflcko commented on pull request "doc: add release notes for #22729":
(https://github.com/bitcoin/bitcoin/pull/30502#discussion_r1687790716)
```suggestion
use Tor. Previously, a configuration `bind=addr:port` would result in
```
Is this an example? If yes, it would be good to list all breaking changes, instead of only one.
(https://github.com/bitcoin/bitcoin/pull/30502#discussion_r1687790716)
```suggestion
use Tor. Previously, a configuration `bind=addr:port` would result in
```
Is this an example? If yes, it would be good to list all breaking changes, instead of only one.
💬 maflcko commented on pull request "doc: add release notes for #22729":
(https://github.com/bitcoin/bitcoin/pull/30502#discussion_r1687788247)
```suggestion
- Previously, if Bitcoin Core were listening for P2P connections, then
```
grammar nit.
(https://github.com/bitcoin/bitcoin/pull/30502#discussion_r1687788247)
```suggestion
- Previously, if Bitcoin Core were listening for P2P connections, then
```
grammar nit.
💬 maflcko commented on pull request "guix: use GCC 13 to builds releases":
(https://github.com/bitcoin/bitcoin/pull/29881#discussion_r1687859398)
Maybe try https://packages.ubuntu.com/noble/g++-mingw-w64-x86-64-posix , or do you need https://packages.debian.org/trixie/mingw-w64-x86-64-dev ?
(https://github.com/bitcoin/bitcoin/pull/29881#discussion_r1687859398)
Maybe try https://packages.ubuntu.com/noble/g++-mingw-w64-x86-64-posix , or do you need https://packages.debian.org/trixie/mingw-w64-x86-64-dev ?
💬 Sjors commented on pull request "net: additional disconnect logging":
(https://github.com/bitcoin/bitcoin/pull/28521#issuecomment-2244921840)
Thanks @davidgumberg. I added a helper function `CNode::DisconnectMsg`, and it's now also used for:
* "receiving message bytes failed"
* "socket send error for"
* "socket recv error"
* "socket closed"
In order to get rid of the redundant generic "disconnecting peer" in `CNode::CloseSocketDisconnect()`, I added the log statement to the two remaining call sites.
Since it's a method on `CNode` I can use it in net_processing as well, for which I added a separate commit. It has the effect o
...
(https://github.com/bitcoin/bitcoin/pull/28521#issuecomment-2244921840)
Thanks @davidgumberg. I added a helper function `CNode::DisconnectMsg`, and it's now also used for:
* "receiving message bytes failed"
* "socket send error for"
* "socket recv error"
* "socket closed"
In order to get rid of the redundant generic "disconnecting peer" in `CNode::CloseSocketDisconnect()`, I added the log statement to the two remaining call sites.
Since it's a method on `CNode` I can use it in net_processing as well, for which I added a separate commit. It has the effect o
...
💬 fjahr commented on pull request "test: assumeutxo: add missing tests in wallet_assumeutxo.py":
(https://github.com/bitcoin/bitcoin/pull/30455#discussion_r1687867805)
> Thanks for the feedback. I added the fastprune option to my local copy and I'm working to understand how pruning operates.
In general, check `feature_index_prune.py`, there extensive tests using pruning and I think you can learn a bit more from that how much you have to mine for the fast pruning to work as you need it.
> However, after the background validation and sync to the tip ([here](https://github.com/bitcoin/bitcoin/pull/30455/files#diff-f763a7cf53e7a5183f66f2634baef84245919265a09
...
(https://github.com/bitcoin/bitcoin/pull/30455#discussion_r1687867805)
> Thanks for the feedback. I added the fastprune option to my local copy and I'm working to understand how pruning operates.
In general, check `feature_index_prune.py`, there extensive tests using pruning and I think you can learn a bit more from that how much you have to mine for the fast pruning to work as you need it.
> However, after the background validation and sync to the tip ([here](https://github.com/bitcoin/bitcoin/pull/30455/files#diff-f763a7cf53e7a5183f66f2634baef84245919265a09
...
💬 vasild commented on issue "Fuzzing related configuration/build options can be improved":
(https://github.com/bitcoin/bitcoin/issues/30318#issuecomment-2244972146)
Opened https://github.com/hebasto/bitcoin/pull/276 to address this. I decided to preserve the behavior of the option controlling `ABORT_ON_FAILED_ASSUME` plus to set up the build targets because that will hopefully result in less friction with people that are already familiar with the existent workflow. Just rename the option.
As a fancy extension, just an idea, maybe detect if `-DSANITIZERS=fuzzer` has been used an enable the option automatically.
(https://github.com/bitcoin/bitcoin/issues/30318#issuecomment-2244972146)
Opened https://github.com/hebasto/bitcoin/pull/276 to address this. I decided to preserve the behavior of the option controlling `ABORT_ON_FAILED_ASSUME` plus to set up the build targets because that will hopefully result in less friction with people that are already familiar with the existent workflow. Just rename the option.
As a fancy extension, just an idea, maybe detect if `-DSANITIZERS=fuzzer` has been used an enable the option automatically.
💬 vasild commented on pull request "doc: add release notes for #22729":
(https://github.com/bitcoin/bitcoin/pull/30502#discussion_r1687896551)
Done.
(https://github.com/bitcoin/bitcoin/pull/30502#discussion_r1687896551)
Done.
💬 vasild commented on pull request "doc: add release notes for #22729":
(https://github.com/bitcoin/bitcoin/pull/30502#discussion_r1687896685)
Done.
(https://github.com/bitcoin/bitcoin/pull/30502#discussion_r1687896685)
Done.
💬 vasild commented on pull request "doc: add release notes for #22729":
(https://github.com/bitcoin/bitcoin/pull/30502#discussion_r1687899396)
Applied the `s/For example/Previously/` change. Initially I wrote "for example `1.2.3.4:8333`" but changed it to "for example `addr:port`", now this covers all is not an example anymore.
(https://github.com/bitcoin/bitcoin/pull/30502#discussion_r1687899396)
Applied the `s/For example/Previously/` change. Initially I wrote "for example `1.2.3.4:8333`" but changed it to "for example `addr:port`", now this covers all is not an example anymore.
💬 vasild commented on pull request "doc: add release notes for #22729":
(https://github.com/bitcoin/bitcoin/pull/30502#issuecomment-2244994924)
`620bbc77d9...307659733e`: address suggestions
(https://github.com/bitcoin/bitcoin/pull/30502#issuecomment-2244994924)
`620bbc77d9...307659733e`: address suggestions
💬 hodlinator commented on pull request "rest: Reject truncated hex txid early in getutxos parsing":
(https://github.com/bitcoin/bitcoin/pull/30482#discussion_r1687924682)
> Only src/util/time.h, but those are just type-related; And src/logging.h, but those are just aliases; So I don't think they apply here.
Thanks, those are good examples. I guess they are/were used more widely and directly compared to SetHex(), making deprecation a more obvious approach. If you deem this sufficiently big to break up into multiple PRs, I trust your judgement.
`SetHexDeprecated` or `SetHexDiscouraged` work for me.
(https://github.com/bitcoin/bitcoin/pull/30482#discussion_r1687924682)
> Only src/util/time.h, but those are just type-related; And src/logging.h, but those are just aliases; So I don't think they apply here.
Thanks, those are good examples. I guess they are/were used more widely and directly compared to SetHex(), making deprecation a more obvious approach. If you deem this sufficiently big to break up into multiple PRs, I trust your judgement.
`SetHexDeprecated` or `SetHexDiscouraged` work for me.
💬 vasild commented on issue "Feature Request: Broadcast Pool":
(https://github.com/bitcoin/bitcoin/issues/30471#issuecomment-2245028663)
> Sparrow broadcasts over Tor via a random external service such as mempool.space for greater privacy. As I understand it, this would no longer be necessary with `-privatebroadcast=1`
Correct.
> But for full Electrum servers like ElectrumX and Fulcrum which use RPCs like getrawmempool, some other way to retrieve unbroadcast transactions will be necessary
Maybe. Note that usually a transaction will be in unbroadcast pool and not in the mempool only for a few seconds, until it round-trips
...
(https://github.com/bitcoin/bitcoin/issues/30471#issuecomment-2245028663)
> Sparrow broadcasts over Tor via a random external service such as mempool.space for greater privacy. As I understand it, this would no longer be necessary with `-privatebroadcast=1`
Correct.
> But for full Electrum servers like ElectrumX and Fulcrum which use RPCs like getrawmempool, some other way to retrieve unbroadcast transactions will be necessary
Maybe. Note that usually a transaction will be in unbroadcast pool and not in the mempool only for a few seconds, until it round-trips
...
💬 alfonsoromanz commented on pull request "test: assumeutxo: add missing tests in wallet_assumeutxo.py":
(https://github.com/bitcoin/bitcoin/pull/30455#discussion_r1687958674)
Thank you very much for your explanation and suggestions! I'll definitely check `feature_index_prune.py` and https://github.com/bitcoin/bitcoin/pull/23813 to better understand how this works.
Here is a branch where I pushed my current status: [debugging_assumeutxo_tests](https://github.com/alfonsoromanz/bitcoin/tree/debugging_assumeutxo_tests)
(https://github.com/bitcoin/bitcoin/pull/30455#discussion_r1687958674)
Thank you very much for your explanation and suggestions! I'll definitely check `feature_index_prune.py` and https://github.com/bitcoin/bitcoin/pull/23813 to better understand how this works.
Here is a branch where I pushed my current status: [debugging_assumeutxo_tests](https://github.com/alfonsoromanz/bitcoin/tree/debugging_assumeutxo_tests)
🤔 TheCharlatan reviewed a pull request: "guix: (explicitly) build Linux GCC with `--enable-cet`"
(https://github.com/bitcoin/bitcoin/pull/30438#pullrequestreview-2193845385)
Guix builds (aarch64)
```
find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
8ac09f23c01b91cd88d3f1329c0d27105f98d31cd1735bba4a0299de18c607c0 guix-build-1d2758f6f044/output/aarch64-linux-gnu/SHA256SUMS.part
94dc28c726e81bc51df3ca7dba068d74a74828b6e5162f130c7e210e4f4a2eff guix-build-1d2758f6f044/output/aarch64-linux-gnu/bitcoin-1d2758f6f044-aarch64-linux-gnu-debug.tar.gz
101f0a64ea62eb82a060827edac75b71562628520d4d3da2dc796
...
(https://github.com/bitcoin/bitcoin/pull/30438#pullrequestreview-2193845385)
Guix builds (aarch64)
```
find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
8ac09f23c01b91cd88d3f1329c0d27105f98d31cd1735bba4a0299de18c607c0 guix-build-1d2758f6f044/output/aarch64-linux-gnu/SHA256SUMS.part
94dc28c726e81bc51df3ca7dba068d74a74828b6e5162f130c7e210e4f4a2eff guix-build-1d2758f6f044/output/aarch64-linux-gnu/bitcoin-1d2758f6f044-aarch64-linux-gnu-debug.tar.gz
101f0a64ea62eb82a060827edac75b71562628520d4d3da2dc796
...
💬 dergoegge commented on pull request "fuzz: Deglobalize signature cache in sigcache test":
(https://github.com/bitcoin/bitcoin/pull/30447#discussion_r1687978640)
Should be fine for this test since there isn't a lot of reachable code here, i.e. coverage quickly plateaus.
Could be avoided by fixing the cache size but I don't think it matters too much.
(https://github.com/bitcoin/bitcoin/pull/30447#discussion_r1687978640)
Should be fine for this test since there isn't a lot of reachable code here, i.e. coverage quickly plateaus.
Could be avoided by fixing the cache size but I don't think it matters too much.
💬 Sjors commented on pull request "net, net_processing: additional and consistent disconnect logging":
(https://github.com/bitcoin/bitcoin/pull/28521#issuecomment-2245128004)
Pushed an update to consistently replace any `LogPrint` we touch with `LogDebug`, and every `LogPrintf ` with `LogInfo`. Also fixed some missing `\n` characters as found by tidy.
(https://github.com/bitcoin/bitcoin/pull/28521#issuecomment-2245128004)
Pushed an update to consistently replace any `LogPrint` we touch with `LogDebug`, and every `LogPrintf ` with `LogInfo`. Also fixed some missing `\n` characters as found by tidy.
🚀 fanquake merged a pull request: "doc: use proper doxygen formatting for CTxMemPool::cs"
(https://github.com/bitcoin/bitcoin/pull/30504)
(https://github.com/bitcoin/bitcoin/pull/30504)
📝 glozow opened a pull request: "m_tx_download_mutex followups"
(https://github.com/bitcoin/bitcoin/pull/30507)
Followup to #30111. Includes suggestions:
- https://github.com/bitcoin/bitcoin/pull/30111#discussion_r1686303768
- https://github.com/bitcoin/bitcoin/pull/30111#discussion_r1686314984
- https://github.com/bitcoin/bitcoin/pull/30111#discussion_r1683186792
- https://github.com/bitcoin/bitcoin/pull/30111#issuecomment-2242819514
- https://github.com/bitcoin/bitcoin/pull/30111#discussion_r1686372826
(https://github.com/bitcoin/bitcoin/pull/30507)
Followup to #30111. Includes suggestions:
- https://github.com/bitcoin/bitcoin/pull/30111#discussion_r1686303768
- https://github.com/bitcoin/bitcoin/pull/30111#discussion_r1686314984
- https://github.com/bitcoin/bitcoin/pull/30111#discussion_r1683186792
- https://github.com/bitcoin/bitcoin/pull/30111#issuecomment-2242819514
- https://github.com/bitcoin/bitcoin/pull/30111#discussion_r1686372826
💬 glozow commented on pull request "locks: introduce mutex for tx download, flush rejection filters once per tip change":
(https://github.com/bitcoin/bitcoin/pull/30111#issuecomment-2245143075)
> In the master branch, the design of the TxOrphanage class is thread-safe for all purposes. I don't think that the fact that m_orphanage does not require additional synchronization justifies the change of TxOrphanage class design.
See https://github.com/bitcoin/bitcoin/pull/30111#discussion_r1601902513 for background. `TxOrphanage` is not a multi-purpose container. It has 1 specific purpose, and its user will need to externally synchronize it with other data structures to fulfill that purpo
...
(https://github.com/bitcoin/bitcoin/pull/30111#issuecomment-2245143075)
> In the master branch, the design of the TxOrphanage class is thread-safe for all purposes. I don't think that the fact that m_orphanage does not require additional synchronization justifies the change of TxOrphanage class design.
See https://github.com/bitcoin/bitcoin/pull/30111#discussion_r1601902513 for background. `TxOrphanage` is not a multi-purpose container. It has 1 specific purpose, and its user will need to externally synchronize it with other data structures to fulfill that purpo
...
💬 glozow commented on pull request "locks: introduce mutex for tx download, flush rejection filters once per tip change":
(https://github.com/bitcoin/bitcoin/pull/30111#discussion_r1687993425)
thanks, added in #30507
(https://github.com/bitcoin/bitcoin/pull/30111#discussion_r1687993425)
thanks, added in #30507