💬 m3dwards commented on issue "IBD performance regression in 27.0rc1 on Windows":
(https://github.com/bitcoin/bitcoin/issues/29785#issuecomment-2034615159)
I can try and recreate this. @vostrnad do you have a script that records / plots the graphs?

⚠️ maflcko opened an issue: "fuzz: minisketch: Undefined-shift in std::__1::vector<Field<unsigned int, 32, 141u, RecLinTrans<unsigned int, 6, 6, 5"
(https://github.com/bitcoin/bitcoin/issues/29799)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

Undefined-shift

### Expected behaviour

no Undefined-shift

### Steps to reproduce

* Compile fuzz targets with `./configure CC=clang CXX=clang++ --enable-fuzz --with-sanitizers=fuzzer,undefined`
* Create crash input: `echo 'Av////////////8gICD///8gIP8g/yAg/yA=' | base64 --decode > /tmp/crash.bin`
* Run Fuzz target: `FUZZ=minisketch ./src/test/fuzz/fuzz /tmp/crash.bin`

### Relevant log
...

💬 maflcko commented on issue "fuzz: minisketch: Undefined-shift in std::__1::vector<Field<unsigned int, 32, 141u, RecLinTrans<unsigned int, 6, 6, 5":
(https://github.com/bitcoin/bitcoin/issues/29799#issuecomment-2034781086)
Ref https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=67799

💬 Sjors commented on pull request "rpc: Optimize serialization disk space of dumptxoutset - Reloaded":
(https://github.com/bitcoin/bitcoin/pull/29612#issuecomment-2034809713)
@mzumsande I don't know, but it seems such applications should use a more portable format like SQL, see #27432

💬 Sjors commented on pull request "contrib: add tool to convert compact-serialized UTXO set to SQLite database":
(https://github.com/bitcoin/bitcoin/pull/27432#issuecomment-2034831892)
Concept ACK

💬 Sjors commented on pull request "contrib: add tool to convert compact-serialized UTXO set to SQLite database":
(https://github.com/bitcoin/bitcoin/pull/27432#discussion_r1549905845)
TIL we compress certain standard `scriptPubKey` types.

💬 sipa commented on issue "fuzz: minisketch: Undefined-shift in std::__1::vector<Field<unsigned int, 32, 141u, RecLinTrans<unsigned int, 6, 6, 5":
(https://github.com/bitcoin/bitcoin/issues/29799#issuecomment-2034842852)
See https://github.com/sipa/minisketch/pull/81 for a fix.

📝 hebasto opened a pull request: "ci: Drop duplicated compiler flags"
(https://github.com/bitcoin/bitcoin/pull/29800)
On the master branch @ 0d509bab45d292caeaf34600e57b5928757c6005, it is easy to check the _"Options used to compile and link"_ section in the `configure` script output and observe duplicated compiler flags.

This PR cleans such cases up.

💬 maflcko commented on pull request "ci: Drop duplicated compiler flags":
(https://github.com/bitcoin/bitcoin/pull/29800#issuecomment-2034899685)
lgtm ACK be97652c07a10399c08ecb98dbbaeb33b84af774

The depends ones are not needed, because depends already passes the flags through. The `-g` isn't needed, because it is always set by default.

💬 m3dwards commented on issue "IBD performance regression in 27.0rc1 on Windows":
(https://github.com/bitcoin/bitcoin/issues/29785#issuecomment-2034922309)
Running IBD in a loop, will report back tomorrow.

🤔 vasild reviewed a pull request: "Simplify network-adjusted time warning logic"
(https://github.com/bitcoin/bitcoin/pull/29623#pullrequestreview-1976691061)
Approach ACK 2ef71c73582be554e565ada3f8a6ca77c2cd79f1

Wrt adjusting the peer's offset with local clock corrections in order to stop the warnings immediately after the local clock is fixed (instead of after 4-5h) https://github.com/bitcoin/bitcoin/pull/29623#discussion_r1544783265, consider this:

<details>
<summary>[patch] adjust peer time with local clock corrections (untested)</summary>

```diff
diff --git i/src/node/timeoffsets.cpp w/src/node/timeoffsets.cpp
index 8488e47ff5..4dce6f
...

💬 vasild commented on pull request "Simplify network-adjusted time warning logic":
(https://github.com/bitcoin/bitcoin/pull/29623#discussion_r1549713660)
This implies that `int64_t` is the same as the internal type of `std::chrono::seconds`. Better get a random integer in the range of `[std::chrono::seconds::min().count(), std::chrono::seconds::max().count()]`.

💬 vasild commented on pull request "Simplify network-adjusted time warning logic":
(https://github.com/bitcoin/bitcoin/pull/29623#discussion_r1549743266)
```suggestion
static bool IsWarningRaised(std::vector<std::chrono::seconds> check_offsets)
```

💬 vasild commented on pull request "Simplify network-adjusted time warning logic":
(https://github.com/bitcoin/bitcoin/pull/29623#discussion_r1549674844)
Same as above:

```suggestion
static constexpr std::chrono::minutes WARNING_WAIT{60};
```

💬 vasild commented on pull request "Simplify network-adjusted time warning logic":
(https://github.com/bitcoin/bitcoin/pull/29623#discussion_r1549666557)
Since this is a constant, like `N`:

```suggestion
static constexpr std::chrono::minutes WARN_THRESHOLD{10};
```

💬 vasild commented on pull request "Simplify network-adjusted time warning logic":
(https://github.com/bitcoin/bitcoin/pull/29623#discussion_r1549730951)
In this case performance is irrelevant but in general `in` arguments that are expensive to copy like `vector` are passed by const reference.

💬 emc99 commented on pull request "[RFC] Switch and/or align debugging flags (back) to `-Og`":
(https://github.com/bitcoin/bitcoin/pull/29796#issuecomment-2034942067)
Concept ACK

💬 vasild commented on pull request "Logging cleanup":
(https://github.com/bitcoin/bitcoin/pull/29798#discussion_r1549991365)
Calling this with `NONE` does not make sense logically and no caller does it currently. Thus I removed this. But you are right that a (nonsensical?) caller could be added in the future. Would you prefer an assert like `assert(category != LogFlags::NONE);` here or to restore this line as it is (was)?

💬 fanquake commented on pull request "ci: Drop duplicated compiler flags":
(https://github.com/bitcoin/bitcoin/pull/29800#discussion_r1549996173)
I guess going forward, we'll have to remember to never add any logic/other flags to the `--with-sanitizers/cmake equivalent` option, that differ from just setting `-fsanitize=x`? Otherwise they could possibly get missed in CI.

Removing this also now skips the link check(s) and I don't see where flags are getting added to LDFLAGS elsewhere? So this is at least a change in behaviour and I'd say somewhat less clear.

💬 vasild commented on pull request "Logging cleanup":
(https://github.com/bitcoin/bitcoin/pull/29798#issuecomment-2034968337)
> See #27231.

There you mention:
> I'm going to open an alternate pull that does what you suggest...

Is the current PR doing what you meant? E.g. drop that help text and return an error for "0" ("none" already returns an error in `master`)?