Using `returndatasize()` in assembly.

[remedcu]

This PR closes #734 by directly using the returndatasize() instead of storing it in memory and retrieving it later. ~Accessing returndatasize() is always cheaper than mstore and multiple mload's~.

On checking further, it was found that there are no differences in terms of gas usage for transactions (extra tests were added for MultiSend to verify), while 1 byte is saved during deployment (checked using codesize).

Note: Benchmark failing in this PR is expected: Source

[coveralls]

Pull Request Test Coverage Report for Build 8187280342

Details

• 0 of 0 changed or added relevant lines in 0 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage remained the same at 94.397%

---

Totals
Change from base Build 7970315636:	0.0%
Covered Lines:	396
Relevant Lines:	404

---

💛 - Coveralls

[mmv08]

I'm comparing two benchmarks: this pr, latest commit on the main branch

It doesn't seem that every interaction has become cheaper. Some are cheaper, and some are more expensive. Not sure if the change is worth it.

[mmv08]

is always cheaper than mstore and multiple mload's.

I don't think the returndatasize is stored in memory, I think it's placed on the stack.

[remedcu]

I'm comparing two benchmarks: this pr, latest commit on the main branch

It doesn't seem that every interaction has become cheaper. Some are cheaper, and some are more expensive. Not sure if the change is worth it.

I concur 👍🏾 Maybe this PR could say: Checked, but not implemented? (cc @nlordell)

is always cheaper than mstore and multiple mload's.

I don't think the returndatasize is stored in memory, I think it's placed on the stack.

Yes, it is. Source: https://www.evm.codes/#3d?fork=shanghai

[nlordell]

Wow, this is slightly concerning... AFAICT, the Proxy - chain specific creation benchmark does not make use of the MultiSend at all! It just executes a Safe transaction.

This means that assembly code in one contract is impacting code generation for an unrelated contract. If we can confirm this is true, I would report this to the Solidity team, as it seems like extremely unexpected behaviour to me.

[nlordell]

The other possibility are calldata bytes going from 0 -> non-0 for non-deterministic things (the addresses have changed, so there is a possibility of more or less 0-bytes in the calldata).

It looks like none of the benchmarks use the MultiSend* contracts or the testing DelegateCaller contract, so in theory it should not affect the benchmarks at all.

I think we should change the benchmarking to output gas with and without the calldata costs, so we can see if there are changes in calldata 0/non-0 bytes or actual gas characteristic changes.

With this in mind, I think there is still some more investigation work for this issue and we shouldn't close it just yet.

[remedcu]

PR Description updated based on the findings of transaction gas usage and overall codesize.

[nlordell]

For some additional context, the reason there is no change is that the modified code only affects the revert path, and not the usual execution path.

Additionally, this PR modified the benchmarking, this is because we found that benchmarking was using random saltNumbers, this led to unpredictability of gas usage, as the random Safe address would sometimes have extra 0s and sometimes not, which would impact the amount of gas we pay for calldata.