💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1669602373)
Done.
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1669602373)
Done.
💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#issuecomment-2216328752)
Added a `CoinsViewCacheCursor` struct to encapsulate the iteration of the linked list when passed up to `BatchWrite`.
(https://github.com/bitcoin/bitcoin/pull/28280#issuecomment-2216328752)
Added a `CoinsViewCacheCursor` struct to encapsulate the iteration of the linked list when passed up to `BatchWrite`.
📝 mzumsande opened a pull request: "rpc, rest: Improve getblock error when only header is available"
(https://github.com/bitcoin/bitcoin/pull/30410)
Fixes #20978
If a block was pruned, `getblock` already returns a specific error: "Block not available (pruned data)".
But if we haven't received the full block yet (e.g. in a race with block downloading after a new block was received headers-first, or during IBD) we just return an unspecific "Block not found on disk" error and log
`ERROR: ReadBlockFromDisk: OpenBlockFile failed for FlatFilePos(nFile=-1, nPos=0) `
which suggest something went wrong even though this is a completely normal an
...
(https://github.com/bitcoin/bitcoin/pull/30410)
Fixes #20978
If a block was pruned, `getblock` already returns a specific error: "Block not available (pruned data)".
But if we haven't received the full block yet (e.g. in a race with block downloading after a new block was received headers-first, or during IBD) we just return an unspecific "Block not found on disk" error and log
`ERROR: ReadBlockFromDisk: OpenBlockFile failed for FlatFilePos(nFile=-1, nPos=0) `
which suggest something went wrong even though this is a completely normal an
...
💬 mzumsande commented on issue "RPC `getblock` resulted in 500 and ReadBlockFromDisk: OpenBlockFile failed for FlatFilePos(nFile=-1, nPos=0)":
(https://github.com/bitcoin/bitcoin/issues/20978#issuecomment-2216427835)
see #30410
(https://github.com/bitcoin/bitcoin/issues/20978#issuecomment-2216427835)
see #30410
💬 TheCharlatan commented on pull request "rpc, rest: Improve getblock error when only header is available":
(https://github.com/bitcoin/bitcoin/pull/30410#issuecomment-2216485352)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/30410#issuecomment-2216485352)
Concept ACK
💬 melvincarvalho commented on pull request "Testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2216495066)
I've been running this for about a month, observing the blocks closely. For the most part I have found it to work really well. Much easier to work with than testnet3. There is a strange occurrence that happens, not sure I would call it an "attack" but someone is shifting timestamps by exactly 20 minutes 1 second. Sometimes several times in a row. Generally it is up to 6 blocks at most. I am not sure what the exact trigger is, but it doesnt happen too often.

I've been running this for about a month, observing the blocks closely. For the most part I have found it to work really well. Much easier to work with than testnet3. There is a strange occurrence that happens, not sure I would call it an "attack" but someone is shifting timestamps by exactly 20 minutes 1 second. Sometimes several times in a row. Generally it is up to 6 blocks at most. I am not sure what the exact trigger is, but it doesnt happen too often.

(https://github.com/bitcoin/bitcoin/pull/30411)
✅ sidhujag closed a pull request: "Governanceupdate"
(https://github.com/bitcoin/bitcoin/pull/30411)
(https://github.com/bitcoin/bitcoin/pull/30411)
💬 Sjors commented on pull request "Testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2216695796)
> someone is shifting timestamps by exactly 20 minutes 1 second
This lets them produce blocks at difficulty 1, i.e. with a CPU instead of an ASIC.
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2216695796)
> someone is shifting timestamps by exactly 20 minutes 1 second
This lets them produce blocks at difficulty 1, i.e. with a CPU instead of an ASIC.
💬 jsarenik commented on pull request "Testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2216737254)
> > someone is shifting timestamps by exactly 20 minutes 1 second
>
> This lets them produce blocks at difficulty 1, i.e. with a CPU instead of an ASIC.
@Sjors, how can I do that, too? What tools could be used to produce a block header with a particular shifted timestamp so that in the next step one could use `bitcoin-util grind` to perform proof of work on hex header string (i.e. CPU-mine such a block)?
What conditions need to be met to cause a blockchain reorg this way?
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2216737254)
> > someone is shifting timestamps by exactly 20 minutes 1 second
>
> This lets them produce blocks at difficulty 1, i.e. with a CPU instead of an ASIC.
@Sjors, how can I do that, too? What tools could be used to produce a block header with a particular shifted timestamp so that in the next step one could use `bitcoin-util grind` to perform proof of work on hex header string (i.e. CPU-mine such a block)?
What conditions need to be met to cause a blockchain reorg this way?
💬 maflcko commented on pull request "refactor: Remove Span operator==, Use std::ranges::equal":
(https://github.com/bitcoin/bitcoin/pull/29071#issuecomment-2216745065)
rebased
(https://github.com/bitcoin/bitcoin/pull/29071#issuecomment-2216745065)
rebased
🤔 maflcko reviewed a pull request: "policy: Add PayToAnchor(P2A), `OP_TRUE <0x4e73>` as a standard output script for spending"
(https://github.com/bitcoin/bitcoin/pull/30352#pullrequestreview-2165268157)
See https://github.com/bitcoin/bitcoin/issues/29806 for the wallet intermittent unrelated test issue.
(https://github.com/bitcoin/bitcoin/pull/30352#pullrequestreview-2165268157)
See https://github.com/bitcoin/bitcoin/issues/29806 for the wallet intermittent unrelated test issue.
💬 maflcko commented on pull request "policy: Add PayToAnchor(P2A), `OP_TRUE <0x4e73>` as a standard output script for spending":
(https://github.com/bitcoin/bitcoin/pull/30352#discussion_r1669873936)
nit: May be cleaner to defer to script serialization instead of doing it manually?
```suggestion
nested_true_tx_spend.vin[0].scriptSig = CScript([bytes(PAY_TO_ANCHOR)])
```
(https://github.com/bitcoin/bitcoin/pull/30352#discussion_r1669873936)
nit: May be cleaner to defer to script serialization instead of doing it manually?
```suggestion
nested_true_tx_spend.vin[0].scriptSig = CScript([bytes(PAY_TO_ANCHOR)])
```
💬 maflcko commented on pull request "Fix issues with CI on forks":
(https://github.com/bitcoin/bitcoin/pull/29274#issuecomment-2216852393)
ACK 576828e732bacb61b608cd89f669a2936d3e8d00
(https://github.com/bitcoin/bitcoin/pull/29274#issuecomment-2216852393)
ACK 576828e732bacb61b608cd89f669a2936d3e8d00
💬 Sjors commented on pull request "Introduce waitTipChanged() mining interface, replace RPCNotifyBlockChange and drop CRPCSignals":
(https://github.com/bitcoin/bitcoin/pull/30409#issuecomment-2216910420)
Some of the functional tests that use the affected RPC calls still fail under TSAN, so keeping this draft until that's resolved.
(https://github.com/bitcoin/bitcoin/pull/30409#issuecomment-2216910420)
Some of the functional tests that use the affected RPC calls still fail under TSAN, so keeping this draft until that's resolved.
💬 dergoegge commented on pull request "net: fix race condition in self-connect detection":
(https://github.com/bitcoin/bitcoin/pull/30394#discussion_r1670008364)
This isn't fully atomic, i.e. in theory two threads could enter this section at the same time.
Not tested but maybe the following works:
```suggestion
if (!pto->IsInboundConn() && peer->m_initial_version_message_sent.compare_exchange_strong(false, true)) {
PushNodeVersion(*pto, *peer);
}
```
Alternatively, we could guard `m_initial_version_message_sent` with `NetEventsInterface::g_msgproc_mutex`, since it is only accessed from the "msghand" thread.
(https://github.com/bitcoin/bitcoin/pull/30394#discussion_r1670008364)
This isn't fully atomic, i.e. in theory two threads could enter this section at the same time.
Not tested but maybe the following works:
```suggestion
if (!pto->IsInboundConn() && peer->m_initial_version_message_sent.compare_exchange_strong(false, true)) {
PushNodeVersion(*pto, *peer);
}
```
Alternatively, we could guard `m_initial_version_message_sent` with `NetEventsInterface::g_msgproc_mutex`, since it is only accessed from the "msghand" thread.
💬 dergoegge commented on pull request "net: fix race condition in self-connect detection":
(https://github.com/bitcoin/bitcoin/pull/30394#discussion_r1669983154)
(not in this PR but) Since we don't want any messages send in `InitializeNode`, maybe it makes sense to disallow that somehow, perhaps by not passing a `CNode`?
(https://github.com/bitcoin/bitcoin/pull/30394#discussion_r1669983154)
(not in this PR but) Since we don't want any messages send in `InitializeNode`, maybe it makes sense to disallow that somehow, perhaps by not passing a `CNode`?
💬 m3dwards commented on pull request "net: Allow -proxy=[::1] on nodes with IPV6 lo only":
(https://github.com/bitcoin/bitcoin/pull/30245#issuecomment-2217001579)
@pinheadmz @tdb3 I should have addressed all of your comments.
@vasild the change since your review is rather than always check twice and discard duplicates, we now only check on error. I chose not to look for the specific error as it was not POSIX and was different on different platforms.
(https://github.com/bitcoin/bitcoin/pull/30245#issuecomment-2217001579)
@pinheadmz @tdb3 I should have addressed all of your comments.
@vasild the change since your review is rather than always check twice and discard duplicates, we now only check on error. I chose not to look for the specific error as it was not POSIX and was different on different platforms.
💬 Sjors commented on pull request "Introduce waitTipChanged() mining interface, replace RPCNotifyBlockChange and drop CRPCSignals":
(https://github.com/bitcoin/bitcoin/pull/30409#issuecomment-2217017610)
Fixed the deadlock. I ran into the same issue before when working on #29432: `UpdateTip` requires holding `cs_main`. It then takes `g_best_block_mutex` in order to announces the new block. So then lines after `g_best_block_cv.wait_until` should release `g_best_block_mutex` before trying to lock `cs_main`. That's not a problem here.
(https://github.com/bitcoin/bitcoin/pull/30409#issuecomment-2217017610)
Fixed the deadlock. I ran into the same issue before when working on #29432: `UpdateTip` requires holding `cs_main`. It then takes `g_best_block_mutex` in order to announces the new block. So then lines after `g_best_block_cv.wait_until` should release `g_best_block_mutex` before trying to lock `cs_main`. That's not a problem here.
👋 Sjors's pull request is ready for review: "Introduce waitTipChanged() mining interface, replace RPCNotifyBlockChange and drop CRPCSignals"
(https://github.com/bitcoin/bitcoin/pull/30409)
(https://github.com/bitcoin/bitcoin/pull/30409)