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 API functions to start and stop local Monero node [$300] #137

Closed
woodser opened this issue Oct 27, 2021 · 77 comments · Fixed by haveno-dex/haveno-ts#76
Closed

Add API functions to start and stop local Monero node [$300] #137

woodser opened this issue Oct 27, 2021 · 77 comments · Fixed by haveno-dex/haveno-ts#76
Assignees
Labels
a:api Related to the gRPC API 💰bounty There is a bounty on this issue is:feature Request for a new feature P3 normal priority

Comments

@woodser
Copy link
Contributor

woodser commented Oct 27, 2021

This issue requests adding new API functions to start and stop a local Monero node.

The following functions are requested as additions to HavenoDaemon.ts. Feedback is welcome.

API Function Return Description
havenod.startMoneroNode(rpcUsername: string, rpcPassword: string) void Start local monerod process, throw error if fails.
havenod.stopMoneroNode() void Stop local monerod process, throw error if fails.

How to implement

Start an internal monerod process using a constructor in MoneroDaemonRpc.java.

Refer to how monero-wallet-rpc instances are started and stopped in Haveno for reference as this pattern will be similar.

Follow these instructions to add and test new API functions end-to-end.

@erciccione erciccione added a:api Related to the gRPC API is:feature Request for a new feature P2 high priority labels Oct 27, 2021
@erciccione erciccione added this to To do in Build Haveno's API Oct 27, 2021
@woodser woodser added the help wanted Extra attention is needed label Oct 27, 2021
@erciccione erciccione changed the title Add API functions to start and stop local Monero node Add API functions to start and stop local Monero node [$400] Oct 29, 2021
@erciccione erciccione added 💰bounty There is a bounty on this issue and removed help wanted Extra attention is needed labels Oct 29, 2021
@erciccione erciccione added this to Open in Bounty program via automation Oct 29, 2021
@github-actions
Copy link

There is a bounty on this issue, the amount is in the title. The bounty will be awarded to the first person(s) who resolves this issue. Read the full conditions in the 'bounties.md' file. If you are starting to work on this issue, please write a comment here, so that we can assign the issue to you and avoid duplicated work.

@HashedMantisShrimp
Copy link

Hi guys, I would like to take on this bounty on my spare time, I have never programmed in Java but have some minor experience with other languages and am quite familiar with the Linux CLI.
As a matter of fact, I have already forked the project and am working on setting it up on my computer.

Could someone assign this bounty to me? Also, what is an acceptable delivery time-frame for a P2 issue of this sort?

@HashedMantisShrimp
Copy link

Hi guys, I would like to take on this bounty on my spare time, I have never programmed in Java but have some minor experience with other languages and am quite familiar with the Linux CLI. As a matter of fact, I have already forked the project and am working on setting it up on my computer.

Could someone assign this bounty to me? Also, what is an acceptable delivery time-frame for a P2 issue of this sort?

I should also mention that I'm having problems getting past the Deploy instructions on installing.md, specifically with make seednode.

