View: Truncate instead of Copy

[wjmelements]

Reviewer @aarshkshah1992
Solidity memory arrays cannot increase in size once allocated, but it safe to truncate them.
Because memory gas is quadratic, the cost of this copy grows quickly with the size of the array.
By truncating the array we can avoid the copy step.

Changes

• replace array copy with array truncation

Test plan

• verified that without truncation at least one test fails

[wjmelements]

This change's diff to the .gas-snapshot, using #193:

diff --git a/.gas-snapshot b/.gas-snapshot
index 7ec0148..bd1394b 100644
--- a/.gas-snapshot
+++ b/.gas-snapshot
@@ -58,9 +58,9 @@ DepositWithPermitAndOperatorApproval:testDepositWithPermitAndOperatorApproval_Mu
 DepositWithPermitAndOperatorApproval:testDepositWithPermitAndOperatorApproval_ZeroAmount() (gas: 314923)
 ERC1967ProxyTest:testInitialSetup() (gas: 21033)
 ERC1967ProxyTest:testOwnershipTransfer() (gas: 39095)
-ERC1967ProxyTest:testUpgradeImplementation() (gas: 7603080)
+ERC1967ProxyTest:testUpgradeImplementation() (gas: 7576386)
 ERC1967ProxyTest:test_RevertWhen_TransferFromNonOwner() (gas: 37166)
-ERC1967ProxyTest:test_RevertWhen_UpgradeFromNonOwner() (gas: 7600044)
+ERC1967ProxyTest:test_RevertWhen_UpgradeFromNonOwner() (gas: 7573350)
 FeeOnTransferVulnerabilityTest:testFeeOnTransferVulnerabilityBasic() (gas: 189206)
 FeeOnTransferVulnerabilityTest:testFeeOnTransferWithDepositWithPermit() (gas: 247245)
 FeeOnTransferVulnerabilityTest:testFeeOnTransferWithDepositWithPermitAndApproveOperator() (gas: 365288)
@@ -94,10 +94,10 @@ OperatorApprovalUsageLeakTest:testOperatorLockupUsageLeakOnRailFinalization() (g
 PayeeFaultArbitrationBugTest:testLockupReturnedWithFault() (gas: 910072)
 PayeeFaultArbitrationBugTest:testLockupReturnedWithFaultReducedDuration() (gas: 2059695)
 PayeeFaultArbitrationBugTest:testLockupReturnedWithFaultTermination() (gas: 888076)
-PayeeRailsTest:testEmptyResult() (gas: 57771)
-PayeeRailsTest:testGetRailsForPayeeAndToken() (gas: 149980)
-PayeeRailsTest:testGetRailsForPayerAndToken() (gas: 146086)
-PayeeRailsTest:testRailsBeyondEndEpoch() (gas: 346932)
+PayeeRailsTest:testEmptyResult() (gas: 55199)
+PayeeRailsTest:testGetRailsForPayeeAndToken() (gas: 142479)
+PayeeRailsTest:testGetRailsForPayerAndToken() (gas: 139457)
+PayeeRailsTest:testRailsBeyondEndEpoch() (gas: 331722)
 PaymentsEventsTest:testAccountLockupSettledEvent() (gas: 615226)
 PaymentsEventsTest:testDepositRecordedEvent() (gas: 317742)
 PaymentsEventsTest:testOperatorApprovalUpdatedEvent() (gas: 128062)

It's a small improvement for small arrays but it would be larger for large arrays. There is a also a small reduction in codesize.

While these view methods are unlikely to be used during execution, if their gas usage is too large, eth_call node RPC could revert with out-of-gas. This is also a reason to avoid unbounded looping, even in view methods.

[wjmelements]

memory gas is quadratic

I'm not sure that this is the case for Filecoin but I still think this is a good change.

For Ethereum, here is the gas savings by number of results:

Results	Savings
0	643
300	310600
600	654079
900	1031309
1200	1442287
1500	1887016
1800	2365495
2100	2877724

[rvagg]

nice

[aarshkshah1992]

Learnt something new here ! Thanks @wjmelements