fix: various accounting bugs on migration to shares

contracts/0.4.24/Lido.sol

@@ -126,11 +126,7 @@ contract Lido is Versioned, StETHPermit, AragonApp {
         0x2ab18be87d6c30f8dc2a29c9950ab4796c891232dbcc6a95a6b44b9f8aad9352; // keccak256("lido.Lido.externalShares");
     /// @dev maximum allowed ratio of external shares to total shares in basis points
     bytes32 internal constant MAX_EXTERNAL_RATIO_POSITION =
-        0x5248bc99214b4b9bfb04eed7603bdab7b47ab5b436236fcbf7bda3acc9aea148; // keccak256("lido.Lido.maxExternalRatioBP")
-    bytes32 internal constant MAX_EXTERNAL_BALANCE_POSITION =
-        0x5d9acd3b741c556363e77af693c2f6219b9bf4d826159e864c4e3c3f08e6d97a; // keccak256("lido.Lido.maxExternalBalance")
-    bytes32 internal constant EXTERNAL_BALANCE_POSITION =
-        0x2a094e9f51934d7c659e7b6195b27a4a50d3f8a3c5e2d91b2f6c2e68c16c485b; // keccak256("lido.Lido.externalBalance")
+        0xf243b7ab6a2698a3d0a16e54fb43706d25b46e82d0a92f60e7e1a4aa86c30e1f; // keccak256("lido.Lido.maxExternalRatioBP")
 
     // Staking was paused (don't accept user's ether submits)
     event StakingPaused();

contracts/0.8.25/Accounting.sol

@@ -232,9 +232,10 @@ contract Accounting is VaultHub {
             update.sharesToMintAsFees
         );
 
+        update.postExternalShares = _pre.externalShares + totalTreasuryFeeShares;
+
         // Add the treasury fee shares to the total pooled ether and external shares
         update.postTotalPooledEther += totalTreasuryFeeShares * update.postTotalPooledEther / update.postTotalShares;
-        update.postExternalShares += totalTreasuryFeeShares;
     }
 
     /// @dev return amount to lock on withdrawal queue and shares to burn depending on the finalization batch parameters
@@ -342,7 +343,10 @@ contract Accounting is VaultHub {
         );
 
         if (vaultFeeShares > 0) {
-            STETH.mintExternalShares(LIDO_LOCATOR.treasury(), vaultFeeShares);
+            // Q: should we change it to mintShares and update externalShares before on the 2nd step?
+            STETH.mintShares(LIDO_LOCATOR.treasury(), vaultFeeShares);
+
+            // TODO: consistent events?
         }
 
         _notifyObserver(_contracts.postTokenRebaseReceiver, _report, _pre, _update);

contracts/0.8.25/interfaces/ILido.sol

@@ -9,6 +9,8 @@ interface ILido {
 
     function transferFrom(address, address, uint256) external;
 
+    function transferSharesFrom(address, address, uint256) external returns (uint256);
+
     function getTotalPooledEther() external view returns (uint256);
 
     function getExternalEther() external view returns (uint256);

contracts/0.8.25/vaults/Dashboard.sol

@@ -235,7 +235,7 @@ contract Dashboard is AccessControlEnumerable {
     function _voluntaryDisconnect() internal {
         uint256 shares = sharesMinted();
         if (shares > 0) {
-            _rebalanceVault(STETH.getPooledEthByShares(shares));
+            _rebalanceVault(_getPooledEthFromSharesRoundingUp(shares));
         }
 
         vaultHub.voluntaryDisconnect(address(stakingVault));
@@ -293,7 +293,7 @@ contract Dashboard is AccessControlEnumerable {
      * @param _amountOfShares Amount of tokens to burn
      */
     function _burn(uint256 _amountOfShares) internal {
-        STETH.transferFrom(msg.sender, address(vaultHub), _amountOfShares);
+        STETH.transferSharesFrom(msg.sender, address(vaultHub), _amountOfShares);
         vaultHub.burnSharesBackedByVault(address(stakingVault), _amountOfShares);
     }
 
@@ -305,6 +305,17 @@ contract Dashboard is AccessControlEnumerable {
         stakingVault.rebalance(_ether);
     }
 
+    function _getPooledEthFromSharesRoundingUp(uint256 _shares) internal view returns (uint256) {
+        uint256 pooledEth = STETH.getPooledEthByShares(_shares);
+        uint256 backToShares = STETH.getSharesByPooledEth(pooledEth);
+
+        if (backToShares < _shares) {
+            return pooledEth + 1;
+        }
+
+        return pooledEth;
+    }
+
     // ==================== Events ====================
 
     /// @notice Emitted when the contract is initialized

contracts/0.8.25/vaults/VaultHub.sol

@@ -310,7 +310,7 @@ abstract contract VaultHub is AccessControlEnumerableUpgradeable {
             revert AlreadyBalanced(_vault, sharesMinted, threshold);
         }
 
-        uint256 mintedStETH = STETH.getPooledEthByShares(sharesMinted);
+        uint256 mintedStETH = STETH.getPooledEthByShares(sharesMinted); // TODO: fix rounding issue
         uint256 reserveRatioBP = socket.reserveRatioBP;
         uint256 maxMintableRatio = (TOTAL_BASIS_POINTS - reserveRatioBP);
 

test/integration/vaults-happy-path.integration.ts

@@ -34,7 +34,7 @@ const TARGET_APR = 3_00n; // 3% APR
 const PROTOCOL_FEE = 10_00n; // 10% fee (5% treasury + 5% node operators)
 const TOTAL_BASIS_POINTS = 100_00n; // 100%
 
-const VAULT_OWNER_FEE = 1_00n; // 1% owner fee
+const VAULT_OWNER_FEE = 1_00n; // 1% AUM owner fee
 const VAULT_NODE_OPERATOR_FEE = 3_00n; // 3% node operator fee
 
 describe("Scenario: Staking Vaults Happy Path", () => {
@@ -406,7 +406,9 @@ describe("Scenario: Staking Vaults Happy Path", () => {
     const { lido } = ctx.contracts;
 
     // Mario can approve the vault to burn the shares
-    const approveVaultTx = await lido.connect(mario).approve(vault101AdminContract, vault101MintingMaximum);
+    const approveVaultTx = await lido
+      .connect(mario)
+      .approve(vault101AdminContract, await lido.getPooledEthByShares(vault101MintingMaximum));
     await trace("lido.approve", approveVaultTx);
 
     const burnTx = await vault101AdminContract.connect(mario).burn(vault101MintingMaximum);