Is this the place I should go to in order to clear my doubts and request help - Development discussions: Haveno Development (#haveno-dev:haveno.network) relayed on Libera (IRC) (#haveno-dev) ?

@woodser
Copy link
Contributor Author

woodser commented Oct 31, 2021

Excellent, I have assigned the issue to you.

I can help implement (or at least review) the constructor of MoneroDaemonRpc.java in monero-java that launches the daemon process. It should be nearly identical to the constructor in MoneroWalletRpc.java, and will need a corresponding test.

Would hope for a timeframe within the next month or two, as other API calls are being built.

@HashedMantisShrimp
Copy link

HashedMantisShrimp commented Oct 31, 2021

That timeframe sounds definitely reasonable! If I managed to finish this fast enough I'll jump onto other API bounties.

As for my issue, something went wrong when I was following this step

"Logs" for the errors found:

User@User:~/Documents/Projects/Monero/Haveno-Projs/haveno$ make seednode
./haveno-seednode
--baseCurrencyNetwork=XMR_STAGENET
--useLocalhostForP2P=true
--useDevPrivilegeKeys=true
--nodePort=2002
--appName=haveno-XMR_STAGENET_Seed_2002
make: ./haveno-seednode: Command not found
make: *** [Makefile:36: seednode] Error 127

Same error happened when I ran make deploy, except it didn't recognize the 'screen' command.

Then I tried this:

User@User:/Documents/Projects/Monero/Haveno-Projs/haveno/seednode$ make seednode
make: *** No rule to make target 'seednode'. Stop.
User@User:
/Documents/Projects/Monero/Haveno-Projs/haveno/seednode$ sh install_seednode_debian.sh
[] Bisq Seednode installation script
[
] Updating apt repo sources
[sudo] password for User:
Hit:1 http://lt.archive.ubuntu.com/ubuntu focal InRelease
Hit:2 http://lt.archive.ubuntu.com/ubuntu focal-updates InRelease
Hit:3 http://dl.google.com/linux/chrome/deb stable InRelease
Hit:4 https://download.docker.com/linux/ubuntu focal InRelease
Hit:5 http://ppa.launchpad.net/mkusb/ppa/ubuntu focal InRelease
Ign:6 http://ppa.launchpad.net/unit3/bfgminer/ubuntu focal InRelease
Get:7 http://security.ubuntu.com/ubuntu focal-security InRelease [114 kB]
Err:8 http://ppa.launchpad.net/unit3/bfgminer/ubuntu focal Release
404 Not Found [IP: 91.189.95.85 80]
Ign:9 https://github.com/luke-jr/bfgminer.git focal InRelease
Err:10 https://github.com/luke-jr/bfgminer.git focal Release
404 Not Found [IP: 140.82.121.4 443]
Reading package lists...
E: The repository 'http://ppa.launchpad.net/unit3/bfgminer/ubuntu focal Release' does not have a Release file.
E: The repository 'https://github.com/luke-jr/bfgminer.git focal Release' does not have a Release file.
User@User:~/Documents/Projects/Monero/Haveno-Projs/haveno/seednode$ sh install_seednode_debian.sh
[] Bisq Seednode installation script
[
] Updating apt repo sources
Hit:1 http://lt.archive.ubuntu.com/ubuntu focal InRelease
Hit:2 http://lt.archive.ubuntu.com/ubuntu focal-updates InRelease
Hit:3 http://ppa.launchpad.net/mkusb/ppa/ubuntu focal InRelease
Hit:4 http://dl.google.com/linux/chrome/deb stable InRelease
Hit:5 https://download.docker.com/linux/ubuntu focal InRelease
Ign:6 http://ppa.launchpad.net/unit3/bfgminer/ubuntu focal InRelease
Err:7 http://ppa.launchpad.net/unit3/bfgminer/ubuntu focal Release
404 Not Found [IP: 91.189.95.85 80]
Get:8 http://security.ubuntu.com/ubuntu focal-security InRelease [114 kB]
Ign:9 https://github.com/luke-jr/bfgminer.git focal InRelease
Err:10 https://github.com/luke-jr/bfgminer.git focal Release
404 Not Found [IP: 140.82.121.4 443]
Reading package lists...
E: The repository 'http://ppa.launchpad.net/unit3/bfgminer/ubuntu focal Release' does not have a Release file.
E: The repository 'https://github.com/luke-jr/bfgminer.git focal Release' does not have a Release file.

I also encountered a minor "error" during the first step when I ran make, should I paste that one as well?

@woodser
Copy link
Contributor Author

woodser commented Oct 31, 2021

@erciccione Something you can help with?

@erciccione
Copy link
Contributor

erciccione commented Oct 31, 2021

@HashedMantisShrimp what OS are you on? Looks like a local problem. Also from a quick check on that Error 127 there could be an issue with your $PATH.

Same error happened when I ran make deploy, except it didn't recognize the 'screen' command.

Make sure you have screen installed if you are trying make deploy, but if it doesn't work with make seednode i doubt will work anyway.

install_seednode_debian.sh

This is a script we inherit from Bisq, it's not really update to work with Haveno.

@HashedMantisShrimp
Copy link

HashedMantisShrimp commented Nov 1, 2021

Thank you, I will look into it! I was under the impression that this was a Haveno specific problem (because of the ./haveno-seednode command not found), since it's make related shouldnt be too hard to fix.
Am using an Ubuntu 20.04 btw.

@woodser
Copy link
Contributor Author

woodser commented Nov 1, 2021

The problem appears to be that the ./haveno-seednode executable has not been built. This suggests make didn't complete successfully. Do you see any errors when you run make?

@HashedMantisShrimp
Copy link

HashedMantisShrimp commented Nov 1, 2021

@woodser, that's the same conclusion I came to and was about to edit my other comment. Yes I saw an error when I initially ran make but disregarded it, now I can't find the terminal window with the history. I'll start over so I can investigate the first error (Just realized this is what you suggested, nvm lol).
I'll ping one of you if I can't resolve the issue with make

EDIT FINAL - make error log:

mkdir -p .localnet
./scripts/xmr_btc_deps.sh
monero-bins-haveno-linux.tar.gz: OK
-> Monero binaries downloaded and verified
tar: Ignoring unknown extended header keyword 'LIBARCHIVE.xattr.security.selinux'
tar: Ignoring unknown extended header keyword 'LIBARCHIVE.xattr.security.selinux'
tar: Ignoring unknown extended header keyword 'LIBARCHIVE.xattr.security.selinux'
bitcoin-0.21.1-x86_64-linux-gnu.tar.gz: OK
-> Bitcoin binaries downloaded and verified
./gradlew build
Starting a Gradle Daemon (subsequent builds will be faster)

Configure project :
Pricenode: Skipping spot provider tests

FAILURE: Build failed with an exception.

  • What went wrong:
    A problem occurred configuring project ':desktop'.

Checksum failed for com.github.JesusMcCloud:jtorctl:6465e0b22b921344a065a6e82a5390ab2f9dac5ab293c38bd8a76baddead6492

  • Try:
    Run with --stacktrace option to get the stack trace. Run with --info or --debug option to get more log output. Run with --scan to get full insights.

  • Get more help at https://help.gradle.org

Deprecated Gradle features were used in this build, making it incompatible with Gradle 7.0.
Use '--warning-mode all' to show the individual deprecation warnings.
See https://docs.gradle.org/6.6.1/userguide/command_line_interface.html#sec:command_line_warnings

BUILD FAILED in 33s
make: *** [Makefile:18: build-haveno] Error 1

@erciccione
Copy link
Contributor

erciccione commented Nov 1, 2021

That seems to be related to the change of checksum (#127 #132). We are looking into it.

@HashedMantisShrimp
Copy link

Ah okay. I will wait for that to be resolved then. Or is it okay if I just run with the proposed change? I could edit the file locally and go with that.

@woodser
Copy link
Contributor Author

woodser commented Nov 1, 2021

You can try manually reverting the checksum to 389d61b1b5a85eb2f23c582c3913ede49f80c9f2b553e4762382c836270e57e5.

@HashedMantisShrimp
Copy link

That worked, thanks!

@woodser
Copy link
Contributor Author

woodser commented Nov 2, 2021

The new hash should work and the old hash will stop working after clearing your gradle cache with rm -rf ~/.gradle and building clean with make clean && make.

@HashedMantisShrimp
Copy link

Actually, my build seems to work without the new hash - prior to this I had deleted my ~/.gradle files and ran make clean && make, there was no error so I ran make a second time, just to be sure:

User@User:~/Documents/Projects/Monero/Haveno-Projs/haveno$ make
mkdir -p .localnet
./scripts/xmr_btc_deps.sh
monero-bins-haveno-linux.tar.gz: OK
-> Monero binaries downloaded and verified
tar: Ignoring unknown extended header keyword 'LIBARCHIVE.xattr.security.selinux'
tar: Ignoring unknown extended header keyword 'LIBARCHIVE.xattr.security.selinux'
tar: Ignoring unknown extended header keyword 'LIBARCHIVE.xattr.security.selinux'
bitcoin-0.21.1-x86_64-linux-gnu.tar.gz: OK
-> Bitcoin binaries downloaded and verified
./gradlew build

Configure project :
Pricenode: Skipping spot provider tests

Deprecated Gradle features were used in this build, making it incompatible with Gradle 7.0.
Use '--warning-mode all' to show the individual deprecation warnings.
See https://docs.gradle.org/6.6.1/userguide/command_line_interface.html#sec:command_line_warnings

BUILD SUCCESSFUL in 22s
118 actionable tasks: 5 executed, 113 up-to-date

User@User:~/Documents/Projects/Monero/Haveno-Projs/haveno$ cat gradle/witness/gradle-witness.gradle | grep -i com.github.JesusMcCloud:jtorctl
'com.github.JesusMcCloud:jtorctl:389d61b1b5a85eb2f23c582c3913ede49f80c9f2b553e4762382c836270e57e5',

Futhermore, all other make commands seem to work. If it's not that big a deal I'll keep using the old hash but I'll be sure to merge from your master branch before I submit my code.

@HashedMantisShrimp
Copy link

HashedMantisShrimp commented Nov 6, 2021

@woodser, were you guys aware of these errors? I followed these instructions to run the existing tests and found an error when using the docker command:

[2021-11-06 15:40:19.256][1][critical][main] [source/server/server.cc:113] error initializing configuration '/envoy.test.yaml': Unable to convert YAML as JSON:
[2021-11-06 15:40:19.256][1][info][main] [source/server/server.cc:815] exiting
Unable to convert YAML as JSON:

As well as an error when running npm start:

src/HavenoDaemon.tsx
Line 130:13: Expected an assignment or function call and instead saw an expression @typescript-eslint/no-unused-expressions

I attempted to remove 'false' on line 130 (leaving only the semi colon) but then got this:

haveno-ui-poc/src/HavenoDaemon.tsx
TypeScript error in /home/User/Documents/Projects/Monero/Haveno-Projs/haveno-ui-poc/src/HavenoDaemon.tsx(249,22):
Argument of type 'TradeInfo | undefined' is not assignable to parameter of type 'TradeInfo | PromiseLike'.
Type 'undefined' is not assignable to type 'TradeInfo | PromiseLike'. TS2345

247 |         if (err) reject(err);
248 |         else if (response.getFailureReason() && response.getFailureReason()!.getAvailabilityResult() !== AvailabilityResult.AVAILABLE) reject(response.getFailureReason()!.getDescription());

249 | else resolve(response.getTrade());
| ^
250 | });
251 | });
252 | }

