💬 Sjors commented on pull request "rpc: add target to getmininginfo field and show next block info":
(https://github.com/bitcoin/bitcoin/pull/31583#discussion_r2365643082)
In 2a7bfebd5e788e1d9e7e07a9f1b8e3625a0301cd _Add target to getblock(header) in RPC and REST_: as pointed out in #33440 this is a bug. Unlike `getmininginfo` in baa504fdfaff4a9f61bc939035df5d5f2978cfd7 and `getblockchaininfo` in f153f57acc9f9a6f84af161d5bed9aa9965abaa3, we should not use the `tip` here but `blockindex` (like for nBits).
(https://github.com/bitcoin/bitcoin/pull/31583#discussion_r2365643082)
In 2a7bfebd5e788e1d9e7e07a9f1b8e3625a0301cd _Add target to getblock(header) in RPC and REST_: as pointed out in #33440 this is a bug. Unlike `getmininginfo` in baa504fdfaff4a9f61bc939035df5d5f2978cfd7 and `getblockchaininfo` in f153f57acc9f9a6f84af161d5bed9aa9965abaa3, we should not use the `tip` here but `blockindex` (like for nBits).
💬 TheCharlatan commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-3315058883)
Updated 0fc068b735d267c7ef4a3b23e32dab1771df2509 -> 2ac9d60c54a777978101c369f5895a933208a44c ([kernelApi_65](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_65) -> [kernelApi_66](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_66), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelApi_65..kernelApi_66))
* Changed `btck_BlockHash` into an opaque struct.
* Improved coverage from the tests a bit.
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-3315058883)
Updated 0fc068b735d267c7ef4a3b23e32dab1771df2509 -> 2ac9d60c54a777978101c369f5895a933208a44c ([kernelApi_65](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_65) -> [kernelApi_66](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_66), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelApi_65..kernelApi_66))
* Changed `btck_BlockHash` into an opaque struct.
* Improved coverage from the tests a bit.
💬 Sjors commented on issue "RPC: getblock(header) returns the same target for every block":
(https://github.com/bitcoin/bitcoin/issues/33440#issuecomment-3315061205)
This is indeed a bug. The other commits in that PR use the tip, but that's incorrect for these methods. See https://github.com/bitcoin/bitcoin/pull/31583/commits/2a7bfebd5e788e1d9e7e07a9f1b8e3625a0301cd#r2365643082
I'll PR a fix shortly, still working on adding test coverage.
(https://github.com/bitcoin/bitcoin/issues/33440#issuecomment-3315061205)
This is indeed a bug. The other commits in that PR use the tip, but that's incorrect for these methods. See https://github.com/bitcoin/bitcoin/pull/31583/commits/2a7bfebd5e788e1d9e7e07a9f1b8e3625a0301cd#r2365643082
I'll PR a fix shortly, still working on adding test coverage.
📝 hebasto opened a pull request: "ci: Update Clang in "tidy" job"
(https://github.com/bitcoin/bitcoin/pull/33445)
This PR:
1. Updates to [IWYU 0.25](https://github.com/include-what-you-use/include-what-you-use/releases/tag/0.25), which is compatible with Clang 21.
2. Fixes new "modernize-use-default-member-init" warnings. The warning in `interpreter.cpp` is a false positive, so it has been suppressed.
(https://github.com/bitcoin/bitcoin/pull/33445)
This PR:
1. Updates to [IWYU 0.25](https://github.com/include-what-you-use/include-what-you-use/releases/tag/0.25), which is compatible with Clang 21.
2. Fixes new "modernize-use-default-member-init" warnings. The warning in `interpreter.cpp` is a false positive, so it has been suppressed.
💬 hebasto commented on pull request "ci: Update Clang in "tidy" job":
(https://github.com/bitcoin/bitcoin/pull/33445#issuecomment-3315082319)
@ryanofsky
Could you please take a look at the following IPC-related warning:
```
/usr/include/kj/async-inl.h:404:12: warning: Out of bound access to memory preceding the region [clang-analyzer-security.ArrayBound]
404 | ctor(*ptr, kj::mv(next), kj::fwd<Params>(params)...);
| ^
```
(https://github.com/bitcoin/bitcoin/pull/33445#issuecomment-3315082319)
@ryanofsky
Could you please take a look at the following IPC-related warning:
```
/usr/include/kj/async-inl.h:404:12: warning: Out of bound access to memory preceding the region [clang-analyzer-security.ArrayBound]
404 | ctor(*ptr, kj::mv(next), kj::fwd<Params>(params)...);
| ^
```
💬 andrewtoth commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-3315093703)
re-ACK 3aa74b517621950b7c987d9c6db1856b4bd70ed1
Trivial changes since my last ACK (`git range-diff 7cc9a08706...4327327dd0 edb871cba2...3aa74b5176`).
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-3315093703)
re-ACK 3aa74b517621950b7c987d9c6db1856b4bd70ed1
Trivial changes since my last ACK (`git range-diff 7cc9a08706...4327327dd0 edb871cba2...3aa74b5176`).
📝 Sjors opened a pull request: "rpc: fix getblock(header) returns target for tip"
(https://github.com/bitcoin/bitcoin/pull/33446)
A target field was added to the getblock and getblockheader RPC calls in #31583, but it mistakingly always used the tip value.
This commit fixes it to return the target for the given block.
Because regtest does not have difficulty adjustment, a test is added for mainnet instead. This required mining an additional block.
Also fix a few minor mistakes in the test (suite):
- rename the create_coinbase retarger_period argument to halving_period. Before #31583 this was hardcoded for regtest
...
(https://github.com/bitcoin/bitcoin/pull/33446)
A target field was added to the getblock and getblockheader RPC calls in #31583, but it mistakingly always used the tip value.
This commit fixes it to return the target for the given block.
Because regtest does not have difficulty adjustment, a test is added for mainnet instead. This required mining an additional block.
Also fix a few minor mistakes in the test (suite):
- rename the create_coinbase retarger_period argument to halving_period. Before #31583 this was hardcoded for regtest
...
💬 Sjors commented on pull request "rpc: add target to getmininginfo field and show next block info":
(https://github.com/bitcoin/bitcoin/pull/31583#discussion_r2365712567)
I ended up wasting my own time in #33446 forgetting this, so added more instructions :-)
(https://github.com/bitcoin/bitcoin/pull/31583#discussion_r2365712567)
I ended up wasting my own time in #33446 forgetting this, so added more instructions :-)
💬 Sjors commented on issue "RPC: getblock(header) returns the same target for every block":
(https://github.com/bitcoin/bitcoin/issues/33440#issuecomment-3315102729)
#33446 fixes this
(https://github.com/bitcoin/bitcoin/issues/33440#issuecomment-3315102729)
#33446 fixes this
💬 Sjors commented on pull request "rpc: fix getblock(header) returns target for tip":
(https://github.com/bitcoin/bitcoin/pull/33446#issuecomment-3315104012)
This should probably be backported to v29 and v30.
(https://github.com/bitcoin/bitcoin/pull/33446#issuecomment-3315104012)
This should probably be backported to v29 and v30.
👍 hebasto approved a pull request: "system: improve handling around GetTotalRAM()"
(https://github.com/bitcoin/bitcoin/pull/33435#pullrequestreview-3249158788)
ACK 8fcf71ca005449e639f189dec7ec0163a07e6d37.
(https://github.com/bitcoin/bitcoin/pull/33435#pullrequestreview-3249158788)
ACK 8fcf71ca005449e639f189dec7ec0163a07e6d37.
💬 hebasto commented on pull request "system: improve handling around GetTotalRAM()":
(https://github.com/bitcoin/bitcoin/pull/33435#discussion_r2365733195)
4ebf456dc589a993faa73b8b377c0d1919fbd577
Could you please update the commit message so it reflects the changes?
(https://github.com/bitcoin/bitcoin/pull/33435#discussion_r2365733195)
4ebf456dc589a993faa73b8b377c0d1919fbd577
Could you please update the commit message so it reflects the changes?
💬 hebasto commented on pull request "system: improve handling around GetTotalRAM()":
(https://github.com/bitcoin/bitcoin/pull/33435#discussion_r2365731315)
```suggestion
// file COPYING or https://opensource.org/license/mit/.
```
(https://github.com/bitcoin/bitcoin/pull/33435#discussion_r2365731315)
```suggestion
// file COPYING or https://opensource.org/license/mit/.
```
💬 zaidmstrr commented on pull request "rpc: Handle -named argument parsing where '=' character is used":
(https://github.com/bitcoin/bitcoin/pull/32821#discussion_r2365757547)
Although using class has no performance issues, but I think the stateless point is valid, and using namespace here is a better choice to group related functions and for readability. This will also remove indirection function calls.
(https://github.com/bitcoin/bitcoin/pull/32821#discussion_r2365757547)
Although using class has no performance issues, but I think the stateless point is valid, and using namespace here is a better choice to group related functions and for readability. This will also remove indirection function calls.
💬 zaidmstrr commented on pull request "rpc: Handle -named argument parsing where '=' character is used":
(https://github.com/bitcoin/bitcoin/pull/32821#discussion_r2365757618)
Thanks, fixed.
(https://github.com/bitcoin/bitcoin/pull/32821#discussion_r2365757618)
Thanks, fixed.
💬 zaidmstrr commented on pull request "rpc: Handle -named argument parsing where '=' character is used":
(https://github.com/bitcoin/bitcoin/pull/32821#discussion_r2365757992)
Thanks for suggesting. Fixed in the latest commit.
(https://github.com/bitcoin/bitcoin/pull/32821#discussion_r2365757992)
Thanks for suggesting. Fixed in the latest commit.
💬 zaidmstrr commented on pull request "rpc: Handle -named argument parsing where '=' character is used":
(https://github.com/bitcoin/bitcoin/pull/32821#discussion_r2365758773)
Agreed. Now in the latest commit, I removed this and shifted it above the function with more descriptive info and an example command.
(https://github.com/bitcoin/bitcoin/pull/32821#discussion_r2365758773)
Agreed. Now in the latest commit, I removed this and shifted it above the function with more descriptive info and an example command.
💬 Sjors commented on pull request "rpc: fix getblock(header) returns target for tip":
(https://github.com/bitcoin/bitcoin/pull/33446#issuecomment-3315194787)
I split 88fb025c83c7aa84766a5385e6a797102dd94513 in two commits: one that adds the mock mainnet block and updates the docs, and one that's focussed on fixing the actual bug and adding test coverage for it (using this new block).
(https://github.com/bitcoin/bitcoin/pull/33446#issuecomment-3315194787)
I split 88fb025c83c7aa84766a5385e6a797102dd94513 in two commits: one that adds the mock mainnet block and updates the docs, and one that's focussed on fixing the actual bug and adding test coverage for it (using this new block).
💬 BitByBitByBitByBit commented on pull request "Release: 30.0 translations update":
(https://github.com/bitcoin/bitcoin/pull/33275#issuecomment-3315273628)
> > 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)
>
> UPD. French (fr) and Spanish (es) coordinators have been notified via Transifex messages.
>
> UPD
...
(https://github.com/bitcoin/bitcoin/pull/33275#issuecomment-3315273628)
> > 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)
>
> UPD. French (fr) and Spanish (es) coordinators have been notified via Transifex messages.
>
> UPD
...
🤔 hodlinator reviewed a pull request: "qa: Improvements to debug_assert_log + busy_wait_for_debug_log"
(https://github.com/bitcoin/bitcoin/pull/33423#pullrequestreview-3244741286)
Thanks for the feedback!
> Thanks for reducing useless work - did you measure any speedup?
Rebased new push on top of #33336 which has a lot of `assert_debug_log()` (010cf81407c0df8de766fd2a116463d180f25f33), the below represents somewhat average runs:
```
₿ git co 2025/09/assert_debug_log_rebased~3
₿ hyperfine -r 30 -N ./build/test/functional/feature_assumevalid.py
Benchmark 1: ./build/test/functional/feature_assumevalid.py
Time (mean ± σ): 6.058 s ± 0.049 s [User: 4.263 s
...
(https://github.com/bitcoin/bitcoin/pull/33423#pullrequestreview-3244741286)
Thanks for the feedback!
> Thanks for reducing useless work - did you measure any speedup?
Rebased new push on top of #33336 which has a lot of `assert_debug_log()` (010cf81407c0df8de766fd2a116463d180f25f33), the below represents somewhat average runs:
```
₿ git co 2025/09/assert_debug_log_rebased~3
₿ hyperfine -r 30 -N ./build/test/functional/feature_assumevalid.py
Benchmark 1: ./build/test/functional/feature_assumevalid.py
Time (mean ± σ): 6.058 s ± 0.049 s [User: 4.263 s
...