[3.0] Upgrade exchange-forwarder to solidity ^0.5.5 and unpin deps #1732
Conversation
| @@ -175,7 +176,7 @@ contract MixinExchangeWrapper is | |||
| addFillResults(totalFillResults, singleFillResults); | |||
|
|
|||
| // Stop execution if the entire amount of makerAsset has been bought | |||
| uint256 makerAssetFilledAmount = totalFillResults.makerAssetFilledAmount; | |||
There was a problem hiding this comment.
Just curious, how come this change?
There was a problem hiding this comment.
Solidity now implements C99-style scoping rules for function local variables, that is, variables can only be used after they have been declared and only in the same or nested scopes. Variables declared in the initialization block of a for loop are valid at any point inside the loop.
https://solidity.readthedocs.io/en/v0.5.0/050-breaking-changes.html
| // ERC20 asset proxy: 0xf47261b. | ||
| mstore(add(assetData, 32), 0xf47261b000000000000000000000000000000000000000000000000000000000) | ||
| // Then we encode parameter 1, the token address. | ||
| mstore(add(assetData, 36), address) |
There was a problem hiding this comment.
Just a note -- when storing a payload ≤ 32 bytes we typically apply a mask to zero out the unused portion of the word. What we have here is probably okay since (i) assetData is allocated with new directly above, and (ii) historically the compiler has been very liberal with masks so it probably applies one to address. That said, neither (i) nor (ii) completely guarantee the word will be zeroed out.
There was a problem hiding this comment.
I can make this change, np.
Unpin `exchange-forwarder` deps.
…on will silently cause your constructors to not run, and a function that takes an `address` is NOT the same as one that takes an `address payable`.
…rantERC20Token.sol`
dorothy-zbornak
force-pushed
the
feature/contracts/3.0/exchange-forwarder-0.5.5
branch
from
82bf53e to
b3907b1
Compare
Correct changelog for `contracts/exchange-forwarder` after rebase.
…upin deps. Cherry picked from PR #1732.
Description
Just a simple update of the
exchange-forwardercontracts tosolidity ^0.5.5, while also unpinning its dependencies. This is a prereq for some other PRs coming down the line.This PR depends on #1736
Testing instructions
Types of changes
Checklist:
[WIP]if necessary.