Bitcoin Core Github
44 subscribers
121K links
Download Telegram
💬 ajtowns commented on pull request "p2p: Drop m_recently_announced_invs bloom filter":
(https://github.com/bitcoin/bitcoin/pull/27675#discussion_r1267822689)
Dropped
💬 kristapsk commented on pull request "During IBD, prune as much as possible until we get close to where we will eventually keep blocks":
(https://github.com/bitcoin/bitcoin/pull/20827#issuecomment-1641766302)
Concept ACK
📝 MarcoFalke opened a pull request: " lint: Add missing `set -ex` to ci/lint/06_script.sh "
(https://github.com/bitcoin/bitcoin/pull/28103)
Requested in https://github.com/bitcoin/bitcoin/pull/28083#pullrequestreview-1535304219.

Also, one doc commit.
💬 MarcoFalke commented on pull request "ci: Use DOCKER_BUILDKIT for lint image":
(https://github.com/bitcoin/bitcoin/pull/28083#issuecomment-1641769027)
Done in https://github.com/bitcoin/bitcoin/pull/28103
👍 hebasto approved a pull request: "depends: xcb-proto 1.15.2"
(https://github.com/bitcoin/bitcoin/pull/28097#pullrequestreview-1536745294)
ACK 7cb88c8b46723d306b96953a6a60c90a4ab211e3, tested on Ubuntu Mantic with:
```
$ python3 --version
Python 3.12.0b4
```
💬 vasild commented on pull request "net: disconnect inside AttemptToEvictConnection":
(https://github.com/bitcoin/bitcoin/pull/27912#issuecomment-1641806637)
Here is a patch on top of this PR that should address all concerns above about thread safetyness.

<details>
<summary>[patch] thread safe</summary>

```diff
diff --git i/src/net.cpp w/src/net.cpp
index c2160e945f..058f615dd5 100644
--- i/src/net.cpp
+++ w/src/net.cpp
@@ -886,12 +886,14 @@ size_t CConnman::SocketSendData(CNode& node) const
* to forge. In order to partition a node the attacker must be
* simultaneously better at all of them than honest peers.
* If we find
...
💬 MarcoFalke commented on pull request "build: promote -Wunused-member-function to -Wunused":
(https://github.com/bitcoin/bitcoin/pull/28102#issuecomment-1641808324)
For gcc this is already in Wall. Not sure for clang.

Not sure if related: https://cirrus-ci.com/task/6059497324544000?logs=ci#L2338
💬 hebasto commented on pull request "build: promote -Wunused-member-function to -Wunused":
(https://github.com/bitcoin/bitcoin/pull/28102#issuecomment-1641810745)
For clang, [`-Wunused`](https://clang.llvm.org/docs/DiagnosticsReference.html#wunused) does not control `-Wunused-member-function`, no?
💬 hebasto commented on pull request "depends: xcb-proto 1.15.2":
(https://github.com/bitcoin/bitcoin/pull/28097#issuecomment-1641813666)
Guix builds:
```
6d1725c9346bdf04e1eeec2deffda90957bd081700ba896834a143756410fb0e guix-build-7cb88c8b4672/output/aarch64-linux-gnu/SHA256SUMS.part
34486c39daea8a3ce8d92e9c1501f26e5f5e300d9612a25bd56c48ab56353329 guix-build-7cb88c8b4672/output/aarch64-linux-gnu/bitcoin-7cb88c8b4672-aarch64-linux-gnu-debug.tar.gz
cb0080d75d0fc4fc5c7fc022b5771470c60609c35836e7b66c4f479a2edcd098 guix-build-7cb88c8b4672/output/aarch64-linux-gnu/bitcoin-7cb88c8b4672-aarch64-linux-gnu.tar.gz
395c332f83206125819
...
💬 fanquake commented on pull request "Add `UNREACHABLE` macro":
(https://github.com/bitcoin/bitcoin/pull/26504#issuecomment-1641813690)
> I don't care about updating the existing code,

To be clear, I agree, and am still a NACK to any code changes here.
💬 fanquake commented on pull request "I2P: also sleep after errors in Accept() & destroy the session if we get "Session was closed"":
(https://github.com/bitcoin/bitcoin/pull/28077#issuecomment-1641828054)
> I should report this upstream as per https://geti2p.net/en/faq#bug.

Great. Can you link to the issue once it's reported upstream, as it'd be good to know which (if any) of the changes here are required just to work around buggy router software.
🤔 hebasto reviewed a pull request: "validation: use noexcept instead of deprecated throw()"
(https://github.com/bitcoin/bitcoin/pull/28090#pullrequestreview-1536785324)
Post-merge ACK 047daad4f59942488163c6be8516a69291646294.
💬 fanquake commented on pull request "build: promote -Wunused-member-function to -Wunused":
(https://github.com/bitcoin/bitcoin/pull/28102#issuecomment-1641843137)
> Not sure if related: https://cirrus-ci.com/task/6059497324544000?logs=ci#L2338

I thought we'd cleaned these up at some point, can be left as-is in any case.
fanquake closed a pull request: "build: promote -Wunused-member-function to -Wunused"
(https://github.com/bitcoin/bitcoin/pull/28102)
fanquake closed an issue: "libsecp CI failure [no wallet, libbitcoinkernel] [focal]"
(https://github.com/bitcoin/bitcoin/issues/28079)
🚀 fanquake merged a pull request: "subtree: update libsecp256k1 to latest master"
(https://github.com/bitcoin/bitcoin/pull/28093)
💬 hebasto commented on issue "guix: re-enable exported symbol checking for RISC-V":
(https://github.com/bitcoin/bitcoin/issues/28095#issuecomment-1641848538)
Historically, symbol checking for RISC-V was skipped from the [beginning](https://github.com/bitcoin/bitcoin/pull/13724) for the following [reason](https://github.com/bitcoin/bitcoin/pull/13724#issuecomment-413652059):
> Need to skip RISC-V for now, the linker would export so many symbols.
💬 fanquake commented on issue "guix: re-enable exported symbol checking for RISC-V":
(https://github.com/bitcoin/bitcoin/issues/28095#issuecomment-1641850050)
I don't think that's relevant anymore? I've listed the exported symbols above. It's 3.
⚠️ fanquake opened an issue: "win: non-static libssp in libbitcoinconsensus DLL"
(https://github.com/bitcoin/bitcoin/issues/28104)
If you Guix build for Windows, and call objdump on `libbitcoinconsensus-0.dll`, you'll find it uses a non-static libssp:
```bash
objdump -x src/.libs/libbitcoinconsensus-0.dll
...
The Import Tables (interpreted .idata section contents)
vma: Hint Time Forward DLL First
Table Stamp Chain Name Thunk
00263000 00263050 00000000 00000000 0026409c 00263490

DLL Name: libssp-0.dll
vma: Hint/Ord Member-Name Bound-To
2638d0 3 _
...
💬 MarcoFalke commented on pull request "test: move remaining rand code from util/setup_common to util/random":
(https://github.com/bitcoin/bitcoin/pull/27425#issuecomment-1641903050)
My suggestion, for future reference:

```diff
commit bfaa758ca1e690ad6d2ea34b0213f7aedc2645bb
Author: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>
Date: Wed Jul 19 12:41:00 2023 +0200

scripted-diff: Rename global test random context to g_mock_rand

Instead of calling it "insecure", clarify that it is mocked, so that
test failures due to the seed are hopefully more obviously attributed to
it.

-BEGIN VERIFY SCRIPT-
sed -i 's/g_insecure_rand_ctx/g_moc
...