updates and disables the chain tx if unable to connect to an rpc

[dmikey]

going to smash this one through, but removes the requirement to connect to the app chain when running the node.

{"level":"info","component":"node","request":"1340ceab-d04e-45fa-8849-5d8a537b04a5","function":"bafybeiaugwh3mktzbnudurzk7jcyvmdhyy6jnairiu2phuf2t7c7orowjq","node_count":1,"cluster_size":1,"responded":1,"time":"2024-01-25T17:51:42-06:00","message":"received execution responses"}
{"level":"debug","time":"2024-01-25T17:51:46-06:00","message":"inference results would have been submitted to chain"}

[linear]

ORA-481 Remove strict dependency between appchain and compute node

Data scientists should not have to be bothered with all the devops involved in running the appchain. They should just have to worry about their model. However our appchain and compute node are currently coupled in our docker-compose. This shouldn't be the case!

Task here is to split these apart! And also communicate it with the team so minimal surprises. See this convo for more.

[Maelkum]

Leaving a few comments, though some may be outdated by another PR, which I am going to look at next :)

[Maelkum]

I think these should wrap error and return, and uppermost function should log. Right now, the function is ran as a goroutine and its return value never checked.

Upper function should be something like:

func (ap *AppChain) start(ctx context.Context) {
	go func() {
		err := ap.startClient(ctx, ap.Config)
		if err != nil {
			ap.Config.Logger.Error().Err(err).Msg("appchain client start failed")
		}
	}()
}

(with logger outside of the config)

[Maelkum]

If there's no chain available, this will fail first. Error for me was error while requesting node 'http://localhost:26657': post failed: Post \"http://localhost:26657\": dial tcp 127.0.0.1:26657: connect: connection refused.

Few issues I think:

1. Right now SubmitTx is never initialized to true? It starts off as and will forever be false.
2. I think we have some distinct scenarios:
   a) chain being there or not
   b) me wanting to use chain or not

Combination of those two is possible:
a) chain is there but I don't want to use it
b) chain is there, and I do want to use it. If I can't - it's an error
c) chain is not there, and I don't want to use it
d) chain is not there, and I do want to use it - error

Scenarios b) and d) I think are clear errors that should result in stopping the process, not silently continuing. IMO, not even logging and continuing.

[Maelkum]

This code path doesn't clear the SubmitTx boolean. It is never set in the first place either, but if that is fixed, this should also clear it.

[Maelkum]

Bug because of how this function is used - config is a function parameter, while there's also ap.Config used.

This line should probably use ap.Config.SubmitTx (that's the one that's changed in the upper conditions).

Also, if you invert the function, you can get rid of the nesting:

	if !ap.Config.SubmitTx {
		config.Logger.Warn().Err(err).Msg("could not retrieve allora blockchain address, transactions will not be submitted to chain")
		return nil
	}

	if !queryIsNodeRegistered(ctx, client, address, config) {
		// not registered, register the node
		registerWithBlockchain(ctx, client, account, config)
	}
	

[Maelkum]

I think this is a bug.. You want to do this async, in the background, but you use the echo.Context as a context. In the next function you use ctx.Request().Context(). From the docs:

// For incoming server requests, the context is canceled when the
// client's connection closes, the request is canceled (with HTTP/2),
// or when the ServeHTTP method returns.

So, if the client drops off OR - it simply received our response, is content and closes the connection => this context gets canceled, and we may not complete whatever it is we set out to do. This should be context.Background() if we want it truly to run in the background.