haveno-dex / haveno

Decentralized P2P exchange platform built on Monero and Tor
https://haveno.exchange
GNU Affero General Public License v3.0
1.02k stars 112 forks source link

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

Closed woodser closed 2 years ago

woodser commented 3 years ago

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.

woodser commented 2 years ago

What do you say @woodser?

The daemon version should display 1.6.3 when running.

Please pull the latest commits to haveno and haveno-ui-poc which hardcodes Alice and Bob's wallet ports so the port no longer needs manually copied to the tests.

If possible, please add trace logging to the tests to see exactly what call is failing (logging should be added to the tests anyway).

randall-coding commented 2 years ago

Is there a stacktrace in the daemon consoles?

Here is the stacktrace

Nov-17 08:23:28.166 [BisqDaemonMain] ERROR b.d.grpc.GrpcWalletsService: java.lang.IllegalStateException: wallet is locked at bisq.core.api.CoreWalletsService.verifyEncryptedWalletIsUnlocked(CoreWalletsService.java:464) at bisq.core.api.CoreWalletsService.getBalances(CoreWalletsService.java:147) at bisq.core.api.CoreApi.getBalances(CoreApi.java:276) at bisq.daemon.grpc.GrpcWalletsService.getBalances(GrpcWalletsService.java:97) at bisq.proto.grpc.WalletsGrpc$MethodHandlers.invoke(WalletsGrpc.java:1036) at io.grpc.stub.ServerCalls$UnaryServerCallHandler$UnaryServerCallListener.onHalfClose(ServerCalls.java:172) at io.grpc.internal.ServerCallImpl$ServerStreamListenerImpl.halfClosed(ServerCallImpl.java:331) at io.grpc.internal.ServerImpl$JumpToApplicationThreadServerStreamListener$1HalfClosed.runInContext(ServerImpl.java:814) at io.grpc.internal.ContextRunnable.run(ContextRunnable.java:37) at io.grpc.internal.SerializingExecutor.run(SerializingExecutor.java:123) at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128) at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628) at java.base/java.lang.Thread.run(Thread.java:834)

I'm looking at the code, I see there is a tempAesKey that is generated at some point. Perhaps my server had tried to generate that when my firewall was up and I didn't see the error message. Not sure.

I also see that unlocking the wallet relies on a password which I didn't set. I'm assuming somewhere in the code that gets set automatically?

UPDATE: I went through all the steps again with my firewall off, but still ending up with the two "wallet is locked" errors. I don't know much about wallet locking / unlocking. Was there I step I missed where I unlock the wallet?

randall-coding commented 2 years ago

I am using Ufw on Ubuntu 20.04, I tried completely disabling the firewall, didn't help. But I think this is expected? Because all our processes are running on localhost inside the same machine (at least mine are), so enabling or disabling the firewall shouldn't affect them - I think.

@HashedMantisShrimp That was the change that did it for me. There must be more than one way to produce the 400/500 error unfortunately.

HashedMantisShrimp commented 2 years ago

I should probably have taken a look at the webpage console, it does have a "stack-trace" and it also has the same http error as the tests. Meaning that they're likely being caused by the same thing.

I spent my whole weekend playing around with grpc-web and protobuf. I'm sure that neither of those are the problem because I managed to run their example project, which depends on me having a working executable file for grpc-web and the right installation of protobuf.

I'll switch my focus to the issue on the webpage, I'm sure that once it's solved some of my tests will run as well. Meanwhile, woodser the seednode daemon outputs this version at the very begining of the stack-trace - SeedNode.VERSION: 1.6.2

