Bitcoin Core Github
44 subscribers
121K links
Download Telegram
💬 vasild commented on pull request "util: explicitly close all AutoFiles that have been written":
(https://github.com/bitcoin/bitcoin/pull/29307#discussion_r2071708123)
Done.
💬 vasild commented on pull request "util: explicitly close all AutoFiles that have been written":
(https://github.com/bitcoin/bitcoin/pull/29307#discussion_r2071708360)
Done.
💬 vasild commented on pull request "util: explicitly close all AutoFiles that have been written":
(https://github.com/bitcoin/bitcoin/pull/29307#discussion_r2071715279)
Hmm, the OS `feof()` returns nonzero if end-of-file. This makes it possible to use the natural:

```cpp
if (feof()) { // end-of-file has been reached
```

But `fclose()` returns zero if the file has been closed. I do not want to invert the return value because that would be confusing for people that know the interface of the OS `fclose()`. Or even more confusing, if the return is not inverted and converted to `bool`:

```
bool fclose(...);

if (fclose()) { // error!
```
💬 vasild commented on pull request "util: explicitly close all AutoFiles that have been written":
(https://github.com/bitcoin/bitcoin/pull/29307#discussion_r2071716526)
Elaborated a little bit:

```cpp
// Callers that wrote to the file must have closed it explicitly
// with the fclose() method and checked that the close succeeded.
// This is because here from the destructor we have no way to signal
// error due to close which, after write, could mean the file is
// corrupted and must be handled properly at the call site.
// Destructors in C++ cannot signal an error to the callers be
...
💬 vasild commented on pull request "util: explicitly close all AutoFiles that have been written":
(https://github.com/bitcoin/bitcoin/pull/29307#discussion_r2071717206)
I think it is better to stop the test immediately if closing a file gives an error.
💬 vasild commented on pull request "util: explicitly close all AutoFiles that have been written":
(https://github.com/bitcoin/bitcoin/pull/29307#discussion_r2071717840)
What to be added to `AutoFile::write_buffer`?
💬 hebasto commented on issue "build: x86 afl++ ASan build broken "error: inline assembly requires more registers than available"":
(https://github.com/bitcoin/bitcoin/issues/31913#issuecomment-2847384462)
My guess is that `/AFLplusplus/afl-clang-fast++` fails to handle the `-fcf-protection=full -fcf-protection=none` flags properly.
💬 vasild commented on pull request "util: explicitly close all AutoFiles that have been written":
(https://github.com/bitcoin/bitcoin/pull/29307#issuecomment-2847391766)
`66087035cf...fe2d1955f6`: add missing includes in the first commit
hebasto closed a pull request: "[POC] cmake: Introduce LLVM's Source-based Code Coverage reports"
(https://github.com/bitcoin/bitcoin/pull/31394)
💬 mzumsande commented on pull request "cli: add -usefile option":
(https://github.com/bitcoin/bitcoin/pull/32399#issuecomment-2847462346)
> I've been using `-stdin` (e.g. `cat block.hex | bitcoin-cli -stdin submitblock`) for this. Would this work here too?

Oh, I didn't think of that - I tried it out and I think that would work too - that would make it simpler, no changes to bitcoin-cli necessary, will change to that approach.
💬 brunoerg commented on pull request "fuzz: wallet: add target for spkm migration":
(https://github.com/bitcoin/bitcoin/pull/29694#discussion_r2071773924)
Yes, these other things are also not necessary when using in memory db. I'm gonna address it.
💬 mzumsande commented on pull request "cli: add -usefile option":
(https://github.com/bitcoin/bitcoin/pull/32399#issuecomment-2847489011)
Since this will only change the test framework now, I'll just add it to #32290 - no extra PR needed anymore.
mzumsande closed a pull request: "cli: add -usefile option"
(https://github.com/bitcoin/bitcoin/pull/32399)
📝 fanquake opened a pull request: "build: replace header checks with `__has_include`"
(https://github.com/bitcoin/bitcoin/pull/32405)
Replace the checks in CMake, with the equivalent functionality provided by the standard library (since C++17).
See https://en.cppreference.com/w/cpp/preprocessor/include.

Guix Build:
```bash
02f63c78b657e08dfc2a14b910ba28bdd00e67726093ff680e8eb8b4fd3787ea guix-build-e7fdd31ee0ce/output/aarch64-linux-gnu/SHA256SUMS.part
fbb9e2abd636f5d125141d2fe60350bb0678ae830f66e7047ae0b47451a63b81 guix-build-e7fdd31ee0ce/output/aarch64-linux-gnu/bitcoin-e7fdd31ee0ce-aarch64-linux-gnu-debug.tar.gz
855
...
💬 brunoerg commented on pull request "fuzz: wallet: add target for spkm migration":
(https://github.com/bitcoin/bitcoin/pull/29694#discussion_r2071800794)
Why? the for after `// Unload the wallets` are basically playing with `created_wallets` which is used only if the db is not in memory. Same for the lines after `// Verify that there is no dangling wallet` which needs `ptr_wallet`.
📝 instagibbs opened a pull request: "policy: uncap datacarrier by default"
(https://github.com/bitcoin/bitcoin/pull/32406)
Retains the `-datacarrier*` args, marks them as deprecated, and does not require another argument for multiple OP_RETURN outputs.

If a user has set `-datacarriersize` the value is "budgeted" across all seen OP_RETURN output scriptPubKeys. In other words the total script bytes stays the same, but can be spread across any number of outputs. This is done to not introduce an additional argument to support multiple outputs.

I do not advise people use the option with custom arguments and it is m
...
🤔 BrandonOdiwuor reviewed a pull request: "Wallet: Fix Non-Ranged Descriptors with Range [0,0] Trigger Unexpected Wallet Errors in AddWalletDescriptor"
(https://github.com/bitcoin/bitcoin/pull/32344#pullrequestreview-2812344598)
Code Review ACK fa0993462b3bc8db40c1a6ea2e7f5fd4a22353a6
💬 l0rinc commented on pull request "validation: write chainstate to disk every hour":
(https://github.com/bitcoin/bitcoin/pull/30611#issuecomment-2847561360)
Post-merge good news - the periodic flushing doesn't seem to introduce any slowdown for reindex-chainstate:

<details>
<summary>reindex-chainstate</summary>

```bash
COMMITS="68ac9f116c0228a277f18f60ba2278b56356e6ac 58e7798f7e85ecda97e32eec29c190e5ac958dc0"; \
STOP_HEIGHT=888888; DBCACHE=45000; \
CC=gcc; CXX=g++; \
BASE_DIR="/mnt/my_storage"; DATA_DIR="$BASE_DIR/BitcoinData"; LOG_DIR="$BASE_DIR/logs"; \
(echo ""; for c in $COMMITS; do git fetch origin $c -q && git log -1 --pretty=forma
...
💬 Sjors commented on pull request "Shuffle depends instructions and recommend modern make for macOS":
(https://github.com/bitcoin/bitcoin/pull/32086#discussion_r2071824993)
Added, cc @vasild
💬 Sjors commented on pull request "Shuffle depends instructions and recommend modern make for macOS":
(https://github.com/bitcoin/bitcoin/pull/32086#issuecomment-2847576668)
Rebased just in case and added a commit to switch FreeBSD recommendation to `gmake`.