Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SUPPORTED_CHAINS ENV var #1900

Merged
merged 15 commits into from
May 12, 2019
Merged

SUPPORTED_CHAINS ENV var #1900

merged 15 commits into from
May 12, 2019

Conversation

vbaranov
Copy link
Member

@vbaranov vbaranov commented May 7, 2019

Closes #1899

Motivation

We have no ability to manage supported chains list without redeploying the application

Changelog

Enhancements

A new ENV var SUPPORTED_CHAINS has been added. It represents a minified array of JSON objects

[
    {
      "title": "POA Core",
      "url": "https://blockscout.com/poa/core"
    },
    {
      "title": "POA Sokol",
      "url": "https://blockscout.com/poa/sokol",
      "test_net?": true
    },
    {
      "title": "xDai Chain",
      "url": "https://blockscout.com/poa/dai"
    },
    {
      "title": "Ethereum Mainnet",
      "url": "https://blockscout.com/eth/mainnet"
    },
    {
      "title": "Kovan Testnet",
      "url": "https://blockscout.com/eth/kovan",
      "test_net?": true
    },
    {
      "title": "Ropsten Testnet",
      "url": "https://blockscout.com/eth/ropsten",
      "test_net?": true
    },
    {
      "title": "Goerli Testnet",
      "url": "https://blockscout.com/eth/goerli",
      "test_net?": true
    },
    {
      "title": "Rinkeby Testnet",
      "url": "https://blockscout.com/eth/rinkeby",
      "test_net?": true
    },
    {
      "title": "Ethereum Classic",
      "url": "https://blockscout.com/etc/mainnet",
      "other?": true
    },
    {
      "title": "Aerum Mainnet",
      "url": "https://blockscout.com/aerum/mainnet",
      "other?": true
    },
    {
      "title": "Callisto Mainnet",
      "url": "https://blockscout.com/callisto/mainnet",
      "other?": true
    },
    {
      "title": "RSK Mainnet",
      "url": "https://blockscout.com/rsk/mainnet",
      "other?": true
    },
    {
      "title": "LUKSO L14 testnet",
      "url": "https://blockscout.com/lukso/l14",
      "test_net?": true,
      "hide_in_dropdown?": true
    }
  ]

and the minified version:

export SUPPORTED_CHAINS='[ { "title": "POA Core", "url": "https://blockscout.com/poa/core" }, { "title": "POA Sokol", "url": "https://blockscout.com/poa/sokol", "test_net?": true }, { "title": "xDai Chain", "url": "https://blockscout.com/poa/dai" }, { "title": "Ethereum Mainnet", "url": "https://blockscout.com/eth/mainnet" }, { "title": "Kovan Testnet", "url": "https://blockscout.com/eth/kovan", "test_net?": true }, { "title": "Ropsten Testnet", "url": "https://blockscout.com/eth/ropsten", "test_net?": true }, { "title": "Goerli Testnet", "url": "https://blockscout.com/eth/goerli", "test_net?": true }, { "title": "Rinkeby Testnet", "url": "https://blockscout.com/eth/rinkeby", "test_net?": true }, { "title": "Ethereum Classic", "url": "https://blockscout.com/etc/mainnet", "other?": true }, { "title": "Aerum Mainnet", "url": "https://blockscout.com/aerum/mainnet", "other?": true }, { "title": "Callisto Mainnet", "url": "https://blockscout.com/callisto/mainnet", "other?": true }, { "title": "RSK Mainnet", "url": "https://blockscout.com/rsk/mainnet", "other?": true }, { "title": "LUKSO L14 testnet", "url": "https://blockscout.com/lukso/l14", "test_net?": true, "hide_in_dropdown?": true } ]'
  • fallback const @default_other_networks has been added to be able to run without ENV var
  • hide_in_dropdown has been added to the chain JSON object to support hiding of chain in the dropdown list whereas keeping it in the footer

The list of ENV vars updated https://forum.poa.network/t/faq-environment-variables/1814

Checklist for your PR

  • I added an entry to CHANGELOG.md with this PR
  • If I added new functionality, I added tests covering it.
  • If I fixed a bug, I added a regression test to prevent the bug from silently reappearing again.
  • I checked whether I should update the docs and did so if necessary

@ghost ghost assigned vbaranov May 7, 2019
@ghost ghost added the in progress label May 7, 2019
@vbaranov vbaranov added ready for review This PR is ready for reviews. and removed in progress labels May 7, 2019
@ghost ghost added the in progress label May 7, 2019
Copy link
Contributor

@goodsoft goodsoft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This kinda complicates local development, doesn't it?
I think that some kind of default list should be present in the code, so that it is possible to run the app server with only actually local configuration (i.e. node/db credentials)

@coveralls
Copy link

coveralls commented May 7, 2019

Pull Request Test Coverage Report for Build 2be3a366-52b7-4da6-b426-e1329df72978

  • 7 of 7 (100.0%) changed or added relevant lines in 1 file are covered.
  • 6 unchanged lines in 3 files lost coverage.
  • Overall coverage decreased (-0.08%) to 81.716%

Files with Coverage Reduction New Missed Lines %
apps/indexer/lib/indexer/fetcher/token.ex 1 78.57%
apps/indexer/lib/indexer/fetcher/token_balance.ex 2 87.1%
apps/indexer/lib/indexer/block/fetcher.ex 3 86.57%
Totals Coverage Status
Change from base Build 611097b2-d5ee-4ff8-9a33-d20b5a47b491: -0.08%
Covered Lines: 4639
Relevant Lines: 5677

💛 - Coveralls

@@ -96,7 +96,11 @@ defmodule BlockScoutWeb.LayoutView do

def other_networks do
:block_scout_web
|> Application.get_env(:other_networks, [])
|> Application.get_env(:other_networks)
|> Poison.decode!()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think just adding a

|> Kernel.||(@default_other_networks) would be good for those that don't want to customize this list.

Copy link
Contributor

@zachdaniel zachdaniel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small piece of feedback, but otherwise 👍

@vbaranov
Copy link
Member Author

vbaranov commented May 7, 2019

@zachdaniel @goodsoft tnx for comments. I added the fallback to @default_other_networks const for those who don't need to set this env var.
In addition, hide_in_dropdown? attribute has been added to chain's object to be able to manage visibility in chains dropdown

@vbaranov
Copy link
Member Author

Missing tests were added

@vbaranov vbaranov merged commit cc3b0df into master May 12, 2019
@ghost ghost removed the in progress label May 12, 2019
@vbaranov vbaranov deleted the supported-chains-env-var branch May 12, 2019 20:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready for review This PR is ready for reviews.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make the supported chains configurable via an environment variable
5 participants