-
Notifications
You must be signed in to change notification settings - Fork 5k
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
ENS reverse resolution support #7177
Merged
whymarrh
merged 9 commits into
MetaMask:develop
from
whymarrh:ens-support-reverse-resolution
Nov 1, 2019
Merged
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
e654f19
ENS Reverse Resolution support
whymarrh d48a381
Save punycode for ENS domains with Unicode characters
whymarrh 4e86b71
Clear EnsController store on network change
whymarrh 9cc1d32
Use cached results when reverse-resolving ENS names
whymarrh dfcc465
Update SenderToRecipient recipientEns tooltip
whymarrh cf548b3
Display ENS names in tx activity log
whymarrh 14d2b89
Always subscribe to network changes for EnsController
whymarrh 05b2fff
fixup! ENS Reverse Resolution support
whymarrh 37ba4d8
Cleanup EnsController return values
whymarrh File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
const EthJsEns = require('ethjs-ens') | ||
const ensNetworkMap = require('ethjs-ens/lib/network-map.json') | ||
|
||
class Ens { | ||
static getNetworkEnsSupport (network) { | ||
return Boolean(ensNetworkMap[network]) | ||
} | ||
|
||
constructor ({ network, provider } = {}) { | ||
this._ethJsEns = new EthJsEns({ | ||
network, | ||
provider, | ||
}) | ||
} | ||
|
||
lookup (ensName) { | ||
return this._ethJsEns.lookup(ensName) | ||
} | ||
|
||
reverse (address) { | ||
return this._ethJsEns.reverse(address) | ||
} | ||
} | ||
|
||
module.exports = Ens |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,75 @@ | ||
const ethUtil = require('ethereumjs-util') | ||
const ObservableStore = require('obs-store') | ||
const punycode = require('punycode') | ||
const Ens = require('./ens') | ||
|
||
const ZERO_ADDRESS = '0x0000000000000000000000000000000000000000' | ||
const ZERO_X_ERROR_ADDRESS = '0x' | ||
|
||
class EnsController { | ||
constructor ({ ens, provider, networkStore } = {}) { | ||
const initState = { | ||
ensResolutionsByAddress: {}, | ||
} | ||
|
||
this._ens = ens | ||
if (!this._ens) { | ||
const network = networkStore.getState() | ||
if (Ens.getNetworkEnsSupport(network)) { | ||
this._ens = new Ens({ | ||
network, | ||
provider, | ||
}) | ||
} | ||
} | ||
|
||
this.store = new ObservableStore(initState) | ||
networkStore.subscribe((network) => { | ||
this.store.putState(initState) | ||
this._ens = new Ens({ | ||
network, | ||
provider, | ||
}) | ||
}) | ||
} | ||
|
||
reverseResolveAddress (address) { | ||
return this._reverseResolveAddress(ethUtil.toChecksumAddress(address)) | ||
} | ||
|
||
async _reverseResolveAddress (address) { | ||
if (!this._ens) { | ||
return undefined | ||
} | ||
|
||
const state = this.store.getState() | ||
if (state.ensResolutionsByAddress[address]) { | ||
return state.ensResolutionsByAddress[address] | ||
} | ||
|
||
const domain = await this._ens.reverse(address) | ||
const registeredAddress = await this._ens.lookup(domain) | ||
if (registeredAddress === ZERO_ADDRESS || registeredAddress === ZERO_X_ERROR_ADDRESS) { | ||
return undefined | ||
} | ||
|
||
if (ethUtil.toChecksumAddress(registeredAddress) !== address) { | ||
return undefined | ||
} | ||
|
||
this._updateResolutionsByAddress(address, punycode.toASCII(domain)) | ||
return domain | ||
} | ||
|
||
_updateResolutionsByAddress (address, domain) { | ||
const oldState = this.store.getState() | ||
this.store.putState({ | ||
ensResolutionsByAddress: { | ||
...oldState.ensResolutionsByAddress, | ||
[address]: domain, | ||
}, | ||
}) | ||
} | ||
} | ||
|
||
module.exports = EnsController |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,135 @@ | ||
const assert = require('assert') | ||
const sinon = require('sinon') | ||
const ObservableStore = require('obs-store') | ||
const HttpProvider = require('ethjs-provider-http') | ||
const EnsController = require('../../../../app/scripts/controllers/ens') | ||
|
||
const ZERO_ADDRESS = '0x0000000000000000000000000000000000000000' | ||
const ZERO_X_ERROR_ADDRESS = '0x' | ||
|
||
describe('EnsController', function () { | ||
describe('#constructor', function () { | ||
it('should construct the controller given a provider and a network', async () => { | ||
const provider = new HttpProvider('https://ropsten.infura.io') | ||
const currentNetworkId = '3' | ||
const networkStore = new ObservableStore(currentNetworkId) | ||
const ens = new EnsController({ | ||
provider, | ||
networkStore, | ||
}) | ||
|
||
assert.ok(ens._ens) | ||
}) | ||
|
||
it('should construct the controller given an existing ENS instance', async () => { | ||
const networkStore = { | ||
subscribe: sinon.spy(), | ||
} | ||
const ens = new EnsController({ | ||
ens: {}, | ||
networkStore, | ||
}) | ||
|
||
assert.ok(ens._ens) | ||
}) | ||
}) | ||
|
||
describe('#reverseResolveName', function () { | ||
it('should resolve to an ENS name', async () => { | ||
const address = '0x8e5d75d60224ea0c33d0041e75de68b1c3cb6dd5' | ||
const networkStore = { | ||
subscribe: sinon.spy(), | ||
} | ||
const ens = new EnsController({ | ||
ens: { | ||
reverse: sinon.stub().withArgs(address).returns('peaksignal.eth'), | ||
lookup: sinon.stub().withArgs('peaksignal.eth').returns(address), | ||
}, | ||
networkStore, | ||
}) | ||
|
||
const name = await ens.reverseResolveAddress(address) | ||
assert.equal(name, 'peaksignal.eth') | ||
}) | ||
|
||
it('should only resolve an ENS name once', async () => { | ||
const address = '0x8e5d75d60224ea0c33d0041e75de68b1c3cb6dd5' | ||
const reverse = sinon.stub().withArgs(address).returns('peaksignal.eth') | ||
const lookup = sinon.stub().withArgs('peaksignal.eth').returns(address) | ||
const networkStore = { | ||
subscribe: sinon.spy(), | ||
} | ||
const ens = new EnsController({ | ||
ens: { | ||
reverse, | ||
lookup, | ||
}, | ||
networkStore, | ||
}) | ||
|
||
assert.equal(await ens.reverseResolveAddress(address), 'peaksignal.eth') | ||
assert.equal(await ens.reverseResolveAddress(address), 'peaksignal.eth') | ||
assert.ok(lookup.calledOnce) | ||
assert.ok(reverse.calledOnce) | ||
}) | ||
|
||
it('should fail if the name is registered to a different address than the reverse-resolved', async () => { | ||
const address = '0x8e5d75d60224ea0c33d0041e75de68b1c3cb6dd5' | ||
const networkStore = { | ||
subscribe: sinon.spy(), | ||
} | ||
const ens = new EnsController({ | ||
ens: { | ||
reverse: sinon.stub().withArgs(address).returns('peaksignal.eth'), | ||
lookup: sinon.stub().withArgs('peaksignal.eth').returns('0xfoo'), | ||
}, | ||
networkStore, | ||
}) | ||
|
||
const name = await ens.reverseResolveAddress(address) | ||
assert.strictEqual(name, undefined) | ||
}) | ||
|
||
it('should throw an error when the lookup resolves to the zero address', async () => { | ||
const address = '0x8e5d75d60224ea0c33d0041e75de68b1c3cb6dd5' | ||
const networkStore = { | ||
subscribe: sinon.spy(), | ||
} | ||
const ens = new EnsController({ | ||
ens: { | ||
reverse: sinon.stub().withArgs(address).returns('peaksignal.eth'), | ||
lookup: sinon.stub().withArgs('peaksignal.eth').returns(ZERO_ADDRESS), | ||
}, | ||
networkStore, | ||
}) | ||
|
||
try { | ||
await ens.reverseResolveAddress(address) | ||
assert.fail('#reverseResolveAddress did not throw') | ||
} catch (e) { | ||
assert.ok(e) | ||
} | ||
}) | ||
|
||
it('should throw an error the lookup resolves to the zero x address', async () => { | ||
const address = '0x8e5d75d60224ea0c33d0041e75de68b1c3cb6dd5' | ||
const networkStore = { | ||
subscribe: sinon.spy(), | ||
} | ||
const ens = new EnsController({ | ||
ens: { | ||
reverse: sinon.stub().withArgs(address).returns('peaksignal.eth'), | ||
lookup: sinon.stub().withArgs('peaksignal.eth').returns(ZERO_X_ERROR_ADDRESS), | ||
}, | ||
networkStore, | ||
}) | ||
|
||
try { | ||
await ens.reverseResolveAddress(address) | ||
assert.fail('#reverseResolveAddress did not throw') | ||
} catch (e) { | ||
assert.ok(e) | ||
} | ||
}) | ||
}) | ||
}) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
export { default } from './transaction-list-item-details.component' | ||
export { default } from './transaction-list-item-details.container' |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: it would be nice to know what happened in these cases though - e.g. whether the address didn't resolve, or whether it did but it couldn't be verified, etc.
Maybe a
log.debug
statement would be appropriate in each of these cases? With something like the error messages you had before.