search

gitcoinco / web

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

Grant owners cannot self-fund grants via recurring subscription #5459

Closed danlipert closed 4 years ago

danlipert commented 5 years ago

Describe the bug If a grant owner attempts to self fund a grant, the subscriptions will fail in the subminer due to a check in the grant contract code that the origin and destination addresses are not the same: https://github.com/danlipert/grants1337/blob/specify-relayer/contracts/Subscription.sol#L186

To Reproduce Create a grant and then with the same recipient address create a recurring subscription. Run the subminer and the metatx will fail.

Expected behavior Given that this code is already in the deployed contracts, changing it now may not be wise. I suggest we popup a message or disable recurring subscriptions in this case or something similar instead of changing the contract. @owocki what are your thoughts?

owocki commented 5 years ago

its not the grant owner that cant be the same, its the origin/destination addresses right? owner is a seperate field

https://gitcoin.co/grants/151/codefund-part-time-software-engineer-2 for example ; my subs is going fine bc the destination addr != my address (though i am owner)

regardless, i think ur fix is a good one @danlipert ! shalli bounty away!?

danlipert commented 5 years ago

@owocki ah yes - good catch there! Seems like a good one for a bounty 👍

gitcoinbot commented 5 years ago

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

This issue now has a funding of 0.43 ETH (80.08 USD @ $186.24/ETH) attached to it.

gitcoinbot commented 5 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, 3 weeks ago. Please review their action plans below:

1) zoek1 has been approved to start work.

I already worked with grant on previous bounties, so i'm familiarwith the codebase. The steps i'll take are following:

  1. Add validation to the client side to prevent populate the same address using alerts to give feedbak to the user.
  2. Add validation to the server to apply same logic.
  3. Add test to ensure this behavior

Learn more on the Gitcoin Issue Details page.

thelostone-mc commented 5 years ago

@danlipert just to add the UI flow does have a check in place to prevent the owner from funding his own grant

danlipert commented 5 years ago

@thelostone-mc Kevin recently removed it

owocki commented 5 years ago

Yes it’s true

On Wed, Nov 13, 2019 at 6:36 AM Dan Lipert notifications@github.com wrote:

@thelostone-mc https://github.com/thelostone-mc Kevin recently removed it

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/gitcoinco/web/issues/5459?email_source=notifications&email_token=AAD5PCKTWQL4JA2QZXLHOCTQTQGGVA5CNFSM4JJW2IGKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOED6K6DY#issuecomment-553430799, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAD5PCMPU2EUN45FFFB66NDQTQGGVANCNFSM4JJW2IGA .

--

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

check out what i'm building on github http://github.com/owocki

or what i'm shooting on photoshelter http://photography.owocki.com/

or find me on twitter http://www.twitter.com/owocki, facebook http://www.facebook.com/kevinowocki, instagram http://instagram.com/owocki, linkedin http://www.linkedin.com/in/owocki, and owocki.com http://www.owocki.com/?utm_source=emailsig.

see you around teh internets!

PS -- Come to the next Boulder Blockchain https://www.meetup.com/Boulder-Blockchain/ meetup. Be there or be ^ 2. PPS -- Have I mentioned that *G*itcoin is live? More @ https://gitcoin.co

thelostone-mc commented 5 years ago

@owocki wouldn't that mess up CLR stuff if we allowed folks to fund their own grant ?

owocki commented 5 years ago

If it does, we can exclude self contributors from the CLR set.

The reason I reenabled it is so I can use the platform to pay contractors and community members on a regular basis.

On Wed, Nov 13, 2019 at 11:56 AM Aditya Anand M C notifications@github.com wrote:

@owocki https://github.com/owocki wouldn't that mess up CLR stuff if we allowed folks to fund their own grant ?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/gitcoinco/web/issues/5459?email_source=notifications&email_token=AAD5PCOVTL5GC55DWDZEMW3QTRLXDA5CNFSM4JJW2IGKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOED7OGQA#issuecomment-553575232, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAD5PCJNUFYAFMYZA2JCIIDQTRLXDANCNFSM4JJW2IGA .

--

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

check out what i'm building on github http://github.com/owocki

or what i'm shooting on photoshelter http://photography.owocki.com/

or find me on twitter http://www.twitter.com/owocki, facebook http://www.facebook.com/kevinowocki, instagram http://instagram.com/owocki, linkedin http://www.linkedin.com/in/owocki, and owocki.com http://www.owocki.com/?utm_source=emailsig.

see you around teh internets!

PS -- Come to the next Boulder Blockchain https://www.meetup.com/Boulder-Blockchain/ meetup. Be there or be ^ 2. PPS -- Have I mentioned that *G*itcoin is live? More @ https://gitcoin.co

gitcoinbot commented 4 years ago

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

zoek1 commented 4 years ago

I'll work on this tomorrow, i'm focusing on solve creation, funding and canceling grants #5466 :sweat_smile:

gitcoinbot commented 4 years ago

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

zoek1 commented 4 years ago

Yep, still working on this!

El vie., 22 de noviembre de 2019 11:16 a. m., Gitcoin.co Bot < notifications@github.com> escribiĂł:

@zoek1 https://github.com/zoek1 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!

  • reminder (3 days)
  • escalation to mods (6 days)

Funders only: Snooze warnings for 1 day https://gitcoin.co/issue/gitcoinco/web/5459/3686?snooze=1 | 3 days https://gitcoin.co/issue/gitcoinco/web/5459/3686?snooze=3 | 5 days https://gitcoin.co/issue/gitcoinco/web/5459/3686?snooze=5 | 10 days https://gitcoin.co/issue/gitcoinco/web/5459/3686?snooze=10 | 100 days https://gitcoin.co/issue/gitcoinco/web/5459/3686?snooze=100

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/gitcoinco/web/issues/5459?email_source=notifications&email_token=AAFBL3JKQHHQQSANUDNP6CLQVAHW5A5CNFSM4JJW2IGKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEE6I7DQ#issuecomment-557617038, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFBL3JEJNU4QO2PCABOUB3QVAHW5ANCNFSM4JJW2IGA .

gitcoinbot commented 4 years ago

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

zoek1 commented 4 years ago

yes, @gitcoinbot. I'll submit a PR the monday.

gitcoinbot commented 4 years ago

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

owocki commented 4 years ago

@zoek1 how goes brah

zoek1 commented 4 years ago

Good, you can see the changes here @owocki #5599

gitcoinbot commented 4 years ago

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

Work for 0.43 ETH (64.22 USD @ $149.35/ETH) has been submitted by:

  1. @zoek1

@owocki please take a look at the submitted work:

thelostone-mc commented 4 years ago

@owocki / @danlipert so we ended up going with https://github.com/gitcoinco/web/pull/5807 which blocks the grant owners from funding their own grant all together

I've closed out the PR as it's not needed any longer but the contributor @zoek1 's PR had the changes that was originally asked for

gitcoinbot commented 4 years ago

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

The funding of 0.43 ETH (87.36 USD @ $203.16/ETH) attached to this issue has been approved & issued to @zoek1.