📝 fanquake locked a pull request: "ci: Improve `ccache` cache hit rates"
(https://github.com/bitcoin/bitcoin/pull/27083)
Changes that are not affecting binaries, e.g., #27070 or #27072, are expected to show up to 100% `ccache` hit rate. But that is not the case:
- https://cirrus-ci.com/build/4720733276864512
- https://cirrus-ci.com/build/5915469295648768

Zero hit rate in the "macOS 10.15" is well [known](https://github.com/bitcoin/bitcoin/issues/21552). But a [fix](https://github.com/bitcoin/bitcoin/pull/24620) was [considered](https://github.com/bitcoin/bitcoin/pull/24620#issuecomment-1100850691) not "a good
...

📝 fanquake locked a pull request: "mempool: Increase mempool default size"
(https://github.com/bitcoin/bitcoin/pull/27079)
> Bitcoin Core 0.12 will have a strict maximum size on the mempool. The
default value is 300 MB and can be configured with the `-maxmempool`
parameter. Whenever a transaction would cause the mempool to exceed
its maximum size, the transaction that (along with in-mempool descendants) has
the lowest total feerate (as a package) will be evicted and the node's effective
minimum relay feerate will be increased to match this feerate plus the initial
minimum relay feerate. The initial minimum rel
...

📝 fanquake locked a pull request: "ci: Make `ccache` hashing content of compiler binary"
(https://github.com/bitcoin/bitcoin/pull/27077)
By default, `ccache` includes the modification time (`mtime`) and size of the compiler in the hash to ensure that results retrieved from the cache are accurate. But in CI environment compiler's `mtime` can be unique each run.

Effectively, this PR fixes https://github.com/bitcoin/bitcoin/pull/27063#issuecomment-1423901418:
> Looks like ccache isn't working here
>
> https://cirrus-ci.com/task/4571293245243392?logs=ci#L4178

📝 fanquake locked a pull request: "doc: git install command"
(https://github.com/bitcoin/bitcoin/pull/27049)
This is just a documentation error - git does not come pre-installed on macOS.

💬 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