Stake external implementation

.solcover.js

@@ -4,6 +4,7 @@ module.exports = {
         "test/BridgeRelayer.sol",
         "test/MockActivityChecker.sol",
         "test/MockVE.sol",
-        "test/SafeToL2Setup.sol"
+        "test/SafeToL2Setup.sol"б
+        "test/StakingTokenV1.sol"
     ]
 };

README.md

@@ -26,6 +26,7 @@ while maintaining exposure to staking rewards.
 
 ### L2 Layer (i.e. Base, etc)
 - **StakingManager**: Orchestrates service deployment and staking operations
+- **ExternalStakingDistributor**: Performs service deployment and staking for external staking contracts
 - **StakingTokenLocked**: Manages individual staking instances and reward distribution
 - **ActivityModule**: Handles service activity verification and reward claiming
 - **Collector**: Collects and bridges rewards back to L1
@@ -48,6 +49,7 @@ contracts/
 │   └── UnstakeRelayer.sol # Unstake request handling
 ├── l2/                    # L2 (Gnosis Chain) contracts
 │   ├── StakingManager.sol    # Staking orchestration
+│   ├── ExternalStakingDistributor # External staking orchestration
 │   ├── StakingTokenLocked.sol # Individual staking instances
 │   ├── ActivityModule.sol     # Service activity management
 │   └── Collector.sol          # Reward collection and bridging
@@ -78,7 +80,8 @@ doc/                       # Documentation and whitepaper
 1. Services are deployed on L2 with OLAS backing
 2. Rewards accumulate based on service performance
 3. ActivityModule verifies service liveness and required KPI performance
-4. Collector gathers rewards and bridges them back to L1 via a Distributor contract
+4. ExternalStakingDistributor curates all external staking and forces unstakes, if required
+5. Collector gathers rewards and bridges them back to L1 via a Distributor contract
 
 ### 3. Withdrawal Process
 1. User requests withdrawal through Treasury

audits/README.md

