Skip to content

Commit

Permalink
Enforce chainId (#324)
Browse files Browse the repository at this point in the history 
* chainif

* txcontroller

* not default

* mutex

* test

* Revert "mutex"

This reverts commit 20b0adb.

* tests

* fulltest

* format

* comments
  • Loading branch information
@estebanmino
estebanmino committed Jan 19, 2021
1 parent 8a914d1 commit 23cdb2d
Show file tree
Hide file tree
Showing 7 changed files with 125 additions and 65 deletions.
20 changes: 15 additions & 5 deletions src/network/NetworkController.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,16 @@ const { Mutex } = require('await-semaphore');
*/
export type NetworkType = 'kovan' | 'localhost' | 'mainnet' | 'rinkeby' | 'goerli' | 'ropsten' | 'rpc';

export enum NetworksChainId {
mainnet = '1',
kovan = '42',
rinkeby = '4',
goerli = '5',
ropsten = '3',
localhost = '',
rpc = '',
}

/**
* @type ProviderConfig
*
Expand All @@ -25,7 +35,7 @@ export type NetworkType = 'kovan' | 'localhost' | 'mainnet' | 'rinkeby' | 'goerl
export interface ProviderConfig {
rpcTarget?: string;
type: NetworkType;
chainId?: string;
chainId: string;
ticker?: string;
nickname?: string;
}
Expand Down Expand Up @@ -170,7 +180,7 @@ export class NetworkController extends BaseController<NetworkConfig, NetworkStat
super(config, state);
this.defaultState = {
network: 'loading',
provider: { type: 'mainnet' },
provider: { type: 'mainnet', chainId: NetworksChainId.mainnet },
};
this.initialize();
}
Expand Down Expand Up @@ -213,7 +223,7 @@ export class NetworkController extends BaseController<NetworkConfig, NetworkStat
this.update({
provider: {
...providerState,
...{ type, ticker: 'ETH' },
...{ type, ticker: 'ETH', chainId: NetworksChainId[type] },
},
});
this.refreshNetwork();
Expand All @@ -223,11 +233,11 @@ export class NetworkController extends BaseController<NetworkConfig, NetworkStat
* Convenience method to update provider RPC settings
*
* @param rpcTarget - RPC endpoint URL
* @param chainId? - Network ID as per EIP-155
* @param chainId - Network ID as per EIP-155
* @param ticker? - Currency ticker
* @param nickname? - Personalized network name
*/
setRpcTarget(rpcTarget: string, chainId?: string, ticker?: string, nickname?: string) {
setRpcTarget(rpcTarget: string, chainId: string, ticker?: string, nickname?: string) {
this.update({
provider: {
...this.state.provider,
Expand Down
16 changes: 11 additions & 5 deletions src/transaction/TransactionController.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -417,9 +417,12 @@ export class TransactionController extends BaseController<TransactionConfig, Tra
transaction = normalizeTransaction(transaction);
validateTransaction(transaction);

/* istanbul ignore next */
const networkID = network?.state?.provider?.chainId;

const transactionMeta = {
id: random(),
networkID: network ? network.state.network : /* istanbul ignore next */ '1',
networkID,
origin,
status: 'unapproved',
time: Date.now(),
Expand Down Expand Up @@ -472,20 +475,23 @@ export class TransactionController extends BaseController<TransactionConfig, Tra
const { transactions } = this.state;
const network = this.context.NetworkController as NetworkController;
/* istanbul ignore next */
const currentNetworkID = network ? network.state.network : '1';
const currentChainId = network?.state?.provider?.chainId;
const index = transactions.findIndex(({ id }) => transactionID === id);
const transactionMeta = transactions[index];
const { from } = transactionMeta.transaction;

if (!this.sign) {
this.failTransaction(transactionMeta, new Error('No sign method defined.'));
return;
} else if (!currentChainId) {
this.failTransaction(transactionMeta, new Error('No chainId defined.'));
return;
}

try {
transactionMeta.status = 'approved';
transactionMeta.transaction.nonce = await this.query('getTransactionCount', [from, 'pending']);
transactionMeta.transaction.chainId = parseInt(currentNetworkID, undefined);
transactionMeta.transaction.chainId = parseInt(currentChainId, undefined);

const ethTransaction = new Transaction({ ...transactionMeta.transaction });
await this.sign(ethTransaction, transactionMeta.transaction.from);
Expand Down Expand Up @@ -674,7 +680,7 @@ export class TransactionController extends BaseController<TransactionConfig, Tra
async queryTransactionStatuses() {
const { transactions } = this.state;
const network = this.context.NetworkController;
const currentNetworkID = network.state.network;
const currentNetworkID = network.state.provider.chainId;
let gotUpdates = false;
await safelyExecute(() =>
Promise.all(
Expand Down Expand Up @@ -726,7 +732,7 @@ export class TransactionController extends BaseController<TransactionConfig, Tra
if (!network) {
return;
}
const currentNetworkID = network.state.network;
const currentNetworkID = network.state.provider.chainId;
const newTransactions = this.state.transactions.filter(({ networkID }) => networkID !== currentNetworkID);
this.update({ transactions: newTransactions });
}
Expand Down
36 changes: 18 additions & 18 deletions tests/AssetsController.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { getOnce } from 'fetch-mock';
import AssetsController from '../src/assets/AssetsController';
import ComposableController from '../src/ComposableController';
import PreferencesController from '../src/user/PreferencesController';
import { NetworkController } from '../src/network/NetworkController';
import { NetworkController, NetworksChainId } from '../src/network/NetworkController';
import { AssetsContractController } from '../src/assets/AssetsContractController';

const HttpProvider = require('ethjs-provider-http');
Expand Down Expand Up @@ -166,11 +166,11 @@ describe('AssetsController', () => {
it('should add token by provider type', async () => {
const firstNetworkType = 'rinkeby';
const secondNetworkType = 'ropsten';
network.update({ provider: { type: firstNetworkType } });
network.update({ provider: { type: firstNetworkType, chainId: NetworksChainId[firstNetworkType] } });
await assetsController.addToken('foo', 'bar', 2);
network.update({ provider: { type: secondNetworkType } });
network.update({ provider: { type: secondNetworkType, chainId: NetworksChainId[secondNetworkType] } });
expect(assetsController.state.tokens).toHaveLength(0);
network.update({ provider: { type: firstNetworkType } });
network.update({ provider: { type: firstNetworkType, chainId: NetworksChainId[firstNetworkType] } });
expect(assetsController.state.tokens[0]).toEqual({
address: '0xfoO',
decimals: 2,
Expand Down Expand Up @@ -204,13 +204,13 @@ describe('AssetsController', () => {
it('should remove token by provider type', async () => {
const firstNetworkType = 'rinkeby';
const secondNetworkType = 'ropsten';
network.update({ provider: { type: firstNetworkType } });
network.update({ provider: { type: firstNetworkType, chainId: NetworksChainId[firstNetworkType] } });
await assetsController.addToken('fou', 'baz', 2);
network.update({ provider: { type: secondNetworkType } });
network.update({ provider: { type: secondNetworkType, chainId: NetworksChainId[secondNetworkType] } });
await assetsController.addToken('foo', 'bar', 2);
assetsController.removeToken('0xfoO');
expect(assetsController.state.tokens).toHaveLength(0);
network.update({ provider: { type: firstNetworkType } });
network.update({ provider: { type: firstNetworkType, chainId: NetworksChainId[firstNetworkType] } });
expect(assetsController.state.tokens[0]).toEqual({
address: '0xFOu',
decimals: 2,
Expand Down Expand Up @@ -310,11 +310,11 @@ describe('AssetsController', () => {
sandbox
.stub(assetsController, 'getCollectibleInformation' as any)
.returns({ name: 'name', image: 'url', description: 'description' });
network.update({ provider: { type: firstNetworkType } });
network.update({ provider: { type: firstNetworkType, chainId: NetworksChainId[firstNetworkType] } });
await assetsController.addCollectible('foo', 1234);
network.update({ provider: { type: secondNetworkType } });
network.update({ provider: { type: secondNetworkType, chainId: NetworksChainId[secondNetworkType] } });
expect(assetsController.state.collectibles).toHaveLength(0);
network.update({ provider: { type: firstNetworkType } });
network.update({ provider: { type: firstNetworkType, chainId: NetworksChainId[firstNetworkType] } });
expect(assetsController.state.collectibles[0]).toEqual({
address: '0xfoO',
description: 'description',
Expand Down Expand Up @@ -393,14 +393,14 @@ describe('AssetsController', () => {
.returns({ name: 'name', image: 'url', description: 'description' });
const firstNetworkType = 'rinkeby';
const secondNetworkType = 'ropsten';
network.update({ provider: { type: firstNetworkType } });
network.update({ provider: { type: firstNetworkType, chainId: NetworksChainId[firstNetworkType] } });
await assetsController.addCollectible('fou', 4321);
network.update({ provider: { type: secondNetworkType } });
network.update({ provider: { type: secondNetworkType, chainId: NetworksChainId[secondNetworkType] } });
await assetsController.addCollectible('foo', 1234);
assetsController.removeToken('0xfoO');
assetsController.removeCollectible('0xfoO', 1234);
expect(assetsController.state.collectibles).toHaveLength(0);
network.update({ provider: { type: firstNetworkType } });
network.update({ provider: { type: firstNetworkType, chainId: NetworksChainId[firstNetworkType] } });
expect(assetsController.state.collectibles[0]).toEqual({
address: '0xFOu',
description: 'description',
Expand All @@ -415,7 +415,7 @@ describe('AssetsController', () => {
const address = '0x123';
preferences.update({ selectedAddress: address });
expect(assetsController.context.PreferencesController.state.selectedAddress).toEqual(address);
network.update({ provider: { type: networkType } });
network.update({ provider: { type: networkType, chainId: NetworksChainId[networkType] } });
expect(assetsController.context.NetworkController.state.provider.type).toEqual(networkType);
});

Expand Down Expand Up @@ -445,7 +445,7 @@ describe('AssetsController', () => {
)
.catch((error) => {
expect(error.message).toContain('Asset of type ERC721 not supported');
resolve();
resolve('');
});
});
});
Expand All @@ -467,7 +467,7 @@ describe('AssetsController', () => {
});
result.catch((error) => {
expect(error.message).toContain('User rejected to watch the asset.');
resolve();
resolve('');
});
});
});
Expand All @@ -485,7 +485,7 @@ describe('AssetsController', () => {
result.then((res) => {
expect(assetsController.state.suggestedAssets).toHaveLength(0);
expect(res).toBe('0xe9f786dfdd9ae4d57e830acb52296837765f0e5b');
resolve();
resolve('');
});
await assetsController.acceptWatchAsset(suggestedAssetMeta.id);
});
Expand All @@ -509,7 +509,7 @@ describe('AssetsController', () => {
await assetsController.acceptWatchAsset(suggestedAssetMeta.id);
result.catch((error) => {
expect(error.message).toContain('Asset of type ERC721 not supported');
resolve();
resolve('');
});
});
});
Expand Down
10 changes: 5 additions & 5 deletions tests/AssetsDetectionController.test.ts
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { createSandbox, stub } from 'sinon';
import { getOnce, get } from 'fetch-mock';
import { AssetsDetectionController } from '../src/assets/AssetsDetectionController';
import { NetworkController } from '../src/network/NetworkController';
import { NetworkController, NetworksChainId } from '../src/network/NetworkController';
import { PreferencesController } from '../src/user/PreferencesController';
import { ComposableController } from '../src/ComposableController';
import { AssetsController } from '../src/assets/AssetsController';
Expand Down Expand Up @@ -189,7 +189,7 @@ describe('AssetsDetectionController', () => {
expect(mockCollectibles.calledTwice).toBe(true);
mockTokens.restore();
mockCollectibles.restore();
resolve();
resolve('');
}, 15);
});
});
Expand All @@ -210,7 +210,7 @@ describe('AssetsDetectionController', () => {
expect(mockCollectibles.called).toBe(false);
mockTokens.restore();
mockCollectibles.restore();
resolve();
resolve('');
});
});

Expand Down Expand Up @@ -444,9 +444,9 @@ describe('AssetsDetectionController', () => {
expect(detectAssets.calledTwice).toBe(false);
preferences.update({ selectedAddress: firstAddress });
expect(assetsDetection.context.PreferencesController.state.selectedAddress).toEqual(firstAddress);
network.update({ provider: { type: secondNetworkType } });
network.update({ provider: { type: secondNetworkType, chainId: NetworksChainId[secondNetworkType] } });
expect(assetsDetection.context.NetworkController.state.provider.type).toEqual(secondNetworkType);
network.update({ provider: { type: firstNetworkType } });
network.update({ provider: { type: firstNetworkType, chainId: NetworksChainId[firstNetworkType] } });
expect(assetsDetection.context.NetworkController.state.provider.type).toEqual(firstNetworkType);
assets.update({ tokens: TOKENS });
expect(assetsDetection.config.tokens).toEqual(TOKENS);
Expand Down
8 changes: 4 additions & 4 deletions tests/ComposableController.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import ComposableController from '../src/ComposableController';
import PreferencesController from '../src/user/PreferencesController';
import TokenRatesController from '../src/assets/TokenRatesController';
import { AssetsController } from '../src/assets/AssetsController';
import { NetworkController } from '../src/network/NetworkController';
import { NetworkController, NetworksChainId } from '../src/network/NetworkController';
import { AssetsContractController } from '../src/assets/AssetsContractController';
import CurrencyRateController from '../src/assets/CurrencyRateController';

Expand Down Expand Up @@ -47,7 +47,7 @@ describe('ComposableController', () => {
},
NetworkController: {
network: 'loading',
provider: { type: 'mainnet' },
provider: { type: 'mainnet', chainId: NetworksChainId.mainnet },
},
PreferencesController: {
featureFlags: {},
Expand Down Expand Up @@ -93,7 +93,7 @@ describe('ComposableController', () => {
lostIdentities: {},
nativeCurrency: 'ETH',
network: 'loading',
provider: { type: 'mainnet' },
provider: { type: 'mainnet', chainId: NetworksChainId.mainnet },
selectedAddress: '',
suggestedAssets: [],
tokens: [],
Expand Down Expand Up @@ -147,7 +147,7 @@ describe('ComposableController', () => {
lostIdentities: {},
nativeCurrency: 'ETH',
network: 'loading',
provider: { type: 'mainnet' },
provider: { type: 'mainnet', chainId: NetworksChainId.mainnet },
selectedAddress: '',
suggestedAssets: [],
tokens: [],
Expand Down
Loading

0 comments on commit 23cdb2d

Please sign in to comment.