[WIP] Added support for EIP-1102 #2548

usmanmuhd · 2018-10-27T15:14:07Z

Fixes #1964

app/assets/v2/js/metamask-approval.js

usmanmuhd · 2018-10-27T15:18:27Z

@owocki Can we get to know from the metamask team,

If there is a way to know without actually authorizing if the present website has been authorized?

codecov · 2018-10-27T15:21:04Z

Codecov Report

Merging #2548 into master will decrease coverage by 0.04%.
The diff coverage is n/a.

@@            Coverage Diff            @@
##           master   #2548      +/-   ##
=========================================
- Coverage   28.65%   28.6%   -0.05%     
=========================================
  Files         163     163              
  Lines       13097   13097              
  Branches     1753    1753              
=========================================
- Hits         3753    3747       -6     
- Misses       9244    9250       +6     
  Partials      100     100

Impacted Files	Coverage Δ
app/dashboard/embed.py	`28.16% <0%> (-3.45%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6f1651c...def45fa. Read the comment docs.

thelostone-mc · 2018-10-28T10:04:59Z

Not sure if it makes more sense

to keep a disabled / keep a enabled list / let the check on every page
aka add it to footer_scripts.js something like this :

window.addEventListener('load', async () => {
  if(window.ethereum) {
    window.web3 = new Web3(ethereum);
    try {
      await ethereum.enable();
    } catch (errow) {
      console.log("alert: user disabled transaction");
    }
  } else if (window.web3) {
    window.web3 = new Web3(web3.currentProvider);
  } else {
    console.log("alert: use dapp browser")
  }
});

cc @owocki

@usmanmuhd

If there is a way to know without actually authorizing if the present website has been authorized?

I think the code above would take care ethereum.enable does that check internally

usmanmuhd · 2018-10-28T11:57:48Z

@thelostone-mc ethereum.enable takes a bit of time to return after approval. Some times in the bounty page it does not recognise a user as the owner. Thus I added it to header.
I wanted to know if there is way without authorising because currently it makes the user almost unable to use gitcoin without pressing the enable option. That way I can make a user continue using gitcoin without bugging him to accept the authorisation.

mbeacom · 2018-11-01T14:34:33Z

Relevant: https://github.com/portis-project/cryptoauth
I spoke with the guys from Portis the other day and they discussed the above repo a bit. Seems to make sense 🤔

@thelostone-mc @SaptakS @owocki What does everything think of this repo?

owocki · 2018-11-02T22:02:47Z

If there is a way to know without actually authorizing if the present website has been authorized?

i donno, you could ask around or log an issue on their github issue board

owocki · 2018-11-02T22:03:49Z

If there is a way to know without actually authorizing if the present website has been authorized?

i could probably manage/create a list in localstorage

owocki · 2018-11-02T22:04:46Z

app/retail/templates/shared/head.html

@@ -63,6 +63,9 @@
 <title>{% if title %}{{title}}{% else %}{% trans "Grow Open Source" %}{% endif %} | Gitcoin</title>
 {% include 'shared/favicon.html' %}


from the metamask blog post about preparing your dapp for EIP 1102

Previously, dapps would access the MetaMask provider by using window.web3.currentProvider. While this will still work, the new, standard way to access the Ethereum provider in a Web browser is to use window.ethereum.

I dont see any updates for this in this repo...

Oh, I completely missed this. Will make the updates.

@owocki where is web3.currentProvider used and for what purpose?

Searching 18920 files for "currentProvider" /Users/kevinowocki/Sites/gitcoin/web/app/assets/ens/index.js: 4 var method = 'personal_sign'; 5 6: web3.currentProvider.sendAsync({ 7 method, 8 params, /Users/kevinowocki/Sites/gitcoin/web/app/assets/onepager/js/web3alert.js: 25 }; 26 var metaMaskWarning = function() { 27: if (typeof web3 == 'undefined' || !web3.currentProvider || !web3.currentProvider.isMetaMask) { 28 if (typeof document.suppressweb3alert != 'undefined') { 29 _alert({ message: gettext('You must install <a href=https://metamask.io/>Metamask</a> to use this tool.') }, 'info'); /Users/kevinowocki/Sites/gitcoin/web/app/assets/v2/js/truncate-hash.js: 70 new metamaskAddress(); 71 72: web3.currentProvider.publicConfigStore.on('update', function(e) { 73 new metamaskAddress(); 74 }); /Users/kevinowocki/Sites/gitcoin/web/app/assets/v2/js/pages/kudos_send.js: 561 } 562 563: // web3.currentProvider.publicConfigStore.on('update', function(e) { 564 var error; 565 ... 572 var network = e ? e.networkVersion : web3.version.network; 573 574: console.log(web3.currentProvider); 575 if (network === '4' || network === '1') { 576 console.log(network); /Users/kevinowocki/Sites/gitcoin/web/app/assets/v2/js/pages/wallet_estimate.js: 1 window.addEventListener('load', function() { 2: if (web3.currentProvider.isTrust) { 3 $('#trust_label').show(); 4 } else {

You can find the answer by grepping the codebase. I dont know more about the 'why' than you do.

@owocki From my discussions with @PumpkingWok, It is most probably updated only in the below case:

if (window.ethereum) { window.web3 = new Web3(window.ethereum) // updated case try { await window.ethereum.enable() } catch (error) { console.error(error) console.error(`Refresh the page to approve/reject again`) window.web3 = null } } else if(window.web3) { // for legacy dapp browsers window.web3 = new Web3(window.web3.currentProvider) }

This is also the only example given in the blog post as well.

app/assets/v2/js/metamask-approval.js

usmanmuhd · 2018-11-03T01:00:54Z

If there is a way to know without actually authorizing if the present website has been authorized?

i could probably manage/create a list in localstorage

Ah I was thinking of doing the same.
But then it might lead to cases where the local storage says that gitcoin has been authenticated but the user clears the privacy data. It will anyway lead to only 1 time nagging.

owocki · 2018-11-03T16:59:56Z

But then it might lead to cases where the local storage says that gitcoin has been authenticated but the user clears the privacy data. It will anyway lead to only 1 time nagging.

yes i agree. we'll have to figure out a way to manage that... or move it into a v2 ticket

usmanmuhd · 2018-11-03T17:19:19Z

Metamask has provided a way to check if a user has approved in metamask. It had a bug in the older build. Updating to latest build seems to solve it. I will get this done by tomorrow.

pinkiebell · 2018-11-03T22:05:47Z

Can we put this into the existing web3 listener?
Also, right now we don't have to source our own web3.js because metamask still injects it.
In the future we want to do something like var web3 = web3 || new Web3() in shared.js and setting the metmask provider if available.

For now we can do simply:

diff --git a/app/assets/v2/js/shared.js b/app/assets/v2/js/shared.js
index ad6bb8295..9a011e906 100644
--- a/app/assets/v2/js/shared.js
+++ b/app/assets/v2/js/shared.js
@@ -830,6 +830,18 @@ var listen_for_web3_changes = function() {
   } else if (typeof web3 == 'undefined' || typeof web3.eth == 'undefined' || typeof web3.eth.coinbase == 'undefined' || !web3.eth.coinbase) {
     currentNetwork('locked');
     trigger_form_hooks();
+
+    if (window.ethereum) {
+      window.ethereum.enable().then(
+        function() {
+          //approved
+        }
+      ).catch(
+        function() {
+          //rejected
+        }
+      );
+    }
   } else {
     web3.eth.getBalance(web3.eth.coinbase, function(errors, result) {
       if (typeof result != 'undefined') {

usmanmuhd · 2018-11-04T15:04:59Z

@pinkiebell I am not entirely sure that we should do it that way.
The current method will make it better for the next update to web3js 1.0

app/assets/v2/js/metamask-approval.js

usmanmuhd · 2018-11-07T10:09:57Z

I think this is good to test now.
cc: @thelostone-mc

thelostone-mc · 2018-11-07T11:13:15Z

@usmanmuhd DM-ed you a few changes ! Could you get that done too ?
Apart from that this looks good from my side

app/assets/v2/js/metamask-approval.js

owocki · 2018-11-08T00:22:51Z

QA notes:

i should not have to refresh the page after i give metamask perms -- the curernt site notices you've unlocked and shows the right elements without a refresh
faucet page doesnt prompt me to connect to metamask, still asks to unlock
same with tip page / kudos page, contribute page and /explorer, and i presume, all the other pages that check for metamask

usmanmuhd · 2018-11-08T07:46:35Z

@thelostone-mc How do I handle the metamask connection request on kudos, tips and ens?
I was planning on showing an alert with a button.

app/assets/v2/js/metamask-approval.js

usmanmuhd · 2018-11-08T12:19:10Z

@owocki @thelostone-mc I guess this is good to go (except for web3.currentProvider)!

owocki · 2018-11-09T16:14:30Z

How do I handle the metamask connection request on kudos, tips and ens?

you can just use the same styles as exists on bounties, no?

i think we need to handle this before shipping

owocki · 2018-11-09T16:27:49Z

testing now

usmanmuhd · 2018-11-09T16:32:17Z

@owocki I handled it using _alert. If that is not proper I will change it.

owocki · 2018-11-10T00:50:40Z

this is awesome! just tested and it seems really good.. merging now, will deploy monday

usmanmuhd · 2018-11-10T02:56:28Z

Thanks, waiting to use it!

thelostone-mc · 2018-11-10T08:58:12Z

Thanks man for updating the faucet banners ^_^

stickler-ci reviewed Oct 27, 2018

View reviewed changes

app/assets/v2/js/metamask-approval.js Outdated Show resolved Hide resolved

app/assets/v2/js/metamask-approval.js Outdated Show resolved Hide resolved

usmanmuhd mentioned this pull request Oct 27, 2018

migrate gitcoin from web3js 0.2 to 1.0 #2370

Closed

usmanmuhd force-pushed the eip1102 branch from 67f4004 to ae2cf62 Compare October 27, 2018 16:10

thelostone-mc added upgrade frontend This needs frontend expertise. labels Oct 28, 2018

thelostone-mc requested review from owocki, mbeacom and thelostone-mc October 28, 2018 10:06

owocki reviewed Nov 2, 2018

View reviewed changes

stickler-ci reviewed Nov 5, 2018

View reviewed changes

app/assets/v2/js/metamask-approval.js Outdated Show resolved Hide resolved

usmanmuhd force-pushed the eip1102 branch from 032f3c2 to 2904ec0 Compare November 5, 2018 13:50

thelostone-mc reviewed Nov 7, 2018

View reviewed changes

app/assets/v2/js/metamask-approval.js Show resolved Hide resolved

stickler-ci reviewed Nov 7, 2018

View reviewed changes

app/assets/v2/js/metamask-approval.js Outdated Show resolved Hide resolved

usmanmuhd force-pushed the eip1102 branch from 0d660d3 to 31c5d5e Compare November 7, 2018 15:14

usmanmuhd mentioned this pull request Nov 8, 2018

Update Gitcoin for breaking MetaMask changes #1964

Closed

Muhammad Usman added 10 commits November 8, 2018 09:12

Check if approved

094c716

Added button to connect

d1a7e0d

Check metamask lock rectified

f10386e

Fixed error with button

93792e6

Added CSS for button

6338ee7

Added banner for metamask not connected

302fe53

Fixed grammar in banner

5c708e0

Fixed lint errors

ed6def5

Added banner for faucet

ee308bb

Added _alert when metamask connect request rejected

d012ac9

usmanmuhd force-pushed the eip1102 branch from 78e8a3d to d012ac9 Compare November 8, 2018 03:43

Muhammad Usman added 2 commits November 8, 2018 11:26

Changed faucet to use latest banners and fixed tips

949e86b

Added metamask-approval to ens

081757e

Alert for metamask not connected in tips, ens and kudos

01d7a15

stickler-ci reviewed Nov 8, 2018

View reviewed changes

app/assets/v2/js/metamask-approval.js Show resolved Hide resolved

app/assets/v2/js/metamask-approval.js Outdated Show resolved Hide resolved

app/assets/v2/js/metamask-approval.js Show resolved Hide resolved

Muhammad Usman added 4 commits November 8, 2018 14:24

Fixed lint errors

7560522

Fixed lint errors

a522d2a

set is_metamask_approved to true on enable

a45c2f8

Fixed auto-updating for new issue, faucet

def45fa

owocki merged commit def45fa into gitcoinco:master Nov 10, 2018

usmanmuhd deleted the eip1102 branch November 10, 2018 11:31

mbeacom mentioned this pull request Nov 29, 2018

WOC: Update Gitcoin for breaking MetaMask changes #1992

Closed

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Added support for EIP-1102 #2548

[WIP] Added support for EIP-1102 #2548

usmanmuhd commented Oct 27, 2018 •

edited

Loading

usmanmuhd commented Oct 27, 2018

codecov bot commented Oct 27, 2018 •

edited

Loading

thelostone-mc commented Oct 28, 2018 •

edited

Loading

usmanmuhd commented Oct 28, 2018

mbeacom commented Nov 1, 2018

owocki commented Nov 2, 2018

owocki commented Nov 2, 2018 •

edited

Loading

owocki Nov 2, 2018

usmanmuhd Nov 3, 2018

usmanmuhd Nov 7, 2018

owocki Nov 8, 2018

usmanmuhd Nov 8, 2018 •

edited

Loading

usmanmuhd commented Nov 3, 2018

owocki commented Nov 3, 2018

usmanmuhd commented Nov 3, 2018

pinkiebell commented Nov 3, 2018

usmanmuhd commented Nov 4, 2018

usmanmuhd commented Nov 7, 2018

thelostone-mc commented Nov 7, 2018

owocki commented Nov 8, 2018

usmanmuhd commented Nov 8, 2018

usmanmuhd commented Nov 8, 2018 •

edited

Loading

owocki commented Nov 9, 2018 •

edited

Loading

owocki commented Nov 9, 2018

usmanmuhd commented Nov 9, 2018

owocki commented Nov 10, 2018

usmanmuhd commented Nov 10, 2018 •

edited

Loading

thelostone-mc commented Nov 10, 2018

		@@ -63,6 +63,9 @@
		<title>{% if title %}{{title}}{% else %}{% trans "Grow Open Source" %}{% endif %} \| Gitcoin</title>
		{% include 'shared/favicon.html' %}

[WIP] Added support for EIP-1102 #2548

[WIP] Added support for EIP-1102 #2548

Conversation

usmanmuhd commented Oct 27, 2018 • edited Loading

usmanmuhd commented Oct 27, 2018

codecov bot commented Oct 27, 2018 • edited Loading

Codecov Report

thelostone-mc commented Oct 28, 2018 • edited Loading

usmanmuhd commented Oct 28, 2018

mbeacom commented Nov 1, 2018

owocki commented Nov 2, 2018

owocki commented Nov 2, 2018 • edited Loading

owocki Nov 2, 2018

Choose a reason for hiding this comment

usmanmuhd Nov 3, 2018

Choose a reason for hiding this comment

usmanmuhd Nov 7, 2018

Choose a reason for hiding this comment

owocki Nov 8, 2018

Choose a reason for hiding this comment

usmanmuhd Nov 8, 2018 • edited Loading

Choose a reason for hiding this comment

usmanmuhd commented Nov 3, 2018

owocki commented Nov 3, 2018

usmanmuhd commented Nov 3, 2018

pinkiebell commented Nov 3, 2018

usmanmuhd commented Nov 4, 2018

usmanmuhd commented Nov 7, 2018

thelostone-mc commented Nov 7, 2018

owocki commented Nov 8, 2018

usmanmuhd commented Nov 8, 2018

usmanmuhd commented Nov 8, 2018 • edited Loading

owocki commented Nov 9, 2018 • edited Loading

owocki commented Nov 9, 2018

usmanmuhd commented Nov 9, 2018

owocki commented Nov 10, 2018

usmanmuhd commented Nov 10, 2018 • edited Loading

thelostone-mc commented Nov 10, 2018

usmanmuhd commented Oct 27, 2018 •

edited

Loading

codecov bot commented Oct 27, 2018 •

edited

Loading

thelostone-mc commented Oct 28, 2018 •

edited

Loading

owocki commented Nov 2, 2018 •

edited

Loading

usmanmuhd Nov 8, 2018 •

edited

Loading

usmanmuhd commented Nov 8, 2018 •

edited

Loading

owocki commented Nov 9, 2018 •

edited

Loading

usmanmuhd commented Nov 10, 2018 •

edited

Loading