Skip to content
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

prevent users on OFAC list from donating to Endaoment projects #4449

Closed
divine-comedian opened this issue Jul 18, 2024 · 26 comments
Closed

prevent users on OFAC list from donating to Endaoment projects #4449

divine-comedian opened this issue Jul 18, 2024 · 26 comments
Assignees

Comments

@divine-comedian
Copy link
Contributor

divine-comedian commented Jul 18, 2024

Users who are on the OFAC list should not be allowed to donate to Endaoment projects from the Giveth FE.

Endaoment will provide code snippets on how to use the TRM Endpoint which we can use to check on the front-end, here it is:
https://github.com/pedroyan/trm-sanctions-demo

If a project is detected on this list we should give them a modal warning that prevents them from proceeding in making a donation and can redirect them to the all-projects page

Flow

  1. User navigates to donate page for Endaoment project
  2. Perform check if the connected address is on sanctions list
  3. If true, show pop-up modal with the following attributes:
    a. Has header and description text (will provide below)
    b. Cannot be closed while the sanctioned wallet is connected to dapp, user must disconnect from app, switch wallet or leave the page
    c. modal contains button to take user back to all-projects page
  4. If false, user continues as normal making their donation

modal text

Header:

Sanctioned Address Detected

text:

This address has been found on the USA OFAC sanctions list. Unfortunately, Endaoment does not permit addresses on the OFAC sanction list to donate to projects delivered by Endaoment. Check out another project to donate to.

button text:

View All Projects


@mohammadranjbarz - please check out the code snippet above and get familiar with it.

@Tosinolawale - can we draft up this modal that we need to implement?

@divine-comedian divine-comedian self-assigned this Jul 18, 2024
@github-project-automation github-project-automation bot moved this to New Issues in All-Devs Jul 18, 2024
@divine-comedian divine-comedian moved this from New Issues to Design in All-Devs Jul 19, 2024
@Tosinolawale

This comment was marked as outdated.

@divine-comedian

This comment was marked as outdated.

@Tosinolawale
Copy link

We'll have to see but I think it is very quick - I hope we can implement this feature with just 1 modal and a loading spinner if necessary. 90% of donors I think will never need to see this modal - the words OFAC, sanctions etc... are just bad vibes.

Info
https://www.figma.com/design/vMiyVd1Ly5LjgSyZrChVV8/Giveth-users?node-id=3045-18067&t=8AgZPV0S7EDbd1da-1
we can just have this then and have the loading state in the background

@divine-comedian divine-comedian moved this from Design to Dev Research in All-Devs Jul 23, 2024
@divine-comedian
Copy link
Contributor Author

@mohammadranjbarz - the design is ready for you to pickup once you got a good grasp on integrating the above mentioned TRM endpoint for checking addresses

@divine-comedian
Copy link
Contributor Author

Can wrap an endpoint in our back-end so the FE only needs to communicate with our back-end and not another service.

@mohammadranjbarz
Copy link
Contributor

Can wrap an endpoint in our back-end so the FE only needs to communicate with our back-end and not another service.

I didn't do that because their webservice has rate limit, so if we call all requests from our server will get 429 error but if we call it from client side would not get that error, but once we get the apiKey from them in future we can do it (and also not implementing BE side was faster)

@LatifatAbdullahi
Copy link

@mohammadranjbarz @divine-comedian

Test Update

  • The modal has header and description text (will provide below) - Pass
  • The modal cannot be closed while the sanctioned wallet is connected to dapp, user must disconnect from app, switch wallet or leave the page - Pass
  • modal contains button to take user back to all-projects page - Pass

image

Give.directly.to.for-good.projects.with.crypto.zero.fees.-.Google.Chrome.2024-08-09.11-24-25.mp4

@divine-comedian divine-comedian moved this from Dev Research to QA in All-Devs Aug 9, 2024
@divine-comedian
Copy link
Contributor Author

@mohammadranjbarz - are you trigger this on the FE when the user tries to click the donate button or is there a delay?

Ideally we don't let the user get so far as to select tokens and click the donate button, as soon as we have an address connected to the dapp on this page we run the OFAC check and show the modal.

@divine-comedian
Copy link
Contributor Author

We can launch this feature as is and fix the above as a fast-follow

@divine-comedian
Copy link
Contributor Author

@mohammadranjbarz to come back to this once we finish polygon zkEVM

@divine-comedian divine-comedian moved this from QA to In Progress in All-Devs Aug 21, 2024
@divine-comedian divine-comedian moved this from In Progress to Code Review/PR in All-Devs Aug 21, 2024
@divine-comedian
Copy link
Contributor Author

@HrithikSampson - please tag @LatifatAbdullahi to test one this is ready for QA on develop

@divine-comedian
Copy link
Contributor Author

