💬 polespinasa commented on pull request "rpc:generatetomany":
(https://github.com/bitcoin/bitcoin/pull/32468#issuecomment-2870027925)
> How is it duplicating entries?

First address is duplicated. Anyway it is duplicated in case you want to assign an amount to an address + remainder.

👍 TheCharlatan approved a pull request: "checkqueue: make the queue non-optional for CCheckQueueControl and drop legacy locking macro usage"
(https://github.com/bitcoin/bitcoin/pull/32467#pullrequestreview-2831468672)
ACK 5aca850c205a20a0f198827f9797fb8053f2b3dd

💬 TheCharlatan commented on pull request "checkqueue: make the queue non-optional for CCheckQueueControl and drop legacy locking macro usage":
(https://github.com/bitcoin/bitcoin/pull/32467#discussion_r2083592266)
I think it would be nice if we could get some kind of locking warning here if a second `CCheckQueueControl` is created from the same `CCheckQueue`, instead of just deadlocking. Tried coming up with a combination of LOCK and UNLOCK_FUNCTION annotation, but could not find something that actually worked.

💬 Cyberwiz9000 commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2870050321)
Approach NACK

I can understand the technical reasons for lifting the `OP_RETURN` but I oppose the removal of node configurability. Both `-datacarrier` and `-datacarriersize` should be retained. Node operators should be able to choose what kind of data they relay without having to rely on other Node implementations that have less developper support.

Additionally, I object to allowing multiple `OP_RETURN` outputs per transaction. This may increase mempool complexity, raise attack surface, an
...

💬 liviu-liviu commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2870055985)
Concept NACK We should create more (and possibly easier) avenues that help node operators express their will. This PR is moving us in the opposite direction.

💬 liviu-liviu commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2870056177)
Concept NACK We should create more (and possibly easier) avenues that help node operators express their will. This PR is moving us in the opposite direction.

💬 stringintech commented on pull request "kernel: Separate UTXO set access from validation functions":
(https://github.com/bitcoin/bitcoin/pull/32317#discussion_r2083615207)
Oh, it makes sense. Thanks for the insights.

💬 l0rinc commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2870192790)
> I think we should have an option (default on) to obfuscate the block data files on startup if they are not yet obfuscated.

For the record, @andrewtoth pushed a separate tool to fix this issue since, see: https://github.com/bitcoin/bitcoin/pull/32451

---

As for removing the limit, I don't think it makes any meaningful difference either way; this discussion was way overblown for some reason. My only objection was how aggressively this was pushed - we should have started with a long and
...

💬 1440000bytes commented on pull request "[Policy] Discourage Unsigned Annexes":
(https://github.com/bitcoin/bitcoin/pull/32453#issuecomment-2870220123)
Concept ACK

I couldn't think of any legit use case that would require unsigned annex.

💬 1440000bytes commented on pull request "config: allow setting -proxy per network":
(https://github.com/bitcoin/bitcoin/pull/32425#issuecomment-2870221917)
Concept ACK

Multiple proxies for different networks would be useful. Might need release notes if this eventually gets merged.

💬 1440000bytes commented on pull request "wallet: init, don't error out when loading legacy wallets":
(https://github.com/bitcoin/bitcoin/pull/32449#discussion_r2083631109)
Would this look better with a multi line comment?

💬 1440000bytes commented on pull request "contrib: add xor-blocks tool to obfuscate blocks directory":
(https://github.com/bitcoin/bitcoin/pull/32451#issuecomment-2870224955)
Concept ACK

Could be a useful tool for paranoid users who can run this tool.

Is it possible to add this feature in bitcoin core itself?

🤔 1440000bytes reviewed a pull request: "rpc: Undeprecate rpcuser/rpcpassword, store all credentials hashed in memory"
(https://github.com/bitcoin/bitcoin/pull/32423#pullrequestreview-2831519829)
utACK https://github.com/bitcoin/bitcoin/pull/32423/commits/e49a7274a2141dcb9e188bc4b45c2d7b928ccecd

Although I am not sure if it doesn't break anything.

💬 maflcko commented on pull request "rpc:generatetomany":
(https://github.com/bitcoin/bitcoin/pull/32468#discussion_r2083836431)
test-only code is code that is only called by tests (unit, or functional tests, or on regtest, testnet, signet, ...), but never on mainnet.

`static UniValue generateBlocks` is a test-only function, so a similar approach could be used here.

⚠️ Lompecacas opened an issue: "@gmaxwell was asking about testing. This was tested successfully with a one-line pynode patch (pynode HEAD deb07f51435cd0d18596bddfee28e337a5a6454e). pynode issues 'mempool' at startup, and fills its own mempool with the returned results."
(https://github.com/bitcoin/bitcoin/issues/32470)
@gmaxwell was asking about testing. This was tested successfully with a one-line pynode patch (pynode HEAD deb07f51435cd0d18596bddfee28e337a5a6454e). pynode issues 'mempool' at startup, and fills its own mempool with the returned results.

```
--- a/node.py
+++ b/node.py
@@ -238,6 +238,7 @@ class NodeConn(asyncore.dispatcher):
self.send_message(msg_verack(self.ver_send))
if self.ver_send >= CADDR_TIME_VERSION:

...

✅ maflcko closed an issue: "@gmaxwell was asking about testing. This was tested successfully with a one-line pynode patch (pynode HEAD deb07f51435cd0d18596bddfee28e337a5a6454e). pynode issues 'mempool' at startup, and fills its own mempool with the returned results."
(https://github.com/bitcoin/bitcoin/issues/32470)

💬 polespinasa commented on pull request "rpc:generatetomany":
(https://github.com/bitcoin/bitcoin/pull/32468#discussion_r2083962636)
So you suggest duplicating `CreateNewBlock` with a test-only version?
I don't see how to implement the changes outside that function...

💬 maflcko commented on pull request "rpc:generatetomany":
(https://github.com/bitcoin/bitcoin/pull/32468#discussion_r2083970563)
It is possible to call CNB from the test-only function. Something like:

```cpp
def CNB_test()
b = CNB()
b.vtx[0].output_script = b'aa'
... etc

🤔 janb84 reviewed a pull request: "rpc: Undeprecate rpcuser/rpcpassword, store all credentials hashed in memory"
(https://github.com/bitcoin/bitcoin/pull/32423#pullrequestreview-2832467514)
reACK https://github.com/bitcoin/bitcoin/commit/e49a7274a2141dcb9e188bc4b45c2d7b928ccecd

Changes since last ACK:
- Added Enum + logic to use Enum in GenerateAuthCookieResult and InitRPCAuthentication
- the user and pass are now stored separate variables
- small change in the logwarning

Reviewed:

- Compiled & run tests ✅
- Code review ✅
- Some manual testing ✅ (Did some additional testing and weirdly enough an empty user with a password is allowed. This is consistent with the curre
...

💬 polespinasa commented on pull request "rpc:generatetomany":
(https://github.com/bitcoin/bitcoin/pull/32468#discussion_r2084241023)
This is necessary to avoid `RegenerateCommitments` to return -1 so breaking the code.
Idk if there's a better approach to it.