💬 furszy commented on pull request "refactor: interfaces, make 'createTransaction' less error-prone ":
(https://github.com/bitcoin-core/gui/pull/807#discussion_r1549695878)
`CreatedTransactionResult` is an existing struct. The suggestion wouldn't compile.
We could rename the struct if that is generating confusion.

🤔 tdb3 reviewed a pull request: "test: Bump timeouts in feature_index_prune and wallet_importdescriptors"
(https://github.com/bitcoin/bitcoin/pull/29791#pullrequestreview-1976750902)
Nice work @cbergqvist. The code changes lgtm.

It may be worth investigating why these tests are taking longer to run, and if that is expected or a potential performance regression that went unnoticed until now. I'm assuming that the tests were passing at some point with the existing timeout values (and that those values were chosen for a reason).

I don't have all the history on this, so I could be missing something. Perhaps it's dependent on how the tests are run (e.g. `--jobs` chosen), and if
...

🤔 maflcko reviewed a pull request: "Simplify network-adjusted time warning logic"
(https://github.com/bitcoin/bitcoin/pull/29623#pullrequestreview-1976762473)
lgtm 2ef71c73582be554e565ada3f8a6ca77c2cd79f1 🛴

<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 2ef71c73582be554e565ad
...

💬 maflcko commented on pull request "Simplify network-adjusted time warning logic":
(https://github.com/bitcoin/bitcoin/pull/29623#discussion_r1549717587)
```suggestion
"this may lead to consensus failure. After you've confirmed your computer's clock, this message "
```

nit: The clock may or may not already be correct, so there may be nothing to correct, only to confirm.

💬 maflcko commented on pull request "Simplify network-adjusted time warning logic":
(https://github.com/bitcoin/bitcoin/pull/29623#discussion_r1549711736)
I don't think the use of "GUI" is correct here, is it?

The GUI only calls `getWarnings()`, aka `GetWarnings(true)`, aka `warnings_concise`, which is never set in this pull request.

Currently it is only set for RPC and the "No-UI" interface.

💬 maflcko commented on pull request "Simplify network-adjusted time warning logic":
(https://github.com/bitcoin/bitcoin/pull/29623#issuecomment-2034583200)
Reminder for myself: https://github.com/bitcoin/bitcoin/pull/29623#discussion_r1535417573

💬 maflcko commented on pull request "Simplify network-adjusted time warning logic":
(https://github.com/bitcoin/bitcoin/pull/29623#discussion_r1549731528)
nit: Could also mention the RPCs to get the per-peer offset, and the median, as well as the NodeClock time?

💬 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)
```