-
Notifications
You must be signed in to change notification settings - Fork 28
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
Adding a Dockerfile #30
Conversation
@j-berman I have no clue what I'm doing (smooth brain here), but I'm trying to wire this up to an existing monerod service with docker compose....annnndd... I think I got it (to turn on)!
|
Okay, was able to get this working with MyMonero over the internets (!!!) with Seth's monerod docker image and traefik/dns provider using (not including a few items like traefik/dns service setup):
@j-berman I think it would be good to add jq to the image, then you shell into the container using: and then from within the container: |
Dockerfile
Outdated
# expose commands `monero-lws-daemon` and `monero-lws-admin` via PATH | ||
ENV PATH "$PATH:/monero-lws/build/src" | ||
|
||
ENTRYPOINT ["monero-lws-daemon"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Data persistence issue- I'm not a Docker pro, but I think we want to add something like this after line 40 because the container is not actually persisting data when you specify a volume on the host/container (as specified in your startup command)
CMD ["--db-path=/home/monero-lws/.bitmonero/light_wallet_server"]
Basically, if you don't add this line as part of your docker/docker-compose startup command, i think the container defaults to putting the .mdb files in some root access directory and they don't persist in the specified docker volume. I think adding the CMD line makes more sense than telling people to specify it manually.
Dockerfile
Outdated
&& apt-get update \ | ||
&& apt-get upgrade -y -q \ | ||
&& apt-get install -y -q git build-essential cmake pkg-config libssl-dev libzmq3-dev libunbound-dev libsodium-dev libunwind8-dev \ | ||
liblzma-dev libreadline6-dev libldns-dev libexpat1-dev libpgm-dev qttools5-dev-tools libhidapi-dev libusb-1.0-0-dev \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add jq
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
jq
is not needed to run, but is useful for the bash commands listed in the documentation. I think it makes more sense to omit it as a default installed component, but I am also less familiar with docker norms.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking a good next step for this PR would be basing the image on Alpine to reduce size and complexity, as well as moving to a multi-step build where the built binaries (admin and the daemon?) are essentially copy/pasted into a subsequent build process that only adds 'necessary' packages for running and using these binaries- this is where I would probably put jq...
I'm not super familiar with LWS, but if there aren't any RPC methods for admin stuff and jq is the easiest current way to do these maintenance tasks (it appears to be the documented way), then I would probably support adding it in. Otherwise, everyone that reads the current documentation will have to manually install it within the running container every time it restarts... and they probably will do that.
There's a P2Pool Dockerfile that basically does this exact multi-step build process (including adding jq at the end): https://github.com/WeebDataHoarder/p2pool-compose/blob/master/docker/p2pool/Dockerfile in case you wanto to check it out.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd definitely recommend adding jq in build if it's at all necessary to normal operations and admin functions for LWS (which it seems to be), as installing packages after build is usually quite difficult as the running user in the container should not be root or have sudo permissions.
@CryptoGrampy thank you for going further with this :D ACK to all your comments, will take another pass soon. On traefik, I've never used it before but with a quick pass, it looks like the architecture you've got set up is an internet-facing load balancer which functions as a proxy to a private network, where 2 important notes: If hazy on the above, I think it would be good to include a disclaimer that for people trying this setup out, they should only use test view key until we have definitive answers. I'll be able to help out with this soon, but while we might see some traffic until we have solid answers, I think a disclaimer would be good to include (unless you've got the answers already :D ) |
If anyone is able to get this running through reading the full PR/comments, I hope they don't put their life savings on it :-) That would most certainly not be a good idea, so anyone reading this... we're TESTING here! But you're correct- I have Traefik set up as a reverse proxy on a home server- it intercepts any external network traffic on 443/80 and sends it where it needs to go (just a few docker services). I believe with my the current setup, the only way to access the LWS server is through the lws.server.com that i've specified and any connection attempt I'm making over :80 seems to be getting upgraded to :443 in the browser with the proper LE cert. I don't seem to be able to connect to LWS from within my LAN with this config either. I am also connecting to the server from outside my network without any hiccups. I'll pull MyMonero and attempt running it in dev mode and watch the network traffic (initial requests in particular) to LWS. BTW, I'm running a setup very similar to Seth: https://github.com/sethforprivacy/self-hosted-services/blob/main/docker-compose.yml |
Just want to confirm (but I could certainly be wrong!) that when I specify lws.myserver.com in the MyMonero app on Linux Desktop and then create a new wallet, the first post request that contains |
What wallet are you using? It looks like MyMonero? There was some funkiness with that wallet and https, but I haven't checked the last release (or few) to see how the situation changed. |
I'm gonna double check the code for all different MyMonero clients (whatever's in the latest Android, iOS and desktop repos) to see what they're doing when I get the chance EDIT: obviously the code might not be what's in the officially released versions, but deterministic builds + checking signed releases would be the next step from there |
Using the latest 1.2.4 MyMonero AppImage on Linux |
I have some ideas on how to make this a multi-stage build which should reduce the final image size drastically. It will require basically a total rewrite, but in the meantime the Dockerfile looks like a great start (pending me having a chance to test it personally). Would you rather I make recommendations for a multi-stage build here, or wait and open a PR once this is merged? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My biggest recommendation pending a multi-stage build is to switch to a non-root user before running the entry point, unless absolutely necessary.
For an example, see L127-131:
https://github.com/sethforprivacy/simple-monerod-docker/blob/main/Dockerfile#L127
@sethforprivacy I'm thinking may as well wait to put something out until you get a go at it. Considering people will have persistent volumes stored somewhere as a result of whatever gets merged into the repo, it makes sense to try and get the volume stored in the optimal spot in the right way from the get-go. Can feel free to just submit a new PR starting from scratch, nbd :) |
This is an amazing base and knocks out all of the hard work for the Dockerfile, thankful I don't need to start from scratch now! I'll copy-paste from here, give credit, and open a new PR after some testing later today. Thanks so much for the work on this, @j-berman! |
Initial take on a more optimized Dockerfile is here: https://github.com/sethforprivacy/monero-lws/blob/develop/Dockerfile This is probably as small and clean as we can make it until static compilation is possible for monero-lws. I reached out to @vtnerd and @j-berman about static compilation directly, but that's the biggest missing piece for a better Dockerfile at this point. @j-berman -- feel free to either snag what you like from mine and roll it into your PR (I don't care about attribution to mine) or I can open a PR after I test mine a bit more. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Started from @sethforprivacy's Dockerfile in the comment above, and hacked my way to linking a static build of monerod
to monero-lws
. I hope this can serve as a springboard for someone with stronger "make-fu" than me to see where things can be cut down/improved.
The final image size is 241mb on my machine.
Leaving comments on some areas I'm certain can probably be done cleaner and probably "more correctly" (i.e. where I "hacked").
&& cd /monero && nice -n 19 ionice -c2 -n7 make -j${NPROC:-$(nproc)} -C build/release daemon | ||
|
||
# TODO: remove the need to manually make this static liblmdb_lib.a | ||
RUN cd /monero/build/release/src/lmdb && make && cd /monero |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixes:
CMake Error at CMakeLists.txt:242 (message):
Unable to find required Monero library lmdb_lib
${Boost_PROGRAM_OPTIONS_LIBRARY} | ||
${Boost_FILESYSTEM_LIBRARY} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixes compiler error #7 :)
These changes look great, glad someone who actually understands make/cmake could figure this one out 😅 Will give it a try as I have time. |
For anyone testing this out, I'm already finding some bad privacy behavior with the MyMonero app- please just use this for testing only. mymonero/mymonero-app-js#458 |
Another similarly bad bug on Android specifically -- mymonero/mymonero-android-js#83 Only use testing wallets without funds when testing LWS until these serious issues are resolved. |
Tested without issues on two separate hosts, looks great and greatly reduces the size of the image! Excellent work on this. |
Once this is done, tested, and merged I will PR Github Actions for automatically building and publishing Docker images using the Dockerfile. Will pull heavily from here so that end-users won't have to build from source. If that is undesired @vtnerd let me know and I can keep a fork up that publishes under my own user instead. |
Looks like MyMonero has resolved the privacy issues in their desktop and Android wallets (not sure on iOS) but haven't tested myself... Is it time to revisit getting this image optimized/published @sethforprivacy @j-berman ? |
Fees are still fingerprintable |
CMakeLists.txt
Outdated
if(STATIC) | ||
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -DZMQ_STATIC") | ||
endif() | ||
find_path(ZMQ_INCLUDE_PATH zmq.h) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand these changes here, why not use the same stuff from the daemon? And why the extra libraries, are there linker errors in Ubuntu (this is reference to protolib, etc.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As a follow-up I read the above comment, but it's not making any sense. The existing code should be pulling in the same path ... ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Credit to @TheCharlatan for cleaning it up (and doing the static build right) in the latest 2 commits :)
FYI I could build for ARM64 arch using this PR, see my branch: Using this Dockerfile on top of |
@vdo, The problem might be that I typically build on Gentoo against libraries that have been built with |
@vtnerd I just tested this against the latest MyMonero and it is working flawlessly. Anything else needed before we can get this merged? I'd like to setup automatic build/push via Github CI once it's merged in so we can pull images from Docker Hub/GHCR 😄 |
Dockerfile
Outdated
ARG MONERO_BRANCH=v0.17.3.0 | ||
ARG MONERO_COMMIT_HASH=ab18fea3500841fc312630d49ed6840b3aedb34d |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably bump this to v0.17.3.2
and 424e4de16b98506170db7b0d7d87a79ccf541744
before merging.
Might make sense to set this to v0.18 before merging? Since there were some unbound related changes. |
@j-berman, version is |
@sethforprivacy the problem is this PR is targeted for the 0.17 branch, so it doesn't make any sense to do that. It's probably easiest to merge this now, then do some sort of cherry-pick into master/develop with a change to 0.18, then do an immediate branch for 0.18. Although I'm not really sure how to leave the master/develop branches with this patch because it's better suited only in a release branch. |
That's fine, it's a simple 2-line PR to bump to v0.18.0.0 in the Dockerfile once monero-lws as a whole is ready for v0.18.0.0. |
@sethforprivacy there's a new branch with the relevant |
I'm getting errors when building that branch: |
@vdo I think a new issue should be opened for this. It looks like some issue with linking libunbound statically. Presumably other instances used a shared library, which picked up the relevant dependencies of libunbound. |
You have to compile static unbound manually, like this: https://github.com/sethforprivacy/simple-monerod-docker/pull/61/files |
@vdo does |
In summary, use a |
@vtnerd We did have an issue open but it was closed: monero-project/monero#8439 (comment) Do you have an idea of how to fix this upstream properly? |
should do the trick on most platforms (assuming
Howard probably dropped because adding this to the cmake file might be a different source of pain when building (because |
Scratch all that above, that is pulling information from the dynamic library. The |
@vtnerd Just to be clear, this happens when building using the Dockerfile. I will open a new issue for clarity. |
See PR #39 |
Requested on the reddit
Instructions
If you're already running
monerod
on the same machine you'd like to hostmonero-lws
, you can use this Dockerfile to spin up amonero-lws
docker container by running the following commands on Linux:On Linux,
--network="host"
will expose the host machine's network to the Docker container. This is what allows the Docker container to retrieve data from the runningmonerod
instance on the machine. More on this here.On Windows and Mac, the
docker run
command I believe should be this instead (untested):There should be a way to write a consistent docker run command for all platforms as of Docker v20.10, but couldn't get it working so moving on. Certain there's room for cleanup in these commands by docker afficionados.