@@ -8,6 +8,7 @@ An `audit3` with a focus `post-internal-audit` branch [audit3](https://github.co
 An `audit4` with a focus `post-lock` branch [audit4](https://github.com/kupermind/olas-lst/blob/main/audits/audit4). <br>
 An `audit5` with a focus `main` branch [audit5](https://github.com/kupermind/olas-lst/blob/main/audits/audit5). <br>
 An `audit6` with a focus `main` branch [audit6](https://github.com/kupermind/olas-lst/blob/main/audits/audit6). <br>
+An `audit7` with a focus `main` branch [audit6](https://github.com/LemonTreeTechnologies/olas-lst/blob/main/audits/audit7). <br>
 
 ### External audits
 External audit reports are listed in their historical order:

audits/audit7/README.md

@@ -0,0 +1,94 @@
+# Audit 7 - External Staking Implementation
+
+**Audit Date:** December 29, 2024  
+**Commit Range:** `d7db0db` to `2d32914` (inclusive)  
+**Branch:** `stake_external`  
+**Tag:** `v0.2.0-pre-internal-audit`  
+**Repository:** `https://github.com/LemonTreeTechnologies/olas-lst`
+
+## Objectives
+
+This audit focuses on the security review of the external staking implementation, which introduces:
+- New `ExternalStakingDistributor` contract for distributing OLAS across external staking contracts
+- Modifications to `Depository`, `Treasury`, `Collector`, and bridging contracts to support external staking
+- Integration with Safe multisig contracts for service management
+
+## Scope
+
+The audit reviewed all changes between commits `d7db0db` (main branch) and `2d32914` (HEAD), including:
+- 1 new contract: `ExternalStakingDistributor.sol` (787 lines)
+- 9 modified contracts
+- 2 modified test files
+- 1 documentation update
+
+## Findings
+### Critical. The "create vs. update" flag in _deployAndStake is reversed.
+```
+    function _deployAndStake(
+        address stakingProxy,
+        uint256 minStakingDeposit,
+        uint256 serviceId,
+        uint256 agentId,
+        bytes32 configHash,
+        address agentInstance
+    ) internal returns (uint256) {
+        // Get service creation flag
+        bool createService = serviceId > 0 ? true : false; # condition ? value_if_true : value_if_false
+        ->  serviceId > 0 -> createService = true
+        ->
+        if (createService) { // if serviceId > 0
+            // Create a service owned by this contract
+            serviceId = IService(serviceManager)
+                .create(address(this), olas, configHash, agentIds, agentParams, uint32(THRESHOLD));
+        } else {
+            // Update service owned by this contract
+            IService(serviceManager).update(olas, configHash, agentIds, agentParams, uint32(THRESHOLD), serviceId);
+        }
+    Fix: bool createService = (serviceId == 0); ?
+```
+[x] Fixed
+
+### Critical. Incorrect mapServiceIdCuratingAgents entry in stake for serviceId == 0
+```
+/// @param serviceId Service Id: non-zero if service is owned by address(this) and could be reused, zero otherwise.
+    function stake(address stakingProxy, uint256 serviceId, uint256 agentId, bytes32 configHash, address agentInstance)
+
+    mapServiceIdCuratingAgents[serviceId] = msg.sender;
+    serviceId = _deployAndStake(..., serviceId, ...);
+
+    Problem: If serviceId == 0 (creating a new service), the entry is made to key 0, and the real serviceId appears after _deployAndStake. As a result: for the new service, mapServiceIdCuratingAgents[realServiceId] will remain zero, and mapServiceIdCuratingAgents[0] will be equal to some address (curator).
+```
+[x] Fixed
+
+### Critical? abi.encodePacked(address(0))
+```
+    _deployAndStake(...)
+    address multisig; // zero!
+        if (createService) {
+            // Create multisig with address(this) as module and swap owners to agentInstance
+            _createMultisigWithSelfAsModule(agentInstance);
+
+            // Deploy service via same address multisig
+            multisig = IService(serviceManager).deploy(serviceId, safeSameAddressMultisig, abi.encodePacked(multisig)); // abi.encodePacked(address(0)) ?!
+    ...
+    Comment needed.
+```
+[x] Fixed
+
+### Medium. changeRewardFactors() is only available before initialize
+```
+function changeRewardFactors(
+        uint256 _collectorRewardFactor,
+        uint256 _protocolRewardFactor,
+        uint256 _curatingAgentRewardFactor
+    ) public {
+        if (owner != address(0)) {
+            revert AlreadyInitialized();
+        }
+    Should the owner be able to change this later?
+```
+[x] Fixed
+
+[Security Audit Report](findings/security-audit-report.md)
+
+

audits/audit7/changes/ABICreator.sol.diff

@@ -0,0 +1,14 @@
+diff --git a/contracts/test/ABICreator.sol b/contracts/test/ABICreator.sol
+index be6025d..2f83883 100644
+--- a/contracts/test/ABICreator.sol
++++ b/contracts/test/ABICreator.sol
+@@ -17,6 +17,9 @@ import {ServiceManagerProxy} from "../../lib/autonolas-registries/contracts/Serv
+ import {GnosisSafeMultisig} from "../../lib/autonolas-registries/contracts/multisigs/GnosisSafeMultisig.sol";
+ import {GnosisSafeSameAddressMultisig} from
+     "../../lib/autonolas-registries/contracts/multisigs/GnosisSafeSameAddressMultisig.sol";
++import {RecoveryModule} from "../../lib/autonolas-registries/contracts/multisigs/RecoveryModule.sol";
++import {SafeMultisigWithRecoveryModule} from
++    "../../lib/autonolas-registries/contracts/multisigs/SafeMultisigWithRecoveryModule.sol";
+ import {StakingVerifier} from "../../lib/autonolas-registries/contracts/staking/StakingVerifier.sol";
+ import {StakingFactory} from "../../lib/autonolas-registries/contracts/staking/StakingFactory.sol";
+ import {ERC20Token} from "../../lib/autonolas-registries/contracts/test/ERC20Token.sol";

audits/audit7/changes/BaseStakingProcessorL2.sol.diff

@@ -0,0 +1,28 @@
+diff --git a/contracts/l2/bridging/BaseStakingProcessorL2.sol b/contracts/l2/bridging/BaseStakingProcessorL2.sol
+index 43e618b..4c7f312 100644
+--- a/contracts/l2/bridging/BaseStakingProcessorL2.sol
++++ b/contracts/l2/bridging/BaseStakingProcessorL2.sol
+@@ -51,6 +51,7 @@ contract BaseStakingProcessorL2 is DefaultStakingProcessorL2 {
+     /// @dev GnosisTargetDispenserL2 constructor.
+     /// @param _olas OLAS token address.
+     /// @param _stakingManager StakingManager address.
++    /// @param _externalStakingDistributor ExternalStakingDistributor address.
+     /// @param _collector Collector address.
+     /// @param _l2TokenRelayer L2 token relayer bridging contract address.
+     /// @param _l2MessageRelayer L2 message relayer bridging contract address (AMBHomeProxy).
+@@ -59,6 +60,7 @@ contract BaseStakingProcessorL2 is DefaultStakingProcessorL2 {
+     constructor(
+         address _olas,
+         address _stakingManager,
++        address _externalStakingDistributor,
+         address _collector,
+         address _l2TokenRelayer,
+         address _l2MessageRelayer,
+@@ -68,6 +70,7 @@ contract BaseStakingProcessorL2 is DefaultStakingProcessorL2 {
+         DefaultStakingProcessorL2(
+             _olas,
+             _stakingManager,
++            _externalStakingDistributor,
+             _collector,
+             _l2TokenRelayer,
+             _l2MessageRelayer,

audits/audit7/changes/Collector.sol.diff

@@ -0,0 +1,44 @@
+diff --git a/contracts/l2/Collector.sol b/contracts/l2/Collector.sol
+index aeb9747..3e6c707 100644
+--- a/contracts/l2/Collector.sol
++++ b/contracts/l2/Collector.sol
+@@ -21,11 +21,6 @@ interface IToken {
+     /// @param amount Amount to transfer to.
+     /// @return True if the function execution is successful.
+     function transferFrom(address from, address to, uint256 amount) external returns (bool);
+-
+-    /// @dev Gets the amount of tokens owned by a specified account.
+-    /// @param account Account address.
+-    /// @return Amount of tokens owned.
+-    function balanceOf(address account) external view returns (uint256);
+ }
+
+ /// @dev Zero value.
+@@ -185,6 +180,27 @@ contract Collector is Implementation {
+         emit OperationReceiversSet(operations, receivers);
+     }
+
++    /// @dev Tops up address(this) with a specified amount for protocol assets.
++    /// @param amount OLAS amount.
++    function topUpProtocol(uint256 amount) external {
++        // Reentrancy guard
++        if (_locked == 2) {
++            revert ReentrancyGuard();
++        }
++        _locked = 2;
++
++        // Pull OLAS amount
++        IToken(olas).transferFrom(msg.sender, address(this), amount);
++
++        // Update protocol balance
++        uint256 curProtocolBalance = protocolBalance + amount;
++        protocolBalance = curProtocolBalance;
++
++        emit ProtocolBalanceUpdated(curProtocolBalance);
++
++        _locked = 1;
++    }
++
+     /// @dev Tops up address(this) with a specified amount according to a selected operation.
+     /// @param amount OLAS amount.
+     /// @param operation Operation type.

audits/audit7/changes/DefaultStakingProcessorL2.sol.diff

@@ -0,0 +1,115 @@
+diff --git a/contracts/l2/bridging/DefaultStakingProcessorL2.sol b/contracts/l2/bridging/DefaultStakingProcessorL2.sol
+index 62ee10f..9a86935 100644
+--- a/contracts/l2/bridging/DefaultStakingProcessorL2.sol
++++ b/contracts/l2/bridging/DefaultStakingProcessorL2.sol
+@@ -29,6 +29,18 @@ interface IStakingManager {
+     function unstake(address stakingProxy, uint256 amount, bytes32 operation) external;
+ }
+
++interface IExternalStakingDistributor {
++    /// @dev Deposits OLAS for further staking.
++    /// @param amount OLAS amount.
++    /// @param operation Stake operation type.
++    function deposit(uint256 amount, bytes32 operation) external;
++
++    /// @dev Requests withdraw via specified unstake operation.
++    /// @param amount Unstake amount.
++    /// @param operation Unstake operation type.
++    function withdraw(uint256 amount, bytes32 operation) external;
++}
++
+ // Necessary ERC20 token interface
+ interface IToken {
+     /// @dev Sets `amount` as the allowance of `spender` over the caller's tokens.
+@@ -90,6 +102,8 @@ abstract contract DefaultStakingProcessorL2 is IBridgeErrors {
+     address public immutable olas;
+     // Staking manager address
+     address public immutable stakingManager;
++    // External staking distributor address
++    address public immutable externalStakingDistributor;
+     // Collector address
+     address public immutable collector;
+     // L2 Relayer address that receives the message across the bridge from the source L1 network
+@@ -115,6 +129,7 @@ abstract contract DefaultStakingProcessorL2 is IBridgeErrors {
+     /// @dev DefaultStakerL2 constructor.
+     /// @param _olas OLAS token address on L2.
+     /// @param _stakingManager StakingManager address.
++    /// @param _externalStakingDistributor ExternalStakingDistributor address.
+     /// @param _collector Collector address.
+     /// @param _l2TokenRelayer L2 token relayer bridging contract address.
+     /// @param _l2MessageRelayer L2 message relayer bridging contract address.
+@@ -122,6 +137,7 @@ abstract contract DefaultStakingProcessorL2 is IBridgeErrors {
+     constructor(
+         address _olas,
+         address _stakingManager,
++        address _externalStakingDistributor,
+         address _collector,
+         address _l2TokenRelayer,
+         address _l2MessageRelayer,
+@@ -130,8 +146,9 @@ abstract contract DefaultStakingProcessorL2 is IBridgeErrors {
+     ) {
+         // Check for zero addresses
+         if (
+-            _olas == address(0) || _stakingManager == address(0) || _collector == address(0)
+-                || _l2TokenRelayer == address(0) || _l2MessageRelayer == address(0) || _l1DepositProcessor == address(0)
++            _olas == address(0) || _stakingManager == address(0) || _externalStakingDistributor == address(0)
++                || _collector == address(0) || _l2TokenRelayer == address(0) || _l2MessageRelayer == address(0)
++                || _l1DepositProcessor == address(0)
+         ) {
+             revert ZeroAddress();
+         }
+@@ -149,6 +166,7 @@ abstract contract DefaultStakingProcessorL2 is IBridgeErrors {
+         // Immutable parameters assignment
+         olas = _olas;
+         stakingManager = _stakingManager;
++        externalStakingDistributor = _externalStakingDistributor;
+         collector = _collector;
+         l2TokenRelayer = _l2TokenRelayer;
+         l2MessageRelayer = _l2MessageRelayer;
+@@ -194,12 +212,22 @@ abstract contract DefaultStakingProcessorL2 is IBridgeErrors {
+
+                 // Check the OLAS balance and the contract being unpaused
+                 if (olasBalance >= amount) {
+-                    // Approve OLAS for stakingManager
+-                    IToken(olas).approve(stakingManager, amount);
+-
+-                    // This is a low level call since it must never revert
+-                    bytes memory stakeData = abi.encodeCall(IStakingManager.stake, (target, amount, operation));
+-                    (success,) = stakingManager.call(stakeData);
++                    if (target == externalStakingDistributor) {
++                        // Approve OLAS for externalStakingDistributor
++                        IToken(olas).approve(externalStakingDistributor, amount);
++
++                        // This is a low level call since it must never revert
++                        bytes memory stakeData =
++                            abi.encodeCall(IExternalStakingDistributor.deposit, (amount, operation));
++                        (success,) = externalStakingDistributor.call(stakeData);
++                    } else {
++                        // Approve OLAS for stakingManager
++                        IToken(olas).approve(stakingManager, amount);
++
++                        // This is a low level call since it must never revert
++                        bytes memory stakeData = abi.encodeCall(IStakingManager.stake, (target, amount, operation));
++                        (success,) = stakingManager.call(stakeData);
++                    }
+                 } else {
+                     // Insufficient OLAS balance
+                     status = RequestStatus.INSUFFICIENT_OLAS_BALANCE;
+@@ -210,9 +238,14 @@ abstract contract DefaultStakingProcessorL2 is IBridgeErrors {
+             }
+         } else if (operation == UNSTAKE || operation == UNSTAKE_RETIRED) {
+             // Note that if UNSTAKE* is requested, it must be finalized in any case since changes are recorded on L1
+-            // This is a low level call since it must never revert
+-            bytes memory unstakeData = abi.encodeCall(IStakingManager.unstake, (target, amount, operation));
+-            (success,) = stakingManager.call(unstakeData);
++            // These are low level calls since they must never revert
++            if (target == externalStakingDistributor) {
++                bytes memory unstakeData = abi.encodeCall(IExternalStakingDistributor.withdraw, (amount, operation));
++                (success,) = stakingManager.call(unstakeData);
++            } else {
++                bytes memory unstakeData = abi.encodeCall(IStakingManager.unstake, (target, amount, operation));
++                (success,) = stakingManager.call(unstakeData);
++            }
+         } else {
+             // Unsupported operation type
+             status = RequestStatus.UNSUPPORTED_OPERATION_TYPE;