still waiting for PR review from @mohammadranjbarz @MohammadPCh or @jainkrati to move this issue to QA
#4589

@mohammadranjbarz
Copy link
Contributor

still waiting for PR review from @mohammadranjbarz @MohammadPCh or @jainkrati to move this issue to QA #4589

Thanks for reminding, I just reviewed right now

@HrithikSampson HrithikSampson moved this from Code Review/PR to QA in All-Devs Sep 4, 2024
@HrithikSampson
Copy link
Collaborator

@LatifatAbdullahi This issue is ready for QA on develop.

@LatifatAbdullahi
Copy link

LatifatAbdullahi commented Sep 4, 2024

@HrithikSampson @mohammadranjbarz

Where should I test this fix? I can't test it staging, as I can make donations there.

Or, probably add this address to the list 0x94C38D692C888C7CBFb7d4c0Dd75424acCc1609B, so I can test with it

@HrithikSampson
Copy link
Collaborator

Hi @LatifatAbdullahi ,

I forgot to mention that I had created this pull request for testing it

#4590

The preview link is here:

https://giveth-dapps-v2-git-test-issue-4449-dont-merge-givethio.vercel.app/

@LatifatAbdullahi
Copy link

@divine-comedian

Here is the current behaviour

Give.directly.to.for-good.projects.with.crypto.zero.fees.-.Google.Chrome.2024-09-04.09-29-39.mp4

@divine-comedian
Copy link
Contributor Author

Thanks for the video Latifat

The issue I see is that when you are connected to an unsupported network and click on the View All Projects from the "Sanctioned Address" modal instead of redirecting you to the page the "unsupported chain" modal pops up and prevents the redirect.

@HrithikSampson - when the user clicks the View All Projects button it should redirect to eh All projects page and should not be interrupted by any other modals

@HrithikSampson
Copy link
Collaborator

Hi @divine-comedian @LatifatAbdullahi ,
https://giveth-dapps-v2-git-dontmergebuttestissue4449-givethio.vercel.app/

I am not sure whether to do the above process in OneTime Donation or the Superfluid Donation which is appearing now since the WrongNetworkModel is in OneTimeDonation Card but I think I was able to solve this issue but in the OneTime Donation Card. I can transfer the OneTime Donation Modal to the Superfluid Donation or should I keep it in OneTimeDonation Card.

Previously Endaoment Projects only had OneTimeDonation Card.
Screenshot 2024-09-10 at 11 55 27 AM

@divine-comedian

Here is the current behaviour

Give.directly.to.for-good.projects.with.crypto.zero.fees.-.Google.Chrome.2024-09-04.09-29-39.mp4

@divine-comedian
Copy link
Contributor Author

divine-comedian commented Sep 10, 2024

I pushed a hotfix for this on production - you can copy the same
#4710

endaoment projects are not eligible for recurring donations so we need to make sure it isn't possible to land on that recurring donation view. Safe to say then we should only need to do the OFAC check on one time view

@HrithikSampson HrithikSampson moved this from In Progress to Code Review/PR in All-Devs Sep 12, 2024
@divine-comedian
Copy link
Contributor Author

@HrithikSampson if the testing PR you made is ready and with the latest changes we made can we mark this as ready for test? Please assign @LatifatAbdullahi if it is ready to go.

@HrithikSampson
Copy link
Collaborator

@LatifatAbdullahi @divine-comedian ,Testing PR has been made. The preview link is this : https://giveth-dapps-v2-git-dontmergebuttestissue4449-givethio.vercel.app/

I was just waiting for #4711 to be approved

@LatifatAbdullahi
Copy link

LatifatAbdullahi commented Sep 17, 2024

@divine-comedian @HrithikSampson

This is the current behavior

Give.directly.to.for-good.projects.with.crypto.zero.fees.-.Google.Chrome.2024-09-17.15-02-52.mp4

@HrithikSampson
Copy link
Collaborator

Thanks @LatifatAbdullahi , I will try to fix this glitching.

@HrithikSampson
Copy link
Collaborator

@LatifatAbdullahi ,I still have to get review for my PR.
I think I have fixed it in this preview link
https://giveth-dapps-v2-git-dontmergebuttestissue4449-givethio.vercel.app/

@LatifatAbdullahi
Copy link

@HrithikSampson @HrithikSampson

This issue seems fixed

Nova.Ukraine._.Giveth.-.Google.Chrome.2024-09-23.20-32-51.mp4

@divine-comedian divine-comedian moved this from Code Review/PR to Done in All-Devs Sep 23, 2024
@divine-comedian divine-comedian closed this as completed by moving to Merged to Production in All-Devs Oct 3, 2024
@github-project-automation github-project-automation bot moved this from Merged to Production to Done in All-Devs Oct 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

No branches or pull requests

5 participants