Bitcoin Core Github
44 subscribers
120K links
Download Telegram
👍 stickies-v approved a pull request: "Fixes for GCC 15 compatibility"
(https://github.com/bitcoin/bitcoin/pull/30633#pullrequestreview-2232801790)
ACK 055bc05792ff5d5b084563044818ebec12bfd748
💬 fanquake commented on pull request "test: assumeutxo: check that UTXO-querying RPCs operate on snapshot chainstate":
(https://github.com/bitcoin/bitcoin/pull/30636#issuecomment-2283913115)
https://github.com/bitcoin/bitcoin/actions/runs/10351325972/job/28649600800?pr=30636#step:7:23340:
```bash
test 2024-08-12T12:48:42.142000Z TestFramework (ERROR): JSONRPC error
Traceback (most recent call last):
File "/home/runner/work/_temp/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/test_framework.py", line 132, in main
self.run_test()

...
🚀 fanquake merged a pull request: "Fixes for GCC 15 compatibility"
(https://github.com/bitcoin/bitcoin/pull/30633)
💬 sipa commented on pull request "cluster mempool: optimized candidate search":
(https://github.com/bitcoin/bitcoin/pull/30286#discussion_r1713731387)
@glozow Yeah I only re-discovered (and re-tested...) this fact (that after moving from new-UH to I in an exclusion branch, that former-UH also become eligible to move to I) while writing this. I don't have a good explanation for why it doesn't affect the further run of the algorithm though.
💬 fanquake commented on pull request "Fixes for GCC 15 compatibility":
(https://github.com/bitcoin/bitcoin/pull/30633#issuecomment-2283953684)
Backported to 27.x in #30558.
💬 fanquake commented on pull request "[27.x] Even more backports":
(https://github.com/bitcoin/bitcoin/pull/30558#discussion_r1713746902)
Added.
👍 hodlinator approved a pull request: "contrib: support reading XORed blocks in linearize-data.py script"
(https://github.com/bitcoin/bitcoin/pull/30607#pullrequestreview-2232995233)
ACK 77ff0ec1f185b818b30877de2bedc1750319e6c4

Good to see follow-up work for #28052 being carried out.

1cbc5ae87a8efe44226080ceaaaf10c5575d629d explains purpose of linearize-scripts (as it was not immediately apparent to me). Could be cherry-picked into this PR, or left for a further follow-up.

1. `bitcoind -regtest &`
2. `bitcoin-cli -regtest getblockcount` returns height=120.
3. `contrib/linearize/linearize-hashes.py contrib/linearize/example-linearize.cfg > hashlist.txt` (with modif
...
💬 fjahr commented on pull request "test: assumeutxo: check that UTXO-querying RPCs operate on snapshot chainstate":
(https://github.com/bitcoin/bitcoin/pull/30636#issuecomment-2283996399)
Concept ACK
💬 hebasto commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1713786718)
Also https://github.com/hebasto/bitcoin/pull/324.
💬 hebasto commented on pull request "contrib: fix check-deps.sh to check for weak symbols":
(https://github.com/bitcoin/bitcoin/pull/30415#issuecomment-2284008706)
cc @maflcko @fanquake
💬 Sjors commented on pull request "Introduce waitTipChanged() mining interface, replace RPCNotifyBlockChange and drop CRPCSignals":
(https://github.com/bitcoin/bitcoin/pull/30409#issuecomment-2284019673)
I simplified the `waitforblock` and `waitfornewblock` tests.

They're not very thorough at the moment, because `reconsiderblock` is either atomic or too fast. A better way to test the asynchronous behaviour would be to spin up a second node, and feed it blocks through `add_p2p_connection`.
💬 furszy commented on issue "An "output descriptor" should not have many different checksums":
(https://github.com/bitcoin/bitcoin/issues/30632#issuecomment-2284066800)
The goal of the checksum is to protect the human-readable string from data transmission errors, not to establish a global output identifier. You can specify the same output script in different ways, and this doesn't mean they will share the same checksum. Look:

1. Your descriptor `"wpkh([b8688df1]xprv9s21ZrQH143K2x4gnzRB1eZDq92Uuvy9CXbvgQGdvykXZ9mkkot6LBjzDpgaAfvzkuxJe9JKJXQ38VoPutxvACA5MsyoBs5UyQ4HZKGshGs/84'/0'/0'/0/0)#mpfvuvu6"`
2. Descriptor without origin `"wpkh(xprv9s21ZrQH143K2x4gnzRB
...
💬 rkrux commented on pull request "BlockAssembler: return selected packages vsize and feerate":
(https://github.com/bitcoin/bitcoin/pull/30391#discussion_r1713862201)
Thank you for accepting the suggestion, the diff now has become smaller and to the point.
👍 rkrux approved a pull request: "BlockAssembler: return selected packages vsize and feerate"
(https://github.com/bitcoin/bitcoin/pull/30391#pullrequestreview-2233172836)
reACK [8a830f1](https://github.com/bitcoin/bitcoin/commit/8a830f15cf4923f57abf0bdf30b98b04b47c6e7b)
🤔 glozow reviewed a pull request: "wallet: Fix listwalletdir listing of migrated default wallets and generated backup files"
(https://github.com/bitcoin/bitcoin/pull/30265#pullrequestreview-2233246739)
ACK 6b2dcba07670f04f32c0dc3a2c86fd805c85f12d

Tested by creating legacy default and non-default wallets, then migrating, then using `listwalletdir`. I can see the default wallet is listed as a .bak on master, then disappears on 3ddbdd1815c676a88345b3b0e55a551d2a569e28, then is listed with its name on 6b2dcba07670f04f32c0dc3a2c86fd805c85f12d.
💬 hebasto commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#issuecomment-2284184677)
@fanquake [wrote](https://github.com/bitcoin/bitcoin/pull/30454#issuecomment-2277886777):
> I think we can improve the output when `-DWITH_CCACHE=OFF` is used. Depending on the system, that output might be:
>
> ```shell
> cmake -B build -DWITH_CCACHE=OFF
> < snip >
> Use ccache for compiling .............. ccache masquerades as the compiler
> ```
>
> We should probably at least indicate that the option was respected by the build-system.

The issue, along with another bug I've notice
...
🤔 fjahr reviewed a pull request: "test: assumeutxo: check that UTXO-querying RPCs operate on snapshot chainstate"
(https://github.com/bitcoin/bitcoin/pull/30636#pullrequestreview-2233277476)
Code review ACK 2e9072c137e81c75c58d0c0788295c10fdafdc9b

This is a good regression test to have. I left a bunch of minor nits but this is fine to merge as-is so feel free to leave them unless you have to re-touch.
💬 fjahr commented on pull request "test: assumeutxo: check that UTXO-querying RPCs operate on snapshot chainstate":
(https://github.com/bitcoin/bitcoin/pull/30636#discussion_r1713929119)
nit

```suggestion
assert_equal(scan_result['bestblock'], snapshot_hash)
```
💬 fjahr commented on pull request "test: assumeutxo: check that UTXO-querying RPCs operate on snapshot chainstate":
(https://github.com/bitcoin/bitcoin/pull/30636#discussion_r1713932403)
nit: Would be a bit more readable IMO if you would break it into two lines
💬 fjahr commented on pull request "test: assumeutxo: check that UTXO-querying RPCs operate on snapshot chainstate":
(https://github.com/bitcoin/bitcoin/pull/30636#discussion_r1713927674)
nit: could have moved this up and used it in the `gettxoutsetinfo` as well
💬 fjahr commented on pull request "test: assumeutxo: check that UTXO-querying RPCs operate on snapshot chainstate":
(https://github.com/bitcoin/bitcoin/pull/30636#discussion_r1713931438)
On second thought: maybe just pull all three of these loaded keys out in the beginning since you use each of them at least twice and they are used again further below... or you could deduplicate the `assert_equal` triplet but it's not a big deal either way...