Bitcoin Core Github
43 subscribers
123K links
Download Telegram
💬 EthanHeilman commented on pull request "tests: improves tapscript unit tests":
(https://github.com/bitcoin/bitcoin/pull/31640#discussion_r1915820425)
Fixed! Thanks
💬 RolloTomasiii commented on issue "Error unlocking wallet: some keys decrypt but not all. your wallet file may be corrupt. ":
(https://github.com/bitcoin/bitcoin/issues/29789#issuecomment-2591482979)
> ### Is there an existing issue for this?
> * [x] I have searched the existing issues
>
> ### Current behaviour
> My personal computer crashed and I've already imported "wallet.dat" in the latest Bitcoin Core v26.0.0. It takes time to synchronize the wallet that was about 6 days.
>
> Anyhow, now my wallet is updated and I can see the correct balance of my Bitcoin in the "Overview section". I have all the needed information which I took as a backup such as "passphrases" and "wallet.dat".
>
>
...
💬 davidgumberg commented on pull request "ci: Bump centos stream 10":
(https://github.com/bitcoin/bitcoin/pull/31593#issuecomment-2591506347)
Concept ACK

The CentOS Stream 9 CI task as-is uses the core python3 packages which are `3.9`, below the minimum `3.10` and results in a syntax error any time `test/functional/combine_logs.py` is invoked since [it uses](https://github.com/bitcoin/bitcoin/commit/c9fb38a590e3961e68e942d71f202e357466d15f) a `match` statement.

It seems that CMake has been complaining in every single centos CI run[^1] since the minimum version was bumped to [3.10](https://github.com/bitcoin/bitcoin/pull/30527/)
...
💬 TheBlueMatt commented on issue "Mining Interface doesn't allow for Bitcoin Core to create blocks when it wants":
(https://github.com/bitcoin/bitcoin/issues/31109#issuecomment-2591522083)
> 1. reduces (tail) latency when requesting a block. In most cases it won't matter, but immediately after finding a new block there is potentially a lot of cs_main contention with other peers sending us copies of the block header or peers requesting the block's transactions. Even ignoring the on-block stuff, you don't want to find yourself waiting for some relatively large transaction to fetch a bunch of UTXO entries off disk to get into the mempool to build your new block!

An interface to re
...
💬 davidgumberg commented on pull request "ci: Turn CentOS task into native one":
(https://github.com/bitcoin/bitcoin/pull/31651#issuecomment-2591535568)
ACK https://github.com/bitcoin/bitcoin/pull/31651/commits/fabefd9915802d53d8c83ae66e5cb259196d9422

Ran locally and all tests passed.

```bash
env -i HOME="$HOME" PATH="$PATH" USER="$USER" bash -c 'FILE_ENV="./ci/test/00_setup_env_native_centos.sh" ./ci/test_run_all.sh'
```
📝 theStack opened a pull request: "test: p2p: fix sending of manual INVs in tx download test"
(https://github.com/bitcoin/bitcoin/pull/31658)
The `test_inv_block` sub-test in p2p_tx_download.py has a subtle bug: the manual msg_inv announcements from peers currently have no effect, since they don't match the wtxidrelay setting (=true by default for `P2PInterface` instances) and are hence ignored by the nodes (since 2d282e0c / PR #18044):

https://github.com/bitcoin/bitcoin/blob/e7c479495509c068215b73f6df070af2d406ae15/src/net_processing.cpp#L3904-L3911

Though the test still passes on master, it does so without the intended scenari
...
🤔 tdb3 reviewed a pull request: "test: avoid internet traffic"
(https://github.com/bitcoin/bitcoin/pull/31646#pullrequestreview-2551472928)
Post-merge ACK 2ed161c5ce648cb66ec3d2941b02d68b6ca4c390

Executed functional tests while running Wireshark.
Observed no node related traffic with external addresses.
Traffic to 0.0.0.1, 0.0.0.2, and 1.2.3.4 was observed on master before this PR was merged (e.g. bb5f76ee013ee439f70e86319d22f308db8a24ea).
📝 manlovepang opened a pull request: "1"
(https://github.com/bitcoin/bitcoin/pull/31659)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...
💬 ryanofsky commented on pull request "kernel: Move kernel-related cache constants to kernel cache":
(https://github.com/bitcoin/bitcoin/pull/31483#discussion_r1915912236)
> Ah, of course. The actual problem is that we are shifting even out of the widened type's range, so it wraps around.

I see. So the max amount you can safely shift is without overflowing is not `std::numeric_limits<W>::digits - 1` but actually `std::numeric_limits<W>::digits - std::numeric_limits<T>::digits` since that is how much free space there is to the right of T's bits in W.

So maybe if you switch to that amount and change the test cases to:

TestOverflow<int8_t, int64_t>(fuzz
...
manlovepang closed a pull request: "1"
(https://github.com/bitcoin/bitcoin/pull/31659)
📝 manlovepang reopened a pull request: "1"
(https://github.com/bitcoin/bitcoin/pull/31659)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...
💬 i-am-yuvi commented on pull request "test: p2p: fix sending of manual INVs in tx download test":
(https://github.com/bitcoin/bitcoin/pull/31658#issuecomment-2591646519)
Concept ACK

Good catch @theStack!
💬 ryanofsky commented on issue "Mining Interface doesn't allow for Bitcoin Core to create blocks when it wants":
(https://github.com/bitcoin/bitcoin/issues/31109#issuecomment-2591648579)
> After the notification fires, the miner calling back in to request a CreateNewBlock run will now have to wait behind `cs_main` for random P2P stuff

I don't see why this would be true. There's no reason Mining.createNewBlock or BlockTemplate.waitNext methods need to use cs_main. cs_main is an implementation detail and not part of the interface. For example we could have:

```c++
std::shared_ptr<CBlockTemplate> g_block_template;
std::mutex g_block_template_mutex;
std::condition_variable g_block
...
🤔 mzumsande reviewed a pull request: "test: p2p: fix sending of manual INVs in tx download test"
(https://github.com/bitcoin/bitcoin/pull/31658#pullrequestreview-2551617694)
Concept ACK

Good find! I think that `p2p_orphan_handling.py` has a similar [spot](https://github.com/bitcoin/bitcoin/blob/e7c479495509c068215b73f6df070af2d406ae15/test/functional/p2p_orphan_handling.py#L431).
💬 i-am-yuvi commented on pull request "test: p2p: fix sending of manual INVs in tx download test":
(https://github.com/bitcoin/bitcoin/pull/31658#issuecomment-2591741584)
It could be good followup! Can you also change that as well @theStack
👍 i-am-yuvi approved a pull request: "test: p2p: fix sending of manual INVs in tx download test"
(https://github.com/bitcoin/bitcoin/pull/31658#pullrequestreview-2551649291)
Tested ACK e0b333682222927d64217b07bb8cfd7ff3139660
fanquake closed a pull request: "1"
(https://github.com/bitcoin/bitcoin/pull/31659)
📝 fanquake locked a pull request: "1"
(https://github.com/bitcoin/bitcoin/pull/31659)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...
💬 maflcko commented on pull request "test: p2p: fix sending of manual INVs in tx download test":
(https://github.com/bitcoin/bitcoin/pull/31658#issuecomment-2591895415)
> It might be good to avoid issues like this in the future, happy to add test framework improvements if someone has a concrete idea.

Would it not be possible to use mocktime and sync_with_ping?
💬 TheCharlatan commented on pull request "kernel: Move kernel-related cache constants to kernel cache":
(https://github.com/bitcoin/bitcoin/pull/31483#discussion_r1916121562)
Yeah, that should work. I'll fuzz with this for some time.