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

Add a new column to the table that displays the fee per line item transparently #161

Open
gentlementlegen opened this issue Oct 16, 2024 · 17 comments

Comments

@gentlementlegen
Copy link
Member

gentlementlegen commented Oct 16, 2024

New QA: Meniole#23 (comment)

The failing tests are the ones involving the RPCs.

Thanks for the QA

As part of a separate task, I think we should consider adding a new column to the table that displays the fee per line item transparently

Originally posted by @0x4007 in #159 (comment)

@jaykayudo
Copy link
Contributor

/start

Copy link

Deadline Thu, Oct 17, 3:04 AM UTC
Beneficiary 0x830E2a4D7714989361BA76d3eA9C49e093f0A04C

Tip

  • 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.

This was referenced Oct 17, 2024
@jaykayudo
Copy link
Contributor

@0x4007 @gentlementlegen according to this code section, the reward is the amount gotten by multiplying the feeRate and the original. In that sense, if the feeRate is 20% (0.2) and the original reward is 100, then rewardAfterFee is 20. wouldn't that make the fee 80? is this how it is supposed to be?

@gentlementlegen
Copy link
Member Author

It should be that a 20% fee rate is applied to the total, so the final result should be total: 80 and feeRate: 20%. Did you see something different?

@jaykayudo
Copy link
Contributor

Yes. From the algorithm, if i am looking at it correctly, feeRate: 20% and total: 20. when you multiplied the feeRate to the total, you assigned the value directly to the total instead of substracting that value from the original total.

@jaykayudo
Copy link
Contributor

jaykayudo commented Oct 18, 2024

Yes. From the algorithm, if i am looking at it correctly, feeRate: 20% and total: 20. when you multiplied the feeRate to the total, you assigned the value directly to the total instead of substracting that value from the original total.

I think i got where my confusion is coming from. i think the value is wrong for the feeRate. I think the value should be

result[key].feeRate = new Decimal(1).minus(feeRateDecimal).toNumber();

or

result[key].feeRate =new Decimal(env.PERMIT_FEE_RATE).div(100).toNumber()

What do you think?

@gentlementlegen
Copy link
Member Author

I think the correct calculation is

result[key].feeRate = new Decimal(env.PERMIT_FEE_RATE).toNumber()

since the PERMIT_FEE_RATE directly represents what percentage was applied. But that's only affecting the percentage displayed, the result for the rewards is correct.

@jaykayudo
Copy link
Contributor

jaykayudo commented Oct 19, 2024

I think the correct calculation is

result[key].feeRate = new Decimal(env.PERMIT_FEE_RATE).toNumber()

since the PERMIT_FEE_RATE directly represents what percentage was applied. But that's only affecting the percentage displayed, the result for the rewards is correct.

Yes.. it only affects the displayed fees. Should i make the change within this issue PR since it depends on the correct feeRate or you’ve got it ?

@gentlementlegen
Copy link
Member Author

You can do it. Also we should wrap the <a/> with spaces because currently it doesn't have spacing around.
image

@jaykayudo
Copy link
Contributor

Okay

@jaykayudo
Copy link
Contributor

new Decimal(env.PERMIT_FEE_RATE).toNumber()

I just realized something. with this implementation, the feeRate will be 20 instead of 0.2. Is this how it should work from now on?

@gentlementlegen
Copy link
Member Author

It's a percentage so 20% makes sense to me.

@jaykayudo
Copy link
Contributor

It's a percentage so 20% makes sense to me.

That also means that this line of code will change as well. Yeah?

@jaykayudo
Copy link
Contributor

@gentlementlegen I have updated the PR. The feeRate is now in percentage.

Copy link

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

@0x4007
Copy link
Member

0x4007 commented Nov 2, 2024

I think we need to set our settings for disqualifier to be slower like 7 days for follow up.

This is because the new algorithm will divide by priority level.

7 days / 2 = 3 days 12 hours 
7 days / 3 = 2 days 8 hours 
7 days / 4 = 1 day 18 hours 
7 days / 5 = 1 day 9 hours 36 minutes 

@gentlementlegen
Copy link
Member Author

And also now we instantly remove if there is no PR opened which is why more people get kicked out.

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

Successfully merging a pull request may close this issue.

3 participants