💬 maflcko commented on issue "macOS 13.7 depends build can't find qt":
(https://github.com/bitcoin/bitcoin/issues/31050#issuecomment-2435026697)
Is `/Volumes/SSD/Dev/` and `/Users/sjors/dev/` the same path? I don't know why, but it seems the two are mixed up. My recommendation would be to put the source code and the depends dir on the same path (one of the two) and try again.
(https://github.com/bitcoin/bitcoin/issues/31050#issuecomment-2435026697)
Is `/Volumes/SSD/Dev/` and `/Users/sjors/dev/` the same path? I don't know why, but it seems the two are mixed up. My recommendation would be to put the source code and the depends dir on the same path (one of the two) and try again.
💬 maflcko commented on pull request "test: use context managers and add file existence checks in feature_fee_estimation.py":
(https://github.com/bitcoin/bitcoin/pull/31030#issuecomment-2435052022)
Are you still working on this?
(https://github.com/bitcoin/bitcoin/pull/31030#issuecomment-2435052022)
Are you still working on this?
💬 laanwj commented on pull request "rest: Support transaction broadcast in REST interface":
(https://github.com/bitcoin/bitcoin/pull/31065#issuecomment-2435060786)
> It allows for a binary response format, which is not possible with the (JSON-)RPC interface. For large queries, not having the JSON de/serialization round-trip can make a very meaningful performance difference.
Exactly. Univalue was designed to be a simple and easy to review JSON library, it wasn't designed for performance. The encoding/decoding overhead can matter.
> What is the use of REST interface that doesn't have enough endpoints?
That is a more existential question. There are
...
(https://github.com/bitcoin/bitcoin/pull/31065#issuecomment-2435060786)
> It allows for a binary response format, which is not possible with the (JSON-)RPC interface. For large queries, not having the JSON de/serialization round-trip can make a very meaningful performance difference.
Exactly. Univalue was designed to be a simple and easy to review JSON library, it wasn't designed for performance. The encoding/decoding overhead can matter.
> What is the use of REST interface that doesn't have enough endpoints?
That is a more existential question. There are
...
💬 laanwj commented on pull request "contrib: skip missing binaries in gen-manpages":
(https://github.com/bitcoin/bitcoin/pull/30986#issuecomment-2435085619)
i'm divided on this. There's a use-case, for sure, but this script is mainly used to generate the manpages for a release. And we've had exactly the opposite issue where conditional behavior resulted in incomplete manual pages.
(Prompting me to open #17506)
So not sure it should be the default. As an option, sure.
(https://github.com/bitcoin/bitcoin/pull/30986#issuecomment-2435085619)
i'm divided on this. There's a use-case, for sure, but this script is mainly used to generate the manpages for a release. And we've had exactly the opposite issue where conditional behavior resulted in incomplete manual pages.
(Prompting me to open #17506)
So not sure it should be the default. As an option, sure.
💬 l0rinc commented on pull request "optimization: pack util::Xor into 64 bit chunks instead of doing it byte-by-byte":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r1814847538)
Added RISC-V compiler to https://godbolt.org/z/n5rMeYeas - where it seems to my untrained eyes that it uses two separate 32 bit xors to emulate the 64 bit operation:
```asm
xor a4,a4,s0
xor a5,a5,t2
srli t6,a4,8
srli t5,a4,16
srli t4,a4,24
srli a7,a5,8
```
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r1814847538)
Added RISC-V compiler to https://godbolt.org/z/n5rMeYeas - where it seems to my untrained eyes that it uses two separate 32 bit xors to emulate the 64 bit operation:
```asm
xor a4,a4,s0
xor a5,a5,t2
srli t6,a4,8
srli t5,a4,16
srli t4,a4,24
srli a7,a5,8
```
💬 sipa commented on issue "Remove BIP94 from regtest":
(https://github.com/bitcoin/bitcoin/issues/31137#issuecomment-2435106952)
For the first option, the BIP94 timewarp behavior could perhaps be turned into a (versionbits) deployment?
Then mainnet and testnet3 can have it off (by not having a deployment defined), always on on testnet4, and it would be implicitly configurable on regtest using `-vbparams`?
(https://github.com/bitcoin/bitcoin/issues/31137#issuecomment-2435106952)
For the first option, the BIP94 timewarp behavior could perhaps be turned into a (versionbits) deployment?
Then mainnet and testnet3 can have it off (by not having a deployment defined), always on on testnet4, and it would be implicitly configurable on regtest using `-vbparams`?
💬 l0rinc commented on pull request "optimization: pack util::Xor into 64 bit chunks instead of doing it byte-by-byte":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r1814852528)
To get rid of the goosebumps I'm handling the remaining 4 bytes as a single 32 bit xor now, so the final loop (when keys are 8 bytes long, which is mostly the case for us, I think) does 3 iterations at most. So even if it's not optimized away, we should be fine doing 3 divisions by a nice round number like 8.
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r1814852528)
To get rid of the goosebumps I'm handling the remaining 4 bytes as a single 32 bit xor now, so the final loop (when keys are 8 bytes long, which is mostly the case for us, I think) does 3 iterations at most. So even if it's not optimized away, we should be fine doing 3 divisions by a nice round number like 8.
💬 maflcko commented on issue "Remove BIP94 from regtest":
(https://github.com/bitcoin/bitcoin/issues/31137#issuecomment-2435118435)
Anything is probably fine here. A pre-mined testnet4 chain can possibly be used for the test as well, instead.
(https://github.com/bitcoin/bitcoin/issues/31137#issuecomment-2435118435)
Anything is probably fine here. A pre-mined testnet4 chain can possibly be used for the test as well, instead.
💬 laanwj commented on issue "Support transaction broadcast in REST interface":
(https://github.com/bitcoin/bitcoin/issues/31017#issuecomment-2435123562)
We're kind of having the conceptual discussion over in #31065. Probably not a good idea to split it. But in retrospect it should have been done here, and sooner, before someone started working on it.
(https://github.com/bitcoin/bitcoin/issues/31017#issuecomment-2435123562)
We're kind of having the conceptual discussion over in #31065. Probably not a good idea to split it. But in retrospect it should have been done here, and sooner, before someone started working on it.
💬 maflcko commented on pull request "optimization: pack util::Xor into 64 bit chunks instead of doing it byte-by-byte":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r1814863916)
> divisions by a nice round number like 8.
I don't think the compiler knows the number here, so can't use it to optimize the code based on it.
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r1814863916)
> divisions by a nice round number like 8.
I don't think the compiler knows the number here, so can't use it to optimize the code based on it.
💬 l0rinc commented on pull request "optimization: pack util::Xor into 64 bit chunks instead of doing it byte-by-byte":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r1814865983)
Is that important for max 3 divisions?
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r1814865983)
Is that important for max 3 divisions?
💬 fanquake commented on pull request "build: replace custom `MAC_OSX` macro with existing `__APPLE__`":
(https://github.com/bitcoin/bitcoin/pull/29450#issuecomment-2435152474)
Guix Build:
```bash
245051dc0f18c4da7060996df1ce9eb298b449e48b62cbad30017707c94faed2 guix-build-6c6b2442edab/output/aarch64-linux-gnu/SHA256SUMS.part
55b4b8b790069826c0564c1d3372fada3c8e71dc1a172f2c360aa74c7c041ef1 guix-build-6c6b2442edab/output/aarch64-linux-gnu/bitcoin-6c6b2442edab-aarch64-linux-gnu-debug.tar.gz
133bdd966b70875c8aa6b62eed27235fc327dfeaa8e048db524ec9ccf6085602 guix-build-6c6b2442edab/output/aarch64-linux-gnu/bitcoin-6c6b2442edab-aarch64-linux-gnu.tar.gz
f34455a7dc5581bb
...
(https://github.com/bitcoin/bitcoin/pull/29450#issuecomment-2435152474)
Guix Build:
```bash
245051dc0f18c4da7060996df1ce9eb298b449e48b62cbad30017707c94faed2 guix-build-6c6b2442edab/output/aarch64-linux-gnu/SHA256SUMS.part
55b4b8b790069826c0564c1d3372fada3c8e71dc1a172f2c360aa74c7c041ef1 guix-build-6c6b2442edab/output/aarch64-linux-gnu/bitcoin-6c6b2442edab-aarch64-linux-gnu-debug.tar.gz
133bdd966b70875c8aa6b62eed27235fc327dfeaa8e048db524ec9ccf6085602 guix-build-6c6b2442edab/output/aarch64-linux-gnu/bitcoin-6c6b2442edab-aarch64-linux-gnu.tar.gz
f34455a7dc5581bb
...
💬 fanquake commented on issue "How to compile the GUI on opensuse tumbleweed with cmake?":
(https://github.com/bitcoin-core/gui/issues/842#issuecomment-2435157929)
Even though the package is available, looks like it didn't work there either.
(https://github.com/bitcoin-core/gui/issues/842#issuecomment-2435157929)
Even though the package is available, looks like it didn't work there either.
👍 fanquake approved a pull request: "build: replace custom `MAC_OSX` macro with existing `__APPLE__`"
(https://github.com/bitcoin/bitcoin/pull/29450#pullrequestreview-2392337705)
ACK 6c6b2442edabe9e3f77d29aacd6681bc3fa16d9e - at this point it seems unlikely that we'll need to accomodate non-macOS Apple, so consolidating to `__APPLE__` seems ok for now.
(https://github.com/bitcoin/bitcoin/pull/29450#pullrequestreview-2392337705)
ACK 6c6b2442edabe9e3f77d29aacd6681bc3fa16d9e - at this point it seems unlikely that we'll need to accomodate non-macOS Apple, so consolidating to `__APPLE__` seems ok for now.
💬 sdaftuar commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1814887974)
No I think this one is just "belt" -- if you look at lines 146-148 above, we're not always modifying the "child" transaction here, so sometimes it'll be a duplicate of something we already have.
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1814887974)
No I think this one is just "belt" -- if you look at lines 146-148 above, we're not always modifying the "child" transaction here, so sometimes it'll be a duplicate of something we already have.
💬 sdaftuar commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1814895952)
Yep you're right. I swapped the order of those two commits to avoid this problem, and I also added a test for package rbf with priority (which fails on https://github.com/bitcoin/bitcoin/commit/1c33e133db5f876f2ba6eeb28877a5dcf3f6ac04, but passes by the end of this branch).
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1814895952)
Yep you're right. I swapped the order of those two commits to avoid this problem, and I also added a test for package rbf with priority (which fails on https://github.com/bitcoin/bitcoin/commit/1c33e133db5f876f2ba6eeb28877a5dcf3f6ac04, but passes by the end of this branch).
💬 maflcko commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2435184924)
Could remove "To run release binaries on macOS, [Monterey 12](https://support.apple.com/en-us/103260) or later is required." from the pull description, now that macOS 13 is required and this is no longer relevant?
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2435184924)
Could remove "To run release binaries on macOS, [Monterey 12](https://support.apple.com/en-us/103260) or later is required." from the pull description, now that macOS 13 is required and this is no longer relevant?
💬 hebasto commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2435190097)
> Could remove "To run release binaries on macOS, [Monterey 12](https://support.apple.com/en-us/103260) or later is required." from the pull description, now that macOS 13 is required and this is no longer relevant?
Removed.
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2435190097)
> Could remove "To run release binaries on macOS, [Monterey 12](https://support.apple.com/en-us/103260) or later is required." from the pull description, now that macOS 13 is required and this is no longer relevant?
Removed.
💬 fanquake commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2435192271)
Add the https://github.com/bitcoin/bitcoin/pull/31042#issuecomment-2428995399 requirement at the same time?
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2435192271)
Add the https://github.com/bitcoin/bitcoin/pull/31042#issuecomment-2428995399 requirement at the same time?
🚀 fanquake merged a pull request: "build: replace custom `MAC_OSX` macro with existing `__APPLE__`"
(https://github.com/bitcoin/bitcoin/pull/29450)
(https://github.com/bitcoin/bitcoin/pull/29450)