Bitcoin Core Github
44 subscribers
120K links
Download Telegram
💬 maflcko commented on pull request "depends: add new LLVM debug macro":
(https://github.com/bitcoin/bitcoin/pull/29781#issuecomment-2044347304)
I wonder why the CI runtime didn't change. Are the checks so cheap?

Before:

https://cirrus-ci.com/task/6133955961815040?logs=ci#L4016

```
ALL | ✓ Passed | 4916 s (accumulated)
```

This pull:

https://cirrus-ci.com/task/4514150150307840?logs=ci#L6071

```
ALL | ✓ Passed | 4906 s (accumulated)
```

My understanding is that the previous macro does not exist in llvm 18 (wh
...
🚀 fanquake merged a pull request: "test: Update --tmpdir doc string to say directory must not exist"
(https://github.com/bitcoin/bitcoin/pull/29498)
💬 maflcko commented on pull request "Revert "ci: Temporarily disable bpfcc-tools"":
(https://github.com/bitcoin/bitcoin/pull/29832#issuecomment-2044354811)
lgtm ACK c15170c27d96a66422cb86c6653c931aa204bbb0

on green CI
👍 hebasto approved a pull request: "Revert "ci: Temporarily disable bpfcc-tools""
(https://github.com/bitcoin/bitcoin/pull/29832#pullrequestreview-1988360200)
ACK c15170c27d96a66422cb86c6653c931aa204bbb0.
📝 brunoerg opened a pull request: "i2p: fix and improve logs"
(https://github.com/bitcoin/bitcoin/pull/29833)
This PR improves and fixes i2p logs (joint work with @vasild).

- It replaces `LogPrint` to `LogPrintLevel` so we can log according to the severity.
- Fix log when interruption happens during `Accept`. Before this PR, when an interruption happens, it just logs "Error accepting:", no reason is logged as it does for other situations. This PR changes it to log "Accept interrupted".
- Log errors according to the severity. Stuff like creating SAM session, destroying SAM session, etc... are logge
...
👋 fanquake's pull request is ready for review: "[27.x] More backports (and maybe finalize)"
(https://github.com/bitcoin/bitcoin/pull/29780)
🤔 maflcko reviewed a pull request: "tracing: Only prepare tracepoint arguments when actually tracing"
(https://github.com/bitcoin/bitcoin/pull/26593#pullrequestreview-1986597703)
left a nit/question
💬 maflcko commented on pull request "tracing: Only prepare tracepoint arguments when actually tracing":
(https://github.com/bitcoin/bitcoin/pull/26593#discussion_r1555929881)
```suggestion
#define TRACEPOINT(context, event, ...) \
do { \
if (TRACEPOINT_ACTIVE(context, event)) { \
STAP_PROBEV(context, event, __VA_ARGS__); \
```

question: Does the following not work in C++20?
💬 maflcko commented on pull request "validation: Use witness maleation flag for non-segwit blocks":
(https://github.com/bitcoin/bitcoin/pull/29540#issuecomment-2044401032)
-0. Not sure if this is worth it? While the race should have been fixed, are there any benefits that warrant the review?
💬 theuni commented on pull request "build: replace custom `MAC_OSX` macro with standard `__APPLE__` for consistent macOS detection":
(https://github.com/bitcoin/bitcoin/pull/29450#issuecomment-2044405345)
@paplorinc I agree with @fanquake that some of these are macos specific.

Would you be interested in opening a new PR for just the crypto changes? Otherwise I will.
💬 fanquake commented on pull request "Drop Windows Socket dependency for `randomenv.cpp`":
(https://github.com/bitcoin/bitcoin/pull/29786#discussion_r1557183407)
> I guess we should be doing this anyway at some point,

I will try follow up here with something more globally applicable.
👍 fanquake approved a pull request: "Drop Windows Socket dependency for `randomenv.cpp`"
(https://github.com/bitcoin/bitcoin/pull/29786#pullrequestreview-1988414244)
ACK 03b87a3e64305ba651e22a730e35271dea8fea64
🚀 fanquake merged a pull request: "Drop Windows Socket dependency for `randomenv.cpp`"
(https://github.com/bitcoin/bitcoin/pull/29786)
fanquake closed an issue: "ci: Enable bpfcc-tools"
(https://github.com/bitcoin/bitcoin/issues/29804)
🚀 fanquake merged a pull request: "Revert "ci: Temporarily disable bpfcc-tools""
(https://github.com/bitcoin/bitcoin/pull/29832)
🚀 fanquake merged a pull request: "fuzz: Some `test/fuzz/test_runner.py` improvements"
(https://github.com/bitcoin/bitcoin/pull/29821)
💬 maflcko commented on issue "ci: failure in `rpc_scanblocks.py`":
(https://github.com/bitcoin/bitcoin/issues/29831#issuecomment-2044502164)
Is this a race where a validation event is lost?

```
node0 2024-04-08T13:15:11.629578Z [httpworker.0] [validationinterface.cpp:212] [BlockConnected] [validation] Enqueuing BlockConnected: block hash=2194b76c425a2423728356cb8b9c82ad89068bcddfe8dc287c93732fb3354b59 block height=201
...
node0 2024-04-08T13:15:11.629801Z [scheduler] [validationinterface.cpp:212] [operator()] [validation] BlockConnected: block hash=2194b76c425a2423728356cb8b9c82ad89068bcddfe8dc287c93732fb3354b59 block heigh
...
🤔 ajtowns reviewed a pull request: "fees: Remove CLIENT_VERSION serialization"
(https://github.com/bitcoin/bitcoin/pull/29702#pullrequestreview-1988526952)
Concept ACK
💬 ajtowns commented on pull request "fees: Remove CLIENT_VERSION serialization":
(https://github.com/bitcoin/bitcoin/pull/29702#discussion_r1557291850)
`static` ? Move it to the top of the file?

Could consider defining three values:

```
static constexpr int FEE_FILE_MIN_READ_VERSION = 149900;
static constexpr int FEE_FILE_MAX_READ_VERSION = 279900;
static constexpr int FEE_FILE_WRITE_VERSION = 149900;
```
💬 ajtowns commented on pull request "fees: Remove CLIENT_VERSION serialization":
(https://github.com/bitcoin/bitcoin/pull/29702#discussion_r1557296093)
Storing `nVersionThatWrote` seems somewhat useful for manually debugging a corrupt fee data file, and figuring out how the bug occurred. If we're just storing a constant, then the value isn't going to be useful, so it seems better to just write `(int)0` instead -- that's the amount of information it will convey... In any event, "version that wrote the file" is not accurate anymore if we're making it something different from `CLIENT_VERSION`.