💬 glozow commented on pull request "refactor: Use typesafe Wtxid in compact block encodings":
(https://github.com/bitcoin/bitcoin/pull/29752#discussion_r1549601987)
nit: I generally prefer to add braces or put these on one line (also [see](https://stackoverflow.com/questions/12193170/whats-the-purpose-of-using-braces-i-e-for-a-single-line-if-or-loop))

💬 fanquake commented on issue "IBD performance regression in 27.0rc1 on Windows":
(https://github.com/bitcoin/bitcoin/issues/29785#issuecomment-2034408348)
> The regression is in the pre-built 27.0rc1 binary for Windows

Ok. It would be good if someone else on Windows can confirm this. Can you also let us know if you're using any particular config options etc.

💬 maflcko commented on pull request "Logging cleanup":
(https://github.com/bitcoin/bitcoin/pull/29798#discussion_r1549650210)
not sure. Just because it currently isn't called with that value, I fail to see what prevents that in the future.

💬 furszy commented on pull request "Fix create unsigned transaction fee bump":
(https://github.com/bitcoin-core/gui/pull/812#discussion_r1549656233)
The change was introduced because the OS system notification wasn't popping up on macOS. Either way is ok for me.

📝 willcl-ark opened a pull request: "Don't permit port in proxy IP option"
(https://github.com/bitcoin-core/gui/pull/813)
Fixes: #809

Previously it was possible through the GUI to enter an IP address:port into the "Proxy IP" configuration box. After the node was restarted the errant setting would prevent the node starting back up until manually removed from settings.json.

Prevent this with a simple check for ":" in the string. This is acceptable here in the GUI setting because we already fail on a hostname such as "http://x.x.x.x", so it won't cause false positives.

<!--
*** Please remove the following he
...

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

💬 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.