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: modify referral fees #284

Merged
merged 19 commits into from
Jul 2, 2024
Merged

feat: modify referral fees #284

merged 19 commits into from
Jul 2, 2024

Conversation

jonathandiep
Copy link
Contributor

No description provided.

@jonathandiep jonathandiep requested a review from a team as a code owner June 18, 2024 16:12
@jonathandiep jonathandiep force-pushed the jon/modify-affiliate-fee branch 2 times, most recently from 1fc7d7a to 15d2227 Compare June 18, 2024 20:34
Copy link
Member

@Quazia Quazia left a comment

Choose a reason for hiding this comment

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

The code looks good and the switching of internal/external look right but the use of affiliate vs referral is sort of confusing so can you just state out the desired target for the changes and when it's affiliate vs referral?


// Setup default state
questFactoryContract = IQuestFactory(payable(msg.sender));
queued = true;
referralClaimTotal = 0;
totalReferralsFeesClaimed = 0;
referralRewardFee = 250; // 2.5%
Copy link
Member

Choose a reason for hiding this comment

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

Why hardcode vs pass it in?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We are going to hardcode it, but we could pass it in (and move hardcode to the QuestFactory instead)

test/Quest.t.sol Outdated Show resolved Hide resolved
test/Quest.t.sol Outdated Show resolved Hide resolved
test/helpers/TestUtils.sol Outdated Show resolved Hide resolved
Copy link
Member

@Quazia Quazia left a comment

Choose a reason for hiding this comment

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

LGTM - confirmed the withdrawal logic too just to be safe. Everything checks out.

@jonathandiep jonathandiep changed the title feat: modify affiliate/referral fees feat: modify referral fees Jun 22, 2024
@jonathandiep jonathandiep merged commit 0fdf2f5 into main Jul 2, 2024
2 checks passed
@jonathandiep jonathandiep deleted the jon/modify-affiliate-fee branch July 2, 2024 16:46
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.

2 participants