💬 hodlinator commented on pull request "cmake: Move internal binaries from bin/ to libexec/":
(https://github.com/bitcoin/bitcoin/pull/31679#discussion_r2124090430)
nit:
```suggestion
- README and bitcoin.conf files are included in binary releases but not installed in source builds.
```
(https://github.com/bitcoin/bitcoin/pull/31679#discussion_r2124090430)
nit:
```suggestion
- README and bitcoin.conf files are included in binary releases but not installed in source builds.
```
💬 hodlinator commented on pull request "cmake: Move internal binaries from bin/ to libexec/":
(https://github.com/bitcoin/bitcoin/pull/31679#discussion_r2127330034)
Do we want to *introduce* installation of this binary? Should be called out in PR description and/or commit message if kept.
(https://github.com/bitcoin/bitcoin/pull/31679#discussion_r2127330034)
Do we want to *introduce* installation of this binary? Should be called out in PR description and/or commit message if kept.
🤔 janb84 reviewed a pull request: "test: apply microsecond precision to test framework logging"
(https://github.com/bitcoin/bitcoin/pull/32676#pullrequestreview-2897953035)
ACK ed179e0a6528c39b3bca76365f256716f917e19b
The new output is of the same length but of higher precision (replacing the unused 0) 👍
(keeping it the same format should prevent any parse problems of 3rd party scripts)
- code review ✅
- compiled & tested ✅
(https://github.com/bitcoin/bitcoin/pull/32676#pullrequestreview-2897953035)
ACK ed179e0a6528c39b3bca76365f256716f917e19b
The new output is of the same length but of higher precision (replacing the unused 0) 👍
(keeping it the same format should prevent any parse problems of 3rd party scripts)
- code review ✅
- compiled & tested ✅
💬 davidgumberg commented on pull request "doc: update tor docs to use bitcoind binary from path":
(https://github.com/bitcoin/bitcoin/pull/32679#issuecomment-2941338921)
ACK https://github.com/bitcoin/bitcoin/commit/4ce53495e5e18370b7935551b3b8700faa720a33
I think this makes the documentation more generic, stable, and useful for general users of Bitcoin Core, I think people that are downloading and unzipping binaries are likely to still be able to figure out what to do from these instructions.
(https://github.com/bitcoin/bitcoin/pull/32679#issuecomment-2941338921)
ACK https://github.com/bitcoin/bitcoin/commit/4ce53495e5e18370b7935551b3b8700faa720a33
I think this makes the documentation more generic, stable, and useful for general users of Bitcoin Core, I think people that are downloading and unzipping binaries are likely to still be able to figure out what to do from these instructions.
🤔 janb84 reviewed a pull request: "clang-tidy: Apply modernize-deprecated-headers"
(https://github.com/bitcoin/bitcoin/pull/32673#pullrequestreview-2898033481)
ACK fa9ca13f35be0a023aeed78775ad66f95717b28b
PR Adds check for deprecated headers & replaces deprecated C standard library headers with the C++ equivalents. Makes the code more consistent and the added Clang-tidy check will keep it that way.
- code review ✅
- compile & test ✅
- checked that re-introducing the deprecated headers will result in a clang-tidy violation. ✅
(https://github.com/bitcoin/bitcoin/pull/32673#pullrequestreview-2898033481)
ACK fa9ca13f35be0a023aeed78775ad66f95717b28b
PR Adds check for deprecated headers & replaces deprecated C standard library headers with the C++ equivalents. Makes the code more consistent and the added Clang-tidy check will keep it that way.
- code review ✅
- compile & test ✅
- checked that re-introducing the deprecated headers will result in a clang-tidy violation. ✅
💬 achow101 commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2127435207)
Since multipath is similar to ranged expansion, I've applied the same restrictions that are on ranged to multipath as well. I suppose this should be mentioned in the BIP.
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2127435207)
Since multipath is similar to ranged expansion, I've applied the same restrictions that are on ranged to multipath as well. I suppose this should be mentioned in the BIP.
💬 ariard commented on pull request "BIP-119 (OP_CHECKTEMPLATEVERIFY) (regtest only)":
(https://github.com/bitcoin/bitcoin/pull/31989#issuecomment-2941526149)
the rational why it can be valuable to make CTV a P2TR upgrade only for CTV+sig kind of redeem script
https://github.com/lightning/bolts/issues/1266
(https://github.com/bitcoin/bitcoin/pull/31989#issuecomment-2941526149)
the rational why it can be valuable to make CTV a P2TR upgrade only for CTV+sig kind of redeem script
https://github.com/lightning/bolts/issues/1266
💬 hMsats commented on issue "bitcoind 29.0 much slower than 28.0 on my system: cause found":
(https://github.com/bitcoin/bitcoin/issues/32455#issuecomment-2941527069)
@l0rinc thanks again. I will let v29 run for a longer time and have a look at the file lengths (in txindex) but it will take a few days as something went wrong here and I'm putting back a backup of all the block chain data ...
(https://github.com/bitcoin/bitcoin/issues/32455#issuecomment-2941527069)
@l0rinc thanks again. I will let v29 run for a longer time and have a look at the file lengths (in txindex) but it will take a few days as something went wrong here and I'm putting back a backup of all the block chain data ...
💬 achow101 commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2127464652)
It's deferred to `GetPubKey` as the participants may involve hardened derivation which requires private keys that the constructor doesn't have access to.
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2127464652)
It's deferred to `GetPubKey` as the participants may involve hardened derivation which requires private keys that the constructor doesn't have access to.
💬 achow101 commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2127475291)
Done
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2127475291)
Done
💬 achow101 commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2127475367)
Done
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2127475367)
Done
💬 achow101 commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2127475461)
Done
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2127475461)
Done
💬 achow101 commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2127475603)
Done
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2127475603)
Done
💬 achow101 commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2127475727)
Done
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2127475727)
Done
💬 achow101 commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2127475822)
Done
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2127475822)
Done
💬 hMsats commented on issue "bitcoind 29.0 much slower than 28.0 on my system: cause found":
(https://github.com/bitcoin/bitcoin/issues/32455#issuecomment-2941576744)
I thought the `txindex` files would become 32 MB from the moment the `max_file_size` is changed and all the other `txindex` files stay 2MB and would become 32MB only after a `-reindex`. If it happens automatically in the background than yes that could easily explain my issue and a warning would indeed be very necessary. I'll report back.
(https://github.com/bitcoin/bitcoin/issues/32455#issuecomment-2941576744)
I thought the `txindex` files would become 32 MB from the moment the `max_file_size` is changed and all the other `txindex` files stay 2MB and would become 32MB only after a `-reindex`. If it happens automatically in the background than yes that could easily explain my issue and a warning would indeed be very necessary. I'll report back.
💬 JeremyRubin commented on pull request "[Policy] Discourage Unsigned Annexes":
(https://github.com/bitcoin/bitcoin/pull/32453#issuecomment-2941613640)
https://github.com/bitcoin/bitcoin/blob/a5e98dc3ae63fe8ee689917db440954206893a9f/src/policy/policy.cpp#L283-L286
(https://github.com/bitcoin/bitcoin/pull/32453#issuecomment-2941613640)
https://github.com/bitcoin/bitcoin/blob/a5e98dc3ae63fe8ee689917db440954206893a9f/src/policy/policy.cpp#L283-L286
💬 tnndbtc commented on issue "bitcoind shouldn't fail to progress with synchronization: endless [leveldb] Generated table ... logs":
(https://github.com/bitcoin/bitcoin/issues/31882#issuecomment-2941627475)
@GregTonoski For your particular test, you used snapshot for Mainnet (not assumeutxo@840,000) I assume?
(https://github.com/bitcoin/bitcoin/issues/31882#issuecomment-2941627475)
@GregTonoski For your particular test, you used snapshot for Mainnet (not assumeutxo@840,000) I assume?
💬 fjahr commented on pull request "index: Check all necessary block data is available before starting to sync":
(https://github.com/bitcoin/bitcoin/pull/29770#discussion_r2127492669)
This is just addressing a specific comment from #29668: https://github.com/bitcoin/bitcoin/pull/29668#discussion_r1651138486 I'm not sure which CI you mean? But anyway I think I don't want to extend the scope by adding more include fixes, I can drop these if you want.
(https://github.com/bitcoin/bitcoin/pull/29770#discussion_r2127492669)
This is just addressing a specific comment from #29668: https://github.com/bitcoin/bitcoin/pull/29668#discussion_r1651138486 I'm not sure which CI you mean? But anyway I think I don't want to extend the scope by adding more include fixes, I can drop these if you want.
💬 fjahr commented on pull request "index: Check all necessary block data is available before starting to sync":
(https://github.com/bitcoin/bitcoin/pull/29770#discussion_r2127497118)
Right, good catch on the pointers!
I can try to add such a unit test, do you have specific scenarios in mind that you would like to see covered? If we just want to make sure this block is covered I think I could also add a functional test that basically executes the edge case that @furszy described above: https://github.com/bitcoin/bitcoin/pull/29770#discussion_r2029126932 and I think that should be fairly easy to do.
(https://github.com/bitcoin/bitcoin/pull/29770#discussion_r2127497118)
Right, good catch on the pointers!
I can try to add such a unit test, do you have specific scenarios in mind that you would like to see covered? If we just want to make sure this block is covered I think I could also add a functional test that basically executes the edge case that @furszy described above: https://github.com/bitcoin/bitcoin/pull/29770#discussion_r2029126932 and I think that should be fairly easy to do.