💬 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
...
(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.
(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
(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
...
(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)
(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
...
(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?
(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?
(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)
(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.
...
(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`).
(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
...
(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`.
(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)
(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
(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
💬 craigraw commented on pull request "Add feerate histogram to getmempoolinfo":
(https://github.com/bitcoin/bitcoin/pull/21422#issuecomment-1609436322)
Using `getrawmempool true` is unfortunately not a solution that can be applied generally to solve this use case. On systems with low memory (4Gb or less, for example a Raspberry Pi or the Futurebit Apollo) this RPC call causes the kernel to terminate the Bitcoin Core process (OOM) when the mempool is reasonably full. This occurs consistently on all released versions AFAIK.
One workaround is to use `getrawmempool false` followed by repeated calls to `getmempoolentry`. This of course is even mo
...
(https://github.com/bitcoin/bitcoin/pull/21422#issuecomment-1609436322)
Using `getrawmempool true` is unfortunately not a solution that can be applied generally to solve this use case. On systems with low memory (4Gb or less, for example a Raspberry Pi or the Futurebit Apollo) this RPC call causes the kernel to terminate the Bitcoin Core process (OOM) when the mempool is reasonably full. This occurs consistently on all released versions AFAIK.
One workaround is to use `getrawmempool false` followed by repeated calls to `getmempoolentry`. This of course is even mo
...
💬 sdaftuar commented on pull request "Rework validation logic for assumeutxo":
(https://github.com/bitcoin/bitcoin/pull/27746#discussion_r1243705025)
It doesn't seem necessary to me, because the block arrival information is something that seems unrelated to setting up a chainstate (and I don't see why it would need to be reset in the event of a chainstate activation).
I think if we need to reset the counters it would be for tests, which is the only place outside of `init.cpp` where we use this... I thought I addressed adding `ResetBlockSequenceCounters()` where it was necessary, but I should look at that again to see if I missed a case.
(https://github.com/bitcoin/bitcoin/pull/27746#discussion_r1243705025)
It doesn't seem necessary to me, because the block arrival information is something that seems unrelated to setting up a chainstate (and I don't see why it would need to be reset in the event of a chainstate activation).
I think if we need to reset the counters it would be for tests, which is the only place outside of `init.cpp` where we use this... I thought I addressed adding `ResetBlockSequenceCounters()` where it was necessary, but I should look at that again to see if I missed a case.
💬 sdaftuar commented on pull request "Rework validation logic for assumeutxo":
(https://github.com/bitcoin/bitcoin/pull/27746#discussion_r1243732248)
Good idea -- and it looks like my code here is wrong anyway, because if we're a disabled background chainstate, then we'll be adding blocks to setBlockIndexCandidates as long as they have more work than the tip!
(https://github.com/bitcoin/bitcoin/pull/27746#discussion_r1243732248)
Good idea -- and it looks like my code here is wrong anyway, because if we're a disabled background chainstate, then we'll be adding blocks to setBlockIndexCandidates as long as they have more work than the tip!
💬 willcl-ark commented on pull request "build: debug enable addrman consistency checks":
(https://github.com/bitcoin/bitcoin/pull/27300#issuecomment-1609505161)
Nope, I also agree with the conclusions from AJ in 24709 about just setting this as a runtime option, so will close this.
(https://github.com/bitcoin/bitcoin/pull/27300#issuecomment-1609505161)
Nope, I also agree with the conclusions from AJ in 24709 about just setting this as a runtime option, so will close this.
✅ willcl-ark closed a pull request: "build: debug enable addrman consistency checks"
(https://github.com/bitcoin/bitcoin/pull/27300)
(https://github.com/bitcoin/bitcoin/pull/27300)