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.

Sign up for GitHub

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

ETH-786: fix Delegated event #901

Closed
wants to merge 1 commit into from
Closed

ETH-786: fix Delegated event #901

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions packages/network-contracts/contracts/OperatorTokenomics/Operator.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -296,13 +296,16 @@ contract Operator is Initializable, ERC2771ContextUpgradeable, IERC677Receiver,
}

_delegate(delegator, amount);
emit Delegated(delegator, amount);
payOutQueue(0);
}

/** 2-step delegation: first call DATA.approve(operatorContract.address, amountWei) then this function */
function delegate(uint amountWei) external {
token.transferFrom(_msgSender(), address(this), amountWei);
_delegate(_msgSender(), amountWei);
address delegator = _msgSender();
token.transferFrom(delegator, address(this), amountWei);
_delegate(delegator, amountWei);
emit Delegated(delegator, amountWei);
payOutQueue(0);
}

Expand Down Expand Up @@ -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);
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

emit BalanceUpdate(delegator, balanceOf(delegator), totalSupply(), valueWithoutEarnings());
emit OperatorValueUpdate(totalStakedIntoSponsorshipsWei - totalSlashedInSponsorshipsWei, token.balanceOf(address(this)));
}
Expand Down
2 changes: 1 addition & 1 deletion packages/network-contracts/contracts/OperatorTokenomics/OperatorPolicies/StakeModule.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Suggested change
// Self-delegate the operator's cut === mint new operator tokens
Mint new operator tokens

// because _delegate is assumed to be called AFTER the DATA token transfer, the result of calling it is equivalent to:
// 1) send operator's cut in DATA tokens to the operator (removed from DATA balance, NO burning of tokens)
// 2) the operator delegates them back to the contract (added back to DATA balance, minting new tokens)
Expand Down
4 changes: 2 additions & 2 deletions packages/network-subgraphs/schema.graphql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Copy link
Contributor

Choose a reason for hiding this comment

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

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?)

totalDelegatedWei: BigInt!

"Sum of DATA tokens undelegated from this operator today, by all delegators. Updated when Undelegated event is fired"
"DATA value of sent/burned operator tokens, by all delegators. Updated when BalanceUpdate event is fired"
totalUndelegatedWei: BigInt!

"Sum of earnings today, less operator's share"
Expand Down
Loading