💬 Willtech commented on issue "Setting `onlynet=onion` still makes some IPv4 connections.":
(https://github.com/bitcoin/bitcoin/issues/12344#issuecomment-1436004885)
@1440000bytes Has network been updated in all that time? There is no need to update unless there have been changes that at least have the possibility to resolve the problem. Afer we go back to v0.x versioning I will look at updating. v0 versioning is for not commercial release until it is v1 is commercial release.

💬 TheCharlatan commented on pull request "refactor, kernel: Decouple ArgsManager from blockstorage":
(https://github.com/bitcoin/bitcoin/pull/27125#issuecomment-1436078314)
> ```shell
> validation.cpp:80:13: error: using decl 'ReadBlockFromDisk' is unused [misc-unused-using-decls,-warnings-as-errors]
> using node::ReadBlockFromDisk;
> ^
> /tmp/cirrus-ci-build/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/validation.cpp:80:13: note: remove the using
> using node::ReadBlockFromDisk;
> ```

Thank you, I'm improving my clang tidy workflow.

📝 PastaPastaPasta opened a pull request: "refactor: replace all implicit C-style const/const+reinterpret with explicit casts"
(https://github.com/bitcoin/bitcoin/pull/27126)
This PR aims to replace all implicit C-style const (and const+reinterpret) casts with explicit casts to improve the code's readability and maintainability.

Implicit casts can cause subtle issues and can introduce undefined behavior. Explicit casting improves the code's readability and makes it easier to understand the intent of the code. Additionally, it makes it easier for developers to grep for these casts and do more substantial refactoring that may eliminate the need for casting altogethe
...

📝 theStack opened a pull request: "rpc: fix successful broadcast count in `submitpackage` error msg"
(https://github.com/bitcoin/bitcoin/pull/27127)
If a `submitpackage` RPC call errors due to any of the individual tx broadcasts failing, the returned error message is supposed to contain the number of successful broadcasts so far:

https://github.com/bitcoin/bitcoin/blob/4395b7f0845d2dca60f3b4e007ef5770ce8e2aa9/src/rpc/mempool.cpp#L848-L849


Right now this is wrongly always shown as zero. Fix this by adding the missing increment of the counter. While touching that area, the variable is also renamed to better reflect its purpose (s/num_s
...

💬 TheCharlatan commented on pull request "Convert ArgsManager::GetDataDir to a read-only function":
(https://github.com/bitcoin/bitcoin/pull/27073#issuecomment-1436632437)
Concept ACK

You can mark both `GetConfigFilePath` and `EnsureDataDir` as `const`.

💬 TheCharlatan commented on pull request "src/node/miner cleanups, follow-ups for #26695":
(https://github.com/bitcoin/bitcoin/pull/26883#discussion_r1111745040)
Nit: IWYU says you can now remove this include from `miner.cpp`

💬 willcl-ark commented on pull request "Convert ArgsManager::GetDataDir to a read-only function":
(https://github.com/bitcoin/bitcoin/pull/27073#discussion_r1111781354)
Added in https://github.com/bitcoin/bitcoin/commit/5626101c8d385501a60e231bbbdda9439af5e881

💬 willcl-ark commented on pull request "Convert ArgsManager::GetDataDir to a read-only function":
(https://github.com/bitcoin/bitcoin/pull/27073#issuecomment-1436739879)
> Concept ACK
>
> You can mark both `GetConfigFilePath` and `EnsureDataDir` as `const`.

Updated in 5626101

💬 1440000bytes commented on pull request "script: add description for the functionality of each opcode":
(https://github.com/bitcoin/bitcoin/pull/27109#issuecomment-1436763116)
> > It will help a lot for developers who want to do script programming.
>
> No strong opinion, but this purpose is already served by this [detailed wiki article](https://en.bitcoin.it/wiki/Script#Opcodes) presenting how each opcode works along with a bit of history. Using a "opcode, input, output, description" table it is even probably better at explaining what each opcode does than we can ever get in code comments.

Agree with this. So many single line comments look weird. If wiki has som
...

👍 TheCharlatan approved a pull request: "depends: harden libevent"
(https://github.com/bitcoin/bitcoin/pull/27118)
ACK 778e34e8625cc83d0e5a93493c71f01712bef81d

💬 glozow commented on pull request "src/node/miner cleanups, follow-ups for #26695":
(https://github.com/bitcoin/bitcoin/pull/26883#issuecomment-1436796938)
ACK 6a5e88e5cf

🚀 glozow merged a pull request: "src/node/miner cleanups, follow-ups for #26695"
(https://github.com/bitcoin/bitcoin/pull/26883)

💬 glozow commented on pull request "rpc: Add a parameter to sendrawtransaction which sets a maximum output for unspendable transactions.":
(https://github.com/bitcoin/bitcoin/pull/25943#discussion_r1111854092)
nit: (un)spendable should refer to outputs, not transactions. Same issue with the PR description.
```suggestion
- `sendrawtransaction` has a new, optional argument, `maxburnamount` with a default value of `0`. Any transaction containing an unspendable output with value greater than `maxburnamount` will not be submitted. At present, the outputs deemed unspendable are those with scripts that begin with an `OP_RETURN` code (known as 'datacarriers'), scripts that exceed the maximum script size, an
...

💬 glozow commented on pull request "rpc: Add a parameter to sendrawtransaction which sets a maximum output for unspendable transactions.":
(https://github.com/bitcoin/bitcoin/pull/25943#discussion_r1111867440)
This isn't true? If set to 0, transactions with provably unspendable outputs are fine as long as the output values are 0. Perhaps remove this line, as we wouldn't want a user to mistakenly think they're filtering out all datacarrier transactions by setting this (also, the same thing happens if this option is left blank since 0 is default).

💬 glozow commented on pull request "rpc: Add a parameter to sendrawtransaction which sets a maximum output for unspendable transactions.":
(https://github.com/bitcoin/bitcoin/pull/25943#discussion_r1111867497)
Perhaps add "If burning funds through unspendable outputs is desired, increase this value." If a downstream app is (hypothetically) regularly burning money and upgrades their bitcoind, it should be easy to understand what to do to make their error go away.

👍 TheCharlatan approved a pull request: "Convert ArgsManager::GetDataDir to a read-only function"
(https://github.com/bitcoin/bitcoin/pull/27073)
Code review ACK 0af17e161d751b15f90615800411f36e11b3fcde

💬 brunoerg commented on pull request "p2p: cleanup `LookupIntern`, `Lookup` and `LookupHost`":
(https://github.com/bitcoin/bitcoin/pull/26261#issuecomment-1436994880)
Force-pushed rebasing.

💬 vasild commented on pull request "test: add end-to-end tests for CConnman and PeerManager":
(https://github.com/bitcoin/bitcoin/pull/26812#discussion_r1111936979)
Done. This also reduced the size of the diff (the last commit). Thanks!

💬 vasild commented on pull request "test: add end-to-end tests for CConnman and PeerManager":
(https://github.com/bitcoin/bitcoin/pull/26812#issuecomment-1436999481)
`7c591c868d...a81fe4ff9b`: rebase and put the fuzz tests in `fuzz/process_message.cpp` instead of in a new file, as [suggested](https://github.com/bitcoin/bitcoin/pull/26812#discussion_r1068245906).

💬 willcl-ark commented on issue "signrawtransactionwithkey command shouldn't output the "Witness program was passed an empty witness" error for a TapRoot transaction":
(https://github.com/bitcoin/bitcoin/issues/27017#issuecomment-1437012050)
The output of `help signrawtransactionwithkey` states:

```bash
3. prevtxs (json array, optional) The previous dependent transaction outputs
[
{ (json object)
"txid": "hex", (string, required) The transaction id
"vout": n, (numeric, required) The output number
"scriptPubKey": "hex", (string, required) script key
"redeemScript": "hex", (string) (required
...