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

Jump to bottom

Enforce chainId #324

Merged
merged 11 commits into from
Jan 19, 2021
Merged

Enforce chainId #324

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
dd9b09d
chainif
estebanmino Dec 17, 2020
2c39def
txcontroller
estebanmino Dec 17, 2020
ef70593
not default
estebanmino Dec 17, 2020
20b0adb
mutex
estebanmino Dec 17, 2020
fcea632
test
estebanmino Jan 13, 2021
2fad094
Revert "mutex"
estebanmino Jan 13, 2021
aa0dcc7
tests
estebanmino Jan 13, 2021
3ced465
fulltest
estebanmino Jan 13, 2021
2af5c3c
format
estebanmino Jan 13, 2021
130e079
comments
estebanmino Jan 15, 2021
939e331
Merge branch 'develop' into enforce-chain-id
estebanmino Jan 19, 2021
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
20 changes: 15 additions & 5 deletions src/network/NetworkController.ts
View file
Open in desktop
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 = '',
Comment on lines +15 to +21
Copy link
Member

Choose a reason for hiding this comment

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

Could we store these in hexadecimal instead?

Possibly related: #326

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'll do it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh wait, this would need a migration as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

for transactions, I'd put that together with the work on the tx controller

Copy link
Member

Choose a reason for hiding this comment

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

We decided to do this in a follow-up.

}

/**
* @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
Open in desktop
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,
rekmarks marked this conversation as resolved.
Show resolved Hide resolved
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
Open in desktop
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
Open in desktop
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
Open in desktop
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