Update MAP + add validation to assetDataUtils #1964
Conversation
| @@ -90,7 +90,10 @@ contract MultiAssetProxy is | |||
| // offset to assetData. | |||
|
|
|||
| // Load offset to `assetData` | |||
| let assetDataOffset := calldataload(4) | |||
| let assetDataOffset := add(calldataload(4), 4) | |||
There was a problem hiding this comment.
This was a good call. It really cleaned up the buffer arithmetic 👍
There was a problem hiding this comment.
This looks good for the most part. It does seem like we can have more stringent input validation on the calldata and the nestedAssetData though. After reviewing a couple times, I don't see any glaring issues with either, but it might be good to include a few more sanity checks.
| nestedAssetDataOffset, | ||
| add(nestedAssetDataElementOffset, 100) | ||
| ) | ||
| ) |
There was a problem hiding this comment.
It seems like there is a gas efficiency reason for not asserting that nestedAssetDataContentsStart + nestedAssetDataLen <= assetDataEnd, but I wonder if it would be worth it to include it just to ensure that the nestedAssetData is in 1:1 correspondence with the data that is actually sent.
There was a problem hiding this comment.
In general I'm all for extra validation, but I primarily want to get this exact version merged since it is the one deployed to mainnet.
abandeali1
force-pushed
the
feature/contracts/consistentErrorCodes
branch
from
19b24df to
05d50b6
Compare
Description
This includes the MAP code that is currently deployed to mainnet.
Testing instructions
Types of changes
Checklist:
[WIP]if necessary.