💬 fanquake commented on pull request "build: LLD based macOS toolchain":
(https://github.com/bitcoin/bitcoin/pull/21778#discussion_r1596235623)
Yea. Split into #30078.
(https://github.com/bitcoin/bitcoin/pull/21778#discussion_r1596235623)
Yea. Split into #30078.
💬 fanquake commented on pull request "build: LLD based macOS toolchain":
(https://github.com/bitcoin/bitcoin/pull/21778#issuecomment-2103855244)
Darwin Guix build (aarch64):
```bash
1de4299604e1711c90134a94a7ba6bc2685cc43e4acb283a7b3689afba883205 guix-build-c5f2df00a26c/output/arm64-apple-darwin/SHA256SUMS.part
7dcb77afb8dfa36c39be686827bec06bd39d197faed179b6e17d62492567ce9a guix-build-c5f2df00a26c/output/arm64-apple-darwin/bitcoin-c5f2df00a26c-arm64-apple-darwin-unsigned.tar.gz
2f2bcf41e7b1f7ac28258e6db4600b0230db515f9549dae1c714948c9986c433 guix-build-c5f2df00a26c/output/arm64-apple-darwin/bitcoin-c5f2df00a26c-arm64-apple-darwin
...
(https://github.com/bitcoin/bitcoin/pull/21778#issuecomment-2103855244)
Darwin Guix build (aarch64):
```bash
1de4299604e1711c90134a94a7ba6bc2685cc43e4acb283a7b3689afba883205 guix-build-c5f2df00a26c/output/arm64-apple-darwin/SHA256SUMS.part
7dcb77afb8dfa36c39be686827bec06bd39d197faed179b6e17d62492567ce9a guix-build-c5f2df00a26c/output/arm64-apple-darwin/bitcoin-c5f2df00a26c-arm64-apple-darwin-unsigned.tar.gz
2f2bcf41e7b1f7ac28258e6db4600b0230db515f9549dae1c714948c9986c433 guix-build-c5f2df00a26c/output/arm64-apple-darwin/bitcoin-c5f2df00a26c-arm64-apple-darwin
...
💬 fanquake commented on pull request "build: swap cctools otool for llvm-objdump":
(https://github.com/bitcoin/bitcoin/pull/29739#issuecomment-2103926877)
> There is a mismatch on the windows debug zip :/
I've rebuilt, and see matching now (you and Sjors):
```bash
67f4db93b4f985590948dc344306042841ab747b40ad73bf97ba62d77eb7bfd4 guix-build-7f5ac4520d15/output/aarch64-linux-gnu/SHA256SUMS.part
62e019f164d3c8cf8a1a649b2caa500dd85b00a319910feaf97658020b8cd1e3 guix-build-7f5ac4520d15/output/aarch64-linux-gnu/bitcoin-7f5ac4520d15-aarch64-linux-gnu-debug.tar.gz
c2227a058cf91c0bc3eb244cb254c3c6fcecddc21ba619bca8839d9d79bb1961 guix-build-7f5ac452
...
(https://github.com/bitcoin/bitcoin/pull/29739#issuecomment-2103926877)
> There is a mismatch on the windows debug zip :/
I've rebuilt, and see matching now (you and Sjors):
```bash
67f4db93b4f985590948dc344306042841ab747b40ad73bf97ba62d77eb7bfd4 guix-build-7f5ac4520d15/output/aarch64-linux-gnu/SHA256SUMS.part
62e019f164d3c8cf8a1a649b2caa500dd85b00a319910feaf97658020b8cd1e3 guix-build-7f5ac4520d15/output/aarch64-linux-gnu/bitcoin-7f5ac4520d15-aarch64-linux-gnu-debug.tar.gz
c2227a058cf91c0bc3eb244cb254c3c6fcecddc21ba619bca8839d9d79bb1961 guix-build-7f5ac452
...
💬 Sjors commented on pull request "Testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2104027691)
@wiz the v28 release isn't going to happen sooner, so I assume you'd like this merged so people can more easily test on the master branch? That's probably fine - worst case people need to delete their `~/.bitcoin/testnet4` directory when the release comes out.
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2104027691)
@wiz the v28 release isn't going to happen sooner, so I assume you'd like this merged so people can more easily test on the master branch? That's probably fine - worst case people need to delete their `~/.bitcoin/testnet4` directory when the release comes out.
💬 Sjors commented on issue "BIP352 tracking issue":
(https://github.com/bitcoin/bitcoin/issues/28536#issuecomment-2104063959)
You can point to the actual BIP now in the description 🎉
(https://github.com/bitcoin/bitcoin/issues/28536#issuecomment-2104063959)
You can point to the actual BIP now in the description 🎉
💬 0xB10C commented on pull request "tracing: network connection tracepoints":
(https://github.com/bitcoin/bitcoin/pull/25832#discussion_r1596384279)
I fully agree what you've said. I'm leaving as is for now. If #26593 is merged and I retouch this, I'll reconsider changing this. I'll leave this as "unresolved" for now.
(https://github.com/bitcoin/bitcoin/pull/25832#discussion_r1596384279)
I fully agree what you've said. I'm leaving as is for now. If #26593 is merged and I retouch this, I'll reconsider changing this. I'll leave this as "unresolved" for now.
💬 Sjors commented on pull request "crypto: add `NUMS_H` const":
(https://github.com/bitcoin/bitcoin/pull/30048#issuecomment-2104114821)
tACK 57a06646952fed98c1c281f02fe58a0758a8ed5a
It seems that BIP341 introduced H as just an example of a NUMS point:
> One example of such a point is H
The BIP got it from libsecp256k1 which IIUC only uses it for tests. As does Bitcoin Core so far.
[BIP352](https://github.com/bitcoin/bips/blob/master/bip-0352.mediawiki) gives it special status:
> The one exception is script path spends that use NUMS point H as their internal key
(https://github.com/bitcoin/bitcoin/pull/30048#issuecomment-2104114821)
tACK 57a06646952fed98c1c281f02fe58a0758a8ed5a
It seems that BIP341 introduced H as just an example of a NUMS point:
> One example of such a point is H
The BIP got it from libsecp256k1 which IIUC only uses it for tests. As does Bitcoin Core so far.
[BIP352](https://github.com/bitcoin/bips/blob/master/bip-0352.mediawiki) gives it special status:
> The one exception is script path spends that use NUMS point H as their internal key
💬 S3RK commented on pull request "crypto: add `NUMS_H` const":
(https://github.com/bitcoin/bitcoin/pull/30048#issuecomment-2104126580)
Code Review ACK 57a06646952fed98c1c281f02fe58a0758a8ed5a
The constant definition is consistent with BIP-341. PR removes code duplication and provides a single place to refer to `NUMS_H` either as vector of bytes or x-only pubkey.
(https://github.com/bitcoin/bitcoin/pull/30048#issuecomment-2104126580)
Code Review ACK 57a06646952fed98c1c281f02fe58a0758a8ed5a
The constant definition is consistent with BIP-341. PR removes code duplication and provides a single place to refer to `NUMS_H` either as vector of bytes or x-only pubkey.
👍 laanwj approved a pull request: "net: log connections failures via SOCKS5 with less severity"
(https://github.com/bitcoin/bitcoin/pull/30064#pullrequestreview-2049515955)
Agree these don't need to be logged at high severity, this matches our general idea that network problems on the open internet are normal and there's no reason to scream at the user for them.
ACK 7524aaf58f5dbe077d4997b3a5dcf635a027a659
(https://github.com/bitcoin/bitcoin/pull/30064#pullrequestreview-2049515955)
Agree these don't need to be logged at high severity, this matches our general idea that network problems on the open internet are normal and there's no reason to scream at the user for them.
ACK 7524aaf58f5dbe077d4997b3a5dcf635a027a659
💬 willcl-ark commented on pull request "[27.x] Backports":
(https://github.com/bitcoin/bitcoin/pull/29888#issuecomment-2104165366)
reACK bd5860bc7a892c6bcffe313246dd6b81b973b9c6
(https://github.com/bitcoin/bitcoin/pull/29888#issuecomment-2104165366)
reACK bd5860bc7a892c6bcffe313246dd6b81b973b9c6
💬 0xB10C commented on pull request "tracing: network connection tracepoints":
(https://github.com/bitcoin/bitcoin/pull/25832#discussion_r1596441523)
dropped in 8109319e102c41d3aeed0ecfbc3a0e23b7fea807
(https://github.com/bitcoin/bitcoin/pull/25832#discussion_r1596441523)
dropped in 8109319e102c41d3aeed0ecfbc3a0e23b7fea807
👍 kristapsk approved a pull request: "net: log connections failures via SOCKS5 with less severity"
(https://github.com/bitcoin/bitcoin/pull/30064#pullrequestreview-2049587375)
cr utACK 7524aaf58f5dbe077d4997b3a5dcf635a027a659
(https://github.com/bitcoin/bitcoin/pull/30064#pullrequestreview-2049587375)
cr utACK 7524aaf58f5dbe077d4997b3a5dcf635a027a659
👍 stickies-v approved a pull request: "[27.x] Backports"
(https://github.com/bitcoin/bitcoin/pull/29888#pullrequestreview-2049591642)
re-ACK bd5860bc7a892c6bcffe313246dd6b81b973b9c6
(https://github.com/bitcoin/bitcoin/pull/29888#pullrequestreview-2049591642)
re-ACK bd5860bc7a892c6bcffe313246dd6b81b973b9c6
💬 rkrux commented on pull request "test: add conflicting topology test case":
(https://github.com/bitcoin/bitcoin/pull/30066#discussion_r1596471709)
I see, the part I'm a little confused about is that how can we test for something that's not done yet. Is the intention to have this test written before the topology is relaxed so that we have more robustness in the tests?
Also, am I correct to assume that this error message would need to be updated when the topology is relaxed later? `'package_msg': 'package-not-child-with-unconfirmed-parents'`
(https://github.com/bitcoin/bitcoin/pull/30066#discussion_r1596471709)
I see, the part I'm a little confused about is that how can we test for something that's not done yet. Is the intention to have this test written before the topology is relaxed so that we have more robustness in the tests?
Also, am I correct to assume that this error message would need to be updated when the topology is relaxed later? `'package_msg': 'package-not-child-with-unconfirmed-parents'`
💬 fanquake commented on pull request "depends: set AR & RANLIB for CMake":
(https://github.com/bitcoin/bitcoin/pull/30078#issuecomment-2104234716)
Guix Build (aarch64):
```bash
cf21ea004d1f8d6d7b53d4ef725083229b18469948439f79872a2b019e8bc966 guix-build-7efd6bf03755/output/aarch64-linux-gnu/SHA256SUMS.part
1fa79040aba310b34075b11eefbdb570dd1f053d25ea432faf9bca596603cabb guix-build-7efd6bf03755/output/aarch64-linux-gnu/bitcoin-7efd6bf03755-aarch64-linux-gnu-debug.tar.gz
a97ee58be3ea463c565ec8098ed3f98a04f70b7a897c2b1961df083f5142e07b guix-build-7efd6bf03755/output/aarch64-linux-gnu/bitcoin-7efd6bf03755-aarch64-linux-gnu.tar.gz
1f4f55
...
(https://github.com/bitcoin/bitcoin/pull/30078#issuecomment-2104234716)
Guix Build (aarch64):
```bash
cf21ea004d1f8d6d7b53d4ef725083229b18469948439f79872a2b019e8bc966 guix-build-7efd6bf03755/output/aarch64-linux-gnu/SHA256SUMS.part
1fa79040aba310b34075b11eefbdb570dd1f053d25ea432faf9bca596603cabb guix-build-7efd6bf03755/output/aarch64-linux-gnu/bitcoin-7efd6bf03755-aarch64-linux-gnu-debug.tar.gz
a97ee58be3ea463c565ec8098ed3f98a04f70b7a897c2b1961df083f5142e07b guix-build-7efd6bf03755/output/aarch64-linux-gnu/bitcoin-7efd6bf03755-aarch64-linux-gnu.tar.gz
1f4f55
...
📝 ismaelsadeeq opened a pull request: "Fee Estimation: Ignore all transactions with an in-block child"
(https://github.com/bitcoin/bitcoin/pull/30079)
Another attempt at #25380 with an alternate approach
This PR updates `CBlockPolicyEstimator` to ignore all transactions with an in-block child when a new block is received.
This fixes the assumption that all transactions are confirmed solely because of their fee rate.
As some transactions are confirmed due to a CFPP by some child.
The downside of this approach is that transactions that are not CPFP’d will aslo be ignored.
The correct approach is to linearize all transactions remov
...
(https://github.com/bitcoin/bitcoin/pull/30079)
Another attempt at #25380 with an alternate approach
This PR updates `CBlockPolicyEstimator` to ignore all transactions with an in-block child when a new block is received.
This fixes the assumption that all transactions are confirmed solely because of their fee rate.
As some transactions are confirmed due to a CFPP by some child.
The downside of this approach is that transactions that are not CPFP’d will aslo be ignored.
The correct approach is to linearize all transactions remov
...
💬 Sjors commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#issuecomment-2104258773)
Rebased and renamed to `-bip352index` (stored in `indexes/bip352`). Presumably a future version will have its own BIP number, and by default get a fresh index. By that time we can figure out if it's better to expand the existing index or have two separate non-overlapping indexes.
The index functions `GetSilentPaymentKeys` and `FindSilentPayment` are kept generic, so we have the option of subclassing the index.
I kept the `getsilentpaymentblockdata` RPC name generic, but renamed to `tweaks`
...
(https://github.com/bitcoin/bitcoin/pull/28241#issuecomment-2104258773)
Rebased and renamed to `-bip352index` (stored in `indexes/bip352`). Presumably a future version will have its own BIP number, and by default get a fresh index. By that time we can figure out if it's better to expand the existing index or have two separate non-overlapping indexes.
The index functions `GetSilentPaymentKeys` and `FindSilentPayment` are kept generic, so we have the option of subclassing the index.
I kept the `getsilentpaymentblockdata` RPC name generic, but renamed to `tweaks`
...
💬 josibake commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#issuecomment-2104281397)
> Rebased (on https://github.com/bitcoin/bitcoin/pull/28122) and renamed to -bip352index (stored in indexes/bip352).
Nice! Worth mentioning that now the indexes can be built without needing the wallet to be compiled.
(https://github.com/bitcoin/bitcoin/pull/28241#issuecomment-2104281397)
> Rebased (on https://github.com/bitcoin/bitcoin/pull/28122) and renamed to -bip352index (stored in indexes/bip352).
Nice! Worth mentioning that now the indexes can be built without needing the wallet to be compiled.
💬 hebasto commented on pull request "depends: set AR & RANLIB for CMake":
(https://github.com/bitcoin/bitcoin/pull/30078#discussion_r1596566961)
In contrary to the `CC` and `CXX` environment variables, CMake [docs](https://cmake.org/cmake/help/latest/manual/cmake-env-variables.7.html) do not mention that `AR` and `RANLIB` are honored as well.
I suggest to use [`CMAKE_AR`](https://cmake.org/cmake/help/latest/variable/CMAKE_AR.html) and [`CMAKE_RANLIB`](https://cmake.org/cmake/help/latest/variable/CMAKE_RANLIB.html) instead.
(https://github.com/bitcoin/bitcoin/pull/30078#discussion_r1596566961)
In contrary to the `CC` and `CXX` environment variables, CMake [docs](https://cmake.org/cmake/help/latest/manual/cmake-env-variables.7.html) do not mention that `AR` and `RANLIB` are honored as well.
I suggest to use [`CMAKE_AR`](https://cmake.org/cmake/help/latest/variable/CMAKE_AR.html) and [`CMAKE_RANLIB`](https://cmake.org/cmake/help/latest/variable/CMAKE_RANLIB.html) instead.
💬 fanquake commented on pull request "depends: set AR & RANLIB for CMake":
(https://github.com/bitcoin/bitcoin/pull/30078#discussion_r1596571855)
I tried both of those before landing on this change, and they didn't seem to work properly. Happy to have a look if you can show a patch that works. There's also no CMAKE_NM, so that isn't going to work if we want to pass that through as well.
(https://github.com/bitcoin/bitcoin/pull/30078#discussion_r1596571855)
I tried both of those before landing on this change, and they didn't seem to work properly. Happy to have a look if you can show a patch that works. There's also no CMAKE_NM, so that isn't going to work if we want to pass that through as well.