feat: split AllocationManager

[0xClandestine]

Motivation:

The AllocationManager contract was hitting the 24KB bytecode size limit, which would have blocked deployment. We needed a solution to reduce the contract size while maintaining backwards compatibility with existing integrations that call view functions on AllocationManager.

Modifications:

• Created a new SplitContractMixin that uses a fallback to delegate unmatched function calls via delegatecall to a secondary contract
• Split AllocationManager into two contracts: the main contract handles state-mutating operations, while AllocationManagerView handles all read-only view functions
• Both contracts inherit from AllocationManagerStorage to share the same storage layout, enabling the view contract to read from the main contract's storage
• Updated AllocationManager constructor to accept and store the AllocationManagerView address
• Modified all deployment scripts (devnet, local, integration) to deploy both proxies and implementations
• Updated ExistingDeploymentParser, test harnesses, and unit/integration tests to work with the split architecture

Result:

• The AllocationManager is now about ~ 4.8KB under the 24KB limit with room to grow.
• The AllocationManagerView contract has 16KB of available space for future view functions.

TODO: Add a ci check (I don't have privs but the code is written locally)

1. Get list of all contracts ending in View.sol in the repo.
2. Use forge inspect abi and check all mutability fields == view|pure.
3. Basic search over the file to see if sstore or delegatecall is used on a for additional sanity.

[0xrajath]

Can we get some tests here to ensure that only view functions can be delegate called? And revert otherwise.

[0xClandestine]

Bit of an odd thing to test, since the compiler explicitly does this.

[nadir-akhtar]

Tests for this would actually fail. If the delegate implements a non-view function, it can be called with delegateView successfully. _delegateView does not enforce that delegate calls do not modify state; it's more of a suggestion that the underlying function doesn't modify state.

If you're curious, try plugging the below code block into SplitContractMixin.t.sol locally, and run forge t SplitContractMixin.t.sol. You'll see that test_setValue passes, indicating that setValue (which is a view function and uses delegateView) is able to execute a state-modifying operation.

    // rest of SplitContractMixinTest above

    function setValue(uint) public view returns (uint result) {
        _delegateView(address(delegate));
        result;
    }

    function test_setValue() public {
        (bool success, bytes memory data) = address(this).call(abi.encodeWithSelector(this.setValue.selector, 100));
        assertTrue(success);

        (bool success, bytes memory data) = address(this).call(abi.encodeWithSelector(this.getValue.selector));
        assertTrue(success);
        uint result = abi.decode(data, (uint));
        assertEq(result, 100);
    }
}

// Mock contract to test delegation
contract Delegate is Test {
    uint value;

    function getValue() public view returns (uint result) {
        return value;
    }

    function setValue(uint _value) public {
        value = _value;
    }
}

[0xClandestine]

@nadir-akhtar We're talking about different things here, @0xrajath was suggesting adding tests to confirm that if we send calldata with a selector that's unknown to the view contract it reverts. I'm saying it's standard solidity behavior for a contract to revert if the functions unrecognized.

[nadir-akhtar]

Split looks like a solid start! Especially with the layout at <storage slot> syntax making this super convenient.

A few notes on lingering TODOs, as well as some room for clarification in our docs on _delegateView allowing for delegatecall effects that may change state if the delegate contract implements such a function.

[nadir-akhtar]

Tests for this would actually fail. If the delegate implements a non-view function, it can be called with delegateView successfully. _delegateView does not enforce that delegate calls do not modify state; it's more of a suggestion that the underlying function doesn't modify state.

If you're curious, try plugging the below code block into SplitContractMixin.t.sol locally, and run forge t SplitContractMixin.t.sol. You'll see that test_setValue passes, indicating that setValue (which is a view function and uses delegateView) is able to execute a state-modifying operation.

    // rest of SplitContractMixinTest above

    function setValue(uint) public view returns (uint result) {
        _delegateView(address(delegate));
        result;
    }

    function test_setValue() public {
        (bool success, bytes memory data) = address(this).call(abi.encodeWithSelector(this.setValue.selector, 100));
        assertTrue(success);

        (bool success, bytes memory data) = address(this).call(abi.encodeWithSelector(this.getValue.selector));
        assertTrue(success);
        uint result = abi.decode(data, (uint));
        assertEq(result, 100);
    }
}

// Mock contract to test delegation
contract Delegate is Test {
    uint value;

    function getValue() public view returns (uint result) {
        return value;
    }

    function setValue(uint _value) public {
        value = _value;
    }
}

[nadir-akhtar]

Looks good! Much needed improvement to our contract codesize 💪

[ypatil12]

LGTM, left one non-blocking comment

[ypatil12]

Can we also slash here just for completeness to write to every (I think?) slot

[0xClandestine]

will do in semver removal pr for sake of avoiding collecting approvals again

[0xrajath]

+1