smartcontractkit / external-initiator Goto Github PK

Potentially add runlog & fluxmonitor distinct types on JobSpec? (For now the convention is that if nothing is passed then it is runlog. But there might be other types from time to time.)

FluxMonitor spec might need some validations.

If oracle is not eligible to submit, FM is not started. We should probably log the reason for the non-eligibility, so we can know if it is expected behavior or something has gone wrong.

As of current PoC FM integration with Substrate, we requesting the FA state once on FM, and then changing it directly on FM on new incoming events from BM.

We might need to have the FA state synced as close as possible to the actual blockchain state. For e.g something might go wrong in FM logic, and have it do an internal state change that doesn't reflect the true state of the blockchain. Or maybe the blockchain has some other unexpected behaviour that FM haven't taken into account, so the FA is in a different state.

A solution could be to do the syncing(querying the whole FA state anew) more frequently, on incoming blockchain events, to prevent faulty results.

(We might need to move all FA state handling to BM and have FM only to read it).

i have ran ./integration/setup without issue.
But when i ran ./integration/run_test, the integration_chainlink container kept restarting.
Here is the log from that container:

2020-09-15T00:10:35Z [INFO]  API exposed for user [email protected]          cmd/local_client.go:95
2020-09-15T00:10:35Z [DEBUG] eth.Client#Dial(...)                               eth/client.go:110
error starting app: dial tcp 127.0.0.1:8546: connect: connection refused

i didn't change the integration/chainlink.env file under integration which has ETH_DISABLED=true

As part of #139, the FM service now has access to contexts in requests to the blockchain implementation.

The FM service should implement these by setting a timeout for requests, and calling the context cancel on subscriptions on shutdown.

FM starts even the related job is not present in the CL node. What should the behavior be?

At least we might need an easy way to delete subscriptions(both to CL node + the blockchain), without having to manually do it through db? Maybe something like clear/reset of all subscriptions at least?

If the job run is not successful, EI cannot submit again, due to lastSubmittedRound already incremenented. Potentially increment the lastSubmittedRound at a later point, after successfully submitting to the contract itself.

Currently it takes ~6 min for the CI to set up the integration tests, and roughly 10 minutes in total. This probably won't scale very well as we integrate more (non-EVM) chains.

Unit tests in non-blockchain packages should be blockchain agnostic

The FM service currently has a lot of unclear logic that should be cleaned up.

A few of the issues:

• Too many pointers (it's not clear how big the risk of nil pointer dereferences are)
• Events from the blockchain manger first goes to one listeners, which interprets the response, which sends the events in a new channel, which is then read by a new listener.

While it was discussed earlier that the blockchain manager sending a single FM state struct would be the best option, I now think that the best solution here is if we have a generic channel in which the blockchain manager sends different types of events defined in the blockchain package. This should reduce the number of pointers used, and hopefully clean up some of the logic in the FM service.

There are cases where we don't want a poll timer and only use a heartbeat, or cases where we only want a poller (trigger on deviation) and no heartbeat.

This includes:

• Support for a single request - response model
• Support for reconnecting (should re-open any pre-existing subscriptions) with incremental backoff

See chainlink/node_test.go for inspiration