Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ItchySats App #1149

Merged
merged 3 commits into from
Dec 30, 2021
Merged

Add ItchySats App #1149

merged 3 commits into from
Dec 30, 2021

Conversation

klochowicz
Copy link
Contributor

@klochowicz klochowicz commented Dec 7, 2021

App Submission

App name

ItchySats

Version

v0.3.0

One line description of the app

Peer-2-peer derivatives on Bitcoin

Summary of the app

ItchySats enables peer-2-peer CFD trading on Bitcoin using DLCs (discreet log contracts). No account needed, no trusted third-party - just you and your keys.

This is beta software. We tested it on test- and mainnet, but there are no guarantees that it will always behave as expected.
Please be mindful with how much money you trust the application with.
CFDs trading is inherently risky, be sure to read up on it before using this application.

That said: This is pretty awesome, go nuts!

  1. Fund the ItchySats wallet
  2. Open a position
  3. Watch the price go up
  4. Profit

Limitations of the mainnet beta:

  1. You can only open long positions at the moment
  2. Minimum position quantity is $100, maximum $1000
  3. CFDs period ends after 7 days - perpetual positions are in the making :)
  4. The leverage is fixed at 2

We are woking hard on perpetual positions and allowing sell positions.
Update to be expected soon!

Developer name

ItchySats

Developer website

https://itchysats.network

Source code link

https://github.com/itchysats/itchysats

Support link

https://github.com/itchysats/itchysats/issues

Requires

  • Bitcoin Core
  • Electrum server
  • LND

256x256 SVG icon

Itchilogo.svg.zip
(as zip as svgur.com is over quota and does not work now...)

Gallery images

(3 screenshots with captions)

1
"Start it. Trade! It’s that simple"

2
"Bitcoin CFDs without a middle-man"

3
"No accounts, just you and your keys!"

I have tested my app on:

@klochowicz klochowicz marked this pull request as draft December 7, 2021 06:10
@1010Tom

This comment has been minimized.

- ${APP_DATA_DIR}/data:/data
command:
- --maker=mainnet.itchysats.network:9999
- --maker-id=7e35e34801e766a6a29ecb9e22810ea4e3476c2b37bf75882edf94a68b1d9607
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this id used for?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's the maker's public key which is needed to have an encrypted connection with the maker behind mainnet.itchysats.network:9999.

@klochowicz
Copy link
Contributor Author

I've force-pushed after rebasing against the recent umbrel master to avoid merge conflicts (no other changes).

Copy link
Member

@lukechilds lukechilds left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great guys, some minor tweaks required.

Btw, I'm unable to test this on my mainnet node, when I try to install I get:

umbrel@umbrel-mainnet:~/umbrel $ ./scripts/app install itchysats
Setting up data dir for app itchysats...
sending incremental file list

sent 126 bytes  received 13 bytes  278.00 bytes/sec
total size is 654  speedup is 4.71
Pulling images for app itchysats...
Pulling web ... done
Starting app itchysats...
Creating itchysats_web_1 ... error

ERROR: for itchysats_web_1  Cannot start service web: OCI runtime create failed: invalid mount {Destination:data Type:bind Source:/var/lib/docker/volumes/3373facda23457bc501bfff215d91789cc1ee4f20c043d4b929bed56fc691305/_data Options:[rbind]}: mount destination data not absolute: unknown

ERROR: for web  Cannot start service web: OCI runtime create failed: invalid mount {Destination:data Type:bind Source:/var/lib/docker/volumes/3373facda23457bc501bfff215d91789cc1ee4f20c043d4b929bed56fc691305/_data Options:[rbind]}: mount destination data not absolute: unknown
ERROR: Encountered errors while bringing up the project.

I was however able to succesfully install on my testnet node, so seems like I'm hitting some strange Docker bug and not an issue with your app specifically. Just checking you were able to install ok?

Also are you able to allow edits from maintainers so I can push to this PR branch?

Edit: Also one other thing, I noticed there's no authentication, so currently any user on the local network, or anyone that gets access to the users remote Tor address, can open positions and withdraw funds from the users account. We pass in an $APP_PASSWORD env var that is a 128 bit random hex string unique to each app which apps can optionally use to add authentication. The plain text password will be show to the user in the dashboard for them to easily copy/paste it in when opening the app. I think ItchySats should make use of this, or alternatively implement some other type of custom auth. In it's current state it's quite risky, e.g a user that shares a screenshot of ItchySats on Twitter when accessing over Tor (and therefore accidentily leaking their Tor hidden service address) could have their account drained.

