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

Generate rolled up payouts #289

Open
rndquu opened this issue Aug 28, 2024 · 22 comments
Open

Generate rolled up payouts #289

rndquu opened this issue Aug 28, 2024 · 22 comments

Comments

@rndquu
Copy link
Member

rndquu commented Aug 28, 2024

Depends on ubiquity-os-marketplace/text-conversation-rewards#64

This issue adds the "reward amount rollup" feature which adds a new DB table rewards.

Overall right now (when ubiquity-os-marketplace/text-conversation-rewards#64 is solved) there are 2 tables related to contributors' rewards:

  1. rewards: matches a single github issue with a single user reward
  2. permits: stores generated accumulated rewards

The plan is to allow contributors to select the reward amount they want to redeem (either via permits either via gift cards).

Example flow with permits:

  1. User solves an issue worth 10 WXDAI (permit is not generated, the rewards table has 1 DB entity).
  2. User solves another issue worth 20 WXDAI. (permit is not generated, at this point the rewards table has 2 entities)
  3. User opens pay.ubq.fi, selects 25 WXDAI to redeem and generates a single permit worth 25 WXDAI (a new entity is added to the permits table).
  4. User opens pay.ubq.fi again, selects 5 WXDAI to redeem and generates a single permit worth 5 WXDAI (a new entity is added to the permits table).
  5. User opens pay.ubq.fi again, selects 100 WXDAI to redeem and gets an error since there are no more funds available

Example flow with gift cards:

  1. User solves an issue worth 10 WXDAI (the rewards table has 1 DB entity).
  2. User solves another issue worth 20 WXDAI. (at this point the rewards table has 2 entities)
  3. User opens pay.ubq.fi, selects 25 WXDAI to redeem and generates a new gift card worth 25 WXDAI (a new entity is added to the gift_cards table).
  4. User opens pay.ubq.fi again, selects 5 WXDAI to redeem and generates another gift card worth 5 WXDAI (a new entity is added to the gift_cards table).
  5. User opens pay.ubq.fi again, selects 100 WXDAI to redeem and gets an error since there are no more funds available

So as a part of this issue we should:

  1. Allow users to select available rewards amount (on the backend subtract the sum of redeemed permits and gift cards from total rewards stored in the rewards table).
  2. On gift card creation save it to a DB (to the newly added gift_cards table)
  3. On permit generation save it to a DB (to the permits table that already exists)
@rndquu
Copy link
Member Author

rndquu commented Aug 28, 2024

@EresDev @gentlementlegen @whilefoo @Keyrxng Guys pls check the description

@Keyrxng
Copy link
Member

Keyrxng commented Aug 29, 2024

One potential issue is that issues get re-opened and permits get regenerated where the 2nd is a higher value than the first.

Scenario:

  • issue closes and $400 reward is generated and claimed
  • issue reopens and for whatever reason a higher value permit is created
  • the rewards entry would be updated with the new amount despite the original permit having been claimed

The multiple permits per issue possibility is what made me doubt myself in ubiquity/.github#109

So how would we effectively manage the case where multiple permits on a task for a user considering the claimed amount is final and is intended to be so that once you claim that's it?

@rndquu
Copy link
Member Author

rndquu commented Aug 29, 2024

One potential issue is that issues get re-opened and permits get regenerated where the 2nd is a higher value than the first.

Scenario:

  • issue closes and $400 reward is generated and claimed
  • issue reopens and for whatever reason a higher value permit is created
  • the rewards entry would be updated with the new amount despite the original permit having been claimed

The multiple permits per issue possibility is what made me doubt myself in ubiquity/.github#109

So how would we effectively manage the case where multiple permits on a task for a user considering the claimed amount is final and is intended to be so that once you claim that's it?

Good point. We had this issue in the bot v1.

I guess right now the ubiquibot/conversation-rewards plugin should be responsible for penalties. So when issue is reopened we add a new DB entry to the penalties table, like:

user_id reward_id
10 20

How it works:

  • when issue is reopened we simply add a new entry to the penalties table, on the backend we can still count the available reward amount (reward amount = rewards - generated permits - generated gift cards - penalties)
  • if issue is reopened 2nd time we simply don't add anything to the penalties table (since penalty is already added)
  • if issue is closed as "completed" then we delete the penalty record for that issue reward

@Keyrxng
Copy link
Member

Keyrxng commented Aug 29, 2024

if issue is reopened 2nd time we simply don't add anything to the penalties table (since penalty is already added)
if issue is closed as "completed" then we delete the penalty record for that issue reward

Do you think we'd also need to:

  • verify who completed the issue after it was re-opened as the penalty might still be valid if the original assignee didn't resolve it
  • verify it was the same assignee when it was re-opened the nth time. Otherwise we may not add a penalty to the 2nd contributor who completed the task and had it re-opened. Does that make sense?

@whilefoo
Copy link
Contributor

This means that the user will need to sign in with Github (oAuth) so we can verify which user it is.
Since this is deployed on Cloudflare, I guess we can use Pages functions as backend (probably easier) or a separate Worker.

Additionally we could remove /wallet command because they can choose wallet when they generate a permit on pay.ubq.fi (we could also make them sign a message to make sure they own the wallet and are not using CEX), but I'm not sure how we can use the gas faucet with this flow.

How it works:

  • when issue is reopened we simply add a new entry to the penalties table, on the backend we can still count the available reward amount (reward amount = rewards - generated permits - generated gift cards - penalties)
  • if issue is reopened 2nd time we simply don't add anything to the penalties table (since penalty is already added)
  • if issue is closed as "completed" then we delete the penalty record for that issue reward

If we delete the penalty when the issue is closed as completed then the reward amount will increase

For example:

  • Issue is closed (reward 100$)
  • user claims 100$
  • issue is reopened (penalty 100$)
  • issue is closed 2nd time with reward 200$
  • available amount to claim is 100$(first reward) - 100$(claimed) - 100$(reopened penalty) + 200$(new reward) = 100$
  • if we removed the penalty, available amount would be 200$

@EresDev
Copy link
Contributor

EresDev commented Aug 31, 2024

I have been thinking differently about this. We will get two tables from the previous issue ubiquity-os-marketplace/text-conversation-rewards#64 reward, and permits.

The plan is to allow contributors to select the reward amount they want to redeem (either via permits either via gift cards).

To achieve the goal of this issue, what if we reduce the tables instead of increasing them? Keep the the credits and debits in the same table. This probably goes against the relational database normalization techniques, but based on our use case, this looks more efficient to me.

id created issue_id user_id amount (signed int) reason details
1 2023-12-13 13:58:11.49168+00 12 111 100 issue_assignee null
2 2023-12-13 13:58:11.49168+00 0 111 -60 gift_card reloadly tx id
3 2023-12-13 13:58:11.49168+00 0 111 -40 permit permit 2 maybe in json

We should store the amount as a signed integer. So that we can store both credits and debits in one table. The benefit is simplicity. Less tables to understand. No separate aggregate function runs on the separate tables. I don't think we are going to store much data in details. In fact, I see reducing it even more because we can retrieve it from services and blockchain that are already storing it.

We are not going to store Reloadly virtual card number in our DB. Why expose ourselves to risk? Reloadly has it. We can retrieve it when needed. In fact, we don't even need to store reloadly tx id, as we can just store the id of this table row in reloadly.

@rndquu
Copy link
Member Author

rndquu commented Sep 5, 2024

I have been thinking differently about this. We will get two tables from the previous issue ubiquibot/conversation-rewards#64 reward, and permits.

The plan is to allow contributors to select the reward amount they want to redeem (either via permits either via gift cards).

To achieve the goal of this issue, what if we reduce the tables instead of increasing them? Keep the the credits and debits in the same table. This probably goes against the relational database normalization techniques, but based on our use case, this looks more efficient to me.

This approach definitely breaks DB normalization but:

  1. Way simpler to comprehend compared to the separate tables for permits, rewards, penalties approach (since we can use all of those entities in a single table)
  2. It's easy to add new payout methods like virtual cards or NFT rewards

By the way supabase allows searching over JSON fields in a DB.

If @whilefoo and @Keyrxng are fine with this "single table" approach then I'll update descriptions for ubiquity-os-marketplace/text-conversation-rewards#64 and current github issue.

@rndquu
Copy link
Member Author

rndquu commented Sep 5, 2024

@Keyrxng

verify who completed the issue after it was re-opened as the penalty might still be valid if the original assignee didn't resolve it

Yes

verify it was the same assignee when it was re-opened the nth time. Otherwise we may not add a penalty to the 2nd contributor who completed the task and had it re-opened. Does that make sense?

Yes

@rndquu
Copy link
Member Author

rndquu commented Sep 5, 2024

@whilefoo

This means that the user will need to sign in with Github (oAuth) so we can verify which user it is.
Since this is deployed on Cloudflare, I guess we can use Pages functions as backend (probably easier) or a separate Worker.

Yes

Additionally we could remove /wallet command because they can choose wallet when they generate a permit on pay.ubq.fi

I suppose it's better to keep the /wallet command (at least for now) in case there's smth wrong on the pay.ubq.fi side (on wallet generation step)

but I'm not sure how we can use the gas faucet with this flow

Right now the plan for the "account abstaction" feature v1 is this simple flow:

  1. User solves an issue
  2. Reward is accumulated in a DB (permit is not generated)
  3. User opens pay.ubq.fi and generates a wallet via webauthn
  4. User opens github back and calls /wallet to register a reward address (at this step crypto-faucet-plugin catches the /wallet command event and sends some funds)

If we delete the penalty when the issue is closed as completed then the reward amount will increase
For example:

  • Issue is closed (reward 100$)
  • user claims 100$
  • issue is reopened (penalty 100$)
  • issue is closed 2nd time with reward 200$
  • available amount to claim is 100$(first reward) - 100$(claimed) - 100$(reopened penalty) + 200$(new reward) = 100$
  • if we removed the penalty, available amount would be 200$

In most cases (at least as I've seen so far) the reopened issue will be solved by another contributor, not the one who initially tried to solve it. But this example provides an interesting edge case where:

  1. Task reward is increased from 100$ to 200$
  2. The same contributor solves the reopened issue again

I believe the example flow with the removed penalty is still correct.

Example (issue is solved by another contributor):

  1. Issue is closed (reward 100$)
  2. User1 claims 100$
  3. Issue is reopened (penalty 100$ for User1)
  4. Issue is closed 2nd time with reward 200$ for User2

Example (issue is solved by the same contributor):

  1. Issue is closed (reward 100$)
  2. User1 claims 100$
  3. Issue is reopened (penalty 100$ for User1)
  4. Issue is closed 2nd time with reward 200$ for User1

In both cases we treat the reopened issue as a new task and should reward the full amount (200$) even for a reopened issue regardless of who solved the issue 2nd time (initial solver or another contributor).

@whilefoo
Copy link
Contributor

whilefoo commented Sep 5, 2024

I believe the example flow with the removed penalty is still correct.

Example (issue is solved by another contributor):

  1. Issue is closed (reward 100$)
  2. User1 claims 100$
  3. Issue is reopened (penalty 100$ for User1)
  4. Issue is closed 2nd time with reward 200$ for User2

In this case why would you remove penalty for user1?

If @whilefoo and @Keyrxng are fine with this "single table" approach then I'll update descriptions for ubiquibot/conversation-rewards#64 and current github issue.

I agree with this approach. I think it's also easier to query how much is the available amount by just summing all amount rows or displaying all transactions to the user.

@rndquu
Copy link
Member Author

rndquu commented Sep 5, 2024

I believe the example flow with the removed penalty is still correct.

Example (issue is solved by another contributor):

  1. Issue is closed (reward 100$)
  2. User1 claims 100$
  3. Issue is reopened (penalty 100$ for User1)
  4. Issue is closed 2nd time with reward 200$ for User2

In this case why would you remove penalty for user1?

  • If issue is solved 2nd time by the same contributor then we only pay reward - penalty. So in the example above User1 would get 200$ (100$ for solving initial issue + 100$ because issue reward has been updated from 100$ to 200$)
  • If issue is solved 2nd time by another contributor then we pay the full 200$ reward

This is what you mean, right?

@whilefoo
Copy link
Contributor

whilefoo commented Sep 5, 2024

So you agree with my approach? Because I thought you wanted to remove penalties when issue is closed by saying I believe the example flow with the removed penalty is still correct.

@whilefoo
Copy link
Contributor

whilefoo commented Sep 5, 2024

When thinking about this I've just remembered that we initially wanted that partners can set their own reward token which can be any ERC20 token which might not be pegged to USD and they could also set the network.

By implementing rolled up rewards, we are practically restricting payment token and network to only one that we choose. @0x4007 rfc

@0x4007
Copy link
Member

0x4007 commented Sep 6, 2024

For simplicity we can make v1 of rolled up rewards only support a single asset on a single network I think that's reasonable. Ideally the user can select what asset and network and then roll them up.

I think it's reasonable for me to start finding partners to try and settle either in wxdai or ubiquity dollars on gnosis chain / ethereum

@hhio618
Copy link
Contributor

hhio618 commented Sep 9, 2024

@rndquu Has the database schema been finalized?

@whilefoo
Copy link
Contributor

whilefoo commented Sep 12, 2024

@rndquu I've just realised that we can't roll up rewards across all orgs since every org has their own wallet for permits, so we have to roll up based on the org so the user has to claim multiple times which is not ideal but better than doing it for every task.

Unless we find another way like transferring the amount to our wallet and then generating a permit, for example:

  • User has earned 20$ from org A and 30$ from org B
  • User goes to pay.ubq.fi to claim their rewards
  • The backend generate 20$ permit from org A's wallet and 30$ permit from org B's wallet to ubiquity's owned wallet
  • The backend claims those permits and generates 50$ - (fees for transfers to our wallet) permit from ubiquity's wallet

However this flow is quite complex and a lot of points of failures like 1 permit can't be claimed so others have to be reverted...so probably not worth implementing

@0x4007
Copy link
Member

0x4007 commented Sep 12, 2024

We could roll up based on the wallet? Both our orgs use the same wallet, so perhaps from the perspective of the plugin it's essentially one "org"

@0x4007
Copy link
Member

0x4007 commented Sep 30, 2024

@whilefoo rfc

@whilefoo
Copy link
Contributor

We can optimize by also looking at the wallet but the UX might be confusing to the user, however I don't anticipate that many partners will have multiple Github organizations.

Based on @EresDev 's schema and previous conversation I propose this modified schema:

id created org_id issue_id user_id amount (signed int) reason details
1 2023-12-13 13:58:11.49168+00 1 12 111 100 task_reward null
2 2023-12-13 13:58:11.49168+00 1 null 111 -60 gift_card reloadly tx id
3 2023-12-13 13:58:11.49168+00 1 null 111 -40 permit permit 2 maybe in json

We will need to remove the reward token and network option in the config since the user will choose which token and network they want on claim page (at first there will be only one option).

Another area to consider is non-standard rewards such as NFTs and governance/utility tokens. Do we plan to support that @0x4007 ?

@0x4007
Copy link
Member

0x4007 commented Sep 30, 2024

Another area to consider is non-standard rewards such as NFTs and governance/utility tokens. Do we plan to support that @0x4007 ?

The planned NFTs are basically POAPs and don't have monetary value.

As for the governance/utility tokens, I suppose its up to the plugins themselves to handle them. It is nice to keep tabs on our ledger but we need to snapshot its market value at the time of payout. Perhaps the column name should be a reference to that "market value (at time of permit generation)" which would apply for all tokens, including stablecoins.

@whilefoo
Copy link
Contributor

whilefoo commented Oct 1, 2024

It is nice to keep tabs on our ledger but we need to snapshot its market value at the time of payout. Perhaps the column name should be a reference to that "market value (at time of permit generation)" which would apply for all tokens, including stablecoins.

Why would we need to keep the market value? If the reward is in governance tokens it should be paid out in governance tokens not USD, no?

If we want to keep track of all different rewards we will need another column 'reward_type' which can be 'usd', 'poap', 'token'.

@0x4007
Copy link
Member

0x4007 commented Oct 1, 2024

Intuitively I feel that most companies would keep track of the value of the tokens going through the system for accounting and marketing reasons.

I could see it on our UBQ.FI page under the network stats. But if it's too complicated then we can kick the can down the road.

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

No branches or pull requests

7 participants
@hhio618 @0x4007 @EresDev @Keyrxng @rndquu @whilefoo and others