Bitcoin Core Github
42 subscribers
126K links
Download Telegram
💬 fjahr commented on pull request "test: Assumeutxo: import snapshot in a node with a divergent chain":
(https://github.com/bitcoin/bitcoin/pull/29996#discussion_r1646149435)
Yeah, just that each of them is at the height you expect.
💬 fjahr commented on pull request "test: Assumeutxo: import snapshot in a node with a divergent chain":
(https://github.com/bitcoin/bitcoin/pull/29996#discussion_r1646164004)
> I'm not sure why it needs to "lead to a much earlier error," though, or lead to any error. If the node is syncing to some chain that seems to have the most work, but then headers for a second chain are announced that has more work, the chainstate tip is not going to switch to the second chain, even though it has more work, until enough blocks from it are downloaded and validated, and a block from the second chain is reached that is that is valid and has more work than the chainstate tip. Befor
...
💬 alfonsoromanz commented on pull request "test: Assumeutxo: import snapshot in a node with a divergent chain":
(https://github.com/bitcoin/bitcoin/pull/29996#discussion_r1646189792)
Thanks. Updated
👍 tdb3 approved a pull request: "Introduce Mining interface"
(https://github.com/bitcoin/bitcoin/pull/30200#pullrequestreview-2128264782)
re ACK a9716c53f05082d6d89ebea51a46d4404efb12d7

Re-built and ran unit/functional tests (passed).
Tested https://github.com/bitcoin/bitcoin/pull/30200#issuecomment-2172702822 by mining with cpuminer (https://github.com/pooler/cpuminer) for a custom signet (with `signetchallenge=51` set in bitcoin.conf to allow mining without signature).

Mined for 5000 blocks, and created and spent several transactions (one from 135 coinbase outputs and two derivative transactions). Temporarily paused mini
...
💬 fanquake commented on pull request "depends: build libevent with CMake":
(https://github.com/bitcoin/bitcoin/pull/29835#issuecomment-2178709604)
> Is something required to undraft this PR?

Fixing the broken Windows build. Which is now done.
👋 fanquake's pull request is ready for review: "depends: build libevent with CMake"
(https://github.com/bitcoin/bitcoin/pull/29835)
💬 m3dwards commented on pull request "ci: move ASan job to GitHub Actions from Cirrus CI":
(https://github.com/bitcoin/bitcoin/pull/30193#discussion_r1646226121)
CCache is currently written to cache docker volume which I assume on Cirrus CI gets shared between multiple jobs? As a github actions is more idempotent / ephemeral this isn't going to work as the ccache dir on the host is left empty at the end of the run.

I can see that this is the same strategy used for depends output and sources and previous_releases.

The solution could be to modify `02_run_container.sh` script to have "GHA mode" where instead of using docker volumes we just bind mount
...
👍 TheCharlatan approved a pull request: "fuzz: Fix wallet_bdb_parser 32-bit unhandled fseek error"
(https://github.com/bitcoin/bitcoin/pull/30307#pullrequestreview-2128357217)
ACK fa7bc9bbca9348cf31b97bee0789ea7caeec635c
💬 maflcko commented on pull request "ci: move ASan job to GitHub Actions from Cirrus CI":
(https://github.com/bitcoin/bitcoin/pull/30193#discussion_r1646250212)
Ah right. I forgot about this. Your suggestion sounds good.
💬 maflcko commented on pull request "kernel: De-globalize validation caches":
(https://github.com/bitcoin/bitcoin/pull/30141#discussion_r1646290588)
May be good to explain why this is needed, when `m_node.chainman->m_validation_cache` already exists?
💬 m3dwards commented on issue "ci: Move more tasks to GHA?":
(https://github.com/bitcoin/bitcoin/issues/30304#issuecomment-2178864909)
Is the plan to move everything from Cirrus to GHA?
💬 fanquake commented on pull request "depends: build libevent with CMake":
(https://github.com/bitcoin/bitcoin/pull/29835#issuecomment-2178870629)
Guix Build (aarch64):
```bash
db8e2595bb6b4d7088860f1df0c952be13a4ae3db3396a0c3a42de98165529bc guix-build-a5180bffbe78/output/aarch64-linux-gnu/SHA256SUMS.part
e90d5520e9bec9efe81387012d6b9ddd189ed7f4737726d825cd8077c4c0b50b guix-build-a5180bffbe78/output/aarch64-linux-gnu/bitcoin-a5180bffbe78-aarch64-linux-gnu-debug.tar.gz
a0b5ee10285feddbc964a694c134dcd0bb9193065985423063a353c2e518442b guix-build-a5180bffbe78/output/aarch64-linux-gnu/bitcoin-a5180bffbe78-aarch64-linux-gnu.tar.gz
d49594
...
📝 luke-jr opened a pull request: "QA: Expect PACKAGE_NAME rather than constant "Bitcoin Core""
(https://github.com/bitcoin/bitcoin/pull/30308)
Followup to #29144
💬 dergoegge commented on pull request "test: Add Compact Block Encoding test `ReceiveWithExtraTransactions` covering non-empty `extra_txn`":
(https://github.com/bitcoin/bitcoin/pull/30237#discussion_r1646353890)
No need for the actual rng, we can just consume the nonce from the fuzz data.

```suggestion
CBlockHeaderAndShortTxIDs cmpctblock{*block, fuzzed_data_provider.ConsumeIntegral<uint64_t>()};
```
💬 kevkevinpal commented on pull request "QA: Expect PACKAGE_NAME rather than constant "Bitcoin Core"":
(https://github.com/bitcoin/bitcoin/pull/30308#issuecomment-2178928580)
ACK [197b540](https://github.com/bitcoin/bitcoin/pull/30308/commits/197b5404b0f4a1d6e989000845b45c8bd24e0bc6)

we're doing this in these files aswell so seems fine to me
```
test/functional/wallet_multiwallet.py
test/functional/tool_wallet.py
test/functional/interface_bitcoin_cli.py
test/functional/feature_filelock.py
```
💬 maflcko commented on issue "ci: Move more tasks to GHA?":
(https://github.com/bitcoin/bitcoin/issues/30304#issuecomment-2178933486)
If someone finds a solution to all cache issues, then it can be done. (Moving back should be easy in any case)

For now, see the issue description:

> ideally only tasks with `NO_DEPENDS=1` are moved for now. It would be:
>
> * `ci/test/00_setup_env_native_fuzz.sh:export NO_DEPENDS=1`
>
> * `ci/test/00_setup_env_native_tidy.sh:export NO_DEPENDS=1`
💬 kevkevinpal commented on pull request "QA: Expect PACKAGE_NAME rather than constant "Bitcoin Core"":
(https://github.com/bitcoin/bitcoin/pull/30308#issuecomment-2178943305)
I see we're doing something similar in `./test/functional/wallet_backwards_compatibility.py`

```
node_v17.assert_start_raises_init_error(["-wallet=w3"], "Error: Error loading w3: Wallet requires newer version of Bitcoin Core")

and

node_v16.assert_start_raises_init_error([f"-wallet={wallet_name}"], f"Error: Error loading {wallet_name}: Wallet requires newer version of Bitcoin Core")
```

not sure if you want to update here aswell in this PR
💬 TheCharlatan commented on pull request "kernel: De-globalize validation caches":
(https://github.com/bitcoin/bitcoin/pull/30141#discussion_r1646370563)
I was thinking it might be useful to control the caches directly for the tests, but on second thought that does not seem like a good thing. Will revert.
💬 TheCharlatan commented on pull request "kernel: De-globalize validation caches":
(https://github.com/bitcoin/bitcoin/pull/30141#issuecomment-2178956138)
6ad4aa82056030a8a72b1315676e0703c1500d0d -> fa660a266b3aa03f40c47a9330e3bce8be89bb54 ([noGlobalScriptCache_4](https://github.com/TheCharlatan/bitcoin/tree/noGlobalScriptCache_4) -> [noGlobalScriptCache_5](https://github.com/TheCharlatan/bitcoin/tree/noGlobalScriptCache_5), [compare](https://github.com/TheCharlatan/bitcoin/compare/noGlobalScriptCache_4..noGlobalScriptCache_5))

* Addressed @stickies-v's [comment](https://github.com/bitcoin/bitcoin/pull/30141#discussion_r1645856227), changing in
...
💬 maflcko commented on pull request "kernel: De-globalize validation caches":
(https://github.com/bitcoin/bitcoin/pull/30141#discussion_r1646381635)
nit: You can combine the cast by using `()` over `{}`. Also you can specify the template to avoid the `size_t` cast, and ensure the right template is always picked:

```cpp
uint32_t requested_num_elems(std::min<size_t>(