💬 luisschwab commented on issue "Incorrect balance when dealing with coinbase UTXOs that will be mature on `h + 1`":
(https://github.com/bitcoin/bitcoin/issues/32098#issuecomment-2741163824)
I understand that, but I think the wallet should strive for correctness, given we're dealing with coinbase UTXOs which only knowledgeable people have access to (miners and developers). This is an edge case regular users won't ever experience, myself included.
(https://github.com/bitcoin/bitcoin/issues/32098#issuecomment-2741163824)
I understand that, but I think the wallet should strive for correctness, given we're dealing with coinbase UTXOs which only knowledgeable people have access to (miners and developers). This is an edge case regular users won't ever experience, myself included.
💬 maflcko commented on pull request "refactor: Enforce readability-avoid-const-params-in-decls":
(https://github.com/bitcoin/bitcoin/pull/31650#issuecomment-2741178844)
rebased
(https://github.com/bitcoin/bitcoin/pull/31650#issuecomment-2741178844)
rebased
💬 sipa commented on issue "Incorrect balance when dealing with coinbase UTXOs that will be mature on `h + 1`":
(https://github.com/bitcoin/bitcoin/issues/32098#issuecomment-2741178910)
@luisschwab Well it's not incorrect to be conservative. And if regular users don't experience it, why does it matter at all?
(https://github.com/bitcoin/bitcoin/issues/32098#issuecomment-2741178910)
@luisschwab Well it's not incorrect to be conservative. And if regular users don't experience it, why does it matter at all?
💬 vasild commented on pull request "test: add end-to-end tests for CConnman and PeerManager":
(https://github.com/bitcoin/bitcoin/pull/26812#issuecomment-2741184656)
`53d2c8e0e2...c1c6a056a0`: rebase due to conflicts
(https://github.com/bitcoin/bitcoin/pull/26812#issuecomment-2741184656)
`53d2c8e0e2...c1c6a056a0`: rebase due to conflicts
💬 jonatack commented on pull request "doc: add guidance for RPC to developer notes":
(https://github.com/bitcoin/bitcoin/pull/30142#issuecomment-2741185074)
ACK c6eca6f3961d780d6a395c3346e44be1a0072f08
(https://github.com/bitcoin/bitcoin/pull/30142#issuecomment-2741185074)
ACK c6eca6f3961d780d6a395c3346e44be1a0072f08
💬 jonatack commented on pull request "doc: add guidance for RPC to developer notes":
(https://github.com/bitcoin/bitcoin/pull/30142#issuecomment-2741194008)
(Meta: the current GitHub safetyism and friction is annoying, when all you want to do is ACK 😄)
**"This issue is highly active. Reconsider commenting unless you have read all the comments and have something to add."**
**"Your link may be misinterpreted by GitHub."**
(https://github.com/bitcoin/bitcoin/pull/30142#issuecomment-2741194008)
(Meta: the current GitHub safetyism and friction is annoying, when all you want to do is ACK 😄)
**"This issue is highly active. Reconsider commenting unless you have read all the comments and have something to add."**
**"Your link may be misinterpreted by GitHub."**
💬 luisschwab commented on issue "Incorrect balance when dealing with coinbase UTXOs that will be mature on `h + 1`":
(https://github.com/bitcoin/bitcoin/issues/32098#issuecomment-2741201839)
I don't think not being able to select an otherwise spendable UTXO is correct.
(https://github.com/bitcoin/bitcoin/issues/32098#issuecomment-2741201839)
I don't think not being able to select an otherwise spendable UTXO is correct.
💬 maflcko commented on pull request "[IBD] batch block reads/writes during `AutoFile` serialization":
(https://github.com/bitcoin/bitcoin/pull/31551#discussion_r2006146926)
I just mean that the assert gives a hard upper bound, but the assume is a bit weaker on release builds, so if the code is used in a place where the upper bound is too small, the assume-version would continue to work on machines with enough memory and only crash "when needed" (aka when running oom).
(https://github.com/bitcoin/bitcoin/pull/31551#discussion_r2006146926)
I just mean that the assert gives a hard upper bound, but the assume is a bit weaker on release builds, so if the code is used in a place where the upper bound is too small, the assume-version would continue to work on machines with enough memory and only crash "when needed" (aka when running oom).
💬 vasild commented on pull request "rpc: provide per message stats for global traffic via new RPC 'getnetmsgstats'":
(https://github.com/bitcoin/bitcoin/pull/29418#issuecomment-2741214252)
`db442f3d6e...626cc06c69`: rebase due to conflicts
(https://github.com/bitcoin/bitcoin/pull/29418#issuecomment-2741214252)
`db442f3d6e...626cc06c69`: rebase due to conflicts
💬 vasild commented on pull request "net: replace manual reference counting of CNode with shared_ptr":
(https://github.com/bitcoin/bitcoin/pull/32015#issuecomment-2741231347)
`8c0ce1ca1c...dcb0be18ac`: rebase due to conflicts
(https://github.com/bitcoin/bitcoin/pull/32015#issuecomment-2741231347)
`8c0ce1ca1c...dcb0be18ac`: rebase due to conflicts
💬 murchandamus commented on issue "BnB untested/unused condition in UTXO exclusion optimization":
(https://github.com/bitcoin/bitcoin/issues/32047#issuecomment-2741233287)
The `long_term_fee_rate` and `fee_rate` are the same for all UTXOs in a single coin selection attempt, so if two UTXOs have the same fee, their inputs have the same weight, if they also have the same effective value, they are interchangeable. If they only have the same effective value but different fees, it means that they have differing weights. Similarly, waste simply scales with the weight across UTXOs, as it is weight times `long_term_fee_rate - fee_rate`. So, if two UTXOs have the same effe
...
(https://github.com/bitcoin/bitcoin/issues/32047#issuecomment-2741233287)
The `long_term_fee_rate` and `fee_rate` are the same for all UTXOs in a single coin selection attempt, so if two UTXOs have the same fee, their inputs have the same weight, if they also have the same effective value, they are interchangeable. If they only have the same effective value but different fees, it means that they have differing weights. Similarly, waste simply scales with the weight across UTXOs, as it is weight times `long_term_fee_rate - fee_rate`. So, if two UTXOs have the same effe
...
🤔 Jassor909 reviewed a pull request: "refactor: Simplifies ProcessMessage for NetMsgType::TX"
(https://github.com/bitcoin/bitcoin/pull/32104#pullrequestreview-2703636828)
@DrahtBot
(https://github.com/bitcoin/bitcoin/pull/32104#pullrequestreview-2703636828)
@DrahtBot
💬 vasild commented on pull request "Fuzz: extend CConnman tests":
(https://github.com/bitcoin/bitcoin/pull/28584#issuecomment-2741259247)
`72ff6d2b50...696b6671da`: rebase due to conflicts
(https://github.com/bitcoin/bitcoin/pull/28584#issuecomment-2741259247)
`72ff6d2b50...696b6671da`: rebase due to conflicts
🤔 BrandonOdiwuor reviewed a pull request: "test: avoid treating hash results as integers"
(https://github.com/bitcoin/bitcoin/pull/32050#pullrequestreview-2703676924)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/32050#pullrequestreview-2703676924)
Concept ACK
💬 sr-gi commented on pull request "refactor: Enforces Txid and Wtxid types in RelayTransaction":
(https://github.com/bitcoin/bitcoin/pull/32104#discussion_r2006210663)
I've updated the approach to enforce RelayTransaction types, and extended it to all other calls to the method
(https://github.com/bitcoin/bitcoin/pull/32104#discussion_r2006210663)
I've updated the approach to enforce RelayTransaction types, and extended it to all other calls to the method
💬 yancyribbens commented on pull request "doc: clarify the documentation of `Assume` assertion":
(https://github.com/bitcoin/bitcoin/pull/32100#discussion_r2006228794)
I tested however the benchmarks don't seem to be sensitive enough for this to make a difference.
(https://github.com/bitcoin/bitcoin/pull/32100#discussion_r2006228794)
I tested however the benchmarks don't seem to be sensitive enough for this to make a difference.
🤔 Jassor909 reviewed a pull request: "refactor: Enforces Txid and Wtxid types in RelayTransaction"
(https://github.com/bitcoin/bitcoin/pull/32104#pullrequestreview-2703737664)
#
(https://github.com/bitcoin/bitcoin/pull/32104#pullrequestreview-2703737664)
#
💬 hebasto commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r2006247029)
... or a poor design of Qt's internal find modules.
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r2006247029)
... or a poor design of Qt's internal find modules.
💬 hebasto commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2741372636)
My Guix build:
```
aarch64
8948298e720d0d9077a1f57c6e44fff86a234830547f5d57278d37c672236541 guix-build-7b51bf1d4c19/output/aarch64-linux-gnu/SHA256SUMS.part
43cec316ed0a89274e9b805b845e5be6b164b9ba5b58750fee470335ed4f7904 guix-build-7b51bf1d4c19/output/aarch64-linux-gnu/bitcoin-7b51bf1d4c19-aarch64-linux-gnu-debug.tar.gz
023042d946d06d6a89e5a611bd5eb9e73680c541e676e4b4c0c28ea780e9fc71 guix-build-7b51bf1d4c19/output/aarch64-linux-gnu/bitcoin-7b51bf1d4c19-aarch64-linux-gnu.tar.gz
513e04e1
...
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2741372636)
My Guix build:
```
aarch64
8948298e720d0d9077a1f57c6e44fff86a234830547f5d57278d37c672236541 guix-build-7b51bf1d4c19/output/aarch64-linux-gnu/SHA256SUMS.part
43cec316ed0a89274e9b805b845e5be6b164b9ba5b58750fee470335ed4f7904 guix-build-7b51bf1d4c19/output/aarch64-linux-gnu/bitcoin-7b51bf1d4c19-aarch64-linux-gnu-debug.tar.gz
023042d946d06d6a89e5a611bd5eb9e73680c541e676e4b4c0c28ea780e9fc71 guix-build-7b51bf1d4c19/output/aarch64-linux-gnu/bitcoin-7b51bf1d4c19-aarch64-linux-gnu.tar.gz
513e04e1
...
💬 Sjors commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#issuecomment-2741407835)
re-ACK 418236c106e32abd7357551d309f8e6d1e494f72
Same as my last review, but the two commits shared with #31866 are merged.
Also tested (ad hoc signed) guix builds on macOS (M4 with 15.3.2, Intel with 13.7.4).
(https://github.com/bitcoin/bitcoin/pull/31375#issuecomment-2741407835)
re-ACK 418236c106e32abd7357551d309f8e6d1e494f72
Same as my last review, but the two commits shared with #31866 are merged.
Also tested (ad hoc signed) guix builds on macOS (M4 with 15.3.2, Intel with 13.7.4).