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.

Sign up for GitHub

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

Configuration: refactor network configuration, migrate to chainId, remove unnecessary connectivity polling #1485

Closed
wants to merge 10 commits into from
Closed

Configuration: refactor network configuration, migrate to chainId, remove unnecessary connectivity polling #1485

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
a4f33cd
Configuration: refactor network configuration and align strings using…
sohkai Jul 10, 2020
5d24e1f
Account: remove unused code for detecting network mismatch
sohkai Jul 10, 2020
584456c
Network: update configuration to use 'ethereum' as id of main network
sohkai Jul 10, 2020
5468dba
Environment: update function names for eth endpoint configuration
sohkai Jul 10, 2020
c456ea8
Preferences: update connection settings' copy and base checks on chai…
sohkai Jul 10, 2020
4f20948
Network: replace network.type with network.chainId
sohkai Jul 10, 2020
d507385
Onboarding: move selectable networks into a constant
sohkai Jul 10, 2020
851e9b6
Network: remove top-level polling for client provider's connection st…
sohkai Jul 10, 2020
9c19599
Account: remove unused client block number polling
sohkai Jul 10, 2020
47e9d2a
Account: refactor connection info rendering
sohkai Jul 10, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/CONFIGURATION.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ Address of the ENS registry that [aragonPM](https://hack.aragon.org/docs/apm-int

### `ARAGON_ETH_NETWORK_TYPE`

Expected network type to connect to. Either one of `main`, `rinkeby` or `local`.
Expected blockchain network to connect to. One of `mainnet`, `rinkeby`, or `local`. `xdai` and `ropsten` are also available but considered experimental networks.

### `ARAGON_ETH_SUBSCRIPTION_EVENT_DELAY`

Expand Down
2 changes: 1 addition & 1 deletion now-mainnet.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"alias": "nightly.aragon.org",
"build": {
"env": {
"ARAGON_ETH_NETWORK_TYPE": "main",
"ARAGON_ETH_NETWORK_TYPE": "ethereum",
"ARAGON_FORTMATIC_API_KEY": "@aragon-client-fortmatic-api-key",
"ARAGON_PORTIS_DAPP_ID": "@aragon-client-portis-dapp-id",
"ARAGON_SENTRY_DSN": "@aragon-client-sentry-dsn",
Expand Down
5 changes: 2 additions & 3 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
]
},
"dependencies": {
"@aragon/templates-tokens": "^1.2.1",
"@aragon/ui": "^1.4.2",
"@aragon/wrapper": "^5.0.0-rc.28",
"@sentry/browser": "^5.17.0",
Expand Down Expand Up @@ -105,14 +104,14 @@
"bundlewatch": "bundlewatch",
"start": "node scripts/start",
"start:local": "node scripts/launch-local",
"start:mainnet": "cross-env ARAGON_ETH_NETWORK_TYPE=main npm start",
"start:mainnet": "cross-env ARAGON_ETH_NETWORK_TYPE=ethereum npm start",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've started renaming our internal references to "main" or "mainnet" to "Ethereum", as "mainnet" is not descriptive enough (e.g. xDai has a mainnet, Aragon Chain will have a mainnet).

"start:rinkeby": "npm start",
"start:staging": "cross-env ARAGON_ENS_REGISTRY_ADDRESS=0xfe03625ea880a8cba336f9b5ad6e15b0a3b5a939 npm start",
"start:ropsten": "cross-env ARAGON_ETH_NETWORK_TYPE=ropsten npm start",
"start:xdai": "cross-env ARAGON_ETH_NETWORK_TYPE=xdai npm start",
"build": "node scripts/build",
"build:local": "node scripts/build-local",
"build:mainnet": "cross-env ARAGON_ETH_NETWORK_TYPE=main npm run build",
"build:mainnet": "cross-env ARAGON_ETH_NETWORK_TYPE=ethereum npm run build",
"build:rinkeby": "npm run build",
"build:staging": "cross-env ARAGON_ENS_REGISTRY_ADDRESS=0xfe03625ea880a8cba336f9b5ad6e15b0a3b5a939 npm run build",
"build:ropsten": "cross-env ARAGON_ETH_NETWORK_TYPE=ropsten npm run build",
Expand Down
6 changes: 3 additions & 3 deletions scripts/start
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@ const execute = require('child_process').execSync
const clientPort = process.env.ARAGON_PORT || process.env.REACT_APP_PORT || 3000

if (
process.env.ARAGON_ETH_NETWORK_TYPE === 'main' &&
process.env.ARAGON_ETH_NETWORK_TYPE === 'ethereum' &&
!process.env.ARAGON_DEFAULT_ETH_NODE
) {
console.log(
'⚠️ You are connecting the Aragon client to mainnet but have not specified an Ethereum node.'
'⚠️ You are connecting the Aragon client to Ethereum mainnet but have not specified an Ethereum node.'
)
console.log(
' Connecting to the default node (wss://mainnet.eth.aragon.network/ws) in a local'
Expand All @@ -25,7 +25,7 @@ if (
"➡️ During development, it is recommended to override the 'ARAGON_DEFAULT_ETH_NODE' environment"
)
console.log(
' variable with an Infura (infura.io) mainnet node to accelerate loading Ethereum events and state.'
' variable with an Infura (infura.io) Ethereum mainnet node to accelerate loading Ethereum events and state.'
)
console.log(
' You may do so by directly setting an environment variable or by using a .env file.'
Expand Down
39 changes: 7 additions & 32 deletions src/App.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,15 @@ import React from 'react'
import PropTypes from 'prop-types'
import { Spring, animated } from 'react-spring'
import { useTheme } from '@aragon/ui'
import { EthereumAddressType, ClientThemeType } from './prop-types'
import { useWallet } from './wallet'
import { network, web3Providers } from './environment'
import initWrapper from './aragonjs-wrapper'
import { web3Providers } from './environment'
import { useClientTheme } from './client-theme'
import { useRouting } from './routing'
import initWrapper, { pollConnectivity } from './aragonjs-wrapper'
import { Onboarding } from './onboarding'
import { getWeb3 } from './web3-utils'
import { EthereumAddressType, ClientThemeType } from './prop-types'
import { log } from './utils'
import { useWallet } from './wallet'
import { getWeb3 } from './web3-utils'
import { ActivityProvider } from './contexts/ActivityContext'
import { FavoriteDaosProvider } from './contexts/FavoriteDaosContext'
import { PermissionsProvider } from './contexts/PermissionsContext'
Expand Down Expand Up @@ -45,18 +45,6 @@ const INITIAL_DAO_STATE = {
repos: [],
}

const SELECTOR_NETWORKS = [
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This felt like an odd place to configure the available networks during onboarding, so I've moved it out into the /onboarding folder as static configuration.

['main', 'Ethereum Mainnet', 'https://mainnet.aragon.org/'],
['rinkeby', 'Ethereum Testnet (Rinkeby)', 'https://rinkeby.aragon.org/'],
]
if (network.type === 'ropsten') {
SELECTOR_NETWORKS.push([
'ropsten',
'Ethereum Testnet (Ropsten)',
'https://aragon.ropsten.aragonpm.com/',
])
}

class App extends React.Component {
static propTypes = {
clientTheme: ClientThemeType.isRequired,
Expand All @@ -67,23 +55,14 @@ class App extends React.Component {

state = {
...INITIAL_DAO_STATE,
connected: false,
fatalError: null,
identityIntent: null,
selectorNetworks: SELECTOR_NETWORKS,
transactionBag: null,
signatureBag: null,
web3: getWeb3(web3Providers.default),
wrapper: null,
}

componentDidMount() {
// Only the default, because the app can work without the wallet
pollConnectivity([web3Providers.default], connected => {
this.setState({ connected })
Copy link
Contributor Author

Choose a reason for hiding this comment

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

As far as I could tell, this connected state is actually not used anymore now that we do the block polling in the Account module.

Good riddance—this was spiking our API requests to ETH nodes!

Copy link

Choose a reason for hiding this comment

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

🚀

})
}

componentDidUpdate(prevProps, prevState) {
const { clientTheme, routing, walletAccount } = this.props
const { wrapper } = this.state
Expand Down Expand Up @@ -300,7 +279,6 @@ class App extends React.Component {
permissions,
permissionsLoading,
repos,
selectorNetworks,
transactionBag,
signatureBag,
web3,
Expand Down Expand Up @@ -361,7 +339,7 @@ class App extends React.Component {
/>
<FavoriteDaosProvider>
<ActivityProvider
daoDomain={daoAddress.domain}
daoAddress={daoAddress.address}
web3={web3}
>
<PermissionsProvider
Expand Down Expand Up @@ -389,10 +367,7 @@ class App extends React.Component {
</div>
</PermissionsProvider>

<Onboarding
selectorNetworks={selectorNetworks}
web3={web3}
/>
<Onboarding web3={web3} />

<GlobalPreferences
apps={appsWithIdentifiers}
Expand Down
23 changes: 9 additions & 14 deletions src/apps/Organization/Organization.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@ import {
useTheme,
} from '@aragon/ui'
import LocalIdentityBadge from '../../components/IdentityBadge/LocalIdentityBadge'
import appIds from '../../known-app-ids'
import CHAIN_IDS from '../../chain-ids'
import { network } from '../../environment'
import { getProviderString } from '../../ethereum-providers'
import { sanitizeNetworkType } from '../../network-config'
import appIds from '../../known-app-ids'
import { AppType, DaoAddressType } from '../../prop-types'
import { useRouting, ARAGONID_ENS_DOMAIN } from '../../routing'
import airdrop, { testTokensEnabled } from '../../testnet/airdrop'
Expand Down Expand Up @@ -104,14 +104,13 @@ const Organization = React.memo(function Organization({
const hasFinanceApp = apps.some(app => app.appId === appIds.Finance)
const checksummedDaoAddr =
daoAddress.address && toChecksumAddress(daoAddress.address)
const enableTransactions =
wallet.connected && wallet.networkType === network.type
const isMainnet = network.type === 'main'
const enableTransactions = wallet.connected
const isMainnet = network.chainId === CHAIN_IDS.ETHEREUM
const shortAddresses = layoutName !== 'large'

const organizationText = checksummedDaoAddr ? (
<span>
This organization is deployed on the Ethereum {network.name}.{' '}
This organization is deployed on the {network.name} network.{' '}
{canUpgradeOrg ? (
<span>
<Link onClick={onShowOrgVersionDetails}>
Expand Down Expand Up @@ -281,8 +280,8 @@ const Organization = React.memo(function Organization({
) : (
<span>
Unfortunately, importing into Tenderly is not available on
the {sanitizeNetworkType(network.type)} network. Please
use Aragon on Ethereum mainnet instead.
the {network.name} network. Please use Aragon on the
Ethereum main network instead.
</span>
)}
</p>
Expand All @@ -291,7 +290,7 @@ const Organization = React.memo(function Organization({
)}
</React.Fragment>
)}
{hasFinanceApp && testTokensEnabled(network.type) && (
{hasFinanceApp && testTokensEnabled() && (
<Box heading="Request test tokens">
<p
css={`
Expand Down Expand Up @@ -335,11 +334,7 @@ const Organization = React.memo(function Organization({
</Info>
) : (
<Info mode="warning">
{`Please ${
wallet.networkType !== network.type
? `select the ${sanitizeNetworkType(network.type)} network`
: 'unlock your account'
} in ${getProviderString(
{`Please unlock your account in ${getProviderString(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Similar to the Account module, I've simplified places where we check the connect account's network vs. the expected network as the user currently can never have their account enabled on a different network.

'your Ethereum wallet',
wallet.providerInfo.id
)}.`}
Expand Down
27 changes: 1 addition & 26 deletions src/aragonjs-wrapper.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import { NoConnection, DAONotFound } from './errors'
import { getEthSubscriptionEventDelay } from './local-settings'
import { workerFrameSandboxDisabled } from './security/configuration'
import { appBaseUrl } from './url-utils'
import { noop, removeStartingSlash, pollEvery } from './utils'
import { noop, removeStartingSlash } from './utils'
import {
getGasPrice,
getWeb3,
Expand All @@ -26,8 +26,6 @@ import {
import SandboxedWorker from './worker/SandboxedWorker'
import WorkerSubscriptionPool from './worker/WorkerSubscriptionPool'

const POLL_DELAY_CONNECTIVITY = 2000

const applyAppOverrides = apps =>
apps.map(app => ({ ...app, ...(appOverrides[app.appId] || {}) }))

Expand Down Expand Up @@ -75,29 +73,6 @@ const prepareAppsForFrontend = (apps, daoAddress, gateway) => {
.sort(sortAppsPair)
}

export const pollConnectivity = pollEvery((providers = [], onConnectivity) => {
let lastFound = null
return {
request: async () => {
try {
await Promise.all(
providers.map(p => getWeb3(p).eth.net.getNetworkType())
)
return true
} catch (err) {
return false
}
},
onResult: connected => {
if (connected !== lastFound) {
lastFound = connected
onConnectivity(connected)
}
},
}
// web.eth.net.isListening()
}, POLL_DELAY_CONNECTIVITY)

export const resolveEnsDomain = async domain => {
try {
return await ensResolve(domain, {
Expand Down
21 changes: 21 additions & 0 deletions src/chain-ids.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
const CHAIN_IDS = {
ETHEREUM: 1,
ROPSTEN: 3,
RINKEBY: 4,
KOVAN: 42,
GOERLI: 5,
XDAI: 100,
}

// Error on invalid accesses to CHAIN_IDS
const PROTECTED_CHAIN_IDS = new Proxy(CHAIN_IDS, {
get(target, property) {
if (property in target) {
return target[property]
} else {
throw new Error(`Chain ID '${property}' not supported`)
}
},
})
Comment on lines +11 to +19
Copy link

Choose a reason for hiding this comment

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

Nice :)

Copy link
Contributor

Choose a reason for hiding this comment

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

✨


export default PROTECTED_CHAIN_IDS
20 changes: 5 additions & 15 deletions src/components/AccountModule/AccountModule.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,7 @@
import React, { useCallback, useEffect, useMemo, useRef, useState } from 'react'
import { useWallet } from '../../wallet'
import { useLocalIdentity } from '../../hooks'
import {
useNetworkConnectionData,
useSyncInfo,
useWalletConnectionDetails,
} from './connection-hooks'
import { useSyncInfo, useWalletConnectionDetails } from './connection-hooks'
import AccountModulePopover from './AccountModulePopover'
import ButtonConnect from './ButtonConnect'
import ButtonAccount from './ButtonAccount'
Expand Down Expand Up @@ -58,10 +54,10 @@ function AccountModule() {
clientSyncDelay,
connectionColor,
connectionMessage,
hasNetworkMismatch,
label,
walletConnectionStatus,
walletListening,
walletOnline,
walletSyncDelay,
} = useConnectionInfo()

Expand Down Expand Up @@ -132,7 +128,6 @@ function AccountModule() {
<ButtonAccount
connectionColor={connectionColor}
connectionMessage={connectionMessage}
hasNetworkMismatch={hasNetworkMismatch}
label={label}
onClick={toggle}
/>
Expand Down Expand Up @@ -189,7 +184,7 @@ function AccountModule() {
providerInfo={providerInfo}
walletConnectionStatus={walletConnectionStatus}
walletListening={walletListening}
walletOnline={walletListening}
walletOnline={walletOnline}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure, but I think this should be a fix 😄

Copy link
Contributor

Choose a reason for hiding this comment

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

Whoops! Haha I can definitely tell you this is a fix. 😆

walletSyncDelay={walletSyncDelay}
/>
)
Expand All @@ -213,7 +208,7 @@ function useConnectionInfo() {
isOnline: walletOnline,
connectionStatus: walletConnectionStatus,
syncDelay: walletSyncDelay,
} = useSyncInfo('wallet')
} = useSyncInfo(wallet.web3)

const {
isListening: clientListening,
Expand All @@ -222,16 +217,13 @@ function useConnectionInfo() {
syncDelay: clientSyncDelay,
} = useSyncInfo()

const { walletNetworkName, hasNetworkMismatch } = useNetworkConnectionData()

const { connectionMessage, connectionColor } = useWalletConnectionDetails(
clientListening,
walletListening,
clientOnline,
walletOnline,
clientSyncDelay,
walletSyncDelay,
walletNetworkName
walletSyncDelay
)

return {
Expand All @@ -241,11 +233,9 @@ function useConnectionInfo() {
clientSyncDelay,
connectionColor,
connectionMessage,
hasNetworkMismatch,
label,
walletConnectionStatus,
walletListening,
walletNetworkName,
walletOnline,
walletSyncDelay,
}
Expand Down
Loading