Use of RPCManager with auto switching over node configs

[xmariachi]

Purpose of Changes and their Description

• Creation of a RPCManager that controls the rpc endpoints.
• An RPC (client) is defined by a NodeConfig. The RPCManager holds a number of NodeConfigs, preconfigured.
• Initialization, parameterisation of calls for
• Compliant with current separation of responsibilities, usecase keeps node management (before initialised and never changed; now integrated as an RPCManager), while client-internal operations fall under lib (ie NodeConfig-based operations).
• Definition of situations where a switch of node is required. For now, error 429 and +Mempool is full, maybe to be expanded.
• RPCManager as an interface with its testing mock

Also:

• custom errors and error refactorization
• fix whitelist extra check
• improve checking gas routine

Link(s) to Ticket(s) or Issue(s) resolved by this PR

Are these changes tested and documented?

• If tested, please describe how. If not, why tests are not needed. -- being tested against chain
• If documented, please describe where. If not, describe why docs are not needed. -- Added section in README
• Added to Unreleased section of CHANGELOG.md?

Still Left Todo

• testing with several workers under load, and adding unit testing.
• review current usage processes esp on registration where several calls are made, etc.
• potentially more refactors

[amimart]

That's good stuff well done! I've let some remarks I'll let you consider

[kpeluso]

I would just want to talk ab the algorithm being employed. Because the RPCManager is initialized at startup time and reused across all processes, then if there is load, we're guaranteed to hit the rate limit, although much higher chance of recovering still (because we cycle through RPCs upon 429). However, with a randomized algo, we'd avoid hitting a rate limit for longer (because we're not necessarily using the last-used RPC) while maintaining a high chance of recovery upon 429 (because we still cycle through RPCs with some probability). Obviously this is dependent on both the number of topics, their epochs, rate limits of each RPC, and number of RPCs involved, but afaict the randomized algo will work better for a wider set of those arguments than this serial approach.

[kpeluso]

Not going to block on this but you could add a parameter that meters how often one loops through the full list, or simply how many rpcs are attempted, with replacement (so you can retry preexisting ones).

[xmariachi]

I want to add number of "bad failures" (failures attributable to the rpc instability, not chain values) per node, and a "jail" where nodes can spend some time out of the valid set.

Also we can set Prometheus metrics for some of these failures.

[spooktheducks]

maybe skill issue, but my experience with that sort of thing is: at first, it looks like the smart thing to do, but complexity and tuning quickly become burdensome. i think just having good metrics/logging will get us to a perfectly acceptable place

[xmariachi]

+1 on avoiding early optimization. Improvements (if understood as such!) are expressed as future work, sorry that wasn't explicit.

But metrics here are needed first, yes. There is a ticket for this since it needs some rework on the way metrics are currently defined and the new places where we want to use it. Probably the client to receive a Metrics object optionally to configure it internally - current metrics impl I think it was a quick addition for just getting it running, but not very carefully thought in terms of app structure.

[xmariachi]

Pending - to jail bad nodes --> In a separate pr

[xmariachi]

Rebased to dev (timeouts, logging and minor things)

[xmariachi]

Rebased again with v0.8.1. Re-tested against testnet.

[amimart]

LGTM :)

[guilherme-brandao]

Preemptively approving because I believe if you consider adding the suggestions I made, they will be addressed in your subsequent PR. Additionally, changing the client (which I know you plan to do in your next PR) might enforce some of these changes.