[DRAFT] Create new getstatus API method

[ghubstan]

This PR will never leave DRAFT status, and will be replaced by a clean PR.

A daemon will return an int32 = 1 (the pong) to the CLI if successful, then an api service is available message will be displayed in the CLI's console. If a valid pong is not returned, an api service is not available message is displayed in the console.

TODO: Experiments will be made to see if this implementation can be used to check server availabilty for other requests. This server ping implementation checks for a non-null wallet balance before returning a pong. If this ping impl's wallet check throws an exception to the calling API method, the calling method should throw a gRPC exception to the CLI instead of completing the operation, and the user should see an "api service is not available" message. No other API methods call ping with this change; experiments will be done later, giving other devs time to consider the idea first.

The premise is that if the wallet balance is available, so is the server, else the server is not avaiable. This may be a simple solution to the 'p2p not ready' problem affecting daemons.

A ping test was added to apitest/scripts/mainnet-test.sh.

This cannot be merged, but the code is safely backed up and the concept could be reviewed by other devs.

[chimp1984]

This may be a simple solution to the 'p2p not ready' problem affecting daemons.

I doubt that this is a feasible approach. There are several state in the p2p and wallet which need be treated differently depending on use case.
P2p:

• getInitialDataResponse -> can show offers but is not listening to network as no hidden service published yet.
• HS published -> ok not we are part of the network
• UpdatedDataResonse -> process the maybe missed data in the duration between initiaal response and HS published

Wallet:

• initialize -> wallet is set up
• sufficient peers connected -> it is safe to broadcast txs
• block download complete -> all synced, not needed to use the app but some txs might not show confirmed or we might miss balance if we received funds

So you see this complexity cannot be simulated with a single method. I would not suggest to use ping for more then simple connectivity tests.

[ghubstan]

@chimp1984,
@cbeams sent his thoughts about this problem some months ago in a keybase video before he handed this API project to me. I thought about it, put other work in front, and now trying to solve it before I finish out the "simplest trading script" impl. Chris wanted me to find a way to tell a user if a daemon was ready to perform all functions or no functions, and not show a CLI user anything more detailed than "server up" or "server not ready". (But details would be logged on the server.) And it needs to be used by some server methods (like create offer, take offer,...) so they know the request can be made or not.

So you see this complexity cannot be simulated with a single method. I would not suggest to use ping for more then simple connectivity tests.

The problem is the requirement to do precisely that -- to communicate the "ready=true/false" state with a single method. A simple gRPC connectivity check isn't useful for a user or some API methods that should throw an exception when "server not ready".

I thought a non-null balance was enough to determine if the server was ready or not, because a non-null balance means that allBasicServicesInitialized=true (see BisqSetup#initDomainServices)

As you said, it's not that simple; a non-null wallet balance is not enough to indicate the daemon is ready for trading.

What do you think of this solution for this non-trivial type of "ping"?

if(!bsqNode.isP2pNetworkReady())
	throw new IllegalStateException("p2p network not ready");

if(!p2PService.isBootstrapped())
	throw new IllegalStateException("p2p service not bootstrapped");

if(!daoStateService.isParseBlockChainComplete())
	throw new IllegalStateException("dao block chain not complete");

check for sufficient # of connect peers... a minimum of 4?

then the wallet balance check...

If these checks pass, ping returns a pong, else thows an exception.

[chimp1984]

@ghubstan
Those checks when all is ready are there. It just combines all the required substates. It can be added to your core API class and you simply request that. I would not mix that with ping/pong as that has another meaning (testing network availibility or keep connections alive).

The balance though might be a bit tricky. You can create an offer in Bisq UI with 0 BTC balance but need to fund it then in the create (or take) offer process. So in the UI it is not required to have balance as you also dont know in advance how much is required before the user defines the details of the offer (amount, sec. deposit).
To change too much from the way it works in the UI carries the risk that we get 2 apps to maintain. I would not recommend that, specially if there are good reasons why it is like that.

[ghubstan]

@chimp1984

RE: The balance though might be a bit tricky. You can create an offer in Bisq UI with 0 BTC balance but need to fund it then in the create (or take) offer process. So in the UI it is not required to have balance as you also dont know in advance how much is required before the user defines the details of the offer (amount, sec. deposit).

The balance check is for a non-null value. A 0 balance passes the test. When create & take offer is implemented, it will have to check funds for the offer, but that's not in the scope of this PR. (Those API methods don't exist yet.)

... It can be added to your core API class and you simply request that. I would not mix that with ping/pong as that has another meaning (testing network availibility or keep connections alive).

I agree ping is not the proper name for the method/meaning.

We still need to discuss this issue... I'm not sure you know the requirement (now) is to provide a single method for determining if the server is ready for any & all operations -- that all the criteria in the checks are met, or the server is not ready to take requests. There is no requirement (yet) for fine grained checks particular to a specific operation, excepting the existing checks in the CoreWalletsService. That is out of the scope of getting a "simplest trading script" ready to show other devs and users. I'm supposed to keep the demo'd API as simple as possible.

So, ping should be renamed, not exposed to the user, and only be used by other api methods to check readiness for the more complex operations. Would that be OK for you?

[ghubstan]

Replaced by 4534.