Bitcoin Core Github
44 subscribers
120K links
Download Telegram
💬 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).
💬 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.
💬 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.
📝 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.
💬 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)...);
| ^
```
💬 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`).
📝 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
...
💬 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 :-)
💬 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
💬 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.
👍 hebasto approved a pull request: "system: improve handling around GetTotalRAM()"
(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?
💬 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/.
```
💬 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.
💬 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.
💬 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.
💬 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.
💬 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).
💬 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
...
🤔 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
...