Update Gitcoin for breaking MetaMask changes #1964

owocki · 2018-08-08T16:22:16Z

Please read this medium article before reading this desc.

https://medium.com/metamask/https-medium-com-metamask-breaking-change-injecting-web3-7722797916a8

OCTOBER edit: heres a new post from metamask about how to do this: https://medium.com/metamask/eip-1102-preparing-your-dapp-5027b2c9ed76

The scope of this ticket is to update Gitcoin for the above breaking metamask changes.

Please submit a PR with the updates in it; Make sure you've tested

New Bounty
Submit Bounty
Accept Bounty
New Tip
Receive Tip

before you submit the PR

gitcoinbot · 2018-08-08T17:33:03Z

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

Work has been started.

These users each claimed they can complete the work by 1 week, 6 days from now.
Please review their action plans below:

zevaverbach has started work.

I will replace all direct reliance on injected web3 with a check to request permission and initialize web3 manually.

Learn more on the Gitcoin Issue Details page.

gitcoinbot · 2018-08-08T17:54:00Z

💰 A crowdfund contribution worth 0.10000 ETH (36.53 USD @ $365.27/ETH) has been attached to this funded issue from @dmvt.💰

Want to chip in also? Add your own contribution here.

owocki · 2018-08-09T15:50:50Z

Hey Zev, yes to documentation if you find it helpful to document what you learn!

Are the five events described in the issue the only events ever facilitated on Gitcoin?

I don't believe so, no. But they are the five most major use cases right now.

I wonder if instead of trying to migrate away all the global variables, we instead use some of the code that proimps the user to 'unlock' their metamask when there is no web3 enabled, to instead 'enable Gitcoin on their Metamask? This would probably be a much cleaner way to do this.

zevaverbach · 2018-08-11T21:45:41Z

opened WIP PR #1992, making progress. will move whining to there. 😆

owocki · 2018-08-14T22:50:32Z

@owocki one more comment here: Without making sure web3.js is included in several pages in addition to tips + bounties -- such as faucet -- web will break with respect to Metamask because the Web3 class won't be available. Outside the scope of this issue, but FYI.

yea i agree this is an issue. seems like it should be fairly trivial to just include web3.js 0.20.3 as a dependancy on gitcoin and set metamask to the provider if it exists. i believe this PR ( #985 ) does this already!

gitcoinbot · 2018-09-11T16:15:12Z

@zevaverbach 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!

warning (3 days)
escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

PumpkingWok · 2018-09-18T19:36:29Z

Hi @owocki,
I would like to ask you a question regard this issue.
From the article that you linked (https://medium.com/metamask/https-medium-com-metamask-breaking-change-injecting-web3-7722797916a8), edited on 9/13, there is follow code:

window.addEventListener('load', async () => {
    // Modern dapp browsers...
    if (window.ethereum) {
        window.web3 = new Web3(ethereum);
        try {
            // Request account access if needed
            await ethereum.enable();
            // Acccounts now exposed
            web3.eth.sendTransaction({/* ... */});
        } catch (error) {
            // User denied account access...
        }
    }
    // Legacy dapp browsers...
    else if (window.web3) {
        window.web3 = new Web3(web3.currentProvider);
        // Acccounts always exposed
        web3.eth.sendTransaction({/* ... */});
    }
    // Non-dapp browsers...
    else {
        console.log('Non-Ethereum browser detected. You should consider trying MetaMask!');
    }
});

If i understood well in this way the code should be ready also for changing that will be introduce in November, but before that date it is impossible to test the first if condition (if (window.ethereum)), because the ethereum object does not exist yet. right ?

How would you like the code to be implemented ?
I don't understand well if the code initialize only one instance of web3.
I have seen also that you use Web3 python package as python side, right ?

Thanks in advance.

owocki · 2018-09-19T19:23:54Z

@PumpkingWok great question... i just asked them on medium and on the consensys internaly slack. pasting my message below

hey metamask team, 

the gitcoin team is hard at work preparing for this breaking change scheduled to rollout on november 2nd.: https://medium.com/metamask/https-medium-com-metamask-breaking-change-injecting-web3-7722797916a8

question for you : do you plan to release a version of metamask that we can test our changes with, prior to november 2nd?  does this already exist on your github?  if so, could you point us to the branch/tag?

thanks
:gitcoin: team

owocki · 2018-09-19T19:31:26Z

@PumpkingWok from metamask team:

next release: https://github.com/MetaMask/metamask-extension/pull/5256
10% rollout at the moment, should be receiving it soon :tm:

PumpkingWok · 2018-09-20T08:36:02Z

hi @owocki,
Thank you very much for the info.

How would you like to proceed ? while i'm waiting the update i can look again into your code.
I did not understand well all places in the code where i need to edit it.

I have seen listen_to_web3_changes() that calls currentNetwork() inside v2/js/shared.js. Is it the right place for check the network changes ?
Thanks

owocki · 2018-09-20T23:07:56Z

How would you like to proceed ?

you can download the new metamask and use that for testing.

Is it the right place for check the network changes ?

i dont know enough about the new architecture of metamask to answer this question.

PumpkingWok · 2018-09-20T23:12:55Z

Hi,
Thank you for the reply,i’m going to download it. For another question i mean in your code not in metamask.i’m not sure to understood well in your code all places where it checks for metamask connection.thanks

PumpkingWok · 2018-09-25T15:09:53Z

Hi @owocki,
I was testing a new ethereum command from web console but it seems disappear from today.
I have just seen that metamask released a new version (4.11.0) and it reverts to version 4.9.3.
I think that the new command is disappear for this reason.
Thanks

PumpkingWok · 2018-09-28T09:00:35Z

Hi @owocki,
Metamask just released a new version, adding back ethereum object.
I'm back to this issue, thanks.

owocki · 2018-10-02T02:50:33Z

great!

PumpkingWok · 2018-10-15T17:00:12Z

Hi @owocki,
Yes sure, no problem. I did not work in it for some days because Metamask had released a new version again without ethereum object. From yesterday the last version released bring back ethereum.I'm going to work in it deeply from tomorrow.
Note: The UI for user Accept/Reject has not been released yet in official version.

zevaverbach · 2018-10-15T17:58:03Z

@owocki, it seems like @PumpkingWok is still working on this. Should I hold off? Also happy to pair on it, @PumpkingWok. Zev Averbach Averbach Transcription (802) 276-5934 avtranscription.com <http://www.avtranscription.com/>

…

On Mon, Oct 15, 2018 at 12:53 PM, Kevin Owocki ***@***.***> wrote: let me snooze gitcoinbot for a bit.. since we've agreed to come back to this in 10 days time — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1964 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACtRNOtifTD2bKYhJ2zh5P61r0APg3GJks5ulL2kgaJpZM4V0QKx> .

owocki · 2018-10-15T18:41:46Z

@zevaverbach has a PR out @PumpkingWok FYI - #1992 -- recommend that if you start on this, you collaborate on his branch. i'm happy to pay ya both.

@zevaverbach 's current plan is to revisit his PR in a week or two, did i get that correct?

usmanmuhd · 2018-11-05T12:45:44Z

@thelostone-mc I am not sure on how to add the logo. Will take a look at it.
Regarding the first issue:
I was thinking of making the user have an explicit button click to show the approval dialog. There seems to be a window.ethereum._metamask.isApproved() method, but this always returns false in my case. Could you please check this on your system?
If this is implemented properly, then the flow will be:
First time user: button click -> approve ->works normally
Returning user: silent approval -> works normally

thelostone-mc · 2018-11-05T13:18:59Z

@usmanmuhd it looks like that works !
For now -> we could add the button to settings/account under Account Preferences
(as I'm can't seemt to find a better place for it )

Could you write up a button which does that for now ?
As simple as If window.ethereum._metamask.isApproved() is true -> metamask is enabled
else text "Allow Gitcoin to use Metamask" and a button.

Would you be able to wrap that up ?

cc @willsputra If you have a design in mind for this -> I'll fix up the css once @usmanmuhd gets the functionality out ^_^

usmanmuhd · 2018-11-05T13:23:11Z

@thelostone-mc I am not sure if that is the way to go forward. Going by the comments on MetaMask/metamask-extension#4703, it currently returns false in all cases. They basically want to remove it. Maybe that is one of the reasons it works on your system and not mine. Could you please provide me the build that you are running?
Here is the commit that changed it to return false always MetaMask/metamask-extension@9aedb38
Edit 2: It works with the latest build MetaMask/metamask-extension#4703 (comment)

pinkiebell · 2018-11-05T14:35:50Z

@thelostone-mc @willsputra
Injecting a button into no_metamask_error.html should do it?

usmanmuhd · 2018-11-05T15:08:56Z

@pinkiebell just allowing a click on the banner might do right?

pinkiebell · 2018-11-05T15:12:13Z

@usmanmuhd
An additional button with something like Unlock MetaMask should do it.
That way we also don't have to reason about the MetaMask permission,
we just fire window.ethereum.enable().

willsputra · 2018-11-05T15:25:42Z

@pinkiebell @thelostone-mc putting it into no_metamask_error.html makes a lot of sense!

one thing though. currently, if I have MetaMask locked and go to the Issue Explorer, it doesn't redirect me to no_metamask_error.html, does it?

I wonder if we should also put the button into the 'Web 3 locked. Please unlock MetaMask' red bar below. Could also just be a link under 'unlock Metamask'.

usmanmuhd · 2018-11-05T15:48:12Z

From my discussion with @thelostone-mc offline, adding a banner that says "please allow gitcoin to access metamask" might just do. It should be clickable and will call window.etheruem.enable on click.
cc: @willsputra @pinkiebell

thelostone-mc · 2018-11-05T15:48:37Z

@usmanmuhd I took their latest build 4.17

@pinkiebell @usmanmuhd A click on the banner seems a lil misleading to me
Also withinno_metamask_error -> we'd have to add a conditional check to distinguish between when metamask is not enabled and when we need permission. (might make sense to throw in a new banner itself ? )

one thing though. currently, if I have MetaMask locked and go to the Issue Explorer, it doesn't redirect me to no_metamask_error.html, does it?

@willsputra no it does not redirect but TBH we don't need access to web3 on that page as of now. But yeah that error would leaves users confused. Hmm would it be the right choice to trigger the popup on clicking on it too ?

Additionally : Would it make sense to keep it in settings also so as the user has a way to enable it without having to visit the pages where no_metamask_error is rendered ? idk if that's an overkill :P

usmanmuhd · 2018-11-05T15:54:49Z

From my discussion with @thelostone-mc offline, adding a banner that says "please allow gitcoin to access metamask" might just do. It should be clickable and will call window.etheruem.enable on click.
cc: @willsputra @pinkiebell

I guess we should have a new check for the case where we need the user to allow gitcoin to access metamask.

A click on the banner seems a lil misleading to me

We might just redirect it to the settings/account and have a button there to enable. But to me that seems like an overkill.

willsputra · 2018-11-05T16:19:30Z

@thelostone-mc hmm regarding the red bar on issue explorer, yea maybe we don't need to do anything if we don't need web3 access on that page (maybe we don't even need the red bar? 😛but that's a whole other discussion for some other time...).

@usmanmuhd As @pinkiebell mentioned, I think putting a button on the banner is clearer rather than clicking on the banner itself.

Additionally, the verb on the button should be the same as the one that pops up on MetaMask so users are not confused. (They seem to be using the verb 'Connect' MetaMask/metamask-extension#5644 so I used 'Connect' too lol)

@pinkiebell @thelostone-mc is this what you all have in mind?

pinkiebell · 2018-11-05T16:37:21Z

@willsputra
That is nice. But we still need to think about the verb.
The private interface what MetaMask exposes will eventually be pruned and I also discourage the use of it.
That means we don't know if MetaMask is only locked or we don't have permission or both 😄 .
If we have permission and MetaMask is only locked, then calling window.ethereum.enable() is a no-op.

That's how privacy works 😄

usmanmuhd · 2018-11-06T01:15:30Z

@pinkiebell @willsputra From what I understand, currently we have an option to know if metamask is locked or we do not have permission, but it is not something that is absolutely necessary. We can just show a banner that says, "Allow access or Unlock wallet to continue" (can be better worded).

pinkiebell · 2018-11-06T05:17:53Z

Let's assume the user already approved permission but are now logged out of MetaMask, then we still can show that 'Connect MetaMask' button and if ethereum.enable() resolves with an empty [] - means the user already granted permission but is not logged into MetaMask.
If that's the case; we can display an additional copy with Please login into MetaMask or something like that.

usmanmuhd · 2018-11-06T06:43:41Z

@pinkiebell You cannot run ethereum.enable() without logging in. So that case does not arise. It's either logged in and granted permission or not logged in at all.

pinkiebell · 2018-11-06T06:58:12Z

@usmanmuhd
I'm sure it does work. It will also automatically open the MetaMask window if the user did not granted permission.

thelostone-mc · 2018-11-06T08:01:33Z

Can't we just have another banner if user hasn't enabled metamask ? Or reuse the same banner but show the button only if the user hasn't given permission ?

usmanmuhd · 2018-11-06T08:34:01Z

@thelostone-mc I will get a poc by today evening. We can discuss and make changes over it. Hope that is fine?

PixelantDesign · 2018-11-06T11:21:50Z

Hi @usmanmuhd sounds good. Let us know if you have any questions!

cc: @thelostone-mc

thelostone-mc · 2018-11-07T08:14:16Z

@usmanmuhd sure thing! leave a ping when it's good for review ^_^

owocki · 2018-11-07T23:45:46Z

@usmanmuhd hows this going? we are keep to 🚢 since the new metamask is live already ..

usmanmuhd · 2018-11-08T00:08:34Z

@owocki we are good to go except for web3.currentProvider and a minor snag in the banner for /faucet page. Could you please address my queries in the PR #2548

owocki · 2018-11-08T00:20:32Z

sorry which queries specifically?

wahts the web3.currentProvider issue and the faucet issue?

just left some QA comments

usmanmuhd · 2018-11-08T00:38:47Z

@owocki This one.

owocki · 2018-11-08T17:02:14Z

i just responded.. im not sure why this was blocked on me, i dont have any special answers you couldnt find yourself.

usmanmuhd · 2018-11-08T17:13:21Z

@owocki I guess it was specifically for the changes in window.ethereum that were introduced. There is not much documentation regarding that change.

owocki · 2018-11-10T00:47:14Z

just tested this.. works really good!

gitcoinbot · 2018-11-10T00:58:50Z

Issue Status: 1. Open 2. Cancelled

The funding of 150.0 DAI (plus a crowdfund of 0.1 ETH worth 21.08851715 USD) (150.0 USD @ $1.0/DAI) attached to this issue has been cancelled by the bounty submitter

Questions? Checkout Gitcoin Help or the Gitcoin Slack
$53,599.45 more funded OSS Work available on the Gitcoin Issue Explorer

gitcoinbot · 2018-11-10T00:59:01Z

⚡️ A tip worth 150.00000 DAI (150.0 USD @ $1.0/DAI) has been granted to @usmanmuhd for this issue from @owocki. ⚡️

Nice work @usmanmuhd! Your tip has automatically been deposited in the ETH address we have on file.

$53599.45 in Funded OSS Work Available at: https://gitcoin.co/explorer
Incentivize contributions to your repo: Send a Tip or Fund a PR
No Email? Get help on the Gitcoin Slack

This comment has been minimized.

Sign in to view

zevaverbach mentioned this issue Aug 11, 2018

WOC: Update Gitcoin for breaking MetaMask changes #1992

Closed

8 tasks

Update Gitcoin for breaking MetaMask changes #1964

Update Gitcoin for breaking MetaMask changes #1964

Comments

owocki commented Aug 8, 2018 • edited Loading

gitcoinbot commented Aug 8, 2018

gitcoinbot commented Aug 8, 2018

This comment has been minimized.

owocki commented Aug 9, 2018

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

zevaverbach commented Aug 11, 2018

owocki commented Aug 14, 2018

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

gitcoinbot commented Sep 11, 2018

This comment has been minimized.

PumpkingWok commented Sep 18, 2018

owocki commented Sep 19, 2018

owocki commented Sep 19, 2018 • edited Loading

PumpkingWok commented Sep 20, 2018

owocki commented Sep 20, 2018

PumpkingWok commented Sep 20, 2018

PumpkingWok commented Sep 25, 2018

PumpkingWok commented Sep 28, 2018

owocki commented Oct 2, 2018

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

PumpkingWok commented Oct 15, 2018

zevaverbach commented Oct 15, 2018 via email

owocki commented Oct 15, 2018

usmanmuhd commented Nov 5, 2018

thelostone-mc commented Nov 5, 2018 • edited Loading

usmanmuhd commented Nov 5, 2018 • edited Loading

pinkiebell commented Nov 5, 2018

usmanmuhd commented Nov 5, 2018

pinkiebell commented Nov 5, 2018

willsputra commented Nov 5, 2018

usmanmuhd commented Nov 5, 2018

thelostone-mc commented Nov 5, 2018 • edited Loading

usmanmuhd commented Nov 5, 2018 • edited Loading

willsputra commented Nov 5, 2018 • edited Loading

pinkiebell commented Nov 5, 2018

usmanmuhd commented Nov 6, 2018 • edited Loading

pinkiebell commented Nov 6, 2018

usmanmuhd commented Nov 6, 2018 • edited Loading

pinkiebell commented Nov 6, 2018

thelostone-mc commented Nov 6, 2018

usmanmuhd commented Nov 6, 2018

PixelantDesign commented Nov 6, 2018

thelostone-mc commented Nov 7, 2018

owocki commented Nov 7, 2018

usmanmuhd commented Nov 8, 2018 • edited Loading

owocki commented Nov 8, 2018

usmanmuhd commented Nov 8, 2018

owocki commented Nov 8, 2018

usmanmuhd commented Nov 8, 2018

owocki commented Nov 10, 2018

gitcoinbot commented Nov 10, 2018

gitcoinbot commented Nov 10, 2018

owocki commented Aug 8, 2018 •

edited

Loading

owocki commented Sep 19, 2018 •

edited

Loading

thelostone-mc commented Nov 5, 2018 •

edited

Loading

usmanmuhd commented Nov 5, 2018 •

edited

Loading

thelostone-mc commented Nov 5, 2018 •

edited

Loading

usmanmuhd commented Nov 5, 2018 •

edited

Loading

willsputra commented Nov 5, 2018 •

edited

Loading

usmanmuhd commented Nov 6, 2018 •

edited

Loading

usmanmuhd commented Nov 6, 2018 •

edited

Loading

usmanmuhd commented Nov 8, 2018 •

edited

Loading