Conversation
| erc3156/=lib/ERC3156/ | ||
| dss-interfaces/=lib/dss-interfaces/ | ||
| openzeppelin/=lib/openzeppelin-contracts/ | ||
| @openzeppelin/=lib/openzeppelin-contracts/ |
There was a problem hiding this comment.
Why do we need node convention here?
| IJoin baseJoin = getJoin(vault.baseId); | ||
| if (base < 0) baseJoin.join(vault.owner, uint128(-base)); | ||
| if (base > 0) baseJoin.exit(to, uint128(base)); | ||
| else baseJoin.exit(to, uint128(base)); |
There was a problem hiding this comment.
What if base is 0? That wasn't being checked before but is now?
There was a problem hiding this comment.
On 325 we are checking that base != 0
| // ---- Asset and debt management ---- | ||
|
|
||
| /// @dev Move collateral and debt between vaults. | ||
| function stir( |
There was a problem hiding this comment.
Why was stir moved exactly?
There was a problem hiding this comment.
It doesn't update the global debt counters when ilkId != baseId. It could be fixed, but at the same time we haven't ever used it, so better to just reduce the attack surface.
| } | ||
| } | ||
|
|
||
| contract FlashLoanEnabledStateTests is FlashLoanEnabledState { |
There was a problem hiding this comment.
Why remove all these tests exactly?
There was a problem hiding this comment.
Since, flash loan was removed the tests are removed as well.
| ladle.give(vaultId, admin); | ||
| } | ||
|
|
||
| function testOnlyOwnerCouldMove() public { |
There was a problem hiding this comment.
Why remove so many tests from this file? Is it because they're already tested elsewhere?
There was a problem hiding this comment.
The tests related to stir were removed.
…dprotocol/vault-v2 into fix/variable-rate-postaudit
alcueca
left a comment
alcueca
left a comment
There was a problem hiding this comment.
Just cosmetic changes required.
3 participants
This PR contains the fixes based on the reports the auditors submitted. The fixes were first implemented in the auditor-specific repos and then after their approval were aggregated and implemented in this repo. You can find the reports and PRs here:
We need to review if all the accepted changes in the above reports were implemented in this one.