it would be great for Dapp devs to receive those events if a user changes his network or account in metamask
Super important 👍
In the FAQ it's suggested to open an EIP. Maybe EIP is needed eventually but I think it's better to start the discussion here.
Looking into it I suggest to create an event emitter that will emit events on every account/network change.
In case of accounts, the event emitter should emit an accountChanged event in the actions file.
Another question is how to subscribe to that event. Following the logic of web3 modulation I suggest that a subscriber will be added as a web3.eth.accounts.subscribe property. So the developer will do web3.eth.accounts.subcribe('accountChanged')` to subscribe to accounts changes. But not sure how to glue this metamask event emitter into web3.
Issue Status: 1. Open 2. Started 3. Submitted 4. Done
__This issue now has a funding of 3000.0 CLN attached to it.__
@iamonuwa hey, thanks for starting to work on this issue 👍 . As you can see below, I suggested some implementation ideas bellow. Will be glad to hear your thoughts.
Also, I'm not part of core metamask core developers so I cannot approve your work. I opened the bounty to raise attention to this issue, and I've see that it's already working 😏 . With some support from a metamask team I think we can make this happen.
@leonprou LGTM
I've got a few ideas about approaching this one. It looks like I missed the bounty opportunity here, but I still want to share my reaction to the problem statement all the same.
I've noticed that the CyptoKitties web site has an error page when you access their 'My Account' links while using MetaMask on a non-Ehtereum network. Further more, the page responds very quickly to any change of the active metamask network.
In the first few lines of the MetaMask contentscript file, there is a region where _onMessage, _write, and _read methods get assigned to a method prototype. If you set a breakpoint in the debugger just after that assignment, you can easily wrap those methods with interceptors that log the activity. What I see when the page is idle is activity on two Worker ports, named "provider" and "publicConfig". The former channel is used to receive call request and return replies, the latter is used to periodically push an unsolicited configuration report. There is a constant repetition of two exchanges:
This is what happens when a poll from the CryptoKitties in-page element "calls" eth.accounts() through the contentscript worker. The call is in the first message, the response is in the second, and they are correlated by a common id.
OnMessage: {"isTrusted":true,"data":{"target":"contentscript","data":{"name":"provider","data":{"jsonrpc":"2.0","id":1562087762,"method":"eth_accounts","params":[]}}},"origin":"https://www.cryptokitties.co","eventPhase":2,"lastEventId":"","returnValue":true,"type":"message"}
OnMessage: {"isTrusted":true,"data":{"target":"inpage","data":{"name":"provider","data":{"id":1562087762,"jsonrpc":"2.0","result":["0x22f4e425f56d0223b93d3dd9382f8a35a3ff6697"]}}},"origin":"https://www.cryptokitties.co","eventPhase":2,"lastEventId":"","returnValue":true,"type":"message"}
This is what the periodic push on "publicConfig" looks like:
OnMessage: {"isTrusted":true,"data":{"target":"inpage","data":{"name":"publicConfig","data":{"selectedAddress":"0x22f4e425f56d0223b93d3dd9382f8a35a3ff6697","networkVersion":"4"}}},"origin":"https://www.cryptokitties.co","eventPhase":2,"lastEventId":"","returnValue":true,"type":"message"}
If one looks at the first few lines of inpage.js from the MetaMask extension, one can find a block where a Web3 instance is created with a MetamaskInpageProvider. MetaMaskInPageProvider uses a library called "obj-stream" to subscribe to events sent by the contentpage worker. The Cryptokitties page refresh on network change seems to depend on this handler:
t.web3 = new Proxy(e,{
get: function(t, e) {
return o || "currentProvider" === e || (console.warn("MetaMask: web3 will be deprecated in the near future in favor of the ethereumProvider \nhttps://github.com/MetaMask/faq/blob/master/detecting_metamask.md#web3-deprecation"),
o = !0),
a = Date.now(),
t[e]
},
set: function(t, e, r) {
t[e] = r
}
}),
n.subscribe(function(t) {
if (!i) {
var e = t.networkVersion;
if (s) {
if (a && e !== s) {
i = !0;
var n = Date.now() - a;
n > 500 ? r() : setTimeout(r, 500)
}
} else
s = e
}
})
where s is the last known networkVersion and r is a function that calls page reload.
I think the key here is figuring out how to cleanly expose more subscribed events from this obj-store object?
Hmm, stepping back on that last thought a little. That line of reasoning was perhaps a little bit putting the cart before the horse.
The current thought process here is oriented around creating an event emitter inside MetaMask, routing those events into Web3, and then firing triggered events to the developer from Web3.
MetaMask has existing behaviors that show it can detect changes in the network and changes in the account listing, so this sounds like something that could be made to happen. However...
Notice that those behaviors in MetaMask are implemented by polling Web3 from a worker thread. This is to compensate for an absence of the same kinds of events sent from Web3. So, perhaps this would be better staged as a Web3 enhancement to define those events natively in its own infrastructure. That way, MetaMask and Developers alike could benefit from their presence.
This doesn't seem too unreasonable to ask for (again, at face value--the devil is always in the details), since any change of network or account that occurs in MetaMask, or any other Web3 consumer, is ultimately implemented by delegation to a call into Web3, making it authoritative for that state.
Wait a moment... Does MetaMask already provide this feature? It looks like the mechanism I described above that implements polling is already exposed by an EventHandler exposed by the PublicConfigStore object I described interacting with above, and that object is in turn exposed through the MetaMaskInPageProvider.
So, when the current provider is set by MetaMask, the following code works as one would desire, when called with the window web3 as an argument. It's console.log messages will only occur when a change actually happens, even though MetaMask is polling for account/network changes roughy 10 times per second.
function watchForChanges(web3) {
let lastState = web3.currentProvider.publicConfigStore.getState();
function checkForUpdate(event) {
if (event.selectedAddress !== lastState.selectedAddress) {
console.log('Account has change from', lastState.selectedAddress, 'to', event.selectedAddress);
lastState = event;
} else if (event.networkVersion !== lastState.networkVersion) {
console.log('Network has change from', lastState.networkVersion, 'to', event.networkVersion);
lastState = event;
}
}
web3.currentProvider.publicConfigStore.on('update', checkForUpdate);
}
The web3 documentation's stance on this seems to be a recommendation to poll, so, IMHO, if there is any work to be done, its in Web3, and until then MetaMask users should be able to get by with what's already there if they don't want to re-implement polling as suggested by web3.
Relevant Medium article here
@jheinnic wow :astonished:, thanks for so detailed explanation!
Two points:
checkForUpdate event) is working though no ideal. I checked it and noticed that the callback function is called multiple times even if no address or network really changed. You know why? Maybe it's updated multiple times with the same values. But it's do getting called when the change occurs, so I can use this :smiley: .So, MetaMask already provides some mechanism to listen to events, though it can be implemented in a better way. For now, I want to look better into it, @jheinnic can you show the code where MetaMask polls web3?
In EIP 2020, which defines an updated provider API, there is a definition for an event emitter which includes accountChanged and networkChanged. We are currently refactoring our provider (pending #4279) which will allow us to easily support subscriptions including these, and so I don't recommend any bounty hunters attempt this before that refactor is complete.
In case of accounts, the event emitter should emit an accountChanged event in the actions file.
This is incorrect. The linked actions file is for our UI, it does not relate to the injected ethereum provider, which is defined here:
https://github.com/MetaMask/metamask-extension/blob/develop/app/scripts/lib/inpage-provider.js
Wait a moment... Does MetaMask already provide this feature?
No, it does not. You've discovered an internal API, which is subject to change. I wouldn't be sure it lasts over a month more.
Sorry for the apparently slow progress, I assure you things are actually moving, and encourage you to be patient.
@danfinlay, thank you for clarifying the temporary nature of the existing implementation detail! Allow me to attempt to restate what I think I've read in my own words to check my understanding for errors.
EIP 2020 will change the Ethereum Provider contract to provide semantics of accountChanged and networkChanged events and expose the new events via its own EventEmitter. Instead of providing a MetaMask-specific extension to the provider API, MetaMask's own implementation of that contract, the MetamaskInpageProvider, will evolve to satisfy the new standard contract, replacing or deprecating the internal PublicConfigStore interface.
That leaves two workarounds for the interim.
1) Anyone can build their own EventEmitter for accountChanged and networkChanged by caching the last update event from PublicConfigStore, and then using that cached value to filter out updates that do not change the state, selectively re-emitting only those that indicate a change. The upside here is that it does not require a redundant poll. However, the code would be MetaMask specific and subject to break as users begin adopting the first release where EIP 2020 replaces the unsupported PublicConfigStore API.
2) Follow the recommendation found in the MetaMask FAQ that implements polling:
var account = web3.eth.accounts[0];
var accountInterval = setInterval(function() {
if (web3.eth.accounts[0] !== account) {
account = web3.eth.accounts[0];
updateInterface();
}
}, 100);
This can equally well be used to track last-known state and selectively emit events inside the implementation of its updateInterface method. The polling would be redundant with that performed by MetaMask, but the implementation would be less susceptible to breaking for having consumed a non-standard internal API.
In either case, a savvy developer using either workaround would be wise to take a look at EIP 2020 and reuse its event naming and structure in order to minimize effort required to port their workaround to Ethereum Provider supported APIs when they become available.
Responding to network changes requires a little more consideration, given that MetaMask implements a location reload on detecting network changes. In this case, registering for update events from PublicConfigStore does have a noteworthy extra quality. PublicConfigStore emits its update event synchronously, and so a handler listening for them will delay the page refresh until after it has returned.
Handlers for update events from PublicConfigStore will delay MetaMask's subsequent page refresh above and beyond what the poll interval itself already does, because those handlers are called synchronously before the page code that refreshes the page. Conversely, implementations using a redundant poll to detect network changes cannot guarantee they will be called before MetaMask's implementation refreshes the page out from underneath them--it's a timing-dependent race condition.
Sorry for the apparently slow progress, I assure you things are actually moving, and encourage you to be patient.
No problem, thanks for the clarifying some stuff. The EIP2020 is indeed got the functionality I'm looking for, I'll try to follow it.
For now it's best to wait until the web3 refactor to be complete (waiting for it some time actually to use the subscriptions API 😄 )
@allnewso Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!
Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days
Our inpage provider module is now in its own repository, and it is now also a node-style EventEmitter, so adding these events should be easier than ever.
Apparently the message above translates from Thai to:
I do not know what to do, I do not know how to do it.
So I would not take the current bounty hunter too seriously.
@danfinlay I also see that https://github.com/MetaMask/metamask-extension/pull/4279 is finally merged. Super cool, I'm excited about the stuff that could be built now 😊
[EIP-1193] (https://github.com/ethereum/EIPs/blob/master/EIPS/eip-1193.md) Is the spec that should be implemented, right?
So yeah, the bounty is still relevant!
While #4279 is merged, we're living on it in development for a while before pushing it to production. We want to catch any stray issues as early as possible. Will probably publish it the week after next.
@allnewso Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!
Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days
@danfinlay thx for the update....
@allnewso Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!
Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days
@allnewso Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!
Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days
@allnewso Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!
Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days
@allnewso Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!
Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days
@allnewso Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!
Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days
@leonprou would you like @vs77bb to remove @allnewso from the bounty? looks like they aren't making progress.
@leonprou @bdresser just removed the hunter, sorry for the Gitcoin Bot commentary, it should have stopped after 2 comments... working on a fix for that on our side!
Thanks,
Appologizing for being not responsive lately, I'm on vacation currently.
Just want to state that we still interested in it 😊.
Tue, Sep 11, 2018, 21:25 Vivek Singh notifications@github.com wrote:
@leonprou https://github.com/leonprou @bdresser
https://github.com/bdresser just removed the hunter, sorry for the
Gitcoin Bot commentary, it should have stopped after 2 comments... working
on a fix for that on our side!—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/MetaMask/metamask-extension/issues/4161#issuecomment-420372634,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AG5NrBAsETxm_z9MTVl3JoxLWyYGGi2Lks5uaAAVgaJpZM4TuPx8
.
@leonprou - Frank from Gitcoin here, are you still interested in working on this bounty?
Just caught some a recent notification about activity on this thread and think I'll catch a break later this afternoon/evening to browse through the recent changes to see if I can get my head around what all might yet there to be done.
This was deployed in version 4.0.3, but I'll be writing docs next.
@danfinlay cool, so I'll close the bounty
We should revisit https://github.com/MetaMask/metamask-extension/issues/3599 once docs for on('networkChanged') exist
Docs are now available on our new docs site, which is beginning to flesh up now:
https://metamask.github.io/metamask-docs/Advanced_Concepts/Provider_API#ethereum.on(eventname%2C-callback)
Most helpful comment
Wait a moment... Does MetaMask already provide this feature? It looks like the mechanism I described above that implements polling is already exposed by an EventHandler exposed by the PublicConfigStore object I described interacting with above, and that object is in turn exposed through the MetaMaskInPageProvider.
So, when the current provider is set by MetaMask, the following code works as one would desire, when called with the window web3 as an argument. It's console.log messages will only occur when a change actually happens, even though MetaMask is polling for account/network changes roughy 10 times per second.
function watchForChanges(web3) {
let lastState = web3.currentProvider.publicConfigStore.getState();
}
The web3 documentation's stance on this seems to be a recommendation to poll, so, IMHO, if there is any work to be done, its in Web3, and until then MetaMask users should be able to get by with what's already there if they don't want to re-implement polling as suggested by web3.