chore_: network providers reorder #5941
Conversation
Jenkins BuildsClick to see older builds (47)
|
❌ 1 Tests Failed:
View the full list of 1 ❄️ flaky tests
To view individual test run time comparison to the main branch, go to the Test Analytics Dashboard |
|
We should keep Grove because it's the only decentralised provider we have. And it doesn't cost us anything to run. We can change the order of fallback so the requests don't go there, but if there is a state banning event on Infura (or our proxy), Grove can potentially keep the app usable. |
|
@shivekkhurana I am not sure why we would like to keep Grove, as an option, if we proved multiple times that it's unreliable, not only that it returns errors where it should not, but also that it returns inaccurate responses and that's more dangerous. |
saledjenic
force-pushed
the
remove-grove
branch
from
8c4ba20
to
e9c6173
Compare
|
@status-im/status-go-guild what do you think of updating |
As discussed on the call. Grove is the most low prio RPC for doomsday scenarios. |
api/default_networks.go
Outdated
| DefaultFallbackURL2: fmt.Sprintf("https://%s.api.status.im/grove/ethereum/mainnet/", stageName), | ||
| RPCURL: "https://eth-archival.rpc.grove.city/v1/", | ||
| FallbackURL: "https://mainnet.infura.io/v3/", | ||
| DefaultFallbackURL2: "https://mainnet.infura.io/v3/", |
There was a problem hiding this comment.
if proxyProvider.Enabled {
key := ProviderStatusProxy
keyFallback := ProviderStatusProxy + "-fallback"
keyFallback2 := ProviderStatusProxy + "-fallback2"
urls[key] = network.DefaultRPCURL
urls[keyFallback] = network.DefaultFallbackURL
urls[keyFallback2] = network.DefaultFallbackURL2
keys = []string{key, keyFallback, keyFallback2}
authMap[key] = proxyProvider.User + ":" + proxyProvider.Password
authMap[keyFallback] = authMap[key]
authMap[keyFallback2] = authMap[key]
}
keys = append(keys, []string{"main", "fallback"}...)
urls["main"] = network.RPCURL
urls["fallback"] = network.FallbackURL
We do not use authentication for RPCURL. If I understand correctly it should be:
DefaultRPCURL: fmt.Sprintf("https://%s.api.status.im/nodefleet/ethereum/mainnet/", stageName),
DefaultFallbackURL: fmt.Sprintf("https://%s.api.status.im/infura/ethereum/mainnet/", stageName),
DefaultFallbackURL2: fmt.Sprintf("https://%s.api.status.im/grove/ethereum/mainnet/", stageName),
RPCURL: "https://mainnet.infura.io/v3/",
FallbackURL: "https://eth-archival.rpc.grove.city/v1/",
BlockExplorerURL: "https://etherscan.io/",
StatusProxy - Node Fleet
StatusProxy - Infura
Direct Infura
StatusProxy - Grove
Direct Grove
and if we need to have proxy-grove lower than direct-infura, then we should probably add "priority" field to params.network, and respect it in rpc/client.go getEthClients method.
There was a problem hiding this comment.
@friofry good point, I missed that usage. Thanks.
Need to think a bit about how to do it painless for now.
There was a problem hiding this comment.
Now should be good, providers priority is determined by the getProviderPriorityByURL function.
I think it would be great to introduce a one-to-many relationship between |
saledjenic
force-pushed
the
remove-grove
branch
from
e9c6173
to
937aada
Compare
api/default_networks_test.go
Outdated
| require.True(t, strings.Contains(actualNetworks[8].DefaultRPCURL, stageName)) | ||
| require.True(t, strings.Contains(actualNetworks[8].DefaultFallbackURL, stageName)) | ||
| require.True(t, strings.Contains(actualNetworks[8].DefaultFallbackURL2, stageName)) | ||
| ignoreDefaultRPCURLCheck := false // TODO: used just because of Goerli, remove once we remove Goerli from the default networks |
There was a problem hiding this comment.
It's fine to leave it if removing Goerli now will introduce too much complexity in this PR, there's 0 reason to have it since none of the providers we use support it still.
There was a problem hiding this comment.
Here is a new PR that removes Goerli from the code #5945
saledjenic
force-pushed
the
remove-grove
branch
from
937aada
to
53db1bb
Compare
saledjenic
force-pushed
the
remove-grove
branch
from
53db1bb
to
66c7bb8
Compare
saledjenic
changed the title
api/default_networks.go
Outdated
| infura = "infura.io/" | ||
| grove = "grove.city/" | ||
| ) | ||
|
|
||
| for _, n := range networks { |
There was a problem hiding this comment.
My suggestion is to add a functor to make it a bit shorter and avoid duplication:
func setRPCs(networks []params.Network, request *requests.WalletSecretsConfig) []params.Network {
var networksWithRPC []params.Network
const (
infura = "infura.io/"
grove = "grove.city/"
)
appendToken := func(url string) string {
if strings.Contains(url, infura) && request.InfuraToken != "" {
return url + request.InfuraToken
} else if strings.Contains(url, grove) && request.PoktToken != "" {
return url + request.PoktToken
}
return url
}
for _, n := range networks {
n.DefaultRPCURL = appendToken(n.DefaultRPCURL)
n.DefaultFallbackURL = appendToken(n.DefaultFallbackURL)
n.DefaultFallbackURL2 = appendToken(n.DefaultFallbackURL2)
n.RPCURL = appendToken(n.RPCURL)
n.FallbackURL = appendToken(n.FallbackURL)
if request.GanacheURL != "" {
n.RPCURL = request.GanacheURL
n.FallbackURL = request.GanacheURL
switch n.ChainID {
case mainnetChainID:
n.TokenOverrides = []params.TokenOverride{mainnetGanacheTokenOverrides}
case goerliChainID:
n.TokenOverrides = []params.TokenOverride{goerliGanacheTokenOverrides}
}
}
networksWithRPC = append(networksWithRPC, n)
}
return networksWithRPC
}
rpc/client.go
Outdated
| func (c *Client) getEthClients(network *params.Network) []ethclient.RPSLimitedEthClientInterface { | ||
| // Keys are defining the order of providers being used |
There was a problem hiding this comment.
I see 2 problems:
- if the same key priority is used
- some code duplication
my suggestion to add something like this:
type Provider struct {
Key string
URL string
Auth string
Priority int
}
func createProvider(key, url, credentials string, providers *[]Provider) {
priority := getProviderPriorityByURL(url)
*providers = append(*providers, Provider{
Key: key,
URL: url,
Auth: credentials,
Priority: priority,
})
}
func (c *Client) prepareProviders(network *params.Network) []Provider {
var providers []Provider
// Retrieve the proxy provider configuration
proxyProvider, err := getProviderConfig(c.providerConfigs, ProviderStatusProxy)
if err != nil {
c.log.Warn("could not find provider config for status-proxy", "error", err)
}
// Add main and fallback providers
createProvider(ProviderMain, network.RPCURL, "", &providers)
createProvider(ProviderFallback, network.FallbackURL, "", &providers)
// If the proxy provider is enabled, add it and its fallback options
if proxyProvider.Enabled {
credentials := proxyProvider.User + ":" + proxyProvider.Password
createProvider(ProviderStatusProxy, network.DefaultRPCURL, credentials, &providers)
createProvider(ProviderStatusProxyFallback, network.DefaultFallbackURL, credentials, &providers)
createProvider(ProviderStatusProxyFallback2, network.DefaultFallbackURL2, credentials, &providers)
}
// Sort providers by priority
sort.Slice(providers, func(i, j int) bool {
return providers[i].Priority < providers[j].Priority
})
return providers
}
and use it in getEthClients
|
@friofry thanks for the suggestions, I agree it's better, but I wanted to do it with minimal changes to the current code, that's why I introduced |
That's possible if the same URLs are used for different networks, but that check should be part of the config validation process. |
|
@friofry updated the code as you suggested. Thanks. |
saledjenic
force-pushed
the
remove-grove
branch
from
da4d225
to
2226a8a
Compare
Yeah, I realised that too and deleted my comment :) |
rpc/provider.go
Outdated
| ProviderFallback = "fallback" | ||
| ProviderStatusProxy = "status-proxy" | ||
| ProviderStatusProxyFallback = ProviderStatusProxy + "-fallback" | ||
| ProviderStatusProxyFallback2 = ProviderStatusProxy + "-fallback" |
There was a problem hiding this comment.
I guess it's a typo, should befallback2
There was a problem hiding this comment.
You were faster in commenting on that than me updating it. :)
Thanks, updated.
saledjenic
force-pushed
the
remove-grove
branch
from
2226a8a
to
6979a9b
Compare
A new order of using providers is: 1. StatusProxy - Node Fleet 2. StatusProxy - Infura 3. Direct Infura 4. StatusProxy - Grove 5. Direct Grove
saledjenic
force-pushed
the
remove-grove
branch
from
6979a9b
to
93e88d9
Compare
dlipicar
changed the title
A new order of using providers is:
Closes #5944