feat(incentives): add protocol fee#1691
feat(incentives): add protocol fee#16910xClandestine merged 5 commits intorelease-dev/incentive-councilfrom
Conversation
0xClandestine
changed the title
0xClandestine
force-pushed
the
feat/rewards-protocol-fee
branch
from
8933146 to
6eac771
Compare
|
ci failing from flakey unrelated test, disregard during review plz |
0xClandestine
force-pushed
the
feat/rewards-protocol-fee
branch
from
15dd5f7 to
a1f9bd8
Compare
0xClandestine
force-pushed
the
feat/rewards-protocol-fee
branch
from
a1f9bd8 to
d64c649
Compare
- used intra org no need for fees
| uint32 _activationDelay, | ||
| uint16 _defaultSplitBips | ||
| uint16 _defaultSplitBips, | ||
| address _feeRecipient |
There was a problem hiding this comment.
we need to use reinitializer here .. since we plan to call initialize when upgrade to set _feeRecipient .. or we plan to call the function setFeeRecipient after upgrade instead ??
There was a problem hiding this comment.
You're right, should use reinitializer.
There was a problem hiding this comment.
Another option: Don't add a re-initializer and just call the function as part of the atomic batch tx of the upgrade.
There was a problem hiding this comment.
Would then need to re-add non-zero address checks. Keeping as-is for now.
|
|
||
| // 1. Set fee recipient to address(0) | ||
| cheats.prank(rewardsCoordinator.owner()); | ||
| rewardsCoordinator.setFeeRecipient(address(0)); |
There was a problem hiding this comment.
this test is failing here .. because you can't set a 0 address as feeRecipient
also the logic of that test seems wrong .. since feeRecipient should never be 0 ?
| 0, // activationDelay | ||
| 0 // defaultSplitBips | ||
| 0, // defaultSplitBips | ||
| address(0) // feeRecipient |
There was a problem hiding this comment.
should add the feeRecipient param to both deployment scripts as well (devnet and local) .. that's why the deployFromScratch test is failing
| _validateRewardsSubmission(rewardsSubmission); | ||
|
|
||
| // Then transfer the full amount to the contract. | ||
| rewardsSubmission.token.safeTransferFrom(msg.sender, address(this), rewardsSubmission.amount); |
There was a problem hiding this comment.
Thoughts on moving all of this to a helper function and we just return the new rewardSubmission, nonce, and hash? Mainly for deduplication
There was a problem hiding this comment.
If we do so, I'd want to cleanup the other methods as well. Right now they're implemented for readability since size isn't of massive concern.
- adding fee amount doesn't make sense, this isn't RC
0xClandestine
merged commit b7a78d0
into
release-dev/incentive-council
**Motivation:** We want to add a configurable protocol fee mechanism to the RewardsCoordinator contract to enable revenue generation. This allows reward submitters to opt into paying fees while maintaining backward compatibility with existing integrations. **Modifications:** - Added protocolFee (fixed constant amount) and feeRecipient state variables - Implemented setOptInProtocolFee() for submitters to opt into fee payments - Implemented setFeeRecipient() (owner-only) to set the fee destination address **Result:** Submitters can voluntarily enable protocol fees on their reward distributions. When opted in, the fixed fee amount is deducted and sent to the designated recipient. Existing integrations continue working unchanged since fees are disabled by default.
3 participants
Motivation:
We want to add a configurable protocol fee mechanism to the RewardsCoordinator contract to enable revenue generation. This allows reward submitters to opt into paying fees while maintaining backward compatibility with existing integrations.
Modifications:
Result:
Submitters can voluntarily enable protocol fees on their reward distributions. When opted in, the fixed fee amount is deducted and sent to the designated recipient. Existing integrations continue working unchanged since fees are disabled by default.