-
Notifications
You must be signed in to change notification settings - Fork 369
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
Implement sending of allocated validator payments #11197
Merged
martinvol
merged 25 commits into
feat/l2-epoch-system
from
m-chrzan/send-validator-payment
Sep 18, 2024
Merged
Changes from 20 commits
Commits
Show all changes
25 commits
Select commit
Hold shift + click to select a range
78cc1a6
Set up mocking for payment sending
m-chrzan cb6eded
Set up basic test for sending payment
m-chrzan 0813287
Send commission to group
m-chrzan a853665
Send payment to beneficiary
m-chrzan ee1eb46
Remove unnecessary test case
m-chrzan ff0c20b
Test sending to both group and beneficiary
m-chrzan 4d73b88
Test when not allocated
m-chrzan 54a1037
Cleanup test
m-chrzan 80873ba
Merge branch 'feat/l2-epoch-system' into m-chrzan/send-validator-payment
m-chrzan 195e718
Check ERC-20 transfer return values
m-chrzan e1beb8f
Emit an event on payment sent
m-chrzan 771bdfb
Add natspecs
m-chrzan 07839eb
Merge branch 'feat/l2-epoch-system' into m-chrzan/send-validator-payment
m-chrzan 8889bf3
Get pending payment based on signer
m-chrzan 78c5303
Use relative import
m-chrzan e0493cf
Move v8 interface to v8 directory
m-chrzan 1cb64bf
Merge branch 'feat/l2-epoch-system' into m-chrzan/send-validator-payment
m-chrzan 7da47c6
Correct mint to transfer
m-chrzan 624e348
Add delegation info to event
m-chrzan 2a29cd1
Don't transfer to validator if payment is 0
m-chrzan 3df09eb
Merge branch 'feat/l2-epoch-system' into m-chrzan/send-validator-payment
m-chrzan 1a50a3f
Rename variable
m-chrzan b47fd27
Merge branch 'feat/l2-epoch-system' into m-chrzan/send-validator-payment
martinvol 34d8949
Trigger CI
martinvol 85f9957
Merge branch 'feat/l2-epoch-system' into m-chrzan/send-validator-payment
martinvol File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -8,6 +8,7 @@ import "./interfaces/IOracle.sol"; | |||||
import "./interfaces/IStableToken.sol"; | ||||||
import "../common/UsingRegistry.sol"; | ||||||
|
||||||
import "../../contracts/common/FixidityLib.sol"; | ||||||
import "../../contracts/common/Initializable.sol"; | ||||||
import "../../contracts/common/interfaces/IEpochManager.sol"; | ||||||
import "../../contracts/common/interfaces/ICeloVersionedContract.sol"; | ||||||
|
@@ -19,6 +20,8 @@ contract EpochManager is | |||||
ReentrancyGuard, | ||||||
ICeloVersionedContract | ||||||
{ | ||||||
using FixidityLib for FixidityLib.Fraction; | ||||||
|
||||||
struct Epoch { | ||||||
uint256 firstBlock; | ||||||
uint256 lastBlock; | ||||||
|
@@ -73,6 +76,22 @@ contract EpochManager is | |||||
*/ | ||||||
event EpochProcessingEnded(uint256 indexed epochNumber); | ||||||
|
||||||
/** | ||||||
* @notice Emitted when an epoch payment is sent. | ||||||
* @param validator Address of the validator. | ||||||
* @param validatorPayment Amount of cUSD sent to the validator. | ||||||
* @param group Address of the validator's group. | ||||||
* @param groupPayment Amount of cUSD sent to the group. | ||||||
*/ | ||||||
event ValidatorEpochPaymentDistributed( | ||||||
address indexed validator, | ||||||
uint256 validatorPayment, | ||||||
address indexed group, | ||||||
uint256 groupPayment, | ||||||
address indexed beneficiary, | ||||||
uint256 delegatedPayment | ||||||
); | ||||||
|
||||||
modifier onlyEpochManagerInitializer() { | ||||||
require(msg.sender == epochManagerInitializer, "msg.sender is not Initializer"); | ||||||
_; | ||||||
|
@@ -319,4 +338,53 @@ contract EpochManager is | |||||
CELOequivalent | ||||||
); | ||||||
} | ||||||
|
||||||
/** | ||||||
* @notice Sends the allocated epoch payment to a validator, their group, and | ||||||
* delegation beneficiary. | ||||||
* @param validator Account of the validator. | ||||||
*/ | ||||||
function sendValidatorPayment(address validator) external { | ||||||
IAccounts accounts = IAccounts(getAccounts()); | ||||||
address signer = accounts.getValidatorSigner(validator); | ||||||
|
||||||
FixidityLib.Fraction memory totalPayment = FixidityLib.newFixed( | ||||||
validatorPendingPayments[signer] | ||||||
); | ||||||
|
||||||
IValidators validators = getValidators(); | ||||||
address group = validators.getValidatorsGroup(validator); | ||||||
(, uint256 commissionUnwrapped, , , , , ) = validators.getValidatorGroup(group); | ||||||
|
||||||
uint256 groupPayment = totalPayment.multiply(FixidityLib.wrap(commissionUnwrapped)).fromFixed(); | ||||||
FixidityLib.Fraction memory remainingPayment = FixidityLib.newFixed( | ||||||
totalPayment.fromFixed() - groupPayment | ||||||
martinvol marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
); | ||||||
(address beneficiary, uint256 fraction) = getAccounts().getPaymentDelegation(validator); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
So it is more clear what does it stand for ? |
||||||
uint256 delegatedPayment = remainingPayment.multiply(FixidityLib.wrap(fraction)).fromFixed(); | ||||||
uint256 validatorPayment = remainingPayment.fromFixed() - delegatedPayment; | ||||||
martinvol marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
|
||||||
IStableToken stableToken = IStableToken(getStableToken()); | ||||||
|
||||||
if (validatorPayment > 0) { | ||||||
require(stableToken.transfer(validator, validatorPayment), "transfer failed to validator"); | ||||||
} | ||||||
|
||||||
if (groupPayment > 0) { | ||||||
require(stableToken.transfer(group, groupPayment), "transfer failed to validator group"); | ||||||
} | ||||||
|
||||||
if (delegatedPayment > 0) { | ||||||
require(stableToken.transfer(beneficiary, delegatedPayment), "transfer failed to delegatee"); | ||||||
} | ||||||
|
||||||
emit ValidatorEpochPaymentDistributed( | ||||||
validator, | ||||||
validatorPayment, | ||||||
group, | ||||||
groupPayment, | ||||||
beneficiary, | ||||||
delegatedPayment | ||||||
); | ||||||
} | ||||||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
10 changes: 10 additions & 0 deletions
10
packages/protocol/contracts-0.8/common/mocks/EpochManager_WithMocks.sol
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
// SPDX-License-Identifier: UNLICENSED | ||
pragma solidity >=0.8.7 <0.8.20; | ||
|
||
import "../EpochManager.sol"; | ||
|
||
contract EpochManager_WithMocks is EpochManager(true) { | ||
function _setPaymentAllocation(address validator, uint256 amount) external { | ||
validatorPendingPayments[validator] = amount; | ||
} | ||
} |
43 changes: 43 additions & 0 deletions
43
packages/protocol/contracts-0.8/common/mocks/MockAccounts.sol
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,43 @@ | ||
// SPDX-License-Identifier: UNLICENSED | ||
pragma solidity >=0.8.7 <0.8.20; | ||
|
||
import "../../../contracts/common/FixidityLib.sol"; | ||
|
||
contract MockAccounts { | ||
using FixidityLib for FixidityLib.Fraction; | ||
|
||
struct PaymentDelegation { | ||
// Address that should receive a fraction of validator payments. | ||
address beneficiary; | ||
// Fraction of payment to delegate to `beneficiary`. | ||
FixidityLib.Fraction fraction; | ||
} | ||
|
||
mapping(address => PaymentDelegation) delegations; | ||
mapping(address => address) accountToSigner; | ||
|
||
function setPaymentDelegationFor( | ||
address validator, | ||
address beneficiary, | ||
uint256 fraction | ||
) public { | ||
delegations[validator] = PaymentDelegation(beneficiary, FixidityLib.wrap(fraction)); | ||
} | ||
|
||
function deletePaymentDelegationFor(address validator) public { | ||
delete delegations[validator]; | ||
} | ||
|
||
function getPaymentDelegation(address account) external view returns (address, uint256) { | ||
PaymentDelegation storage delegation = delegations[account]; | ||
return (delegation.beneficiary, delegation.fraction.unwrap()); | ||
} | ||
|
||
function setValidatorSigner(address account, address signer) external { | ||
accountToSigner[account] = signer; | ||
} | ||
|
||
function getValidatorSigner(address account) external returns (address) { | ||
return accountToSigner[account]; | ||
} | ||
} |
61 changes: 61 additions & 0 deletions
61
packages/protocol/contracts-0.8/governance/test/IMockValidators.sol
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,61 @@ | ||
pragma solidity >=0.8.7 <0.8.20; | ||
|
||
interface IMockValidators { | ||
function isValidator(address) external returns (bool); | ||
function isValidatorGroup(address) external returns (bool); | ||
|
||
function updateEcdsaPublicKey(address, address, bytes calldata) external returns (bool); | ||
|
||
function updatePublicKeys( | ||
address, | ||
address, | ||
bytes calldata, | ||
bytes calldata, | ||
bytes calldata | ||
) external returns (bool); | ||
|
||
function setValidator(address) external; | ||
|
||
function setValidatorGroup(address group) external; | ||
|
||
function affiliate(address group) external returns (bool); | ||
|
||
function setDoesNotMeetAccountLockedGoldRequirements(address account) external; | ||
|
||
function setNumRegisteredValidators(uint256 value) external; | ||
|
||
function setMembers(address group, address[] calldata _members) external; | ||
|
||
function setCommission(address group, uint256 commission) external; | ||
|
||
function setAccountLockedGoldRequirement(address account, uint256 value) external; | ||
|
||
function halveSlashingMultiplier(address) external; | ||
|
||
function forceDeaffiliateIfValidator(address validator) external; | ||
|
||
function getTopGroupValidators(address group, uint256 n) external view returns (address[] memory); | ||
|
||
function getValidatorGroup( | ||
address | ||
) | ||
external | ||
view | ||
returns (address[] memory, uint256, uint256, uint256, uint256[] memory, uint256, uint256); | ||
|
||
function getValidatorGroupSlashingMultiplier(address) external view returns (uint256); | ||
|
||
function meetsAccountLockedGoldRequirements(address account) external view returns (bool); | ||
|
||
function getNumRegisteredValidators() external view returns (uint256); | ||
|
||
function getAccountLockedGoldRequirement(address account) external view returns (uint256); | ||
|
||
function calculateGroupEpochScore(uint256[] calldata uptimes) external view returns (uint256); | ||
|
||
function getGroupsNumMembers(address[] calldata groups) external view returns (uint256[] memory); | ||
|
||
function groupMembershipInEpoch(address addr, uint256, uint256) external view returns (address); | ||
|
||
function getGroupNumMembers(address group) external view returns (uint256); | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe we didn't want to use signers @mcortesi @martinvol
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was basing this on how the current code works:
elected
array in EpochManager gets set by callingElection.electNValidatorSigners
Validators.getTopGroupValidators
So in the current state, the correct way to index into
elected
is via signer addresses.If we want to change this logic, we should thoroughly double check everything is returning/using the correct type of validator address. As mentioned on Slack, a similar issue exists with the current use of
Validators.computeEpochReward
, which expects a validator account, but gets a signer address (generated by the election). I would suggest this be done in a follow-up issue/PR that covers all instances of this.In general I agree, we can probably just use account addresses everywhere. If I understand correctly, the only reason we used signer addresses in some places was because that's what celo-blockchain cared about in the PoS logic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, we don't want signeers. EpochManager will have a list of elected account (not signers) and we should change elections to not give signers, i believe we even have a draft methos doing that...
the client didn't care about account, only about signers and so it sent signers everywhere, but that's unnecessary complexity now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Created an issue to track progress on this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mcortesi, sounds great, that's how I understood things.
As mentioned, I would merge this PR as is, so it's consistent with the current state of the logic, and remove signers in the PR that will fix @soloseng's issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR making the changes.