💬 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.
(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
...
(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.
(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.
(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?
(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?
(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.
(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.
(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:
...
(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)
(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...
(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
(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
...
(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.
(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.
💬 polespinasa commented on pull request "rpc:generatetomany":
(https://github.com/bitcoin/bitcoin/pull/32468#discussion_r2084243740)
Please see 5a5ff2a20cdff56b5756fa0b6c9cbb9184f17818
Tried to follow your approach, actually it makes things easier :)
Will squash commits after ack the new approach better than the first one.
(https://github.com/bitcoin/bitcoin/pull/32468#discussion_r2084243740)
Please see 5a5ff2a20cdff56b5756fa0b6c9cbb9184f17818
Tried to follow your approach, actually it makes things easier :)
Will squash commits after ack the new approach better than the first one.
💬 juanitoddd commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2871805846)
Concept NACK
As a node runner, removing this configurations limits my sovereignty over what my node relays.
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2871805846)
Concept NACK
As a node runner, removing this configurations limits my sovereignty over what my node relays.
💬 Sjors commented on pull request "qt: drop unused watch-only functionality":
(https://github.com/bitcoin/bitcoin/pull/32459#discussion_r2084301483)
Dropped the file.
(https://github.com/bitcoin/bitcoin/pull/32459#discussion_r2084301483)
Dropped the file.
👋 Sjors's pull request is ready for review: "qt: drop unused watch-only functionality"
(https://github.com/bitcoin/bitcoin/pull/32459)
(https://github.com/bitcoin/bitcoin/pull/32459)
📝 Sjors converted_to_draft a pull request: "qt: drop unused watch-only functionality"
(https://github.com/bitcoin/bitcoin/pull/32459)
The watch-only functionality in the GUI was only used for legacy wallets. Watch-only descriptor wallets do not use this.
The only visible changes of this commit should be dropping the "Spendable:" label from the overview tab.
(https://github.com/bitcoin/bitcoin/pull/32459)
The watch-only functionality in the GUI was only used for legacy wallets. Watch-only descriptor wallets do not use this.
The only visible changes of this commit should be dropping the "Spendable:" label from the overview tab.
💬 Sjors commented on pull request "qt: drop unused watch-only functionality":
(https://github.com/bitcoin/bitcoin/pull/32459#issuecomment-2871909501)
Somehow the Date column disappeared.
(https://github.com/bitcoin/bitcoin/pull/32459#issuecomment-2871909501)
Somehow the Date column disappeared.