π¬ Sjors commented on pull request "wallet/rpc: fix listdescriptors RPC fails to return descriptors with private key information when wallet contains descriptors missing any key":
(https://github.com/bitcoin/bitcoin/pull/32471#issuecomment-3245132183)
utACK 46860de809a1e9c7d0a1d716f77e9df7fb0bcd1c
I studied 4d07eff13002384ed59a3b7f592be56a25b88c15 from scratch since it had quite a few changes.
(https://github.com/bitcoin/bitcoin/pull/32471#issuecomment-3245132183)
utACK 46860de809a1e9c7d0a1d716f77e9df7fb0bcd1c
I studied 4d07eff13002384ed59a3b7f592be56a25b88c15 from scratch since it had quite a few changes.
π¬ dergoegge commented on issue "asan: GCC warning about use-after-free":
(https://github.com/bitcoin/bitcoin/issues/33188#issuecomment-3245138164)
This is a false positive. If someone is using gcc with asan we could move `ASAN_UNPOISON_MEMORY_REGION(chunk, m_chunk_size_bytes);` above the `delete` call to avoid this warning (I suspect).
(https://github.com/bitcoin/bitcoin/issues/33188#issuecomment-3245138164)
This is a false positive. If someone is using gcc with asan we could move `ASAN_UNPOISON_MEMORY_REGION(chunk, m_chunk_size_bytes);` above the `delete` call to avoid this warning (I suspect).
π¬ hebasto commented on pull request "Release: 30.0 translations update":
(https://github.com/bitcoin/bitcoin/pull/33275#issuecomment-3245221402)
> Sure, see https://github.com/maflcko/b-c-gui-translations-review/tree/99bf41c4e5ab74eee8c248b240e8e940e47e09ec/reviews
Friendly ping to coordinators for addressing issues:
- @sr-gi -- Catalan (ca)
- @ostruvek -- Czech (cs)
- @pryds -- Danish (da)
- @laanwj @sipa -- Dutch (nl)
- @Emzy -- German (de)
- @cryptomeow -- Greek (el)
- @jesterhodl -- Polish (pl)
(https://github.com/bitcoin/bitcoin/pull/33275#issuecomment-3245221402)
> Sure, see https://github.com/maflcko/b-c-gui-translations-review/tree/99bf41c4e5ab74eee8c248b240e8e940e47e09ec/reviews
Friendly ping to coordinators for addressing issues:
- @sr-gi -- Catalan (ca)
- @ostruvek -- Czech (cs)
- @pryds -- Danish (da)
- @laanwj @sipa -- Dutch (nl)
- @Emzy -- German (de)
- @cryptomeow -- Greek (el)
- @jesterhodl -- Polish (pl)
π ryanofsky opened a pull request: "clang-tidy: disable clang-analyzer UndefinedBinaryOperatorResult warning"
(https://github.com/bitcoin/bitcoin/pull/33281)
Disable `clang-analyzer-core.UndefinedBinaryOperatorResult` warning because it produces a false positive in a Cap'n Proto header:
```c++
/usr/include/kj/async-inl.h:609:37: warning: The left operand of '+' is a garbage value [clang-analyzer-core.UndefinedBinaryOperatorResult]
609 | return *(void**)(*(char**)obj + voff);
```
This was reported by fanquake in https://github.com/bitcoin/bitcoin/issues/33256 and was previously addressed in https://github.com/bitcoin-core/libmultiproc
...
(https://github.com/bitcoin/bitcoin/pull/33281)
Disable `clang-analyzer-core.UndefinedBinaryOperatorResult` warning because it produces a false positive in a Cap'n Proto header:
```c++
/usr/include/kj/async-inl.h:609:37: warning: The left operand of '+' is a garbage value [clang-analyzer-core.UndefinedBinaryOperatorResult]
609 | return *(void**)(*(char**)obj + voff);
```
This was reported by fanquake in https://github.com/bitcoin/bitcoin/issues/33256 and was previously addressed in https://github.com/bitcoin-core/libmultiproc
...
π stickies-v opened a pull request: "cmake: fatal error when PIE not supported"
(https://github.com/bitcoin/bitcoin/pull/33282)
This simultaneously avoids silently overriding user preferences while making it harder to accidentally build without PIE support by requiring an explicit setting.
This introduces two behaviour changes:
1. a user-defined `CMAKE_POSITION_INDEPENDENT_CODE=OFF will now be respected by our build system, instead of silently overriding it like before
2. when not explicitly disabled, lack of PIE support will raise a FATAL_ERROR instead of a WARNING, with a help instruction that this can be overridd
...
(https://github.com/bitcoin/bitcoin/pull/33282)
This simultaneously avoids silently overriding user preferences while making it harder to accidentally build without PIE support by requiring an explicit setting.
This introduces two behaviour changes:
1. a user-defined `CMAKE_POSITION_INDEPENDENT_CODE=OFF will now be respected by our build system, instead of silently overriding it like before
2. when not explicitly disabled, lack of PIE support will raise a FATAL_ERROR instead of a WARNING, with a help instruction that this can be overridd
...
π¬ ryanofsky commented on issue "ci: tidy job is producing output":
(https://github.com/bitcoin/bitcoin/issues/33256#issuecomment-3245345508)
re: https://github.com/bitcoin/bitcoin/issues/33256#issuecomment-3244852511
> I don't think we should have spurious/undocumented output in the CI, so it'd be good to fix this.
Agree it's important to fix because it makes output not useful for identifying real errors. Opened #33281 to try to work around this.
(https://github.com/bitcoin/bitcoin/issues/33256#issuecomment-3245345508)
re: https://github.com/bitcoin/bitcoin/issues/33256#issuecomment-3244852511
> I don't think we should have spurious/undocumented output in the CI, so it'd be good to fix this.
Agree it's important to fix because it makes output not useful for identifying real errors. Opened #33281 to try to work around this.
π¬ glozow commented on pull request "rpc: followups for min fee changes":
(https://github.com/bitcoin/bitcoin/pull/33189#discussion_r2316100070)
Agree 8 places is preferable when it's BTC but 0 places would have been better here. Didn't get the chance to fix this here, but keeping in mind for the future.
(https://github.com/bitcoin/bitcoin/pull/33189#discussion_r2316100070)
Agree 8 places is preferable when it's BTC but 0 places would have been better here. Didn't get the chance to fix this here, but keeping in mind for the future.
π¬ glozow commented on pull request "rpc: followups for min fee changes":
(https://github.com/bitcoin/bitcoin/pull/33189#issuecomment-3245366459)
> Not everyone knows what "33106" is. To avoid it being interpreted as trying to hide something, could you please detail the changes in the PR title and description to help the reviewers and provide full transparency?
> Can we please make this not common practice anymore? Memorising PR numbers is no fun. "Followups for min fee changes" would have been fine.
Agree with this practice, changed now. The intention was not to "hide something" - that's pretty hurtful.
(https://github.com/bitcoin/bitcoin/pull/33189#issuecomment-3245366459)
> Not everyone knows what "33106" is. To avoid it being interpreted as trying to hide something, could you please detail the changes in the PR title and description to help the reviewers and provide full transparency?
> Can we please make this not common practice anymore? Memorising PR numbers is no fun. "Followups for min fee changes" would have been fine.
Agree with this practice, changed now. The intention was not to "hide something" - that's pretty hurtful.
π fanquake opened a pull request: "contrib: update fixed seeds"
(https://github.com/bitcoin/bitcoin/pull/33283)
Update the fixed seeds pre 30 branch off.
(https://github.com/bitcoin/bitcoin/pull/33283)
Update the fixed seeds pre 30 branch off.
π¬ hebasto commented on pull request "Release: 30.0 translations update":
(https://github.com/bitcoin/bitcoin/pull/33275#issuecomment-3245511525)
> Sure, see https://github.com/maflcko/b-c-gui-translations-review/tree/99bf41c4e5ab74eee8c248b240e8e940e47e09ec/reviews
Many thanks, @maflcko! This is indeed very useful.
I discarded only a few suggestions for the Ukrainian translation:
1.
```
<source>Copy &raw transaction</source>
<translation>ΠΠΎΠΏΡΡΠ²Π°ΡΠΈ &Π²ΡΡ ΡΡΠ°Π½Π·Π°ΠΊΡΡΡ</translation>
ERR
The term "raw transaction" in the Bitcoin context refers specifically to the transaction in its raw (serialized) he
...
(https://github.com/bitcoin/bitcoin/pull/33275#issuecomment-3245511525)
> Sure, see https://github.com/maflcko/b-c-gui-translations-review/tree/99bf41c4e5ab74eee8c248b240e8e940e47e09ec/reviews
Many thanks, @maflcko! This is indeed very useful.
I discarded only a few suggestions for the Ukrainian translation:
1.
```
<source>Copy &raw transaction</source>
<translation>ΠΠΎΠΏΡΡΠ²Π°ΡΠΈ &Π²ΡΡ ΡΡΠ°Π½Π·Π°ΠΊΡΡΡ</translation>
ERR
The term "raw transaction" in the Bitcoin context refers specifically to the transaction in its raw (serialized) he
...
π¬ glozow commented on pull request "p2p: never check tx rejections by txid":
(https://github.com/bitcoin/bitcoin/pull/33066#issuecomment-3245518101)
Closing for now. We can revisit this cleanup later.
(https://github.com/bitcoin/bitcoin/pull/33066#issuecomment-3245518101)
Closing for now. We can revisit this cleanup later.
β
glozow closed a pull request: "p2p: never check tx rejections by txid"
(https://github.com/bitcoin/bitcoin/pull/33066)
(https://github.com/bitcoin/bitcoin/pull/33066)
β
glozow closed a pull request: "[NO MERGE] BIP331 Ancestor Package Relay"
(https://github.com/bitcoin/bitcoin/pull/27742)
(https://github.com/bitcoin/bitcoin/pull/27742)
π¬ glozow commented on pull request "[NO MERGE] BIP331 Ancestor Package Relay":
(https://github.com/bitcoin/bitcoin/pull/27742#issuecomment-3245588079)
Ok Drahty I'll take the hint :joy:.
Most of the code here is in master now - the Orphan Resolution Module is part of `TxDownloadManager` (#30110, #31397, etc.) and the `TxOrphanage` eviction changes (#31829, etc.) replace the token bucket protection stuff here. One difference is that we didnβt add the `TxRequestTracker orphan_request_tracker` for vanilla orphan resolution, but that is probably needed for ancestor package relay.
Parts 2 and 3 are relatively smaller, but still need the mempo
...
(https://github.com/bitcoin/bitcoin/pull/27742#issuecomment-3245588079)
Ok Drahty I'll take the hint :joy:.
Most of the code here is in master now - the Orphan Resolution Module is part of `TxDownloadManager` (#30110, #31397, etc.) and the `TxOrphanage` eviction changes (#31829, etc.) replace the token bucket protection stuff here. One difference is that we didnβt add the `TxRequestTracker orphan_request_tracker` for vanilla orphan resolution, but that is probably needed for ancestor package relay.
Parts 2 and 3 are relatively smaller, but still need the mempo
...
π¬ ellenkampguus commented on issue "Old wallet support (Berkeley 4.8)":
(https://github.com/bitcoin/bitcoin/issues/33273#issuecomment-3245607749)
I want to reopen the discussion as I am starting to doubt if Bitcoin Core is still catering to the Bitcoin user's needs.
(https://github.com/bitcoin/bitcoin/issues/33273#issuecomment-3245607749)
I want to reopen the discussion as I am starting to doubt if Bitcoin Core is still catering to the Bitcoin user's needs.
π¬ sipa commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#issuecomment-3245620914)
Rebased, and cherry-picked @Sjors' commit.
(https://github.com/bitcoin/bitcoin/pull/33201#issuecomment-3245620914)
Rebased, and cherry-picked @Sjors' commit.
π¬ pinheadmz commented on issue "Old wallet support (Berkeley 4.8)":
(https://github.com/bitcoin/bitcoin/issues/33273#issuecomment-3245622643)
What "user need" do you feel is not being addressed? Legacy wallets can be converted to the new improved format. This has been at least a [five-year project](https://github.com/bitcoin/bitcoin/issues/20160)
(https://github.com/bitcoin/bitcoin/issues/33273#issuecomment-3245622643)
What "user need" do you feel is not being addressed? Legacy wallets can be converted to the new improved format. This has been at least a [five-year project](https://github.com/bitcoin/bitcoin/issues/20160)
π€ danielabrozzoni reviewed a pull request: "rpc, logging: add backgroundvalidation to getblockchaininfo"
(https://github.com/bitcoin/bitcoin/pull/33259#pullrequestreview-3176862036)
Concept ACK
I did a very rough first pass and the code looks good to me. As others have said, I would like to see a functional test testing the newly introduced fields when the background validation is in progress
(https://github.com/bitcoin/bitcoin/pull/33259#pullrequestreview-3176862036)
Concept ACK
I did a very rough first pass and the code looks good to me. As others have said, I would like to see a functional test testing the newly introduced fields when the background validation is in progress
π¬ hebasto commented on pull request "cmake: fatal error when PIE not supported":
(https://github.com/bitcoin/bitcoin/pull/33282#issuecomment-3245626058)
cc @purpleKarrot @theuni
(https://github.com/bitcoin/bitcoin/pull/33282#issuecomment-3245626058)
cc @purpleKarrot @theuni
π¬ hebasto commented on pull request "clang-tidy: disable clang-analyzer UndefinedBinaryOperatorResult warning":
(https://github.com/bitcoin/bitcoin/pull/33281#issuecomment-3245657952)
Can it be localized into the `src/ipc/capnp/.clang-tidy` file?
(https://github.com/bitcoin/bitcoin/pull/33281#issuecomment-3245657952)
Can it be localized into the `src/ipc/capnp/.clang-tidy` file?