It's possible to lose the ownership under specific circumstances.
Because an human error it's possible to set a new invalid owner. When you want to change the owner's address it's better to propose a new owner, and then accept this ownership with the new wallet.
The following lines require a contract as an argument, in some of them, once the value is set, it cannot be changed again, so it is mandatory to check if these values are as expected.
But recently solidity released a new version with important Bugfixes:
The first one is related to ABI-encoding nested arrays directly from calldata. You can find more information here.
The second bug is triggered in certain inheritance structures and can cause a memory pointer to be interpreted as a calldata pointer or vice-versa. We also have a dedicated blog post about this bug.
Apart from these, there are several minor bug fixes and improvements.
The transferNotionalFee method of VaultTracker does not check that the argument overflows, as other methods in the same contract do. The logic must be unified.
+ if (a > oVault.notional) { revert Exception(31, a, oVault.notional, address(0), address(0)); }
// remove notional from its owner
oVault.notional -= a;
5. Emit event for important state changes
It's important to emit events when the governance change important values in the contract, in order to be more open and transparent.
For example, when a contract is paused or unpaused like in the following lines:
The code that contains "open todos" reflects that the development is not finished and that the code can change a posteriori, prior release, with or without audit.
Low
1. Lack of ACK during owner change
It's possible to lose the ownership under specific circumstances.
Because an human error it's possible to set a new invalid owner. When you want to change the owner's address it's better to propose a new owner, and then accept this ownership with the new wallet.
Affected source code:
2. Lack of EOAs check
The following lines require a contract as an argument, in some of them, once the value is set, it cannot be changed again, so it is mandatory to check if these values are as expected.
Affected source code:
3. Outdated compiler
The pragma version used is:
But recently solidity released a new version with important Bugfixes:
The first one is related to ABI-encoding nested arrays directly from calldata. You can find more information here.
The second bug is triggered in certain inheritance structures and can cause a memory pointer to be interpreted as a calldata pointer or vice-versa. We also have a dedicated blog post about this bug.
Apart from these, there are several minor bug fixes and improvements.
The minimum required version should be 0.8.14
4. Unify errors
The
transferNotionalFee
method ofVaultTracker
does not check that the argument overflows, as other methods in the same contract do. The logic must be unified.5. Emit event for important state changes
It's important to emit events when the governance change important values in the contract, in order to be more open and transparent.
For example, when a contract is paused or unpaused like in the following lines:
6. Ensure that
ecrecover
returns is notaddress(0)
The method
ecrecover
returnsaddress(0)
when the signature is wrong, so if a user useaddress(0)
as amaker
the return will betrue
.Affected source code:
Non critical
7. Open TODO
The code that contains "open todos" reflects that the development is not finished and that the code can change a posteriori, prior release, with or without audit.
Affected source code: