sidestream-tech / unified-auctions-ui

Unified MakerDAO auctions
https://unified-auctions.makerdao.com
GNU Affero General Public License v3.0
16 stars 13 forks source link

Support custom RPC url(s) #222

Closed valiafetisov closed 2 years ago

valiafetisov commented 2 years ago

Goal

The user is able to supply RPC url via environment variable instead of infura project id

Context

  1. Currently we only let users (who wants to start our UI or the bot locally) provide INFURA_PROJECT_ID. But this is limiting, since users may use different providers as well as custom / local nodes
  2. Currently the user is only able to connect to a set of networks from the list of supported ones and there is no possibility to dynamically add new networks to our UI
    • Use case: user is able to use custom PRC endpoint of their own
    • Use case: we're able to integrate chaoslabs forks

Note: most probably, those two scenarios require two different solutions. This issue outlines them here because we don't want them to be in conflict with each other.

Tasks

valiafetisov commented 2 years ago

Just thinking out loud atm:

There are several problems which needs to be taken into account when switching to custom RPC_URL instead of INFURA_PROJECT_ID:

valiafetisov commented 2 years ago

So the proposal would be to:

zoey-kaiser commented 2 years ago

I had one additional question. We currently have a etherscanURL, assigned for every Network. How would we want to handle this if a rpcURL is assigned, with no matching etherscanURL (+ how would we know if one is accessible or not)?

I would recommend the following solution:

I would love to hear some of your thoughts on how to handle this! 😄

zoey-kaiser commented 2 years ago

Alternatively,

we could include a second environment variable, called etherscan link. If this is set to false or undefined, the links are disabled.

valiafetisov commented 2 years ago

We currently have a etherscanURL, assigned for every Network. How would we want to handle this if a rpcURL is assigned

Good question. I imagine that we can just derive network name from chainId (fetched from the RPC) and construct urls based on that. In case of the testing environment, when id will be the same as the main network, the url will be invalid, but the developer should be aware of that. Adding support for block explorers that work with localhost can be added in another issue and is out of scope of this one.

zoey-kaiser commented 2 years ago

Good question. I imagine that we can just derive network name from chainId (fetched from the RPC) and construct urls based on that. In case of the testing environment, when id will be the same as the main network, the url will be invalid, but the developer should be aware of that. Adding support for block explorers that work with localhost can be added in another issue and is out of scope of this one.

Sounds like a plan! I should easily be able to derive it from the name and then I can open a new issue, once this PR is merged for adding custom block explores. But I also agree, that this does not have the highest priority and developers, hosting the site via their own custom RPC or networks, should be able to manage this issue for themselves until we get to it!

zoey-kaiser commented 2 years ago

After further discussion in a call we settled on the following system:

Note: Only infura project IDs will generate the correct EtherScan links. If you use a network not supported by Etherscan, the links will point to the mainnet browser, which will be missing your data. However until we get to it, developers will need to manage this bythemselves.

valiafetisov commented 2 years ago

Thanks for the summary!

You can add and remove providers/signers after first load (allows us to add simulation setups)

I think provides/signers are different from networks – you may have 3 networks, where only one of them is initialized (ie have provider). So the idea is to have networks global state similar to providers/signers (eg with setNetwork function which adds new network to the list based on PRC url alone)

On first load the RPC Url is parsed

Not necessarily on the first load setNetwork can be called later on, on the first load we parse RPC_URL env variable, but the same steps of detecting project id etc can be applied on every setNetwork call

Everything else sounds good ✨

zoey-kaiser commented 2 years ago

I had a question about how to manage/sync the network store on the frontend. I wanted to make the network dropdown be based on the networks global variable defined in the core. How can I ensure, that the UI is synced and updated when a new Network is added. In theory I need to use a VueX store for this, but is there a way to sync said store with the core functions?

Maybe there is another alternative approach I am missing, that I currently face the issue, that I update the network value in from the auctions-core, but the changed values are not reflected in the frontend.

Edit: I am also having issues, with having the bot reflect the updated changes to the network global store.

valiafetisov commented 2 years ago

One possible approach is to emit network changes. So the setNetwork can not only modify global state, but re-emit the whole network config via subscribeToNetworks(callback) – which is then can be used in the store:


// inside to the core
const callbacks = [];
const subscribeToNetworks = function (callback) {
    callbacks.push(callback);
}
const setNetwork = function () {
    // the new function that adds a network
    callbacks.forEach(callback => callback(allNetworks));
};

// inside to the store
export const actions = {
    // ...
    setup({ rootState, dispatch }: ActionContext<State, any>) {
        subscribeToNetworks(() => {
            // modify state 
        });
    },
};
zoey-kaiser commented 2 years ago

The next issue I am running into is how to define the currently selected Network. I was thinking of storing this variable, just like all the networks. I would then also setup a subscription service, that the frontend can callback to this.

This would remove the need for us to pass network too call core functions, as the selectedNetwork would be retrieved from the core. I think I would keep the network prop, but make it optional, so that one could in theory override and use a different network, from the one you are currently on.

Does this make sense or should I approach it in a different way?

valiafetisov commented 2 years ago

how to define the currently selected Network

I think this doesn't need to be stored in the core. And it's already stored in the network vuex store. As we discussed earlier, refactoring network parameters is out of scope right now.

zoey-kaiser commented 2 years ago

how to define the currently selected Network

I think this doesn't need to be stored in the core. And it's already stored in the network vuex store. As we discussed earlier, refactoring network parameters is out of scope right now.

The bigger issue is the bot. As we want to remove the network parameter for this, I am not too sure how to store it there. Should I just make a variable and then set the network during the bot setup?

valiafetisov commented 2 years ago

One possible approach is to emit network changes

Another one is that setNetwork can simply return newly added network config.

As we want to remove the network parameter for this, I am not too sure how to store it there.

Technically, you don't need to store it, since you can derive it from the RPC_URL. You can simply get the first of the added network either via const config = await setNetwork(RPC_URL) (if you call setNetwork from within the bot) or by using subscribeToNetworks from above if you've already implemented it