ETH-786: fix Delegated event #901
Conversation
TODO: check tests
ETH-768: fix Delegated event
🚨 Report Summary
For more details view the full report in OpenZeppelin Code Inspector |
| @@ -331,7 +334,6 @@ contract Operator is Initializable, ERC2771ContextUpgradeable, IERC677Receiver, | |||
| } | |||
|
|
|||
| latestDelegationTimestamp[delegator] = block.timestamp; // solhint-disable-line not-rely-on-time | |||
| emit Delegated(delegator, amountDataWei); | |||
There was a problem hiding this comment.
Could rename the _delegate function as our definition of delegation is now stricter.
The linear ticket defines:
"Delegated should mean DATA was moved into Operator contract via delegate or transferAndCall, not the other self-delegation hacks (fisherman, operator's cut)"
If I understood correctly, this also defines what delegation means as a concept, not just when we emit the event? As we should of course emit the Delegated event always when a delegation happens, and never when it doesn't happen.
| @@ -127,7 +127,7 @@ contract StakeModule is IStakeModule, Operator { | |||
| uint protocolFee = earningsDataWei * streamrConfig.protocolFeeFraction() / 1 ether; | |||
| token.transfer(streamrConfig.protocolFeeBeneficiary(), protocolFee); | |||
|
|
|||
| // "self-delegate" the operator's share === mint new operator tokens | |||
| // Self-delegate the operator's cut === mint new operator tokens | |||
There was a problem hiding this comment.
Could avoid the "self-delegate" term, or at least keep that in quotes.
As we have now defined that self-delegation is not a delegation at all, it is very misleading term. If we want to have a totally separate term, that could be e.g. "invest", or "fund"? But "self-delegation" in quotes is maybe ok for now, too.
| // Self-delegate the operator's cut === mint new operator tokens | |
| Mint new operator tokens |
| @@ -372,10 +372,10 @@ type OperatorDailyBucket @entity { | |||
| "Delegators that joined today. Updated when Delegation entity is created" | |||
| delegatorCountChange: Int! | |||
|
|
|||
| "Sum of DATA tokens delegated to this operator today, by all delegators. Updated when Delegated event is fired" | |||
| "DATA value of received operator tokens, by all delegators. Updated when BalanceUpdate event is fired" | |||
There was a problem hiding this comment.
Does this include only real delegations, or some of the "hack-delegations? If it includes only the real delegations, should it be updated only by the Delegated/Undelegated events?
(And if it need to include also hack-delegations, should we deprecate this field and have a separate field for the real delegations?)
|
This pull request is stale because it has been open 60 days with no activity. Remove no-pr-activity label or comment or this will be closed in 7 days |
|
This pull request is closed due to inactivity. |
No description provided.