search

Start9Labs / bitcoind-startos

wrapper for building bitcoind.s9pk
Other
14 stars 20 forks source link

Add a 'Connections' property #110

Closed biotic21 closed 9 months ago

biotic21 commented 1 year ago

Add a new property to display the number of peers connected, including inbound and outbound.

Code not tested, so creating a draft PR as requested here.

chrisguida commented 1 year ago

Thanks for the PR! Let us know where you're getting stuck testing and we'll get you sorted out :)

kn0wmad commented 1 year ago

Add a new property to display the number of peers connected, including inbound and outbound.

Code not tested, so creating a draft PR as requested here.

Have you had a chance to test this?

biotic21 commented 11 months ago

I apologize for the very long delay on this. I wasn't originally intending to build or test, as I had no experience with any of this... including Rust, Docker, Make, Typescript, and on and on. I only created the draft PR after being requested to.

Anyway, I finally got the motivation to learn and was able to spin up an Ubuntu server on a VM to build the .s9pk and then load it onto a StartOS VM. The new property worked as expected! I've attached a screenshot below showing the property value in the Web UI compared to running the bitcoin-cli getnetworkinfo command in the service's container.

The only change I had to make was to the root project's Makefile (unrelated to this PR):

  1. Replaced all instances of embassy-cli and embassy-sdk with start-cli and start-sdk.
  2. Replaced both instances of ~/.embassy/config.yaml with ./start9/config.yaml. I don't know if this one made any difference, since it didn't affect me working through the hello-world-startos demo. But I did inspect electrs-startos to see how services made RPC calls to the Bitcoin Core service and noticed that this file was moved.

Outbound and Inbound

kn0wmad commented 11 months ago

This looks awesome, is it ready for review? Happy to test

biotic21 commented 11 months ago

Marked as ready for review. This is my first PR (ever), so feel free to change anything or let me know what I should change or do differently in the future.

kn0wmad commented 10 months ago

Sorry for the delay - I'm testing this and I don't see the Connections Property at all. Is it possible you forgot to commit something? Master will also need to be merged in. I also see your screenshot shows 0.0GiB disk usage, which is surely wrong.

image

biotic21 commented 10 months ago

Thanks for trying this out and sorry that there was an issue. I've merged the latest from the Start9Lab:master to my property-test branch, rebuilt with make, and retested by sideloading the bitcoind.s9pk package file to update the Bitcoin Core service. I can see the Connections property, and I've confirmed that what is in my property-test branch is exactly what I built and tested. (Before, as I noted, I didn't commit some simple changes in the Makefile to update embassy-cli and embassy-sdk to start-cli and start-sdk, just to get make to work. Perhaps this was the problem.)

Btw, the 0.0GiB disk usage is because I sideloaded the .s9pk package file I created onto a VM with a freshly installed StartOS. I did not update a Bitcoin Core service on an existing StartOS machine. So it was still syncing headers in the VM when I took that screenshot. I forced the inbound connection by adding the VM node's peer address to my main bitcoin node.

kn0wmad commented 9 months ago

Merging and manually fixing the Makefile issue as @biotic21 has become unresponsive. PR inbound

biotic21 commented 8 months ago

Thank you all for the help getting this completed!

Sorry that I didn't realize you were waiting for me to respond to something. I'm not sure I'm knowledgeable enough about the ins and outs of the Makefile anyway to be of much help. But thanks again!

kn0wmad commented 8 months ago

No worries, we dropped the ball for a while as well. Thank you very much for the PR