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 · 53 comments
Open

Reward amount rollup #64

rndquu opened this issue Jul 19, 2024 · 53 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.

Copy link

Passed the deadline and no activity is detected, removing assignees: @zugdev.

@0x4007
Copy link
Member

0x4007 commented Nov 28, 2024

I also really want to remove Supabase/database dependencies from everywhere in our system. It adds a ton of friction for DAO contributors to work on our infrastructure. I think that smart contracts as databases for finances probably makes sense, we just need to be careful about gas fees. Otherwise I prefer git based JSON storage, perhaps with encryption, because it is likely to be the easiest to share the data and set up for maintenance + onboarding new developers.

@zugdev
Copy link

zugdev commented Nov 29, 2024

Gas is excusable in L2s, my main concern with a contract solution was the need to handle multiple chains. We wanted to abstract chains as much as possible in pay.ubq.fi but ultimately we can't. Allowing partners to setup payments in any chains implies this complication, which I don't think the JSON/DB storage would fix 100%, so therefore the downside of contracts is not a complete tradeoff. The JSON idea with the PK is much safer and transparent than DB, and perhaps the easiest reach. It's best to avoid managing contracts in a lot of chains and OFC auditing. I am planning to go with the JSON git storage, while saving the JSON files to Supabase for backup, though GitHub is reliable I am not a big fan of lonely entry points. I'll venture this way, then. Agreed? @0x4007 @rndquu

@0x4007
Copy link
Member

0x4007 commented Nov 29, 2024

while saving the JSON files to Supabase for backup, though GitHub is reliable I am not a big fan of lonely entry points.

It's an interesting idea for enhancing reliability. Perhaps it's more worthwhile after we see problems with git based storage.

@sshivaditya2019
Copy link

Otherwise I prefer git based JSON storage, perhaps with encryption, because it is likely to be the easiest to share the data and set up for maintenance + onboarding new developers.

We could use GitRows, but since it's no longer maintained, we'd need to fork it. It supports CRUD operations on repo data, works with private repos, and lets users log in with their PAT.

@zugdev
Copy link

zugdev commented Dec 1, 2024

@gentlementlegen

Thinking this through, and your feedback is important. From what I understand, currently we use this repo's compute.yml workflow and permit-generation to write the individual permits. My plan:

  1. Setup reward repo
    This repo will consist of JSON files, one for each rewarded user. I think normalized is the best way to go about this, because it gets really easy to pull data in UI (single JSON pull) and querying is O(1):

0xmyaddress.json:

{
  "rewards": [
    {
      "treasury": "0xTREASURY1",
      "chain_id": "1",
      "token": "0xTOKEN1",
      "user": "0xUSER1",
      "reward_number": "1",
      "status": "pending",
      "amount": "100",
      "timestamp": "2022-01-01T00:00:00Z"
    },
    {
      "treasury": "0xTREASURY2",
      "chain_id": "100",
      "token": "0xTOKEN2",
      "user": "0xUSER1",
      "reward_number": "1",
      "status": "canceled",
      "amount": "100",
      "timestamp": "2022-01-01T00:00:00Z"
    },  ]
}

My idea is that only a GitHub app can change this repo's content using the EVM private key. Scripts would be placed outside of this repo for maintainance, the rewards repo would contain workflows that run the scripts from the other repo.

  1. Add reward and cancel reward scripts in another repo:

The scripts would then be called using workflows in the rewards repo, the idea is that we protect any possibility of manually changing rewards.

@gentlementlegen
Copy link
Member

That can work but then if you want to test locally or for external users, how do you set it up?

@zugdev
Copy link

zugdev commented Dec 4, 2024

That can work but then if you want to test locally or for external users, how do you set it up?

Not that simple, I guess. We will need content perm to add/edit the workflows either way. I think having repo admin with 2FA is good enough. We just must ensure the jsons are not edited by someone other than the app, perhaps there is a way to isolate it.

@0x4007 0x4007 added good first issue Good for newcomers and removed good first issue Good for newcomers labels Dec 4, 2024
Copy link

Note

The following contributors may be suitable for this task:

gentlementlegen

2% Match ubiquity-os-marketplace/daemon-pricing#47

@whilefoo
Copy link
Contributor

whilefoo commented Dec 4, 2024

  1. Setup reward repo
    This repo will consist of JSON files, one for each rewarded user. I think normalized is the best way to go about this, because it gets really easy to pull data in UI (single JSON pull) and querying is O(1):

0xmyaddress.json:

{
  "rewards": [
    {
      "treasury": "0xTREASURY1",
      "chain_id": "1",
      "token": "0xTOKEN1",
      "user": "0xUSER1",
      "reward_number": "1",
      "status": "pending",
      "amount": "100",
      "timestamp": "2022-01-01T00:00:00Z"
    },
    {
      "treasury": "0xTREASURY2",
      "chain_id": "100",
      "token": "0xTOKEN2",
      "user": "0xUSER1",
      "reward_number": "1",
      "status": "canceled",
      "amount": "100",
      "timestamp": "2022-01-01T00:00:00Z"
    },  ]
}

Where do you plan to store permits/gift cards/other types of payouts? I think the consensus was that we will use one table approach (ledger) to keep things simple and more easier to track.

I think status property should be reconsidered. Cancelling a permit that was already claimed won't work this way.
I think in most cases the reward would be cancelled because the task was not properly finished or accidentally closed as finished, so instead of having a status property on each reward, we can create a new entry with type penalty which would have a reward_id and reason associated with it.
This way the user's balance can go negative and when they complete another task this penalty will be automatically deducted.


I think that rewards should be based on the Github user not the wallet address because wallets change and users stay and also it's easier to query a user than a wallet which can change multiple times, for example if you query a user on you would need to first check all wallets and then all rewards associated with those wallets, instead you can query the user directly.

In my head the ideal flow would be that I go to the reward page, I login with Github, I see all my rewards related to tasks, reviews, conversation rewards...along with the breakdown of rewards, already claimed permits, penalties...and then:

  1. option: I connect with my wallet like MetaMask and sign a message to confirm I own this wallet, then the backend generates a permit with the amount and the recipient address based on the signed message.
  2. option: I select a gift card

I would say that it's better to first implement reward rollup with a traditional database. Later after we will implement Git based storage and test it on small plugins and make sure it's working correctly and without bugs, we can easily switch.

  1. This plugin deals with financial data (money) so until our Git based storage is mature and battle-tested it would be unwise to use it for this purpose
  2. We can develop Git based storage and reward rollups in parallel

@rndquu
Copy link
Member Author

rndquu commented Dec 4, 2024

To sum up, moving from permit2 to "smart contracts for finances" is a huge pivot which touches too many repositories to achieve it in a reasonable amount of time. We could think more on it after the "reward rollup" feature is ready.

@zugdev I suppose you have 2 ways:

  1. Use supabase DB
  2. Go the json DB way:
    a) Start with Add "json as a DB" methods ubiquity-os/plugin-sdk#30
    b) Propose a solution how permits can be safely stored in github json files (sign the json file with the kernel private key or check commit history for unauthorized commits in the json DB?)

@whilefoo
Copy link
Contributor

whilefoo commented Dec 5, 2024

Another thing to consider is partner's token / chain ID.
Partner's token address, chain ID or even the private key can change over time, this means that rewards need to store at least token address and chain ID but we can't store the private key so we have to warn partners to not change the private key or if they do, they need to make sure it's still compatible with all previous tokens and chains.
At the payout we have group rewards by organization and also by token/chain and the user has to claim each one individually.

The ideal flow would be that the user could choose the chain and/or the token however I feel like at this stage it's not feasible...maybe we can have revisit this in the future

@0x4007
Copy link
Member

0x4007 commented Dec 12, 2024

Maybe a fast middle ground but is it possible to multicall permit claims @rndquu?

We can abstract this away from the user on a special permit roll up claim UI. It can let a user import several permits somehow (possibly with a copy paste text box) later there may be a way to automate this.

The UI can check the sum total and just display a big claim button under that sum total.

Ideally at this step the user signs one transaction and claims all of them in one shot.

It's not the optimal UI/UX but if multicall is possible this could step us closer in the right direction.

@rndquu
Copy link
Member Author

rndquu commented Dec 12, 2024

Maybe a fast middle ground but is it possible to multicall permit claims @rndquu?

We can abstract this away from the user on a special permit roll up claim UI. It can let a user import several permits somehow (possibly with a copy paste text box) later there may be a way to automate this.

The UI can check the sum total and just display a big claim button under that sum total.

Ideally at this step the user signs one transaction and claims all of them in one shot.

It's not the optimal UI/UX but if multicall is possible this could step us closer in the right direction.

Uniswap's permit2 doesn't work with multicall

@rndquu
Copy link
Member Author

rndquu commented Dec 12, 2024

/start

Copy link

! This task does not reflect a business priority at the moment. You may start tasks with one of the following labels: Priority: 3 (High), Priority: 4 (Urgent), Priority: 5 (Emergency)

@rndquu rndquu self-assigned this Dec 12, 2024
Copy link

@rndquu the deadline is at Fri, Dec 13, 12:22 PM UTC

Copy link

Important

  • Be sure to link a pull-request before the first reminder to avoid disqualification.
  • Reminders will be sent every 3 days and 12 hours if there is no activity.
  • Assignees will be disqualified after 7 days of inactivity.

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