👍 instagibbs approved a pull request: "test: Add test to check tx in the last block can be downloaded"
(https://github.com/bitcoin/bitcoin/pull/27695#pullrequestreview-1433101407)
ACK fa4c16b186a34f4172deda617166813c8cb92c59
broke test intentionally by disabling non-mempool checks in `PeerManagerImpl::FindTxForGetData`, resulted in expected failure
(https://github.com/bitcoin/bitcoin/pull/27695#pullrequestreview-1433101407)
ACK fa4c16b186a34f4172deda617166813c8cb92c59
broke test intentionally by disabling non-mempool checks in `PeerManagerImpl::FindTxForGetData`, resulted in expected failure
💬 furszy commented on pull request "index: improve initialization and pruning violation check":
(https://github.com/bitcoin/bitcoin/pull/27607#issuecomment-1553369947)
CI failure is not related.
(https://github.com/bitcoin/bitcoin/pull/27607#issuecomment-1553369947)
CI failure is not related.
💬 0xB10C commented on pull request "Improve performance of p2p inv to send queues":
(https://github.com/bitcoin/bitcoin/pull/27610#issuecomment-1553373102)
> I've been running this branch for 7 days on a VPS and noticed this morning the CPU on b-msghand is back up to 100%. It was about that high running v24 release but dropped to 30% or so when I first switched to this branch and restarted.
@pinheadmz I've been running the patch on multiple nodes for a week now and haven't seen 100% CPU usage in the b-msghand thread again. If you haven't restarted or if it happens again, it would be helpful to see which functions are slow. `perf top -p $(pidof b
...
(https://github.com/bitcoin/bitcoin/pull/27610#issuecomment-1553373102)
> I've been running this branch for 7 days on a VPS and noticed this morning the CPU on b-msghand is back up to 100%. It was about that high running v24 release but dropped to 30% or so when I first switched to this branch and restarted.
@pinheadmz I've been running the patch on multiple nodes for a week now and haven't seen 100% CPU usage in the b-msghand thread again. If you haven't restarted or if it happens again, it would be helpful to see which functions are slow. `perf top -p $(pidof b
...
💬 DanM3rcurius commented on issue "Can't compile v24.0.1":
(https://github.com/bitcoin/bitcoin/issues/27680#issuecomment-1553373128)
Thanks for taking my feedback into consideration.
Have a wonderful weekend ahead!
Sent from Proton Mail for iOS
On Thu, May 18, 2023 at 12:58, MacrabFalke ***@***.***> wrote:
>> Also I didn't quite understand the note and how to specify the absolute path ...
>
> Thanks, I agree that the section about absolute folders is confusing. So I removed it in [#27685](https://github.com/bitcoin/bitcoin/pull/27685)
>
> —
> Reply to this email directly, [view it on GitHub](https://github.com/bitcoin/bitc
...
(https://github.com/bitcoin/bitcoin/issues/27680#issuecomment-1553373128)
Thanks for taking my feedback into consideration.
Have a wonderful weekend ahead!
Sent from Proton Mail for iOS
On Thu, May 18, 2023 at 12:58, MacrabFalke ***@***.***> wrote:
>> Also I didn't quite understand the note and how to specify the absolute path ...
>
> Thanks, I agree that the section about absolute folders is confusing. So I removed it in [#27685](https://github.com/bitcoin/bitcoin/pull/27685)
>
> —
> Reply to this email directly, [view it on GitHub](https://github.com/bitcoin/bitc
...
💬 MarcoFalke commented on pull request "index: improve initialization and pruning violation check":
(https://github.com/bitcoin/bitcoin/pull/27607#issuecomment-1553376251)
See https://github.com/bitcoin/bitcoin/issues/27492#issuecomment-1527772394
(https://github.com/bitcoin/bitcoin/pull/27607#issuecomment-1553376251)
See https://github.com/bitcoin/bitcoin/issues/27492#issuecomment-1527772394
💬 MarcoFalke commented on pull request "Add wallet method to detect if a key is "active"":
(https://github.com/bitcoin/bitcoin/pull/27216#issuecomment-1553377467)
Needs rebase
(https://github.com/bitcoin/bitcoin/pull/27216#issuecomment-1553377467)
Needs rebase
💬 ryanofsky commented on pull request "index: improve initialization and pruning violation check":
(https://github.com/bitcoin/bitcoin/pull/27607#issuecomment-1553379017)
re: https://github.com/bitcoin/bitcoin/pull/27607#issue-1702212340
> * Fixed a small race, where we set the index `m_synced` flag (which enables `BlockConnected` events) before calling to the child class init function. So, for example, the block filter index could theoretically process a block before initializing the next filter position field and end up overwriting the first stored filter.
Can you clarify this? I don't see m_synced getting set to true before the index is synced, and d
...
(https://github.com/bitcoin/bitcoin/pull/27607#issuecomment-1553379017)
re: https://github.com/bitcoin/bitcoin/pull/27607#issue-1702212340
> * Fixed a small race, where we set the index `m_synced` flag (which enables `BlockConnected` events) before calling to the child class init function. So, for example, the block filter index could theoretically process a block before initializing the next filter position field and end up overwriting the first stored filter.
Can you clarify this? I don't see m_synced getting set to true before the index is synced, and d
...
💬 MarcoFalke commented on pull request "Improve performance of p2p inv to send queues":
(https://github.com/bitcoin/bitcoin/pull/27610#issuecomment-1553383935)
Could also make sense to double check the debug.log to ensure you restarted `bitcoind` after compiling? :sweat_smile:
(https://github.com/bitcoin/bitcoin/pull/27610#issuecomment-1553383935)
Could also make sense to double check the debug.log to ensure you restarted `bitcoind` after compiling? :sweat_smile:
💬 TheCharlatan commented on pull request "kernel: Remove interface_ui, util/system from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27636#issuecomment-1553391517)
Updated 861582b4087f1602e7115ee4a70ef7a7263eb36b -> 5a01a63ac8267debf152c8757e24f300e18ae379 ([chainstateRmKernelUiInterface_8](https://github.com/TheCharlatan/bitcoin/commits/chainstateRmKernelUiInterface_8) -> [chainstateRmKernelUiInterface_9](https://github.com/TheCharlatan/bitcoin/commits/chainstateRmKernelUiInterface_9), [compare](https://github.com/TheCharlatan/bitcoin/compare/chainstateRmKernelUiInterface_8..chainstateRmKernelUiInterface_9)).
* Addressed @ryanofsky's [comment](https://gi
...
(https://github.com/bitcoin/bitcoin/pull/27636#issuecomment-1553391517)
Updated 861582b4087f1602e7115ee4a70ef7a7263eb36b -> 5a01a63ac8267debf152c8757e24f300e18ae379 ([chainstateRmKernelUiInterface_8](https://github.com/TheCharlatan/bitcoin/commits/chainstateRmKernelUiInterface_8) -> [chainstateRmKernelUiInterface_9](https://github.com/TheCharlatan/bitcoin/commits/chainstateRmKernelUiInterface_9), [compare](https://github.com/TheCharlatan/bitcoin/compare/chainstateRmKernelUiInterface_8..chainstateRmKernelUiInterface_9)).
* Addressed @ryanofsky's [comment](https://gi
...
💬 pinheadmz commented on pull request "Improve performance of p2p inv to send queues":
(https://github.com/bitcoin/bitcoin/pull/27610#issuecomment-1553393935)
> Could also make sense to double check the debug.log to ensure you restarted `bitcoind` after compiling? 😅
Phew! That would've been embarassing
```
2023-05-10T15:39:40Z Bitcoin Core version v25.99.0-5b3406094f26 (debug build)
```
> `perf top -p $(pidof bitcoind)` should do the trick.
Not familiar with this tool but it looks cool! This is at the top. Every other line in the output is < 0.10%
```
99.56% bitcoind [.] boost::multi_index::detail::safe_iterator_
...
(https://github.com/bitcoin/bitcoin/pull/27610#issuecomment-1553393935)
> Could also make sense to double check the debug.log to ensure you restarted `bitcoind` after compiling? 😅
Phew! That would've been embarassing
```
2023-05-10T15:39:40Z Bitcoin Core version v25.99.0-5b3406094f26 (debug build)
```
> `perf top -p $(pidof bitcoind)` should do the trick.
Not familiar with this tool but it looks cool! This is at the top. Every other line in the output is < 0.10%
```
99.56% bitcoind [.] boost::multi_index::detail::safe_iterator_
...
💬 0xB10C commented on pull request "build: Detect USDT the same way how it is used in the code":
(https://github.com/bitcoin/bitcoin/pull/27458#issuecomment-1553394031)
ACK b53cab0083d99e9610d74517d1d41fc615953770
Tracepoints for FreeBSD can happen at a later point, if needed.
(https://github.com/bitcoin/bitcoin/pull/27458#issuecomment-1553394031)
ACK b53cab0083d99e9610d74517d1d41fc615953770
Tracepoints for FreeBSD can happen at a later point, if needed.
⚠️ dooglus opened an issue: "v25.0rc2 unusably slow at times"
(https://github.com/bitcoin/bitcoin/issues/27700)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
I'm testing v25.0rc2. Occasionally it becomes unresponsive. Simple requests like 'getblockcount' time out.
I tried running in gdb to see where the code was getting hung up. It was loading the mempool.dat file, spending lots of time in `CTxMemPool::addUnchecked()`, running this line:
```vTxHashes.emplace_back(tx.GetWitnessHash(), newit);```
and also in `CTxMemPool::TrimToSize()`.
...
(https://github.com/bitcoin/bitcoin/issues/27700)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
I'm testing v25.0rc2. Occasionally it becomes unresponsive. Simple requests like 'getblockcount' time out.
I tried running in gdb to see where the code was getting hung up. It was loading the mempool.dat file, spending lots of time in `CTxMemPool::addUnchecked()`, running this line:
```vTxHashes.emplace_back(tx.GetWitnessHash(), newit);```
and also in `CTxMemPool::TrimToSize()`.
...
💬 furszy commented on pull request "index: improve initialization and pruning violation check":
(https://github.com/bitcoin/bitcoin/pull/27607#issuecomment-1553400917)
> re: [#27607 (comment)](https://github.com/bitcoin/bitcoin/pull/27607#issue-1702212340)
>
> > * Fixed a small race, where we set the index `m_synced` flag (which enables `BlockConnected` events) before calling to the child class init function. So, for example, the block filter index could theoretically process a block before initializing the next filter position field and end up overwriting the first stored filter.
>
> Can you clarify this? I don't see m_synced getting set to true befor
...
(https://github.com/bitcoin/bitcoin/pull/27607#issuecomment-1553400917)
> re: [#27607 (comment)](https://github.com/bitcoin/bitcoin/pull/27607#issue-1702212340)
>
> > * Fixed a small race, where we set the index `m_synced` flag (which enables `BlockConnected` events) before calling to the child class init function. So, for example, the block filter index could theoretically process a block before initializing the next filter position field and end up overwriting the first stored filter.
>
> Can you clarify this? I don't see m_synced getting set to true befor
...
💬 MarcoFalke commented on issue "v25.0rc2 unusably slow at times":
(https://github.com/bitcoin/bitcoin/issues/27700#issuecomment-1553401313)
See https://github.com/bitcoin/bitcoin/issues/27586
(https://github.com/bitcoin/bitcoin/issues/27700#issuecomment-1553401313)
See https://github.com/bitcoin/bitcoin/issues/27586
💬 MarcoFalke commented on issue "v25.0rc2 unusably slow at times":
(https://github.com/bitcoin/bitcoin/issues/27700#issuecomment-1553401790)
(I presume you run with `--enable-debug`, in which case you can work around by disabling that)
(https://github.com/bitcoin/bitcoin/issues/27700#issuecomment-1553401790)
(I presume you run with `--enable-debug`, in which case you can work around by disabling that)
🤔 stickies-v reviewed a pull request: "httpserver, rest: improving URI validation"
(https://github.com/bitcoin/bitcoin/pull/27253#pullrequestreview-1430879440)
> I would have liked to remove the `replySent` argument, but that's secondary to this PR which fixes a serious bug.
Me too, especially since this is now no longer a bugfix PR. I think the workarounds that need to introduced in this PR make things unnecessarily messy, and updating the fuzzer as e.g. per your suggestion [here](https://github.com/bitcoin/bitcoin/pull/27253#issuecomment-1490015098) look like a reasonable approach for this PR to me.
(https://github.com/bitcoin/bitcoin/pull/27253#pullrequestreview-1430879440)
> I would have liked to remove the `replySent` argument, but that's secondary to this PR which fixes a serious bug.
Me too, especially since this is now no longer a bugfix PR. I think the workarounds that need to introduced in this PR make things unnecessarily messy, and updating the fuzzer as e.g. per your suggestion [here](https://github.com/bitcoin/bitcoin/pull/27253#issuecomment-1490015098) look like a reasonable approach for this PR to me.
💬 stickies-v commented on pull request "httpserver, rest: improving URI validation":
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1197670181)
The two `req->GetQueryParameter` calls in `rest_mempool` need to be updated as well.
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1197670181)
The two `req->GetQueryParameter` calls in `rest_mempool` need to be updated as well.
💬 stickies-v commented on pull request "httpserver, rest: improving URI validation":
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1196635667)
Since docs guarantee that GetURI() returns non-nullptr, would add an Assert here
```suggestion
return Assert(m_uri);
```
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1196635667)
Since docs guarantee that GetURI() returns non-nullptr, would add an Assert here
```suggestion
return Assert(m_uri);
```
💬 stickies-v commented on pull request "httpserver, rest: improving URI validation":
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1197694855)
What's the rationale for this change? No callsite seems to be preferring a C-style string, and returning a `std::string` would also make the non-nullptr promise explicit.
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1197694855)
What's the rationale for this change? No callsite seems to be preferring a C-style string, and returning a `std::string` would also make the non-nullptr promise explicit.
💬 stickies-v commented on pull request "httpserver, rest: improving URI validation":
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1197667936)
nit: clearer language
```suggestion
/** Get requested URI. Will never return nullptr.
```
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1197667936)
nit: clearer language
```suggestion
/** Get requested URI. Will never return nullptr.
```