💬 fanquake commented on pull request "[25.x] Backports":
(https://github.com/bitcoin/bitcoin/pull/28768#issuecomment-1893369155)
> Also, PR description needs to be updated to remove the reference to https://github.com/bitcoin/bitcoin/pull/28919.
Updated.
> https://github.com/bitcoin/bitcoin/pull/29200 would be relevant.
There'll be another 25.x backports PR, so it could be included there.
(https://github.com/bitcoin/bitcoin/pull/28768#issuecomment-1893369155)
> Also, PR description needs to be updated to remove the reference to https://github.com/bitcoin/bitcoin/pull/28919.
Updated.
> https://github.com/bitcoin/bitcoin/pull/29200 would be relevant.
There'll be another 25.x backports PR, so it could be included there.
💬 glozow commented on pull request "log mempool loading progress":
(https://github.com/bitcoin/bitcoin/pull/29227#issuecomment-1893375082)
Leaving gui notifications (https://github.com/bitcoin/bitcoin/pull/29227#discussion_r1452031664) and dumping logs (https://github.com/bitcoin/bitcoin/pull/29227#pullrequestreview-1817064507) up for grabs
(https://github.com/bitcoin/bitcoin/pull/29227#issuecomment-1893375082)
Leaving gui notifications (https://github.com/bitcoin/bitcoin/pull/29227#discussion_r1452031664) and dumping logs (https://github.com/bitcoin/bitcoin/pull/29227#pullrequestreview-1817064507) up for grabs
🚀 fanquake merged a pull request: "build: remove `--enable-lto`"
(https://github.com/bitcoin/bitcoin/pull/29185)
(https://github.com/bitcoin/bitcoin/pull/29185)
💬 fanquake commented on pull request "depends: add NM output to gen_id":
(https://github.com/bitcoin/bitcoin/pull/29249#discussion_r1453173548)
Yep, bad copy-paste.
(https://github.com/bitcoin/bitcoin/pull/29249#discussion_r1453173548)
Yep, bad copy-paste.
💬 glozow commented on pull request "log mempool loading progress":
(https://github.com/bitcoin/bitcoin/pull/29227#issuecomment-1893393129)
26.x backport in #29209
(https://github.com/bitcoin/bitcoin/pull/29227#issuecomment-1893393129)
26.x backport in #29209
👍 stickies-v approved a pull request: "[25.x] Backports"
(https://github.com/bitcoin/bitcoin/pull/28768#pullrequestreview-1822937595)
ACK 53bbda51141e48730b95913c6ae8e1ea50578fbb
(https://github.com/bitcoin/bitcoin/pull/28768#pullrequestreview-1822937595)
ACK 53bbda51141e48730b95913c6ae8e1ea50578fbb
💬 maflcko commented on pull request "test: Remove all-lint.py script":
(https://github.com/bitcoin/bitcoin/pull/29228#issuecomment-1893412173)
> looks like we can replace the [comment to run all the lint commands](https://github.com/bitcoin/bitcoin/blob/master/test/README.md)
Thanks, added a commit to move the documentation to one place.
(https://github.com/bitcoin/bitcoin/pull/29228#issuecomment-1893412173)
> looks like we can replace the [comment to run all the lint commands](https://github.com/bitcoin/bitcoin/blob/master/test/README.md)
Thanks, added a commit to move the documentation to one place.
💬 maflcko commented on pull request "test: Treat msg_version.relay as unsigned, Remove `struct` packing in messages.py":
(https://github.com/bitcoin/bitcoin/pull/29067#issuecomment-1893416499)
CI failure can be ignored
(https://github.com/bitcoin/bitcoin/pull/29067#issuecomment-1893416499)
CI failure can be ignored
💬 maflcko commented on pull request "util: Faster std::byte (pre)vector (un)serialize":
(https://github.com/bitcoin/bitcoin/pull/29114#issuecomment-1893418212)
CI failure can be ignored
(https://github.com/bitcoin/bitcoin/pull/29114#issuecomment-1893418212)
CI failure can be ignored
💬 willcl-ark commented on issue "rpc: actually deprecate `rpcuser` & `rpcpass`":
(https://github.com/bitcoin/bitcoin/issues/29240#issuecomment-1893453193)
Surprisingly I found very few instances of `rpcuser` and `rpcpassword` remaining in the docs. I updated their usage, and the example init scripts, in [this branch](https://github.com/bitcoin/bitcoin/compare/master...willcl-ark:bitcoin:deprecate-rpcuser) to see the scope of changes that would be required on the doc side.
I think if we want to fully deprecate these options, the changes in that branch along with #28167 as mentioned above, should come first.
My opinion is that it would be best
...
(https://github.com/bitcoin/bitcoin/issues/29240#issuecomment-1893453193)
Surprisingly I found very few instances of `rpcuser` and `rpcpassword` remaining in the docs. I updated their usage, and the example init scripts, in [this branch](https://github.com/bitcoin/bitcoin/compare/master...willcl-ark:bitcoin:deprecate-rpcuser) to see the scope of changes that would be required on the doc side.
I think if we want to fully deprecate these options, the changes in that branch along with #28167 as mentioned above, should come first.
My opinion is that it would be best
...
🚀 fanquake merged a pull request: "doc: update -loglevel help to add `info` to the always logged levels"
(https://github.com/bitcoin/bitcoin/pull/29230)
(https://github.com/bitcoin/bitcoin/pull/29230)
🚀 fanquake merged a pull request: "[25.x] Backports"
(https://github.com/bitcoin/bitcoin/pull/28768)
(https://github.com/bitcoin/bitcoin/pull/28768)
💬 vasild commented on pull request "RFC: Deprecate libconsensus":
(https://github.com/bitcoin/bitcoin/pull/29189#issuecomment-1893591435)
Concept ACK on removing the library.
This PR documents the library as deprecated, removes its tests but leaves the library. So, for some time there will be a library that will be untested. What is the merit of doing that? Isn't it better to document it as deprecated now and later remove the library and its tests at the same time?
(https://github.com/bitcoin/bitcoin/pull/29189#issuecomment-1893591435)
Concept ACK on removing the library.
This PR documents the library as deprecated, removes its tests but leaves the library. So, for some time there will be a library that will be untested. What is the merit of doing that? Isn't it better to document it as deprecated now and later remove the library and its tests at the same time?
👍 stickies-v approved a pull request: "logging: Update to new logging API"
(https://github.com/bitcoin/bitcoin/pull/29231#pullrequestreview-1823164145)
ACK 1441ab74d6cd40e75f1dba80b68bc4b1791bf667
(https://github.com/bitcoin/bitcoin/pull/29231#pullrequestreview-1823164145)
ACK 1441ab74d6cd40e75f1dba80b68bc4b1791bf667
👍 stickies-v approved a pull request: "[26.x] more backports"
(https://github.com/bitcoin/bitcoin/pull/29209#pullrequestreview-1823233187)
ACK 234f32860dd0e450e214c93af09e9dd9df20e9ea
(https://github.com/bitcoin/bitcoin/pull/29209#pullrequestreview-1823233187)
ACK 234f32860dd0e450e214c93af09e9dd9df20e9ea
💬 Kenshiro-28 commented on issue "Implement PayJoin / Pay-to-EndPoint":
(https://github.com/bitcoin/bitcoin/issues/19148#issuecomment-1893693072)
@DanGould while PayJoin offers improved privacy over traditional Bitcoin transactions, there's a concern about the potential exposure of UTXO balances. This is particularly risky for users with substantial BTC holdings, as the balance of the UTXOs used in a transaction is visible to the payee.
I propose an enhancement to PayJoin that involves using UTXOs below a certain threshold (e.g., 0.1 BTC). This would entail:
- Threshold Implementation: Automatically selecting UTXOs for PayJoin trans
...
(https://github.com/bitcoin/bitcoin/issues/19148#issuecomment-1893693072)
@DanGould while PayJoin offers improved privacy over traditional Bitcoin transactions, there's a concern about the potential exposure of UTXO balances. This is particularly risky for users with substantial BTC holdings, as the balance of the UTXOs used in a transaction is visible to the payee.
I propose an enhancement to PayJoin that involves using UTXOs below a certain threshold (e.g., 0.1 BTC). This would entail:
- Threshold Implementation: Automatically selecting UTXOs for PayJoin trans
...
💬 kristapsk commented on issue "Implement PayJoin / Pay-to-EndPoint":
(https://github.com/bitcoin/bitcoin/issues/19148#issuecomment-1893710520)
For payjoin receiver, IMHO it makes sense to always contribute smallest amount UTXO available. That way you likely don't expose some big value UTXOs (assuming they aren't the only ones in your wallet) and also it works as UTXO consolidation.
(https://github.com/bitcoin/bitcoin/issues/19148#issuecomment-1893710520)
For payjoin receiver, IMHO it makes sense to always contribute smallest amount UTXO available. That way you likely don't expose some big value UTXOs (assuming they aren't the only ones in your wallet) and also it works as UTXO consolidation.
💬 stickies-v commented on pull request "log: Nuke error(...)":
(https://github.com/bitcoin/bitcoin/pull/29236#discussion_r1453410392)
nit: it seems the return value from `SerializeFileDB` is never used to actually shut down the node. In the case of failing to open file, I think `LogError` is appropriate because that probably should lead to a shutdown. In the case of "Failed to flush file", I think a `LogWarning` could be more appropriate.
I think it's best to leave the PR as is to make progress, bikeshedding over warning vs error categories doesn't have a huge amount of benefit here. Just leaving this comment for visibility
...
(https://github.com/bitcoin/bitcoin/pull/29236#discussion_r1453410392)
nit: it seems the return value from `SerializeFileDB` is never used to actually shut down the node. In the case of failing to open file, I think `LogError` is appropriate because that probably should lead to a shutdown. In the case of "Failed to flush file", I think a `LogWarning` could be more appropriate.
I think it's best to leave the PR as is to make progress, bikeshedding over warning vs error categories doesn't have a huge amount of benefit here. Just leaving this comment for visibility
...
🤔 stickies-v reviewed a pull request: "log: Nuke error(...)"
(https://github.com/bitcoin/bitcoin/pull/29236#pullrequestreview-1823323902)
Approach ACK fa7658b8d190580eab385691a5e571035e399129
I too have been confused by `return error` many a times.
(https://github.com/bitcoin/bitcoin/pull/29236#pullrequestreview-1823323902)
Approach ACK fa7658b8d190580eab385691a5e571035e399129
I too have been confused by `return error` many a times.
💬 brunoerg commented on pull request "RFC: Deprecate libconsensus":
(https://github.com/bitcoin/bitcoin/pull/29189#issuecomment-1893731972)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/29189#issuecomment-1893731972)
Concept ACK