This is just an FYI, I'll get back at debugging this later today.

EDIT:

Am I wasting my time trying to get Haveno UI PoC to run w/o errors? I think not because at the end of the day I will be running tests to ensure my code hasn't broken anything, but I'd like a second opinion on this.

EDIT 2:
Still haven't managed to fix it. Will keep it up whenever I find time in the coming week.

@woodser
Copy link
Contributor Author

woodser commented Nov 9, 2021

Regarding the jtorctl, ./gradlew build --refresh-dependencies will cause the latest hash in the repo to be used instead of the old cached hash.

error initializing configuration '/envoy.test.yaml': Unable to convert YAML as JSON:

I'm not seeing this error when starting the envoy proxy. Is your local path correct when invoking this command: docker run --rm -it -v ~/git/haveno-ui-poc/config/envoy.test.yaml:/envoy.test.yaml -p 8080:8080 -p 8081:8081 envoyproxy/envoy-dev:8a2143613d43d17d1eb35a24b4a4a4c432215606 -c /envoy.test.yaml ?

As well as an error when running npm start:

src/HavenoDaemon.tsx
Line 130:13: Expected an assignment or function call and instead saw an expression @typescript-eslint/no-unused-expressions

Good catch, opened a PR to correctly set this parameter: https://github.com/haveno-dex/haveno-ui-poc/pull/26

