Giveth / impact-graph

MIT License
49 stars 18 forks source link

Custom token donation verification issue #1226

Open aminlatifi opened 8 months ago

aminlatifi commented 8 months ago

Donations made with custom tokens flow will be incomplete (stays in the pending state forever) if we haven't listed the token in our supported tokens. In other words, the custom token support isn't open to accept donations in any new token. We have two options:

  1. Restrict custom token donation to the list of tokens we support
  2. Update our donation verification process to verify custom token donations. If we go for that, we still won't have the price and we must either consider it as unknown or again think about a way to fetch the price.
aminlatifi commented 8 months ago

@laurenluz @MoeNick

mohammadranjbarz commented 8 months ago

I write here to just dont forget myself, if we want to do it instead of throwing an exception in https://github.com/Giveth/impact-graph/blob/82b412f00ca375bd0278b532cf447ed94c62c3b1/src/services/chains/evm/transactionService.ts#L285

we should get decimals and token's symbol from chain like below comment (and do the same for solana)

https://stackoverflow.com/a/67806822/4650625

aminlatifi commented 8 months ago

I don't think we have to support custom token for solana!

jainkrati commented 7 months ago

@divine-comedian @aminlatifi @mohammadranjbarz whats the status of this task? Do you need any help or business/product/tech confirmations here?

mohammadranjbarz commented 7 months ago

@divine-comedian @aminlatifi @mohammadranjbarz whats the status of this task? Do you need any help or business/product/tech confirmations here?

I think first of all we need to know what we want to do, because if we verify the donation we still could have problem on filling price, do we want to keep this feature?

MoeNick commented 7 months ago

@aminlatifi can we detect if we can support the price or not when the user pastes the address, then if we can't we through error. If it's possible I vote for the 2nd suggestion you provided.

For now, what is important and P0 is to remove this feature from prod if it's buggy.

divine-comedian commented 7 months ago

I'm okay if we can not show the price if it isn't in our list of supported tokens - so we can verify the donation, show the donation on the project's list of donations received but we're not responsible for fetching a price for a token we don't support.

Do we know how often this happens? If we choose to support the token later will the price be fetched retroactively?

jainkrati commented 6 months ago

Many custom tokens have prices close to 0. The list of custom tokens should be something we maintain actively. What do you say @laurenluz @MoeNick @mohammadranjbarz @aminlatifi

MoeNick commented 6 months ago

This is the suggested UX:

If you can verify, that is possible, let's make it happen only on EVM compatible chains. @jainkrati @aminlatifi

jainkrati commented 6 months ago

@MoeNick are you saying that we accept donation even if the token price is 0?

jainkrati commented 6 months ago

Why is this part of the epic "Improve Dapp Performance & Resolve Tech Debts#3672" @divine-comedian . It is not a tech debt or perf issue. It is more of a user journey edge case. Can you please delink?

MoeNick commented 6 months ago

@MoeNick are you saying that we accept donation even if the token price is 0?

Yes according to @divine-comedian comments and others commet this is the best approach.

The reason it was under DApp performance was the users reported on polygon round due to save donation error

jainkrati commented 6 months ago

@mohammadranjbarz pls take this up next

divine-comedian commented 4 months ago

@RamRamez - Is there any progress on this one?

jainkrati commented 3 months ago

@RamRamez pls guide @lovelgeorge99 to take this up

lovelgeorge99 commented 2 months ago

Hi, if a donation is made with custom token do we also have to update the token database with the new custom token? @RamRamez @jainkrati

MoeNick commented 2 months ago

Hi, if a donation is made with custom token do we also have to update the token database with the new custom token? @RamRamez @jainkrati

I am tagging @divine-comedian here for the answer. We need more clarity here as the context of this issue at the top is something else.

divine-comedian commented 2 months ago

I'm going to increase the scope of this issue, since I think not trying to handle the price might lead to confusion on some donors and more likely project owners. if we show the price as non-existant or zero for a token that could be well known and have a price available this could come back to us from users reporting it as a bug or requesting support from us.

We should be able to use this coingecko API call to get the token info by address instead of coingecko Id: https://docs.coingecko.com/reference/coins-contract-address

This is the acceptance criteria:

In one-time donation flow, user can:

we then look for the token info using the above API or another method, we should be able to save and show:

If we are able to retrieve this info, we can save it into our DB, making it available for future users.

If there is no available info from coingecko for given token:

We're not able to find the details of this token, choose another token or add this token to Coingecko to allow it to be supported in the future.

Show token list.

(use current "Token List" link already in UI)

This way we allow flexible support of tokens, and if anything goes wrong it's coingecko's fault and not ours :D

If the query returns an error (our fault or API outage)

image

I hope this answers your question @lovelgeorge99