Bitcoin Core Github
43 subscribers
123K links
Download Telegram
💬 MarcoFalke commented on pull request "net processing: clamp PeerManager::Options user input":
(https://github.com/bitcoin/bitcoin/pull/28149#discussion_r1286001205)
```suggestion
options.max_orphan_txs = uint32_t(std::clamp<int64_t>(*value, 0, std::numeric_limits<uint32_t>::max()));
```

nit, if you re-touch?
💬 andrewtoth commented on pull request "net: don't lock cs_main while reading blocks in net processing":
(https://github.com/bitcoin/bitcoin/pull/26326#issuecomment-1668030656)
@furszy thank you for the review. Sorry I forgot to address your nits on this rebase. Will do them if I have to rebase again.
👋 fanquake's pull request is ready for review: "ci: Integrate `bitcoin-tidy` clang-tidy plugin"
(https://github.com/bitcoin/bitcoin/pull/26296)
💬 fanquake commented on pull request "ci: Integrate `bitcoin-tidy` clang-tidy plugin":
(https://github.com/bitcoin/bitcoin/pull/26296#issuecomment-1668052979)
> is anything left to be done here

I missed that Cory had already ACK'd, and was waiting for him to re-ACK. Given that's already happened, I don't think there's anything left here, and docs can be fixed in the followup.
💬 glozow commented on pull request "mempool: Persist with XOR":
(https://github.com/bitcoin/bitcoin/pull/28207#issuecomment-1668053761)
concept ACK
💬 MarcoFalke commented on pull request "doc: remove Fedora libdb4-*-devel install docs":
(https://github.com/bitcoin/bitcoin/pull/28231#issuecomment-1668066449)
lgtm ACK 11a499eb4d57f2c5eabd7955bcc7e419364b3194
🚀 fanquake merged a pull request: "ci: Integrate `bitcoin-tidy` clang-tidy plugin"
(https://github.com/bitcoin/bitcoin/pull/26296)
💬 TheCharlatan commented on pull request "refactor: Remove unused includes from wallet.cpp":
(https://github.com/bitcoin/bitcoin/pull/28200#issuecomment-1668073018)
Concept ACK

What about this one:
```
wallet/wallet.h should remove these lines:
- #include <boost/signals2/signal.hpp> // lines 52-52
```
Probably should be moved to the implementation file, no?
📝 ChrisCho-H opened a pull request: "fix: bad opcode err msg includes reserved opcode"
(https://github.com/bitcoin/bitcoin/pull/28234)
bad opcode err msg includes `reserved`, because reserved opcode is evaluated as BAD_OPCODE(and it's wrong to state `missing or not undestood` for it).
👍 MarcoFalke approved a pull request: "kernel: Prune leveldb headers"
(https://github.com/bitcoin/bitcoin/pull/28186#pullrequestreview-1565583066)
ACK d8f1222ac50f089a0af29eaf8ce0555bad8366ef 🔠

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: ACK d8f1222ac50f089a0af29eaf8
...
💬 MarcoFalke commented on pull request "kernel: Prune leveldb headers":
(https://github.com/bitcoin/bitcoin/pull/28186#discussion_r1286038136)
c95b37d641b1eed4a62d55ca5342a6ed8c7a1ce7: same
💬 MarcoFalke commented on pull request "kernel: Prune leveldb headers":
(https://github.com/bitcoin/bitcoin/pull/28186#discussion_r1286025216)
style nit in ea8135de7e617259cda3fc7b1c8e7569d454fd57: May be better to add a newline before `:` to reduce the leading whitespace bloating block.
💬 MarcoFalke commented on pull request "refactor: Remove unused includes from wallet.cpp":
(https://github.com/bitcoin/bitcoin/pull/28200#issuecomment-1668087768)
Can you post a patch that compiles?
💬 jonatack commented on pull request "fix: bad opcode err msg includes reserved opcode":
(https://github.com/bitcoin/bitcoin/pull/28234#issuecomment-1668107467)
If this change is correct, the failing tests would need to be updated in the same commit (see https://jonatack.github.io/articles/how-to-contribute-pull-requests-to-bitcoin-core for more details).
💬 RandyMcMillan commented on pull request "mempool: Persist with XOR":
(https://github.com/bitcoin/bitcoin/pull/28207#issuecomment-1668112752)
Concept ACK
👍 MarcoFalke approved a pull request: "assumeutxo cleanup: Move IsInitialBlockDownload & NotifyHeaderTip to ChainstateManager"
(https://github.com/bitcoin/bitcoin/pull/28218#pullrequestreview-1565641144)
lgtm. Left style nits
💬 MarcoFalke commented on pull request "assumeutxo cleanup: Move IsInitialBlockDownload & NotifyHeaderTip to ChainstateManager":
(https://github.com/bitcoin/bitcoin/pull/28218#discussion_r1286059909)
Same?
💬 MarcoFalke commented on pull request "assumeutxo cleanup: Move IsInitialBlockDownload & NotifyHeaderTip to ChainstateManager":
(https://github.com/bitcoin/bitcoin/pull/28218#discussion_r1286059126)
nit: Missing clang-format on touched line?
💬 MarcoFalke commented on pull request "assumeutxo cleanup: Move IsInitialBlockDownload & NotifyHeaderTip to ChainstateManager":
(https://github.com/bitcoin/bitcoin/pull/28218#discussion_r1286059790)
nit: Add missing `{}` on touched line for multiline if?
💬 1ma commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1668117041)
Thank you for your input @samouraiwallet.

I admit that when I gave my ACK I thought this change was a no-brainer (for the reasons @mikeinspace exposed above) and now I'm conflicted. I wasn't aware of BIP47 v3, and in my personal opinion it is a valid use case. I've reviewed the extent of the work you and others have done on it in the last years and I've realized how extremely pissed you and the more privacy-focused Bitcoin community must be about this proposed change.

However, I have to sa
...