๐ฌ Crypt-iQ commented on issue "compact block fingerprinting":
(https://github.com/bitcoin/bitcoin/issues/28272#issuecomment-3002106439)
> > The relevant code is in UDPRelayBlock
>
> `UDPRelayBlock` is relevant only for FIBRE to FIBRE connections, and these also have [bespoke](https://github.com/w0xlt/bitcoinfibre/blob/45897a826b37eb417cba93ac17ff7bebb272893c/src/udprelay.cpp#L495) logic for receiving FIBRE block announcements. The way that FIBRE nodes announce to "default" nodes is the same as the way that "default" nodes announce to each other. [Below](https://github.com/w0xlt/bitcoinfibre/blob/45897a826b37eb417cba93ac17ff7beb
...
(https://github.com/bitcoin/bitcoin/issues/28272#issuecomment-3002106439)
> > The relevant code is in UDPRelayBlock
>
> `UDPRelayBlock` is relevant only for FIBRE to FIBRE connections, and these also have [bespoke](https://github.com/w0xlt/bitcoinfibre/blob/45897a826b37eb417cba93ac17ff7bebb272893c/src/udprelay.cpp#L495) logic for receiving FIBRE block announcements. The way that FIBRE nodes announce to "default" nodes is the same as the way that "default" nodes announce to each other. [Below](https://github.com/w0xlt/bitcoinfibre/blob/45897a826b37eb417cba93ac17ff7beb
...
๐ฌ Crypt-iQ commented on pull request "p2p: Drop unsolicited CMPCTBLOCK from non-HB peer":
(https://github.com/bitcoin/bitcoin/pull/32606#issuecomment-3002134730)
> The fact that you're enabling FIBRE on a connection can just be taken as implicitly soliciting compact blocks on that connection, so I don't think this is a problem.
Right, my earlier comment was wrong.
I think that dropping unsolicited cmpctblock in the non -blocksonly case is kind of ineffective as pointed out by others. But I also don't have a strong opinion about it given that it doesn't affect FIBRE.
(https://github.com/bitcoin/bitcoin/pull/32606#issuecomment-3002134730)
> The fact that you're enabling FIBRE on a connection can just be taken as implicitly soliciting compact blocks on that connection, so I don't think this is a problem.
Right, my earlier comment was wrong.
I think that dropping unsolicited cmpctblock in the non -blocksonly case is kind of ineffective as pointed out by others. But I also don't have a strong opinion about it given that it doesn't affect FIBRE.
๐ฌ Crypt-iQ commented on pull request "p2p: Drop unsolicited CMPCTBLOCK from non-HB peer":
(https://github.com/bitcoin/bitcoin/pull/32606#discussion_r2165122471)
> One could special-case it to account for allowing empty block reconstruction on blocks-only nodes but it feels like dead weight in the code.
Yeah, I feel like the complexity isn't worth it.
(https://github.com/bitcoin/bitcoin/pull/32606#discussion_r2165122471)
> One could special-case it to account for allowing empty block reconstruction on blocks-only nodes but it feels like dead weight in the code.
Yeah, I feel like the complexity isn't worth it.
๐ฌ luke-jr commented on pull request "Refactor: Redefine CTransaction equality to include witness data":
(https://github.com/bitcoin/bitcoin/pull/32723#issuecomment-3002139031)
Seems like it might be best to forbid operator== entirely and require comparisons to be explicit about whether witness data is compared or not.
(https://github.com/bitcoin/bitcoin/pull/32723#issuecomment-3002139031)
Seems like it might be best to forbid operator== entirely and require comparisons to be explicit about whether witness data is compared or not.
๐ฌ glozow commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#issuecomment-3002183417)
I adapted the bench for any announcement limit, and changed the ManyPeers scenario https://github.com/glozow/bitcoin/commits/2025-06-bench-31829/
Ultimately, regardless of the number of peers you have, the most you can exceed the usage limit with 1 transaction is ~400K, so the max number of transactions you can evict is 400K/240 (240wu being the smallest transaction size). We can increase this with more heap operations by making sure each peer is neck and neck for "DoSiest" at each iteration.
...
(https://github.com/bitcoin/bitcoin/pull/31829#issuecomment-3002183417)
I adapted the bench for any announcement limit, and changed the ManyPeers scenario https://github.com/glozow/bitcoin/commits/2025-06-bench-31829/
Ultimately, regardless of the number of peers you have, the most you can exceed the usage limit with 1 transaction is ~400K, so the max number of transactions you can evict is 400K/240 (240wu being the smallest transaction size). We can increase this with more heap operations by making sure each peer is neck and neck for "DoSiest" at each iteration.
...
๐ฌ truongsonplus3 commented on issue "IPC via TCP Sockets":
(https://github.com/bitcoin/bitcoin/issues/32802#issuecomment-3002318845)
tฦฐฦกng tแปฑ
(https://github.com/bitcoin/bitcoin/issues/32802#issuecomment-3002318845)
tฦฐฦกng tแปฑ
๐ฌ davidgumberg commented on pull request "refactor: CFeeRate encapsulates FeeFrac internally":
(https://github.com/bitcoin/bitcoin/pull/32750#discussion_r2165233501)
nit: the argument name should probably be `m_feerate_kvb`
(https://github.com/bitcoin/bitcoin/pull/32750#discussion_r2165233501)
nit: the argument name should probably be `m_feerate_kvb`
๐ฌ davidgumberg commented on pull request "refactor: CFeeRate encapsulates FeeFrac internally":
(https://github.com/bitcoin/bitcoin/pull/32750#discussion_r2165221968)
can drop this since the `std::ceil` call is dropped.
(https://github.com/bitcoin/bitcoin/pull/32750#discussion_r2165221968)
can drop this since the `std::ceil` call is dropped.
๐ฌ davidgumberg commented on pull request "refactor: CFeeRate encapsulates FeeFrac internally":
(https://github.com/bitcoin/bitcoin/pull/32750#discussion_r2165236159)
The reason that the fuzz tests fail before changing this to `uint32_t` (e.g. this commit: https://github.com/davidgumberg/bitcoin/commit/01a4557b10b31e620716cff04c554e6f1a95653a) is because `CFeeRate::GetFee` takes an unsigned int, but then passes it to `EvaluateFeeUp()` which implicitly converts `at_size` to a signed value, and large examples of `uint32_t` values become negative integers, this results in `EvaluateFee()` getting passed a negative number and asserting.
Wouldn't it be better
...
(https://github.com/bitcoin/bitcoin/pull/32750#discussion_r2165236159)
The reason that the fuzz tests fail before changing this to `uint32_t` (e.g. this commit: https://github.com/davidgumberg/bitcoin/commit/01a4557b10b31e620716cff04c554e6f1a95653a) is because `CFeeRate::GetFee` takes an unsigned int, but then passes it to `EvaluateFeeUp()` which implicitly converts `at_size` to a signed value, and large examples of `uint32_t` values become negative integers, this results in `EvaluateFee()` getting passed a negative number and asserting.
Wouldn't it be better
...
๐ฌ jsarenik commented on issue "getdescriptorinfo returns unusable descriptor":
(https://github.com/bitcoin/bitcoin/issues/29320#issuecomment-3003152393)
Yes. It works also on v29.0.0 (Termux arm64 build). Thanks!
Closing.
(https://github.com/bitcoin/bitcoin/issues/29320#issuecomment-3003152393)
Yes. It works also on v29.0.0 (Termux arm64 build). Thanks!
Closing.
โ
jsarenik closed an issue: "getdescriptorinfo returns unusable descriptor"
(https://github.com/bitcoin/bitcoin/issues/29320)
(https://github.com/bitcoin/bitcoin/issues/29320)
๐ฌ polespinasa commented on pull request "refactor: CFeeRate encapsulates FeeFrac internally":
(https://github.com/bitcoin/bitcoin/pull/32750#discussion_r2165819775)
done
(https://github.com/bitcoin/bitcoin/pull/32750#discussion_r2165819775)
done
๐ฌ polespinasa commented on pull request "refactor: CFeeRate encapsulates FeeFrac internally":
(https://github.com/bitcoin/bitcoin/pull/32750#discussion_r2165819887)
done
(https://github.com/bitcoin/bitcoin/pull/32750#discussion_r2165819887)
done
๐ฌ polespinasa commented on pull request "refactor: CFeeRate encapsulates FeeFrac internally":
(https://github.com/bitcoin/bitcoin/pull/32750#issuecomment-3003478781)
db63d5bf81895d78d39fe8cf94ded6f63d81d614 removes an unused include and some nit on arg names.
990010f49ca97479f9fe46e288ed0f0fbb0fde94 modifies the code to use uint instead of ints see https://github.com/bitcoin/bitcoin/pull/32750#discussion_r2165236159 for context
(https://github.com/bitcoin/bitcoin/pull/32750#issuecomment-3003478781)
db63d5bf81895d78d39fe8cf94ded6f63d81d614 removes an unused include and some nit on arg names.
990010f49ca97479f9fe46e288ed0f0fbb0fde94 modifies the code to use uint instead of ints see https://github.com/bitcoin/bitcoin/pull/32750#discussion_r2165236159 for context
๐ฌ polespinasa commented on pull request "refactor: CFeeRate encapsulates FeeFrac internally":
(https://github.com/bitcoin/bitcoin/pull/32750#discussion_r2165877940)
That would make sense. Implemented on a separated commit 990010f49ca97479f9fe46e288ed0f0fbb0fde94 (will clean and squash later).
@sipa, I would like to know your opinion on this. Is there any reason you used int32 for `EvaluateFee` in the first place? The two `Assume` at the beginning of the function cut the function to only positive numbers.
(https://github.com/bitcoin/bitcoin/pull/32750#discussion_r2165877940)
That would make sense. Implemented on a separated commit 990010f49ca97479f9fe46e288ed0f0fbb0fde94 (will clean and squash later).
@sipa, I would like to know your opinion on this. Is there any reason you used int32 for `EvaluateFee` in the first place? The two `Assume` at the beginning of the function cut the function to only positive numbers.
๐ค ismaelsadeeq reviewed a pull request: "util: detect and warn when using exFAT on MacOS"
(https://github.com/bitcoin/bitcoin/pull/31453#pullrequestreview-2956792851)
Code review and tested ACK 2ad18fc9784e6a37e453bf844ba10c4e46f54754
bitcoind
```
2025-06-25T06:12:15Z [warning] The following paths are on exFAT which is known to have intermittent corruption problems on MacOS: data directory ("/Volumes/StoreJet/bitcoin_temp"). Move these directories to a non-exFAT formatted drive to avoid corruption.
```
gui
<img width="577" alt="Screenshot 2025-06-25 at 07 29 37" src="https://github.com/user-attachments/assets/1e1340e6-9e3e-4aed-beb6-8ab5a5eed678" /
...
(https://github.com/bitcoin/bitcoin/pull/31453#pullrequestreview-2956792851)
Code review and tested ACK 2ad18fc9784e6a37e453bf844ba10c4e46f54754
bitcoind
```
2025-06-25T06:12:15Z [warning] The following paths are on exFAT which is known to have intermittent corruption problems on MacOS: data directory ("/Volumes/StoreJet/bitcoin_temp"). Move these directories to a non-exFAT formatted drive to avoid corruption.
```
gui
<img width="577" alt="Screenshot 2025-06-25 at 07 29 37" src="https://github.com/user-attachments/assets/1e1340e6-9e3e-4aed-beb6-8ab5a5eed678" /
...
๐ฌ ismaelsadeeq commented on pull request "util: detect and warn when using exFAT on MacOS":
(https://github.com/bitcoin/bitcoin/pull/31453#discussion_r2165887565)
nitty-nit: right now the ouput seems to be plural, i.e directories when it's one so this might be better?
```suggestion
if (!exfat_paths.empty()) {Add commentMore actions
for (const auto& path : exfat_paths) {
InitWarning(strprintf(_("The specified %s is on exFAT which is known to have intermittent corruption problems on macOS. "
"Restart with non-exFAT formatted drive to avoid corruption."),
path))
...
(https://github.com/bitcoin/bitcoin/pull/31453#discussion_r2165887565)
nitty-nit: right now the ouput seems to be plural, i.e directories when it's one so this might be better?
```suggestion
if (!exfat_paths.empty()) {Add commentMore actions
for (const auto& path : exfat_paths) {
InitWarning(strprintf(_("The specified %s is on exFAT which is known to have intermittent corruption problems on macOS. "
"Restart with non-exFAT formatted drive to avoid corruption."),
path))
...
๐ฌ ismaelsadeeq commented on pull request "util: detect and warn when using exFAT on MacOS":
(https://github.com/bitcoin/bitcoin/pull/31453#discussion_r2165892974)
e.g
```
2025-06-25T06:13:16Z [warning] The following paths are on exFAT which is known to have intermittent corruption problems on MacOS: blocks directory ("/Volumes/StoreJet/blocks_temp/blocks"). Move these directories to a non-exFAT formatted drive to avoid corruption.
Warning: The following paths are on exFAT which is known to have intermittent corruption problems on MacOS: blocks directory ("/Volumes/StoreJet/blocks_temp/blocks"). Move these directories to a non-exFAT formatted drive to a
...
(https://github.com/bitcoin/bitcoin/pull/31453#discussion_r2165892974)
e.g
```
2025-06-25T06:13:16Z [warning] The following paths are on exFAT which is known to have intermittent corruption problems on MacOS: blocks directory ("/Volumes/StoreJet/blocks_temp/blocks"). Move these directories to a non-exFAT formatted drive to avoid corruption.
Warning: The following paths are on exFAT which is known to have intermittent corruption problems on MacOS: blocks directory ("/Volumes/StoreJet/blocks_temp/blocks"). Move these directories to a non-exFAT formatted drive to a
...
๐ฌ willcl-ark commented on pull request "util: detect and warn when using exFAT on MacOS":
(https://github.com/bitcoin/bitcoin/pull/31453#discussion_r2165924249)
I'm pretty sure this is actually just two different warning styles both being logged to the same place by the user. I haven't tested, but this code: https://github.com/bitcoin/bitcoin/blob/ad654a4807cd584be9ffcd8640f628ab40cb5170/src/noui.cpp#L33-L46 logs one error to log (with timestamp) and when running the daemon the same error (without timestamp) to `stderr`.
When running daemon in the foreground, both messages appear printed in the console. But IMO this makes sense for the daemonised pro
...
(https://github.com/bitcoin/bitcoin/pull/31453#discussion_r2165924249)
I'm pretty sure this is actually just two different warning styles both being logged to the same place by the user. I haven't tested, but this code: https://github.com/bitcoin/bitcoin/blob/ad654a4807cd584be9ffcd8640f628ab40cb5170/src/noui.cpp#L33-L46 logs one error to log (with timestamp) and when running the daemon the same error (without timestamp) to `stderr`.
When running daemon in the foreground, both messages appear printed in the console. But IMO this makes sense for the daemonised pro
...
๐ฌ ismaelsadeeq commented on pull request "test: fix an incorrect `feature_fee_estimation.py` subtest":
(https://github.com/bitcoin/bitcoin/pull/32463#issuecomment-3003557599)
cc @glozow Is this rfm? I'd like to make a quick follow-up on this, as mentioned in
https://github.com/bitcoin/bitcoin/pull/32463#issuecomment-2930709631
Also, this can be backported to 29.1 cc @fanquake.
(https://github.com/bitcoin/bitcoin/pull/32463#issuecomment-3003557599)
cc @glozow Is this rfm? I'd like to make a quick follow-up on this, as mentioned in
https://github.com/bitcoin/bitcoin/pull/32463#issuecomment-2930709631
Also, this can be backported to 29.1 cc @fanquake.