💬 agroce commented on issue "fuzz: `script`: Assertion `!extract_destination_ret' failed.":
(https://github.com/bitcoin/bitcoin/issues/30615#issuecomment-2283704534)
> I think we have a few harnesses that exit early if the input size is smaller or larger than expected, e.g.:
Not sure the equivalent in AFL++, but in libFuzzer (and Eclipser) for some targets really limiting the size of input gives the mutator a much better chance at finding lots of things. This is a good example of very different time-to-find depending on max_len. It might be a good idea to either add a (rare) config for targets with some appropriate "very short but long enough to be inte
...
(https://github.com/bitcoin/bitcoin/issues/30615#issuecomment-2283704534)
> I think we have a few harnesses that exit early if the input size is smaller or larger than expected, e.g.:
Not sure the equivalent in AFL++, but in libFuzzer (and Eclipser) for some targets really limiting the size of input gives the mutator a much better chance at finding lots of things. This is a good example of very different time-to-find depending on max_len. It might be a good idea to either add a (rare) config for targets with some appropriate "very short but long enough to be inte
...
💬 glozow commented on pull request "cluster mempool: optimized candidate search":
(https://github.com/bitcoin/bitcoin/pull/30286#discussion_r1713603196)
@sipa thanks this is really helpful, particularly this part:
> 4. anc(t) will always include at least one UL transaction (*)
> (*) The jump ahead is exactly what guarantees that anc(t) always includes a UL transaction. If anc(t) was entirely within UH, it would be topologically valid, and thus the parent work item's jump ahead step would have moved it to I already.
However I'm still having trouble with this part:
> Compare this to what happens in an exclusion branch ...
> 4. desc(t) m
...
(https://github.com/bitcoin/bitcoin/pull/30286#discussion_r1713603196)
@sipa thanks this is really helpful, particularly this part:
> 4. anc(t) will always include at least one UL transaction (*)
> (*) The jump ahead is exactly what guarantees that anc(t) always includes a UL transaction. If anc(t) was entirely within UH, it would be topologically valid, and thus the parent work item's jump ahead step would have moved it to I already.
However I'm still having trouble with this part:
> Compare this to what happens in an exclusion branch ...
> 4. desc(t) m
...
📝 Sjors opened a pull request: "rpc: add optional blockhash to waitfornewblock"
(https://github.com/bitcoin/bitcoin/pull/30635)
The `waitfornewblock` is inherently racy as the tip may have changed since the last RPC call, and can even change during initial processing of this call.
Add an optional `blockhash` argument so the called can specify their current tip. Return immediately if our tip is different.
I've made it fail if `LookupBlockIndex` fails. This should never happen if the user got the block hash from our RPC in the first place.
Builds on #30409.
(https://github.com/bitcoin/bitcoin/pull/30635)
The `waitfornewblock` is inherently racy as the tip may have changed since the last RPC call, and can even change during initial processing of this call.
Add an optional `blockhash` argument so the called can specify their current tip. Return immediately if our tip is different.
I've made it fail if `LookupBlockIndex` fails. This should never happen if the user got the block hash from our RPC in the first place.
Builds on #30409.
💬 0xB10C commented on issue "Intermittent failures in interface_usdt_mempool.py":
(https://github.com/bitcoin/bitcoin/issues/27380#issuecomment-2283747252)
Thanks! Did a few CI run of https://github.com/0xB10C/bitcoin/commit/3d806eafa04415791b95ed20525eed2fd54f5b38. Your patch doesn't seem to improve the situation.
- Attempt 1 failed: https://github.com/0xB10C/bitcoin/actions/runs/10348240621/attempts/1 (test iteration 4/100)
- Attempt 2 failed: https://github.com/0xB10C/bitcoin/actions/runs/10348240621/attempts/2 (test iteration 18/100)
- Attempt 3 succeded: https://github.com/0xB10C/bitcoin/actions/runs/10348240621/attempts/3
- Attempt 4 su
...
(https://github.com/bitcoin/bitcoin/issues/27380#issuecomment-2283747252)
Thanks! Did a few CI run of https://github.com/0xB10C/bitcoin/commit/3d806eafa04415791b95ed20525eed2fd54f5b38. Your patch doesn't seem to improve the situation.
- Attempt 1 failed: https://github.com/0xB10C/bitcoin/actions/runs/10348240621/attempts/1 (test iteration 4/100)
- Attempt 2 failed: https://github.com/0xB10C/bitcoin/actions/runs/10348240621/attempts/2 (test iteration 18/100)
- Attempt 3 succeded: https://github.com/0xB10C/bitcoin/actions/runs/10348240621/attempts/3
- Attempt 4 su
...
💬 Sjors commented on pull request "Introduce waitTipChanged() mining interface, replace RPCNotifyBlockChange and drop CRPCSignals":
(https://github.com/bitcoin/bitcoin/pull/30409#issuecomment-2283747278)
As for race conditions with RPC calls that rely on `waitForBlock()`:
* `waitforblock [hash]`: the while loop breaks if `block.hash == hash`, which together with the early return inside `waitTipChanged` should prevent any blockage
* `waitforblockheight`: similar to `waitforblock`, the while loop checks `block.height >= height`
* `waitfornewblock`: I opened a draft PR to add an optional `blockhash` argument #30635
(https://github.com/bitcoin/bitcoin/pull/30409#issuecomment-2283747278)
As for race conditions with RPC calls that rely on `waitForBlock()`:
* `waitforblock [hash]`: the while loop breaks if `block.hash == hash`, which together with the early return inside `waitTipChanged` should prevent any blockage
* `waitforblockheight`: similar to `waitforblock`, the while loop checks `block.height >= height`
* `waitfornewblock`: I opened a draft PR to add an optional `blockhash` argument #30635
📝 theStack opened a pull request: "test: assumeutxo: check that UTXO-querying RPCs operate on snapshot chainstate"
(https://github.com/bitcoin/bitcoin/pull/30636)
Inspired by some manual testing I did for #28553, this PR checks that RPCs which explicitly query the UTXO set database (i.e. `gettxoutsetinfo`, `scantxoutset` and `gettxout`) operate on the snapshot chainstate as expected.
(https://github.com/bitcoin/bitcoin/pull/30636)
Inspired by some manual testing I did for #28553, this PR checks that RPCs which explicitly query the UTXO set database (i.e. `gettxoutsetinfo`, `scantxoutset` and `gettxout`) operate on the snapshot chainstate as expected.
👍 stickies-v approved a pull request: "Fixes for GCC 15 compatibility"
(https://github.com/bitcoin/bitcoin/pull/30633#pullrequestreview-2232801790)
ACK 055bc05792ff5d5b084563044818ebec12bfd748
(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()
...
(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)
(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.
(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.
(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.
(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
...
(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
(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.
(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
(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`.
(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
...
(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.
(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)
(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.
(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.