"developer": "ItchySats",
"website": "https://itchysats.network",
"dependencies": [
"bitcoind",
Copy link
Member

@lukechilds lukechilds Dec 23, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't directly depend on bitcoind right? Only indirectly via Electrum? If so we can remove this dependency.

Suggested change
"bitcoind",

Copy link
Contributor

@1010Tom 1010Tom Dec 24, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


# itchysats Hidden Service
HiddenServiceDir /var/lib/tor/app-itchysats
HiddenServicePort 80 <app-itchysats-ip>:<app-itchysats-port>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be the port on the container not on the host.

Suggested change
HiddenServicePort 80 <app-itchysats-ip>:<app-itchysats-port>
HiddenServicePort 80 <app-itchysats-ip>:8000

Copy link
Contributor

@1010Tom 1010Tom Dec 24, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -218,6 +218,8 @@ APP_SYNCTHING_PORT="8384"
APP_SYNCTHING_SYNC_PORT="22000"
APP_UPTIME_KUMA_PORT="8385"
APP_UPTIME_KUMA_IP="10.21.21.62"
APP_ITCHYSATS_IP="10.21.21.63"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is an IP collision with $APP_RIDE_THE_LIGHTNING_BOLTZ_IP in master here.

Suggested change
APP_ITCHYSATS_IP="10.21.21.63"
APP_ITCHYSATS_IP="10.21.21.64"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lukechilds
Copy link
Member

Also where's the best place to DM you guys for some more general questions on ItchySats?

@lukechilds
Copy link
Member

Also what license is the ItchySats codebase licensed under? Currently I don't see a license file on your repo.

@lukechilds
Copy link
Member

Seems like the volume issue is that this line https://github.com/itchysats/itchysats/blob/61c7bcbb322ab4adc43c199210a423287862a8d1/Dockerfile#L13 should be VOLUME /data (absolute instead of relative path).

Fixed here: get10101/itchysats#971

@lukechilds
Copy link
Member

Hitting this trying to open a position:

Screenshot 2021-12-24 at 18 01 39

Is that expected currently if the app is still in testing and you don't have a lot of funds on the maker node or is something wrong?

Also seeing lots of stuff like this spamming the logs:

web_1  | 2021-12-24 11:09:36  WARN daemon::tokio_ext: Task failed: Failed to GET https://h00.ooo/x/BitMEX/BXBT/2021-12-26T12:00:00.price?n=20: error sending request for url (https://h00.ooo/x/BitMEX/BXBT/2021-12-26T12:00:00.price?n=20): error trying to connect: dns error: failed to lookup a
ddress information: Temporary failure in name resolution: error trying to connect: dns error: failed to lookup address information: Temporary failure in name resolution: dns error: failed to lookup address information: Temporary failure in name resolution: failed to lookup address informati
on: Temporary failure in name resolution
web_1  | 2021-12-24 11:09:36  WARN daemon::tokio_ext: Task failed: Failed to GET https://h00.ooo/x/BitMEX/BXBT/2021-12-31T03:00:00.price?n=20: error sending request for url (https://h00.ooo/x/BitMEX/BXBT/2021-12-31T03:00:00.price?n=20): error trying to connect: dns error: failed to lookup a
ddress information: Temporary failure in name resolution: error trying to connect: dns error: failed to lookup address information: Temporary failure in name resolution: dns error: failed to lookup address information: Temporary failure in name resolution: failed to lookup address informati
on: Temporary failure in name resolution
web_1  | 2021-12-24 11:09:36  WARN daemon::tokio_ext: Task failed: Failed to GET https://h00.ooo/x/BitMEX/BXBT/2021-12-26T15:00:00.price?n=20: error sending request for url (https://h00.ooo/x/BitMEX/BXBT/2021-12-26T15:00:00.price?n=20): error trying to connect: dns error: failed to lookup a
ddress information: Temporary failure in name resolution: error trying to connect: dns error: failed to lookup address information: Temporary failure in name resolution: dns error: failed to lookup address information: Temporary failure in name resolution: failed to lookup address informati
on: Temporary failure in name resolution
web_1  | 2021-12-24 11:09:36 DEBUG daemon::oracle: Fetching announcement for /x/BitMEX/BXBT/2021-12-30T11:00:00.price?n=20
web_1  | 2021-12-24 11:09:36 DEBUG daemon::oracle: Fetching announcement for /x/BitMEX/BXBT/2021-12-29T12:00:00.price?n=20
web_1  | 2021-12-24 11:09:36 DEBUG daemon::oracle: Fetching announcement for /x/BitMEX/BXBT/2021-12-29T13:00:00.price?n=20
web_1  | 2021-12-24 11:09:36 DEBUG daemon::oracle: Fetching announcement for /x/BitMEX/BXBT/2021-12-29T14:00:00.price?n=20

@1010Tom
Copy link
Contributor

1010Tom commented Dec 24, 2021

Hi @lukechilds

thanks for reviewing our PR. I've fixed your comments in separate commits.

Regarding the authentication: that's a good point. We do have a ticket for this get10101/itchysats#609 . Do you want this in the first release or is ok if we postpone it for the next release?

Also where's the best place to DM you guys for some more general questions on ItchySats?

You can find us on matrix: https://matrix.to/#/#comit-hermes:matrix.org or reach us via telegram https://t.me/joinchat/ULycH50PLV1jOTI0

Is that expected currently if the app is still in testing and you don't have a lot of funds on the maker node or is something wrong?

This looks correct and maybe we should better displayed the "No Liquidity" error a warning and not as an error: We are testing with safe amounts at the moment: $100-$1000 and the taker will create a new offer after a few seconds.

The error below (Announcement not found) was not expected :/ It looks like there was a DNS error on your machine?

Also what license is the ItchySats codebase licensed under? Currently I don't see a license file on your repo.

Let me get back to you on that. Should be an open source license.

p.s. merry christmas :)

