ubiquibot / conversation-rewards

0 stars 10 forks source link

Feat/permit fees #43

Closed rndquu closed 19 hours ago

rndquu commented 3 weeks ago

Depends on https://github.com/ubiquibot/conversation-rewards/pull/40

Resolves https://github.com/ubiquibot/permit-generation/issues/16

This PR introduces permit fees. When issue is closed as completed then additional permit will be generated for the treasury. Treasury must be a separate github account (i.e. registered with /wallet).

QA (no fees): https://github.com/rndquu-org/test-repo/issues/62#issuecomment-2222181924

QA (10% fee, i.e. PERMIT_FEE_RATE=10, PERMIT_TREASURY_GITHUB_USERNAME=rndquu3): https://github.com/rndquu-org/test-repo/issues/62#issuecomment-2222201252

2 permits (assignee reward + treasury fee) are generated in a DB:

Screenshot 2024-07-11 at 10 17 58
rndquu commented 1 week ago

@0x4007 This PR generates a separate fee permit for the treasury address (along with our traditional permits for solving an issue and comments).

Example with 10% fee:

  1. User1 gets 90 WXDAI for solving an issue
  2. User2 gets 9 WXDAI for comments
  3. Treasury gets 11 WXDAI fee

The thing with this approach is that treasury address must have a github account (i.e. registered with the /wallet command). This simplifies code greatly. Is this approach fine?

0x4007 commented 1 week ago

The thing with this approach is that treasury address must have a github account (i.e. registered with the /wallet command). This simplifies code greatly. Is this approach fine?

Oh I see. If it simplifies the code then yeah it probably is the best approach.

rndquu commented 5 days ago

@gentlementlegen Help

There are 5 failing tests in this CI run which are failing with the supabaseUrl is required error.

More logs:

$ jest --setupFiles dotenv/config --coverage
FAIL tests/parser/permit-generation-module.test.ts
  ● permit-generation-module.ts › applyFees() › Should not apply fees if PERMIT_FEE_RATE is empty

    supabaseUrl is required.

      [35](https://github.com/ubiquibot/conversation-rewards/actions/runs/9872254093/job/27262322258#step:4:36) | export class PermitGenerationModule implements Module {
      [36](https://github.com/ubiquibot/conversation-rewards/actions/runs/9872254093/job/27262322258#step:4:37) |   readonly _configuration: PermitGenerationConfiguration = configuration.incentives.permitGeneration;
    > [37](https://github.com/ubiquibot/conversation-rewards/actions/runs/9872254093/job/27262322258#step:4:38) |   readonly _supabase = createClient<Database>(process.env.SUPABASE_URL, process.env.SUPABASE_KEY);
         |                                    ^
      [38](https://github.com/ubiquibot/conversation-rewards/actions/runs/9872254093/job/27262322258#step:4:39) |
      39 |   async transform(data: Readonly<IssueActivity>, result: Result): Promise<Result> {
      [40](https://github.com/ubiquibot/conversation-rewards/actions/runs/9872254093/job/27262322258#step:4:41) |     const payload: Context["payload"] & Payload = {

Somehow process.env.SUPABASE_URL and process.env.SUPABASE_KEY are not set although they are passed in the jest workflow and also set in my fork:

Screenshot 2024-07-10 at 13 25 34

Perhaps you see what's wrong?

gentlementlegen commented 5 days ago

@rndquu When the Jest runs, since it's a fork, it doesn't have access to secrets: https://github.com/ubiquibot/conversation-rewards/actions/runs/9872254093/job/27262322258?pr=43#step:1:32 To fix this, two solutions:

rndquu commented 5 days ago

@rndquu When the Jest runs, since it's a fork, it doesn't have access to secrets: https://github.com/ubiquibot/conversation-rewards/actions/runs/9872254093/job/27262322258?pr=43#step:1:32 To fix this, two solutions:

  • mock supabase module entirely
  • give dummy values and intercept the requests with msw

As far as I understand it does have access to secrets but only to those set in the forked instance.

Ok, thank you, I'll try one of the proposed solutions.

rndquu commented 2 days ago

this looks good but I have another idea:

how about we add another variable fee (percentage) and totalAfterFee/totalBeforeFee and we leave other values unmodified? this way we can display a comment which includes information about the total reward, how much was the fee and total reward after the fee.

This would increase transparency so the user can clearly see how much the comments/task was worth and sees how much was the fee

You mean adding those properties to the result data structure, right? I'll add it in a separate PR.