📝 VolodymyrBg opened a pull request: "qt: Add addressList field to SendCoinsRecipient for multiple addresses"
(https://github.com/bitcoin-core/gui/pull/857)
This pull request adds a new field addressList to the SendCoinsRecipient class
to properly handle multiple addresses from unauthenticated payment requests.
Previously, the address field was being abused for this purpose, as noted
in a TODO comment.
The changes include:
- Adding a new addressList field to SendCoinsRecipient
- Updating the serialization methods to handle the new field
- Modifying PaymentServer to populate the new field when multiple addresses
are present
- Updating Re
...
(https://github.com/bitcoin-core/gui/pull/857)
This pull request adds a new field addressList to the SendCoinsRecipient class
to properly handle multiple addresses from unauthenticated payment requests.
Previously, the address field was being abused for this purpose, as noted
in a TODO comment.
The changes include:
- Adding a new addressList field to SendCoinsRecipient
- Updating the serialization methods to handle the new field
- Modifying PaymentServer to populate the new field when multiple addresses
are present
- Updating Re
...
💬 l0rinc commented on issue "Fully validated AssumeUTXO starts revalidating after restart":
(https://github.com/bitcoin/bitcoin/issues/32029#issuecomment-2715508521)
23 minutes on my M4 Max - likely ~50 minutes on commodity hardware
(https://github.com/bitcoin/bitcoin/issues/32029#issuecomment-2715508521)
23 minutes on my M4 Max - likely ~50 minutes on commodity hardware
🤔 darosior reviewed a pull request: "qt: 29.0 translations update"
(https://github.com/bitcoin/bitcoin/pull/32004#pullrequestreview-2675997883)
Sanity checked the three French translation files (`bitcoin_fr.ts`, `bitcoin_fr_CM.ts` and `bitcoin_fr_LU.cs`) and i did not find anything malicious as of 9132824947005421057f6a5f035082c7b99f3853. There is a lot of unnecessary churn however.
(https://github.com/bitcoin/bitcoin/pull/32004#pullrequestreview-2675997883)
Sanity checked the three French translation files (`bitcoin_fr.ts`, `bitcoin_fr_CM.ts` and `bitcoin_fr_LU.cs`) and i did not find anything malicious as of 9132824947005421057f6a5f035082c7b99f3853. There is a lot of unnecessary churn however.
💬 darosior commented on pull request "qt: 29.0 translations update":
(https://github.com/bitcoin/bitcoin/pull/32004#discussion_r1990054501)
Seems harmless but there is some spurious symbol inserted in some sentences, such as this period here.
(https://github.com/bitcoin/bitcoin/pull/32004#discussion_r1990054501)
Seems harmless but there is some spurious symbol inserted in some sentences, such as this period here.
🤔 glozow reviewed a pull request: "qt: 29.0 translations update"
(https://github.com/bitcoin/bitcoin/pull/32004#pullrequestreview-2676023046)
ACK 9132824947005421057f6a5f035082c7b99f3853
Lightly sanity checked by pulling translations and diffing with this, grepping for URLs and addresses, and spot-checking a few strings.
(https://github.com/bitcoin/bitcoin/pull/32004#pullrequestreview-2676023046)
ACK 9132824947005421057f6a5f035082c7b99f3853
Lightly sanity checked by pulling translations and diffing with this, grepping for URLs and addresses, and spot-checking a few strings.
💬 hodlinator commented on pull request "net: replace manual reference counting of CNode with shared_ptr":
(https://github.com/bitcoin/bitcoin/pull/32015#discussion_r1989248213)
Seems brittle on master that we just `delete` the node here without checking the refcount. From what I see we only call `StopNodes()` from tests and `CConnman::Stop()`, and we only create `NodesSnapshot`s inside threads we've already stopped earlier in `Stop()`.
(https://github.com/bitcoin/bitcoin/pull/32015#discussion_r1989248213)
Seems brittle on master that we just `delete` the node here without checking the refcount. From what I see we only call `StopNodes()` from tests and `CConnman::Stop()`, and we only create `NodesSnapshot`s inside threads we've already stopped earlier in `Stop()`.
💬 hodlinator commented on pull request "net: replace manual reference counting of CNode with shared_ptr":
(https://github.com/bitcoin/bitcoin/pull/32015#discussion_r1989217778)
Function body was removed.
(https://github.com/bitcoin/bitcoin/pull/32015#discussion_r1989217778)
Function body was removed.
💬 hodlinator commented on pull request "net: replace manual reference counting of CNode with shared_ptr":
(https://github.com/bitcoin/bitcoin/pull/32015#discussion_r1990071370)
In thread https://github.com/bitcoin/bitcoin/pull/32015#discussion_r1984875189:
Started proving out my suggestion and landed on something closer to master.
- Resurrect `m_nodes_disconnected` (now holding `shared_ptr`) along with clearly defined lifetimes.
- Resurrect `DeleteNode()`, but make it take an r-value `shared_ptr` and assert that `use_count() == 1`.
- Avoid adding `~CNode()` and `CNode::m_destruct_cb` along with sending it in everywhere in the constructor. This means we don't need
...
(https://github.com/bitcoin/bitcoin/pull/32015#discussion_r1990071370)
In thread https://github.com/bitcoin/bitcoin/pull/32015#discussion_r1984875189:
Started proving out my suggestion and landed on something closer to master.
- Resurrect `m_nodes_disconnected` (now holding `shared_ptr`) along with clearly defined lifetimes.
- Resurrect `DeleteNode()`, but make it take an r-value `shared_ptr` and assert that `use_count() == 1`.
- Avoid adding `~CNode()` and `CNode::m_destruct_cb` along with sending it in everywhere in the constructor. This means we don't need
...
🤔 glozow reviewed a pull request: "validation, fix: Use wtxid instead of txid in `CheckEphemeralSpends`"
(https://github.com/bitcoin/bitcoin/pull/32025#pullrequestreview-2676077768)
ACK e637dc2c01c, hooray for type safety
(https://github.com/bitcoin/bitcoin/pull/32025#pullrequestreview-2676077768)
ACK e637dc2c01c, hooray for type safety
📝 VolodymyrBg opened a pull request: "docs: fix grammatical typos in documentation files"
(https://github.com/bitcoin/bitcoin/pull/32036)
Fixed errors and inaccuracies in the project's textual materials.
Improved readability of texts and documentation.
(https://github.com/bitcoin/bitcoin/pull/32036)
Fixed errors and inaccuracies in the project's textual materials.
Improved readability of texts and documentation.
💬 glozow commented on issue "cmake inconsistently overriding `-O3` (sometimes)":
(https://github.com/bitcoin/bitcoin/issues/31491#issuecomment-2715691417)
I see a note in https://github.com/bitcoin-core/bitcoin-devwiki/wiki/29.0-Release-Notes-draft#build-system, so will remove this from the 29 milestone. Lmk if this seems wrong.
(https://github.com/bitcoin/bitcoin/issues/31491#issuecomment-2715691417)
I see a note in https://github.com/bitcoin-core/bitcoin-devwiki/wiki/29.0-Release-Notes-draft#build-system, so will remove this from the 29 milestone. Lmk if this seems wrong.
💬 l0rinc commented on pull request "docs: fix grammatical typos in documentation files":
(https://github.com/bitcoin/bitcoin/pull/32036#issuecomment-2715701259)
Nack, misses context completely
(https://github.com/bitcoin/bitcoin/pull/32036#issuecomment-2715701259)
Nack, misses context completely
💬 yancyribbens commented on pull request "Fee Estimation via Fee rate Forecasters":
(https://github.com/bitcoin/bitcoin/pull/30157#discussion_r1990152450)
This is confusing I think. It's titled `estimatefee` however a fee is usually a bitcoin amount. A feerate however is a bitcoin amount per size.
(https://github.com/bitcoin/bitcoin/pull/30157#discussion_r1990152450)
This is confusing I think. It's titled `estimatefee` however a fee is usually a bitcoin amount. A feerate however is a bitcoin amount per size.
💬 yancyribbens commented on pull request "Fee Estimation via Fee rate Forecasters":
(https://github.com/bitcoin/bitcoin/pull/30157#discussion_r1990153325)
should this be "estimatefeerate"?
(https://github.com/bitcoin/bitcoin/pull/30157#discussion_r1990153325)
should this be "estimatefeerate"?
💬 yancyribbens commented on pull request "Fee Estimation via Fee rate Forecasters":
(https://github.com/bitcoin/bitcoin/pull/30157#discussion_r1990157249)
shouldn't the first positional argument be `conf_target` ?
(https://github.com/bitcoin/bitcoin/pull/30157#discussion_r1990157249)
shouldn't the first positional argument be `conf_target` ?
💬 yancyribbens commented on pull request "Fee Estimation via Fee rate Forecasters":
(https://github.com/bitcoin/bitcoin/pull/30157#discussion_r1990158173)
```suggestion
// Add transactions with high_fee until mempool transactions weight is more than 25th percent of DEFAULT_BLOCK_MAX_WEIGHT
```
(https://github.com/bitcoin/bitcoin/pull/30157#discussion_r1990158173)
```suggestion
// Add transactions with high_fee until mempool transactions weight is more than 25th percent of DEFAULT_BLOCK_MAX_WEIGHT
```
💬 yancyribbens commented on pull request "Fee Estimation via Fee rate Forecasters":
(https://github.com/bitcoin/bitcoin/pull/30157#discussion_r1990159751)
> than 25th percent of DEFAULT_BLOCK_MAX_WEIGHT
Maybe others understand this, but I'm not sure what 25th percent of ... means. Is that the same as 25% of ..?
(https://github.com/bitcoin/bitcoin/pull/30157#discussion_r1990159751)
> than 25th percent of DEFAULT_BLOCK_MAX_WEIGHT
Maybe others understand this, but I'm not sure what 25th percent of ... means. Is that the same as 25% of ..?
💬 hebasto commented on pull request "Require sqlite when building the wallet":
(https://github.com/bitcoin/bitcoin/pull/31961#issuecomment-2715787967)
> I could change depends here, but maybe it's easier to do that in the final PR that drops BDB itself?
Changes to depends shouldn't be big. Otherwise this statement from the PR description:
> ... it is no longer possible to not have sqlite available.
does not always hold (see a counter-example in https://github.com/bitcoin/bitcoin/pull/31961#issuecomment-2715187814).
(https://github.com/bitcoin/bitcoin/pull/31961#issuecomment-2715787967)
> I could change depends here, but maybe it's easier to do that in the final PR that drops BDB itself?
Changes to depends shouldn't be big. Otherwise this statement from the PR description:
> ... it is no longer possible to not have sqlite available.
does not always hold (see a counter-example in https://github.com/bitcoin/bitcoin/pull/31961#issuecomment-2715187814).
💬 hebasto commented on pull request "qa: Enable feature_init.py on Windows":
(https://github.com/bitcoin/bitcoin/pull/32021#discussion_r1990194152)
Can this approach be used for all platforms?
(https://github.com/bitcoin/bitcoin/pull/32021#discussion_r1990194152)
Can this approach be used for all platforms?
💬 fjahr commented on issue "Fully validated AssumeUTXO starts revalidating after restart":
(https://github.com/bitcoin/bitcoin/issues/32029#issuecomment-2715987586)
> 23 minutes on my M4 Max - likely ~40 minutes on commodity hardware
Huh, that sounds wrong, were you using a dev/debug build? Or is this probably cause by low resource allocation?
This is me on a vanilla M4 and `gettoutsetinfo` shouldn't do much different than verifying the hash:
```
$ time build/src/bitcoin-cli gettxoutsetinfo
{
"height": 887381,
"bestblock": "000000000000000000023ce7c194520dd6ef38a1d2633f8a726e3a7346549c09",
"txouts": 174298133,
"bogosize": 13621053583,
"hash_ser
...
(https://github.com/bitcoin/bitcoin/issues/32029#issuecomment-2715987586)
> 23 minutes on my M4 Max - likely ~40 minutes on commodity hardware
Huh, that sounds wrong, were you using a dev/debug build? Or is this probably cause by low resource allocation?
This is me on a vanilla M4 and `gettoutsetinfo` shouldn't do much different than verifying the hash:
```
$ time build/src/bitcoin-cli gettxoutsetinfo
{
"height": 887381,
"bestblock": "000000000000000000023ce7c194520dd6ef38a1d2633f8a726e3a7346549c09",
"txouts": 174298133,
"bogosize": 13621053583,
"hash_ser
...