Bitcoin Core Github
44 subscribers
120K links
Download Telegram
🤔 danielabrozzoni reviewed a pull request: "coins: warn on oversized `-dbcache`"
(https://github.com/bitcoin/bitcoin/pull/33333#pullrequestreview-3239885117)
reACK 168360f4ae47cbfdb30a2cc4704435bc67e12f16
zaidmstrr closed an issue: "Test interface_ipc.py fails with Duplicate ID error when libmultiprocess is installed system-wide"
(https://github.com/bitcoin/bitcoin/issues/33417)
🤔 ismaelsadeeq reviewed a pull request: "cluster mempool: control/optimize TxGraph memory usage"
(https://github.com/bitcoin/bitcoin/pull/33157#pullrequestreview-3239910323)
Code Review ACK 66401148ddcba04c27751e39e53fc41e6deb5d14

I shutdown a long-running node at height 915249.

```
.rw------- 128M ismaelsadeeq 18 Sep 14:46 mempool.dat
```

Restarted the node on master 1444ed855f4 with:

```bash
build/bin/bitcoind -connect=0 -debug
```

After loading the mempool, `getmempoolinfo` returned:

```json
{
"loaded": true,
"size": 104944,
"bytes": 43254588,
"usage": 260130384,
"total_fee": 0.10523099,
"maxmempool":
...
💬 vasild commented on pull request "test: don't throw from the destructor of DebugLogHelper":
(https://github.com/bitcoin/bitcoin/pull/33388#issuecomment-3307658710)
`7f87cddd36...ba6689aa9e`: drop some unrelated changes and pick some suggestions
💬 vasild commented on pull request "test: don't throw from the destructor of DebugLogHelper":
(https://github.com/bitcoin/bitcoin/pull/33388#discussion_r2359495317)
Done.
💬 vasild commented on pull request "test: don't throw from the destructor of DebugLogHelper":
(https://github.com/bitcoin/bitcoin/pull/33388#discussion_r2359496859)
Removed the parameter change altogether.
💬 vasild commented on pull request "test: don't throw from the destructor of DebugLogHelper":
(https://github.com/bitcoin/bitcoin/pull/33388#discussion_r2359498826)
Reverted to the original comment and its location.
💬 vasild commented on pull request "test: don't throw from the destructor of DebugLogHelper":
(https://github.com/bitcoin/bitcoin/pull/33388#discussion_r2359504773)
Could add `std::cerr.flush()`, but I think it is not necessary because [cerr is unbuffered](https://www.geeksforgeeks.org/cpp/difference-between-cerr-and-clog/).
💬 vasild commented on pull request "test: don't throw from the destructor of DebugLogHelper":
(https://github.com/bitcoin/bitcoin/pull/33388#discussion_r2359513993)
Hmm, I am not sure either. Maybe it should be done like you say. Unrelated to this PR.
💬 vasild commented on pull request "test: don't throw from the destructor of DebugLogHelper":
(https://github.com/bitcoin/bitcoin/pull/33388#discussion_r2359516805)
Removed any changes to those parameters.
💬 vasild commented on pull request "test: don't throw from the destructor of DebugLogHelper":
(https://github.com/bitcoin/bitcoin/pull/33388#discussion_r2359525565)
I dropped the changes to the parameters since they are not the aim of this PR and they caused disproportional amount of discussion.
💬 vasild commented on pull request "test: don't throw from the destructor of DebugLogHelper":
(https://github.com/bitcoin/bitcoin/pull/33388#discussion_r2359527018)
Dropped altogether.
💬 Eunovo commented on pull request "Silent Payments: Receiving":
(https://github.com/bitcoin/bitcoin/pull/32966#issuecomment-3307718331)
I changed `CKey::TweakAdd()` to `CKey::ComputeBIP352Key()`
💬 purpleKarrot commented on pull request "test: don't throw from the destructor of DebugLogHelper":
(https://github.com/bitcoin/bitcoin/pull/33388#issuecomment-3307759393)
Unrelated to the PR, but it is error prone that `DebugLogHelper` is copyable: https://godbolt.org/z/YdT5EWzMM

I suggest to augment the PR with a commit that contains:
```cpp
DebugLogHelper(const DebugLogHelper&) = delete;
DebugLogHelper& operator=(const DebugLogHelper&) = delete;
```
💬 sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2359615241)
Happy to clear this comment up to get rid of double-negatives, but just to make sure I've got this right:
* AcceptSubPackage() will, in the case of single transaction acceptance, enable sibling eviction, right?
* And in package validation, sibling eviction is disabled?
💬 vasild commented on pull request "test: don't throw from the destructor of DebugLogHelper":
(https://github.com/bitcoin/bitcoin/pull/33388#issuecomment-3307790546)
@purpleKarrot,
> > Throwing an exception from the destructor of a class is a bad practice because the destructor will be called when an object of that type is alive on the stack and another exception is thrown, which will result in "exception during the exception". This would terminate the program without any messages.

> This is not precisely what happens. If an exception is emitted from a function marked `noexcept(false)`, the current terminate handler will be invoked.

Hmm, not what I ob
...
💬 vasild commented on pull request "test: don't throw from the destructor of DebugLogHelper":
(https://github.com/bitcoin/bitcoin/pull/33388#issuecomment-3307822493)
@purpleKarrot
> I suggest to augment the PR with a commit that contains...

Done
💬 instagibbs commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2359689084)
correct
💬 purpleKarrot commented on pull request "test: don't throw from the destructor of DebugLogHelper":
(https://github.com/bitcoin/bitcoin/pull/33388#issuecomment-3307930444)
> If an exception is emitted from a function marked `noexcept(false)`, the current terminate handler will be invoked.

My bad. That is the behavior for `noexcept(true)`, of course. I agree that marking a destructor with `noexcept(false)` is bad.
💬 sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2359723768)
Yeah the limits that were previously enforced by earlier versions of this function would be calculated including the tx itself, but not the ancestor set. Thanks for catching.