Bitcoin Core Github
43 subscribers
123K links
Download Telegram
📝 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
...
💬 MarcoFalke commented on pull request "ci: Use qemu-user through container engine":
(https://github.com/bitcoin/bitcoin/pull/28087#issuecomment-1668122010)
rebased :smiling_face_with_tear:
💬 MarcoFalke commented on pull request "util: Replace std::filesystem with util/fs.h":
(https://github.com/bitcoin/bitcoin/pull/28076#issuecomment-1668136794)
rebased
🤔 furszy reviewed a pull request: "p2p: adaptive connections services flags"
(https://github.com/bitcoin/bitcoin/pull/28170#pullrequestreview-1565687522)
> I don't think the scenario you describe is likely to happen since we consider ourselves to be in IBD if our tip is older than 24h (which corresponds to ~144 blocks).

Thats not entirely true, misses an important part. We consider in IBD if our tip is older than 24hs (or the custom max tip age) and we haven't completed IBD yet. Once IBD is completed, the node remains out of IBD for the entire software lifecycle.

> > Suppose the node has successfully synced the chain, but later experienced
...
💬 garlonicon commented on pull request "Remove arbitrary restrictions on OP_RETURN by default":
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1668239375)
> Rational: there's lots of ways for people to publish data in the Bitcoin chain, and lots of data has been published.

When it comes to witness data, I can easily prune all of them by downgrading my node to be a non-witness node. How to achieve the same thing for large OP_RETURNs?

> There's no reason for us to place artificial limits on this particular method.

The reason is you will encourage people to downgrade the way they publish data. Witness limit is 4 MB for a reason: it is easy t
...
💬 theStack commented on pull request "bench: add readblock benchmark":
(https://github.com/bitcoin/bitcoin/pull/26684#discussion_r1286143759)
as per developer notes, [assertions shouldn't have side-effects](https://github.com/bitcoin/bitcoin/blob/624333455a5745a7f184d0df531dc348d0ac48dd/doc/developer-notes.md?plain=1#L742), so the following would be preferred:
```suggestion
bool success{chainman.m_blockman.ReadBlockFromDisk(block, pos)};
assert(success);
```
(same for the second test below)
🚀 fanquake merged a pull request: "doc: remove Fedora libdb4-*-devel install docs"
(https://github.com/bitcoin/bitcoin/pull/28231)
💬 RandyMcMillan commented on pull request "refactor: Remove unused includes from wallet.cpp":
(https://github.com/bitcoin/bitcoin/pull/28200#issuecomment-1668341684)
Concept ACK

![Screen Shot 2023-08-07 at 1 49 13 PM](https://github.com/bitcoin/bitcoin/assets/152159/562c3b44-d31b-4dde-9cfa-499ab9bb318d)


macOS Catalina Version 10.15.7 (19H2026)
💬 brunoerg commented on pull request "p2p: Allow whitelisting outgoing connections":
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1286218964)
Yea, I'm going to change it