Skip to content

Instantly share code, notes, and snippets.

@hav-noms
Created January 30, 2019 04:25
Show Gist options
  • Save hav-noms/ada2f6442d039498aa7e40c8cb76d56c to your computer and use it in GitHub Desktop.
Save hav-noms/ada2f6442d039498aa7e40c8cb76d56c to your computer and use it in GitHub Desktop.
Authors Response to smart contract Audit
https://gist.github.com/syncikin/0d027dd8bc9d0263b401290fee00f09e
5.3.1
No action taken
Synthetix Response:
Our plan for upgrading the XDR rate is to introduce a replacement XDR Synth with its own rate, then allow people to exchange between these tokens using existing mechanisms. FeePool would need to handle balances in both for a transition period.
5.3.2 Incorrect Exchange Fee Rate Validation
Resolved
5.4.1 Excessive Owner Capabilities
Synthetix Response:
In order to support the upgradability of the contracts the owner requires these privileges, once the system is stable these capabilities will be removed.
5.4.2 Design Comments
All issues resolved except the following.
- Length of currencyKeys should be required to match the length of hdrParticipants
Synthetix Response:
We have decided not to action this feedback because even if we specified an opening rate for each one of the XDR currencies, they can always go stale, which would impact that rate in any case. The oracle protects against this, but we can’t enforce this in the constructor, so we felt like it would add additional complexity to the deployment without providing a benefit.
- Impractical to remove a Nomin once it is in use
Synthetix Response:
The idea behind this code was to give us a way to remove a Synth as long as it was only being tested, once people hold it, we no longer have the ability to remove it. The code prevents removal of a synth from the network without a larger contract upgrade.
- Use of implicit integer sizes in contract implementations
Synthetix Response:
While the size of default uint could change between solidity versions, it would be a significant breaking change. At this stage we have quite a few of these around the code base, so rather than changing it, we believe the best approach is to leave it for the time being, changing it to a more specific type if required (e.g. if uintchanges in a breaking solidity version).
- Inexact Solidity compiler version used
Synthetix Response:
As the SafeMath.sol contract is a dependency managed via NPM, it’d be best to feed this back up to the OpenZeppelin team. I suspect they would want to manage it more comprehensively for their whole library. The rest of our contracts use a hard 0.4.25 peg, so it wouldn’t be possible to compile and deploy these without using 0.4.25, so we didn't see any specific action to take other than to suggest feeding this back to OpenZeppelin.
- Unaudited compiler version
Synthetix Response:
Noted, we’ll keep an eye out for audits / future versions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment