Skip to content

Issue Trust Bonus as a W3C Verifiable Credential to 3Box #9032

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

Open
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

wyc
Copy link

@wyc wyc commented Jun 9, 2021

Description

This change allows users to download their Trust Bonus as a signed W3C Verifiable Credential to their local machine or 3Box profile in addition to minting an NFT. This can then be used to demonstrate their Trust Bonus anywhere supporting the same global data standards. The signing happens with a did-web DID from gitcoin.co, but this component can be further decentralized in the future using ENS + did-eth.

Progress:

  • Implement W3C Verifiable Credential issuance
  • Integrate storage of credentials into 3Box
  • Implement demo application to load credentials from 3Box and verify them
  • Complete tests to hit the endpoint and verify the credential retrieved

A demo deployment with this PR incorporated can be found here: https://gitcoin-demo.spruceid.com/

Tool to retrieve & test the credential: https://demo.spruceid.com/popp/tools/

did-web JSON source: https://gitcoin-demo.spruceid.com/.well-known/did.json

Refers/Fixes

#8811

Testing

image

0c157835-8f1f-4c7e-a8f3-abaccf7c4977

@theosirian theosirian force-pushed the spruceid/feature/trust-bonus-issuance branch from 17dd2c1 to cbecdf9 Compare June 9, 2021 19:10
@wyc wyc marked this pull request as ready for review June 9, 2021 19:16
@theosirian theosirian force-pushed the spruceid/feature/trust-bonus-issuance branch from 49978e0 to 94faf41 Compare June 9, 2021 20:16
Copy link
Contributor

@thelostone-mc thelostone-mc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will need to review this further as though isolated. It introduces

  • typescript
  • svelte
  • tailwind
  • it's own linter + webpack config

@@ -939,3 +939,6 @@ def callback(request):
# GTC Token Distribution
GTC_DIST_API_URL = env('GTC_DIST_API_URL', default='http://localhost:8000/not-valid-url')
GTC_DIST_KEY = env('GTC_DIST_KEY', default='')
DIDKIT_KEY_JWK = env.json('DIDKIT_KEY_JWK', default={})
POPP_VC_ISSUER = env.str('POPP_VC_ISSUER', default='')
POPP_VC_VERIFIER = env.str('POPP_VC_VERIFIER', default='')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

^ could we add in doc on where this would be populated from

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added in 0523e1b

app/app/urls.py Outdated
@@ -271,6 +271,7 @@

#passport
re_path(r'^passport/$', passport.views.index, name='passport_gen'),
re_path(r'^passport-vc$', passport.views.verifiable_credential, name='passport_vc'),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could we rename it tp passport/verify-credentials

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved in 17ac7e5

@@ -103,3 +105,80 @@ def passport(request, pattern):
],
}
return JsonResponse(context)


def verifiable_credential(request):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's use the login decorator that django offers
@login_required

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved in 91afdb8

network = request.GET.get("network")

if not request.user.is_authenticated:
return JsonResponse({"status": "error", "msg": "You must login"})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

^ this can be removed since we are using the decorator

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved in 91afdb8

</head>

<body></body>
</html>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

^ reindent to 2 spaces

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 5ad8ee2

@theosirian
Copy link

The issues that were listed above have been resolved in the latest commits. It's ready for another review, especially the doc section, since I wasn't sure about the level of detail to use there.

@theosirian theosirian force-pushed the spruceid/feature/trust-bonus-issuance branch from 0523e1b to 21eda8e Compare June 14, 2021 17:43
@theosirian
Copy link

Rebased onto current master.

@thelostone-mc
Copy link
Contributor

thelostone-mc commented Jun 15, 2021

The code looks alright to me but I'm not very comfortable on adding

  • ts
  • svelte
  • the custom webpack / lint config files

Can this not be done outside this repo and we just end up including the minified file @wyc / @theosirian

cc @octavioamu / @gdixon / @chibie / @zacheryschiller could you guys check this out as well ?


Also from UI I'm not sure this sticks to out styleguide / presskit.
@willsputra / @PixelantDesign could you guys check this and drop in feedback ?

@wyc
Copy link
Author

wyc commented Jun 15, 2021

Sure thing, we will move it out of this PR for now. Perhaps a good home for it is https://github.com/gitcoinco/PersonhoodPassport

@theosirian theosirian force-pushed the spruceid/feature/trust-bonus-issuance branch from 21eda8e to 3ab6fd7 Compare June 15, 2021 16:50
@wyc
Copy link
Author

wyc commented Jun 15, 2021

The items mentioned in #9032 (comment) have been moved into gitcoinco/PersonhoodPassport#1.

Ready for another review, thanks!

@theosirian theosirian force-pushed the spruceid/feature/trust-bonus-issuance branch 3 times, most recently from a994cde to 8132353 Compare June 28, 2021 14:43
@theosirian theosirian force-pushed the spruceid/feature/trust-bonus-issuance branch 3 times, most recently from ef9e5dd to d260dfb Compare July 14, 2021 14:02
Copy link
Contributor

@octavioamu octavioamu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looking nice, left a few comments about functions and data that already exist.

Comment on lines 2359 to 2363
const accounts = web3.eth.getAccounts();

$.when(accounts).then((result) => {
ethAddress = result[0];
ethProvider = web3.currentProvider;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there are already some methods used on wallet.js that could be used here instead. You will find some global variables like selectedAccount, provider and others generated when someone connect to the site using web3modal

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved in e32dea6.

Comment on lines 2406 to 2417
const vcCopy = () => {
const str = JSON.stringify(credential, null, 2);

navigator.clipboard.writeText(str);
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please use the code on clipboard.js
you can use it like
Copy Text

this string will be copied

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is not using the last clipboard api but that could be updated if required (we already did it on our clipboard component in vue-components.js

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved in e32dea6.

@theosirian theosirian force-pushed the spruceid/feature/trust-bonus-issuance branch from d260dfb to e32dea6 Compare July 29, 2021 19:25
@theosirian
Copy link

Resolved the changes requested by @octavioamu and rebased onto current master.

@theosirian theosirian force-pushed the spruceid/feature/trust-bonus-issuance branch from e32dea6 to 8b741f7 Compare August 26, 2021 19:41
@theosirian theosirian force-pushed the spruceid/feature/trust-bonus-issuance branch from 8b741f7 to 12e4ea9 Compare September 16, 2021 00:57
@obstropolos
Copy link

Hey @octavioamu - we've since migrated this to Ceramic. We're ready for another review - thanks!

@theosirian theosirian force-pushed the spruceid/feature/trust-bonus-issuance branch from 12e4ea9 to 5df725d Compare September 29, 2021 14:54
@anil2307
Copy link

kuvvet itlir

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants