Bitcoin Core Github
44 subscribers
122K links
Download Telegram
📝 MarcoFalke opened a pull request: "ci: Use qemu-user through container engine"
(https://github.com/bitcoin/bitcoin/pull/28087)
Currently the CI containers always run on the host architecture, and only wrap `bitcoind` into `qemu-user` when needed. This has many issues:

* The `i386` tasks can not be run on non-x86 hosts.
* `config.guess` isn't present when building the CI image, which is fine. But it prints a warning, see https://github.com/bitcoin/bitcoin/pull/27739#pullrequestreview-1446580353
* The python tests are run on the host architecture, making it harder to find architecture specific bugs. See for example h
...
💬 naumenkogs commented on pull request "p2p: Drop m_recently_announced_invs bloom filter":
(https://github.com/bitcoin/bitcoin/pull/27675#discussion_r1265149164)
fec00d787b5530bbe892bef575fbb3cdc9d586ca

nit: you may drop the `args.` part and it will work because `bypass_limits` var is initialized above.
👍 dergoegge approved a pull request: "fuzz: Flatten all FUZZ_TARGET macros into one"
(https://github.com/bitcoin/bitcoin/pull/28065#pullrequestreview-1532456010)
ACK fa6dfaaf45bde465969fa7d8fa6b6574a497a6ca
💬 naumenkogs commented on pull request "p2p: Drop m_recently_announced_invs bloom filter":
(https://github.com/bitcoin/bitcoin/pull/27675#discussion_r1265162383)
34eab37b08bb9f23459aad179685a505a46759d7
Seems to be still talking about `current_time`?
💬 naumenkogs commented on pull request "p2p: Drop m_recently_announced_invs bloom filter":
(https://github.com/bitcoin/bitcoin/pull/27675#discussion_r1265171521)
This is done in the following commit [c88deec](https://github.com/bitcoin/bitcoin/pull/27675/commits/c88deec99d53f55ff76fedaa30c318be6b72d085)
💬 stickies-v commented on pull request "rpc: doc: Added `longpollid` and `data` params to `template_request` #27998":
(https://github.com/bitcoin/bitcoin/pull/28056#issuecomment-1637867309)
> But the use of the passed parameter for `longpollid` at
> https://github.com/bitcoin/bitcoin/blob/57b8336dfed6312003cf34cd5ae7099f77115e73/src/rpc/mining.cpp#L715-L721
>
> is the same for both a template and a proposal.

`longpollid` is neither used as a parameter nor returned as a result for `mode==proposal`. The line you reference is never reached in that case, as we [return or throw within this code block](https://github.com/bitcoin/bitcoin/blob/57b8336dfed6312003cf34cd5ae7099f77115e7
...
💬 russeree commented on pull request "rpc: doc: Added `longpollid` and `data` params to `template_request` #27998":
(https://github.com/bitcoin/bitcoin/pull/28056#issuecomment-1637880277)
> >
> > is the same for both a template and a proposal.
>
> `longpollid` is neither used as a parameter nor returned as a result for `mode==proposal`. The line you reference is never reached in that case, as we [return or throw within this code block](https://github.com/bitcoin/bitcoin/blob/57b8336dfed6312003cf34cd5ae7099f77115e73/src/rpc/mining.cpp#L660-L687). See also [the corresponding results](https://github.com/bitcoin/bitcoin/blob/57b8336dfed6312003cf34cd5ae7099f77115e73/src/rpc/minin
...
💬 naumenkogs commented on pull request "p2p: Drop m_recently_announced_invs bloom filter":
(https://github.com/bitcoin/bitcoin/pull/27675#issuecomment-1637899668)
ACK 0753120b6cb270914753d30a2626272f896a8545 modulo [this (probably lost?) comment](https://github.com/bitcoin/bitcoin/pull/27675#discussion_r1265162383)

My favorite achievement of this PR is dropping the complication of associated logic :)
💬 dergoegge commented on pull request "[PoC] Structure aware fuzzing with libprotobuf-mutator":
(https://github.com/bitcoin/bitcoin/pull/26975#issuecomment-1637905257)
> Did you find that this was useless in a benchmark, or did you just close for fun?

Just wasn't my top priority but I plan on revisiting this. I've also been working on a library that does the same as libprotobuf-mutator but for Cap'n Proto, which might be more appropriate if we go the multi-process route (could also be used to fuzz the multi-process interfaces).
💬 MarcoFalke commented on issue "libsecp CI failure [no wallet, libbitcoinkernel] [focal]":
(https://github.com/bitcoin/bitcoin/issues/28079#issuecomment-1637929740)
Depending on how often this happens, we could also bump the subtree here after merge?
💬 MarcoFalke commented on pull request "ci: Use qemu-user through container engine":
(https://github.com/bitcoin/bitcoin/pull/28087#issuecomment-1637960373)
If it doesn't work locally, you can check the enabled architectures via `ls /proc/sys/fs/binfmt_misc/`
💬 MarcoFalke commented on issue "Intermittent failures in interface_usdt_mempool.py":
(https://github.com/bitcoin/bitcoin/issues/27380#issuecomment-1637971779)
Ok, I'll disable the broken test due to lack of progress here.
📝 MarcoFalke opened a pull request: "test: Disable known broken USDT test"
(https://github.com/bitcoin/bitcoin/pull/28088)
The failure is known and running into more failures doesn't help anyone. Not disabling the test would be a waste of CPU and developer time.

https://github.com/bitcoin/bitcoin/issues/27380
💬 MarcoFalke commented on issue "Spurious (?) valgrind failure for p2p_compactblocks.py":
(https://github.com/bitcoin/bitcoin/issues/27741#issuecomment-1637988681)
Tracked it down to `CXXFLAGS="-O1 -g -fipa-sra "` so far.
👍 dergoegge approved a pull request: "fuzz: Bump FuzzedDataProvider.h"
(https://github.com/bitcoin/bitcoin/pull/28086#pullrequestreview-1532614758)
utACK fa367422efa3c00f27dab2b58f2080303ed18b91
💬 Crypt-iQ commented on pull request "http: add evhttp_connection_set_closecb to avoid g_requests hang":
(https://github.com/bitcoin/bitcoin/pull/27909#issuecomment-1637992727)
I modified the diff above and printed out the `evhttp_request` memory address as well as the `output_buffer` memory address. I noticed that the`evhttp_request` address is re-used when there's a crash and the `output_buffer` address is not consistent between callbacks.

<details>
<summary>diff</summary>
<br>

```
diff --git a/src/httpserver.cpp b/src/httpserver.cpp
index 849e9b482..9fefad6bd 100644
--- a/src/httpserver.cpp
+++ b/src/httpserver.cpp
@@ -211,6 +211,7 @@ std::string Reques
...
💬 MarcoFalke commented on issue "Spurious (?) valgrind failure for p2p_compactblocks.py":
(https://github.com/bitcoin/bitcoin/issues/27741#issuecomment-1637998300)
This can be fixed, like most other C++ bugs can be fixed: "Add or remove `*` or `&` in random places, until it works."


Diff:

```diff
while (range.first != range.second) {
- auto [node_id, list_it] = range.first->second;
+ auto& [node_id, list_it] = range.first->second;

```

Or, with added `const`:

```diff
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index 8da2c70..f26522b 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp

...
💬 vasild commented on pull request "net: support unix domain sockets for -proxy and -onion":
(https://github.com/bitcoin/bitcoin/pull/27375#discussion_r1265260663)
> Transport endpoint is not connected

Is this still a problem or it got resolved?
💬 fanquake commented on pull request "fuzz: Bump FuzzedDataProvider.h":
(https://github.com/bitcoin/bitcoin/pull/28086#issuecomment-1638004857)
Pulls in the latest changes from upstream: https://github.com/llvm/llvm-project/blob/main/compiler-rt/include/fuzzer/FuzzedDataProvider.h.
🚀 fanquake merged a pull request: "fuzz: Bump FuzzedDataProvider.h"
(https://github.com/bitcoin/bitcoin/pull/28086)