📝 apoelstra converted_to_draft a pull request: "wallet: allow importing descriptors that have no xprivs, even in a privkey-enabled wallet"
(https://github.com/bitcoin/bitcoin/pull/27337)
Importing pubkey-only descriptors works fine as long as the wallet already has the required privkeys.
Happy to add whatever tests people advise me to.
Fixes #27336
**Edit** Never mind -- we can update balances but not sign coins. More work needs to be done to support this.
(https://github.com/bitcoin/bitcoin/pull/27337)
Importing pubkey-only descriptors works fine as long as the wallet already has the required privkeys.
Happy to add whatever tests people advise me to.
Fixes #27336
**Edit** Never mind -- we can update balances but not sign coins. More work needs to be done to support this.
💬 pablomartin4btc commented on pull request "wallet: finish addressbook encapsulation":
(https://github.com/bitcoin/bitcoin/pull/26836#discussion_r1148773136)
nit:
```suggestion
bool require_is_mine{record.purpose == "receive" && !IsMine(dest)};
bool copied{false};
```
(https://github.com/bitcoin/bitcoin/pull/26836#discussion_r1148773136)
nit:
```suggestion
bool require_is_mine{record.purpose == "receive" && !IsMine(dest)};
bool copied{false};
```
💬 pablomartin4btc commented on pull request "wallet: finish addressbook encapsulation":
(https://github.com/bitcoin/bitcoin/pull/26836#discussion_r1148773578)
```suggestion
const std::string& dest{EncodeDestination(address)};
```
(https://github.com/bitcoin/bitcoin/pull/26836#discussion_r1148773578)
```suggestion
const std::string& dest{EncodeDestination(address)};
```
💬 martinus commented on pull request "clang-tidy: Add more `performance-*` checks and related fixes":
(https://github.com/bitcoin/bitcoin/pull/26642#issuecomment-1484531112)
ACK 03ec5b6f9ca3af28c9ce25cf2393e28ae852d808
(https://github.com/bitcoin/bitcoin/pull/26642#issuecomment-1484531112)
ACK 03ec5b6f9ca3af28c9ce25cf2393e28ae852d808
💬 TheCharlatan commented on pull request "clang-tidy: Add more `performance-*` checks and related fixes":
(https://github.com/bitcoin/bitcoin/pull/26642#issuecomment-1484550109)
re-ACK [03ec5b6](https://github.com/bitcoin/bitcoin/pull/26642/commits/03ec5b6f9ca3af28c9ce25cf2393e28ae852d808)
(https://github.com/bitcoin/bitcoin/pull/26642#issuecomment-1484550109)
re-ACK [03ec5b6](https://github.com/bitcoin/bitcoin/pull/26642/commits/03ec5b6f9ca3af28c9ce25cf2393e28ae852d808)
💬 vstoyanov commented on pull request "ci: cleanup of CI_EXEC & CI_EXEC_ROOT (refs #27321)":
(https://github.com/bitcoin/bitcoin/pull/27333#discussion_r1148824467)
Yes, but the next time they are used in `ci/test/04_install.sh`, they are already redefined on lines 79-84:
`CI_EXEC () {
$CI_EXEC_CMD_PREFIX bash -c "export PATH=${BINS_SCRATCH_DIR}:\$PATH && cd \"$P_CI_DIR\" && $*"
}
CI_EXEC_ROOT () {
$CI_EXEC_CMD_PREFIX_ROOT bash -c "export PATH=${BINS_SCRATCH_DIR}:\$PATH && cd \"$P_CI_DIR\" && $*"
}`
This PR only relates to their usage in base setup.
(https://github.com/bitcoin/bitcoin/pull/27333#discussion_r1148824467)
Yes, but the next time they are used in `ci/test/04_install.sh`, they are already redefined on lines 79-84:
`CI_EXEC () {
$CI_EXEC_CMD_PREFIX bash -c "export PATH=${BINS_SCRATCH_DIR}:\$PATH && cd \"$P_CI_DIR\" && $*"
}
CI_EXEC_ROOT () {
$CI_EXEC_CMD_PREFIX_ROOT bash -c "export PATH=${BINS_SCRATCH_DIR}:\$PATH && cd \"$P_CI_DIR\" && $*"
}`
This PR only relates to their usage in base setup.
💬 MarcoFalke commented on pull request "ci: cleanup of CI_EXEC & CI_EXEC_ROOT (refs #27321)":
(https://github.com/bitcoin/bitcoin/pull/27333#discussion_r1148898438)
I am thinking that the second user account can be removed completely. I think it is fine to run all tests as root. The reason for https://github.com/bitcoin/bitcoin/pull/25900 (`./depends/` being owned by root) no longer applies as `./depends/` is a docker/podman volume.
However, this is probably for a follow-up.
(https://github.com/bitcoin/bitcoin/pull/27333#discussion_r1148898438)
I am thinking that the second user account can be removed completely. I think it is fine to run all tests as root. The reason for https://github.com/bitcoin/bitcoin/pull/25900 (`./depends/` being owned by root) no longer applies as `./depends/` is a docker/podman volume.
However, this is probably for a follow-up.
💬 MarcoFalke commented on pull request "ci: cleanup of CI_EXEC & CI_EXEC_ROOT (refs #27321)":
(https://github.com/bitcoin/bitcoin/pull/27333#issuecomment-1484646255)
lgtm ACK f52d482408af68458f0b5c2775b8a1dffc380466
(https://github.com/bitcoin/bitcoin/pull/27333#issuecomment-1484646255)
lgtm ACK f52d482408af68458f0b5c2775b8a1dffc380466
👍 MarcoFalke approved a pull request: "ci: cleanup of CI_EXEC & CI_EXEC_ROOT (refs #27321)"
(https://github.com/bitcoin/bitcoin/pull/27333)
left a question/nit, feel free to ignore
(https://github.com/bitcoin/bitcoin/pull/27333)
left a question/nit, feel free to ignore
💬 MarcoFalke commented on pull request "ci: cleanup of CI_EXEC & CI_EXEC_ROOT (refs #27321)":
(https://github.com/bitcoin/bitcoin/pull/27333#discussion_r1148907877)
```suggestion
${CI_RETRY_EXE} dnf -y --allowerasing install $CI_BASE_PACKAGES $PACKAGES
```
I guess I know nothing about bash, but putting it this way may also work at no downside?
(https://github.com/bitcoin/bitcoin/pull/27333#discussion_r1148907877)
```suggestion
${CI_RETRY_EXE} dnf -y --allowerasing install $CI_BASE_PACKAGES $PACKAGES
```
I guess I know nothing about bash, but putting it this way may also work at no downside?
💬 MarcoFalke commented on pull request "httpserver, rest: fix segmentation fault on evhttp_uri_get_query":
(https://github.com/bitcoin/bitcoin/pull/27253#issuecomment-1484668108)
> FUZZ=process_message ./src/test/fuzz/fuzz ../qa-assets/fuzz_seed_corpus/http_request/75d231d5e022cc38377bbbf4549058d37b7c59a3
You'll need to set the correct fuzz target:
`FUZZ=http_request ./src/test/fuzz/fuzz ../qa-assets/fuzz_seed_corpus/http_request/75d231d5e022cc38377bbbf4549058d37b7c59a3`
(https://github.com/bitcoin/bitcoin/pull/27253#issuecomment-1484668108)
> FUZZ=process_message ./src/test/fuzz/fuzz ../qa-assets/fuzz_seed_corpus/http_request/75d231d5e022cc38377bbbf4549058d37b7c59a3
You'll need to set the correct fuzz target:
`FUZZ=http_request ./src/test/fuzz/fuzz ../qa-assets/fuzz_seed_corpus/http_request/75d231d5e022cc38377bbbf4549058d37b7c59a3`
💬 MarcoFalke commented on pull request "log: Check that the timestamp string is non-empty to avoid undefined behavior":
(https://github.com/bitcoin/bitcoin/pull/27317#discussion_r1148926296)
If there are steps to reproduce this, it would be good to write a detection for it at Bitcoin Core init time and refuse to start the program (if this isn't already done).
(https://github.com/bitcoin/bitcoin/pull/27317#discussion_r1148926296)
If there are steps to reproduce this, it would be good to write a detection for it at Bitcoin Core init time and refuse to start the program (if this isn't already done).
📝 MarcoFalke opened a pull request: "ci: Use Cirrus CI dockerfile env"
(https://github.com/bitcoin/bitcoin/pull/27340)
Currently the CI env has many intermittent issues:
* The Ubuntu package servers are frequently down
* Occasionally other stuff is down, such as dnf, pip, or the android sdk
* Installing packages is slower than downloading them, at least on Cirrus, which has a fast download speed
Fix all issues by using the Cirrus CI dockerfile env.
(https://github.com/bitcoin/bitcoin/pull/27340)
Currently the CI env has many intermittent issues:
* The Ubuntu package servers are frequently down
* Occasionally other stuff is down, such as dnf, pip, or the android sdk
* Installing packages is slower than downloading them, at least on Cirrus, which has a fast download speed
Fix all issues by using the Cirrus CI dockerfile env.
📝 MarcoFalke converted_to_draft a pull request: "ci: Use Cirrus CI dockerfile env"
(https://github.com/bitcoin/bitcoin/pull/27340)
Currently the CI env has many intermittent issues:
* The Ubuntu package servers are frequently down
* Occasionally other stuff is down, such as dnf, pip, or the android sdk
* Installing packages is slower than downloading them, at least on Cirrus, which has a fast download speed
Fix all issues by using the Cirrus CI dockerfile env.
(https://github.com/bitcoin/bitcoin/pull/27340)
Currently the CI env has many intermittent issues:
* The Ubuntu package servers are frequently down
* Occasionally other stuff is down, such as dnf, pip, or the android sdk
* Installing packages is slower than downloading them, at least on Cirrus, which has a fast download speed
Fix all issues by using the Cirrus CI dockerfile env.
💬 willcl-ark commented on issue "Not able to load multiple wallet more than X number of wallet from RPC":
(https://github.com/bitcoin/bitcoin/issues/15017#issuecomment-1484826665)
@adneerav As there has been no update on this since 2019 (aside from my own follow-up) and I was unable to reproduce this issue with the current version of Bitcoin Core, I'm going to close this for now.
Feel free to comment here if you are still experiencing this problem and want this re-opened to be investigated again.
(https://github.com/bitcoin/bitcoin/issues/15017#issuecomment-1484826665)
@adneerav As there has been no update on this since 2019 (aside from my own follow-up) and I was unable to reproduce this issue with the current version of Bitcoin Core, I'm going to close this for now.
Feel free to comment here if you are still experiencing this problem and want this re-opened to be investigated again.
✅ willcl-ark closed an issue: "Not able to load multiple wallet more than X number of wallet from RPC"
(https://github.com/bitcoin/bitcoin/issues/15017)
(https://github.com/bitcoin/bitcoin/issues/15017)
💬 hebasto commented on pull request "Fixes compile errors in MSVC build #27332":
(https://github.com/bitcoin/bitcoin/pull/27335#discussion_r1149059515)
Why this change is required?
(https://github.com/bitcoin/bitcoin/pull/27335#discussion_r1149059515)
Why this change is required?
💬 hebasto commented on pull request "Fixes compile errors in MSVC build #27332":
(https://github.com/bitcoin/bitcoin/pull/27335#discussion_r1149059179)
```suggestion
/* Assume that an MSVC build will get dependencies from vcpkg and thus use the latest version of libevent */
```
(https://github.com/bitcoin/bitcoin/pull/27335#discussion_r1149059179)
```suggestion
/* Assume that an MSVC build will get dependencies from vcpkg and thus use the latest version of libevent */
```
👍 hebasto approved a pull request: "Fix segfault when shutdown during wallet open"
(https://github.com/bitcoin-core/gui/pull/693)
ACK 9a1d73fdffa4f35e33bc187ac9b3afbba28b1950, I have reviewed the code and it looks OK.
(https://github.com/bitcoin-core/gui/pull/693)
ACK 9a1d73fdffa4f35e33bc187ac9b3afbba28b1950, I have reviewed the code and it looks OK.
💬 hebasto commented on issue "Segmentation fault when closing while loading wallets":
(https://github.com/bitcoin-core/gui/issues/689#issuecomment-1484922884)
@MarcoFalke
Can you confirm that https://github.com/bitcoin-core/gui/pull/693 fixes this issue for you?
(https://github.com/bitcoin-core/gui/issues/689#issuecomment-1484922884)
@MarcoFalke
Can you confirm that https://github.com/bitcoin-core/gui/pull/693 fixes this issue for you?