🤔 BrandonOdiwuor reviewed a pull request: "test: Assumeutxo with more than just coinbase transactions"
(https://github.com/bitcoin/bitcoin/pull/29354#pullrequestreview-1859156221)
Concept ACK

💬 theStack commented on pull request "test: use v2 everywhere for P2PConnection if --v2transport is enabled":
(https://github.com/bitcoin/bitcoin/pull/29358#issuecomment-1923734576)
Concept ACK

💬 brunoerg commented on pull request "Fuzz: extend CConnman tests":
(https://github.com/bitcoin/bitcoin/pull/28584#discussion_r1476030643)
In b1b74a13adb8e943d90fbe56c4a706b0ae91335a: nit: I don't think to set network active here is a must, we could fuzz it with both network active and inactive.

💬 maflcko commented on pull request "refactor: Allow CScript construction from any std::input_iterator":
(https://github.com/bitcoin/bitcoin/pull/29369#issuecomment-1923782707)
@delta1 This is an issue in the C++ standard library. Basically, the standard assumes that `element_type` and `value_type` types are never provided by an iterator type at the same time. This was fixed in https://cplusplus.github.io/LWG/issue3446 and https://github.com/gcc-mirror/gcc/commit/186aa6304570e15065f31482e9c27326a3a6679f

💬 BrandonOdiwuor commented on issue "test: RPC coverage check doesn't work?":
(https://github.com/bitcoin/bitcoin/issues/27593#issuecomment-1923784437)
I have reviewed the issue and it's evident that the wallet RPCs are indeed absent in the generated `rpc_interface.txt` file. This observation aligns with the analysis provided by @kouloumos.

The root cause lies in the invocation of `start_node(...)` within `create_cache.py`, where the `write_all_rpc_commands(...)` function is called. This occurs during the creation of the cache on a node that lacks wallet functionality. Consequently, the wallet RPCs are not included in the generated `rpc_int
...

💬 brunoerg commented on pull request "Fuzz: extend CConnman tests":
(https://github.com/bitcoin/bitcoin/pull/28584#discussion_r1476052411)
In d923b86264df93ad86c7bd557b9970e156585dc7: You could use `ConsumeService` into `ConsumeServiceVector`.

```diff
diff --git a/src/test/fuzz/util/net.h b/src/test/fuzz/util/net.h
index a97017555d..78e61b51d9 100644
--- a/src/test/fuzz/util/net.h
+++ b/src/test/fuzz/util/net.h
@@ -102,8 +102,7 @@ inline std::vector<CService> ConsumeServiceVector(FuzzedDataProvider& fuzzed_dat
const size_t size = fuzzed_data_provider.ConsumeIntegralInRange<size_t>(0, max_vector_size);
ret.reserv
...

💬 BrandonOdiwuor commented on issue "test: RPC coverage check doesn't work?":
(https://github.com/bitcoin/bitcoin/issues/27593#issuecomment-1923793691)
@kouloumos what do you mean by?
> we can avoid this issue, but just a reordering of cache-creation<>coverage-initialization pushes the issue to the next coverage-initialization which in that case will be the first test of the test-runner. If the node in that first test doesn't have a wallet, the issue remains.

Doesn't the coverage-initialization happen inside cache-creation. Do you mean there should be a coverage-initialization (generation of the rpc_interface.txt) before `create_cache.py`
...

💬 delta1 commented on pull request "refactor: Allow CScript construction from any std::input_iterator":
(https://github.com/bitcoin/bitcoin/pull/29369#issuecomment-1923798641)
@maflcko interesting, thanks for the context. does that mean that the previous releases CI job needs to be changed to user a newer gcc?

💬 maflcko commented on issue "test: RPC coverage check doesn't work?":
(https://github.com/bitcoin/bitcoin/issues/27593#issuecomment-1923811199)
Why not simply enable the wallet in `create_cache`?

💬 josibake commented on pull request "wallet: cache IsMine scriptPubKeys to improve performance of descriptor wallets":
(https://github.com/bitcoin/bitcoin/pull/26008#issuecomment-1923826205)
> Dropped the signing changes as they were causing issues with being able to sign transactions that we have the keys for but not the descriptors. https://github.com/bitcoin/bitcoin/pull/23417 would allow us to resolve the performance issues for signing while retaining this behavior.

Is this worth revisiting since your re-write of the cache? The signing stuff could be a follow-up PR, but if it's simple enough it would be really nice to include it here. Otherwise, someone using migrate wallet c
...

💬 glozow commented on pull request "Weaken serfloat tests":
(https://github.com/bitcoin/bitcoin/pull/29192#issuecomment-1923857100)
ACK ea60c95be36a68ba98d1d23018587aa4d4d6bb1a, based on https://github.com/Homebrew/homebrew-core/pull/156745 and https://github.com/bitcoin/bitcoin/pull/29192#issuecomment-1881144857 it looks like this fixes the issue. I think this is the only thing we're waiting on for 26.1.

re https://github.com/bitcoin/bitcoin/pull/29192#issuecomment-1906925728, the values that were failing the tests are irrelevant in the context we're using this, and 21.0 has been eol for a while now - also see https://g
...

💬 epiccurious commented on pull request "test: use v2 everywhere for P2PConnection if --v2transport is enabled":
(https://github.com/bitcoin/bitcoin/pull/29358#issuecomment-1923932584)
Tested ACK.

>not actual tests that can be executed directly
Thanks for the clarification.

Certain tests take longer to run, to be expected with the added sub-tests.
```
TEST | STATUS | DURATION

feature_block.py | ✓ Passed | 197 s
feature_maxuploadtarget.py | ✓ Passed | 59 s
p2p_ibd_stalling.py | ✓ Passed | 59 s
p2p_ibd_stalling.py --v2transport | ✓ Passed | 58 s
p2p_invalid_messages.py | ✓ Passed
...

💬 stickies-v commented on pull request "[WIP] C++20 std::views::reverse":
(https://github.com/bitcoin/bitcoin/pull/28687#issuecomment-1923969918)
Force pushed to rebase, added a commit to bump compilers of failing CI tasks (see [OP](https://github.com/bitcoin/bitcoin/pull/28687#issue-1952153535)), not too sure what I'm doing here though.

🤔 murchandamus reviewed a pull request: "wallet: Set descriptors flag after migrating blank wallets"
(https://github.com/bitcoin/bitcoin/pull/29367#pullrequestreview-1859460255)
Concept ACK

💬 murchandamus commented on pull request "wallet: Set descriptors flag after migrating blank wallets":
(https://github.com/bitcoin/bitcoin/pull/29367#discussion_r1476168453)
Given that the PR describes the setting of the flag as a special case for blank wallets, I’d be more comfortable if the setting of the flag were conditional on a wallet being blank instead of success being true here.

```diff
- success = local_wallet->IsWalletFlagSet(WALLET_FLAG_BLANK_WALLET);
+ if(local_wallet->IsWalletFlagSet(WALLET_FLAG_BLANK_WALLET)) {
+ local_wallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS);
+ success = true;
+ }
+
i
...

⚠️ Thrillseekr opened an issue: "How to get started with Contribution"
(https://github.com/bitcoin-core/gui/issues/791)
Hello @gregwebs @ldenman @casey @eklitzke @rion,
I'm Darshan Jain, interested in contributing to your organization Bitcoin Core.

Could you please help me in guiding how and where to get started.

My interests and skills lies in domain
C++, Object Oriented Programming Techniques, Data Structures and Algorithms

💬 hebasto commented on issue "How to get started with Contribution":
(https://github.com/bitcoin-core/gui/issues/791#issuecomment-1924103289)
@Thrillseekr

You are much welcome!

Please consider https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#getting-started.

💬 darosior commented on pull request "descriptors: Be able to specify change and receiving in a single descriptor string":
(https://github.com/bitcoin/bitcoin/pull/22838#issuecomment-1924145169)
If i re-review this would anyone else review as well? ACKed it multiple times in the past but it didn't get merged because we'd need another person.

It's been a while, i need to review it from scratch so i'm trying to probe if that's worth spending the time.

💬 hebasto commented on pull request "test: Drop `x` modifier in `fsbridge::fopen` call for MinGW builds":
(https://github.com/bitcoin/bitcoin/pull/29357#discussion_r1476232308)
The following patch
```diff
--- a/contrib/guix/manifest.scm
+++ b/contrib/guix/manifest.scm
@@ -108,6 +108,17 @@ desirable for building Bitcoin Core release binaries."
base-libc
base-gcc))

+
+(define-public mingw-w64-x86_64-winpthreads-ucrt
+ (package
+ (inherit mingw-w64-x86_64-winpthreads)
+ (arguments
+ (substitute-keyword-arguments (package-arguments mingw-w64-x86_64-winpthreads)
+ ((#:configure-flags flags)
...

🤔 jonatack reviewed a pull request: "set `DEFAULT_PERMIT_BAREMULTISIG` to false"
(https://github.com/bitcoin/bitcoin/pull/28217#pullrequestreview-1859525850)
ACK 8672721deb06e66854a085c9994e99c99160b8a1

---

> ACK.

@theDavidCoen In your review https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1914214959, per https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#code-review (and to be counted by DrahtBot and the merge script), your ACK would need to be followed by the commit hash. See above here or https://github.com/bitcoin/bitcoin/pull/28217#pullrequestreview-1842509290 for examples.