An attempt to safely release the `nonceLock` upon failure has instead
made failure worse by masking it with a new error. If the call to get
the `nonceLock` throws an exception, then the `finally` block here
would attempt to call `releaseLock` on the `nonceLock` variable, which
is guaranteed to be `undefined` if the previous call failed. The
attempt to call a method on `undefined` throws another error, masking
the original error.
It is safer to obtain the `nonceLock` and release it without using any
`try` or `finally` block. The `nonceLock` is synchronously released
immediately after it is obtained, and any errors bubble up correctly
without being masked. There is no case where the lock is left
unreleased.
If the `signTypedData` background function threw an exception, it would
return `undefined` to the UI, which would throw another exception in
the UI. It now re-throws the error if an error is thrown, which
allows the UI to handle the error.
I'm not sure why this might fail, and I'm not sure we're handling this
failure well, but this is an improvement at least.
* Add data point to 'Swaps Completed' segment event: estimated vs used gas
* Linted
* Correct property name for estimated gas on swapMetaData in _trackSwapsMetrics()
* Set estimated_gas property on swapMetaData to a hex string
* Correct base when dividing by estimated_gas
Co-authored-by: Dan Finlay <dan@danfinlay.com>
* Delete CachedBalancesController.cachedBalances
* Migrate provider to Rinkeby instead of deleting it
* Convert hex transaction metamaskNetworkId values to decimal
* Don't migrate provider state in e2e tests
* Don't kick custom RPC users to Rinkeby unnecessarily
* Use provider.chainId for address book chainId values
* Add address book migration
* Fix failing unit test
* fixup! Merge branch 'develop' into address-book-use-chainId
* Select address book entries for display by chainId
* Merge all address book entry keys
* fixup! Merge all address book entry keys
* Delete localhost provider type
* Use ganache-cli default chain ID for tests
* Delete unused test firstTimeState variable
* Migrate default ganache-cli network to frequentRpcListDetail
* Add default test provider state
* Add test functionality to createJsonRpcClient
* Lint locales
* Update test middleware creation
* fixup! Update test middleware creation
* Update txMeta after postTxBalance has been retrieved
* Use gas used from txReceipt to calculate eth received
* Return null from getSwapsTokensReceivedFromTxMeta in tokenSymbol is ETH and txReceipt is missing
* Get latest txMeta before updating it with postTxBalance in case of a swaps tx in confirmTransaction
* Lint fix
* Use initial transaction for settings swap transaction title params, and remove addition of swap properties to cancel transcations
* Update unit test data
* Use token symbol properties from initial transaction for filitering in transaction list
* call this.txStateManager.setTxStatusConfirmed before async call in confirmTransaction in the transactions controller
* Clone txMeta before setTxStatusConfirmed in confirmTransaction
* Correctly updateTx in confirmTransaction
* Track swaps event only if it is a swap transaction
* Create wrapper function for segment events
* Extract transaction controller metrics calls into own function
Co-authored-by: Mark Stacey <markjstacey@gmail.com>
* Stop passing a gas param to the estimateGas call initiated in the swaps controller timedoutGasReturn
* Stop passing gas params to timedoutGasReturn
* Lint fix
* Stop passing no longer used param to setInitialGasEstimate
When the `chainId` for a custom RPC endpoint is edited, we now migrate
the corresponding address book entries to ensure they are not orphaned.
The address book entries are grouped by the `metamask.network` state,
which unfortunately was sometimes the `chainId`, and sometimes the
`networkId`. It was always the `networkId` for built-in Infura
networks, but for custom RPC endpoints it would be set to the user-set
`chainId` field, with a fallback to the `networkId` of the network.
A recent change will force users to enter valid `chainId`s on all
custom networks, which will be normalized to be hex-prefixed. As a
result, address book contacts will now be keyed by a different string.
The contact entries are now migrated when this edit takes place.
There are some edge cases where two separate entries share the same set
of contacts. For example, if two entries have the same `chainId`, or if
they had the same `networkId` and had no `chainId` set. When the
`chainId` is edited in such cases, the contacts are duplicated on both
networks. This is the best we can do, as we don't have any way to know
which network the contacts _should_ be on.
The `typed-message-manager` unit tests have also been updated as part
of this commit because the addition of `sinon.restore()` to the
preferences controller tests ended up clearing a test object in-between
individual tests in that file. The test object is now re-constructed
before each individual test.
The `_fetchAndSetSwapsLiveness` was accidentally passed to
`setInterval` without being bound first, so the `this` reference was
not defined when it was called. It is now bound before being passed to
`setInterval`.
* Remove network config store
* Remove inline networks variable in network controller
* Re-key network controller 'rpcTarget' to 'rpcUrl'
* Require chainId in lookupNetwork, implement eth_chainId
* Require chain ID in network form
* Add alert, migrations, and tests
* Add chainId validation to addToFrequentRpcList
* Update public config state selector to match new network controller
state
* Use network enums in networks-tab.constants
* Ensure chainId in provider config is current
* Update tests
Right now when editing an address in "Settings > Contact", the contact
is lost after saving. This is because the code awaits
`removeFromAddressBook()` before creating the new contact but
`removeFromAddressBook()` never resolves. This change fixes this bug.
The web3 usage metrics added in #9144 assumed that all web3 properties
were strings. When a `Symbol` property is accessed, our `inpage.js`
script crashes because the `Symbol` cannot be serialized correctly.
A check has been added for non-string property access. The metric event
in these cases is set to the string "typeof ", followed by the type of
the key. (e.g. `typeof symbol` for a `Symbol` property).
Fixes#9234
The web3 usage metrics added in #9144 assumed that all web3 properties
were strings. When a `Symbol` property is accessed, our `inpage.js`
script crashes because the `Symbol` cannot be serialized correctly.
A check has been added for non-string property access. The metric event
in these cases is set to the string "typeof ", followed by the type of
the key. (e.g. `typeof symbol` for a `Symbol` property).
Fixes#9234
Mark Stacey
Dan J Miller
Erik Marks
Whymarrh Whitby
Patryk Łucka
Pedro Pablo Aste Kompen
Brad Decker
Kevin Holder