fix: throw correct error on empty or invalid address

[mikesposito]

Description

Since @metamask/utils has been updated to the latest version, normalize method does not return undefined for empty strings anymore.

getKeyringForAccount checks if the provided address is an empty string (or undefined) and returns an error in case. But as all the other methods of the controller normalize the address before calling getKeyringForAccount what we check in there is 0x, which is not seen as invalid/empty by the function.

To fix the problem and provide a correct error message, this PR uses isValidHexAddress from @metamask/utils to check if the string passed to getKeyringForAccount is a valid address.

Changes

• FIXED: getKeyringForAccount provides a correct error message when an address is invalid

References

• Relates to #1315

Checklist

• I've updated the test suite for new or updated code as appropriate
• I've updated documentation for new or updated code as appropriate (note: this will usually be JSDoc)
• I've highlighted breaking changes using the "BREAKING" category above as appropriate

[socket-security]

No top level dependency changes detected. Learn more about Socket for GitHub ↗︎

[socket-security]

👍 Dependency issues cleared. Learn more about Socket for GitHub ↗︎

This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored.

[mikesposito]

Could be worth waiting for MetaMask/utils#112 to be merged and released as @metamask/controller-utils will not be needed anymore

[legobeat]

Added minor bump of @ethereumjs/tx to keep transitive dependency versions consolidated.

[mikesposito]

LGTM but can we add a test-case for at least one empty/falsey case (ie covering for when the previous !address on line 719 would hold)?

Shouldn't it be still covered by the test case throws error when address is not provided (L715)?

[legobeat]

LGTM but can we add a test-case for at least one empty/falsey case (ie covering for when the previous !address on line 719 would hold)?

Shouldn't it be still covered by the test case throws error when address is not provided (L715)?

My bad, overlooked it. LGTM then!