📝 dergoegge opened a pull request: "[no merge, meta] refactor: net/net processing split"
(https://github.com/bitcoin/bitcoin/pull/28252)
This PR is supposed to provide context for some of the refactoring PRs I've been working on (#25268, #26621, etc).

The aim is to completely decouple CConnman and its internals from PeerManager to allow for isolated testing of our message processing code (isolated from net only as isolating from validation is another can of worms). To get there, this work refactors CConnman's API to use `NodeId` as the primary handle for managing connections and defines a new interface `ConnectionsInterface` t
...

💬 ChrisCho-H commented on pull request "script: throw disabled err for op_ver and its variants":
(https://github.com/bitcoin/bitcoin/pull/28169#discussion_r1290433669)
I think that's the reason why OP_VER and its variants are evaluated as disabled opcode while `OP_RESERVED` is evaluated as reserved.
actually other disabled opcodes(which throw disabled err) are based on history.

💬 ChrisCho-H commented on pull request "script: throw disabled err for op_ver and its variants":
(https://github.com/bitcoin/bitcoin/pull/28169#issuecomment-1673617042)
I move the logic to default clause following @ajtowns opinion, but still include `OP_VER` to throw disabled error(because it's disabled opcode).
I also added `reserved` for bad opcode error in 17a0270da5536547850dca06885a278d3e0d3269 which is for reserved opcode.

I mentioned above, but If there's not any meaningful difference btw disabled and reserved ones, it would make sense to throw disabled error for both of them as it's how it works for disabled opcodes.
like below

```cpp
...

...

💬 brunoerg commented on pull request "net: Continuous ASMap health check":
(https://github.com/bitcoin/bitcoin/pull/27581#discussion_r1290455071)
nit:
```suggestion
* Analyze and log current health of ASMap based buckets.
```

💬 RicYashiroLee commented on pull request "Inscriptions option":
(https://github.com/bitcoin/bitcoin/pull/27589#issuecomment-1673632015)
> @Retropex: Ask on [Bitcoin StackExchange](https://bitcoin.stackexchange.com/) whether what you are suggesting is a good idea (short answer is any default policy change or custom policy option doesn't necessarily stop inscriptions as you can still submit consensus compatible transactions directly to a miner). There is already an [ordinals tag](https://bitcoin.stackexchange.com/questions/tagged/ordinals) with previous Q&A on this topic.

The key word is 'cumbersome', to make hash-anchoring(st
...

💬 theuni commented on pull request "doc: use llvm-config for bitcoin-tidy example":
(https://github.com/bitcoin/bitcoin/pull/28245#discussion_r1290493781)
Ok, I think I see. I can force a similar error on Linux by adding "-Wl,-no-undefined" to the link-line.

I'm guessing ld64 is opinionated about undefined symbols by default. And in this case we're relying on them.

Could you try messing with undefined symbol behavior and seeing what happens?


`/opt/homebrew/opt/ccache/libexec/c++ -O3 -DNDEBUG -arch arm64 -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX13.3.sdk -bundle -Wl,-headerpad_max
...

💬 stevenroose commented on pull request "wallet: Allow users to create a wallet that encrypts all database records":
(https://github.com/bitcoin/bitcoin/pull/28142#issuecomment-1673672150)
Concept ACK :) (Concept Request, in fact)

💬 stevenroose commented on pull request "wallet: Allow users to create a wallet that encrypts all database records":
(https://github.com/bitcoin/bitcoin/pull/28142#issuecomment-1673671724)
> Concept NACK, I think. Wallet encryption is not going to help you if someone steals your wallet, only if someone gains very temporary access to your keyboard/mouse only. I don't think this full-encryption you're proposing has a real use case.
>
> (What might make sense is to support fully encrypting backups.)

I requested this because I have a real use case.. I run a full node that is on an unencrypted external drive. Bitcoin data is public data, it shouldn't be encrypted. I want to use t
...

💬 luke-jr commented on pull request "init: Add option for rpccookie permissions (replace 26088)":
(https://github.com/bitcoin/bitcoin/pull/28167#discussion_r1290509079)
I don't think we need to prepend the zero here.

👍 SambhavsCreation approved a pull request: "test: verify spend from 999-of-999 taproot multisig wallet"
(https://github.com/bitcoin/bitcoin/pull/28212#pullrequestreview-1572527277)
I think it looks good. I think the concerns about testing with different keys are not of critical importance right now. Something could be done about it in the future.

💬 jonatack commented on pull request "doc: use llvm-config for bitcoin-tidy example":
(https://github.com/bitcoin/bitcoin/pull/28245#discussion_r1290575196)
```bash
jon|master:~/bitcoin/bitcoin/contrib/devtools/bitcoin-tidy$ rm -rf build

jon|master:~/bitcoin/bitcoin/contrib/devtools/bitcoin-tidy$ cmake -S . -B build -DLLVM_DIR=$(llvm-config --cmakedir) -DCMAKE_BUILD_TYPE=Release -Wl,-undefined,dynamic_lookup
-- The C compiler identification is AppleClang 14.0.3.14030022
-- The CXX compiler identification is AppleClang 14.0.3.14030022
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /o
...

💬 jonatack commented on pull request "doc: use llvm-config for bitcoin-tidy example":
(https://github.com/bitcoin/bitcoin/pull/28245#discussion_r1290588401)
(The above works with master and 2. above, but not with 1.)

💬 theuni commented on pull request "doc: use llvm-config for bitcoin-tidy example":
(https://github.com/bitcoin/bitcoin/pull/28245#discussion_r1290589149)
Yep, looks like that worked! And it seems to be correct too, turns out [upstream LLVM does the same thing](https://github.com/llvm/llvm-project/blob/main/llvm/cmake/modules/HandleLLVMOptions.cmake#L178).

This should be the actual fix, could you please confirm that building works with it?

```diff
diff --git a/contrib/devtools/bitcoin-tidy/CMakeLists.txt b/contrib/devtools/bitcoin-tidy/CMakeLists.txt
index 9ed18696d4..24216f2fb0 100644
--- a/contrib/devtools/bitcoin-tidy/CMakeLists.txt
+
...

💬 theuni commented on pull request "doc: use llvm-config for bitcoin-tidy example":
(https://github.com/bitcoin/bitcoin/pull/28245#issuecomment-1673822972)
Nit: stale description.

@jonatack's issue is valid but unrelated. I'll PR a fix for that separately once we have it nailed down.

ACK d82bb90a5b9dc1fd48b10514bdcd8f425aced256.

💬 jonatack commented on pull request "doc: use llvm-config for bitcoin-tidy example":
(https://github.com/bitcoin/bitcoin/pull/28245#discussion_r1290624677)
```bash
jon|master =:~/bitcoin/bitcoin/contrib/devtools/bitcoin-tidy$ rm -rf build

jon|master =:~/bitcoin/bitcoin/contrib/devtools/bitcoin-tidy$ cmake -S . -B build -DLLVM_DIR=$(llvm-config --cmakedir) -DCMAKE_BUILD_TYPE=Release
-- The C compiler identification is AppleClang 14.0.3.14030022
-- The CXX compiler identification is AppleClang 14.0.3.14030022
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /opt/homebrew/opt/ccache/lib
...

🤔 furszy reviewed a pull request: "wallet: Allow users to create a wallet that encrypts all database records"
(https://github.com/bitcoin/bitcoin/pull/28142#pullrequestreview-1572652006)
> I requested this because I have a real use case.. I run a full node that is on an unencrypted external drive. Bitcoin data is public data, it shouldn't be encrypted. I want to use the wallet for a watch-only wallet though. Unfortunately the wallet files live alongside the public blockchain data so I would have to store the wallet on an unencrypted drive (that I occasionally want to borrow out to friends for them to copy the blockchain in order not to have to download it). That would reveal my
...

💬 jonatack commented on pull request "doc: use llvm-config for bitcoin-tidy example":
(https://github.com/bitcoin/bitcoin/pull/28245#issuecomment-1673842977)
ACK

The updated steps appear to be working in https://github.com/bitcoin/bitcoin/pull/28245#discussion_r1290624677 apart from the separate issue.

💬 Ayush170-Future commented on pull request "fuzz: wallet, add target for Spend":
(https://github.com/bitcoin/bitcoin/pull/28236#issuecomment-1673851694)
Thanks, @brunoerg for your reviews! In the recent push, I've addressed most of your reviews.

- `clang-format` to fix the lint's issue.
- Changed `PickValueFromArray` to the `PickValue` approach.
- Fixed one grammar mistake.

Your [comment](https://github.com/bitcoin/bitcoin/pull/28236#discussion_r1286425941) is really interesting, but I'm not sure whether the `non-determinism` in this case might pose a significant issue or not.
So, I'll wait for more people to give their thoughts on this
...

🤔 jarolrod reviewed a pull request: "ci: Run "macOS native x86_64" job on GitHub Actions"
(https://github.com/bitcoin/bitcoin/pull/28187#pullrequestreview-1572659956)
ACK 9658d0dc17c270138523c41a982425e276b24271

As long as permissions are looked over and activation of Github Workflows and Actions is safe here: https://docs.github.com/en/rest/actions/permissions?apiVersion=2022-11-28

💬 jarolrod commented on pull request "ci: Run "macOS native x86_64" job on GitHub Actions":
(https://github.com/bitcoin/bitcoin/pull/28187#discussion_r1290633471)
nit: if there is any opinion on the pr/commit title not being sufficient, you could make it something like the following (and add a body):

```
ci: Run "macOS native" job on Github Actions, revert back to x86_64

Updates the macOS native ci job to run on Github Actions which will
allow us to stay on a free plan for the job. Additionally, temporarily
revert back to running on a x86_64 architecture until Github Actions
has a arm64 (Apple Silicon) runner; which is preferable to no macOS na
...