ubiquity / pay.ubq.fi

Generate and claim spender permits (EIP-2612)
https://pay.ubq.fi
8 stars 35 forks source link

Generate rolled up payouts #289

Open rndquu opened 2 weeks ago

rndquu commented 2 weeks ago

Depends on https://github.com/ubiquibot/conversation-rewards/issues/64

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

Overall right now (when https://github.com/ubiquibot/conversation-rewards/issues/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 commented 2 weeks ago

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

Keyrxng commented 2 weeks ago

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

Scenario:


The multiple permits per issue possibility is what made me doubt myself in https://github.com/ubiquity/.github/issues/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 commented 2 weeks ago

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:

Keyrxng commented 2 weeks ago

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:

whilefoo commented 2 weeks ago

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:

EresDev commented 2 weeks ago

I have been thinking differently about this. We will get two tables from the previous issue https://github.com/ubiquibot/conversation-rewards/issues/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 commented 1 week ago

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 https://github.com/ubiquibot/conversation-rewards/issues/64 and current github issue.

rndquu commented 1 week ago

@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 commented 1 week ago

@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 commented 1 week ago

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 commented 1 week ago

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?

This is what you mean, right?

whilefoo commented 1 week ago

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 commented 1 week ago

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 commented 1 week ago

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 commented 5 days ago

@rndquu Has the database schema been finalized?

whilefoo commented 2 days ago

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

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 commented 2 days ago

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"