⚠️ maflcko opened an issue: "Intermittent failure in feature_fee_estimation.py" in sanity_check_rbf_estimates: est_feerate = node.estimatesmartfee(2)["feerate"] (KeyError: 'feerate')"
(https://github.com/bitcoin/bitcoin/issues/30640)
https://cirrus-ci.com/task/5165357323780096?logs=ci#L2840
```
test 2024-08-09T22:10:43.283000Z TestFramework (ERROR): Key error
Traceback (most recent call last):
File "/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/test_framework.py", line 132, in main
self.run_test()
File "/ci_container_bas
...
(https://github.com/bitcoin/bitcoin/issues/30640)
https://cirrus-ci.com/task/5165357323780096?logs=ci#L2840
```
test 2024-08-09T22:10:43.283000Z TestFramework (ERROR): Key error
Traceback (most recent call last):
File "/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/test_framework.py", line 132, in main
self.run_test()
File "/ci_container_bas
...
💬 0xawaz commented on issue "contrib: Automation for Bitcoin Full Node Deployment":
(https://github.com/bitcoin/bitcoin/issues/30638#issuecomment-2284604375)
Thanks @maflcko for pointing out docker discussions I missed. I took some time to walk through all the discussions. It just got me more confused. I am genuinely curious to understand what's happening. So I am asking few questions:
- **Docker**
- Since we all agree that we need an official docker image for Bitcoin Core, what all PR attempts fail to consider?
- My understanding is Dockerfile maintenance is one of the main concerns, could you elaborate more? Do you need more hands in the t
...
(https://github.com/bitcoin/bitcoin/issues/30638#issuecomment-2284604375)
Thanks @maflcko for pointing out docker discussions I missed. I took some time to walk through all the discussions. It just got me more confused. I am genuinely curious to understand what's happening. So I am asking few questions:
- **Docker**
- Since we all agree that we need an official docker image for Bitcoin Core, what all PR attempts fail to consider?
- My understanding is Dockerfile maintenance is one of the main concerns, could you elaborate more? Do you need more hands in the t
...
💬 ryanofsky commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-2284621389)
This seems to offer a lot of nice features, but can you explain the tradeoffs of wrapping the C++ interface in C instead of using C++ from rust directly? It seems like having a C middle layer introduces a lot of boilerplate, and I'm wondering if it is really necessary. For example it seems like there is a rust cxx crate (https://docs.rs/cxx/latest/cxx/, https://chatgpt.com/share/dd4dde59-66d6-4486-88a6-2f42144be056) that lets you call C++ directly from Rust and avoid the need for C boilerplate.
...
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-2284621389)
This seems to offer a lot of nice features, but can you explain the tradeoffs of wrapping the C++ interface in C instead of using C++ from rust directly? It seems like having a C middle layer introduces a lot of boilerplate, and I'm wondering if it is really necessary. For example it seems like there is a rust cxx crate (https://docs.rs/cxx/latest/cxx/, https://chatgpt.com/share/dd4dde59-66d6-4486-88a6-2f42144be056) that lets you call C++ directly from Rust and avoid the need for C boilerplate.
...
💬 Sjors commented on pull request "multiprocess: Add -ipcbind option to bitcoin-node":
(https://github.com/bitcoin/bitcoin/pull/30509#discussion_r1714153255)
a8d4adfea4b50f452d80bc1e7ee322945d886c78: @vasild's #30205 is potentially useful here. Though as long as the test is not brittle, there's no need to wait for that PR to be merged.
(https://github.com/bitcoin/bitcoin/pull/30509#discussion_r1714153255)
a8d4adfea4b50f452d80bc1e7ee322945d886c78: @vasild's #30205 is potentially useful here. Though as long as the test is not brittle, there's no need to wait for that PR to be merged.
🤔 Sjors reviewed a pull request: "multiprocess: Add -ipcbind option to bitcoin-node"
(https://github.com/bitcoin/bitcoin/pull/30509#pullrequestreview-2233467423)
ACK 4137419b0170fa1e9ff9daacde57933f7c995b76
I only lightly looked at the new `ProcessImpl` method bodies.
I tested that I'm able to connect the bitcoin-mine demo #30437, but that (as expected) it immediately fails with a "remote exception: Method not implemented".
(https://github.com/bitcoin/bitcoin/pull/30509#pullrequestreview-2233467423)
ACK 4137419b0170fa1e9ff9daacde57933f7c995b76
I only lightly looked at the new `ProcessImpl` method bodies.
I tested that I'm able to connect the bitcoin-mine demo #30437, but that (as expected) it immediately fails with a "remote exception: Method not implemented".
💬 Sjors commented on pull request "multiprocess: Add -ipcbind option to bitcoin-node":
(https://github.com/bitcoin/bitcoin/pull/30509#discussion_r1714041612)
ef57db8711c7fafb6c4f09b880fd4263bc6dc9dd: I also tested building this outside depends on macOS 14.6.1
(https://github.com/bitcoin/bitcoin/pull/30509#discussion_r1714041612)
ef57db8711c7fafb6c4f09b880fd4263bc6dc9dd: I also tested building this outside depends on macOS 14.6.1
💬 Sjors commented on pull request "multiprocess: Add -ipcbind option to bitcoin-node":
(https://github.com/bitcoin/bitcoin/pull/30509#discussion_r1714185762)
a8d4adfea4b50f452d80bc1e7ee322945d886c78: maybe test what happens when there's a second connection attempt to the same address.
(https://github.com/bitcoin/bitcoin/pull/30509#discussion_r1714185762)
a8d4adfea4b50f452d80bc1e7ee322945d886c78: maybe test what happens when there's a second connection attempt to the same address.
💬 Sjors commented on pull request "multiprocess: Add -ipcbind option to bitcoin-node":
(https://github.com/bitcoin/bitcoin/pull/30509#discussion_r1714199889)
52917ad4be1ca574543e854fdc154e88d65ce57f: nit `/*backlog=*/5`
(https://github.com/bitcoin/bitcoin/pull/30509#discussion_r1714199889)
52917ad4be1ca574543e854fdc154e88d65ce57f: nit `/*backlog=*/5`
💬 Sjors commented on pull request "multiprocess: Add -ipcbind option to bitcoin-node":
(https://github.com/bitcoin/bitcoin/pull/30509#discussion_r1714091657)
4137419b0170fa1e9ff9daacde57933f7c995b76: I have a hard hitting this, instead I tend to get a crash (on intel macOS):
For an existing path:
```
-ipcbind=unix://tmp
...
************************
EXCEPTION: NSt3__112system_errorE
Address already in use
bitcoin in AppInit()
```
Non-existing path that I don't have permission for:
```
-ipcbind=unix://root
...
************************
EXCEPTION: NSt3__112system_errorE
Read-only file system
bitcoin i
...
(https://github.com/bitcoin/bitcoin/pull/30509#discussion_r1714091657)
4137419b0170fa1e9ff9daacde57933f7c995b76: I have a hard hitting this, instead I tend to get a crash (on intel macOS):
For an existing path:
```
-ipcbind=unix://tmp
...
************************
EXCEPTION: NSt3__112system_errorE
Address already in use
bitcoin in AppInit()
```
Non-existing path that I don't have permission for:
```
-ipcbind=unix://root
...
************************
EXCEPTION: NSt3__112system_errorE
Read-only file system
bitcoin i
...
💬 Sjors commented on pull request "multiprocess: Add -ipcbind option to bitcoin-node":
(https://github.com/bitcoin/bitcoin/pull/30509#discussion_r1714085148)
4137419b0170fa1e9ff9daacde57933f7c995b76: In the mean time this has the nice side-effect of being able to run `src/bitcoin-gui -ipcbind=unix` (instead of `bitcoin-node`) and then connect to it with e.g. the miner demo `-ipcconnect=unix:gui.sock`.
See https://github.com/bitcoin/bitcoin/pull/30437#issuecomment-2226847354
(https://github.com/bitcoin/bitcoin/pull/30509#discussion_r1714085148)
4137419b0170fa1e9ff9daacde57933f7c995b76: In the mean time this has the nice side-effect of being able to run `src/bitcoin-gui -ipcbind=unix` (instead of `bitcoin-node`) and then connect to it with e.g. the miner demo `-ipcconnect=unix:gui.sock`.
See https://github.com/bitcoin/bitcoin/pull/30437#issuecomment-2226847354
💬 Sjors commented on pull request "multiprocess: Add -ipcbind option to bitcoin-node":
(https://github.com/bitcoin/bitcoin/pull/30509#discussion_r1714205805)
52917ad4be1ca574543e854fdc154e88d65ce57f: this could use a test.
Or maybe we can introduce something akin to `CNetAddr` for socket addresses? Fortunately this input can only be provided by the user, so we don't have to be overzealous about sanitising.
(https://github.com/bitcoin/bitcoin/pull/30509#discussion_r1714205805)
52917ad4be1ca574543e854fdc154e88d65ce57f: this could use a test.
Or maybe we can introduce something akin to `CNetAddr` for socket addresses? Fortunately this input can only be provided by the user, so we don't have to be overzealous about sanitising.
💬 achow101 commented on pull request "fuzz: improve `scriptpubkeyman` target":
(https://github.com/bitcoin/bitcoin/pull/30563#issuecomment-2284663808)
ACK 401cc4ec70d67ba2aa0e078d2fab214e1c40742c
(https://github.com/bitcoin/bitcoin/pull/30563#issuecomment-2284663808)
ACK 401cc4ec70d67ba2aa0e078d2fab214e1c40742c
💬 achow101 commented on pull request "wallet: fix blank legacy detection":
(https://github.com/bitcoin/bitcoin/pull/30621#discussion_r1714223454)
In 30f701317b4a674fea422766ac3ee179c9c6f554 "wallet: fix, detect blank legacy wallets in IsLegacy"
`IsLegacy()` is unnecessary, same as above.
(https://github.com/bitcoin/bitcoin/pull/30621#discussion_r1714223454)
In 30f701317b4a674fea422766ac3ee179c9c6f554 "wallet: fix, detect blank legacy wallets in IsLegacy"
`IsLegacy()` is unnecessary, same as above.
💬 achow101 commented on pull request "wallet: fix blank legacy detection":
(https://github.com/bitcoin/bitcoin/pull/30621#discussion_r1714223083)
In 30f701317b4a674fea422766ac3ee179c9c6f554 "wallet: fix, detect blank legacy wallets in IsLegacy"
`IsLegacy()` is unnecessary here now since `GetLegacyScriptPubKeyMan()` also checks for `WALLET_FLAG_DESCRIPTORS`.
(https://github.com/bitcoin/bitcoin/pull/30621#discussion_r1714223083)
In 30f701317b4a674fea422766ac3ee179c9c6f554 "wallet: fix, detect blank legacy wallets in IsLegacy"
`IsLegacy()` is unnecessary here now since `GetLegacyScriptPubKeyMan()` also checks for `WALLET_FLAG_DESCRIPTORS`.
💬 achow101 commented on pull request "net: Clarify that m_addr_local is only set once":
(https://github.com/bitcoin/bitcoin/pull/30617#issuecomment-2284691570)
ACK fa6fe432075df5e0eceb1ccd85038159cc820ccc
(https://github.com/bitcoin/bitcoin/pull/30617#issuecomment-2284691570)
ACK fa6fe432075df5e0eceb1ccd85038159cc820ccc
🤔 mzumsande reviewed a pull request: "net: Clarify that m_addr_local is only set once"
(https://github.com/bitcoin/bitcoin/pull/30617#pullrequestreview-2233791304)
utACK fa6fe432075df5e0eceb1ccd85038159cc820ccc
(https://github.com/bitcoin/bitcoin/pull/30617#pullrequestreview-2233791304)
utACK fa6fe432075df5e0eceb1ccd85038159cc820ccc
✅ achow101 closed an issue: "scriptpubkeyman fuzz target TopUp is slow (2/N)"
(https://github.com/bitcoin/bitcoin/issues/30541)
(https://github.com/bitcoin/bitcoin/issues/30541)
🚀 achow101 merged a pull request: "fuzz: improve `scriptpubkeyman` target"
(https://github.com/bitcoin/bitcoin/pull/30563)
(https://github.com/bitcoin/bitcoin/pull/30563)
💬 achow101 commented on pull request "contrib: support reading XORed blocks in linearize-data.py script":
(https://github.com/bitcoin/bitcoin/pull/30607#issuecomment-2284704597)
ACK 77ff0ec1f185b818b30877de2bedc1750319e6c4
(https://github.com/bitcoin/bitcoin/pull/30607#issuecomment-2284704597)
ACK 77ff0ec1f185b818b30877de2bedc1750319e6c4
💬 maflcko commented on issue "contrib: Automation for Bitcoin Full Node Deployment":
(https://github.com/bitcoin/bitcoin/issues/30638#issuecomment-2284705416)
> **Ansible/Monitoring:** I agree, tooling preferences/ideologies are a thing now, I am agnostic, I adapt, my questions:
>
> * What tooling do you, Bitcoin core devs, and community support for system provisioning?
>
> * What tooling for monitoring?
Bitcoin Core is also agnostic, because it should work with any monitoring or provisioning solution out there out of the box with no further changes needed. (There are many out there, including graphana, or self-written ones)
The sa
...
(https://github.com/bitcoin/bitcoin/issues/30638#issuecomment-2284705416)
> **Ansible/Monitoring:** I agree, tooling preferences/ideologies are a thing now, I am agnostic, I adapt, my questions:
>
> * What tooling do you, Bitcoin core devs, and community support for system provisioning?
>
> * What tooling for monitoring?
Bitcoin Core is also agnostic, because it should work with any monitoring or provisioning solution out there out of the box with no further changes needed. (There are many out there, including graphana, or self-written ones)
The sa
...