@bonomat
Copy link
Contributor

bonomat commented Dec 27, 2021

Hi @lukechilds, thanks for giving ItchySats a try.

We have 3 versions running. As far as I remember, we started with version v0.4.8 but then upgraded to v0.4.9. All three version were running without any issues.

Regarding the DNS errors: it looks weird but I think we have seen this before.
Can you open a ticket on our repository and attach some more logs please? Maybe there is something useful in there before the lines you shared above.
Also, it would be good to know if you had a "clean" umbrel node or if you changed something in the dns settings.

Regarding authentication: I would opt for a nginx proxy using APP_PASSWORD for now until we get get10101/itchysats#609 implemented. This should be faster and we can then ship the authentication with a new release.

Also are you able to allow edits from maintainers so I can push to this PR branch?

@klochowicz : please enable edits for maintainers

bors bot added a commit to get10101/itchysats that referenced this pull request Dec 27, 2021
971: Fix volume path r=bonomat a=lukechilds

Using the relative path like this instead of an abolute path when trying to start a container I just get the error:

```
ERROR: for itchysats_web_1  Cannot start service web: OCI runtime create failed: invalid mount {Destination:data Type:bind Source:/var/lib/docker/volumes/3373facda23457bc501bfff215d91789cc1ee4f20c043d4b929bed56fc691305/_data Options:[rbind]}: mount destination data not absolute: unknown

ERROR: for web  Cannot start service web: OCI runtime create failed: invalid mount {Destination:data Type:bind Source:/var/lib/docker/volumes/3373facda23457bc501bfff215d91789cc1ee4f20c043d4b929bed56fc691305/_data Options:[rbind]}: mount destination data not absolute: unknown
ERROR: Encountered errors while bringing up the project.
```

It seems like only certain versions of Docker fail without this, I can get your current Docker image to run on some machines but not others without this change. However with this change it runs correctly on all of my machines.

Related: getumbrel/umbrel#1149 (review)

Co-authored-by: Luke Childs <[email protected]>
@lukechilds
Copy link
Member

p.s. merry christmas :)

Thanks! You too, hope you guys had a great Christmas!

Regarding the authentication: that's a good point. We do have a ticket for this get10101/itchysats#609 . Do you want this in the first release or is ok if we postpone it for the next release?
-- @itchymax

To be honest it's up to you guys but I'd strongly advise adding some sort of basic auth to the initial release. Or atleast showing a big warning that this is an initial beta release with no auth if you decide not to.

This sounds like a great compromise though:

Regarding authentication: I would opt for a nginx proxy using APP_PASSWORD for now until we get get10101/itchysats#609 implemented. This should be faster and we can then ship the authentication with a new release.
-- @bonomat

Re DNS

The error below (Announcement not found) was not expected :/ It looks like there was a DNS error on your machine?
-- @itchymax

Regarding the DNS errors: it looks weird but I think we have seen this before.
Can you open a ticket on our repository and attach some more logs please? Maybe there is something useful in there >before the lines you shared above.
Also, it would be good to know if you had a "clean" umbrel node or if you changed something in the dns settings.
-- @bonomat

I haven't changed any DNS settings. I just remembered some users had some issues with the mempool app that were resolved by changing Docker DNS settings. Sounds like this could be related. Let me try the proposed fix and see if that resolves the ItchySats issue.

@lukechilds
Copy link
Member

Also re this:

We have 3 versions running. As far as I remember, we started with version v0.4.8 but then upgraded to v0.4.9. All three version were running without any issues.

Assuming that version is maker specific? Cos this PR currently includes ghcr.io/itchysats/itchysats/taker:0.3.0.

