π¬ mzumsande commented on pull request "test: Fix intermittent issue in p2p_orphan_handling.py":
(https://github.com/bitcoin/bitcoin/pull/32092#discussion_r2002175061)
it would be good to also add a comment in `disconnect_p2ps` that the wait there does not guarantee that the resources of all nodes (such as outstanding txrequests) are cleared.
(https://github.com/bitcoin/bitcoin/pull/32092#discussion_r2002175061)
it would be good to also add a comment in `disconnect_p2ps` that the wait there does not guarantee that the resources of all nodes (such as outstanding txrequests) are cleared.
π¬ murchandamus commented on issue "BnB untested/unused condition in UTXO exclusion optimization":
(https://github.com/bitcoin/bitcoin/issues/32047#issuecomment-2734990522)
> The only way for the fees to be different is if the effective_value is different, however the previous condition: `utxo.GetSelectionAmount() != utxo_pool.at(utxo_pool_index - 1).GetSelectionAmount()` already compared the effective_values.
This is not accurate. You could have two UTXOs of different output types that have the same effective value but differing fees:
E.g.:
Feerate: 10 sat/vB
UTXO_1:
type: P2WPKH,
input weight: 68 vB
amount: 10,680 sats
eff_value: 10,680 - 680 = 10,000
...
(https://github.com/bitcoin/bitcoin/issues/32047#issuecomment-2734990522)
> The only way for the fees to be different is if the effective_value is different, however the previous condition: `utxo.GetSelectionAmount() != utxo_pool.at(utxo_pool_index - 1).GetSelectionAmount()` already compared the effective_values.
This is not accurate. You could have two UTXOs of different output types that have the same effective value but differing fees:
E.g.:
Feerate: 10 sat/vB
UTXO_1:
type: P2WPKH,
input weight: 68 vB
amount: 10,680 sats
eff_value: 10,680 - 680 = 10,000
...
π darosior approved a pull request: "leveldb: pull upstream C++23 changes"
(https://github.com/bitcoin/bitcoin/pull/31766#pullrequestreview-2696569585)
ACK c8fab356171a0e283d5716647e3243c04810ac51 -- checked it's a clean subtree pull from https://github.com/bitcoin-core/leveldb-subtree/tree/bitcoin-fork
(https://github.com/bitcoin/bitcoin/pull/31766#pullrequestreview-2696569585)
ACK c8fab356171a0e283d5716647e3243c04810ac51 -- checked it's a clean subtree pull from https://github.com/bitcoin-core/leveldb-subtree/tree/bitcoin-fork
π¬ jimhashhq commented on pull request "Multiprocess bitcoin":
(https://github.com/bitcoin/bitcoin/pull/10102#issuecomment-2735072960)
Thank you @ryanofsky, I am in favor of a multiprocess Bitcoin, I find this a well considered change. I am interested to try the updated #19461, thank you, and I do apologize for all of the questions.
From comments, it sounds like 19461 is closest, and provides the most functionality for the feature; would it make any sense to maybe change release order, putting 19461 (and a more complete #19460?) ahead of #31740 and #31741?
In general, RE `cmake` and external packages, how packages res
...
(https://github.com/bitcoin/bitcoin/pull/10102#issuecomment-2735072960)
Thank you @ryanofsky, I am in favor of a multiprocess Bitcoin, I find this a well considered change. I am interested to try the updated #19461, thank you, and I do apologize for all of the questions.
From comments, it sounds like 19461 is closest, and provides the most functionality for the feature; would it make any sense to maybe change release order, putting 19461 (and a more complete #19460?) ahead of #31740 and #31741?
In general, RE `cmake` and external packages, how packages res
...
β οΈ hanis12345 opened an issue: "<!--e57a25ab6845829454e8d69fc972939a-->"
(https://github.com/bitcoin/bitcoin/issues/32094)
<!--e57a25ab6845829454e8d69fc972939a-->
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
<!--006a51241073e994b41acfe9ec718e94-->
### Code Coverage & Benchmarks
For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/31766.
<!--021abf342d371248e50ceaed478a90ca-->
### Reviews
See [the guideline](https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#code-review) for information on the review process.
|
...
(https://github.com/bitcoin/bitcoin/issues/32094)
<!--e57a25ab6845829454e8d69fc972939a-->
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
<!--006a51241073e994b41acfe9ec718e94-->
### Code Coverage & Benchmarks
For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/31766.
<!--021abf342d371248e50ceaed478a90ca-->
### Reviews
See [the guideline](https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#code-review) for information on the review process.
|
...
β
achow101 closed an issue: "."
(https://github.com/bitcoin/bitcoin/issues/32094)
(https://github.com/bitcoin/bitcoin/issues/32094)
:lock: fanquake locked an issue: "."
(https://github.com/bitcoin/bitcoin/issues/32094)
(https://github.com/bitcoin/bitcoin/issues/32094)
π¬ Nouridin commented on issue "BnB untested/unused condition in UTXO exclusion optimization":
(https://github.com/bitcoin/bitcoin/issues/32047#issuecomment-2735237660)
https://tenor.com/view/dirty-docks-shawty-triflin-shawty-triflin-she-gif-22455514
(https://github.com/bitcoin/bitcoin/issues/32047#issuecomment-2735237660)
https://tenor.com/view/dirty-docks-shawty-triflin-shawty-triflin-she-gif-22455514
π¬ jsarenik commented on issue "Migrate from BTC/kvB to sat/vB on RPC and startup options":
(https://github.com/bitcoin/bitcoin/issues/32093#issuecomment-2735442040)
What about making sat/vB fractional? I.e. `1.001` would make sure (by rounding up the absolute sats fee) there is at least 1 sat more compared to any other transaction paying just the minimal fee.
(https://github.com/bitcoin/bitcoin/issues/32093#issuecomment-2735442040)
What about making sat/vB fractional? I.e. `1.001` would make sure (by rounding up the absolute sats fee) there is at least 1 sat more compared to any other transaction paying just the minimal fee.
π¬ 0xB10C commented on pull request "test: replace assert with assert_equal and assert_greater_than":
(https://github.com/bitcoin/bitcoin/pull/32091#issuecomment-2735498232)
had a quick look, lgtm ACK 387385ba1edf9febdc75d39bd77b35b29714b3d0
(https://github.com/bitcoin/bitcoin/pull/32091#issuecomment-2735498232)
had a quick look, lgtm ACK 387385ba1edf9febdc75d39bd77b35b29714b3d0
π¬ hebasto commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r2002562814)
> Why is this needed (now, and not with Qt 5)?
Qt 6 has a completely different build system from Qt 5.
> Why aren't the depends outputs already suitable for static linking?
Not "depends output" but rather pkg-config output.
> Why do we need to restrict the search paths, nothing should be looking outside of depends?
Because that is the goal of depends, isn't it?
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r2002562814)
> Why is this needed (now, and not with Qt 5)?
Qt 6 has a completely different build system from Qt 5.
> Why aren't the depends outputs already suitable for static linking?
Not "depends output" but rather pkg-config output.
> Why do we need to restrict the search paths, nothing should be looking outside of depends?
Because that is the goal of depends, isn't it?
π¬ saikiran57 commented on pull request "wallet: removed duplicate call to GetDescriptorScriptPubKeyMan":
(https://github.com/bitcoin/bitcoin/pull/32023#issuecomment-2735571011)
HI @furszy can you please give some clarity on this comment https://github.com/bitcoin/bitcoin/pull/32023#issuecomment-2731126667
(https://github.com/bitcoin/bitcoin/pull/32023#issuecomment-2735571011)
HI @furszy can you please give some clarity on this comment https://github.com/bitcoin/bitcoin/pull/32023#issuecomment-2731126667
π Sjors opened a pull request: "doc: clarify that testnet min-difficulty is not optional"
(https://github.com/bitcoin/bitcoin/pull/32095)
When 20 minutes have gone by on testnet3 or testnet4, the next block `MUST` have difficulty 1. I've seen people be confused about this several times now in recent months. It doesn't help that the code comment is wrong. So fixing that.
(https://github.com/bitcoin/bitcoin/pull/32095)
When 20 minutes have gone by on testnet3 or testnet4, the next block `MUST` have difficulty 1. I've seen people be confused about this several times now in recent months. It doesn't help that the code comment is wrong. So fixing that.
π¬ Sjors commented on pull request "doc: clarify that testnet min-difficulty is not optional":
(https://github.com/bitcoin/bitcoin/pull/32095#issuecomment-2735661223)
Note that there's a recent proposal to drop this rule entirely: https://gnusha.org/pi/bitcoindev/688E575D-C370-4D7D-A6DB-11E0B56710B1@sprovoost.nl/T/#m08b97860799484a7f1a388892f86649065c11503
(https://github.com/bitcoin/bitcoin/pull/32095#issuecomment-2735661223)
Note that there's a recent proposal to drop this rule entirely: https://gnusha.org/pi/bitcoindev/688E575D-C370-4D7D-A6DB-11E0B56710B1@sprovoost.nl/T/#m08b97860799484a7f1a388892f86649065c11503
π¬ BrandonOdiwuor commented on pull request "Feature: Use different datadirs for different signets":
(https://github.com/bitcoin/bitcoin/pull/29838#issuecomment-2735687207)
rebased and removed the leftover debug statement
(https://github.com/bitcoin/bitcoin/pull/29838#issuecomment-2735687207)
rebased and removed the leftover debug statement
π¬ hebasto commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2735687589)
https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2542998445:
> concept ACK
>
> i've got few warnings during build:
>
> ```
> /home/pyth/cpp/bitcoin/src/qt/sendcoinsdialog.cpp: In constructor βSendCoinsDialog::SendCoinsDialog(const
> PlatformStyle*, QWidget*)β:
> /home/pyth/cpp/bitcoin/src/qt/sendcoinsdialog.cpp:91:56: warning: βvoid QCheckBox::stateChanged(int)β is d
> eprecated: Use checkStateChanged() instead [-Wdeprecated-declarations]
> 91 | connect(ui->checkB
...
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2735687589)
https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2542998445:
> concept ACK
>
> i've got few warnings during build:
>
> ```
> /home/pyth/cpp/bitcoin/src/qt/sendcoinsdialog.cpp: In constructor βSendCoinsDialog::SendCoinsDialog(const
> PlatformStyle*, QWidget*)β:
> /home/pyth/cpp/bitcoin/src/qt/sendcoinsdialog.cpp:91:56: warning: βvoid QCheckBox::stateChanged(int)β is d
> eprecated: Use checkStateChanged() instead [-Wdeprecated-declarations]
> 91 | connect(ui->checkB
...
π¬ BrandonOdiwuor commented on pull request "Feature: Use different datadirs for different signets":
(https://github.com/bitcoin/bitcoin/pull/29838#discussion_r2002704851)
@maflcko fixed
(https://github.com/bitcoin/bitcoin/pull/29838#discussion_r2002704851)
@maflcko fixed
π¬ hodlinator commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r2002650307)
Would be good to remove this line from d79dab0fa999002a0c5b70c1688240e2a5032ce1 in this PR.
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r2002650307)
Would be good to remove this line from d79dab0fa999002a0c5b70c1688240e2a5032ce1 in this PR.
π¬ BrandonOdiwuor commented on pull request "Feature: Use different datadirs for different signets":
(https://github.com/bitcoin/bitcoin/pull/29838#issuecomment-2735712124)
@jsarenik @carnhofdaki the behaviour default signet is not changed. I have updated the PR description to include this in case it was unclear.
This patch only affects custom signets with `signetchallenge` and the goal was to run those on custom directories `signet_XXXXXXXX` so as not to interfere with the default signet which runs on `signet` directory if you run multiple signets
(https://github.com/bitcoin/bitcoin/pull/29838#issuecomment-2735712124)
@jsarenik @carnhofdaki the behaviour default signet is not changed. I have updated the PR description to include this in case it was unclear.
This patch only affects custom signets with `signetchallenge` and the goal was to run those on custom directories `signet_XXXXXXXX` so as not to interfere with the default signet which runs on `signet` directory if you run multiple signets
π¬ l0rinc commented on pull request "improve MallocUsage() accuracy":
(https://github.com/bitcoin/bitcoin/pull/28531#discussion_r2002746377)
After https://github.com/bitcoin/bitcoin/pull/30718 this should likely be:
```suggestion
utxo_to_spend=confirmed_utxos.pop(0), target_vsize=32500)
```
(https://github.com/bitcoin/bitcoin/pull/28531#discussion_r2002746377)
After https://github.com/bitcoin/bitcoin/pull/30718 this should likely be:
```suggestion
utxo_to_spend=confirmed_utxos.pop(0), target_vsize=32500)
```