WIP - Update smart contracts to Solidity 0.5 #123
Conversation
| @@ -1,4 +1,4 @@ | |||
| pragma solidity ^0.4.18; | |||
| pragma solidity ^0.5.0; | |||
There was a problem hiding this comment.
Does this attack still work with 0.5? As I understand the last note here, STATICCALL should prevent it.
There was a problem hiding this comment.
Good catch! Have you tested it?
There was a problem hiding this comment.
No, I just did the 0.4 version and then read the docs...
There was a problem hiding this comment.
Good point, thank you! I'll definitely test it
There was a problem hiding this comment.
Whilst the Elevator contract compiles, to get the test to pass, need to change the interface to no longer be view due to STATICCALL
function isLastFloor(uint) external returns (bool);
There was a problem hiding this comment.
Great, thank you Andrew!
There was a problem hiding this comment.
This level is solvable on new solidity with a view function:
contract Hogwarts is Building {
function isLastFloor(uint floor) view public returns (bool) {
return floor == Elevator(msg.sender).floor();
}
function businessAsUsual(address elevator) public {
Elevator lift = Elevator(elevator);
lift.goTo(lift.floor() + 1);
}
}There was a problem hiding this comment.
Nice work @NieDzejkob.
I will get updated.
|
Hi, I am considering necessary changes in Fallback level (and probably it will repeat in others). I need to make the
The second option requires fewer changes but it makes the smart contract less readable, especially for beginners. What do you think @martriay ? |
|
@paulinablaszk That's interesting... Very annoying that Do any of the levels use functionality of |
|
@frangio No, only |
|
@paulinablaszk Then I'd say we go for the simplest thing:
Note that this also implies adding I actually think it's a good thing for the levels to be self-contained. On the other hand, it's also good to show players/learners that OpenZeppelin exists and is helpful for development. 😃 But |
|
@frangio Could you please take a look at my last commit - I would like to make sure that such a convention will be ok before I make changes in more levels ;) |
contracts/levels/Fallback.sol
Outdated
| contributions[msg.sender] = 1000 * (1 ether); | ||
| } | ||
|
|
||
| function owner() public view returns (address payable) { |
There was a problem hiding this comment.
Instead of writing the manual owner() getter, it would be ok to just use a public variable directly. I think it's preferable because it allows the player to focus on the rest of the code. We don't do that in OpenZeppelin because we want to have internal variables, but that's not a concern here.
There was a problem hiding this comment.
Ok, thank you very much!
Co-Authored-By: Francisco Giordano <frangio.1@gmail.com>
|
My suggestion for a Fallout level is in the forum: |
|
I haven't managed to find a way to convert |
|
As far as I can work out, Locked needs to be removed/retired as a contract with the vulnerability can't be compiled. See the write up in the forum: |
|
Locked level should be retired. Checked with author |
|
AlienCodex can be modified removing the first part of the level: |
@paulinablaszk once Locked level is retired, then all truffle tests should pass. Assume we need to delete the following:
and update:
Once this is done the tests should pass. Next step after this is to update the contract deployment. 😄 |
|
@abcoathup I've just deleted all 'locked' files |
|
Hi @paulinablaszk the build now passes!! Great work. There is an issue with deploying the Ethernaut contracts that I am seeing if I can track down. It looks like it is an issue with package versions. |
|
Hi @paulinablaszk |
|
After deploying the contracts, when I try to run Ethernaut with I assume we need to transpile Failed to compile.
Error in ./~/truffle-contract/lib/execute.js
Module parse failed: /mnt/c/Users/andre/Documents/projects/forum/ethernaut/node_modules/truffle-contract/lib/execute.js Unexpected token (117:39)
You may need an appropriate loader to handle this file type.
SyntaxError: Unexpected token (117:39)
@ ./~/truffle-contract/lib/contract.js 4:16-36
Error in ./~/truffle-contract/lib/contract/constructorMethods.js
Module parse failed: /mnt/c/Users/andre/Documents/projects/forum/ethernaut/node_modules/truffle-contract/lib/contract/constructorMethods.js Unexpected token (36:8)
You may need an appropriate loader to handle this file type.
SyntaxError: Unexpected token (36:8)
@ ./~/truffle-contract/lib/contract/index.js 2:22-53
Error in ./~/truffle-contract/lib/utils.js
Module parse failed: /mnt/c/Users/andre/Documents/projects/forum/ethernaut/node_modules/truffle-contract/lib/utils.js Unexpected token (313:8)
You may need an appropriate loader to handle this file type.
SyntaxError: Unexpected token (313:8)
@ ./~/truffle-contract/lib/contract/properties.js 1:14-33
Error in ./~/truffle-contract/~/scryptsy/lib/scrypt.js
Module parse failed: /mnt/c/Users/andre/Documents/projects/forum/ethernaut/node_modules/truffle-contract/node_modules/scryptsy/lib/scrypt.js Unexpected token (8:6)
You may need an appropriate loader to handle this file type.
SyntaxError: Unexpected token (8:6)
@ ./~/truffle-contract/~/scryptsy/lib/index.js 2:15-34
Error in ./~/truffle-contract/~/scryptsy/lib/utils.js
Module parse failed: /mnt/c/Users/andre/Documents/projects/forum/ethernaut/node_modules/truffle-contract/node_modules/scryptsy/lib/utils.js Unexpected token (52:6)
You may need an appropriate loader to handle this file type.
SyntaxError: Unexpected token (52:6)
@ ./~/truffle-contract/~/scryptsy/lib/scryptSync.js 5:4-22 |
|
I wasn't able to reproduce that error. I checked out this branch, and followed the instructions for "Running locally" in the README. Upon running |
Hi @frangio, We cannot deploy the contracts locally without making the following changes: Which once made means that the app can't be started due to a compile error. Details of my setup process below:
In Attempting to deploy the contracts Need to make the following modifications to be able to deploy Can then deploy the contracts But fails when running |
|
Hi, @abcoathup I'm getting the same error. I also think that it looks like a problem with handling async functions by babel. I was trying to use some other options but without success by now |
|
Okay, I was able to reproduce it. I'm not sure how to fix it. 😅 |
|
Hmm, I've got it working after following @abcoathup last message. I didn't even have errors when runing Could this be because there are some new versions in some dependencies? |
|
Hi @obernardovieira, I am still getting the same error. Using the following branch: To enable |
|
This work was included in #151 |
Issue #117
The goal is to make all smart contracts to compile using Solidity 0.5.0.
Due to a large number of changes, I add this as Work in Progress.
What have been done
zeppelin-soliditytoopenzeppelin-soliditycreateInstancein levels factories (forminstancetoaddress(instance)”addressmember.call()and.delegatecall()arguments.call(), .delegatecall()sha3tokeccak256StandardTokentoDetailedERC20Next steps
Make all the contracts compile correctly:
Level factories to do:
Fallback
King
Re-entrancy
Denial
Update attack contracts
STATUS UPDATE - TESTS
"MUSEUM"?
memorymodifier added to struct insideregisterfunction)