⚠️ ajtowns opened an issue: "Test case for spending bare multisig?"
(https://github.com/bitcoin/bitcoin/issues/29113)
### Please describe the feature you'd like to see added.

I think the only place we currently test that spending a bare multisig utxo is okay is in `AreInputsStandard` in `script_p2sh_tests.cpp`, but this only checks that `AreInputsStandard()` passes, it doesn't check that there isn't some other rule preventing the spend from entering the mempool and eventually being mined.

Adding

```python
self.log.info('Spending a confirmed bare multisig is okay')
node = self.nodes[0]

...

💬 pablomartin4btc commented on pull request "Fix: Ensure 'Transaction View' remains disabled if no wallet is selected":
(https://github.com/bitcoin-core/gui/pull/780#issuecomment-1861954800)
> Being only able to change it requires a wallet to be open, to me would feel like a bug, no? that's more my "concern"

Fair enough, thanks for sharing your insights.

💬 Fiach-Dubh commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1861960853)
ACK

There is evidence that a majority of people do not like or want this behavior. Having it on by default for all node runners, seems contrary to what most node runners would choose as policy if they were consciously given a choice.


🤔 amitiuttarwar reviewed a pull request: "test: create deterministic addrman in the functional tests"
(https://github.com/bitcoin/bitcoin/pull/29007#pullrequestreview-1787928129)
approach ACK. yay for finally having addrman determinism in the functional tests 🙌🏽
couple thoughts to consider-

#### 1. clearer subtest addrman setup in `rpc_net.py`

@0xB10C

> I think it would be ideal if we could avoid the addpeeraddress test leaking into getaddrmaninfo and getrawaddrman tests.
>

agreed. right now, the 3 subtests (`test_addpeeraddress`, `test_getaddrmaninfo` & `test_getrawaddrman`) have a lot of implicit dependencies, making the overall test harder to understa
...

💬 amitiuttarwar commented on pull request "test: create deterministic addrman in the functional tests":
(https://github.com/bitcoin/bitcoin/pull/29007#discussion_r1430791494)
these init params are added here, but are only relevant until a couple subtests later in `test_getaddrmaninfo`. having unrelated init params can lead to unexpected behaviors, like in this recent [example](https://github.com/bitcoin/bitcoin/pull/28538#discussion_r1423241016). implicit dependencies between subtests can also make future development confusing, such as updating setup in an earlier test unexpectedly causing failures in seemingly unrelated tests.

leaving suggestions for improving t
...

💬 amitiuttarwar commented on pull request "test: create deterministic addrman in the functional tests":
(https://github.com/bitcoin/bitcoin/pull/29007#discussion_r1430801807)
I think it makes sense to update the comment further. here's why-

on current master, this test sets up addrman by putting 1 addr into tried, followed by 1 addr into new. the comment explains to future devs why this setup must be maintained.

this patch doesn't update the setup in the `addpeeraddress` test. the comment on master is outdated because we no longer need to preserve the careful ordering, but I find the currently proposed comment misleading since the deterministic addrman isn't y
...

💬 amitiuttarwar commented on pull request "test: create deterministic addrman in the functional tests":
(https://github.com/bitcoin/bitcoin/pull/29007#discussion_r1430822958)
I like the approach of using `-test` to group test-only options

two suggestions for improvement:
* include the options in the help text (eg. how `-onlynet` or `-debug` handles)
* throw some sort of error (or at least warning?) if an invalid option is passed through

💬 kevkevinpal commented on pull request "mempool / rpc: followup to getprioritisedtransactions and delete a mapDeltas entry when delta==0":
(https://github.com/bitcoin/bitcoin/pull/28885#discussion_r1430841261)
@jonatack said that I should run the script in this [comment](https://github.com/bitcoin/bitcoin/pull/28885#discussion_r1406322235) and it lead to there being a indentation, I can remove if needed

🤔 stratospher reviewed a pull request: "net: Make AddrFetch connections to fixed seeds"
(https://github.com/bitcoin/bitcoin/pull/26114#pullrequestreview-1788195228)
ACK dfd635b.

i think this is a good improvement to have! - better peer diversity compared to what's happening on master where we directly open connections to hardcoded seeds.

tested it using `getaddrmaninfo` RPC
- before fixed seeds were added, new table already had around 1679 addresses from the addrfetch connections
- after fixed seeds were added, new table had 2399 addresses

observed 2 addrfetch connections being successfully opened during the 2 minute delay though.

💬 stratospher commented on pull request "net: Make AddrFetch connections to fixed seeds":
(https://github.com/bitcoin/bitcoin/pull/26114#discussion_r1430982146)
7cc4633: could also update the doc comments to `Load addresses of peers from the DNS and from fixed seeds.`

💬 stratospher commented on pull request "net: Make AddrFetch connections to fixed seeds":
(https://github.com/bitcoin/bitcoin/pull/26114#discussion_r1430984088)
611923e: we could remove this fixed seed logic handled in `ThreadOpenConnections` comment.

💬 maflcko commented on issue "Test case for spending bare multisig?":
(https://github.com/bitcoin/bitcoin/issues/29113#issuecomment-1862310039)
pull request welcome :)

💬 maflcko commented on pull request "sqlite: Disallow writing from multiple `SQLiteBatch`s":
(https://github.com/bitcoin/bitcoin/pull/29112#issuecomment-1862494786)
```
File "/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/wallet_descriptor.py", line 68, in test_concurrent_writes
assert_equal(cache_records, 10000)
File "/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/util.py", line 57, in assert_equal
raise AssertionError("not
...

💬 maflcko commented on issue "Discussion: Upgrading to C++20":
(https://github.com/bitcoin/bitcoin/issues/23363#issuecomment-1862522407)
> `ArrayLike` sounds like `ranges::contiguous_range` ? https://en.cppreference.com/w/cpp/ranges/contiguous_range

Not exactly. `contiguous_range` is satisfied for `std::vector<int>`, whose size is runtime variable. My `concept ArrayLike` should be `contiguous_range` *and* checking that the size is known at compile time.

💬 maflcko commented on pull request "Update doc/policy/README.md":
(https://github.com/bitcoin/bitcoin/pull/29095#issuecomment-1862556757)
Are you still working on this, or can this be closed up for grabs?

💬 ajtowns commented on issue "Discussion: Upgrading to C++20":
(https://github.com/bitcoin/bitcoin/issues/23363#issuecomment-1862581792)
https://stackoverflow.com/questions/70482497/detecting-compile-time-constantness-of-range-size ? `std::span<X, N>` seems like it's already pretty much the thing that tells you you've got a contiguous range of exactly N X's to me though...

💬 maflcko commented on issue "Discussion: Upgrading to C++20":
(https://github.com/bitcoin/bitcoin/issues/23363#issuecomment-1862605906)
> https://stackoverflow.com/questions/70482497/detecting-compile-time-constantness-of-range-size ?

Ah nice. So this was fixed in the language, but clang never implemented it.

💬 c0deright commented on issue "Old wallet.dat: Error reading wallet database: keymeta found with unexpected path / All keys read correctly, but transaction data or address metadata may be missing or incorrect":
(https://github.com/bitcoin/bitcoin/issues/29109#issuecomment-1862723349)
Ok, thank you very much

💬 murchandamus commented on pull request "fuzz: coinselection, improve `min_viable_change`/`change_output_size`":
(https://github.com/bitcoin/bitcoin/pull/28372#issuecomment-1862768031)
ACK cd810075eddd8b1a7139559b475b56126f70a93d

Passes all my fuzzing crashes and no new crashes with some light additional fuzzing, code LGTM.

💬 nvk commented on pull request "policy: Enable full-rbf by default":
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-1862776663)
ACK

The current high fee environment is giving a good example as to why is so important to make this behaviour default.