@bonomat
Copy link
Contributor

bonomat commented Dec 27, 2021

Also re this:

We have 3 versions running. As far as I remember, we started with version v0.4.8 but then upgraded to v0.4.9. All three version were running without any issues.

Assuming that version is maker specific? Cos this PR currently includes ghcr.io/itchysats/itchysats/taker:0.3.0.

sorry, I meant Umbrel version.

@lukechilds
Copy link
Member

sorry, I meant Umbrel version.

Ah got it.

Btw just seen you have ghcr.io/itchysats/itchysats/taker:0.3.2 published, we should update this PR to that, right?

@lukechilds
Copy link
Member

Ok so this #1076 (comment) fixed the DNS issue for me, so seems unrelated to ItchySats and some sort of internal Umbrel DNS issue. Looking into it.

Screenshot 2021-12-27 at 18 28 59

Opened a position! Exciting stuff 🚀

@bonomat
Copy link
Contributor

bonomat commented Dec 27, 2021

Re

Btw just seen you have ghcr.io/itchysats/itchysats/taker:0.3.2 published, we should update this PR to that, right?

yes indeed: @itchymax / @klochowicz please upgrade to 0.3.2.

Ok so this #1076 (comment) fixed the DNS issue for me, so seems unrelated to ItchySats and some sort of internal Umbrel DNS issue. Looking into it.

Screenshot 2021-12-27 at 18 28 59

Opened a position! Exciting stuff 🚀

Great that it worked and let's go Bitcoin ! 🚀

@lukechilds
Copy link
Member

Also guys, looks like you are generating the seed randomly on first start. This is fine, but we also expose $APP_SEED which is a hex hex string deterministically derived from the users Umbrel seed. If you wanted to you could make use of this. It would mean that if a user's Umbrel broke and they setup a new one and imported their previous Umbrel seed, the ItchySats app would then derive the same wallet as the previous install and reover the previous funds.

@1010Tom
Copy link
Contributor

1010Tom commented Dec 28, 2021

Also guys, looks like you are generating the seed randomly on first start. This is fine, but we also expose $APP_SEED which is a hex hex string deterministically derived from the users Umbrel seed. If you wanted to you could make use of this. It would mean that if a user's Umbrel broke and they setup a new one and imported their previous Umbrel seed, the ItchySats app would then derive the same wallet as the previous install and reover the previous funds.

That's cool. I've created a ticket for this: get10101/itchysats#978

@lukechilds
Copy link
Member

lukechilds commented Dec 28, 2021

@bonomat just FYI the DNS issue seems like a DNS issue on my network unrelated to ItchySats and Umbrel.

Viewing https://outcome.observer/h00.ooo/x/BitMEX/BXBT in my browser on my workstation on the same network as my Umbrel gives this error:
Screenshot 2021-12-28 at 17 49 22

Changing to a different network or using Tor Browser allows me to view the price feed correctly. You said you had reports of this issue before, maybe there is some sort of problem with the DNS configuration of the price feed server causing it to not work with some ISPs? Any ideas?

@scratchscratchscratchy
Copy link
Contributor

scratchscratchscratchy commented Dec 29, 2021

@lukechilds with upgrading to version 0.3.3 we restrict the API by basic auth (as discussed in comments above). Password is set to $APP_PASSWORD, username defaulted in registry to itchysats.

I just tested upgrading my umbrel and it worked fine displaying the password/username in the appstore.

I think with adding this we should be able to merge. Please let me know if there is anything else needed :)

1010Tom and others added 3 commits December 29, 2021 15:16
Includes using `$APP_PASSWORD` to restrict access to ItchySats within Umbrel by basic auth.
@klochowicz
Copy link
Contributor Author

@lukechilds @itchymax @bonomat Unfortunately I don't seem to be able to allow pushing to this branch as the PR was created from org repo instead of personal one. I can resubmit the PR from a personal github account to allow this, but I'm not sure if it's worth the effort?

@lukechilds
Copy link
Member

@klochowicz ah yes, just seen this isaacs/github#1681 that sucks.

Ok no worries, no need for a new PR, good to keep the history here, I can just submit a PR to your org repo if I need to do any changes.

@lukechilds
Copy link
Member

I think with adding this we should be able to merge. Please let me know if there is anything else needed :)

@scratchscratchscratchy awesome! Testing now!

Copy link
Member

@lukechilds lukechilds left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested and working great. Amazing stuff guys, really excited to ship this!

@lukechilds lukechilds merged commit 3addd45 into getumbrel:master Dec 30, 2021
@bonomat bonomat deleted the itchysats branch January 11, 2022 18:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
app submission A brand new app submission
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants