💬 achow101 commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1191628983)
CI Failure here:
```
C:/Users/ContainerAdministrator/AppData/Local/Temp/cirrus-ci-build/src/test/txpackage_tests.cpp(622): error: in "txpackage_tests/package_submission_tests": check GetVirtualTransactionSize(*tx_parent2) == 191 has failed [190 != 191]
```
I'm going to guess that this is because real signatures are being created, which can occasionally be smaller than 71 bytes, thus resulting in a tx that is one byte smaller. If the script type doesn't matter, I would suggest using tapro
...
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1191628983)
CI Failure here:
```
C:/Users/ContainerAdministrator/AppData/Local/Temp/cirrus-ci-build/src/test/txpackage_tests.cpp(622): error: in "txpackage_tests/package_submission_tests": check GetVirtualTransactionSize(*tx_parent2) == 191 has failed [190 != 191]
```
I'm going to guess that this is because real signatures are being created, which can occasionally be smaller than 71 bytes, thus resulting in a tx that is one byte smaller. If the script type doesn't matter, I would suggest using tapro
...
💬 kroese commented on issue "CPU DoS on mainnet in debug mode":
(https://github.com/bitcoin/bitcoin/issues/27586#issuecomment-1544593209)
I'm also seeing the same issue.. The load average suddenly increased from 0.25 to 1+. From the graph its clear it started on the 6th of May:

(https://github.com/bitcoin/bitcoin/issues/27586#issuecomment-1544593209)
I'm also seeing the same issue.. The load average suddenly increased from 0.25 to 1+. From the graph its clear it started on the 6th of May:

💬 instagibbs commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1191631520)
I'm fine if the magic number checks have a bit of wiggle room tbh, just leave a comment?
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1191631520)
I'm fine if the magic number checks have a bit of wiggle room tbh, just leave a comment?
💬 instagibbs commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1191650786)
if the issue is during warmup, that's a pretty small window, and typically won't actually do parallel fetching, 3 seems rare even when warmed up with limit of 10. I'll gather some more stats on that front, seeing how many parallel blocks are initialized.
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1191650786)
if the issue is during warmup, that's a pretty small window, and typically won't actually do parallel fetching, 3 seems rare even when warmed up with limit of 10. I'll gather some more stats on that front, seeing how many parallel blocks are initialized.
💬 furszy commented on pull request "init: verify blocks data existence only once for all the indexers":
(https://github.com/bitcoin/bitcoin/pull/27607#issuecomment-1544648748)
Thanks for the deep review @ryanofsky!
> Reviewed [86752e0](https://github.com/bitcoin/bitcoin/commit/86752e0cc5bc48f3d4ac1cd07835c37daf078d6a) and it mostly looks good, but does have a null pointer deference currently (see comments and suggested fix). I like this PR because it will simplify #24230 and could potentially improve performance. I'm not actually how sure how much it actually would improve performance in practice though, so I'm curious if that's was the original motivation here or
...
(https://github.com/bitcoin/bitcoin/pull/27607#issuecomment-1544648748)
Thanks for the deep review @ryanofsky!
> Reviewed [86752e0](https://github.com/bitcoin/bitcoin/commit/86752e0cc5bc48f3d4ac1cd07835c37daf078d6a) and it mostly looks good, but does have a null pointer deference currently (see comments and suggested fix). I like this PR because it will simplify #24230 and could potentially improve performance. I'm not actually how sure how much it actually would improve performance in practice though, so I'm curious if that's was the original motivation here or
...
💬 TheBlueMatt commented on issue "One core in CPU usage rate remains at 100% for a long time, causing serious delays in new blocks and forks":
(https://github.com/bitcoin/bitcoin/issues/27623#issuecomment-1544664280)
Are you not running any code to submit blocks across multiple bitcoin daemons quickly such as redundant nodes using the old FIBRE patchset?
(https://github.com/bitcoin/bitcoin/issues/27623#issuecomment-1544664280)
Are you not running any code to submit blocks across multiple bitcoin daemons quickly such as redundant nodes using the old FIBRE patchset?
💬 furszy commented on pull request "wallet: fix deadlock in bdb read write operation":
(https://github.com/bitcoin/bitcoin/pull/27556#discussion_r1191686510)
It is true while you use the db handler only for reading at that point. In that case, you don't need the transaction argument and `batch.txn()` returns a `nullptr`.
If you mix an already open read cursor with some write operations, then the code is no longer performing a read-only operation, so it will need to provide the transaction arg (probably by re-arranging the code in the same way as did with `EraseRecords`).
(https://github.com/bitcoin/bitcoin/pull/27556#discussion_r1191686510)
It is true while you use the db handler only for reading at that point. In that case, you don't need the transaction argument and `batch.txn()` returns a `nullptr`.
If you mix an already open read cursor with some write operations, then the code is no longer performing a read-only operation, so it will need to provide the transaction arg (probably by re-arranging the code in the same way as did with `EraseRecords`).
💬 furszy commented on pull request "wallet: fix deadlock in bdb read write operation":
(https://github.com/bitcoin/bitcoin/pull/27556#discussion_r1191688310)
Yeah, I explained the reason inside the commit description ca1871d48558b356257d2a95b046f63739dfe3a0:
> extra note from the Db.cursor doc:
"If transaction protection is enabled, cursors must be
opened and closed within the context of a transaction"
>
> thus why added a `CloseCursor` call before calling to
`TxnAbort/TxnCommit`.
The `CloseCursor` call is the ptr reset.
Just extra safety measures.
(https://github.com/bitcoin/bitcoin/pull/27556#discussion_r1191688310)
Yeah, I explained the reason inside the commit description ca1871d48558b356257d2a95b046f63739dfe3a0:
> extra note from the Db.cursor doc:
"If transaction protection is enabled, cursors must be
opened and closed within the context of a transaction"
>
> thus why added a `CloseCursor` call before calling to
`TxnAbort/TxnCommit`.
The `CloseCursor` call is the ptr reset.
Just extra safety measures.
💬 sr-gi commented on pull request "net processing: avoid serving non-announced txs as a result of a MEMPOOL message":
(https://github.com/bitcoin/bitcoin/pull/27602#issuecomment-1544692524)
> > > > `txinfo.m_time <= mempool_req`
> >
> >
> > > This condition is checking that our inv (`txinfo`) is more recent than the last `mempool_req`, meaning that once a mempool request has been processed the peer can request any transaction from our mempool.
> >
> >
> > I don't get it. Is it not that the condition is checking whether `txinfo` is _older_ than the last `mempool_req`? In that case the transaction has been part of the `MEMPOOL` reply, so we have sent an `INV` about it.
>
...
(https://github.com/bitcoin/bitcoin/pull/27602#issuecomment-1544692524)
> > > > `txinfo.m_time <= mempool_req`
> >
> >
> > > This condition is checking that our inv (`txinfo`) is more recent than the last `mempool_req`, meaning that once a mempool request has been processed the peer can request any transaction from our mempool.
> >
> >
> > I don't get it. Is it not that the condition is checking whether `txinfo` is _older_ than the last `mempool_req`? In that case the transaction has been part of the `MEMPOOL` reply, so we have sent an `INV` about it.
>
...
📝 theStack opened a pull request: "test: avoid sporadic MINIMALDATA failure in feature_taproot.py (fixes #27595)"
(https://github.com/bitcoin/bitcoin/pull/27631)
The functional test feature_taproot.py fails in some rare cases on the execution of the following `"branched_codesep"` spending script (can be reproduced via `$ ./test/functional/feature_taproot.py --randomseed 9048710178866422833` on master / 137a98c5a22e058ed7a7997a0a4dbd75301de51e):
https://github.com/bitcoin/bitcoin/blob/9d85c03620bf660cfa7d13080f5c0b191579cbc3/test/functional/feature_taproot.py#L741
The problem occurs if the first data-push (having random content with a random lengt
...
(https://github.com/bitcoin/bitcoin/pull/27631)
The functional test feature_taproot.py fails in some rare cases on the execution of the following `"branched_codesep"` spending script (can be reproduced via `$ ./test/functional/feature_taproot.py --randomseed 9048710178866422833` on master / 137a98c5a22e058ed7a7997a0a4dbd75301de51e):
https://github.com/bitcoin/bitcoin/blob/9d85c03620bf660cfa7d13080f5c0b191579cbc3/test/functional/feature_taproot.py#L741
The problem occurs if the first data-push (having random content with a random lengt
...
💬 ismaelsadeeq commented on pull request "Fee estimation: avoid serving stale fee estimate":
(https://github.com/bitcoin/bitcoin/pull/27622#discussion_r1191715247)
Sure :+1: , fixed.
(https://github.com/bitcoin/bitcoin/pull/27622#discussion_r1191715247)
Sure :+1: , fixed.
💬 sipa commented on pull request "test: avoid sporadic MINIMALDATA failure in feature_taproot.py (fixes #27595)":
(https://github.com/bitcoin/bitcoin/pull/27631#issuecomment-1544738994)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/27631#issuecomment-1544738994)
Concept ACK
🤔 stickies-v reviewed a pull request: "net: Allow inbound whitebind connections to more aggresivey evict peers when slots are full"
(https://github.com/bitcoin/bitcoin/pull/27600#pullrequestreview-1423467785)
Concept ACK, makes sense to prioritize whitelisted peers. Will review more in-depth soon.
(https://github.com/bitcoin/bitcoin/pull/27600#pullrequestreview-1423467785)
Concept ACK, makes sense to prioritize whitelisted peers. Will review more in-depth soon.
💬 stickies-v commented on pull request "net: Allow inbound whitebind connections to more aggresivey evict peers when slots are full":
(https://github.com/bitcoin/bitcoin/pull/27600#discussion_r1191709646)
nit
```suggestion
return force ? std::make_optional(last_out.id) : std::nullopt;
```
(https://github.com/bitcoin/bitcoin/pull/27600#discussion_r1191709646)
nit
```suggestion
return force ? std::make_optional(last_out.id) : std::nullopt;
```
💬 jonatack commented on pull request "net: Allow inbound whitebind connections to more aggresivey evict peers when slots are full":
(https://github.com/bitcoin/bitcoin/pull/27600#discussion_r1191732820)
Not sure, but IIRC last time I looked RVO wasn't working with a ternary.
(https://github.com/bitcoin/bitcoin/pull/27600#discussion_r1191732820)
Not sure, but IIRC last time I looked RVO wasn't working with a ternary.
💬 ryanofsky commented on pull request "Multiprocess bitcoin":
(https://github.com/bitcoin/bitcoin/pull/10102#issuecomment-1544783273)
Thanks @Sjors. I've tried a lot of things and have not been able to reproduce the crash exactly, but the log you posted was helpful, and I could reproduce similar error output by adding an extra call from bitcoin-node to bitcoin-wallet that sleeps in bitcoin-wallet, and then manual killing `bitcoin-wallet` process. When I do that I see similar `kj::Exception` errors on the bitcoin-node main thread and bitcoin-node b-capnp-loop thread like in your log. In your log the exact errors are `kj/async
...
(https://github.com/bitcoin/bitcoin/pull/10102#issuecomment-1544783273)
Thanks @Sjors. I've tried a lot of things and have not been able to reproduce the crash exactly, but the log you posted was helpful, and I could reproduce similar error output by adding an extra call from bitcoin-node to bitcoin-wallet that sleeps in bitcoin-wallet, and then manual killing `bitcoin-wallet` process. When I do that I see similar `kj::Exception` errors on the bitcoin-node main thread and bitcoin-node b-capnp-loop thread like in your log. In your log the exact errors are `kj/async
...
📝 jonatack opened a pull request: "Raise on invalid -debug and -loglevel config options"
(https://github.com/bitcoin/bitcoin/pull/27632)
and rename BCLog::BLOCKSTORE to BLOCKSTORAGE so the enum is the same as its value like the other BCLog enums.
Per discussion in bitcoin-core-dev IRC today from https://bitcoin-irc.chaincode.com/bitcoin-core-dev/2023-05-11#921458.
(https://github.com/bitcoin/bitcoin/pull/27632)
and rename BCLog::BLOCKSTORE to BLOCKSTORAGE so the enum is the same as its value like the other BCLog enums.
Per discussion in bitcoin-core-dev IRC today from https://bitcoin-irc.chaincode.com/bitcoin-core-dev/2023-05-11#921458.
💬 amitiuttarwar commented on pull request "rpc: add 'getnetmsgstats', new rpc to view network message statistics":
(https://github.com/bitcoin/bitcoin/pull/27534#discussion_r1191790448)
@satsie is the concern of having a dummy value like `NET_MAX` that it could fall out of
(https://github.com/bitcoin/bitcoin/pull/27534#discussion_r1191790448)
@satsie is the concern of having a dummy value like `NET_MAX` that it could fall out of
🤔 amitiuttarwar reviewed a pull request: "rpc: add 'getnetmsgstats', new rpc to view network message statistics"
(https://github.com/bitcoin/bitcoin/pull/27534#pullrequestreview-1423599288)
approach ACK. did a high-level review and left a few small comments
(https://github.com/bitcoin/bitcoin/pull/27534#pullrequestreview-1423599288)
approach ACK. did a high-level review and left a few small comments
💬 amitiuttarwar commented on pull request "rpc: add 'getnetmsgstats', new rpc to view network message statistics":
(https://github.com/bitcoin/bitcoin/pull/27534#discussion_r1191793368)
this function should handle if `index > NUM_NET_MESSAGE_TYPES`
(https://github.com/bitcoin/bitcoin/pull/27534#discussion_r1191793368)
this function should handle if `index > NUM_NET_MESSAGE_TYPES`