code-423n4 / 2023-06-lukso-findings

3 stars 1 forks source link

Gas Optimizations #87

Open code423n4 opened 1 year ago

code423n4 commented 1 year ago

See the markdown file with the details of this report here.

c4-pre-sort commented 1 year ago

minhquanym marked the issue as high quality report

CJ42 commented 1 year ago

Most of the suggested Gas Optimisations can be confirmed except for the following:

G-01: yes could make sense to put the data location as calldata in the interfaces and also in the implementation contracts + test the change in gas in practice. But we have to be careful with this, as calldata can reduce composability, because these functions are marked as public and could also be called internally inside a function if the contract is inherited.

For instance, not possible to construct an array in memory internally inside a function and then call this function if it takes calldata as parameter. This will not compile). To be discussed

G-02: we are planning to split our contracts between two repositories (the standard version vs the proxy version). We will be able to do this optimisation once we have removed the LSPNCore contracts. Until then, we cannot do anything.

G-06, G-07, G-09, G-21 might be discarded as they do not improve readability.

For G-21 in particular, hardcoded literal are less readable than type(uintN).max, and more likely to be error prone when written (e.g: forgetting a byte 0xff when writing the literal:

For instance below, there is only 15 x 0xff

MAX_UINT128 = 0xffffffffffffffffffffffffffffff

What we could do is have readable constants like below and use them across the code, as something in between:

uint128 constant MAX_LSP5_ARRAY_LENGTH_ALLOWED = type(uint128).max

For the gas optimisations related to assembly (G-13 and G-23), we are not in favour of using assembly for gas optimisation, as it makes the code less readable and potentially less safe.

c4-sponsor commented 1 year ago

CJ42 marked the issue as sponsor confirmed

c4-judge commented 1 year ago

trust1995 marked the issue as grade-a

c4-judge commented 1 year ago

trust1995 marked the issue as selected for report