💬 andrewtoth commented on pull request "net_processing: rename RelayTransaction to better describe what it does":
(https://github.com/bitcoin/bitcoin/pull/33565#discussion_r2421877692)
How about `RelayTransactionAfterDelay`?
(https://github.com/bitcoin/bitcoin/pull/33565#discussion_r2421877692)
How about `RelayTransactionAfterDelay`?
📝 RandyMcMillan opened a pull request: "rpcnestedtests.cpp:remove qtest-obsolete members"
(https://github.com/bitcoin-core/gui/pull/900)
(https://github.com/bitcoin-core/gui/pull/900)
💬 RandyMcMillan commented on pull request "Modernize custom filtering":
(https://github.com/bitcoin-core/gui/pull/899#issuecomment-3392054463)
@hebasto - while testing this PR - I noticed this can be updated.
https://github.com/bitcoin-core/gui/pull/900
(https://github.com/bitcoin-core/gui/pull/899#issuecomment-3392054463)
@hebasto - while testing this PR - I noticed this can be updated.
https://github.com/bitcoin-core/gui/pull/900
💬 RandyMcMillan commented on pull request "rpcnestedtests.cpp:remove qtest-obsolete members":
(https://github.com/bitcoin-core/gui/pull/900#issuecomment-3392082488)
I will remove 4f29ddf3c5cbd373ebbfe05e5cd571cc9b383b0c if requested.
(https://github.com/bitcoin-core/gui/pull/900#issuecomment-3392082488)
I will remove 4f29ddf3c5cbd373ebbfe05e5cd571cc9b383b0c if requested.
💬 1440000bytes commented on issue "No way to clear command history in RPC console or reset the console without restarting the node":
(https://github.com/bitcoin-core/gui/issues/897#issuecomment-3392088866)
> The output gets cleared and the input remains regardless if you close wallet, RPC console. It only resets by restarting the node
This is a bug and core does not clear the history for `migratewallet` RPC from the console.
```C++
// don't add private key handling cmd's to the history
const QStringList historyFilter = QStringList()
<< "signmessagewithprivkey"
<< "signrawtransactionwithkey"
<< "walletpassphrase"
<< "walletpassphrasechange"
<< "encryptwallet";
}
```
https://
...
(https://github.com/bitcoin-core/gui/issues/897#issuecomment-3392088866)
> The output gets cleared and the input remains regardless if you close wallet, RPC console. It only resets by restarting the node
This is a bug and core does not clear the history for `migratewallet` RPC from the console.
```C++
// don't add private key handling cmd's to the history
const QStringList historyFilter = QStringList()
<< "signmessagewithprivkey"
<< "signrawtransactionwithkey"
<< "walletpassphrase"
<< "walletpassphrasechange"
<< "encryptwallet";
}
```
https://
...
🚀 fanquake merged a pull request: "doc: bump the template macOS version"
(https://github.com/bitcoin/bitcoin/pull/33573)
(https://github.com/bitcoin/bitcoin/pull/33573)
💬 glozow commented on issue "test: intermittent issue in p2p_1p1c_network.py":
(https://github.com/bitcoin/bitcoin/issues/33318#issuecomment-3392126924)
> The test does the disconnect earlier but from socket closing to clearing of peer state, this race can occur.
If it's a race, then this should go away if we make node3 the one that pre-receives orphans?
```diff
diff --git a/test/functional/p2p_1p1c_network.py b/test/functional/p2p_1p1c_network.py
index e4d3b738c19..5567b4e7002 100755
--- a/test/functional/p2p_1p1c_network.py
+++ b/test/functional/p2p_1p1c_network.py
@@ -109,14 +109,14 @@ class PackageRelayTest(BitcoinTestFramework):
...
(https://github.com/bitcoin/bitcoin/issues/33318#issuecomment-3392126924)
> The test does the disconnect earlier but from socket closing to clearing of peer state, this race can occur.
If it's a race, then this should go away if we make node3 the one that pre-receives orphans?
```diff
diff --git a/test/functional/p2p_1p1c_network.py b/test/functional/p2p_1p1c_network.py
index e4d3b738c19..5567b4e7002 100755
--- a/test/functional/p2p_1p1c_network.py
+++ b/test/functional/p2p_1p1c_network.py
@@ -109,14 +109,14 @@ class PackageRelayTest(BitcoinTestFramework):
...
💬 hebasto commented on pull request "rpcnestedtests.cpp:remove qtest-obsolete members":
(https://github.com/bitcoin-core/gui/pull/900#discussion_r2421952199)
The minimum supported Qt version [remains 6.2 for now](https://github.com/bitcoin/bitcoin/blob/master/doc/dependencies.md#build-1).
(https://github.com/bitcoin-core/gui/pull/900#discussion_r2421952199)
The minimum supported Qt version [remains 6.2 for now](https://github.com/bitcoin/bitcoin/blob/master/doc/dependencies.md#build-1).
👍 ryanofsky approved a pull request: "coins: fix `cachedCoinsUsage` accounting in `CCoinsViewCache`"
(https://github.com/bitcoin/bitcoin/pull/32313#pullrequestreview-3325631861)
Code review ACK c4293c7b21971e7ddd813ad3fbe574eb5605d60e. The changes are nice simplifications addition to fixing overflows, and they are all nicely explained. Few suggestions you could consider (but ignore them if you prefer current implementation):
- I feel like the new asserts added in b905a592d429b4b4edd48c8b0fa29ea4fdf8d251 and removed later don't really add much value or explain much since they can basically only trigger when there are one or two coins in the cache. I'd consider dropping
...
(https://github.com/bitcoin/bitcoin/pull/32313#pullrequestreview-3325631861)
Code review ACK c4293c7b21971e7ddd813ad3fbe574eb5605d60e. The changes are nice simplifications addition to fixing overflows, and they are all nicely explained. Few suggestions you could consider (but ignore them if you prefer current implementation):
- I feel like the new asserts added in b905a592d429b4b4edd48c8b0fa29ea4fdf8d251 and removed later don't really add much value or explain much since they can basically only trigger when there are one or two coins in the cache. I'd consider dropping
...
💬 ryanofsky commented on pull request "coins: fix `cachedCoinsUsage` accounting in `CCoinsViewCache`":
(https://github.com/bitcoin/bitcoin/pull/32313#discussion_r2421864146)
In commit "refactor: assert newly-created parent cache entry has zero memory usage" (8d5e323b5f475fe9a2bed1402a20e48a40a45137)
Note: this assert will be true because `itUs == cacheCoins.end()` condition above means try_emplace will always insert and `itUs->second` will be a new empty coin
(https://github.com/bitcoin/bitcoin/pull/32313#discussion_r2421864146)
In commit "refactor: assert newly-created parent cache entry has zero memory usage" (8d5e323b5f475fe9a2bed1402a20e48a40a45137)
Note: this assert will be true because `itUs == cacheCoins.end()` condition above means try_emplace will always insert and `itUs->second` will be a new empty coin
💬 RandyMcMillan commented on pull request "rpcnestedtests.cpp:remove qtest-obsolete members":
(https://github.com/bitcoin-core/gui/pull/900#discussion_r2422008368)
I couldn't find where 6.2 was actually being tested against in the CI? Maybe it is a good time to bump the minimum supported version?
(https://github.com/bitcoin-core/gui/pull/900#discussion_r2422008368)
I couldn't find where 6.2 was actually being tested against in the CI? Maybe it is a good time to bump the minimum supported version?
💬 furszy commented on pull request "p2p: Advance pindexLastCommonBlock early after connecting blocks":
(https://github.com/bitcoin/bitcoin/pull/32180#discussion_r2422008781)
> how about a separate PR and changing it in various places of the codebase where the GetAncestor / height check is used?
That would sounds good to me.
(https://github.com/bitcoin/bitcoin/pull/32180#discussion_r2422008781)
> how about a separate PR and changing it in various places of the codebase where the GetAncestor / height check is used?
That would sounds good to me.
💬 RandyMcMillan commented on pull request "rpcnestedtests.cpp:remove qtest-obsolete members":
(https://github.com/bitcoin-core/gui/pull/900#discussion_r2422009803)
6.7 was the earliest I could find?
(https://github.com/bitcoin-core/gui/pull/900#discussion_r2422009803)
6.7 was the earliest I could find?
💬 hebasto commented on pull request "rpcnestedtests.cpp:remove qtest-obsolete members":
(https://github.com/bitcoin-core/gui/pull/900#discussion_r2422058350)
> Maybe it is a good time to bump the minimum supported version?
We still support Ubuntu 22.04: https://packages.ubuntu.com/jammy/qt6-base-dev.
(https://github.com/bitcoin-core/gui/pull/900#discussion_r2422058350)
> Maybe it is a good time to bump the minimum supported version?
We still support Ubuntu 22.04: https://packages.ubuntu.com/jammy/qt6-base-dev.
📝 RandyMcMillan converted_to_draft a pull request: "rpcnestedtests.cpp:remove qtest-obsolete members"
(https://github.com/bitcoin-core/gui/pull/900)
(https://github.com/bitcoin-core/gui/pull/900)
📝 waketraindev opened a pull request: "qt: add createwallet, createwalletdescriptor, and migratewallet to history filter"
(https://github.com/bitcoin-core/gui/pull/901)
Added `createwallet`, `createwalletdescriptor` and `migratewallet` RPC commands to the Qt console history filter since they may include passphrases or other sensitive data that should not be stored in command history.
(https://github.com/bitcoin-core/gui/pull/901)
Added `createwallet`, `createwalletdescriptor` and `migratewallet` RPC commands to the Qt console history filter since they may include passphrases or other sensitive data that should not be stored in command history.
💬 ryanofsky commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2422090129)
In commit "util: introduce general purpose thread pool" (3943630a4cea56b040377b33476803745055510c)
Would be nice to avoid shared_ptr here. Apparently it is needed because std::function objects are required to be copyable and packaged tasks aren't copyable. Could fix this by avoiding std::function which would also simplify the Submit() function:
<details><summary>diff</summary>
<p>
```diff
--- a/src/util/threadpool.h
+++ b/src/util/threadpool.h
@@ -47,7 +47,7 @@ class ThreadPool {
...
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2422090129)
In commit "util: introduce general purpose thread pool" (3943630a4cea56b040377b33476803745055510c)
Would be nice to avoid shared_ptr here. Apparently it is needed because std::function objects are required to be copyable and packaged tasks aren't copyable. Could fix this by avoiding std::function which would also simplify the Submit() function:
<details><summary>diff</summary>
<p>
```diff
--- a/src/util/threadpool.h
+++ b/src/util/threadpool.h
@@ -47,7 +47,7 @@ class ThreadPool {
...
👋 theuni's pull request is ready for review: "multiprocess: Fix high overhead from message logging"
(https://github.com/bitcoin/bitcoin/pull/33517)
(https://github.com/bitcoin/bitcoin/pull/33517)
💬 1440000bytes commented on issue "No way to clear command history in RPC console or reset the console without restarting the node":
(https://github.com/bitcoin-core/gui/issues/897#issuecomment-3392342670)
Related pull request: https://github.com/bitcoin-core/gui/pull/901
(https://github.com/bitcoin-core/gui/issues/897#issuecomment-3392342670)
Related pull request: https://github.com/bitcoin-core/gui/pull/901
💬 ryanofsky commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2422124854)
In commit "util: introduce general purpose thread pool" (3943630a4cea56b040377b33476803745055510c)
I'm a little unclear what this swap is trying to do. I would expect the point of swapping into a temporary vector would be to destroy the callables without holding m_mutex (since they could own resources and take time to destroy). But it looks like the `empty` vector is destroyed while `m_mutex` is still locked, so that isn't happening. Also I don't understand why `m_mutex` is locked twice in th
...
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2422124854)
In commit "util: introduce general purpose thread pool" (3943630a4cea56b040377b33476803745055510c)
I'm a little unclear what this swap is trying to do. I would expect the point of swapping into a temporary vector would be to destroy the callables without holding m_mutex (since they could own resources and take time to destroy). But it looks like the `empty` vector is destroyed while `m_mutex` is still locked, so that isn't happening. Also I don't understand why `m_mutex` is locked twice in th
...