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

draft (tests fail) of moving transfer events to _transferShares #421

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
27 changes: 5 additions & 22 deletions contracts/0.4.24/Lido.sol
Original file line number Diff line number Diff line change
Expand Up @@ -695,18 +695,9 @@ contract Lido is ILido, StETH, AragonApp {
BUFFERED_ETHER_POSITION.setStorageUint256(_getBufferedEther().add(msg.value));
emit Submitted(msg.sender, msg.value, _referral);

_emitTransferAfterMintingShares(msg.sender, sharesAmount);
return sharesAmount;
}

/**
* @dev Emits {Transfer} and {TransferShares} events where `from` is 0 address. Indicates mint events.
*/
function _emitTransferAfterMintingShares(address _to, uint256 _sharesAmount) internal {
emit Transfer(address(0), _to, getPooledEthByShares(_sharesAmount));
emit TransferShares(address(0), _to, _sharesAmount);
}

/**
* @dev Deposits buffered eth to the DepositContract and assigns chunked deposits to node operators
*/
Expand Down Expand Up @@ -828,16 +819,14 @@ contract Lido is ILido, StETH, AragonApp {
)
);

// Mint the calculated amount of shares to this contract address. This will reduce the
// balances of the holders, as if the fee was taken in parts from each of them.
_mintShares(address(this), shares2mint);
// Part by part mint the calculated amount of shares to the recipients. This will reduce
// the balances of the holders, as if the fee was taken in parts from each of them.

(,uint16 insuranceFeeBasisPoints, uint16 operatorsFeeBasisPoints) = getFeeDistribution();

uint256 toInsuranceFund = shares2mint.mul(insuranceFeeBasisPoints).div(TOTAL_BASIS_POINTS);
address insuranceFund = getInsuranceFund();
_transferShares(address(this), insuranceFund, toInsuranceFund);
_emitTransferAfterMintingShares(insuranceFund, toInsuranceFund);
_mintShares(insuranceFund, toInsuranceFund);

uint256 distributedToOperatorsShares = _distributeNodeOperatorsReward(
shares2mint.mul(operatorsFeeBasisPoints).div(TOTAL_BASIS_POINTS)
Expand All @@ -847,8 +836,7 @@ contract Lido is ILido, StETH, AragonApp {
uint256 toTreasury = shares2mint.sub(toInsuranceFund).sub(distributedToOperatorsShares);

address treasury = getTreasury();
_transferShares(address(this), treasury, toTreasury);
_emitTransferAfterMintingShares(treasury, toTreasury);
_mintShares(treasury, toTreasury);
}

/**
Expand All @@ -863,12 +851,7 @@ contract Lido is ILido, StETH, AragonApp {

distributed = 0;
for (uint256 idx = 0; idx < recipients.length; ++idx) {
_transferShares(
address(this),
recipients[idx],
shares[idx]
);
_emitTransferAfterMintingShares(recipients[idx], shares[idx]);
_mintShares(recipients[idx], shares[idx]);
distributed = distributed.add(shares[idx]);
}
}
Expand Down
26 changes: 16 additions & 10 deletions contracts/0.4.24/StETH.sol
Original file line number Diff line number Diff line change
Expand Up @@ -338,12 +338,8 @@ contract StETH is IERC20, Pausable {
*
* @dev The `_sharesAmount` argument is the amount of shares, not tokens.
*/
function transferShares(address _recipient, uint256 _sharesAmount) public returns (uint256) {
_transferShares(msg.sender, _recipient, _sharesAmount);
emit TransferShares(msg.sender, _recipient, _sharesAmount);
uint256 tokensAmount = getPooledEthByShares(_sharesAmount);
emit Transfer(msg.sender, _recipient, tokensAmount);
return tokensAmount;
function transferShares(address _recipient, uint256 _sharesAmount) public returns (uint256 tokensAmount) {
return _transferShares(msg.sender, _recipient, _sharesAmount);
}

/**
Expand All @@ -360,9 +356,10 @@ contract StETH is IERC20, Pausable {
*/
function _transfer(address _sender, address _recipient, uint256 _amount) internal {
uint256 _sharesToTransfer = getSharesByPooledEth(_amount);
_transferShares(_sender, _recipient, _sharesToTransfer);
emit Transfer(_sender, _recipient, _amount);
emit TransferShares(_sender, _recipient, _sharesToTransfer);
uint256 tokenAmount = _transferShares(_sender, _recipient, _sharesToTransfer);

// TODO: this assertion is broken
// assert(tokenAmount == _amount);
}

/**
Expand Down Expand Up @@ -408,7 +405,7 @@ contract StETH is IERC20, Pausable {
* - `_sender` must hold at least `_sharesAmount` shares.
* - the contract must not be paused.
*/
function _transferShares(address _sender, address _recipient, uint256 _sharesAmount) internal whenNotStopped {
function _transferShares(address _sender, address _recipient, uint256 _sharesAmount) internal whenNotStopped returns (uint256 tokensAmount) {
require(_sender != address(0), "TRANSFER_FROM_THE_ZERO_ADDRESS");
require(_recipient != address(0), "TRANSFER_TO_THE_ZERO_ADDRESS");

Expand All @@ -417,6 +414,10 @@ contract StETH is IERC20, Pausable {

shares[_sender] = currentSenderShares.sub(_sharesAmount);
shares[_recipient] = shares[_recipient].add(_sharesAmount);

tokensAmount = getPooledEthByShares(_sharesAmount);
emit Transfer(_sender, _recipient, tokensAmount);
emit TransferShares(_sender, _recipient, _sharesAmount);
}

/**
Expand All @@ -431,11 +432,16 @@ contract StETH is IERC20, Pausable {
function _mintShares(address _recipient, uint256 _sharesAmount) internal whenNotStopped returns (uint256 newTotalShares) {
require(_recipient != address(0), "MINT_TO_THE_ZERO_ADDRESS");

// TODO: There is a contradiction between the events and the Notice comment below

newTotalShares = _getTotalShares().add(_sharesAmount);
TOTAL_SHARES_POSITION.setStorageUint256(newTotalShares);

shares[_recipient] = shares[_recipient].add(_sharesAmount);

emit Transfer(address(0), _recipient, getPooledEthByShares(_sharesAmount));
emit TransferShares(address(0), _recipient, _sharesAmount);

// Notice: we're not emitting a Transfer event from the zero address here since shares mint
// works by taking the amount of tokens corresponding to the minted shares from all other
// token holders, proportionally to their share. The total supply of the token doesn't change
Expand Down