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

Reward amount rollup #64

Open
rndquu opened this issue Jul 19, 2024 · 24 comments
Open

Reward amount rollup #64

rndquu opened this issue Jul 19, 2024 · 24 comments

Comments

@rndquu
Copy link
Member

rndquu commented Jul 19, 2024

Right now the permit redeem flow is the following:

  1. Contributor solves an issue
  2. conversation-rewards plugin generates a permit, saves it to a DB and displays it in github comments
  3. Contributor redeems permit at pay.ubq.fi

This PR introduces claiming rewards to gift cards.

The updated flow of the permit redeem should be following:

  1. Contributor solves an issue
  2. Permit reward amount is accumulated in a DB
  3. Contributor opens pay.ubq.fi, generates either an on-chain single permit (possibly for multiple solved issues) either redeems to a gift card

So as a part of this issue we should accumulate contributor rewards in a DB similar to how 0x4007 initially implemented it with the debits, cerdits, settlements tables.

Screenshot 2024-07-19 at 19 04 18

When this issue of accumulated rewards is ready we can just disable permit generation (via the plugin's permitGeneration.enabled option) and let contributors redeem only at pay.ubq.fi.

@hhio618
Copy link

hhio618 commented Aug 21, 2024

/start

Copy link
Contributor

ubiquity-os bot commented Aug 21, 2024

DeadlineThu, Aug 22, 9:10 AM UTC
Registered Wallet 0x6321286F9B73f427C72e1f9F1bC6b3d25eF06605
Tips:
  • Use /wallet 0x0000...0000 if you want to update your registered payment wallet address.
  • Be sure to open a draft pull request as soon as possible to communicate updates on your progress.
  • Be sure to provide timely updates to us when requested, or you will be automatically unassigned from the task.

@0x4007
Copy link
Member

0x4007 commented Aug 22, 2024

You might want to offer some clarity/help to the assignee @rndquu

@hhio618
Copy link

hhio618 commented Aug 26, 2024

@rndquu
I reviewed the code at https://github.com/ubiquity/pay.ubq.fi/blob/beta and also https://github.com/ubiquity/ubiquibot . To proceed effectively, I need some clarification on the task. Could you please provide answers to the following questions:

Should the new tables you mentioned be added to this repo, or should they be part of the bot repo here: ubiquityos?
What is the purpose of the original tables you referenced? Can I reuse them, and if so, could you provide a reference to the previous work or offer more details about the data flow?
Do I need to add functions to the https://github.com/ubiquity/pay.ubq.fi/blob/beta repo to settle the database after the payout (whether it's a gift card or crypto)?

@rndquu
Copy link
Member Author

rndquu commented Aug 27, 2024

@hhio618 Hey

Should the new tables you mentioned be added to this repo, or should they be part of the bot repo here: ubiquityos?

Those are plugin related tables so definitely not part of ubiquityos. The https://github.com/ubiquibot/conversation-rewards plugin uses https://github.com/ubiquibot/permit-generation plugin for permit generation under the hood. To make things consistent it makes sense to add related DB migrations to https://github.com/ubiquibot/permit-generation/tree/development/supabase/migrations.

What is the purpose of the original tables you referenced? Can I reuse them, and if so, could you provide a reference to the previous work or offer more details about the data flow?

@0x4007 might know better. You may design DB as you see fit. I would start with a single rewards table (for example):

id user_id created updated github_issue_node_id amount token_id
1 2 2023-12-13 13:58:11.49168+00 2023-12-13 13:58:11.49168+00 123asd 100 1
2 3 2023-12-13 13:58:11.49168+00 2023-12-13 13:58:11.49168+00 456qwe 200 1

So the flow could be:

  1. Github issue is closed as "completed"
  2. The https://github.com/ubiquibot/conversation-rewards generates the rewards
  3. Rewards are saved in the rewards table (example above)
  4. Contributor opens pay.ubq.fi and generates a new permit (which includes multiple entities from the rewards table) which is later saved to the permits table

Overall there will be the following tables:

  1. rewards: for storing rewards per user per github issue
  2. permits: for storing generated permits (i.e. accumulated payouts)
  3. permits_rewards: to match single rewards to generated permits
  4. gift_cards: for storing gift cards ordered from https://www.reloadly.com/
  5. gift_cards_rewards: to match single rewards to gift cards (since a single gift card may contain multiple rewards)

So as a part of this issue the rewards and permits_rewards tables should be implemented.

Example 1:

  1. User solves an issue worth 10 WXDAI.
  2. User solves another issue worth 20 WXDAI. (at this point the rewards table has 2 entities)
  3. User opens pay.ubq.fi and generates a single permit worth 30 WXDAI (a new entity is added to the permits table + 2 new entities are added to the permits_rewards table)

Example 2:

  1. User solves an issue worth 10 WXDAI.
  2. User solves another issue worth 20 WXDAI. (at this point the rewards table has 2 entities)
  3. User opens pay.ubq.fi and generates a gift card payout worth 30 WXDAI (a new entity is added to the gift_cards table + 2 new entities are added to the gift_cards_rewards table)

Do I need to add functions to the https://github.com/ubiquity/pay.ubq.fi/blob/beta repo to settle the database after the payout (whether it's a gift card or crypto)?

No, this task is regarding solely for saving rewards (i.e. accumulating) in a DB.

Notice:

  1. Right now there's no feature of "generating a permit on demand" at pay.ubq.fi. It'll be added once this github issue is solved.
  2. You may find the whole DB schema here. (@Keyrxng pls correct me if it's still relevant)

One last thing is that the "reward amount rollup" should work this way (in order to be backwards compatible with our existing infrastructure):

  1. When github issue is closed as completed and permit-generation-module is enabled (i.e. real permit is generated and displayed) then:
    a) a new DB entity as added to the rewards table
    b) a new DB entity is added to the permits table
    c) a new DB entity is added to the permits_rewards table
  2. When github issue is closed as completed and permit-generation-module is disabled (i.e. real permit is not generated since contributor will generate it later at pay.ubq.fi) then:
    a) a new DB entity as added to the rewards table

