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

Toggle option to enable/disable balance and Token rate checking for using third-party API #16772

Merged
merged 59 commits into from
Jan 17, 2023
Merged
Show file tree
Hide file tree
Changes from 54 commits
Commits
Show all changes
59 commits
Select commit Hold shift + click to select a range
3c23abe
adding a new toggle to enable and disable conversion rate checking us…
NiranjanaBinoy Dec 1, 2022
8b572f8
changes in controllers to enable and disable fetch and in ui to rest…
NiranjanaBinoy Dec 2, 2022
7632c12
updating the links and messages
NiranjanaBinoy Dec 2, 2022
9bdbfe4
updating metamask-controller based on new controller changes
NiranjanaBinoy Dec 2, 2022
7bfd261
bumping the asset-controller package and restricting ui based on useC…
NiranjanaBinoy Dec 7, 2022
ad0ed74
reorganizing the links
NiranjanaBinoy Dec 7, 2022
4fca64d
fixing unit test failure
NiranjanaBinoy Dec 7, 2022
1454da7
lavamoat fix
NiranjanaBinoy Dec 8, 2022
5b8cdae
updating yarn.lock file
NiranjanaBinoy Dec 8, 2022
f8cd35d
after running lockfile dedupe fix
NiranjanaBinoy Dec 8, 2022
56c8e27
fixing circle ci failures
NiranjanaBinoy Dec 8, 2022
248fd89
updating fixture data with useCurrencyRateCheck: true to fix e2e failure
NiranjanaBinoy Dec 8, 2022
17b05a8
after rebase and lint fix
NiranjanaBinoy Dec 8, 2022
4583e90
fixing circle ci failures
NiranjanaBinoy Dec 8, 2022
7991f04
e2e test failure fix
NiranjanaBinoy Dec 8, 2022
4128091
addressing review comments
NiranjanaBinoy Dec 13, 2022
7c8fc08
Fix yarn.lock
danjm Dec 14, 2022
1c6f85a
Re-add test assertion in edit-gas-fee.spec.js
danjm Dec 14, 2022
bc9c843
fixing unit test failure
NiranjanaBinoy Dec 14, 2022
1530ca3
fixing e2e failure
NiranjanaBinoy Dec 14, 2022
0fd2b26
updating e2e test failures
NiranjanaBinoy Dec 16, 2022
fc7cb84
removing commented lines
NiranjanaBinoy Dec 16, 2022
e1b4459
lint fix
NiranjanaBinoy Dec 16, 2022
a976034
merge
jpuri Dec 21, 2022
a3ef5ab
lint fix
jpuri Dec 21, 2022
4d6aa0d
fix
jpuri Dec 21, 2022
9b723d0
merge
jpuri Dec 21, 2022
f55e5b7
Merge branch 'develop' into optional-currency-conversion
jpuri Dec 21, 2022
420dee9
Update app/_locales/en/messages.json
jpuri Dec 22, 2022
5f0ad29
fix
jpuri Dec 22, 2022
814eb10
Merge branch 'optional-currency-conversion' of github.com:MetaMask/me…
jpuri Dec 22, 2022
375515e
Merge branch 'develop' into optional-currency-conversion
jpuri Dec 22, 2022
93f8097
fix
jpuri Dec 22, 2022
d1d704e
Merge branch 'optional-currency-conversion' of github.com:MetaMask/me…
jpuri Dec 22, 2022
e15139b
fix
jpuri Dec 22, 2022
237a673
Merge branch 'develop' into optional-currency-conversion
digiwand Dec 22, 2022
363e860
Merge branch 'develop' into optional-currency-conversion
jpuri Dec 22, 2022
3e973bd
updating asset-controller package version
NiranjanaBinoy Jan 5, 2023
6581495
ran lavamoat:auto:ci
NiranjanaBinoy Jan 5, 2023
7ae2936
Merge branch 'develop' into optional-currency-conversion
NiranjanaBinoy Jan 6, 2023
6774504
Merge branch 'develop' into optional-currency-conversion
NiranjanaBinoy Jan 6, 2023
8419784
lavamoat
NiranjanaBinoy Jan 6, 2023
a68e5b5
fixing lavamoat failure
NiranjanaBinoy Jan 9, 2023
9447f96
Merge branch 'develop' into optional-currency-conversion
NiranjanaBinoy Jan 9, 2023
010138f
Merge branch 'develop' into optional-currency-conversion
NiranjanaBinoy Jan 10, 2023
822a3ac
Merge branch 'develop' into optional-currency-conversion
NiranjanaBinoy Jan 10, 2023
4a8d47a
Merge branch 'develop' into optional-currency-conversion
NiranjanaBinoy Jan 11, 2023
8b03515
rearranging the toggle and fixing search issue
NiranjanaBinoy Jan 11, 2023
9e68f23
updating the snapshot
NiranjanaBinoy Jan 11, 2023
e73afb8
adding a comment
NiranjanaBinoy Jan 11, 2023
cc1948d
turning the toggle ON by default
NiranjanaBinoy Jan 12, 2023
14733ce
updating useCurrencyRate check on swaps
NiranjanaBinoy Jan 13, 2023
a144ec9
Merge branch 'develop' into optional-currency-conversion
NiranjanaBinoy Jan 13, 2023
f037623
updating yarn.lock file
NiranjanaBinoy Jan 13, 2023
a1a8c65
updating swaps wuotes details
NiranjanaBinoy Jan 16, 2023
5127185
reverting the change in jest.config file
NiranjanaBinoy Jan 16, 2023
120784b
updating to maxFeeInEth
NiranjanaBinoy Jan 16, 2023
1ac87ae
Merge branch 'develop' into optional-currency-conversion
NiranjanaBinoy Jan 16, 2023
8b96364
updating test
NiranjanaBinoy Jan 16, 2023
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
1 change: 1 addition & 0 deletions .storybook/test-data.js
Original file line number Diff line number Diff line change
Expand Up @@ -1225,6 +1225,7 @@ const state = {
useNonceField: false,
usePhishDetect: true,
useTokenDetection: true,
useCurrencyRateCheck: true,
lostIdentities: {},
forgottenPassword: false,
ipfsGateway: 'dweb.link',
Expand Down
13 changes: 13 additions & 0 deletions app/_locales/en/messages.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion app/scripts/controllers/detect-tokens.js
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ export default class DetectTokensController {
const tokensToDetect = [];
for (const tokenAddress in tokenListUsed) {
if (
!this.tokenAddresses.find(({ address }) =>
!this.tokenAddresses.find((address) =>
digiwand marked this conversation as resolved.
Show resolved Hide resolved
isEqualCaseInsensitive(address, tokenAddress),
) &&
!this.hiddenTokens.find((address) =>
NiranjanaBinoy marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
27 changes: 24 additions & 3 deletions app/scripts/controllers/detect-tokens.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,9 @@ import {
} from '@metamask/assets-controllers';
import { NETWORK_TYPES } from '../../../shared/constants/network';
import { toChecksumHexAddress } from '../../../shared/modules/hexstring-utils';
import { hexToDecimal } from '../../../shared/lib/metamask-controller-utils';
import DetectTokensController from './detect-tokens';
import NetworkController from './network';
import NetworkController, { NETWORK_EVENTS } from './network';
import PreferencesController from './preferences';

describe('DetectTokensController', function () {
Expand Down Expand Up @@ -64,14 +65,34 @@ describe('DetectTokensController', function () {
onPreferencesStateChange: preferences.store.subscribe.bind(
preferences.store,
),
onNetworkStateChange: network.store.subscribe.bind(network.store),
onNetworkStateChange: (cb) =>
network.store.subscribe((networkState) => {
const modifiedNetworkState = {
...networkState,
providerConfig: {
...networkState.provider,
},
};
return cb(modifiedNetworkState);
}),
});

assetsContractController = new AssetsContractController({
onPreferencesStateChange: preferences.store.subscribe.bind(
preferences.store,
),
onNetworkStateChange: network.store.subscribe.bind(network.store),
onNetworkStateChange: (cb) =>
network.on(NETWORK_EVENTS.NETWORK_DID_CHANGE, () => {
const networkState = network.store.getState();
const modifiedNetworkState = {
...networkState,
providerConfig: {
...networkState.provider,
chainId: hexToDecimal(networkState.provider.chainId),
},
};
return cb(modifiedNetworkState);
}),
});

sandbox
Expand Down
10 changes: 10 additions & 0 deletions app/scripts/controllers/preferences.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ export default class PreferencesController {
// set to false will be using the static list from contract-metadata
useTokenDetection: false,
useNftDetection: false,
useCurrencyRateCheck: true,
openSeaEnabled: false,
advancedGasFee: null,

Expand Down Expand Up @@ -156,6 +157,15 @@ export default class PreferencesController {
this.store.updateState({ useNftDetection });
}

/**
* Setter for the `useCurrencyRateCheck` property
*
* @param {boolean} val - Whether or not the user prefers to use currency rate check for ETH and tokens.
*/
setUseCurrencyRateCheck(val) {
this.store.updateState({ useCurrencyRateCheck: val });
}

/**
* Setter for the `openSeaEnabled` property
*
Expand Down
19 changes: 19 additions & 0 deletions app/scripts/controllers/preferences.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -367,4 +367,23 @@ describe('preferences controller', function () {
assert.equal(preferencesController.store.getState().theme, 'dark');
});
});

describe('setUseCurrencyRateCheck', function () {
it('should default to false', function () {
const state = preferencesController.store.getState();
assert.equal(state.useCurrencyRateCheck, true);
});

it('should set the useCurrencyRateCheck property in state', function () {
assert.equal(
preferencesController.store.getState().useCurrencyRateCheck,
true,
);
preferencesController.setUseCurrencyRateCheck(false);
assert.equal(
preferencesController.store.getState().useCurrencyRateCheck,
false,
);
});
});
});
66 changes: 54 additions & 12 deletions app/scripts/metamask-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ export default class MetamaskController extends EventEmitter {
this.networkController.store.subscribe((networkState) => {
const modifiedNetworkState = {
...networkState,
provider: {
providerConfig: {
...networkState.provider,
chainId: hexToDecimal(networkState.provider.chainId),
},
Expand All @@ -291,9 +291,16 @@ export default class MetamaskController extends EventEmitter {
onPreferencesStateChange: this.preferencesController.store.subscribe.bind(
this.preferencesController.store,
),
onNetworkStateChange: this.networkController.store.subscribe.bind(
this.networkController.store,
),
onNetworkStateChange: (cb) =>
this.networkController.store.subscribe((networkState) => {
const modifiedNetworkState = {
...networkState,
providerConfig: {
...networkState.provider,
},
};
return cb(modifiedNetworkState);
}),
config: { provider: this.provider },
state: initState.TokensController,
});
Expand All @@ -307,7 +314,7 @@ export default class MetamaskController extends EventEmitter {
const networkState = this.networkController.store.getState();
const modifiedNetworkState = {
...networkState,
provider: {
providerConfig: {
...networkState.provider,
chainId: hexToDecimal(networkState.provider.chainId),
},
Expand All @@ -327,9 +334,16 @@ export default class MetamaskController extends EventEmitter {
this.preferencesController.store.subscribe.bind(
this.preferencesController.store,
),
onNetworkStateChange: this.networkController.store.subscribe.bind(
this.networkController.store,
),
onNetworkStateChange: (cb) =>
this.networkController.store.subscribe((networkState) => {
const modifiedNetworkState = {
...networkState,
providerConfig: {
...networkState.provider,
},
};
return cb(modifiedNetworkState);
}),
getERC721AssetName:
this.assetsContractController.getERC721AssetName.bind(
this.assetsContractController,
Expand Down Expand Up @@ -504,17 +518,37 @@ export default class MetamaskController extends EventEmitter {
this.networkController.store.subscribe((networkState) => {
const modifiedNetworkState = {
...networkState,
provider: {
providerConfig: {
...networkState.provider,
chainId: hexToDecimal(networkState.provider.chainId),
},
};
return cb(modifiedNetworkState);
}),
},
undefined,
{
disabled:
!this.preferencesController.store.getState().useCurrencyRateCheck,
},
initState.TokenRatesController,
);
this.preferencesController.store.subscribe(
previousValueComparator((prevState, currState) => {
const { useCurrencyRateCheck: prevUseCurrencyRateCheck } = prevState;
const { useCurrencyRateCheck: currUseCurrencyRateCheck } = currState;
if (currUseCurrencyRateCheck && !prevUseCurrencyRateCheck) {
this.currencyRateController.start();
this.tokenRatesController.configure(
{ disabled: false },
false,
false,
);
} else if (!currUseCurrencyRateCheck && prevUseCurrencyRateCheck) {
this.currencyRateController.stop();
this.tokenRatesController.configure({ disabled: true }, false, false);
}
}, this.preferencesController.store.getState()),
);
Copy link
Contributor

Choose a reason for hiding this comment

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

Code above can better be written in tokenRatesController. I would avoid putting code in metamaskController.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Introducing these changes in tokenRatecontroller will need an introduction of useCurrencyRateCheck to Preferencecontroller in the core repo, which is redundant as we are not using a separate Preferencecontroller in the extension. This way, we can minimize the extension-specific code in the core repo.


this.ensController = new EnsController({
provider: this.provider,
Expand Down Expand Up @@ -1245,7 +1279,9 @@ export default class MetamaskController extends EventEmitter {
triggerNetworkrequests() {
this.accountTracker.start();
this.incomingTransactionsController.start();
this.currencyRateController.start();
if (this.preferencesController.store.getState().useCurrencyRateCheck) {
this.currencyRateController.start();
}
if (this.preferencesController.store.getState().useTokenDetection) {
this.tokenListController.start();
}
Expand All @@ -1254,7 +1290,9 @@ export default class MetamaskController extends EventEmitter {
stopNetworkRequests() {
this.accountTracker.stop();
this.incomingTransactionsController.stop();
this.currencyRateController.stop();
if (this.preferencesController.store.getState().useCurrencyRateCheck) {
this.currencyRateController.stop();
}
if (this.preferencesController.store.getState().useTokenDetection) {
this.tokenListController.stop();
}
Expand Down Expand Up @@ -1634,6 +1672,10 @@ export default class MetamaskController extends EventEmitter {
setUseNftDetection: preferencesController.setUseNftDetection.bind(
preferencesController,
),
setUseCurrencyRateCheck:
preferencesController.setUseCurrencyRateCheck.bind(
preferencesController,
),
setOpenSeaEnabled: preferencesController.setOpenSeaEnabled.bind(
preferencesController,
),
Expand Down
3 changes: 2 additions & 1 deletion lavamoat/browserify/beta/policy.json
Original file line number Diff line number Diff line change
Expand Up @@ -501,17 +501,18 @@
"URL": true,
"clearInterval": true,
"clearTimeout": true,
"console.info": true,
"console.log": true,
"setInterval": true,
"setTimeout": true
},
"packages": {
"@ethersproject/contracts": true,
"@ethersproject/providers": true,
"@metamask/assets-controllers>@metamask/contract-metadata": true,
"@metamask/assets-controllers>abort-controller": true,
"@metamask/assets-controllers>multiformats": true,
"@metamask/base-controller": true,
"@metamask/contract-metadata": true,
"@metamask/controller-utils": true,
"@metamask/metamask-eth-abis": true,
"browserify>events": true,
Expand Down
3 changes: 2 additions & 1 deletion lavamoat/browserify/flask/policy.json
Original file line number Diff line number Diff line change
Expand Up @@ -501,17 +501,18 @@
"URL": true,
"clearInterval": true,
"clearTimeout": true,
"console.info": true,
"console.log": true,
"setInterval": true,
"setTimeout": true
},
"packages": {
"@ethersproject/contracts": true,
"@ethersproject/providers": true,
"@metamask/assets-controllers>@metamask/contract-metadata": true,
"@metamask/assets-controllers>abort-controller": true,
"@metamask/assets-controllers>multiformats": true,
"@metamask/base-controller": true,
"@metamask/contract-metadata": true,
"@metamask/controller-utils": true,
"@metamask/metamask-eth-abis": true,
"browserify>events": true,
Expand Down
3 changes: 2 additions & 1 deletion lavamoat/browserify/main/policy.json
Original file line number Diff line number Diff line change
Expand Up @@ -501,17 +501,18 @@
"URL": true,
"clearInterval": true,
"clearTimeout": true,
"console.info": true,
"console.log": true,
"setInterval": true,
"setTimeout": true
},
"packages": {
"@ethersproject/contracts": true,
"@ethersproject/providers": true,
"@metamask/assets-controllers>@metamask/contract-metadata": true,
"@metamask/assets-controllers>abort-controller": true,
"@metamask/assets-controllers>multiformats": true,
"@metamask/base-controller": true,
"@metamask/contract-metadata": true,
"@metamask/controller-utils": true,
"@metamask/metamask-eth-abis": true,
"browserify>events": true,
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@
"@metamask/address-book-controller": "^1.0.0",
"@metamask/announcement-controller": "^1.0.0",
"@metamask/approval-controller": "^1.0.0",
"@metamask/assets-controllers": "^1.0.1",
"@metamask/assets-controllers": "^3.0.1",
"@metamask/base-controller": "^1.0.0",
"@metamask/contract-metadata": "^2.1.0",
"@metamask/controller-utils": "^1.0.0",
Expand Down
5 changes: 5 additions & 0 deletions shared/lib/ui-utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,11 @@ _supportLink = 'https://metamask-flask.zendesk.com/hc';
///: END:ONLY_INCLUDE_IN

export const SUPPORT_LINK = _supportLink;

export const COINGECKO_LINK = 'https://www.coingecko.com/';
export const CRYPTOCOMPARE_LINK = 'https://www.cryptocompare.com/';
export const PRIVACY_POLICY_LINK = 'https://consensys.net/privacy-policy/';

// TODO make sure these links are correct
export const ETHERSCAN_PRIVACY_LINK = 'https://etherscan.io/privacyPolicy';
export const CONSENSYS_PRIVACY_LINK = 'https://consensys.net/privacy-policy/';
Expand Down
1 change: 1 addition & 0 deletions test/data/mock-state.json
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,7 @@
"unapprovedTypedMessages": {},
"unapprovedTypedMessagesCount": 0,
"useTokenDetection": true,
"useCurrencyRateCheck": true,
"advancedGasFee": {
"maxBaseFee": "75",
"priorityFee": "2"
Expand Down
2 changes: 2 additions & 0 deletions test/e2e/fixture-builder.js
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,7 @@ function defaultFixture() {
useNonceField: false,
usePhishDetect: true,
useTokenDetection: false,
useCurrencyRateCheck: true,
useMultiAccountBalanceChecker: true,
},
SmartTransactionsController: {
Expand Down Expand Up @@ -352,6 +353,7 @@ function onboardingFixture() {
useNonceField: false,
usePhishDetect: true,
useTokenDetection: false,
useCurrencyRateCheck: true,
useMultiAccountBalanceChecker: true,
},
SmartTransactionsController: {
Expand Down
1 change: 1 addition & 0 deletions test/e2e/restore/MetaMaskUserData.json
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
"useNonceField": false,
"usePhishDetect": true,
"useTokenDetection": false,
"useCurrencyRateCheck": true,
"useMultiAccountBalanceChecker": true
}
}
1 change: 1 addition & 0 deletions test/jest/mock-store.js
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,7 @@ export const createSwapsMockStore = () => {
postTxBalance: '19a61aaaf06e4bd1',
},
],
useCurrencyRateCheck: true,
conversionRate: 1,
contractExchangeRates: {
'0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48': 2,
Expand Down
Loading