💬 fanquake commented on issue "bitcoin-qt failed assertion on startup":
(https://github.com/bitcoin/bitcoin/issues/31289#issuecomment-2485535623)
I guess move to the GUI repo? Unclear if this will be fixed or not. @hebasto.
(https://github.com/bitcoin/bitcoin/issues/31289#issuecomment-2485535623)
I guess move to the GUI repo? Unclear if this will be fixed or not. @hebasto.
💬 brunoerg commented on pull request "wallet: remove BDB dependency from wallet migration benchmark":
(https://github.com/bitcoin/bitcoin/pull/31241#discussion_r1848241863)
In f7dbb300d7a4d18c9d85f80e5fdd7e5bcd21c6f0: Do we really need `TestLoadWallet` here? Couldn't we simply create the wallet directly (e.g. `std::unique_ptr<CWallet> wallet_ptr{std::make_unique<CWallet>(test_setup->m_node.chain.get()), "", CreateMockableWalletDatabase())};`)?
(https://github.com/bitcoin/bitcoin/pull/31241#discussion_r1848241863)
In f7dbb300d7a4d18c9d85f80e5fdd7e5bcd21c6f0: Do we really need `TestLoadWallet` here? Couldn't we simply create the wallet directly (e.g. `std::unique_ptr<CWallet> wallet_ptr{std::make_unique<CWallet>(test_setup->m_node.chain.get()), "", CreateMockableWalletDatabase())};`)?
👍 BrandonOdiwuor approved a pull request: "cmake: Improve robustness and usability"
(https://github.com/bitcoin/bitcoin/pull/31233#pullrequestreview-2445321566)
tACK 4b6a842c28010a00e121fd36cc0b4e1fa658d249 on Ubuntu 24.04
Was able to successfully test that `util_test_runner` and `util_rpcauth_test` are correctly disabled when Python3 interpreter is not found (screenshots below)
Overally this is an improvement, using the `Python3::Interpreter` target defined directly by find_package(...) is cleaner than relying on the `${PYTHON_COMMAND}` variable we define
without Python3
tACK 4b6a842c28010a00e121fd36cc0b4e1fa658d249 on Ubuntu 24.04
Was able to successfully test that `util_test_runner` and `util_rpcauth_test` are correctly disabled when Python3 interpreter is not found (screenshots below)
Overally this is an improvement, using the `Python3::Interpreter` target defined directly by find_package(...) is cleaner than relying on the `${PYTHON_COMMAND}` variable we define
without Python3
![Screenshot from 2024-11-19 15-24-13](https://github.com/user-attachmen
...
💬 hodlinator commented on pull request "util: Improve documentation and negation of args":
(https://github.com/bitcoin/bitcoin/pull/31212#issuecomment-2485623740)
(The current Windows CI failure ([internal compiler error](https://github.com/bitcoin/bitcoin/actions/runs/11910989737/job/33191550567?pr=31212#step:10:1278)) is not specific to this PR, see #31303).
(https://github.com/bitcoin/bitcoin/pull/31212#issuecomment-2485623740)
(The current Windows CI failure ([internal compiler error](https://github.com/bitcoin/bitcoin/actions/runs/11910989737/job/33191550567?pr=31212#step:10:1278)) is not specific to this PR, see #31303).
📝 defitricks opened a pull request: "Typo Update bitcoin-conf.md"
(https://github.com/bitcoin/bitcoin/pull/31322)
This pull request corrects a minor but important typographical error in the documentation. In the section discussing the placement of comments in the `bitcoin.conf` configuration file, the word **"preferable"** was used incorrectly. The correct word in this context is **"preferred"**.
- **Before:** "Comments may appear in two ways: on their own on an otherwise empty line (_preferable_);"
- **After:** "Comments may appear in two ways: on their own on an otherwise empty line (_preferred_);"
...
(https://github.com/bitcoin/bitcoin/pull/31322)
This pull request corrects a minor but important typographical error in the documentation. In the section discussing the placement of comments in the `bitcoin.conf` configuration file, the word **"preferable"** was used incorrectly. The correct word in this context is **"preferred"**.
- **Before:** "Comments may appear in two ways: on their own on an otherwise empty line (_preferable_);"
- **After:** "Comments may appear in two ways: on their own on an otherwise empty line (_preferred_);"
...
💬 TheCharlatan commented on pull request "Drop script_pub_key arg from createNewBlock":
(https://github.com/bitcoin/bitcoin/pull/31318#issuecomment-2485653947)
I'm not sure why this is really an improvement. Can you elaborate on why no external user would want to pass in a script pubkey directly, instead of manually manipulating the coinbase in the block template?
(https://github.com/bitcoin/bitcoin/pull/31318#issuecomment-2485653947)
I'm not sure why this is really an improvement. Can you elaborate on why no external user would want to pass in a script pubkey directly, instead of manually manipulating the coinbase in the block template?
💬 Sjors commented on pull request "mining: add early return to waitTipChanged()":
(https://github.com/bitcoin/bitcoin/pull/31297#issuecomment-2485712161)
I added a commit to make `m_tip_block` and `std::optional`.
Also, looking at `m_tip_block` again, I'm fairly sure it's set to the tip during node initialization. Otherwise we'd never get something wrong:
https://github.com/bitcoin/bitcoin/blob/746f93b4f0f47c67642057944fb79dddf17369f9/src/init.cpp#L1794-L1806
So I was probably doing something wrong earlier.
I dropped the workaround. An early return only happens if:
1. `m_tip_block` is set; AND
2. `m_tip_block.value()` is different f
...
(https://github.com/bitcoin/bitcoin/pull/31297#issuecomment-2485712161)
I added a commit to make `m_tip_block` and `std::optional`.
Also, looking at `m_tip_block` again, I'm fairly sure it's set to the tip during node initialization. Otherwise we'd never get something wrong:
https://github.com/bitcoin/bitcoin/blob/746f93b4f0f47c67642057944fb79dddf17369f9/src/init.cpp#L1794-L1806
So I was probably doing something wrong earlier.
I dropped the workaround. An early return only happens if:
1. `m_tip_block` is set; AND
2. `m_tip_block.value()` is different f
...
💬 vasild commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#issuecomment-2485715952)
`c4f51c7f61...2b0103705f`: rebase and make the virtual methods of `SockMan` private since they are only used by `SockMan`.
About storing the `CNode` in `SockMan` (which would be templated like `SockMan<CNode>`) and making the communication between `SockMan` and the higher class (e.g. `CConnman`) based on `CNode` instead of node id: this is an excellent idea that will make the code more straight-forward and the higher classes simpler. However it would make this PR larger. I will leave it off f
...
(https://github.com/bitcoin/bitcoin/pull/30988#issuecomment-2485715952)
`c4f51c7f61...2b0103705f`: rebase and make the virtual methods of `SockMan` private since they are only used by `SockMan`.
About storing the `CNode` in `SockMan` (which would be templated like `SockMan<CNode>`) and making the communication between `SockMan` and the higher class (e.g. `CConnman`) based on `CNode` instead of node id: this is an excellent idea that will make the code more straight-forward and the higher classes simpler. However it would make this PR larger. I will leave it off f
...
💬 Sjors commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#issuecomment-2485727139)
Marking this draft, let's review the early return and m_tip_block block changes in #31297 first.
(https://github.com/bitcoin/bitcoin/pull/31283#issuecomment-2485727139)
Marking this draft, let's review the early return and m_tip_block block changes in #31297 first.
📝 Sjors converted_to_draft a pull request: "Add waitNext() to BlockTemplate interface"
(https://github.com/bitcoin/bitcoin/pull/31283)
Alternative approach to #31003, suggested in https://github.com/bitcoin/bitcoin/issues/31109#issuecomment-2451942362.
This PR introduces `waitNext()`. It waits for either the tip to update or for fees at the top of the mempool to rise sufficiently. It then returns a new template, with which the caller can rinse and repeat.
(https://github.com/bitcoin/bitcoin/pull/31283)
Alternative approach to #31003, suggested in https://github.com/bitcoin/bitcoin/issues/31109#issuecomment-2451942362.
This PR introduces `waitNext()`. It waits for either the tip to update or for fees at the top of the mempool to rise sufficiently. It then returns a new template, with which the caller can rinse and repeat.
👍 TheCharlatan approved a pull request: "Prune mining interface"
(https://github.com/bitcoin/bitcoin/pull/31196#pullrequestreview-2445511412)
ACK a67ba64f8f63a0f9f2c86d72cf7240bee7c5388c
(https://github.com/bitcoin/bitcoin/pull/31196#pullrequestreview-2445511412)
ACK a67ba64f8f63a0f9f2c86d72cf7240bee7c5388c
💬 sdaftuar commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1848395907)
I also have a goal of eliminating the need to invoke `CMPA` on a changeset anyway. In #28676, I believe the only place that `CMPA` is used in a changeset is for determining whether an RBF might be replacing something it depends on. I think it should be straightforward to have the changeset logic directly test for that condition and return failure if a dependency is being added to something that is slated for removal, which would then allow us to remove the method altogether.
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1848395907)
I also have a goal of eliminating the need to invoke `CMPA` on a changeset anyway. In #28676, I believe the only place that `CMPA` is used in a changeset is for determining whether an RBF might be replacing something it depends on. I think it should be straightforward to have the changeset logic directly test for that condition and return failure if a dependency is being added to something that is slated for removal, which would then allow us to remove the method altogether.
🤔 hodlinator requested changes to a pull request: "test: Revert to random path element"
(https://github.com/bitcoin/bitcoin/pull/31317#pullrequestreview-2445541192)
Reviewed fa80b08fef0eaa600339caa678fdf80a8aec3ce3
(https://github.com/bitcoin/bitcoin/pull/31317#pullrequestreview-2445541192)
Reviewed fa80b08fef0eaa600339caa678fdf80a8aec3ce3
💬 hodlinator commented on pull request "test: Revert to random path element":
(https://github.com/bitcoin/bitcoin/pull/31317#discussion_r1848404959)
Tested cherrypicking 8e5f8a4f775b93d30b86d28405886eea232cf875 from #30737 on top of this PR to forward `RANDOM_CTX_SEED` on inside *test/fuzz/test_runner.py*.
Ran this ~10 times:
```
RANDOM_CTX_SEED=21 build_fuzz/test/fuzz/test_runner.py ../qa-assets/
```
Many of the failures come down to:
```
Done 2 runs in 0 second(s)
terminate called after throwing an instance of 'std::filesystem::__cxx11::filesystem_error'
what(): filesystem error: cannot remove all: No such file or directory [
...
(https://github.com/bitcoin/bitcoin/pull/31317#discussion_r1848404959)
Tested cherrypicking 8e5f8a4f775b93d30b86d28405886eea232cf875 from #30737 on top of this PR to forward `RANDOM_CTX_SEED` on inside *test/fuzz/test_runner.py*.
Ran this ~10 times:
```
RANDOM_CTX_SEED=21 build_fuzz/test/fuzz/test_runner.py ../qa-assets/
```
Many of the failures come down to:
```
Done 2 runs in 0 second(s)
terminate called after throwing an instance of 'std::filesystem::__cxx11::filesystem_error'
what(): filesystem error: cannot remove all: No such file or directory [
...
💬 ryanofsky commented on pull request "mining: add early return to waitTipChanged()":
(https://github.com/bitcoin/bitcoin/pull/31297#discussion_r1848409060)
In commit "mining: add early return to waitTipChanged()" (71a539c5c3ae165be71a5c8d7f5b1860f8261bd1)
Does this change have any effect? It looks like this is basically changing `cv.wait_for(condition)` to `if (!condition) cv.wait_for(condition)` which I would expect to be equivalent.
(https://github.com/bitcoin/bitcoin/pull/31297#discussion_r1848409060)
In commit "mining: add early return to waitTipChanged()" (71a539c5c3ae165be71a5c8d7f5b1860f8261bd1)
Does this change have any effect? It looks like this is basically changing `cv.wait_for(condition)` to `if (!condition) cv.wait_for(condition)` which I would expect to be equivalent.
✅ fanquake closed a pull request: "refactor: spanify DecodeBits, use constexpr std::array instead of vector"
(https://github.com/bitcoin/bitcoin/pull/31302)
(https://github.com/bitcoin/bitcoin/pull/31302)
💬 Sjors commented on pull request "Drop script_pub_key arg from createNewBlock":
(https://github.com/bitcoin/bitcoin/pull/31318#issuecomment-2485792072)
Afaik the only way external users make blocks today is the `getblocktemplate` RPC, which drops the coinbase transaction from the template it returns.
Similarly in `submitSolution`, used by Stratum v2, we expect to receive the full coinbase transaction and replace whatever was in the `CBlockTemplate`.
Unless you're solo mining, the pool is going to add its own outputs, manipulate the extra nonce, add its own name, etc. Even with a solo pool (a weird concept) you would configure the payout a
...
(https://github.com/bitcoin/bitcoin/pull/31318#issuecomment-2485792072)
Afaik the only way external users make blocks today is the `getblocktemplate` RPC, which drops the coinbase transaction from the template it returns.
Similarly in `submitSolution`, used by Stratum v2, we expect to receive the full coinbase transaction and replace whatever was in the `CBlockTemplate`.
Unless you're solo mining, the pool is going to add its own outputs, manipulate the extra nonce, add its own name, etc. Even with a solo pool (a weird concept) you would configure the payout a
...
💬 fanquake commented on pull request "depends: Update minimum required CMake version":
(https://github.com/bitcoin/bitcoin/pull/31300#issuecomment-2485792150)
> The selected versions in the patches seem random, why not pick the same value for all of them?
Yea. If anything, just consolidate to the same minimum required version as Core.
(https://github.com/bitcoin/bitcoin/pull/31300#issuecomment-2485792150)
> The selected versions in the patches seem random, why not pick the same value for all of them?
Yea. If anything, just consolidate to the same minimum required version as Core.
💬 GIgako19929 commented on pull request "rpc: add getdescriptoractivity":
(https://github.com/bitcoin/bitcoin/pull/30708#issuecomment-2485804884)
This pull request introduces a new RPC method, `getdescriptoractivity`, and includes several related changes. The most important changes are the addition of the new RPC method, updates to documentation, and the introduction of a new utility function.
### New RPC Method:
* Added the `getdescriptoractivity` RPC method to retrieve spend and receive activity for a set of descriptors within specified blocks. This method works in conjunction with the `scanblocks` RPC to reduce the need for additio
...
(https://github.com/bitcoin/bitcoin/pull/30708#issuecomment-2485804884)
This pull request introduces a new RPC method, `getdescriptoractivity`, and includes several related changes. The most important changes are the addition of the new RPC method, updates to documentation, and the introduction of a new utility function.
### New RPC Method:
* Added the `getdescriptoractivity` RPC method to retrieve spend and receive activity for a set of descriptors within specified blocks. This method works in conjunction with the `scanblocks` RPC to reduce the need for additio
...
💬 GIgako19929 commented on pull request "rpc: add getdescriptoractivity":
(https://github.com/bitcoin/bitcoin/pull/30708#issuecomment-2485806660)
This pull request introduces a new RPC method, `getdescriptoractivity`, which provides detailed information on spend and receive activities for specified descriptors and blocks. Additionally, it includes several related updates to the codebase and documentation.
### New RPC Method:
* `getdescriptoractivity` RPC method: This new method allows users to find all spend/receive activity relevant to a given set of descriptors within specified blocks. It pairs well with the `scanblocks` method to r
...
(https://github.com/bitcoin/bitcoin/pull/30708#issuecomment-2485806660)
This pull request introduces a new RPC method, `getdescriptoractivity`, which provides detailed information on spend and receive activities for specified descriptors and blocks. Additionally, it includes several related updates to the codebase and documentation.
### New RPC Method:
* `getdescriptoractivity` RPC method: This new method allows users to find all spend/receive activity relevant to a given set of descriptors within specified blocks. It pairs well with the `scanblocks` method to r
...