Omniscia Euler Finance Audit

BeaconProxy Code Style Findings

BeaconProxy Code Style Findings

BPY-01C: Repetitive Value Literal

TypeSeverityLocation
Code StyleBeaconProxy.sol:L11, L40

Description:

The linked value literal is repeated across the codebase multiple times.

Example:

src/GenericFactory/BeaconProxy.sol
11uint256 constant MAX_TRAILING_DATA_LENGTH = 128;

Recommendation:

We advise it to be set to a constant variable instead, optimizing the legibility of the codebase.

In case the constant has already been declared, we advise it to be properly re-used across the code.

Alleviation (fb2dd77a6ff9b7f710edb48e7eb5437e0db4fc1a):

The statement within the BeaconProxy::constructor properly utilizes the MAX_TRAILING_DATA_LENGTH constant that already existed, optimizing the maintainability and legibility of the codebase.

BPY-02C: Unconditional Appendment of Metadata

Description:

The BeaconProxy::fallback function will append the four metadata slots of the contract unconditionally to the delegated payload, instead transmitting a subset of it in the delegatecall instruction.

Example:

src/GenericFactory/BeaconProxy.sol
45fallback() external payable {
46 address beacon_ = beacon;
47 uint256 metadataLength_ = metadataLength;
48 bytes32 metadata0_ = metadata0;
49 bytes32 metadata1_ = metadata1;
50 bytes32 metadata2_ = metadata2;
51 bytes32 metadata3_ = metadata3;
52
53 assembly {
54 // Fetch implementation address from the beacon
55 mstore(0, IMPLEMENTATION_SELECTOR)
56 let result := staticcall(gas(), beacon_, 0, 4, 0, 32)
57 if iszero(result) {
58 returndatacopy(0, 0, returndatasize())
59 revert(0, returndatasize())
60 }
61 let implementation := mload(0)
62
63 // delegatecall to the implementation with trailing metadata
64 calldatacopy(0, 0, calldatasize())
65 mstore(calldatasize(), metadata0_)
66 mstore(add(32, calldatasize()), metadata1_)
67 mstore(add(64, calldatasize()), metadata2_)
68 mstore(add(96, calldatasize()), metadata3_)
69 result := delegatecall(gas(), implementation, 0, add(metadataLength_, calldatasize()), 0, 0)
70 returndatacopy(0, 0, returndatasize())
71
72 switch result
73 case 0 { revert(0, returndatasize()) }
74 default { return(0, returndatasize()) }
75 }
76}

Recommendation:

As the variables involved are immutable, we advise a dedicated BeaconProxy implementation per slots required to be devised which would optimize the gas cost involved in relaying all payloads to the proxy.

Alleviation (fb2dd77a6f):

The following alleviation chapter is misconceived as the immutable aspect of the variables was ignored. Kindly consult the next alleviation chapter for an up-to-date evaluation of the optimization proposed.

The Euler Finance team evaluated this exhibit and specified that they do not envision the gas savings to justify the legibility decrease.

The gas savings acquired by the recommended optimization are significant, and will reduce the number of SLOAD operations by the number of unused metadata elements in addition to optimizing the function's overall memory usage.

The in-memory statically sized array recommendation advised as "potential" is inadequate, however, the for loop based mstore operations that are advised would correctly reflect the aforementioned reduction in gas.

As a final clarification and as the exhibit's original description details, the code appends the four metadata slots of the contract unconditionally to the delegated payload, instead transmitting a subset of it in the delegatecall instruction.

In other words, the memory location from 0 onwards will redundantly include the metadataX_ items that are ultimately not transmitted by the delegatecall instruction.

We advise the exhibit's optimization to be revisited based on the elaborations we have supplied.

Alleviation (0f2192ac81):

Our original recommendation was improperly defined as it assumed that each variable was located in storage rather than being immutable.

All immutable variables are inaccessible in assembly blocks rendering an optimization under the same implementation impossible to achieve.

We proposed a dedicated BeaconProxy implementation to be introduced instead per number of immutable slots required which would permit the appendment of metadata to be optimally performed.

The Euler Finance team evaluated the gas savings that stem from such an approach, and did not consider them to outweigh the operational burden of maintaining multiple implementations and properly deploying each one.

As such, we consider this exhibit properly acknowledged.