change memory to calldata

[molly-ting]

Changing the data location of function parameters from memory to calldata can save much gas. In the following example, test4 saves about 955 units of gas.

    function test3(uint256[] memory amounts) public {
        uint256 amount = amounts[0];
    }

    function test4(uint256[] calldata amounts) public {
        uint256 amount = amounts[0];
    }

Some variables like targets, values, calldatas in function hashProposal() in Governor.sol can also be changed to calldata, but this will change the interface, so I didn't include this in the commit. Can you have a look at this?

PR Checklist

• [ ] Tests
• [ ] Documentation
• [ ] Changeset entry (run npx changeset add)

[changeset-bot[bot]]

⚠️ No Changeset found

Latest commit: a418bd8da9a6214359c6b8fca3df96a16c8599e4

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[ernestognw]

Hey @MollyHe5,

The reason why we don't use calldata is because it can't be built during execution, so changing these functions is breaking for contracts that rely on the information being passed by memory between each other.

Improvements overall are less than 1% although I admit there are some cases in which the savings are considerable, the fact we can copy from calldata to memory but not the other way around is the reason for this design decision

[Amxx]

There might be some places where calldata is relevant, and other where its not. I'm not sure how this PR decided where to change it, but each one should be discussed separately.

Also, its a breaking change, so I'd wait for 5.0 to do that.

[ernestognw]

This is a breaking change and we've previously avoided using calldata because it reduces the current functionality (can't be called internally). I agree some places would benefit but it's a separate discussion.

Because we've just released 5.0 and this is breaking, we can reconsider for 6.0. Closing for now