💬 maflcko commented on issue "test: use-of-uninitialized-value in sqlite3Strlen30":
(https://github.com/bitcoin/bitcoin/issues/27222#issuecomment-1910086537)
Fixed in https://github.com/bitcoin/bitcoin/pull/29287

💬 stickies-v commented on pull request "rpc: improve submitpackage documentation and other improvements":
(https://github.com/bitcoin/bitcoin/pull/29292#discussion_r1466289182)
Added test coverage: the [test diff](https://github.com/bitcoin/bitcoin/compare/6476ab562327d4690fddefc8336c3ddce211411a..021ecfea062151bce0d67bf1575fe7e5e1ee22db#diff-2757e99ad43064441a6401f5002232a6a311d92d16b49df66f51d427ce448a68) after changing the bounds checking shows how the error messages change

💬 maflcko commented on issue "The `streams_tests/xor_file` test fails on Windows":
(https://github.com/bitcoin/bitcoin/issues/29014#issuecomment-1910102335)
Is this fixed in a later version of gcc?

💬 hebasto commented on pull request "depends: Do not override `CFLAGS` when building SQLite with `DEBUG=1`":
(https://github.com/bitcoin/bitcoin/pull/29287#issuecomment-1910103963)
@fanquake Thank you for your review!

> > Changes in #29282 might not be strictly required now. However, I consider them an improvement.
>
> I think it'd be easiest to make (any) additional changes together here, so we don't have to go over the flags twice. Can you combine any relevant changes and close the other PR.

#29282 has been integrated into this PR.

> Related, I guess `-g` will no-longer be used, when compiling the lib for debugging?

Added `$(package)_cflags_debug += -g`.

✅ hebasto closed a pull request: "depends: Ensure definitions are passed when building SQLite with `DEBUG=1`"
(https://github.com/bitcoin/bitcoin/pull/29282)

💬 hebasto commented on pull request "depends: Ensure definitions are passed when building SQLite with `DEBUG=1`":
(https://github.com/bitcoin/bitcoin/pull/29282#issuecomment-1910104513)
Combined into #29287.

Closing.

📝 maflcko opened a pull request: "refactor: Compile unreachable walletdb code"
(https://github.com/bitcoin/bitcoin/pull/29315)
When unreachable code isn't compiled, compile failures are not detected.

Fix this by leaving it unreachable, but compiling it.

Fixes https://github.com/bitcoin/bitcoin/pull/28999#discussion_r1465010916

💬 vasild commented on pull request "Make (Read/Write)BinaryFile work with char vector, use AutoFile":
(https://github.com/bitcoin/bitcoin/pull/29229#discussion_r1466321697)
Right. That's why I don't see a good solution. It is a design issue with `AutoFile` to flush/close from the destructor which can't signal the failure to the caller.

💬 maflcko commented on pull request "refactor: Compile unreachable walletdb code":
(https://github.com/bitcoin/bitcoin/pull/29315#issuecomment-1910133709)
Can be tested by adding a compile failure to the unreachable code. On master compilation passes, here it fails.

👍 maflcko approved a pull request: "ci: Update cache action"
(https://github.com/bitcoin/bitcoin/pull/29313#pullrequestreview-1843645352)
lgtm

💬 maflcko commented on pull request "validation: improve checkblockindex comments":
(https://github.com/bitcoin/bitcoin/pull/29299#issuecomment-1910156794)
ACK 9819db4ccaa03519a78d4d9ecce9f89f5be669e5 🌦

<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: ACK 9819db4ccaa03519a78d4d9ecc
...

💬 maflcko commented on issue "make cov fails with lcov-2":
(https://github.com/bitcoin/bitcoin/issues/28468#issuecomment-1910192420)
> but it's still broken

Same here. Is there an upstream bug report?

💬 torkelrogstad commented on pull request "rpc: validate fee estimation mode case insensitive":
(https://github.com/bitcoin/bitcoin/pull/29175#discussion_r1466378645)
Sorry, I don't understand what you mean. Do you want me to use one of the functions in `strencodings` to manipulate the `fee_rate` parameter before sending it to `options.pushKV`?

💬 instagibbs commented on pull request "Mempool util: Add RBF diagram checks for single chunks against clusters of size 2":
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1466391538)
fixed

💬 instagibbs commented on pull request "Mempool util: Add RBF diagram checks for single chunks against clusters of size 2":
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1466392222)
I'll leave this as an ergonomics issue for later debate

💬 instagibbs commented on pull request "Mempool util: Add RBF diagram checks for single chunks against clusters of size 2":
(https://github.com/bitcoin/bitcoin/pull/29242#issuecomment-1910238997)
didn't see any fuzzer crashes locally, all comments addressed

💬 torkelrogstad commented on pull request "rpc: validate fee estimation mode case insensitive":
(https://github.com/bitcoin/bitcoin/pull/29175#issuecomment-1910256641)
Rebased, let's see if that solves the CI failure.

💬 ismaelsadeeq commented on pull request "rpc: validate fee estimation mode case insensitive":
(https://github.com/bitcoin/bitcoin/pull/29175#discussion_r1466426175)
I meant Import what you use.
`ToLower()` function is declared here.
https://github.com/bitcoin/bitcoin/blob/4ad83ef09b772f3f1e8ea9ea3a8c43dcd9db8458/src/util/strencodings.h#L327

🤔 furszy reviewed a pull request: "test: ensure output is large enough to pay for its fees"
(https://github.com/bitcoin/bitcoin/pull/29283#pullrequestreview-1843832314)
utACK 3bfc5bd36

💬 furszy commented on pull request "test: ensure output is large enough to pay for its fees":
(https://github.com/bitcoin/bitcoin/pull/29283#discussion_r1466431816)
Maybe:
```suggestion
# Ensure output is large enough to pay for its fees. Following transactions will contain one input and one unspendable output. Conservatively assuming...
```

Also, just as an extra note (no need to do it): could calculate the floor in a more dynamic manner if you want. The input type is known here, and the future tx is always fixed to be a 1 input, 1 bech32 output tx.