Aligned Layer

Description

When a new task is created through BatcherPaymentService, the createNewTask() function is called with appropriate signature arguments to validate user submissions, subtract funds from their accounts, and ultimately create the task via a call to AlignedLayerServiceManager.createNewTask().

However, the signature validation and balance adjustment happens in an intermediate function called checkMerkleRootAndVerifySignatures() which takes almost the same parameters:

contracts/src/core/BatcherPaymentService.sol:L173-L178

function checkMerkleRootAndVerifySignatures(
    bytes32[] calldata leaves,
    bytes32 batchMerkleRoot,
    SignatureData[] calldata signatures,
    uint256 feePerProof
) public {

The issue is that this function is public with no modifiers, unlike createNewTask() which is both onlyBatcher and whenNotPaused. There are no further checks for msg.sender. As a result, any user can call checkMerkleRootAndVerifySignatures() directly with correct signature parameters by frontrunning the createNewTask() transaction, and the contract would validate the signatures and subtract user balances.

Two additional outcomes here are that:

• The user nonces are now updated, so those same signatures arguments that were taken can no longer be used, so the proper transaction that got frontran will be reverted in the same part of the code where the nonce got updated:

contracts/src/core/BatcherPaymentService.sol:L248-L249

require(user_data.nonce == signatureData.nonce, "Invalid Nonce");
user_data.nonce++;

• The feePerProof argument is given directly in checkMerkleRootAndVerifySignatures(), so a malicious user can simply pick a large number to subtract all funds from user balances. Just as a note - in createNewTask() the feePerProof argument is constructed from arguments by an authenticated called which is the batcher, so, while the resulting feePerProof could still be unfairly large, this is at least less possible from an authenticated called which has access to other admin-like functionality on the contract.

The impact of this function’s visibility is that users may lose all their funds, and proper transactions can be reverted.

Recommendation

Adjust visibility and other argument checks on the checkMerkleRootAndVerifySignatures() function.

Description

Several vulnerabilities are present due to insufficient access controls and a lack of proper authentication checks between BatcherPaymentService and AlignedLayerServiceManager. These issues allow unauthorized actions and bypass essential validations. Also, see issue 4.3 .

1. Unauthorized Access to createNewTask()

The function AlignedLayerServiceManager.createNewTask() is not protected by authentication, allowing any user to call it directly without going through the BatcherPaymentService interface. This enables bypassing the onlyBatcher restriction applied in BatcherPaymentService.createNewTask().

contracts/src/core/BatcherPaymentService.sol:L68-L76

// PUBLIC FUNCTIONS
function createNewTask(
    bytes32 batchMerkleRoot,
    string calldata batchDataPointer,
    bytes32[] calldata leaves, // padded to the next power of 2
    SignatureData[] calldata signatures, // actual length (proof sumbitters == proofs submitted)
    uint256 gasForAggregator,
    uint256 gasPerProof
) external onlyBatcher whenNotPaused {

contracts/src/core/AlignedLayerServiceManager.sol:L56-L60

function createNewTask(
    bytes32 batchMerkleRoot,
    string calldata batchDataPointer
) external payable {
    require(

2. Bypassing Merkle Root and Signatures / Balance checks

Direct interaction with AlignedLayerServiceManager.createNewTask() allows users to bypass Merkle root and signature checks enforced by BatcherPaymentService.createNewTask(). This omission provides no incentive to use BatcherPaymentService, undermining the intended validation mechanisms.

contracts/src/core/BatcherPaymentService.sol:L98-L114

checkMerkleRootAndVerifySignatures(
    leaves,
    batchMerkleRoot,
    signatures,
    feePerProof
);

// call alignedLayerServiceManager
// with value to fund the task's response
(bool success, ) = AlignedLayerServiceManager.call{
    value: feeForAggregator
}(
    abi.encodeWithSignature(
        "createNewTask(bytes32,string)",
        batchMerkleRoot,
        batchDataPointer
    )

3. Bypassing Pausable Functionality

BatcherPaymentService has pausable functionality restricted to onlyBatcher, but AlignedLayerServiceManager does not enforce such restrictions and is callable by anyone. This exposes the system to risks during maintenance or emergency situations.

contracts/src/core/BatcherPaymentService.sol:L69-L76

function createNewTask(
    bytes32 batchMerkleRoot,
    string calldata batchDataPointer,
    bytes32[] calldata leaves, // padded to the next power of 2
    SignatureData[] calldata signatures, // actual length (proof sumbitters == proofs submitted)
    uint256 gasForAggregator,
    uint256 gasPerProof
) external onlyBatcher whenNotPaused {

AlignedLayerServiceManager has no pausable restrictions, callable by anyone

contracts/src/core/AlignedLayerServiceManager.sol:L56-L68

function createNewTask(
    bytes32 batchMerkleRoot,
    string calldata batchDataPointer
) external payable {
    require(
        batchesState[batchMerkleRoot].taskCreatedBlock == 0,
        "Batch was already submitted"
    );

    if (msg.value > 0) {
        batchersBalances[msg.sender] += msg.value;
    }

4. Fee Evasion

Users may avoid paying the feePerProof by interacting directly with AlignedLayerServiceManager.createNewTask() rather than using BatcherPaymentService, thus circumventing fees intended for the BatcherWallet in BatcherPaymentService.

contracts/src/core/BatcherPaymentService.sol:L105-L121

// call alignedLayerServiceManager
// with value to fund the task's response
(bool success, ) = AlignedLayerServiceManager.call{
    value: feeForAggregator
}(
    abi.encodeWithSignature(
        "createNewTask(bytes32,string)",
        batchMerkleRoot,
        batchDataPointer
    )
);

require(success, "createNewTask call failed");

payable(BatcherWallet).transfer(
    (feePerProof * signaturesQty) - feeForAggregator
);

Recommendation

Implement access control and authentication checks in AlignedLayerServiceManager to ensure that all critical functions are only callable through authorized channels, such as BatcherPaymentService. Specifically, consider the following actions:

• Restrict direct access to createNewTask() in AlignedLayerServiceManager and enforce access through BatcherPaymentService.
• Ensure that AlignedLayerServiceManager incorporates all necessary checks, including Merkle root and signature validations, to maintain system integrity.
• Review fee structures and enforce them consistently across both contracts to prevent fee evasion.

Description

The AlignedLayerServiceManager.createNewTask() function is vulnerable to front-running attacks due to a lack of authentication. Together with insufficient balance management this can easily lead to Denial of Service (DoS) attacks. Malicious actors can preempt legitimate batcher submissions by setting themselves as the batcher for a merkle root with minimal funds, potentially causing DoS conditions or enabling extortion. The function only requires a non-zero value, allowing tasks that can’t cover transaction costs reimbursed in respondToTask(). Additionally, the user-provided batchDataPointer in emitted events may impact off-chain components if manipulated.

• BatcherPaymentService.createNewTask() calling AlignedLayerServiceManager.createNewTask() (presumably, this is the main call flow)

contracts/src/core/BatcherPaymentService.sol:L109-L115

}(
    abi.encodeWithSignature(
        "createNewTask(bytes32,string)",
        batchMerkleRoot,
        batchDataPointer
    )
);

• AlignedLayerServiceManager.createNewTask() is not authenticated, and batch states are global and merkle root indexed. The function does not enforce a meaningful minimum balance for the batcher; 1 wei is enough to set yourself as the batcher for a merkle root while this task can never be fulfilled because it is not enough to cover transaction costs reimbursed when responding.

contracts/src/core/AlignedLayerServiceManager.sol:L56-L80

function createNewTask(
    bytes32 batchMerkleRoot,
    string calldata batchDataPointer
) external payable {
    require(
        batchesState[batchMerkleRoot].taskCreatedBlock == 0,
        "Batch was already submitted"
    );

    if (msg.value > 0) {
        batchersBalances[msg.sender] += msg.value;
    }

    require(batchersBalances[msg.sender] > 0, "Batcher balance is empty");

    BatchState memory batchState;

    batchState.taskCreatedBlock = uint32(block.number);
    batchState.responded = false;
    batchState.batcherAddress = msg.sender;

    batchesState[batchMerkleRoot] = batchState;

    emit NewBatch(batchMerkleRoot, uint32(block.number), batchDataPointer);
}

• AlignedLayerServiceManager.respondToTask() reimbursing transaction costs

contracts/src/core/AlignedLayerServiceManager.sol:L135-L148

// 70k was measured by trial and error until the aggregator got paid a bit over what it needed
uint256 txCost = (initialGasLeft - finalGasLeft + 70000) * tx.gasprice;

require(
    batchersBalances[batchesState[batchMerkleRoot].batcherAddress] >=
        txCost,
    "Batcher has not sufficient funds for paying this transaction"
);

batchersBalances[
    batchesState[batchMerkleRoot].batcherAddress
] -= txCost;
payable(msg.sender).transfer(txCost);

The batchDataPointer is user provided data that is emitted with the event. By front-running legitimate task creations a malicious actor could submit the same root with a different batchDataPointer which might have consequences for off-chain components picking up that data. Additionally, the solidity type string might not be the most efficient data type for this purpose (string is assumed utf8 encoded, bytes would be more efficient, fixed length bytes would be most gas efficient)

Recommendation

Implement a meaningful minimum balance requirement for createNewTask(). Consider removing transaction cost reimbursement or implement a more robust system. Authenticate the createNewTask() function or index tasks by a unique batchId instead of merkle root. Implement measures to prevent front-running of batcher assignments. Validate and sanitize the batchDataPointer input. Consider using bytes instead of string for batchDataPointer for improved efficiency.

Description

To calculate the refund for a user, the contracts use tx.gasprice. However, this is an effective gas price that the user pays, not a system-set price. In particular, it consists of two gas prices:

• base fee, as set by the system
• priority fee, which is tipped to the miner and defined by the user.

This gas price is used to calculate the transaction cost to refund the user:

contracts/src/core/AlignedLayerServiceManager.sol:L137

uint256 txCost = (initialGasLeft - finalGasLeft + 70000) * tx.gasprice;

In the event that a user==miner, they can:

1. set an absurdly high transaction price
2. call respondToTask()
3. perform the transaction
4. get the inflated transaction refund which would siphon all the funds out from the batchersBalances[batchesState[batchMerkleRoot].batcherAddress]

Recommendation

Consider redesigning the refund mechanism to not be susceptible to user-given parameters, such as a flat refund or a base fee-focused refund.

Description

The AlignedLayerServiceManager contract maintains a balance tracker, batchersBalances[], which is updated either through the receive() function from a user or during createTask(). The latter requires a non-zero value to be sent with the message:

contracts/src/core/AlignedLayerServiceManager.sol:L65-L69

if (msg.value > 0) {
    batchersBalances[msg.sender] += msg.value;
}

require(batchersBalances[msg.sender] > 0, "Batcher balance is empty");

Once a task is created for a batcher address, it remains associated with that address and cannot be removed. When responding to the task, the system verifies that the batcher has sufficient funds to cover the transaction:

contracts/src/core/AlignedLayerServiceManager.sol:L104-L107

require(
    batchersBalances[batchesState[batchMerkleRoot].batcherAddress] > 0,
    "Batcher has no balance"
);

contracts/src/core/AlignedLayerServiceManager.sol:L139-L143

require(
    batchersBalances[batchesState[batchMerkleRoot].batcherAddress] >=
        txCost,
    "Batcher has not sufficient funds for paying this transaction"
);

The initial check ensures a non-zero balance, while the second check requires a specific amount and will revert if the balance is insufficient. This can lead to a situation where a batcher’s task becomes unrespondable if their balance is depleted, whether due to malicious intent or an error. Furthermore, the balance tracker is only updated through receive() or createTask(), and there is no mechanism to top off a batcher’s balance without their direct involvement.

Recommendation

Consider

• asking for specific funds during createTask so the rest of the flow goes through
• allowing for admins to remove tasks
• scratching the refund mechanism

Description

The current implementation uses a raw ecrecover function to verify whether a user signed a data structure by hashing the data directly. This approach is susceptible to cross-network replay attacks. To enhance security and prevent such issues, the implementation should use a domain separator as specified in EIP-712.

contracts/src/core/BatcherPaymentService.sol:L230-L244

function verifySignatureAndDecreaseBalance(
    bytes32 hash,
    SignatureData calldata signatureData,
    uint256 feePerProof
) private {
    bytes32 noncedHash = keccak256(
        abi.encodePacked(hash, signatureData.nonce)
    );

    address signer = ecrecover(
        noncedHash,
        signatureData.v,
        signatureData.r,
        signatureData.s
    );

Recommendation

Utilize the OpenZeppelin EIP712 and ECDSA libraries to implement domain-bound signatures. These libraries provide robust and battle-tested solutions for signature validation, mitigating common issues associated with ECDSA and preventing cross-network replay attacks.

Description

The BatcherPaymentService deployment script using Forge performs a two-step deployment and initialization process. Between the startBroadcast and stopBroadcast directives, the script executes three transactions:

1. Deploys a new BatcherService contract.
2. Deploys a new ERC1967Proxy without initialization calldata.
3. Initializes the BatcherPaymentService contract through the proxy.

contracts/script/deploy/BatcherPaymentServiceDeployer.s.sol:L31-L44

vm.startBroadcast();

BatcherPaymentService batcherPaymentService = new BatcherPaymentService();
ERC1967Proxy proxy = new ERC1967Proxy(
    address(batcherPaymentService),
    ""
);
BatcherPaymentService(payable(address(proxy))).initialize(
    alignedLayerServiceManager,
    batcherPaymentServiceOwner,
    batcherWallet
);

vm.stopBroadcast();

Since these transactions are broadcast individually, there is a risk that a malicious actor could front-run the third transaction to claim control of the newly deployed contract. This issue could force the deployer to re-deploy the contracts.

For further details, refer to the Forge script documentation.

Recommendation

To mitigate this risk, include the initialization calldata when deploying the ERC1967Proxy. This approach ensures that the contract is initialized in the same transaction as its deployment, preventing potential front-running attacks.

Description

Both contracts are intended to receive, store, and use ETH on behalf of users. The calculations for ETH subtractions and usage are for gas purposes, which are difficult to calculate accurately.

As a result, it is probable that some amount of ETH will be stuck in the contracts with nobody being able to use or claim them.

Recommendation

Consider adding an admin authentication-protected withdraw() function that just sends a privileged address the funds.

Description

The unlock() function is designed to set a timelock parameter for a user, allowing them to unlock their account at a specified future time:

contracts/src/core/BatcherPaymentService.sol:L124-L131

function unlock() external whenNotPaused {
    require(
        UserData[msg.sender].balance > 0,
        "User has no funds to unlock"
    );

    UserData[msg.sender].unlockBlock = block.number + UNLOCK_BLOCK_COUNT;
}

However, when the unlock occurs through the withdraw() function, the timelock status is not reset. Consequently, the user remains in an “unlocked” state indefinitely, which allows them to deposit and withdraw without restrictions:

contracts/src/core/BatcherPaymentService.sol:L138-L150

function withdraw(uint256 amount) external whenNotPaused {
    UserInfo storage user_data = UserData[msg.sender];
    require(user_data.balance >= amount, "Payer has insufficient balance");

    require(
        user_data.unlockBlock != 0 && user_data.unlockBlock <= block.number,
        "Funds are locked"
    );

    user_data.balance -= amount;
    payable(msg.sender).transfer(amount);
    emit FundsWithdrawn(msg.sender, amount);
}

After a conversation with the team, it appears that the batcher service will only take signatures from those users that are actively in the locked state. In other words, there is offchain logic that helps alleviate the impact of this issue. The only caveat is that the users would always need to lock themselves after unlocking and withdrawing.

Recommendation

Consider changing the withdraw() function so it resets the user’s unlock status after the unlock period expires. This will prevent users from remaining in an unrestricted state indefinitely.

Description

The system currently uses the direct ecrecover() function to determine the signer of signatureData:

contracts/src/core/BatcherPaymentService.sol:L239-L244

address signer = ecrecover(
    noncedHash,
    signatureData.v,
    signatureData.r,
    signatureData.s
);

This approach has the following issues:

• Error Handling: A failed signature verification returns 0 rather than reverting with an error, potentially leading to incorrect error handling downstream.
• Signature Manipulation: Direct use of ecrecover() is vulnerable to manipulation. For example, adjusting the signatureData.r parameter can yield a random address, allowing for incorrect or malicious signatures to be processed as valid.

Using a function from a well-established library, such as OpenZeppelin’s, can address these concerns.

Recommendation

To improve security and error handling, use the OpenZeppelin ECDSA library for signature verification. This library provides a secure and reliable implementation of ecrecover() that includes robust error handling and mitigates the risk of signature manipulation. Incorporating this library will ensure more accurate and secure signature verification in the contract.

Description

The Aligned team has indicated that they rely on eigenlayer-middleware patches to implement whitelisting features in the Registry Coordinator. The team provided a draft patch for review. However, we advise against directly patching the audited Layr-Labs repository for the following reasons:

• src/BLSSignatureChecker.sol and src/interfaces/IBLSSignatureChecker.sol change the function signature of checkSignatures to exclude quorumNumbers as they are hardcoded to ALIGNED_QUORUM_NUMBER. This is a very protocol-specific changeset that changes the Layer-Labs API which should be avoided by all costs as this will be very hard to maintain when trying to update to newer eigenlayer-middleware releases (i.e. security updates, etc.). Hardcoding the quorums to ALIGNED_QUORUM_NUMBER (hex 0x00) diverges from how eigenlayer-middleware is meant to be used.
• src/Whitelist.sol adds a Whitelist contract. The contract lacks clear visibility specifiers for the internal whitelist mapping (defaults to internal). Adding/Removing addresses from/to whitelist does not yield any events although it is an important administrative action (monitoring/trail of events). The contract should be abstract as it is meant to be inherited by other contracts.
• src/RegistryCoordinator.sol patches the stock RegistryCoordinator inside the eigenlayer repository (!) to include Whitelist functionality. Instead of patching the source contract in the foreign-maintained repository, it would be much better to sub-class the RegistryCoordinator by building an AlignedRegistryCoordinator that inherits from the original RegistryCoordinator overriding functionality with the patched whitelisting features. Furthermore, the changeset changes/shortens error messages emitted by the stock RegistryCoordinator which basically makes the contract diverge from the original API and specification. This may be problematic as the contract does not match the eigenlayer API documentation anymore.
• In general, reformatting the codebase may make it increasingly hard to diff future versions of the aligned version from the upstream.

Recommendation

Avoid making direct modifications to third-party codebases like the Layr-Labs repository. Instead, import the required components, subclass where necessary, and override functionality within the Aligned codebase. This approach preserves compatibility with the original API, reduces maintenance complexity, and ensures alignment with eigenlayer documentation and future updates.

Description

It is recommended to avoid using the low-level, untyped, and unchecked address.call() method when interacting with a contract. Instead, consider invoking the external contract through its defined contract type. This approach ensures type safety, correct function signature matching, and proper revert propagation.

AlignedLayerServiceManager.createNewTask{value:feeForAggregator}(batchMerkleRoot, batchDataPointer);

contracts/src/core/BatcherPaymentService.sol:L105-L117

// call alignedLayerServiceManager
// with value to fund the task's response
(bool success, ) = AlignedLayerServiceManager.call{
    value: feeForAggregator
}(
    abi.encodeWithSignature(
        "createNewTask(bytes32,string)",
        batchMerkleRoot,
        batchDataPointer
    )
);

require(success, "createNewTask call failed");

Description

Critical state-altering operations in the contracts do not emit events, which diminishes transparency and makes it harder to audit changes effectively.

Examples

• The creation of a new task.

• The locking and unlocking of balances before withdrawal:

contracts/src/core/BatcherPaymentService.sol:L123-L136

function unlock() external whenNotPaused {
    require(
        UserData[msg.sender].balance > 0,
        "User has no funds to unlock"
    );

    UserData[msg.sender].unlockBlock = block.number + UNLOCK_BLOCK_COUNT;
}

function lock() external whenNotPaused {
    require(UserData[msg.sender].balance > 0, "User has no funds to lock");
    UserData[msg.sender].unlockBlock = 0;
}

• The checkMerkleRootAndVerifySignatures function, which is public and updates user balances in verifySignatureAndDecreaseBalance but does not emit any events.

Recommendation

Implement event emissions for all significant state changes. This will improve off-chain monitoring, enhance debugging capabilities, and increase transparency in contract interactions.

Description

In createNewTask() there is an argument batchDataPointer which is passed along with the batchMerkleRoot. After all the checks, the batchDataPointer is emitted as part of the event, seemingly as the data that is associated with this root exactly.

However, this isn’t checked or validated, so any valid batchMerkleRoot may be provided to emit the event with any arbitrary, valid, or invalid batchDataPointer as the user would want.

This could cause confusion in some backend event-listening services.

Recommendation

Validate batchDataPointer as needed.

Description

The AlignedLayerServiceManager should use the initializer from the base class ServiceManagerBase to handle ownership transfers rather than performing the transfer directly. This ensures that ownership changes are processed consistently and in accordance with the base class’s established mechanisms.

Examples

contracts/src/core/AlignedLayerServiceManager.sol:L51-L54

function initialize(address _initialOwner) public initializer {
    _transferOwnership(_initialOwner);
}

Recommendation

Modify the AlignedLayerServiceManager to invoke the ownership transfer initializer provided by ServiceManagerBase instead of directly transferring ownership. This approach maintains consistency and leverages the base class’s established initialization processes.

Description

Following the Solidity Style Guide wherever possible is advisable, especially when it comes to variable name conventions and code layout.

This can be consistently enforced across the entire code base by means of an automatic linter, such as Solhint.

Examples

• Variable names should begin lower case. Type declarations (including Contracts) begin upper case.

contracts/src/core/BatcherPaymentService.sol:L35-L40

address public AlignedLayerServiceManager;
address public BatcherWallet;

// map to user data
mapping(address => UserInfo) public UserData;

contracts/src/core/BatcherPaymentService.sol:L49-L61

function initialize(
    address _AlignedLayerServiceManager,
    address _BatcherPaymentServiceOwner,
    address _BatcherWallet
) public initializer {
    __Ownable_init(); // default is msg.sender
    __UUPSUpgradeable_init();
    _transferOwnership(_BatcherPaymentServiceOwner);

    AlignedLayerServiceManager = _AlignedLayerServiceManager;
    BatcherWallet = _BatcherWallet;
}

contracts/src/core/BatcherPaymentService.sol:L63-L66

receive() external payable {
    UserData[msg.sender].balance += msg.value;
    emit PaymentReceived(msg.sender, msg.value);
}

• Order of functions and modifiers

Declare modifiers before functions and don’t mix them.

contracts/src/core/BatcherPaymentService.sol:L159-L180

function _authorizeUpgrade(
    address newImplementation
) internal override onlyOwner {}

// MODIFIERS
modifier onlyBatcher() {
    require(
        msg.sender == BatcherWallet,
        "Only Batcher can call this function"
    );
    _;
}

function checkMerkleRootAndVerifySignatures(
    bytes32[] calldata leaves,
    bytes32 batchMerkleRoot,
    SignatureData[] calldata signatures,
    uint256 feePerProof
) public {
    uint256 numNodesInLayer = leaves.length / 2;
    bytes32[] memory layer = new bytes32[](numNodesInLayer);

Description

The AlignedLayerServiceManagerStorage contract defines the Service Manager storage layout without implementing contract logic. It’s not intended for standalone deployment but isn’t declared abstract, allowing potential misuse. This violates the principle of designing contracts for their intended purpose and could lead to unexpected behavior if deployed independently.

Examples

contracts/src/core/AlignedLayerServiceManagerStorage.sol:L5-L20

contract AlignedLayerServiceManagerStorage {
    struct BatchState {
        uint32 taskCreatedBlock;
        bool responded;
        address batcherAddress;
    }

    /* STORAGE */
    mapping(bytes32 => BatchState) public batchesState;

    // Storage for batchers balances. Used by aggregator to pay for respondToTask
    mapping(address => uint256) internal batchersBalances;

    // storage gap for upgradeability
    uint256[48] private __GAP;
}

Recommendation

Modify the contract declaration to include the ‘abstract’ keyword, changing it to: abstract AlignedLayerServiceManagerStorage. This will prevent direct deployment of the contract and clearly indicate its intended use as a base contract for inheritance.

Description

The following source units are imported but not referenced in the contract:

• AlignedLayerServiceManager unused IPauserRegistry, unused Pausable

contracts/src/core/AlignedLayerServiceManager.sol:L4-L6

import {Pausable} from "eigenlayer-core/contracts/permissions/Pausable.sol";
import {IPauserRegistry} from "eigenlayer-core/contracts/interfaces/IPauserRegistry.sol";

• AlignedLayerServiceManagerStorage.sol should inherit interface IServiceManager

contracts/src/core/AlignedLayerServiceManagerStorage.sol:L3

import "eigenlayer-middleware/interfaces/IServiceManager.sol";

Recommendation

Remove unused imports and implement the IServiceManager interface in AlignedLayerServiceManagerStorage. Conduct a thorough review of all contracts to identify and eliminate any other unused imports or missing interface implementations.

Description

The AlignedLayerServiceManager contract extends the ServiceManagerBase contract by adding public interfaces. To ensure proper contract interaction and adherence to best practices, it is recommended to define an interface contract, IAlignedLayerServiceManager, that outlines these public methods. The AlignedLayerServiceManager contract should then inherit from this interface.

contracts/src/core/AlignedLayerServiceManager.sol:L18-L22

contract AlignedLayerServiceManager is
    ServiceManagerBase,
    BLSSignatureChecker,
    AlignedLayerServiceManagerStorage
{

Recommendation

Define an interface contract IAlignedLayerServiceManager that includes the public methods exposed by AlignedLayerServiceManager. Ensure that AlignedLayerServiceManager inherits from this interface to formalize the contract’s API and enhance code clarity and interaction with external contracts.

Description

Consider using the best type available in the function arguments and declarations instead of accepting address and performing low-level calls or later casting it to the correct type.

• address AlignedLayerServiceManager should be AlignedLayerServiceManager serviceManager

contracts/src/core/BatcherPaymentService.sol:L34-L36

// STORAGE
address public AlignedLayerServiceManager;
address public BatcherWallet;