💬 MarcoFalke commented on pull request "util: Allow std::byte and char Span serialization":
(https://github.com/bitcoin/bitcoin/pull/27927#issuecomment-1609131921)
Did a rebase on clean master to drop the unneeded dependency.

💬 ismaelsadeeq commented on pull request "bumpfee: Allow the user to choose which output is change":
(https://github.com/bitcoin/bitcoin/pull/26467#issuecomment-1609133068)
re ACK e8c31f135c6e9a5f57325dbf4feceafd384f7762

💬 hebasto commented on pull request "guix: use GCC 12.2.0 to build releases":
(https://github.com/bitcoin/bitcoin/pull/27897#issuecomment-1609149881)
> Switch to using GCC 12 to build release binaries.

Concept ACK on this.

💬 fanquake commented on pull request "guix: Update `python-lief` package to 0.13.2":
(https://github.com/bitcoin/bitcoin/pull/27813#issuecomment-1609150774)
Guix Build:
```bash
018716ffdc0c60b1dfa27f95d1fb4e03d0e37f50cfc325ee35d200039b3f419e guix-build-529c92e837b2/output/aarch64-linux-gnu/SHA256SUMS.part
bb6e74d4806879560e7b6e50bce4b47b383ed5d763e6d25feac64b0c944cc036 guix-build-529c92e837b2/output/aarch64-linux-gnu/bitcoin-529c92e837b2-aarch64-linux-gnu-debug.tar.gz
2e6fc404211c94bc9a0265a783fabcd3c0bfddb9a87322b612185489ef6264d9 guix-build-529c92e837b2/output/aarch64-linux-gnu/bitcoin-529c92e837b2-aarch64-linux-gnu.tar.gz
b0f0d9e2e4466cc3
...

💬 glozow commented on pull request "[NO MERGE] BIP331 Ancestor Package Relay":
(https://github.com/bitcoin/bitcoin/pull/27742#discussion_r1243535255)
I think mentioning specific policy here would make things more confusing tbh. I can add a mention to BIP331?

💬 fanquake commented on pull request "build: produce a .zip for macOS distribution":
(https://github.com/bitcoin/bitcoin/pull/27099#issuecomment-1609267853)
```bash
cbe2109a1e88b431f462f6b8a62e785fe1f5f6249da55f984129c2a2ef454503 guix-build-23d89518e907/output/arm64-apple-darwin/SHA256SUMS.part
47c72d2c1c4ec35cd72db66cd602a213b18d43274dc3ab3fa8065f05e8844f83 guix-build-23d89518e907/output/arm64-apple-darwin/bitcoin-23d89518e907-arm64-apple-darwin-unsigned.tar.gz
eeeccfef61a67f39741e153b392cadc3e44ac317fd0608b30fcd8aab206cd2d2 guix-build-23d89518e907/output/arm64-apple-darwin/bitcoin-23d89518e907-arm64-apple-darwin-unsigned.zip
a662ab05fd44ef5
...

💬 psgreco commented on pull request "Fix potential network stalling bug":
(https://github.com/bitcoin/bitcoin/pull/27981#issuecomment-1609279748)
In theory, it should be, but in our tests (mostly @lolhill 's) it's a good component of this situation is latency. I've never been able to replicate this between 2 local hosts, always with a host that's ~100ms away.

👍 stickies-v approved a pull request: "Added static_assert to check that base_blob is using whole bytes."
(https://github.com/bitcoin/bitcoin/pull/27929#pullrequestreview-1500584404)
ACK 5fc4939e17509534eb36727b27ac0afb941e44f7

👍 MarcoFalke approved a pull request: "wallet: Load database records in a particular order"
(https://github.com/bitcoin/bitcoin/pull/24914#pullrequestreview-1498715782)
lgtm ACK 7cb774048c5f04b3e20d95e794c5a350ea4eff97 🍖

<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 ACK 7cb774048c5f04b3
...

💬 MarcoFalke commented on pull request "wallet: Load database records in a particular order":
(https://github.com/bitcoin/bitcoin/pull/24914#discussion_r1242311686)
dc1ef26e43: trailing (typo)

💬 MarcoFalke commented on pull request "wallet: Load database records in a particular order":
(https://github.com/bitcoin/bitcoin/pull/24914#discussion_r1243513852)
nit in 20d1fc50618911decb74e5b977afbb4ae8bb36ce: I think this is still wrong? Currently this only collects the first error (in cases where assigning the error is guarded by an empty check). Also, it is unused by the caller. Suggested fix:

```diff
diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp
index 1fab6a7f9c..e466c1c477 100644
--- a/src/wallet/walletdb.cpp
+++ b/src/wallet/walletdb.cpp
@@ -754,7 +754,6 @@ static DBErrors LoadWalletFlags(CWallet* pwallet, DatabaseBatch& b
...

💬 MarcoFalke commented on pull request "wallet: Load database records in a particular order":
(https://github.com/bitcoin/bitcoin/pull/24914#discussion_r1243571310)
877382511cc63b64e22bb9f06fdbcb1df0c06f55: Any reason to check for err.empty, when the error shouldn't be empty, and the later assignment also doesn't check for empty?

💬 MarcoFalke commented on pull request "wallet: Load database records in a particular order":
(https://github.com/bitcoin/bitcoin/pull/24914#discussion_r1243525495)
Also, could mention that for other record types, previous early returns from LoadWallet like `UNEXPECTED_LEGACY_ENTRY` would now be scheduled for later, and it continues to load more prefix-keys?

✅ n1kcha closed an issue: "Wallet not loaded"
(https://github.com/bitcoin/bitcoin/issues/27982)

💬 hebasto commented on pull request "build: produce a .zip for macOS distribution":
(https://github.com/bitcoin/bitcoin/pull/27099#issuecomment-1609332154)
Guix build on `arm64`:
```
a9eb04f494db558318b52ec609d67478c761b281902204cc1ebfb49450216ac9 guix-build-23d89518e907/output/arm64-apple-darwin/SHA256SUMS.part
47c72d2c1c4ec35cd72db66cd602a213b18d43274dc3ab3fa8065f05e8844f83 guix-build-23d89518e907/output/arm64-apple-darwin/bitcoin-23d89518e907-arm64-apple-darwin-unsigned.tar.gz
04651cc05a9ae9786ecdba7c44f6823559e70aeb75d84875907c61e0ad137cdc guix-build-23d89518e907/output/arm64-apple-darwin/bitcoin-23d89518e907-arm64-apple-darwin-unsigned.
...

💬 darosior commented on pull request "Wallet: estimate the size of signed inputs using descriptors":
(https://github.com/bitcoin/bitcoin/pull/26567#discussion_r1243598951)
Because serializing a `1` as a witness stack element needs to specify a 1-byte long element and then the value of this byte (serialized `0x0101`) whereas encoding a `0` is simply the empty witness (serialized `0x00`).

⚠️ ZenulAbidin opened an issue: "An option for a shell command that runs just before bitcoind completes shutting down."
(https://github.com/bitcoin/bitcoin/issues/27984)
### Please describe the feature you'd like to see added.

It basically works similar to the blocknotify config option which runs a shell script when a block is mined, but the purpose of this new option is different.

It can be called something like "shutdowncomplete" and a shell command is passed to it as an argument, which then runs just before the following message is printed in debug.log:

```
2023-06-26T14:00:07Z Shutdown: done
```

It can be ran utilizing a function such as `runComm
...

💬 fanquake commented on issue "An option for a shell command that runs just before bitcoind completes shutting down.":
(https://github.com/bitcoin/bitcoin/issues/27984#issuecomment-1609340699)
We already have `shutdownnotify`.

🚀 fanquake merged a pull request: "Added static_assert to check that base_blob is using whole bytes."
(https://github.com/bitcoin/bitcoin/pull/27929)

💬 MarcoFalke commented on pull request "wallet: Load database records in a particular order":
(https://github.com/bitcoin/bitcoin/pull/24914#issuecomment-1609403231)
Also, it may be good to add tests for currently untested code: https://marcofalke.github.io/b-c-cov/total.coverage/src/wallet/wallet.cpp.gcov.html#2926