search

code-423n4 / 2021-10-ambire-findings

0 stars 0 forks source link

Cache array length in for loops can save gas #26

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Handle

WatchPug

Vulnerability details

Reading array length at each iteration of the loop takes 6 gas (3 for mload and 3 to place memory_offset) in the stack.

Caching the array length in the stack saves around 3 gas per iteration.

Instances include:

Ivshti commented 2 years ago

resolved in https://github.com/AmbireTech/adex-protocol-eth/commit/059e4fc2217f5894d4f6e2f064931daf961e2947

GalloDaSballo commented 2 years ago

No harm in applying the suggestion

However, from reviewing other contests, reading from calldata has the same cost of reading from memory The information in calldata is read without paying extra gas to store it in memory

Another has disputed the finding and I agree with them

That said, caching the value brings more readability, also the extra cost is 3 gas

Notice that this applies only to calldata arguments When reading length from storage, you 100% wanna cache that

GalloDaSballo commented 2 years ago

For all the cases above, the argument is calldata

I'd highly recommend the sponsor run their own tests, however I believe this finding to be superfluous / ineffective