π¬ stickies-v commented on pull request "build: Set AUTHOR_WARNING on warnings":
(https://github.com/bitcoin/bitcoin/pull/33144#issuecomment-3245006172)
> Sounds good.
Thanks for the feedback, I've opened #33278.
> Happy to put this in draft for now
The changes in this/your PR are small, straightforward, can easily be reverted, and offer immediate practical benefit, so I think it makes sense to not block this on my much less straightforward alternative, so my ACK still stands and I'd be happy if this makes progress.
(https://github.com/bitcoin/bitcoin/pull/33144#issuecomment-3245006172)
> Sounds good.
Thanks for the feedback, I've opened #33278.
> Happy to put this in draft for now
The changes in this/your PR are small, straightforward, can easily be reverted, and offer immediate practical benefit, so I think it makes sense to not block this on my much less straightforward alternative, so my ACK still stands and I'd be happy if this makes progress.
π¬ Sjors commented on pull request "kernel: chainparams & headersync updates for 30.0":
(https://github.com/bitcoin/bitcoin/pull/33274#issuecomment-3245015482)
re-ACK 755152ac819a23acf2f9e70316134d74a04d589b
(https://github.com/bitcoin/bitcoin/pull/33274#issuecomment-3245015482)
re-ACK 755152ac819a23acf2f9e70316134d74a04d589b
π rkrux approved a pull request: "wallet: relax external_signer flag constraints"
(https://github.com/bitcoin/bitcoin/pull/33112#pullrequestreview-3176217358)
re-ACK 03978530ad8dc9124307d2ffc7d64c24b784be0e
```
git range-diff a973403...0397853
```
(https://github.com/bitcoin/bitcoin/pull/33112#pullrequestreview-3176217358)
re-ACK 03978530ad8dc9124307d2ffc7d64c24b784be0e
```
git range-diff a973403...0397853
```
β οΈ HowHsu opened an issue: "-loadblock indicated block files cannot find Magic Bytes"
(https://github.com/bitcoin/bitcoin/issues/33280)
### Is there an existing issue for this?
- [x] I have searched the existing issues
### Current behaviour
I first run `bitcoind -prune=1024 -stopatheight=840000 -datadir=empty_dir`, then there is some block files in `empty_dir/blocks`.
Then I copy empty_dir to empty_dir2, run `bitcoind -prune=1024 -stopatheight=840050 -datadir=empty_dir2`
Compare two blocks dir, I now have some blk.dat files relects height 840000 to 840050.
At last I run `bitcoind -datadir=empty_dir -assumevalid=0 -prune=10240
...
(https://github.com/bitcoin/bitcoin/issues/33280)
### Is there an existing issue for this?
- [x] I have searched the existing issues
### Current behaviour
I first run `bitcoind -prune=1024 -stopatheight=840000 -datadir=empty_dir`, then there is some block files in `empty_dir/blocks`.
Then I copy empty_dir to empty_dir2, run `bitcoind -prune=1024 -stopatheight=840050 -datadir=empty_dir2`
Compare two blocks dir, I now have some blk.dat files relects height 840000 to 840050.
At last I run `bitcoind -datadir=empty_dir -assumevalid=0 -prune=10240
...
π¬ maflcko commented on pull request "Release: 30.0 translations update":
(https://github.com/bitcoin/bitcoin/pull/33275#issuecomment-3245095316)
Sure, see https://github.com/maflcko/b-c-gui-translations-review/tree/99bf41c4e5ab74eee8c248b240e8e940e47e09ec/reviews
(https://github.com/bitcoin/bitcoin/pull/33275#issuecomment-3245095316)
Sure, see https://github.com/maflcko/b-c-gui-translations-review/tree/99bf41c4e5ab74eee8c248b240e8e940e47e09ec/reviews
π¬ 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.