💬 Sjors commented on pull request "Introduce waitTipChanged() mining interface, replace RPCNotifyBlockChange and drop CRPCSignals":
(https://github.com/bitcoin/bitcoin/pull/30409#issuecomment-2283641349)
Rebased and restored signalling `g_best_block_cv` on shutdown.
I created `interfaces/types.h`, moved `BlockKey` there and renamed it to `BlockRef`. I then renamed `getTipHash()` to `getTip()` instead of adding `getTipHeight()`. `waitTipChanged` also returns a `BlockKey`.
Addressed inline comments, except that I still need to look into the tests.
(https://github.com/bitcoin/bitcoin/pull/30409#issuecomment-2283641349)
Rebased and restored signalling `g_best_block_cv` on shutdown.
I created `interfaces/types.h`, moved `BlockKey` there and renamed it to `BlockRef`. I then renamed `getTipHash()` to `getTip()` instead of adding `getTipHeight()`. `waitTipChanged` also returns a `BlockKey`.
Addressed inline comments, except that I still need to look into the tests.
💬 Sjors commented on pull request "Introduce waitTipChanged() mining interface, replace RPCNotifyBlockChange and drop CRPCSignals":
(https://github.com/bitcoin/bitcoin/pull/30409#discussion_r1713527845)
`getTip()` and `waitTipChanged()` now both return `BlockRef`.
(https://github.com/bitcoin/bitcoin/pull/30409#discussion_r1713527845)
`getTip()` and `waitTipChanged()` now both return `BlockRef`.
💬 Sjors commented on pull request "Introduce waitTipChanged() mining interface, replace RPCNotifyBlockChange and drop CRPCSignals":
(https://github.com/bitcoin/bitcoin/pull/30409#discussion_r1713527921)
Done. I also simplified the `waitfornewblock` (etc) RPC call to just call getTip() and then pass that into `waitTipChanged`. That gets rid of `struct CUpdatedBlock`.
(https://github.com/bitcoin/bitcoin/pull/30409#discussion_r1713527921)
Done. I also simplified the `waitfornewblock` (etc) RPC call to just call getTip() and then pass that into `waitTipChanged`. That gets rid of `struct CUpdatedBlock`.
💬 Sjors commented on pull request "Introduce waitTipChanged() mining interface, replace RPCNotifyBlockChange and drop CRPCSignals":
(https://github.com/bitcoin/bitcoin/pull/30409#discussion_r1713527986)
Done
(https://github.com/bitcoin/bitcoin/pull/30409#discussion_r1713527986)
Done
💬 Sjors commented on pull request "Introduce waitTipChanged() mining interface, replace RPCNotifyBlockChange and drop CRPCSignals":
(https://github.com/bitcoin/bitcoin/pull/30409#discussion_r1713528130)
I don't fully understand the original test either. Will look into it later.
(https://github.com/bitcoin/bitcoin/pull/30409#discussion_r1713528130)
I don't fully understand the original test either. Will look into it later.
💬 Sjors commented on pull request "Introduce waitTipChanged() mining interface, replace RPCNotifyBlockChange and drop CRPCSignals":
(https://github.com/bitcoin/bitcoin/pull/30409#discussion_r1713528797)
I made a note to either add that argument in this PR or make a followup.
(https://github.com/bitcoin/bitcoin/pull/30409#discussion_r1713528797)
I made a note to either add that argument in this PR or make a followup.
💬 Sjors commented on pull request "Introduce waitTipChanged() mining interface, replace RPCNotifyBlockChange and drop CRPCSignals":
(https://github.com/bitcoin/bitcoin/pull/30409#discussion_r1713528958)
I moved it to `StopRPC()`.
(https://github.com/bitcoin/bitcoin/pull/30409#discussion_r1713528958)
I moved it to `StopRPC()`.
💬 Sjors commented on pull request "Introduce waitTipChanged() mining interface, replace RPCNotifyBlockChange and drop CRPCSignals":
(https://github.com/bitcoin/bitcoin/pull/30409#discussion_r1713529075)
Done
(https://github.com/bitcoin/bitcoin/pull/30409#discussion_r1713529075)
Done
💬 maprezentalok commented on issue ""heapleakdetection" entry in registry for bitcoin-qt.exe":
(https://github.com/bitcoin/bitcoin/issues/30629#issuecomment-2283688931)
Actually the hex LastDetectionTime translates to:
LDAP: 133626153101189582
Epoch/Unix time: 1718141710
GMT: 2024. June 11., Tuesday 21:35:10
so it cannot be v27.1 as it wasn't released yet, i used v27.0 at that time.
(https://github.com/bitcoin/bitcoin/issues/30629#issuecomment-2283688931)
Actually the hex LastDetectionTime translates to:
LDAP: 133626153101189582
Epoch/Unix time: 1718141710
GMT: 2024. June 11., Tuesday 21:35:10
so it cannot be v27.1 as it wasn't released yet, i used v27.0 at that time.
💬 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.