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

feat: free fees for thor holders before tip 014 #8044

Merged
merged 16 commits into from
Oct 31, 2024
Merged

Conversation

NeOMakinG
Copy link
Collaborator

Description

This is implementing free fees for thor holders before TIP 014 proposal

I don't have enough funds to do a full swap to test if fees are effectively removed, but assuming setting fees to 0 inside calculateFees effectively change it

I didn't make the votePower slide systemic on purpose as this thing is supposed to stay for a few month and the lift would be a bit more risky, happy to discuss it in a follow up if it bring value

Issue (if applicable)

closes #8019

Risk

Medium, fee calculation could lead user wrongly

High Risk PRs Require 2 approvals

What protocols, transaction types, wallets or contract interactions might be affected by this PR?

Testing

  • Check regular trade fees without an account holding THOR, under 1000 and over 1000
  • Check trade using 0xC85feF7A1b039A9e080aadf80FF6f1536DADa088 as a spoofed address, to see that trade fees are free because of THOR holding before the proposal
  • You could also verify that new Date().getFullYear() <= 2025 is true and new Date().getFullYear() <= 2024 is false to verify if it will be disabled programmatically
  • Also verify the mixpanel event is sent on trade (if you have enough funds)

Engineering

n/a

Operations

n/a

  • 🏁 My feature is behind a flag and doesn't require operations testing (yet)

Screenshots (if applicable)

Normal account not holding thor > 1000$ trade

image

Normal account < 1000$ trade

image

thor holding account > 1000$ trade

image

thor holding account < 1000$ trade

image

Banner

image

@NeOMakinG NeOMakinG requested a review from a team as a code owner October 30, 2024 17:30
src/lib/fees/model.ts Outdated Show resolved Hide resolved
@0xean
Copy link
Contributor

0xean commented Oct 30, 2024

Screenshot 2024-10-30 at 1 48 18 PM

Copy here is unfortunately wrong. Trades under 1k are already free

Should say "Thor holders trade for free through 2024"

@NeOMakinG
Copy link
Collaborator Author

Screenshot 2024-10-30 at 1 48 18 PM Copy here is unfortunately wrong. Trades under 1k are already free

Should say "Thor holders trade for free through 2024"

Updated in 8ea8869

image

Also removed the link so we don't block the release with the missing link

Copy link
Member

@woodenfurniture woodenfurniture left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking very nearly good to go, just a few comments and q's before we get this in!

src/lib/fees/model.ts Outdated Show resolved Hide resolved
src/state/apis/snapshot/snapshot.ts Show resolved Hide resolved
src/state/apis/snapshot/snapshot.ts Outdated Show resolved Hide resolved
src/state/apis/snapshot/snapshot.ts Outdated Show resolved Hide resolved
src/state/apis/snapshot/snapshot.ts Outdated Show resolved Hide resolved
src/state/apis/snapshot/snapshot.ts Show resolved Hide resolved
src/lib/fees/model.ts Outdated Show resolved Hide resolved
@kaladinlight
Copy link
Contributor

making a note for posterity that we are currently displaying the thor discount as a fox power discount. easy mode just disable the modal for thor free trades so we don't have to mess with more UI that will only be there for 2 months.
image

@kaladinlight
Copy link
Contributor

Also noticing what looks like an outstanding bug and potential improvement related to getVotingPower.

Bug: switch wallet does not correctly fetch the new/appropriate voting power for the new wallet. i started with my native wallet and then switched to the frame wallet with test address. after switching the wallet, the discount being applied is still for my native wallet.

Improvement: you can start interacting with the trade widget to get quotes immediately and get a quote with fees displayed before the snapshot logic completes. once the snapshot voting power completes, it triggers a refresh of the trade input component and looks weird. seems like we could add a loading state check to keep the quotes displayed, but the fee breakdown loading until voting power fetch is completed and then show (no refresh of state).

@NeOMakinG
Copy link
Collaborator Author

Also noticing what looks like an outstanding bug and potential improvement related to getVotingPower.

Bug: switch wallet does not correctly fetch the new/appropriate voting power for the new wallet. i started with my native wallet and then switched to the frame wallet with test address. after switching the wallet, the discount being applied is still for my native wallet.

Improvement: you can start interacting with the trade widget to get quotes immediately and get a quote with fees displayed before the snapshot logic completes. once the snapshot voting power completes, it triggers a refresh of the trade input component and looks weird. seems like we could add a loading state check to keep the quotes displayed, but the fee breakdown loading until voting power fetch is completed and then show (no refresh of state).

Also noticing what looks like an outstanding bug and potential improvement related to getVotingPower.

Bug: switch wallet does not correctly fetch the new/appropriate voting power for the new wallet. i started with my native wallet and then switched to the frame wallet with test address. after switching the wallet, the discount being applied is still for my native wallet.

Improvement: you can start interacting with the trade widget to get quotes immediately and get a quote with fees displayed before the snapshot logic completes. once the snapshot voting power completes, it triggers a refresh of the trade input component and looks weird. seems like we could add a loading state check to keep the quotes displayed, but the fee breakdown loading until voting power fetch is completed and then show (no refresh of state).

Wallet switch issue: #8058

Loading improvement issue: #8059

@kaladinlight kaladinlight merged commit 9a46362 into develop Oct 31, 2024
3 checks passed
@kaladinlight kaladinlight deleted the thor-fees branch October 31, 2024 18:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Free trades for thor holders.
5 participants