💬 dhruv commented on pull request "BIP324: Enable v2 P2P encrypted transport":
(https://github.com/bitcoin/bitcoin/pull/24545#issuecomment-1462232617)
@vostrnad this is expected because the breaking change is in the p2p layer and not the transport. So a v2 connection is successfully established, and likely the inbound peer (not updated) never sees something that it interprets as the VERSION message. It should have timed out/failed eventually though. Did it do that? This isn't currently considered a downgrade scenario because a v2 transport connection was established. If we downgrade to v1 on p2p unresponsiveness, my thinking is that would lead
...

💬 jamesob commented on pull request "assumeutxo: net_processing changes":
(https://github.com/bitcoin/bitcoin/pull/24008#issuecomment-1462344633)
> Also, as far as I understand it, AssumeUTXO will mess up the order of the blocks stored on disk - the first blockfiles would have the blocks between the AssumeUTXO block and the tip, and the subsequent blockfiles would have the blocks between Genesis and the AssumeUTXO block. I wonder if that could break or slow down something, have you tried a reindex for example?

This is certainly worth addressing. My inclination is to say that given there are only 6925 separate block files on my mainnet
...

👍 kristapsk approved a pull request: "Use string interpolation for default value of -listen"
(https://github.com/bitcoin/bitcoin/pull/27232)
cr utACK 5c938e74cfdf6b89c6c4d5cce2fd07cbfc8b29c2

💬 kristapsk commented on pull request "p2p: set `-dnsseed` and `-listen` false if `maxconnections=0`":
(https://github.com/bitcoin/bitcoin/pull/26899#issuecomment-1462350149)
Concept ACK

💬 achow101 commented on issue "Permission to comment on closed PRs":
(https://github.com/bitcoin/bitcoin/issues/27234#issuecomment-1462349727)
> We can look at if there is some permissions change we can make.

IIRC it requires the "write" permission, which only maintainers have.

💬 achow101 commented on pull request "github: Switch to yaml issue templates":
(https://github.com/bitcoin/bitcoin/pull/27025#issuecomment-1462378021)
ACK 3fa1185dda3b000b9c3956422fd2351e40969dec

💬 dergoegge commented on pull request "tracing: network connection tracepoints":
(https://github.com/bitcoin/bitcoin/pull/25832#issuecomment-1462378467)
Concept ACK

💬 ryanofsky commented on pull request "refactor: Remove CAddressBookData::destdata":
(https://github.com/bitcoin/bitcoin/pull/18608#issuecomment-1462387666)
#27224 is the rebased version of this PR. (I would have reopened this one but ran into github permission issues)

💬 ryanofsky commented on pull request "Blockstorage: Dont access gArgs to get blocks_dir":
(https://github.com/bitcoin/bitcoin/pull/27103#issuecomment-1462391839)
@TheCharlatan

Curious why this PR was closed, and what is its relationship to #27125? Is that a PR a replacement or superset or a different approach?

💬 ryanofsky commented on issue "Permission to comment on closed PRs":
(https://github.com/bitcoin/bitcoin/issues/27234#issuecomment-1462409614)
Thanks for unlocking the PRs.

That is a good point that ability to comment on a locked PR is not really useful because relevant people won't be able to respond. Maybe ideally there would be a way to request that DrahtBot unlocks the PR, and then subsequently relocks it after a period of idle time (like a week or month).

Meantime, I guess I can use this issue to request things to be unlocked if the need arises. (At least until this issue itself is locked :grin:)

✅ ryanofsky closed an issue: "Permission to comment on closed PRs"
(https://github.com/bitcoin/bitcoin/issues/27234)

💬 TheCharlatan commented on pull request "Blockstorage: Dont access gArgs to get blocks_dir":
(https://github.com/bitcoin/bitcoin/pull/27103#issuecomment-1462411439)
> Curious why this PR was closed, and what is its relationship to https://github.com/bitcoin/bitcoin/pull/27125? Is that a PR a replacement or superset or a different approach?

I hit the wrong button by mistake :) - I should have left a comment, or better re-opened as a draft. The approach started here is the same though as in #27125.

🚀 glozow merged a pull request: "github: Switch to yaml issue templates"
(https://github.com/bitcoin/bitcoin/pull/27025)

💬 Sjors commented on pull request "Add test and docs for getblockfrompeer with pruning":
(https://github.com/bitcoin/bitcoin/pull/23813#issuecomment-1462438231)
re-utACK fe329dc936d1e02da406345e4223e11d1fa6fb38

💬 Sjors commented on pull request "assumeutxo":
(https://github.com/bitcoin/bitcoin/pull/15606#issuecomment-1462444460)
CI trips over:

```
src/validation.cpp:5721:1:
error:
return type 'const ChainstateRole' is 'const'-qualified at the top level, which may reduce code readability without improving const correctness [readability-const-return-type,-warnings-as-errors]
```

💬 TheCharlatan commented on pull request "refactor, kernel: Decouple ArgsManager from blockstorage":
(https://github.com/bitcoin/bitcoin/pull/27125#issuecomment-1462445136)
Updated f87c39399823e22c553b797cc66fa4063462a32b -> 6d9826f182d9122d4464d35d6682dc6fb4b1116e ([removeBlockstorageArgs_6](https://github.com/TheCharlatan/bitcoin/tree/removeBlockstorageArgs_6) -> [removeBlockstorageArgs_7](https://github.com/TheCharlatan/bitcoin/tree/removeBlockstorageArgs_7), [compare](https://github.com/TheCharlatan/bitcoin/compare/removeBlockstorageArgs_6..removeBlockstorageArgs_7)) addressing https://github.com/bitcoin/bitcoin/pull/27125#pullrequestreview-1317942691 by insta
...

📝 dergoegge opened a pull request: "Avoid integer overflow in CheckDiskSpace"
(https://github.com/bitcoin/bitcoin/pull/27235)
Starting a fresh node with `-prune=1` causes an integer overflow to happen in `CheckDiskSpace` (e.g. https://cirrus-ci.com/task/5059018708746240) because `nPruneTarget` is to the max `uint64_t` value.
https://github.com/bitcoin/bitcoin/blob/f7bdcfc83f5753349018be3b5a663c8923d1a5eb/src/init.cpp#L1633-L1648

I think side stepping the overflow for this specific case, is better than adding an exception to the UB suppresions file. Alternatively we could use our overflow utils (e.g. `CheckedAdd`).

💬 martinus commented on pull request "Avoid integer overflow in CheckDiskSpace":
(https://github.com/bitcoin/bitcoin/pull/27235#issuecomment-1462486422)
How about setting `nPruneTarget` to something smaller, like `std::numeric_limits<uint64_t>::max() / 2`? That's still large enough

💬 vasild commented on pull request "Avoid integer overflow in CheckDiskSpace":
(https://github.com/bitcoin/bitcoin/pull/27235#discussion_r1131394690)
A few issues:

1. This check uses the knowledge that the special magic constant `std::numeric_limits<uint64_t>::max()` is returned by `GetPruneTarget()` and that in such cases `additional_bytes` does not make sense. That's not for `CheckDiskSpace()` to know - the caller should choose a better `additional_bytes` when calling `CheckDiskSpace()`.

2. This magic constant is used in two other places - when setting `nPruneTarget` and in `LoadChainstate()`. It is better to give it a name instead of
...

💬 1440000bytes commented on pull request "p2p, validation: Don't download witnesses for assumed-valid blocks when running in prune mode":
(https://github.com/bitcoin/bitcoin/pull/27050#issuecomment-1462547782)
> It's important to recognize that this is a security reduction, even though it only applies to assumed-valid blocks: one of the reasons why assume-valid is acceptable, is because the data that we're assuming is valid is widely available for independent auditing and our peers can't distinguish between nodes that are and are not actually checking it. Skipping the downloading of witnesses negates that security by making it possible to sync a pruned node without anyone having that data available.

...