💬 theStack commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1435835891)
more-pythonic-nit:
```suggestion
kwargs['services'] = kwargs.get('services', P2P_SERVICES) | NODE_P2P_V2
```
(also in the `add_outbound_p2p_connection` method below)

👍 theStack approved a pull request: "test/BIP324: functional tests for v2 P2P encryption"
(https://github.com/bitcoin/bitcoin/pull/24748#pullrequestreview-1795584001)
ACK ad0ae3d2128d04ff4f62a4bf612286d153dc314b

💬 theStack commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1435840695)
nit: IIUC, `b""` is also returned if the decrypted package content is empty. As call-sites currently use
the length integer return value (0) to check for the "only part of packet is received" condition, this isn't a problem, but maybe the description can be improved to avoid confusion.

💬 1440000bytes commented on pull request "Compressed Bitcoin Transactions":
(https://github.com/bitcoin/bitcoin/pull/29134#issuecomment-1868547068)
Concept ACK

25-50% savings look good and compressed raw transactions can help in some cases as mentioned in the PR description.

Related Q&A: https://bitcoin.stackexchange.com/questions/98886/compress-transaction-hex-string (still wont fit in one text message but will need less text messages)

🤔 furszy reviewed a pull request: "p2p: make block download logic aware of limited peers threshold"
(https://github.com/bitcoin/bitcoin/pull/28120#pullrequestreview-1795601906)
Sorry for the delay @naumenkogs. 've been on #28170 before returning here. All good now, it is ready to go.

> The code looks alright, but i'd still rather have [this](https://github.com/bitcoin/bitcoin/pull/28120#pullrequestreview-1557211930) addressed first. It looks like even #28170 doesn't directly handle this, but rather prepare the codebase for another PR handling it?

The concern was:
All connections are from limited peers, we are lagging behind and we no longer disconnect from them
...

💬 ismaelsadeeq commented on pull request "Policy: Report reason inputs are non standard":
(https://github.com/bitcoin/bitcoin/pull/29060#discussion_r1435850669)
Thanks updated

💬 fjahr commented on pull request "refactor: C++20: Use std::rotl":
(https://github.com/bitcoin/bitcoin/pull/29085#issuecomment-1868564680)
> If we are going to change it here, should also switch in `hash.cpp`

Done, thanks! I grepped for it didn't find this one, probably because of the all-caps.

💬 GregTonoski commented on issue "./bitcoin.conf file should not cause confusion with ./datadir/bitcoin.conf":
(https://github.com/bitcoin/bitcoin/issues/29139#issuecomment-1868893820)
Yes, the exact prefix in the file is:
```
##
## bitcoin.conf configuration file.
## Generated by contrib/devtools/gen-bitcoin-conf.sh.
##
## Lines beginning with # are comments.
## All possible configuration options are provided. To use, copy this file
## to your data directory (default or specified by -datadir), uncomment
## options you would like to change, and save the file.
##
```

I would suggest renaming the file, e.g. "example-bitcoin.conf".

💬 dimitaracev commented on issue "ci: failure in `wallet_multiwallet.py --legacy-wallet`":
(https://github.com/bitcoin/bitcoin/issues/29073#issuecomment-1868965467)
I can take a look at this.

💬 ismaelsadeeq commented on pull request "Policy: Report reason inputs are non standard":
(https://github.com/bitcoin/bitcoin/pull/29060#discussion_r1436080062)
>Alternatively, could use util::Result failure values after https://github.com/bitcoin/bitcoin/pull/25665 is merged.

will resolve this since I am now using `TxValidationState`

📝 22388o opened a pull request: "doc: upgrade Bitcoin Core license to 2024"
(https://github.com/bitcoin/bitcoin/pull/29142)
See [239450](https://github.com/bitcoin/bitcoin/pull/239450), [20805] (https://github.com/bitcoin/bitcoin/pull/20805), [17801](https://github.com/bitcoin/bitcoin/pull/17801), [15061](https://github.com/bitcoin/bitcoin/pull/15061) , [26748](https://github.com/bitcoin/bitcoin/pull/26748)

Every year had his upgrade :)

🤔 maflcko requested changes to a pull request: "doc: upgrade Bitcoin Core license to 2024"
(https://github.com/bitcoin/bitcoin/pull/29142#pullrequestreview-1795889686)
It should be enough to just link to last years' change in the description. Also, make sure the changes are complete and the commit message is accurate.

💬 maflcko commented on issue "ci: failure in `wallet_multiwallet.py --legacy-wallet`":
(https://github.com/bitcoin/bitcoin/issues/29073#issuecomment-1868987998)
The error seems to be: ` node0 stderr bitcoin-node: wallet/wallet.cpp:246: void wallet::UnloadWallet(std::shared_ptr<CWallet> &&): Assertion `it.second' failed. `?

So I think another bug would be that the error is not caught and reported by the python code?

💬 22388o commented on pull request "doc: upgrade Bitcoin Core license to 2024":
(https://github.com/bitcoin/bitcoin/pull/29142#issuecomment-1868989208)
> It should be enough to just link to last years' change in the description. Also, make sure the changes are complete and the commit message is accurate.

Ok

💬 22388o commented on pull request "doc: upgrade Bitcoin Core license to 2024":
(https://github.com/bitcoin/bitcoin/pull/29142#issuecomment-1868989343)
> It should be enough to just link to last years' change in the description. Also, make sure the changes are complete and the commit message is accurate.

So I can close PR?

💬 maflcko commented on pull request "refactor: C++20: Use std::rotl":
(https://github.com/bitcoin/bitcoin/pull/29085#issuecomment-1868992264)
I guess this is fine, because the shift is constant and the compiler can skip the expensive modulo?

https://github.com/llvm/llvm-project/blob/9fba1d5f3a52af0ae62f386d0c494bd9510fa845/libcxx/include/__bit/rotate.h#L35

💬 maflcko commented on pull request "doc: upgrade Bitcoin Core license to 2024":
(https://github.com/bitcoin/bitcoin/pull/29142#issuecomment-1868992681)
Please squash your commits according to https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits

💬 dimitaracev commented on issue "ci: failure in `wallet_multiwallet.py --legacy-wallet`":
(https://github.com/bitcoin/bitcoin/issues/29073#issuecomment-1868993395)
Maybe we should return a proper message that indicates that the wallet we are trying to unload is already marked for unloading and we can catch that in the python code.

💬 sipa commented on pull request "refactor: C++20: Use std::rotl":
(https://github.com/bitcoin/bitcoin/pull/29085#issuecomment-1869001285)
@maflcko Constant propagation + strength reduction should make that code not emit an actual modulo operation in any compiler from the last 20 years.

But hopefully that pattern in libc++ also matches whatever pattern matching clang has to make sure it instead emits an actual rot instruction on platforms where it exists.

💬 Lajandra commented on issue "ci: failure in `wallet_basic.py --descriptors`":
(https://github.com/bitcoin/bitcoin/issues/29110#issuecomment-1869042800)
> https://cirrus-ci.com/task/5898005006516224?logs=ci#L2515 :
>
> ```shell
> _20231218_134952/wallet_basic_259/node0/regtest/wallets/default_wallet/wallet.dat] SQLite Statement: ROLLBACK TRANSACTION
> node0 2023-12-18T13:53:29.193915Z [scheduler] [wallet/sqlite.cpp:400] [Close] SQLiteBatch: Batch closed unexpectedly without the transaction being explicitly committed or aborted
> node0 2023-12-18T13:53:29.194038Z [scheduler] [wallet/sqlite.cpp:53] [TraceSqlCallback] [/ci_container_base/
...