Open code423n4 opened 3 years ago
Will attempt this modification and place results on reduced costs here
Edit: After experimenting some, it doesn't seem there's an easy spot to use this as it pushes us into stack issues in update valset and elsewhere I've tried it.
Yeah we tried this as well and it didn't work, oh well.
reopening as per judges assessment as "primary issue" on findings sheet
Handle
hrkrshnn
Vulnerability details
Use
calldata
instead ofmemory
for function parametersIn some cases, having function arguments in
calldata
instead ofmemory
is more optimal.Consider the following generic example:
In the above example, the dynamic array
arr
has the storage locationmemory
. When the function gets called externally, the array values are kept incalldata
and copied tomemory
during ABI decoding (using the opcodecalldataload
andmstore
). And during the for loop,arr[i]
accesses the value in memory using amload
. However, for the above example this is inefficient. Consider the following snippet instead:In the above snippet, instead of going via memory, the value is directly read from
calldata
usingcalldataload
. That is, there are no intermediate memory operations that carries this value.Gas savings: In the former example, the ABI decoding begins with copying value from
calldata
tomemory
in a for loop. Each iteration would cost at least 60 gas. In the latter example, this can be completely avoided. This will also reduce the number of instructions and therefore reduces the deploy time cost of the contract.In short, use
calldata
instead ofmemory
if the function argument is only read.Note that in older Solidity versions, changing some function arguments from
memory
tocalldata
may cause "unimplemented feature error". This can be avoided by using a newer (0.8.*
) Solidity compiler.Examples from codebase
Some parameters in examples given above are later hashed. It may be beneficial for those parameters to be in
memory
rather thancalldata
.