Am I wasting my time trying to get Haveno UI PoC to run w/o errors? I think not because at the end of the day I will be running tests to ensure my code hasn't broken anything, but I'd like a second opinion on this.

Definitely not. It's most important that the end-to-end tests run, and the UI POC should run successfully if the tests run.

Still haven't managed to fix it.

Please keep us posted.

FYI: I have implemented changes in monero-java to support launching monerod as an internal process for this issue to use. It will be available in the next release of monero-java in the coming days.

@erciccione erciccione removed the P2 high priority label Nov 9, 2021
@randall-coding
Copy link
Contributor

SOLVED: I narrowed down the issue to the protobuf-compiler version. I was on libprotoc 3.6.1 but woodser was on libprotoc 3.17.3. This was leading to the ts files not being generated correctly.

I had to upgrade my OS version to upgrade my protobuf-compiler version, but it worked. Thanks to @woodser, couldn't have done it without your help. Now I'll get out of the way on this PR.

@woodser
Copy link
Contributor Author

woodser commented Dec 5, 2021

This issue is currently unassigned with HashedMantisShrimp being pulled into other things.

@randall-coding Please let me know if you'd like this issue assigned to you.

@randall-coding
Copy link
Contributor

randall-coding commented Dec 5, 2021

Sure that sounds great, please do. I should be finishing up my other task very soon (working the bugs out of my tests right now).

