Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update Solidity compiler #430

Merged
merged 1 commit into from
Aug 16, 2019
Merged

Update Solidity compiler #430

merged 1 commit into from
Aug 16, 2019

Conversation

filippsen
Copy link
Contributor

Description of the Change

  • Update Solidity compiler to version 0.5.10

  • Update OpenZeppelin package to version 2.3.0

  • Rebuild solc iframe to make sure it synchronizes with latest changes

  • Update pragma statement version for all template contract files

  • Update template contracts to use explicit memory modifier where
    applicable

  • Change cryptopizza generateRandomDna function from external to public

  • Update ERC721 contract interface in Crypto Pizza template

  • Update all address comparisons in example contracts to involve
    explicit type casting

  • Change constant keyword usage in template contracts in favor of the
    view keyword

  • Explicitly add the payable modifier to receiver/beneficiary address in
    Raise To Summon template

  • Explicitly add address type casting to this keyword in all contract
    templates

  • Change the usage of var keyword in favor of explicit data type in all
    contract templates

  • Update Ownable contract in Voting template to use latest OpenZeppelin
    implementation

  • Change test compiler settings to use new compiler

  • Change Raise to Summon test to expect 0 errors instead

  • Update test references to match contract source modifications and
    compiler version

Template files diff

cryptopizza.zip

diff -r ./old/cryptopizza/contracts/CryptoPizza.sol ./new/cryptopizza/contracts/CryptoPizza.sol
1c1
< pragma solidity ^0.4.25;
---
> pragma solidity ^0.5.10;
41c41
<     function _createPizza(string _name, uint _dna)
---
>     function _createPizza(string memory _name, uint _dna)
54c54
<     function createRandomPizza(string _name)
---
>     function createRandomPizza(string memory _name)
62c62
<     function generateRandomDna(string _str, address _owner)
---
>     function generateRandomDna(string memory _str, address _owner)
77c77
<         returns(uint[])
---
>         returns(uint[] memory)
113c113
<         external
---
>         public
125,126c125,126
<     function safeTransferFrom(address from, address to, uint256 pizzaId, bytes _data)
<         external
---
>     function safeTransferFrom(address from, address to, uint256 pizzaId, bytes memory _data)
>         public
137c137
<     function _checkOnERC721Received(address from, address to, uint256 pizzaId, bytes _data)
---
>     function _checkOnERC721Received(address from, address to, uint256 pizzaId, bytes memory _data)
265c265
<     modifier isUnique(string _name, uint256 _dna) {
---
>     modifier isUnique(string memory _name, uint256 _dna) {
293c293
< }
\ No newline at end of file
---
> }
diff -r ./old/cryptopizza/contracts/EIPs/ERC165.sol ./new/cryptopizza/contracts/EIPs/ERC165.sol
1c1
< pragma solidity ^0.4.25;
---
> pragma solidity ^0.5.10;
44c44
< }
\ No newline at end of file
---
> }
diff -r ./old/cryptopizza/contracts/EIPs/IERC165.sol ./new/cryptopizza/contracts/EIPs/IERC165.sol
1c1
< pragma solidity ^0.4.25;
---
> pragma solidity ^0.5.10;
15c15
< }
\ No newline at end of file
---
> }
diff -r ./old/cryptopizza/contracts/EIPs/IERC721.sol ./new/cryptopizza/contracts/EIPs/IERC721.sol
1c1
< pragma solidity ^0.4.25;
---
> pragma solidity ^0.5.10;
24c24
<     function safeTransferFrom(address from, address to, uint256 tokenId) external;
---
>     function safeTransferFrom(address from, address to, uint256 tokenId) public;
26,27c26,27
<     function safeTransferFrom(address from, address to, uint256 tokenId, bytes data) external;
< }
\ No newline at end of file
---
>     function safeTransferFrom(address from, address to, uint256 tokenId, bytes memory data) public;
> }
diff -r ./old/cryptopizza/contracts/EIPs/IERC721Receiver.sol ./new/cryptopizza/contracts/EIPs/IERC721Receiver.sol
1c1
< pragma solidity ^0.4.25;
---
> pragma solidity ^0.5.10;
23,24c23,24
<     function onERC721Received(address operator, address from, uint256 tokenId, bytes data) public returns (bytes4);
< }
\ No newline at end of file
---
>     function onERC721Received(address operator, address from, uint256 tokenId, bytes memory data) public returns (bytes4);
> }
diff -r ./old/cryptopizza/contracts/utils/SafeMath.sol ./new/cryptopizza/contracts/utils/SafeMath.sol
1c1
< pragma solidity ^0.4.25;
---
> pragma solidity ^0.5.10;
65c65
< }
\ No newline at end of file
---
> }