If and when I find time this week (or during the weekend), I'll investigate the webpage angle, meanwhile, here's what I got from the log (take into account that all my procs are running fine, the only proc I'm not running is arbitrator):

Side note, when I attempted to update the protobuf client from haveno-ui-poc, I broke a lot of things. Not gonna mess with that for a while.

Object { message: "Http response at 400 or 500 level", stack: "E@http://localhost:3000/static/js/vendors~main.chunk.js:11549:13\nY@http://localhost:3000/static/js/vendors~main.chunk.js:13036:33\nX/<@http://localhost:3000/static/js/vendors~main.chunk.js:13010:25\nIb@http://localhost:3000/static/js/vendors~main.chunk.js:12175:22\nO@http://localhost:3000/static/js/vendors~main.chunk.js:12161:5\nwc@http://localhost:3000/static/js/vendors~main.chunk.js:12670:21\nyc@http://localhost:3000/static/js/vendors~main.chunk.js:12737:11\n./node_modules/grpc-web/index.js/</n.W@http://localhost:3000/static/js/vendors~main.chunk.js:12682:5\n./node_modules/grpc-web/index.js/</n.R@http://localhost:3000/static/js/vendors~main.chunk.js:12678:59\n", code: 14, metadata: {} } ​ code: 14 ​ message: "Http response at 400 or 500 level" ​ metadata: Object { } ​​

: Object { … } ​ stack: "E@http://localhost:3000/static/js/vendors~main.chunk.js:11549:13\nY@http://localhost:3000/static/js/vendors~main.chunk.js:13036:33\nX/<@http://localhost:3000/static/js/vendors~main.chunk.js:13010:25\nIb@http://localhost:3000/static/js/vendors~main.chunk.js:12175:22\nO@http://localhost:3000/static/js/vendors~main.chunk.js:12161:5\nwc@http://localhost:3000/static/js/vendors~main.chunk.js:12670:21\nyc@http://localhost:3000/static/js/vendors~main.chunk.js:12737:11\n./node_modules/grpc-web/index.js/: Object { constructor: E(a, b, c) , name: "RpcError", stack: "" } ​​ constructor: function E(a, b, c)​​​ O: Error.prototype { stack: "", … } ​​​ arguments: null ​​​ caller: null ​​​ length: 3 ​​​ name: "E" ​​​ prototype: Object { constructor: E(a, b, c) , name: "RpcError", stack: "" } ​​​​ constructor: function E(a, b, c) ​​​​ name: "RpcError" ​​​​ stack: "" ​​​​ : Error.prototype { stack: "", … } ​​​ : function Error() ​​ name: "RpcError" ​​ stack: "" ​​ : Error.prototype { stack: "", … } index.js:1 e index.js:1 componentDidMount App.tsx:47
woodser commented 2 years ago

Why are you not running the arbitrator process? It's needed.

randall-coding commented 2 years ago

I'm back to working on making tests pass. I made progress by deleting all the app data in my ~/.local/shared/ folder (last time I just deleted the wallet file) and everything in haveno/ folder then cloning and building again. No longer seeing wallet is locked error and now seeing money received in monero-rpc console, but...

randall-coding commented 2 years ago

Now I'm seeing errors relating to values being undefined in multiple tests.

TypeError: Cannot read property 'serializeBinary' of undefined (1 test) TypeError: Cannot read property 'setCurrencyCode' of undefined (6 tests)

● Can get market prices

TypeError: Cannot read property 'serializeBinary' of undefined

  728 |     grpc_pb.MarketPriceReply,
  729 |     (request: grpc_pb.MarketPriceRequest) => {
> 730 |       return request.serializeBinary();
      |                      ^
  731 |     },
  732 |     grpc_pb.MarketPriceReply.deserializeBinary
  733 |   );

  at src/protobuf/GrpcServiceClientPb.ts:730:22
  at Nc (node_modules/grpc-web/index.js:62:169)
  at Z.<anonymous> (node_modules/grpc-web/index.js:57:691)
  at Z.rpcCall (node_modules/grpc-web/index.js:57:710)
  at PriceClient.getMarketPrice (src/protobuf/GrpcServiceClientPb.ts:751:27)
  at src/HavenoDaemon.ts:62:25
  at HavenoDaemon.getPrice (src/HavenoDaemon.ts:61:12)
  at Object.<anonymous> (src/HavenoDaemon.test.ts:101:29)

TypeError: Cannot read property 'setCurrencyCode' of undefined

  139 |         address: string): Promise<PaymentAccount> {
  140 |     let that = this;
> 141 |     let request = new CreateCryptoCurrencyPaymentAccountRequest()
      |                   ^
  142 |             .setAccountName(accountName)
  143 |             .setCurrencyCode(currencyCode)
  144 |             .setAddress(address)

  at HavenoDaemon.createCryptoPaymentAccount (src/HavenoDaemon.ts:141:19)
  at postOffer (src/HavenoDaemon.test.ts:513:55)
  at Object.<anonymous> (src/HavenoDaemon.test.ts:281:32)
woodser commented 2 years ago

Guessing that the grpc client needs regenerated from Haveno's updated protobuf definition.

It should happen automatically on npm install or npm test.

Note the haveno and haveno-ui-poc projects need to be in the same directory for the ui project to read the protobuf definition in the haveno project.

randall-coding commented 2 years ago

@woodser

Guessing that the grpc client needs regenerated from Haveno's updated protobuf definition.

I've got those boxes checked off, it's in the same directory and I ran npm install (also ran the bin script on its own a few times). Let me give you some more info on what I'm seeing, I will take as an example one of the tests yielding an error on setCurrencyCode() of undefined.

The error is occurring because GetOffersRequest().setDirection(direction) is yielding undefined, which is what setCurrencyCode() is chained on to. There is a declaration for this object in grpc_pb.d.ts as follows

export class GetOffersRequest extends jspb.Message { getDirection(): string; setDirection(value: string): GetOffersRequest;

getCurrencyCode(): string; setCurrencyCode(value: string): GetOffersRequest;

serializeBinary(): Uint8Array; toObject(includeInstance?: boolean): GetOffersRequest.AsObject; static toObject(includeInstance: boolean, msg: GetOffersRequest): GetOffersRequest.AsObject; static serializeBinaryToWriter(message: GetOffersRequest, writer: jspb.BinaryWriter): void; static deserializeBinary(bytes: Uint8Array): GetOffersRequest; static deserializeBinaryFromReader(message: GetOffersRequest, reader: jspb.BinaryReader): GetOffersRequest; }

And this namespace is declared, but this is the only other reference to GetOffersRequest there is

export namespace GetOffersRequest { export type AsObject = { direction: string, currencyCode: string, } }

So it looks like I have only a declaration of GetOffersRequest but not an implementation of the function details (if I'm understanding this correctly). Should more have been generated? What should I have expected?

woodser commented 2 years ago

My grpc_pb.d.ts looks the same. Have you pulled the latest commits from both upstream repositories? There have been changes to the protobuf definition.

If still stuck I could send you my grpc client code to test.

Have you been able to run all tests without any changes, or could the problem be caused by changes you've made?

randall-coding commented 2 years ago

Just reconfirmed I'm on the most recent changes for both, and on my last pull of master I basically made everything over from scratch.

No modifications to the tests, but I have added some code to grpc.proto file.

Here are all the steps I'm taking.

make make bitcoind (and make btc-blocks) make monero-shared make seednode make arbitrator-desktop (set up accounts with ctrl n and ctrl d) make alice-daemon make bob-daemon start test envoy docker run --rm --add-host host.docker.internal:host-gateway -it -v /home/super/Haveno/haveno-ui-poc/config/envoy.test.yaml:/envoy.test.yaml -p 8080:8080 -p 8081:8081 envoyproxy/envoy-dev:8a2143613d43d17d1eb35a24b4a4a4c432215606 -c /envoy.test.yaml start monero wallet rpc cd ~/Haveno/haveno/.localnet/ && ./monero-wallet-rpc --daemon-address http://localhost:38081 --daemon-login superuser:abctesting123 --stagenet --rpc-bind-port 38084 --rpc-login rpc_user:abc123 --wallet-dir ./ --rpc-access-control-origins http://localhost:8080 npm test (in haveno-ui-poc folder. Using node 12.20.1)

No errors from any of those consoles I have opened, only from npm test (those errors I posted above).

randall-coding commented 2 years ago

Here is the full errors output

FAIL src/HavenoDaemon.test.ts (5.725 s) ● Can get market prices

TypeError: Cannot read property 'serializeBinary' of undefined

  728 |     grpc_pb.MarketPriceReply,
  729 |     (request: grpc_pb.MarketPriceRequest) => {
> 730 |       return request.serializeBinary();
      |                      ^
  731 |     },
  732 |     grpc_pb.MarketPriceReply.deserializeBinary
  733 |   );

  at src/protobuf/GrpcServiceClientPb.ts:730:22
  at Nc (node_modules/grpc-web/index.js:62:169)
  at Z.<anonymous> (node_modules/grpc-web/index.js:57:691)
  at Z.rpcCall (node_modules/grpc-web/index.js:57:710)
  at PriceClient.getMarketPrice (src/protobuf/GrpcServiceClientPb.ts:751:27)
  at src/HavenoDaemon.ts:62:25
  at HavenoDaemon.getPrice (src/HavenoDaemon.ts:61:12)
  at Object.<anonymous> (src/HavenoDaemon.test.ts:101:29)

● Can get offers

TypeError: Cannot read property 'setCurrencyCode' of undefined

  162 |     let that = this;
  163 |     return new Promise(function(resolve, reject) {
> 164 |       that._offersClient.getOffers(new GetOffersRequest().setDirection(direction).setCurrencyCode("XMR"), {password: that._password}, function(err: grpcWeb.RpcError, response: GetOffersReply) {
      |                                    ^
  165 |         if (err) reject(err);
  166 |         else resolve(response.getOffersList());
  167 |       });

  at src/HavenoDaemon.ts:164:36
  at HavenoDaemon.getOffers (src/HavenoDaemon.ts:163:12)
  at Object.<anonymous> (src/HavenoDaemon.test.ts:131:41)

● Can get my offers

TypeError: Cannot read property 'setCurrencyCode' of undefined

  179 |     let that = this;
  180 |     return new Promise(function(resolve, reject) {
> 181 |       that._offersClient.getMyOffers(new GetOffersRequest().setDirection(direction).setCurrencyCode("XMR"), {password: that._password}, function(err: grpcWeb.RpcError, response: GetOffersReply) {
      |                                      ^
  182 |         if (err) reject(err);
  183 |         else resolve(response.getOffersList());
  184 |       });

  at src/HavenoDaemon.ts:181:38
  at HavenoDaemon.getMyOffers (src/HavenoDaemon.ts:180:12)
  at Object.<anonymous> (src/HavenoDaemon.test.ts:138:41)

● Can create crypto payment accounts

TypeError: Cannot read property 'setCurrencyCode' of undefined

  139 |         address: string): Promise<PaymentAccount> {
  140 |     let that = this;
> 141 |     let request = new CreateCryptoCurrencyPaymentAccountRequest()
      |                   ^
  142 |             .setAccountName(accountName)
  143 |             .setCurrencyCode(currencyCode)
  144 |             .setAddress(address)

  at HavenoDaemon.createCryptoPaymentAccount (src/HavenoDaemon.ts:141:19)
  at Object.<anonymous> (src/HavenoDaemon.test.ts:160:54)

● Can post and remove an offer

TypeError: Cannot read property 'setCurrencyCode' of undefined

  139 |         address: string): Promise<PaymentAccount> {
  140 |     let that = this;
> 141 |     let request = new CreateCryptoCurrencyPaymentAccountRequest()
      |                   ^
  142 |             .setAccountName(accountName)
  143 |             .setCurrencyCode(currencyCode)
  144 |             .setAddress(address)

  at HavenoDaemon.createCryptoPaymentAccount (src/HavenoDaemon.ts:141:19)
  at postOffer (src/HavenoDaemon.test.ts:513:55)
  at Object.<anonymous> (src/HavenoDaemon.test.ts:204:32)

● Invalidates offers when reserved funds are spent

TypeError: Cannot read property 'setCurrencyCode' of undefined

  139 |         address: string): Promise<PaymentAccount> {
  140 |     let that = this;
> 141 |     let request = new CreateCryptoCurrencyPaymentAccountRequest()
      |                   ^
  142 |             .setAccountName(accountName)
  143 |             .setCurrencyCode(currencyCode)
  144 |             .setAddress(address)

  at HavenoDaemon.createCryptoPaymentAccount (src/HavenoDaemon.ts:141:19)
  at postOffer (src/HavenoDaemon.test.ts:513:55)
  at Object.<anonymous> (src/HavenoDaemon.test.ts:228:32)

● Can complete a trade

TypeError: Cannot read property 'setCurrencyCode' of undefined

  139 |         address: string): Promise<PaymentAccount> {
  140 |     let that = this;
> 141 |     let request = new CreateCryptoCurrencyPaymentAccountRequest()
      |                   ^
  142 |             .setAccountName(accountName)
  143 |             .setCurrencyCode(currencyCode)
  144 |             .setAddress(address)

  at HavenoDaemon.createCryptoPaymentAccount (src/HavenoDaemon.ts:141:19)
  at postOffer (src/HavenoDaemon.test.ts:513:55)
  at Object.<anonymous> (src/HavenoDaemon.test.ts:281:32)

Alice posting offer PASS src/App.test.tsx

Test Suites: 1 failed, 1 passed, 2 total Tests: 7 failed, 4 passed, 11 total Snapshots: 0 total Time: 7.526 s Ran all test suites.

randall-coding commented 2 years ago

Unfortunately this is looking very idiosyncratic to me at this point, so I'm probably going to have to really go on a mission to figure this out.

randall-coding commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

Hey @Randall-Coding, still working on this or better to re-assign?

randall-coding commented 2 years ago

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

woodser commented 2 years ago

Ok. Thanks :)

duriancrepe commented 2 years ago

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

woodser commented 2 years ago

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

👍 It's all yours. Thanks.

woodser commented 2 years ago

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 commented 2 years ago

Sounds good I can begin looking into the changes soon.

duriancrepe commented 2 years ago

Can I get clarification on what the purpose of monero connection manager (added https://github.com/haveno-dex/haveno/commit/a3586fdef8b4da9af974841239fd636921ce3a73) 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 commented 2 years ago

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.

erciccione commented 2 years ago

Reward sent. Thanks @duriancrepe :)

duriancrepe commented 2 years ago

Received! Thanks :)