Implement ERC1155Proxy and StaticCallProxy in Solidity #1963
Conversation
| uint256[] memory values, | ||
| bytes memory data | ||
| ) = abi.decode( | ||
| assetData.sliceDestructive(4, assetData.length), |
There was a problem hiding this comment.
I'm surprised this works with assetData being calldata and not memory. Is it cloning assetData to memory at the time of this call?
There was a problem hiding this comment.
I believe calldata gets copied to memory when you pass it into any internal function (sliceDestructive in this case). That might not be implemented for things that require ABIv2 encoding, though (or maybe just structs?).
| ); | ||
|
|
||
| // Execute staticcall | ||
| (bool success, bytes memory returnData) = staticCallTarget.staticcall(staticCallData); |
There was a problem hiding this comment.
Should we be doing an extcodesize() check to safeguard people from accidentally calling EOAs? Could be a problem if people expect a keccak(bytes(0)) result.
At the very least, we should add a test that confirms that calling an EOA results in keccak(bytes(0)).
There was a problem hiding this comment.
I will add a test for this, but it feels a bit excessive to me to add an extcodesize or extcodehash check here. You really have to know the interface of and trust the staticCallTarget beforehand in order to use this.
Description
ERC1155ProxyandStaticCallProxyin SolidityStaticCallProxycannot inheritIAssetProxybecause itstransferFromfunction is marked asview.assetDatais a multiple of 32. However, any lengths less than required should fail (TODO: add tests for this)abi.decode. When multiple offsets point to the same location in calldata (i.e ifvalues==ids) then the decoded output is also copied to the same location in memory. This means we cannot modifyvaluesdirectly, or we may end up modifyingidsordataas well.slicevssliceDestructivehere? I opted forsliceDestructivebecauseassetDatais not reused, it is cheaper, and the implementation is arguably simpler.TODO: gas benchmarking
Testing instructions
Types of changes
Checklist:
[WIP]if necessary.