This way we distinguish claimed and unclaimed rewards.

@rndquu
Copy link
Member Author

rndquu commented Aug 27, 2024

Ideally DB design should be the one that supports different payout methods:

  • uniswap permits
  • giftcards
  • virtual cards
  • cash
  • etc...

So DB tables could look like:

  • rewards: match github issue and user's payout
  • payout_types: permits, gift cards, virtual cards, etc...
  • settlements: which abstracts permits, gift cards, etc... into a single entity (usually by a meta json field :()
  • settlements_rewards: matches rewards with payouts

The issue with the settlements table is that:

  1. it abstracts too much, code readability and maintainability will suffer
  2. we're a startup and everything changes too fast
  3. it's hard to filter over the meta json field

So I would prefer simplicity with 5 different tables (compared to the solution described above):

  1. rewards: for storing rewards per user per github issue
  2. permits: for storing generated permits (i.e. accumulated payouts)
  3. permits_rewards: to match single rewards to generated permits
  4. gift_cards: for storing gift cards ordered from https://www.reloadly.com/
  5. gift_cards_rewards: to match single rewards to gift cards (since a single gift card may contain multiple rewards)

P.S. I think when the "permit or gift card generation on demand at pay.ubq.fi" feature is fully ready then it makes sense to refactor to v2 that would support different payout types

@rndquu
Copy link
Member Author

rndquu commented Aug 27, 2024

@0x4007 What payout types do we plan to support except for permits, gift cards and virtual cards?

@0x4007
Copy link
Member

0x4007 commented Aug 28, 2024

The original idea was to support:

  1. XP rewards as a form of payout but I think maybe not a priority. Now the plan is to derive XP based on total rewards earned. According to the XP specification (not implemented yet) we also have the ability to modify this total with any positive or negative number, and associate to any GitHub issue.
  2. Arbitrary tokens like governance tokens.
  3. NFT rewards but maybe not a priority.

@rndquu
Copy link
Member Author

rndquu commented Aug 28, 2024

@hhio618 It occurred to me that the permits_rewards table is not really necessary.

Check this example:

  1. User solves an issue worth 10 WXDAI.
  2. User solves another issue worth 20 WXDAI. (at this point the rewards table has 2 entities)
  3. User opens pay.ubq.fi and generates a single permit worth 25 WXDAI (although the available amount is 30 WXDAI)

So we don't really need to match rewards with permits or gift cards. To calculate the available reward amount we can simply subtract the sum of generated permits and gift cards from solved rewards.

TLDR; I think as a part of this issue only the rewards table should be added.

@hhio618
Copy link

hhio618 commented Sep 3, 2024

So we don't really need to match rewards with permits or gift cards. To calculate the available reward amount we can simply subtract the sum of generated permits and gift cards from solved rewards.

@rndquu
This approach seems reasonable; however, it might slightly impact database integrity. What about a settlement table that only saves or updates the claimed amount from any payout method. This would also simplify the code.

@hhio618
Copy link

hhio618 commented Sep 3, 2024

@rndquu
Please confirm the remaining tasks that I will be completing:

  • Add the reward table to the permit-generation repository.
  • Generate the rolled-up payouts in the pay.ubq.fi repository.
  • Add a reward rollup module in this repository to save each record upon task completion.
  • Generate a matched permit for the reward amount whenever the permit module is enabled (upon task completion).

@rndquu
Copy link
Member Author

rndquu commented Sep 5, 2024

So we don't really need to match rewards with permits or gift cards. To calculate the available reward amount we can simply subtract the sum of generated permits and gift cards from solved rewards.

@rndquu This approach seems reasonable; however, it might slightly impact database integrity. What about a settlement table that only saves or updates the claimed amount from any payout method. This would also simplify the code.

Sorry for the inconvenience, we're still contemplating over the DB schema, pls check these comments:

If this issue's description is updated (in favor of a single DB table mentioned here) then we'll increase this github issue's reward to cover refactoring costs.

@rndquu
Copy link
Member Author

rndquu commented Sep 5, 2024

@rndquu Please confirm the remaining tasks that I will be completing:

  • Add the reward table to the permit-generation repository.
  • Generate the rolled-up payouts in the pay.ubq.fi repository.
  • Add a reward rollup module in this repository to save each record upon task completion.
  • Generate a matched permit for the reward amount whenever the permit module is enabled (upon task completion).

This github issue is solely regarding saving rewards in a DB so the following tasks should be solved as a part of this issue:

  • Add the reward table to the permit-generation repository
  • Add a reward rollup module in this repository to save each record upon task completion
  • Generate a matched permit for the reward amount whenever the permit module is enabled (upon task completion)

While this point is out of scope for this issue (since it will be solved here):

  • Generate the rolled-up payouts in the pay.ubq.fi repository

@hhio618
Copy link

hhio618 commented Sep 11, 2024

/start

Copy link
Contributor

ubiquity-os bot commented Sep 11, 2024

! hhio618 you were previously unassigned from this task. You cannot be reassigned.

@hhio618
Copy link

hhio618 commented Sep 11, 2024

@rndquu Any news on this issue and the related one?

@zugdev
Copy link

zugdev commented Nov 12, 2024

I have a plan to keep the granularity of single permits, which is interesting in terms of administration, while rolling up rewards.

  1. As indirectly stated in the above comments, we should only generate a permit once user claims it in pay.ubq.fi.

  2. We need to refactor the pay.ubq.fi UI to show user's available rewards and breakdown those rewards from the DB, and not from individual permits. This is apparently based in the reward's treasury address and token address, but hopefully we can attach it to organizations. On it's home screen a user can see all his claimable rewards, i.e 1000 UUSD in ubiquity, and 289 WXDAI in ubiquity-os. By clicking on ubiquity he can not only generate a permit of any amount ranging from 0 to 1000 UUSD but he can see a breakdown of the rewards that add up to that amount. We should make address based URL routes ("profile pages") so we can see each other's rewards, this would be specially interesting for the admin which would be able to easily cancel a reward directly from the DB instead of signing a tx to cancel the permit. I imagine this approach would also align well with the planned XP system or a leaderboard.

Based on 2, it would make a lot of sense for the database to include the issue's name. I would love to add an array stating what kind of contribution that reward refers to: writing a spec, reviewing or completing the task. There are a few more details I want to implement. I guess the downside of this approach is having to thrust a DB, but apparently other ends also do that.

@zugdev
Copy link

zugdev commented Nov 12, 2024

/start

@zugdev
Copy link

zugdev commented Nov 14, 2024

I've thoroughly analyzed this, and ultimately, I believe the table approach introduces way more risk than necessary. I propose we keep all related to payments on-chain, with adjustments to the current permit2 architecture. It is not common to permit directly from treasury, a standard solution to this problem is having a keeper contract. So my plan consists of the following steps:

  1. Write a keeper contract (and proper test suite)

We'll deploy a keeper for each possible treasury, which is the EOA that currently is the permit signer. The contracts will have whitelist protected functions for those funding EOAs. We have a public mapping of userToBalance (address -> uint256), public token address for the reward (we can make it configurable and change a little the previous mapping). We have a permit function which we will call at the end of an issue, this will increment a user's token balance. We have a cancelPermit function that the funder can also call at any time. The user can then claim any amount at any time and isn't limited by individual rewards. We keep an on-chain log of rewards, permits, cancellations and redemptions. We could push those logs as objects to on-chain storage, whether we are on Gnosis or any other L2 gas is extremely cheap.

  1. Refactor permit generation/signing to sign a keeper's reward call

  2. Refactor pay.ubq.fi

RFC @0x4007 @rndquu

@0x4007
Copy link
Member

0x4007 commented Nov 15, 2024

@UbiquityOS can you suggest any improvements to @zugdev's proposal?

Can you suggest alternative solutions?

@0x4007
Copy link
Member

0x4007 commented Nov 15, 2024

Seems like a bad approach to deploy a contract per EOA but I'm not very knowledgeable on the implementation details

Copy link

! An error occurred

@0x4007
Copy link
Member

0x4007 commented Nov 15, 2024

@sshivaditya @sshivaditya2019

@zugdev
Copy link

zugdev commented Nov 15, 2024

Seems like a bad approach to deploy a contract per EOA but I'm not very knowledgeable on the implementation details

We can have it configurable, we just add another variable to the mapping pot. That should not be a problem as we are in L2s.

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

4 participants