Bitcoin Core Github
44 subscribers
121K links
Download Telegram
💬 MarcoFalke commented on pull request "ci: Integrate `bitcoin-tidy` clang-tidy plugin":
(https://github.com/bitcoin/bitcoin/pull/26296#discussion_r1276421094)
I guess the build is fast and not needed to be cached for now?
💬 MarcoFalke commented on pull request "ci: Integrate `bitcoin-tidy` clang-tidy plugin":
(https://github.com/bitcoin/bitcoin/pull/26296#discussion_r1276415678)
nit:
```suggestion
make -j$(nproc) -C build
```
💬 fanquake commented on pull request "ci: Integrate `bitcoin-tidy` clang-tidy plugin":
(https://github.com/bitcoin/bitcoin/pull/26296#discussion_r1276437800)
Added.
💬 fanquake commented on pull request "ci: Integrate `bitcoin-tidy` clang-tidy plugin":
(https://github.com/bitcoin/bitcoin/pull/26296#discussion_r1276438436)
clang-formatted everything using `src/.clang-format`.
💬 fanquake commented on pull request "ci: Integrate `bitcoin-tidy` clang-tidy plugin":
(https://github.com/bitcoin/bitcoin/pull/26296#discussion_r1276439295)
Yea, it's only two files, so very fast, especially compared to the runtime of clang-tidy itself.
💬 stickies-v commented on pull request "test doc: tests `acceptstalefeeestimates` option is only supported on regtest chain":
(https://github.com/bitcoin/bitcoin/pull/28157#discussion_r1276442786)
Can be made quite a bit more DRY:
```suggestion
self.log.info(f"Test -acceptstalefeeestimates option is not supported on non-regtest chains")
for chain in ["main", "test", "signet"]:
with open(inc_conf_file_path, 'w', encoding='utf-8') as conf:
conf.write(f'chain={chain}\n')
conf.write('acceptstalefeeestimates=1\n')
self.nodes[0].assert_start_raises_init_error(extra_args=[f"-conf={inc_conf_file_path}", "-allowignoredc
...
💬 stickies-v commented on pull request "test doc: tests `acceptstalefeeestimates` option is only supported on regtest chain":
(https://github.com/bitcoin/bitcoin/pull/28157#discussion_r1276443940)
Would generally avoid unrelated style nits, you're not touching this line (or even file, for that matter) for anything else, so best to just leave as is.
💬 achow101 commented on pull request "refactor: consistently use ApplyArgsManOptions for PeerManager::Options":
(https://github.com/bitcoin/bitcoin/pull/28148#issuecomment-1653841139)
ACK 8a3159728ae84cb8093e2e9fa5d2c2b0a7d545da
fanquake closed a pull request: "Drop macOS ForceActivation workaround"
(https://github.com/bitcoin-core/gui/pull/744)
🚀 achow101 merged a pull request: "refactor: consistently use ApplyArgsManOptions for PeerManager::Options"
(https://github.com/bitcoin/bitcoin/pull/28148)
📝 furszy opened a pull request: "net: adaptive connections services flags"
(https://github.com/bitcoin/bitcoin/pull/28170)
Derived from #28120 discussion.

By relocating the peer desirable services flags into the peer manager, we
allow the connections acceptance process to handle post-IBD potential
stalling scenarios.

The peer manager will be able to dynamically adjust the services flags
based on the node's proximity to the tip (back and forth). Allowing the node
to recover from the following post-IBD scenario:
Suppose the node has successfully synced the chain, but later experienced
dropped connections a
...
💬 MarcoFalke commented on pull request "ci: Integrate `bitcoin-tidy` clang-tidy plugin":
(https://github.com/bitcoin/bitcoin/pull/26296#discussion_r1276473986)
Just for reference, this isn't actually safe, since Bitcoin Core has multiple thread, which may log and interleave at will. But that is a pre-existing bug, and I presume this check just accommodates this, which is fine.
👍 MarcoFalke approved a pull request: "ci: Integrate `bitcoin-tidy` clang-tidy plugin"
(https://github.com/bitcoin/bitcoin/pull/26296#pullrequestreview-1550164036)
lgtm ACK
📝 pinheadmz opened a pull request: "test: blockstore test with chattr instead of chmod"
(https://github.com/bitcoin/bitcoin/pull/28171)
alternative to https://github.com/bitcoin/bitcoin/pull/27850

see https://github.com/bitcoin/bitcoin/pull/27850#issuecomment-1650601548

just testing ci for now
👍 jamesob approved a pull request: "BIP324 ciphersuite"
(https://github.com/bitcoin/bitcoin/pull/28008#pullrequestreview-1550293430)
reACK 1c7582e

Based on the interdiff (https://github.com/jamesob/bitcoin-review-data/blob/master/28008.sipa.bip324_ciphersuite/4.1c7582e/interdiff.3.180909f.diff), which contains the few minor changes @sipa has mentioned above (`const` declarations, doc adds, small refactors).

> by extending my own hacky Python implementation using PyCryptodome (for ChaCha20, ChaCha20Poly1305, SHA256 and HKDF with HMAC_SHA256) + ellswift from our test framework for (see updated gist https://gist.github.com
...
🤔 MarcoFalke reviewed a pull request: "test, rpc: invalid sighashtype coverage"
(https://github.com/bitcoin/bitcoin/pull/28166#pullrequestreview-1550306892)
lgtm ACK 90c8f79e945863f3818748b86572948d1558aec3 🎥

<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: lgtm ACK 90c8f79e945863f3
...
💬 instagibbs commented on pull request "Replace MIN_STANDARD_TX_NONWITNESS_SIZE to preclude 64 non-witness bytes only":
(https://github.com/bitcoin/bitcoin/pull/26398#issuecomment-1653987566)
https://github.com/lightning/bolts/pull/1096#discussion_r1276546764

Just noting that some LN spec work is touching these limits
💬 hebasto commented on pull request "ci: Use docker image cache for "Win64 native [vs2022]" task":
(https://github.com/bitcoin/bitcoin/pull/27771#issuecomment-1653998189)
Closing in the light of https://github.com/bitcoin/bitcoin/issues/28098.
hebasto closed a pull request: "ci: Use docker image cache for "Win64 native [vs2022]" task"
(https://github.com/bitcoin/bitcoin/pull/27771)
💬 dunxen commented on pull request "contrib: add tool to convert compact-serialized UTXO set to SQLite database":
(https://github.com/bitcoin/bitcoin/pull/27432#issuecomment-1654038512)
Concept ACK