Bitcoin Core Github
42 subscribers
126K links
Download Telegram
🤔 glozow reviewed a pull request: "net_processing: make any misbehavior trigger immediate discouragement"
(https://github.com/bitcoin/bitcoin/pull/29575#pullrequestreview-2128012597)
light code review / concept ACK 6eecba475efd025eb011400af58621ad5823994e
💬 fanquake commented on pull request "ci: move ASan job to GitHub Actions from Cirrus CI":
(https://github.com/bitcoin/bitcoin/pull/30193#issuecomment-2178509616)
Backported to 27.x in #30305.
💬 fanquake commented on pull request "ci: remove unused bcc variable from workflow":
(https://github.com/bitcoin/bitcoin/pull/30299#issuecomment-2178509864)
Backported to 27.x in #30305.
💬 maflcko commented on issue "ci: Move more tasks to GHA?":
(https://github.com/bitcoin/bitcoin/issues/30304#issuecomment-2178513643)
I am not sure how increasing the cache size limit would be possible.
💬 alfonsoromanz commented on pull request "test: Assumeutxo: import snapshot in a node with a divergent chain":
(https://github.com/bitcoin/bitcoin/pull/29996#discussion_r1646036157)
Thanks. I removed the line as suggested. I wasn't sure which chainstates check might be useful to add. Something like [this](https://github.com/bitcoin/bitcoin/blob/master/test/functional/feature_assumeutxo.py#L322C7-L328C51), maybe?
💬 Sjors commented on pull request "contrib: add tool to convert compact-serialized UTXO set to SQLite database":
(https://github.com/bitcoin/bitcoin/pull/27432#issuecomment-2178536123)
Ah nice, I'll try that (tomorrow probably).
💬 alfonsoromanz commented on pull request "test: Assumeutxo: import snapshot in a node with a divergent chain":
(https://github.com/bitcoin/bitcoin/pull/29996#issuecomment-2178578099)
Pushed changes to address the latest feedback from fjahr
💬 AngusP commented on pull request "test: Add Compact Block Encoding test `ReceiveWithExtraTransactions` covering non-empty `extra_txn`":
(https://github.com/bitcoin/bitcoin/pull/30237#discussion_r1646108157)
I'm not familiar with the fuzz tests so I hope this change isn't stupid/wrong?
💬 Eunovo commented on pull request "wallet: Fix listwalletdir listing of migrated default wallets and generated backup files":
(https://github.com/bitcoin/bitcoin/pull/30265#issuecomment-2178623704)
Tested ACK https://github.com/bitcoin/bitcoin/pull/30265/commits/0ce7c6294bf58142de55372d6448416a73f4caa5
I ran the `wallet_migration` test locally and also prepared a wallets directory with a default wallet file, a backup file and a legacy wallet file for manual testing. Running `listwalletdir` on master returned the following output
```
{
"wallets": [
{
"name": "boblegacy_1718798800.legacy.bak"
},
{
"name": "boblegacy"
}
]
}
```
And `listwalletdir`
...
💬 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?