@woodser
Copy link
Contributor Author

woodser commented Feb 12, 2022

Hey @randall-coding, still working on this or better to re-assign?

@randall-coding
Copy link
Contributor

randall-coding commented Feb 12, 2022

You can re-assign, I haven't gotten around to this yet.

@woodser
Copy link
Contributor Author

woodser commented Feb 12, 2022

Ok. Thanks :)

@woodser woodser changed the title Add API functions to start and stop local Monero node [$400] Add API functions to start and stop local Monero node [$200] Feb 12, 2022
@woodser woodser changed the title Add API functions to start and stop local Monero node [$200] Add API functions to start and stop local Monero node [$300] Feb 14, 2022
@duriancrepe
Copy link
Contributor

Hi, I would like to work on this next please!

@woodser
Copy link
Contributor Author

woodser commented Feb 15, 2022

Hi, I would like to work on this next please!

👍 It's all yours. Thanks.

@woodser
Copy link
Contributor Author

woodser commented Mar 7, 2022

I think the issue description needs updated after reviewing starting/stopping a local node in more depth.

The official Monero software recognizes when a local node is running at predefined ports (18081, 28081, 38081 for mainnet, testnet, stagenet respectively) and also allows a local node to be started with optional configuration (startup flags, blockchain path, and bootstrap url).

I'm thinking we should do the same to accomodate various use cases.

In that case, the API would be approximately:

API Function Return Description
havenod.isMoneroNodeRunning() boolean Indicates if a local Monero node is running at port 18081, 28081, or 38081 for mainnet, testnet, and stagenet respectively.
havenod.getMoneroNodeSettings() MoneroNodeSettings Get the last used settings to start a local Monero node. This should be saved and encrypted (added to existing preferences?)
havenod.startMoneroNode(settings: MoneroNodeSettings) void Start local Monero node.
havenod.stopMoneroNode() void Stop local Monero node.
MoneroNodeSettings
startupFlags?: string
blockchainPath?: string
bootstrapUrl?: string

For the test, if the local node is already started when the test is run, the test can do some basic checks and pass with a warning that the node was already started and so starting/stopping the node is not being tested. If the local node is not already started, the test can manually start and stop the local node and check the current connection accordingly (await havenod.isConnectedToMonero()).

We can update the issue description and make minor adjustments to the bounty amount if necessary. Thoughts?

@duriancrepe
Copy link
Contributor

Sounds good I can begin looking into the changes soon.

@duriancrepe
Copy link
Contributor

Can I get clarification on what the purpose of monero connection manager (added a3586fd) and how it should interact with a local monero node?

To me it appears that it is used to fall back to other remote monero nodes, which should be disabled if a localhost monero node has been started. Does that mean the default connection detection (localhost stagenet) should not be part of the monero connection manager?

@duriancrepe
Copy link
Contributor

I've updated it so that the new CoreMoneroNodeService will initialize by attempting to connect to a local node via RPC, or start a new local Monero node process from the last known MoneroNodeSettings. If successful, set that daemon connection in the Monero connection manager, otherwise use the same connection heuristics as before.

As for encrypting the MoneroNodeSettings, I believe it would be better to implement encrypted persistence of all persistable objects at once, which I can work on as a separate pull request. I looked at the EncryptedConnectionsList as an alternative implementation strategy for encrypting specific properties with the user's password but I think it will be more maintenance to do that for every persisted setting. Let me know that's OK.

Build Haveno's API automation moved this from In progress to Done Apr 4, 2022
@erciccione erciccione moved this from Patch proposed to Patch accepted in Bounty program Apr 5, 2022
@erciccione
Copy link
Contributor

Reward sent. Thanks @duriancrepe :)

@erciccione erciccione moved this from Patch accepted to Reward sent (resolved) in Bounty program Apr 11, 2022
@duriancrepe
Copy link
Contributor

Received! Thanks :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:api Related to the gRPC API 💰bounty There is a bounty on this issue is:feature Request for a new feature P3 normal priority
Projects
Bounty program
Reward sent (resolved)
5 participants