empty.zip

diff -r ./old/empty/contracts/MyContract.sol ./new/empty/contracts/MyContract.sol
1c1
< pragma solidity ^0.4.25;
---
> pragma solidity ^0.5.10;
6c6
< }
\ No newline at end of file
---
> }

erc-20-token.zip

diff -r ./old/erc-20-token/contracts/TokenERC20.sol ./new/erc-20-token/contracts/TokenERC20.sol
1c1
< pragma solidity ^0.4.25;
---
> pragma solidity ^0.5.10;
30c30
<     constructor(uint256 initialSupply, string tokenName, string tokenSymbol)
---
>     constructor(uint256 initialSupply, string memory tokenName, string memory tokenSymbol)
46c46
<         require(_to != 0x0);
---
>         require(_to != address(0x0));
95c95
< }
\ No newline at end of file
---
> }

hello.zip

diff -r ./old/hello/contracts/HelloWorld.sol ./new/hello/contracts/HelloWorld.sol
1c1
< pragma solidity ^0.4.25;
---
> pragma solidity ^0.5.10;
5,6c5,6
<     
<     constructor(string initMessage) public {
---
> 
>     constructor(string memory initMessage) public {
10c10
<     function update(string newMessage) public {
---
>     function update(string memory newMessage) public {
13c13
< }
\ No newline at end of file
---
> }

newsfeed.zip

diff -r ./old/newsfeed/contracts/NewsFeed.sol ./new/newsfeed/contracts/NewsFeed.sol
1c1
< pragma solidity ^0.4.25;
---
> pragma solidity ^0.5.10;
18c18
<         require(givenAuthor != 0x0);
---
>         require(givenAuthor != address(0x0));
23c23
<     function publish(string title, string hash)
---
>     function publish(string memory title, string memory hash)
38c38
<         constant returns (string, string)
---
>         view returns (string memory, string memory)
42c42
< }
\ No newline at end of file
---
> }

raisetosummon.zip

diff -r ./old/raisetosummon/contracts/RaiseToSummon.sol ./new/raisetosummon/contracts/RaiseToSummon.sol
4c4
< pragma solidity ^0.4.25;
---
> pragma solidity ^0.5.10;
10c10
<     address public receiver;
---
>     address payable public receiver;
22c22
<     constructor(address beneficiary, string message, uint256 secondsUntilExpiration)
---
>     constructor(address payable beneficiary, string memory message, uint256 secondsUntilExpiration)
25c25
<         require(beneficiary != 0x0);
---
>         require(beneficiary != address(0x0));
41c41
<         public
---
>         external
81c81
<         require(this.balance > 0);
---
>         require(address(this).balance > 0);
86c86
<         receiver.transfer(this.balance);
---
>         receiver.transfer(address(this).balance);
98c98
<         var value = donationData[msg.sender];
---
>         uint256 value = donationData[msg.sender];
107c107
<         constant returns (uint256)
---
>         view returns (uint256)
119c119
<         constant returns (uint256)
---
>         view returns (uint256)
124c124
< }
\ No newline at end of file
---
> }

safemath.zip

diff -r ./old/safemath/contracts/SafeMath.sol ./new/safemath/contracts/SafeMath.sol
1c1
< pragma solidity ^0.4.25;
---
> pragma solidity ^0.5.10;
65c65
< }
\ No newline at end of file
---
> }
diff -r ./old/safemath/contracts/TestContract.sol ./new/safemath/contracts/TestContract.sol
1c1
< pragma solidity ^0.4.25;
---
> pragma solidity ^0.5.10;
41c41
< }
\ No newline at end of file
---
> }

voting.zip

diff -r ./old/voting/contracts/Ownable.sol ./new/voting/contracts/Ownable.sol
1c1
< pragma solidity ^0.4.25;
---
> pragma solidity ^0.5.0;
2a3,13
> // Original file: https://raw.githubusercontent.com/OpenZeppelin/openzeppelin-contracts/master/contracts/ownership/Ownable.sol
> 
> /**
>  * @dev Contract module which provides a basic access control mechanism, where
>  * there is an account (an owner) that can be granted exclusive access to
>  * specific functions.
>  *
>  * This module is used through inheritance. It will make available the modifier
>  * `onlyOwner`, which can be applied to your functions to restrict their use to
>  * the owner.
>  */
3a15,17
>     address private _owner;
> 
>     event OwnershipTransferred(address indexed previousOwner, address indexed newOwner);
5c19,25
<     address public owner;
---
>     /**
>      * @dev Initializes the contract setting the deployer as the initial owner.
>      */
>     constructor () internal {
>         _owner = msg.sender;
>         emit OwnershipTransferred(address(0), _owner);
>     }
7,10c27,31
<     constructor() 
<         public
<     {
<         owner = msg.sender;
---
>     /**
>      * @dev Returns the address of the current owner.
>      */
>     function owner() public view returns (address) {
>         return _owner;
12a34,36
>     /**
>      * @dev Throws if called by any account other than the owner.
>      */
14c38
<         require(owner == msg.sender);
---
>         require(isOwner(), "Ownable: caller is not the owner");
18,24c42,75
<     function transferOwnership(address _newOwner) 
<         public
<         onlyOwner
<     {
<         require(_newOwner != owner);
<         require(_newOwner != address(0));
<         owner = _newOwner;
---
>     /**
>      * @dev Returns true if the caller is the current owner.
>      */
>     function isOwner() public view returns (bool) {
>         return msg.sender == _owner;
>     }
> 
>     /**
>      * @dev Leaves the contract without owner. It will not be possible to call
>      * `onlyOwner` functions anymore. Can only be called by the current owner.
>      *
>      * NOTE: Renouncing ownership will leave the contract without an owner,
>      * thereby removing any functionality that is only available to the owner.
>      */
>     function renounceOwnership() public onlyOwner {
>         emit OwnershipTransferred(_owner, address(0));
>         _owner = address(0);
>     }
> 
>     /**
>      * @dev Transfers ownership of the contract to a new account (`newOwner`).
>      * Can only be called by the current owner.
>      */
>     function transferOwnership(address newOwner) public onlyOwner {
>         _transferOwnership(newOwner);
>     }
> 
>     /**
>      * @dev Transfers ownership of the contract to a new account (`newOwner`).
>      */
>     function _transferOwnership(address newOwner) internal {
>         require(newOwner != address(0), "Ownable: new owner is the zero address");
>         emit OwnershipTransferred(_owner, newOwner);
>         _owner = newOwner;
diff -r ./old/voting/contracts/Vote.sol ./new/voting/contracts/Vote.sol
1c1
< pragma solidity ^0.4.25;
---
> pragma solidity ^0.5.10;
16c16
<     function addCandidate(string _name)
---
>     function addCandidate(string memory _name)
36c36
<     function updateCandidate(uint _index, string _updatedName)
---
>     function updateCandidate(uint _index, string memory _updatedName)
73c73
<         returns(string, uint)
---
>         returns(string memory, uint)
91c91
< }
\ No newline at end of file
---
> }

* Update Solidity compiter to version 0.5.10

* Update OpenZeppelin package to version 2.3.0

* Rebuild solc iframe to make sure it synchronizes with latest changes

* Update pragma statement version for all template contract files

* Update template contracts to use explicit memory modifier where
applicable

* Change cryptopizza generateRandomDna function from external to public

* Update ERC721 contract interface in Crypto Pizza template

* Update all address comparisons in example contracts to involve
explicit type casting

* Change constant keyword usage in template contracts in favor of the
view keyword

* Explicitly add the payable modifier to receiver/beneficiary address in
Raise To Summon template

* Explicitly add address type casting to this keyword in all contract
templates

* Change the usage of var keyword in favor of explicit data type in all
contract templates

* Update Ownable contract in Voting template to use latest OpenZeppelin
implementation

* Change test compiler settings to use new compiler

* Change Raise to Summon test to expect 0 errors instead

* Update test references to match contract source modifications and
compiler version
Copy link
Contributor

@bashlund bashlund left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✓ Confirm that Solidity 0.5.10 compiler is being used - OK!

✓ Confirm that new compiler breaks old contracts for 0.4 Solidity - Yes, BREAKS!

✓ Compile and deploy all templates on Browser network - OK!

✓ Manually run through some templates on Browser network - OK!

  • ✓ Empty
  • ✓ Hello world
  • ✓ ERC20
  • ✓ Censorship resistant newsfeed
  • ✓ Summon Nick Johnson

✓ Test to import OpenZeppelin contracts in new projects - OK!

✓ Test to use different accounts when interacting with dapps - OK!

✓ Test to use custom wallet - OK

✓ Test to deploy to public testnet and interact

  • ✓ Tested Hello world on Ropsten. - OK!

✓ Test to export Dapp as html file - OK!

  • Note: exported dapps who use metamask cannot be run as local file because metamask doesn't allow it. (can use python2 -m SimpleHTTPServer 8080.
  • Note2: dapps might have to perform web3.currentProvider.enable() for metamask to open
  • Note3: dapps who use synchronous access to web3.eth.accounts can fail, have to do asnyc call as: web3.eth.getAccounts( (err,accounts) => {...}. This goes for most of our templates.
  • ✓ Tested Hello World on Ropsten and Customer ganache-cli using different account - WORKS!

✓ Test uploading to IPFS - OK!

✓ Test downloading from IPFS - OK!

✓ Try running customer network with ganache-cli and custom wallet - OK!

  • Note: Have to lower the gas limit to about 2000000 in the chain settings then also close the deploy window before redeploying.

Final note: We might need to inform users that all their 0.4 contracts will not compile anymore and provide a link to the Solidity changelog so they can start fixing their contracts.

@filippsen
Copy link
Contributor Author

Thanks for the review!


About the compiler transition:

"compiler breaks old contracts for 0.4 Solidity"

I did not suggest anything because the compiler is supposedly very explicit about what the errors are. At first:

Loading the compiler...
/contracts/HelloWorld.sol:1:1: ParserError: Source file requires different compiler version (current compiler is 0.5.10+commit.5a6ea5b1.Emscripten.clang - note that nightly builds are considered to be strictly less than the released version
pragma solidity ^0.4.25; ...
^----------------------^

Updating the pragma line reported by the previous output, it moves onto language changes territory:

Using Solidity compiler version 0.5.10
/contracts/HelloWorld.sol:6:17: TypeError: Data location must be "memory" for parameter in function, but none was given.
constructor(string initMessage) public {
^----------------^
/contracts/HelloWorld.sol:10:21: TypeError: Data location must be "memory" for parameter in function, but none was given.
function update(string newMessage) public {
^---------------^

About running the exported dapps locally, this has also been explored in open issue #392:

Note: exported dapps who use metamask cannot be run as local file because metamask doesn't allow it. (can use python2 -m SimpleHTTPServer 8080.

From: Extra Notes section in #392 (comment)
[...] to get MetaMask notification to appear, it is required that the dapp file is served by a web server. In case the dapp is accessed via file:///..., calls to window.web3 or window.ethereum won't work.

Note2: dapps might have to perform web3.currentProvider.enable() for metamask to open
Note3: dapps who use synchronous access to web3.eth.accounts can fail, have to do asnyc call as: web3.eth.getAccounts( (err,accounts) => {...}. This goes for most of our templates.

From: Problem section in #392 (comment)
"In order to retrieve account information when using MetaMask, it is now required to ask for user's consent before direct access to the account is granted."

@filippsen filippsen merged commit 3a13b1e into production Aug 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants