Bitcoin Core Github
44 subscribers
120K links
Download Telegram
💬 MarcoFalke commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#discussion_r1247469065)
```suggestion
throw std::ios_base::failure("AutoFile::seek: file handle is nullptr");
```
📝 MarcoFalke opened a pull request: "test: Rename EncodeDecimal to serialization_fallback"
(https://github.com/bitcoin/bitcoin/pull/28011)
The new name better explains that the function handles fallbacks, without listing all in the function name.
👍 MarcoFalke approved a pull request: "util: Show descriptive error messages when FileCommit fails"
(https://github.com/bitcoin/bitcoin/pull/26654#pullrequestreview-1506634475)
nice. lgtm, but I didn't look at the windows stuff
💬 MarcoFalke commented on pull request "util: Show descriptive error messages when FileCommit fails":
(https://github.com/bitcoin/bitcoin/pull/26654#discussion_r1247515634)
```suggestion
LogPrintf("fsync failed: %s\n", SysErrorString(errno));
```

nits:

* Can remove the manual func logging, which isn't needed, because all strings in this function are already unique to the codebase. Also, users can enable the setting which enabled function logging.
* Can change `%d` to `%s`, since it is a string. (No behavior difference though)
💬 FelixWeis commented on issue "Assertion failed: (data.size() > node.nSendOffset), function SocketSendData, file net.cpp, line 837":
(https://github.com/bitcoin/bitcoin/issues/27963#issuecomment-1614300636)
reverted to compiling 6ee3881551f2cd411c4e4d8b0ccedf0f0416d8c2 (tag: v25.0). same cpu, os, compiler version. runs smooth since 3 days
💬 MarcoFalke commented on issue "Assertion failed: (data.size() > node.nSendOffset), function SocketSendData, file net.cpp, line 837":
(https://github.com/bitcoin/bitcoin/issues/27963#issuecomment-1614306854)
> runs smooth since 3 days

This implies it is not an upstream bug, or the issue fixed itself in the meantime. This may take some time, but you could try bisecting?
💬 TheCharlatan commented on pull request "blockstorage: Return on fatal flush errors":
(https://github.com/bitcoin/bitcoin/pull/27866#discussion_r1247595124)
I can't think of a good reason why this shouldn't be `[[nodiscard]]` too. However, as I said [here](https://github.com/bitcoin/bitcoin/pull/27866#discussion_r1246307365) I am trying to keep this PR's scope limited to just the flush functions and their immediate call sites. I added the `[[nodiscard]]` to `FindBlockPos`, because it reports on the flush error now too. `FindUndoPos` on the other hand is not immediately affected by the flush errors. There are many such examples where we ignore error
...
🚀 fanquake merged a pull request: "refactor: remove in-code warning suppression"
(https://github.com/bitcoin/bitcoin/pull/28002)
💬 hebasto commented on pull request "contrib: add macOS test for fixup_chains usage":
(https://github.com/bitcoin/bitcoin/pull/27999#issuecomment-1614340227)
Guix builds:
```
0e17d462808f86aa7157e27a957da88fd1adeb491ad6c01138aca93e5ad1d018 guix-build-7f96638723a0/output/arm64-apple-darwin/SHA256SUMS.part
ceb208e6374f5d7367b73128e90ca6eaeea15d50c69e49c8cf75b47212525ad7 guix-build-7f96638723a0/output/arm64-apple-darwin/bitcoin-7f96638723a0-arm64-apple-darwin-unsigned.dmg
e31663554cfde8a37a9f3438c9c895dde94b90ff87e28f12f78be71ef6421d93 guix-build-7f96638723a0/output/arm64-apple-darwin/bitcoin-7f96638723a0-arm64-apple-darwin-unsigned.tar.gz
68a7b
...
fanquake closed an issue: "index: ThreadSanitizer: data race on vptr "
(https://github.com/bitcoin/bitcoin/issues/27355)
🚀 fanquake merged a pull request: "test: Use same timeout for all index sync"
(https://github.com/bitcoin/bitcoin/pull/27988)
👍 fanquake approved a pull request: "doc: i2p documentation updates"
(https://github.com/bitcoin/bitcoin/pull/27937#pullrequestreview-1506823009)
ACK 11900e5a8aa4647e50e8d97fc9a8b35e9e20772b
🚀 fanquake merged a pull request: "doc: i2p documentation updates"
(https://github.com/bitcoin/bitcoin/pull/27937)
👍 hebasto approved a pull request: "contrib: add macOS test for fixup_chains usage"
(https://github.com/bitcoin/bitcoin/pull/27999#pullrequestreview-1506825726)
ACK 7f96638723a08edf4341a2f4c06b2aa41378fcf7, I have reviewed the code and the patch, and they look OK.
hebasto closed a pull request: "refactor: Move sock from util to common"
(https://github.com/bitcoin/bitcoin/pull/27989)
💬 TheCharlatan commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#discussion_r1247687691)
After testing with a valid database in the fuzzing corpus, I noticed that I messed up the `m_args`, `m_node.args`, `gArgs` division again ([see here](https://github.com/bitcoin/bitcoin/issues/25055)) - sorry about that. I have a basic patch here, but feel free to commit a more elegant fix:

```diff
diff --git a/src/wallet/test/fuzz/wallet_bdb_parser.cpp b/src/wallet/test/fuzz/wallet_bdb_parser.cpp
index aabde093b6..a1acb95178 100644
--- a/src/wallet/test/fuzz/wallet_bdb_parser.cpp
+++ b/sr
...
📝 MarcoFalke opened a pull request: "util: Allow FastRandomContext::randbytes for std::byte, Allow std::byte serialization "
(https://github.com/bitcoin/bitcoin/pull/28012)
I need this for some stuff, but it should also be useful by itself for other developers that need it.
📝 TheCharlatan opened a pull request: "doc: Fix verify-binaries link in contrib README"
(https://github.com/bitcoin/bitcoin/pull/28013)
🤔 Zero-1729 reviewed a pull request: "doc: Fix verify-binaries link in contrib README"
(https://github.com/bitcoin/bitcoin/pull/28013#pullrequestreview-1506947168)
crACK ab8f6733577555d98668e7708638367a1bfeb023
📝 fanquake opened a pull request: "ci: re-enable gui tests for s390x"
(https://github.com/bitcoin/bitcoin/pull/28014)
These work for me now. If they still don't work in other setups, maybe we can better document the issues.

```bash
time FILE_ENV="./ci/test/00_setup_env_s390x.sh" ./ci/test_run_all.sh
...
Running tests: coins_tests from test/coins_tests.cpp
PASS: qt/test/test_bitcoin-qt
Running tests: coinstatsindex_tests from test/coinstatsindex_tests.cpp
...
Stop and remove CI container by ID
+ docker container kill 617bef8accb87530e5fbb03ff07b3b9f0aa9e3030d4da424c9612d153ab98dbf
617bef8accb87530e5f
...