💬 l0rinc commented on pull request "doc: bump the template macOS version since 14 is now the minimum supported version":
(https://github.com/bitcoin/bitcoin/pull/33573#issuecomment-3391920236)
ACK f6567527d8dafa70335de782511791165bb89ccb
The Ubuntu version is a bit far into the future, but I don't mind
(https://github.com/bitcoin/bitcoin/pull/33573#issuecomment-3391920236)
ACK f6567527d8dafa70335de782511791165bb89ccb
The Ubuntu version is a bit far into the future, but I don't mind
💬 maflcko commented on pull request "refactor: Construct g_verify_flag_names on first use":
(https://github.com/bitcoin/bitcoin/pull/33600#issuecomment-3391921802)
<details><summary>for reference, the msan false positive CI logs</summary>
master:
```
...
+ /ci_container_base/ci/scratch/build-aarch64-unknown-linux-gnu/test/fuzz/test_runner.py -j16 -l DEBUG /ci_container_base/ci/scratch/qa-assets/fuzz_corpora/ --empty_min_time=60
==11639==WARNING: MemorySanitizer: use-of-uninitialized-value
#0 0xaaaad25c2408 in SetArgs(int, char**) /ci_container_base/ci/scratch/build-aarch64-unknown-linux-gnu/src/test/fuzz/util/./test/fuzz/fuzz.cpp:52:5
#1
...
(https://github.com/bitcoin/bitcoin/pull/33600#issuecomment-3391921802)
<details><summary>for reference, the msan false positive CI logs</summary>
master:
```
...
+ /ci_container_base/ci/scratch/build-aarch64-unknown-linux-gnu/test/fuzz/test_runner.py -j16 -l DEBUG /ci_container_base/ci/scratch/qa-assets/fuzz_corpora/ --empty_min_time=60
==11639==WARNING: MemorySanitizer: use-of-uninitialized-value
#0 0xaaaad25c2408 in SetArgs(int, char**) /ci_container_base/ci/scratch/build-aarch64-unknown-linux-gnu/src/test/fuzz/util/./test/fuzz/fuzz.cpp:52:5
#1
...
💬 glozow commented on pull request "net_processing: rename RelayTransaction to better describe what it does":
(https://github.com/bitcoin/bitcoin/pull/33565#discussion_r2421785265)
If `Queue` is confusing, then I can also suggest MarkTxForRelay or UpdateTxBroadcastSet
I recognize that we can all read verbs differently. I'm just pointing out that "schedule" in this codebase almost always means putting a task on the `scheduler` background thread with a specific time.
(https://github.com/bitcoin/bitcoin/pull/33565#discussion_r2421785265)
If `Queue` is confusing, then I can also suggest MarkTxForRelay or UpdateTxBroadcastSet
I recognize that we can all read verbs differently. I'm just pointing out that "schedule" in this codebase almost always means putting a task on the `scheduler` background thread with a specific time.
🤔 janb84 reviewed a pull request: "doc: bump the template macOS version since 14 is now the minimum supported version"
(https://github.com/bitcoin/bitcoin/pull/33573#pullrequestreview-3325542629)
ACK f6567527d8dafa70335de782511791165bb89ccb
With just recent references there is no discussion anymore if it is supported or not. Just examples of what someone can fill in.
(https://github.com/bitcoin/bitcoin/pull/33573#pullrequestreview-3325542629)
ACK f6567527d8dafa70335de782511791165bb89ccb
With just recent references there is no discussion anymore if it is supported or not. Just examples of what someone can fill in.
💬 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.