💬 maflcko commented on issue "Unable to open wallet UI with ubuntu 23.10":
(https://github.com/bitcoin/bitcoin/issues/29311#issuecomment-1909918786)
> May I find it somewhere else?

If a backup exists, you can restore it from there.

> Are there logs?

There is a `debug.log` in the datadir (generally `.bitcoin`) which shows when Bitcoin Core was started, and possibly wallet logs.

💬 vasild commented on pull request "Make (Read/Write)BinaryFile work with char vector, use AutoFile":
(https://github.com/bitcoin/bitcoin/pull/29229#discussion_r1466212055)
This could happen:

1. fwrite() succeeds, returns ok the caller, some of the data is buffered in the OS and not yet on disk
2. fclose() is called, it tries to fflush() the buffered data to disk but fails due to IO error. The caller ignores the error returned by fclose()
3. the program continues with the wrong assumption that the data is safely on disk

💬 stickies-v commented on pull request "doc: clarify `BroadcastTransaction` comment":
(https://github.com/bitcoin/bitcoin/pull/29308#discussion_r1466213307)
I don't think we care about which specific RPCs call it? Would just mention that it can be called by RPC, that's the key info - and more maintainable.

```suggestion
// BroadcastTransaction can be called by RPC or by the wallet.
```

💬 kristapsk commented on pull request "Add a `-permitbarepubkey` option":
(https://github.com/bitcoin/bitcoin/pull/29309#issuecomment-1909949991)
Concept ACK.

> there's no point of adding this if default policy is with P2PK enabled

There is, it gives node runners choice.

💬 vasild commented on pull request "doc: clarify `BroadcastTransaction` comment":
(https://github.com/bitcoin/bitcoin/pull/29308#discussion_r1466224985)
_(this is heading in the direction of "amount of discussion is [inversely proportional](https://bikeshed.com/) to the complexity of the change")_

When working on another PR, I found this comment useful and I cared with RPC called the function. Surely there are other means to check who is calling a given function, but this comment gives a quick concise answer at a glance. To be useful it has to be precise and elaborate. If it is vague then it is less useful and it might as well be removed comp
...

💬 maflcko commented on issue "Huge disk fragmentation":
(https://github.com/bitcoin/bitcoin/issues/29296#issuecomment-1909970643)
According to your debug log 5 blocks are advanced per second, which seems fast, even for spinning storage. Not sure if much can be done to improve upon that. In any case, if you have a software solution, no one is holding you back from testing and benchmarking it. If it does really does improve performance significantly, it can be considered.

💬 stickies-v commented on pull request "doc: clarify `BroadcastTransaction` comment":
(https://github.com/bitcoin/bitcoin/pull/29308#discussion_r1466233109)
Extending that logic, should we document all the call sites of each helper function that's used by RPCs because it can be helpful sometimes? That seems abnormal to me. I understood the only relevance of this line to be in relation to the next one: "chainman, mempool and peerman are initialized before the RPC server and wallet are started". For this, it doesn't matter which RPCs call it.

💬 kristapsk commented on pull request "doc: clarify `BroadcastTransaction` comment":
(https://github.com/bitcoin/bitcoin/pull/29308#discussion_r1466236373)
`BroadcastTransaction()` is not just any helper function, it may irreversibly affect user's privacy.

📝 hebasto opened a pull request: "ci: Update cache action"
(https://github.com/bitcoin/bitcoin/pull/29313)
This PR fixes deprecation [warnings](https://github.com/bitcoin/bitcoin/actions/runs/7652979339) for Node.js 16 actions in the GHA CI:

See:
- https://github.com/marketplace/actions/cache
- https://github.com/actions/cache/releases/tag/v4.0.0

💬 Sjors commented on pull request "Make (Read/Write)BinaryFile work with char vector, use AutoFile":
(https://github.com/bitcoin/bitcoin/pull/29229#discussion_r1466268617)
That's not good. I guess we also don't want to sync to disk for every field that's `>>`'d to a file though.

💬 maflcko commented on pull request "ci: Use DEBUG=1 in depends for MSAN jobs":
(https://github.com/bitcoin/bitcoin/pull/27495#discussion_r1466284981)
might as well use 18rc1, which will be tagged tomorrow?

✅ maflcko closed an issue: "test: use-of-uninitialized-value in sqlite3Strlen30"
(https://github.com/bitcoin/bitcoin/issues/27222)

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