📝 l0rinc converted_to_draft a pull request: "refactor: extract `DecodePushData` from `GetScriptOp` and `GetLegacySigOpCount`"
(https://github.com/bitcoin/bitcoin/pull/32532)
This was originally an optimization PR for `GetSigOpCount` - kept only the refactorings without any optimization.
The goal of the PR now is to document the actual behavior better (more descriptive names, parameters, tests, benchmarks, split out sub-functionality).
### Context
Test coverage was thin for this code path that suddenly became popular for different reasons (https://github.com/bitcoin/bitcoin/pull/31624 and https://github.com/bitcoin/bitcoin/pull/32521 and https://github.com/bit
...
(https://github.com/bitcoin/bitcoin/pull/32532)
This was originally an optimization PR for `GetSigOpCount` - kept only the refactorings without any optimization.
The goal of the PR now is to document the actual behavior better (more descriptive names, parameters, tests, benchmarks, split out sub-functionality).
### Context
Test coverage was thin for this code path that suddenly became popular for different reasons (https://github.com/bitcoin/bitcoin/pull/31624 and https://github.com/bitcoin/bitcoin/pull/32521 and https://github.com/bit
...
🤔 hodlinator reviewed a pull request: "cmake: Move internal binaries from bin/ to libexec/"
(https://github.com/bitcoin/bitcoin/pull/31679#pullrequestreview-2892807188)
Concept ACK d253fa9ebe4305118417a02aa72cc06810662ac6
### Concept
Would be nice (but not critical) to get this done in the same release where we introduce the now merged wrapper, bitcoin(.exe).
### Tested
```
₿ cmake --install build --prefix $PWD/install |sort
-- Install configuration: "RelWithDebInfo"
-- Installing: /home/hodlinator/bc/b2/install/bin/bitcoin
-- Installing: /home/hodlinator/bc/b2/install/bin/bitcoin-cli
-- Installing: /home/hodlinator/bc/b2/install/bin/bitcoind
...
(https://github.com/bitcoin/bitcoin/pull/31679#pullrequestreview-2892807188)
Concept ACK d253fa9ebe4305118417a02aa72cc06810662ac6
### Concept
Would be nice (but not critical) to get this done in the same release where we introduce the now merged wrapper, bitcoin(.exe).
### Tested
```
₿ cmake --install build --prefix $PWD/install |sort
-- Install configuration: "RelWithDebInfo"
-- Installing: /home/hodlinator/bc/b2/install/bin/bitcoin
-- Installing: /home/hodlinator/bc/b2/install/bin/bitcoin-cli
-- Installing: /home/hodlinator/bc/b2/install/bin/bitcoind
...
💬 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?