Skip to content

Instantly share code, notes, and snippets.

@ccashwell
Created January 2, 2018 20:14
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save ccashwell/e86b3aecdb2d4ffe4091ae61f475bb53 to your computer and use it in GitHub Desktop.
Save ccashwell/e86b3aecdb2d4ffe4091ae61f475bb53 to your computer and use it in GitHub Desktop.
Audit of the Project Wyvern Platform's Solidity Smart Contracts

Audit: Project Wyvern

Completed on January 2, 2018 @ 7358a6c065

Abstract

Project Wyvern is a decentralized item exchange platform (the “Platform”) comprised of three primary components: (1) The WYV Token; (2) The Wyvern Exchange; and (3) The Wyvern Decentralized Autonomous Organization (DAO). This goal of this audit has been to ascertain the overall security of the Platform. In pursuit of that goal, this author has reviewed the Solidity source code of all relevant Smart Contracts, deployed and tested a live version of the Platform on the Ethereum Main Network.

This audit is presented as a Code Review which reflects a deep analysis of the Smart Contracts that comprise the Platform. It is broken into sub-sections, each of which relate to a single Smart Contract. Each sub-section includes a link to the Smart Contract’s Solidity source code as it stood at the time of review, as well as review commentary and recommendations.

Important Note: Project Wyvern is heavily dependent on OpenZeppelin, which is beyond the scope of this audit. No inherited functionality has been reviewed or considered as part of this audit.

Code Review

Purpose:

Defines WYV (Project Wyvern Token) as an ERC-20 token with a precision of 10^18 and maximum supply of 20,000,000,000,000,000,000,000,000 tokens.

Comments:

During instantiation, the deploying party can specify the maximum number of tokens to be available for redemption in exchange for Unspent Transaction Outputs (UTXOs) from Bitcoin and its forks. Redemptions are issued at a rate of 100,000,000,000 tokens per 1 Satoshi.

Recommendations:

None.

Purpose:

Defines the Project Wyvern Exchange, whose fees are configured upon instantiation.

Comments:

This component is completely reliant on Exchange.sol and cannot be separated from that dependency for the purpose of review.

Recommendations:

None.

Purpose:

Defines the Project Wyvern DAO’s operating parameters.

Comments:

The initial configuration only accepts an address; all other parameters are initially defined by the constants provided within the source code (token holders with >= 0.1% of supply are Shareholders; quorum is reached at >= 10% of supply; debate period is initially 3 days).

Recommendations:

None.

Purpose:

Records migrations to the DAO's ruleset.

Comments:

None.

Recommendations:

None.

Purpose:

Defines a mechanism for delaying the release of minted tokens until a predetermined event.

Comments:

The delayed release mechanism is being used to allow the Platform’s contracts to be deployed and then wired together after instantiation. Upon instantiation, the deploying party is set as the temporary administrator. The temporary administrator’s address is then to be replaced by the address of the deployed DAO contract once the DAO contract has been deployed. Once that replacement occurs, the minted coins will be released to the DAO and an event broadcasted.

Recommendations:

None.

Purpose:

Facilitates the exchange of Unspent Transaction Outputs (UTXOs) of Bitcoin and its forks for WYV tokens.

Comments:

This module only allows basic v1 Bitcoin addresses. Given the wide availability and usage of latter generation address formats, this could introduce challenges for individuals intending to redeem UTXOs from non-legacy addresses. The documentation for #pubKeyToBitcoinAddress is contradictory: the function signature allows a flag to signal whether the public key is compressed, the function description implies that it accepts only uncompressed keys, and the parameter description states that pubKey is expected to be compressed.

Recommendations:

Consider adding support for non-legacy Bitcoin addresses. Revise the NatSpec for #pubKeyToBitcoinAddress to reflect that the function accepts either compressed or uncompressed keys.

Purpose:

Defines the interface for a simple P2P escrow with support for dispute resolution.

Comments:

None.

Recommendations:

None.

Purpose:

Defines the Platform’s item exchange functionality.

Comments:

The item exchange supports three types of sale: fixed (items sell for a fixed price), English-style auction (bids increase until auction ends) and Dutch-style auction (initial price is set by seller and progressively reduced by decay factor until a buyer is found).

Recommendations:

None.

Purpose:

Defines the functionality for a DAO with support for delegated voting and board members.

Comments:

Token owners (Shareholders) can delegate their votes to other Shareholders, which puts the tokens in a “locked” state within the DAO such that they cannot be withdrawn. Delegation may be revoked at any time, including after the delegate has cast a vote but before a Proposal is finalized. Although the impact is insignificant, the #newProposal function increments the proposals array’s length by 1 but does not use the incremented value as the new proposal ID (the value is post-incremented, but incrementing is not necessary). Setting numerous properties on a blank-state object is costlier than replacing the blank-state object with a new, fully qualified instantiation of that object, though the difference may not be substantial. A Shareholder’s voting capacity is not counted until the proposal is finalized, which ensures the weight of their vote is reduced by the number of delegated tokens during the count. The #executeProposal function does not verify the sender, but the impact is near nil as the proposal must have already passed and therefore is reasonably executable by anyone.

Recommendations:

Remove the unnecessary increment in #newProposal. Consider revising #newProposal to minimize the cost of executing the function. Consider rejecting votes that have already been delegated at the time of request in #vote to reduce unnecessary storage overhead.

Purpose:

Facilitates the receipt of WYV tokens and Ether.

Comments:

None.

Recommendations:

None.

Conclusions

Overall, the Platform is well written using Solidity Compiler ^0.4.18. While heavy reliance on third-party libraries can potentially introduce unexpected behavior, OpenZeppelin is well-reviewed and the Project Wyvern developers appear to have taken great care to thoroughly document and test those dependencies. Various recommendations have been made throughout the review process, though none have been found to be critical. This author’s opinion is that, pending review of the recommendations herein, the Project Wyvern platform is ready for production use.

@protinam
Copy link

protinam commented Jan 2, 2018

Thanks for completing the audit.

Quick question: the "proposals.length++" is not unnecessary; it serves to resize the proposal array (with extremely unintuitive syntax specific to Solidity) - see documentation reference - in light of that documentation, do you think the usage is OK or would you recommend another method?

@ccashwell
Copy link
Author

@protinam Because the array size was not explicitly defined during initialization, it is indeed unnecessary to resize it in this way. I would recommend pushing a newly instantiated Proposal to the array, rather than resizing it and altering the blank-state object.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment