💬 davidgumberg commented on pull request "deps: Bump lief to 0.16.5":
(https://github.com/bitcoin/bitcoin/pull/32431#discussion_r2076875091)
In the process of testing this, I'll mark as draft in the meanwhile.

💬 davidgumberg commented on pull request "deps: Bump lief to 0.16.5":
(https://github.com/bitcoin/bitcoin/pull/32431#issuecomment-2857251032)
> From what i understand, that's only used for the example, not for the library itself. The bcrypt-using code (`fill_random` in `examples/examples_util.h`) is never included in the library.
>
> It's unlikely that the LIEF version changes anything there.

Oops, my bad, I must have ran `symbol-check.py` against a stale build of the `bcrypt` branch, rebased to drop adding `bcrypt` to `symbol-check.py`, marking draft until I test this as working in Guix.

📝 davidgumberg converted_to_draft a pull request: "deps: Bump lief to 0.16.5"
(https://github.com/bitcoin/bitcoin/pull/32431)
Partially resolves https://github.com/bitcoin/bitcoin/issues/30520

1. Fixes an existing bug, where `bcrypt.dll` was not one of the expected symbols in `contrib/devtools/symbol-check.py`, even though this symbol gets included by way of `secp256k1`:https://github.com/bitcoin/bitcoin/blob/44057fe38ccc5d05a6249413467e002db2ae023d/src/secp256k1/examples/CMakeLists.txt#L9
2. Updates the version of the `lief` package to 0.16.5
3. Add usage notes for `contrib/devtools/security-check.py`

💬 1440000bytes commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2857253356)
After reading all comment and reviewing code.

utACK https://github.com/bitcoin/bitcoin/pull/32406/commits/3b3fafdb1c7eca8010194d0063579d44d555d546

💬 Antiscaliger commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2857253461)
For many years now, the OP_RETURN size limit has been a steady deterrent to permanent users/developers who can hype up another technology and clog the base of the canonical chain.

I believe that the current release of the OP_RETURN restriction is definitely not the right time and is an artificial disturbance that distracts from real proposals.

💬 hebasto commented on issue "Depends toolchain doesn't contain enough info to build from depends on a fresh NixOS install":
(https://github.com/bitcoin/bitcoin/issues/32428#issuecomment-2857297811)
> > The problem is caused by `set(CMAKE_FIND_USE_CMAKE_ENVIRONMENT_PATH OFF)` in the toolchain file:
> >
> > bitcoin/depends/toolchain.cmake.in](https://github.com/bitcoin/bitcoin/blob/baa848b8d38187ce6b24a57cfadf1ea180209711/depends/toolchain.cmake.in#L86
> >
> > and I believe it goes away if you remove that line. I commented on this previously in [#30940 (review)](https://github.com/bitcoin/bitcoin/pull/30940#pullrequestreview-2322355196)
>
> Here is some additional context on how
...

💬 hebasto commented on issue "Depends toolchain doesn't contain enough info to build from depends on a fresh NixOS install":
(https://github.com/bitcoin/bitcoin/issues/32428#issuecomment-2857343533)
cc @purpleKarrot

⚠️ amg1127 opened an issue: ""rpcallowip=" configuration directive doesn't accept RFC4193 addresses"
(https://github.com/bitcoin/bitcoin/issues/32433)
### Is there an existing issue for this?

- [x] I have searched the existing issues

### Current behaviour

I have a local dual-stack network using RFC1918 addresses on IPv4 stack and RFC4193 addresses on IPv6 stack. I would like to restrict the access to RPC ports of Bitcoin Core nodes running in the network based on the source address of the connections. However, Bitcoin Core 29.0 doesn't accept `rpcallowip=` directives that specify addresses under the `[fc00::/7]` IP block.

### Expected beha
...

💬 i5hi commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2857453312)
> > As a node runner I should be allowed to configure my mempool and not have it filled with what I consider spam.
>
> @i5hi what is the use case for running a mempool at all then?

Filtering spam* doesn't change the usecase of running a mempool. Broadcasting and relaying transactions (that I care about) so they reach miners.

spam* - being subjective since we all seem to have differing opinions on what spam is.

🤔 janb84 reviewed a pull request: "docs: clarify RPC credentials security boundary"
(https://github.com/bitcoin/bitcoin/pull/32424#pullrequestreview-2820704745)
reACK [348dc97](https://github.com/bitcoin/bitcoin/commit/348dc97608e0988e1974200cd531b29477dd374e)

Changes since last ACK:
- Small textual change

@crStiv thnx for incorporating my nit !

💬 i5hi commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2857488874)
Just beacuse these transactions are valid bitcoin transactions and pay fees is not a good argument against it being spam.

Spam in any protocol manages to make its way into the system by staying within the rules of the protocol. So all spam are valid protocol messages.

The reason why these data blobs are considered spam is because they are competing for block space against genuine financial transactions which is the usecase of bitcoin as money - not for storage of arbitrary data. Just beca
...

💬 i5hi commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2857496855)
The reason why Bitcoin is the best monetary network IMHO is because we have maintained conservative values on what the chain is specialized to do.

📝 maflcko opened a pull request: "lint: Remove string exclusion from locale check"
(https://github.com/bitcoin/bitcoin/pull/32434)
The exclusion isn't needed. In fact, it prevents detection of `"bla" + wrong()`.

For example, the following is not detected:

```diff
diff --git a/src/wallet/rpc/addresses.cpp b/src/wallet/rpc/addresses.cpp
index 1c2951deee..c1209013e5 100644
--- a/src/wallet/rpc/addresses.cpp
+++ b/src/wallet/rpc/addresses.cpp
@@ -336,7 +336,8 @@ RPCHelpMan addmultisigaddress()
RPCHelpMan keypoolrefill()
{
return RPCHelpMan{"keypoolrefill",
- "\nFills the keypool."+
+
...

💬 maflcko commented on pull request "docs: Improve `keypoolrefill` RPC docs":
(https://github.com/bitcoin/bitcoin/pull/32429#discussion_r2077012046)
```suggestion
"Refills each descriptor keypool in the wallet up to the specified number of new keys.\n"
```

nit: This is trimmed anyway.

💬 maflcko commented on pull request "docs: Improve `keypoolrefill` RPC docs":
(https://github.com/bitcoin/bitcoin/pull/32429#discussion_r2077039662)
I don't think to_string is allowed, see https://github.com/bitcoin/bitcoin/pull/32434

Could use ToString?

💬 laanwj commented on pull request "random: Use modern Windows randomness functions":
(https://github.com/bitcoin/bitcoin/pull/32400#issuecomment-2857505743)
GUIX build:
```
db8daad79781d10e52b1b225343fd72081ba717b484b44068ad10d646bdcb89c guix-build-345944bd6cf8/output/dist-archive/bitcoin-345944bd6cf8.tar.gz
55ceb9d4bd2f4707b6823d69403c659a8b1af96a24b0e4b2a6ad4a288a98214f guix-build-345944bd6cf8/output/x86_64-w64-mingw32/SHA256SUMS.part
978a9f8d8726d0fe1b81fa498f8601f7b33f88ac1fb9a4b71026edadd879d7b7 guix-build-345944bd6cf8/output/x86_64-w64-mingw32/bitcoin-345944bd6cf8-win64-codesigning.tar.gz
2668cceae2d6793ff38b25a42987b9c3b491c270377e3df
...

💬 fanquake commented on pull request "deps: Bump lief to 0.16.5":
(https://github.com/bitcoin/bitcoin/pull/32431#issuecomment-2857513292)
> Add usage notes for contrib/devtools/security-check.py

I think you can actually remove these docs. The symbol / security scripts are not for general developer usage, and there's no expectation that the scripts should pass, outside of a Guix build. We could move them out of contrib to make this more clear.

> Fixes an existing bug, where bcrypt.dll was not one of the expected symbols in contrib/devtools/symbol-check.py, even though this symbol gets included by way of secp256k1:

This see
...

💬 i5hi commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2857513687)
anyway; i've made all my important points. I don't want to bloat this discussion with points already made but its worth repeating that:

```
If we are removing limits on OP_RETURN as the default setting for technical reasons thats fair; but not allowing a node runner to configure limits on their mempool to make it feasible to run a node (in terms of RAM) goes against our core value of making nodes easy to run for personal use.
```

I hope the core devs to act with integrity and do what is
...

💬 i5hi commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2857514867)
anyway; i've made all my important points. I don't want to bloat this discussion with points already made but its worth repeating that:

**If we are removing limits on OP_RETURN as the default setting for technical reasons thats fair; but not allowing a node runner to configure limits on their mempool to make it feasible to run a node (in terms of RAM) goes against our core value of making nodes easy to run for personal use.**

I hope the core devs to act with integrity and do what is right
...

💬 fanquake commented on pull request "deps: Bump lief to 0.16.5":
(https://github.com/bitcoin/bitcoin/pull/32431#discussion_r2077060913)
Was this added to appease a linter? There should be no need for code like this, as every file passed to these scripts in Guix, is a binary. Leaving the script to fail if `.parse` fails is fine.