search

gitcoinco / web

Grow Open Source
https://gitcoin.co
Other
1.78k stars 771 forks source link

Expose Grant information via API endpoints for Rotki and other api consumers #6493

Closed LefterisJP closed 4 years ago

LefterisJP commented 4 years ago

User Story

We at Rotki plan to create a premium component that would allow grantees to manage their grant earnings as income and take it into account in their income and Profit/loss report. The issue in our repo is here: https://github.com/rotki/rotki/issues/692

We planned to take all data from the chain but after discussion with @owocki if Gitcoin already has the data and can expose it via an endpoint our task will be easier. And this is probably something other Gitcoin users or applications who want to integrate with Gitcoin may appreciate.

We may still add on-chain verification for people who have their own node connected, but that's not relevant to this issue.

Note: We could probably do the same for grant donors, and count donations as expenses which depending on jurisdiction may also be tax-deductible.

Required data

As a Grantee

Given any ethereum address, return the following if it's a grant receiver:

The above should also contain the CLR matching payouts.

With all the above Rotki should have a complete picture of what the user's grant got and when. If possible the endpoint should be filtered by timestamp. Two arguments from_timestamp and to_timestamp. Default from would be 0 and default to would be current time. Time should be UTC.

As a Grant donor

We can do the same for grant donors.

Given an address ask for all my gitcoin contributions. This would returns a list with each entry containing:

developerfred commented 4 years ago

would love to work on this issue

owocki commented 4 years ago

This seems like a good investment to make for us.. The only remaining question is that of security and authentication... Do we want to expose this info for all users to all grants, or only a permissioned basis? How do we want to provide third party AUTH to an app like Rotki to ingest the info?

LefterisJP commented 4 years ago

I don't think you need authentication for any of the above. All of the data above are actually on-chain. You would be saving the time of a tool like Rotki doing its own chain analysis to discover them.

Plus for Rotki authentication is hard since it's a local application. So each user would need to authenticate individually with your API endpoint.

gitcoinbot commented 4 years ago

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

This issue now has a funding of 0.5 ETH (97.62 USD @ $195.24/ETH) attached to it.

gitcoinbot commented 4 years ago

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 month from now. Please review their action plans below:

1) sebastiantf has been approved to start work.

Fairly good and easy task. Can take it up and turn around without much delay

Learn more on the Gitcoin Issue Details page.

gitcoinbot commented 4 years ago

@sebastiantf 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

sebastiantf commented 4 years ago

Oh yes @gitcoinbot. I'm working on it.

owocki commented 4 years ago

from DB

On Sat, May 2, 2020 at 5:39 AM Sebastian T F notifications@github.com wrote:

Should the API fetch on-chain data or the data from the db?

It seems all the data required to be in this API are recorded and available in the database already.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/gitcoinco/web/issues/6493#issuecomment-622940256, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAD5PCP2ZXOXLDNJRTQTAE3RPQBAVANCNFSM4MPD6DPA .

--

@owocki http://www.twitter.com/owocki

gitcoin is live and has generated over $4.0mm for Open Source Software - see our results https://gitcoin.co/results

sebastiantf commented 4 years ago

I needed a quick clarification,

In the Subscription model,

  1. amount_per_period is the amount contributed by the contributor and would match exactly to the API requirement of:

    amount: The final amount received. Minus gitcoin fees. This should be the actual amount the gitcoin grantee receives at the end.

  2. gas_price is the gitcoin fees and would match exactly to the API requirement of:

    gitcoin_maintenance_amount: The amount of asset (if any) that the user contributed to the gitcoin maintenance fund as gitcoin fees.

    and the gitcoin fees in:

    amount: Minus gitcoin fees.


Are these statements 1 & 2 correct?

owocki commented 4 years ago

amount_per_period does not have the gitcoin fees deducted. other that that yes it looks correct

sebastiantf commented 4 years ago

Ok so gas_price is being deducted from amount_per_period. Got it. Thanks.

LefterisJP commented 4 years ago

That's counter-intuitive. So @owocki gas price is the actual network gas price plus the optional fee to the gitcoin sustainability grant?

owocki commented 4 years ago

yeah i agree. it doesnt make sense in hindsight.

some history: the naming convention comes from the architecture of EIP 1337; wherein gas_price is the amount transfered to gitcoin to cover the costs of running a subminer (a node that submits the EIP 1337 recurring txns to chain). over time, as we built in donations to gitcoin that was just folded into the gas price field.

sebastiantf commented 4 years ago

amount_per_period does not have the gitcoin fees deducted. other that that yes it looks correct

And gas_price is actually stored in wei? i.e, fees * 10^decimals of the token

owocki commented 4 years ago

yes

sebastiantf commented 4 years ago

How does this look?:

grants_report_api_WIP_trimmed
gitcoinbot commented 4 years ago

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

Work for 0.5 ETH (94.35 USD @ $188.7/ETH) has been submitted by:

  1. @sebastiantf

@owocki please take a look at the submitted work:

owocki commented 4 years ago

@LefterisJP would this output suffice for you

LefterisJP commented 4 years ago

@owocki @sebastiantf Thanks a lot! Looks good. Looking at the screenshot I have some questions.

  1. how is the usd value calculated? Some kind of oracle for the price at the time of transaction? Cryptocompare?
  2. As a grantee I don't see any gitcoin/gas fee. So the amount shown there is the final one that actually ends up in the grant's donation account, right?
  3. What is the value of the CLR round going to be if it's not during a CLR round? null?
  4. Why not add a CLR round key also in the donor response?
sebastiantf commented 4 years ago

@LefterisJP

  1. usd_value is actually already calculated as part of the Subscription model. (If my inferences are correct,) this in turn is calculated using ConversionRates fetched from Etherdelta, Poloneix, CryptoCompare, Uniswap etc. Maybe @owocki could shed more light on this.
  2. Yes. That is correct.

  3. Yes. It would be null:

    Screenshot 2020-05-13 at 3 36 45 AM
  4. It could be added, if you'd like. I was trying to strictly follow the structure you mentioned in the original comment

You could take a look at the PR #6633 if that would be helpful

LefterisJP commented 4 years ago

usd_value is actually already calculated as part of the Subscription model. (If my inferences are correct,) this in turn is calculated using ConversionRates fetched from Etherdelta, Poloneix, CryptoCompare, Uniswap etc. Maybe @owocki could shed more light on this.

Okay. I am mentioning since the example data seem like DAI is simply taken at 1-1 with the dollar while that's almost never the case.

.2 .3 Thanks for the answers!

4.It could be added, if you'd like. I was trying to strictly follow the structure you mentioned in the original comment

I think it makes sense for consistency. Did not realize it when I was writing the OP.

Will look at the PR if I get the time but I am also not really familiar with the gitcoin codebase.

sebastiantf commented 4 years ago

Will add 4.

I'm not sure about 1. and DAI. @owocki ?

yisiliu commented 4 years ago

Hi @sebastiantf ! Thanks for the excellent job! I've got some extra requests here to see if you can add them to this PR.

  1. The grantee endpoint only takes an ethereum address to do the query. Can you possibly support querying with a grant number?
  2. You are returning all the transactions in the response but can you also add a stats mode that only returns meta of this grant, e.g. address of the grant, number of donors, total amount of each asset?

We will appreciate if you can add these to this PR, which will largely support our work at https://github.com/dimensiondev/maskbook. Let me know if you have any questions! Thanks.

sebastiantf commented 4 years ago

@yisiliu Have you checked out the endpoint at https://gitcoin.co/api/v0.1/grants/. It lists all the data related to all the grants and you could query a specific grant with it's pk: https://gitcoin.co/api/v0.1/grants/149/

You could get the admin address from that endpoint. But I'm not sure about number of donors, total amount of each asset

Please do check that endpoint to see if it helps and feel free to specifically mention any other data that you think you need and the structure of the response as done in the original comment for this issue.

yisiliu commented 4 years ago

@sebastiantf Thanks so much for the reply! I actually didn't realize there was actually an "official" API out there! A total number of donors could be extremely useful in our use case. We can definitely directly query /api/v0.1/grants/report/ADDRESS to get the list and compute the length but it's gonna be extremely helpful if you can add such metadata into your endpoint.

Let me check with our team again and let you know what we think will be also helpful to be added. Thanks for your job! This is super awesome!

sebastiantf commented 4 years ago

Do you want the number of donors to be added to the /api/v0.1/grants/report/ADDRESS? That endpoint actually requires an eth_address to be passed and it could look like:

"grantee": [
        {
            "grant_name": "Go Fund My Test Grant",
            "number_of _donors": 35
            "transactions": [
                {
                    ...
                }
            ],
            ...
        }

Or should I add it to the original /grants/ endpoint, or maybe a /grants/slim endpoint? Shall wait for your team's requirements and use cases.

yisiliu commented 4 years ago

Do you want the number of donors to be added to the /api/v0.1/grants/report/ADDRESS? That endpoint actually requires an eth_address to be passed and it could look like:

"grantee": [
        {
            "grant_name": "Go Fund My Test Grant",
            "number_of _donors": 35
            "transactions": [
                {
                    ...
                }
            ],
            ...
        }

If you do it this way, I guess the number of donors might not be that useful because one can easily do the computation themselves. What we want is an endpoint that only returns the metadata to save bytes.

I am not sure if this is polite to request to add extra things to your job since it requires extra effort and this request is made under another team's grant. I will first wait for both you @sebastiantf and @LefterisJP 's permission and see what we can do here later.

Anyways, thanks for your awesome work!

sebastiantf commented 4 years ago

@yisiliu Maybe you could create a new issue and bounty it on Gitcoin and I'll be glad to take it up. Or maybe you could contribute more funds to this bounty itself on Gitcoin and I could add your requirements too with this PR, maybe creating a new /grants/slim/ endpoint. I'm okay with any of these options.

I do not think @LefterisJP actually wanted this feature only for Rotki, but also for other api consumers as mentioned in the title and opening comment of this issue. But let us know of your thoughts @LefterisJP

yisiliu commented 4 years ago

Yes that'll be a good idea to create a new issue I guess but if @LefterisJP feels good to add extra functionalities here I'd prefer it to be done in this issue together. We can definitely add funds for sure. Let's wait for his thoughts and decide.

LefterisJP commented 4 years ago

I don't mind extra data, in the endpoint. But for the number of donors you can just get the length of the transactions list, right? Or is it number of unique donors you are asking for?

I think an endpoint with metadata for the grant would make more sense. The one @sebastiantf showed looks like a nice candidate

yisiliu commented 4 years ago

Hi @LefterisJP thanks for your reply!

I don't mind extra data, in the endpoint. But for the number of donors you can just get the length of the transactions list, right? Or is it number of unique donors you are asking for?

Actually that's the thing here because we can definitely calculate ourselves but in our scenario, our users would need to make a lot of queries to get the metadata only. Manual calculation definitely works but if we can have another endpoint or even add a param to the query like /api/v0.1/grants/report/ADDRESS?meta_only=true that returns the meta data only, it'll save a lot of bytes!

What do you think?

LefterisJP commented 4 years ago

@sebastiantf I don't know if I should do it here or in the PR, but will continue here since I started all comments here.

I realized one more thing, but I don't know if it's possible for your to get this data. I take it the "clr_payout" field refers to clr payouts the grant got from all rounds, right?

What would be needed from an accounting perspective is a bit more information. Namely a list of the clr payouts with timestamps and amounts. Something like:

No payouts

"clr_payouts": []

Two payouts

"clr_payouts": [{
    "amount": 1000,
    "asset": "DAI",
    "usd_value": "1000",
    "timestamp": "xyz",
    "round": 4
}, {
   "amount": 2000,
    "asset": "DAI",
    "usd_value": "2000",
    "timestamp": "yzz",
    "round": 5
}]

Is that possible?

LefterisJP commented 4 years ago

What do you think?

Another endpoint for the metadata would make sense.

sebastiantf commented 4 years ago

Is that possible?

@LefterisJP I think that would be possible. Will try that out.

sebastiantf commented 4 years ago

Sorry for the delay.

I was able to achieve this:

Screenshot 2020-05-15 at 12 26 14 PM

It works fine for a grant that accepts a specific token like DAI, ETH, etc. But for a grant that accepts Any Token, in what token_symbol or asset does the payout occur? The token_symbol of the payout is not being stored in the CLRMatch object. Or am I missing something here?

Screenshot 2020-05-15 at 12 31 41 PM

Help needed @owocki.

LefterisJP commented 4 years ago

@sebastiantf I think all CLR payouts are so far in DAI no matter what token the grant accepts. Is that correct @owocki?

I asked to add it as a parameter to be future-proof in case it becomes customizable in the future.

sebastiantf commented 4 years ago

Shall I set the asset and token_symbol to calculate usd_value to DAI, then (for now) @owocki?

sebastiantf commented 4 years ago

Meanwhile, anything else to be added @LefterisJP?

LefterisJP commented 4 years ago

@sebastiantf No I don't think so. We will see when we use it, but for now that's all I can think of. Thank you!

owocki commented 4 years ago

the matching payouts are in DAI. the contributions may not be though.

On Fri, May 15, 2020 at 1:38 AM Lefteris Karapetsas < notifications@github.com> wrote:

@sebastiantf https://github.com/sebastiantf I think all CLR payouts are so far in DAI no matter what token the grant accepts. Is that correct @owocki https://github.com/owocki?

I asked to add it as a parameter to be future-proof in case it becomes customizable in the future.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/gitcoinco/web/issues/6493#issuecomment-629082684, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAD5PCLNIT4CFWASSW6CPR3RRTWQ7ANCNFSM4MPD6DPA .

--

@owocki http://www.twitter.com/owocki

gitcoin is live and has generated over $4.6mm for Open Source Software - see our results https://gitcoin.co/results

sebastiantf commented 4 years ago

The requested addition has been added to the PR

owocki commented 4 years ago

https://github.com/gitcoinco/web/issues/6679

looks like this is an issue for ppl

sebastiantf commented 4 years ago

6679

looks like this is an issue for ppl

Although that feels like out of the scope of this issue, CORS can be handled in Django REST framework using django-cors-headers, as mentioned in their docs. The same is being used with Gitcoin too: https://github.com/gitcoinco/web/blob/6866879ba967cc08e8c0de89f5f5a8a05e64f1e8/app/app/settings.py#L73 https://github.com/gitcoinco/web/blob/6866879ba967cc08e8c0de89f5f5a8a05e64f1e8/app/app/settings.py#L148 Haven't looked deep into it, but maybe it's a chrome extension-specific thing: Changes to Cross-Origin Requests in Chrome Extension Content Scripts

Jack-Works commented 4 years ago

Haven't looked deep into it, but maybe it's a chrome extension-specific thing: Changes to Cross-Origin Requests in Chrome Extension Content Scripts

Have no idea, but, does the core headers added yet?

sebastiantf commented 4 years ago

django-cors-headers is already being used.

Jack-Works commented 4 years ago

Strange, but I didn't receive a CORS header from the server.

Request headers:

:authority: gitcoin.co
:method: GET
:path: /api/v0.1/grants/634/
:scheme: https
accept: */*
accept-encoding: gzip, deflate, br
accept-language: en-US,en;q=0.9,ja-JP;q=0.8,ja;q=0.7,zh-CN;q=0.6,zh;q=0.5
dnt: 1
origin: chrome-extension://our-long-long-random-extension-id
sec-ch-ua: "Chromium";v="84", "Google Chrome";v="84"
sec-ch-ua-mobile: ?0
sec-fetch-dest: empty
sec-fetch-mode: cors
sec-fetch-site: cross-site
user-agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/84.0.4143.7 Safari/537.36

Response header:

allow: GET, PUT, PATCH, DELETE, HEAD, OPTIONS
content-language: en
content-length: 3801
content-type: application/json
date: Wed, 20 May 2020 03:43:05 GMT
server: nginx
status: 200
strict-transport-security: max-age=3600; includeSubDomains; preload
vary: Accept, Authorization, Accept-Language, Cookie, Origin
x-content-type-options: nosniff
x-frame-options: DENY
x-xss-protection: 1; mode=block

No CORS response therefore rejected

Jack-Works commented 4 years ago

It doesn't work on normal HTTP websites.

Tedko commented 4 years ago

@octavioamu @sebastiantf Seems like https://github.com/gitcoinco/web/issues/6493#issuecomment-631218226 & https://github.com/gitcoinco/web/issues/6493#issuecomment-631218558 haven't got response yet. Can you take a look and try test it out?

(pls ignore my comment at #6679 and let's stick this discussion here)

owocki commented 4 years ago

seems like it should be an easy fix right, just adding the right header to the API HTTP Response right? if someone wants to PR that up, and its a small enough fix, we can hotfix it. /cc @octavioamu

Tedko commented 4 years ago

seems like it should be an easy fix right, just adding the right header to the API HTTP Response right? if someone wants to PR that up, and its a small enough fix, we can hotfix it. /cc @octavioamu

@owocki @octavioamu any update on the API's hotfix :D ?