Full Smart Contract Upgradeability Discussion

filip · April 30, 2019, 8:19am

Welcome to the discussion thread about this lecture section. Here you can feel free to discuss the topic at hand and ask questions.

site · May 7, 2019, 3:10am

I just started this section and watched the “What was wrong with our simple Proxy?” video and I’m wondering if you can do mappings of mappings? In other words the question in my mind that I suspect may be answered eventually is how to handle adding an accounts in a contract upgrade.

filip · May 7, 2019, 8:09am

Yes you can do mapping to mapping. Not a problem.

site · May 8, 2019, 3:35am

Sweet.

In lecture https://ivanontech.teachable.com/courses/541331/lectures/10064285 you mentioned near the end of the video that you would include a link to understand the low level assembly better. I’m not sure where to find the link.

filip · May 8, 2019, 3:08pm

Sorry about that. I will update the lecture, here is the link: https://solidity.readthedocs.io/en/v0.5.2/assembly.html

Dodd · May 11, 2019, 3:17pm

@filip, it would be nice to have the demo code running within in the truffle unit test. At least, it’s worth having mention that truffle migration scripts are not the best place for the demo code (querying the dogs count, console logging, etc.)

** in my opinion, of course

Dodd · May 11, 2019, 4:02pm

@filip, one more question. I came across some articles about the “selector clash” (selector collision) vulnerability.

It would be nice to have

Some coverage of this attack in the course
Some explanation (maybe here as a reply) on “Can the proxy contract showcased in this course be exploited by the selector clash”.
(Unfortunately, I still need some help figuring this out. I’ll let you know if I get any insights, though).

UPD

– If the caller is the admin of the proxy, the proxy will not delegate any calls, and will only answer management messages it understands.
– If the caller is any other address, the proxy will always delegate the call, no matter if it matches one of the proxy’s own functions.

Seems like this is the recommendation to avoid the “selector clash” exploits.

Dodd · May 11, 2019, 3:41pm

P.S. Thanks for great content, @filip, @ivan . I’m really enjoying this security course. You’ve found a right balance for it being not too complicated and giving a good structure to student’s prior knowledge (if any).

filip · May 12, 2019, 3:28pm

Thank you very much for your feedback and your kind words about the course. We are happy to hear that you enjoyed it.

About the migration vs test. Yes, I agree with you. But I didn’t want to introduce a new layer of complexity, the testing suite of truffle, in this course. There will be a truffle testing course coming later this year.

Thanks for sending that Fascinating attack angle indeed, there are so many weird loopholes and exploits in smart contracts that we have to be aware of.

I could definitely update the course in the future with that in mind. Seems like it’s more of an exploit that can be carried out by a malicious contract creator rather than anyone just calling our functions. So the owner could potentially hide functionality behind weird function names. But it’s not something that makes our proxy contracts potential targets.

I see that you already found the solution to avoid the selector clash, great!

cyberspec · June 23, 2019, 2:45pm

Filip, thanks for the content and assistance!

I have two questions: First to help me understand the ‘truffle’ part vs the actual smart contract part a bit more. In the migration script for the proxy and dog contracts; I noticed we don’t use the ‘deployer.deploy’ method of deploying, but in other examples we do.

For example:

Deploying without deployer method
const proxy = await Proxy.new(dogs.address);

vs

Deploying with deployer method
deployer.deploy(Dogs).then(function(){
return deployer.deploy(Proxy,Dogs.address);
});

When using the deployer method, it gives us the details of the smart contract when we perform a migration. Is this all, is there any other benefit or reason to use one vs the other? Truffle adds some confusion for me when really trying to understand smart contracts, because it has so many of it’s own quirks.

/////////
My second question is another possibly Truffle related confusion. When we deploy an updated contract and run the ‘proxy.update’ function; why can’t we use ‘proxy.getNumberof Dogs()’ as is intended? Isn’t that what the actual dapps would do? I tried this in remix as well, and it also doesn’t work as intended. Will I have to deploy something to the testnet to verify it works as intended, or is there something gaping I’m missing in my understanding?

Thanks in advance!

filip · June 27, 2019, 1:05pm

Good questions @cyberspec. I should have clarified this in the video.

First question: Both the Proxy.new() and the deployer.deploy() will deploy the contract. So you can use both. The different is that deploy() will look for an already deployed instance of the contract and use that one if it exists.

If I remember the videos correctly we might at some point have 2 dog contracts deployed in the same file, in which we would have had to use new() in order to get a new instance of every contract. If we were to use deployer.deploy() truffle might have given us an already deployed instance of the dogcontract. I hope it makes sense. You can read a little bit more about it here: https://ethereum.stackexchange.com/questions/42094/should-i-use-new-or-deployed-in-truffle-unit-tests

I don’t fully understand your second question. Don’t we use the getNumberOfDog function in the videos even after update? If I look at the file we created it looks like we use it as intended? Or am I misunderstanding you?

Link to the file. https://github.com/filipmartinsson/Smart-Contract-Security/blob/master/Upgradeable-Advanced/migrations/2_deploy_proxy.js

cyberspec · June 27, 2019, 3:22pm

Ahh thanks Filip, that makes sense for the deployer.deploy first question.

The second one is probably my confusion with proxy contracts. So in the migration script, the proxyDog variable has to be set to the latest dog contract (e.g. proxydog = dogsupdated.at(proxy.address) for example). I understand this is still hitting the proxy fallback function, but we have to manually tell it.

There’s no way to access the fall back automatically? I mean if I’m writing a dapp, I have to update the dapp whenever the dogs contract is upgraded. Vs having it happening automatically behind the scenes.

Or am I confusing something as I think I am…?

Thanks!

V/r,

-KJ

filip · June 28, 2019, 11:54am

Thank you for clarifying. Let me see if I understand you question. You’re wondering if there is a way to for a contract to automatically update itself when we deploy a new version of it?

The simple answer to that is no. Solidity and Ethereum is built to be immutable, to be unchangeable. It’s not built with update functionality. So in order to circumvent that and provide upgradeability, we need to build special software, like a proxy in order to achieve that.

Before we update the structure looks like this.

Caller -> Proxy -> Contract A

If we deploy a new contract (Contract B) that is suppose to for example fix a bug in Contract A. That will just be a standalone contract like any other, there is nothing that connects it to Contract A just because we deploy it. We need to update the proxy and tell it to point to a new contract.

Caller -> Proxy -> Contract B

This does not remove contract A in any way. Contract A will still exist on the blockchain, but not connected to the proxy contract, which we interact with.

But you could of course automate this yourself with a truffle script. But there is nothing in the blockchain that replaces smart contracts with new ones.

Let me know if I understood your question correct or if I answered something completely different

cyberspec · June 28, 2019, 8:58pm

Thanks Filip,

I’m tracking and understand the idea you are relaying here (it is kind of what I was trying to explain in my own example). I’m trying to go one step deeper without getting things confused.

When we run “proxy.upgrade” we are “pointing” the fallback to the new upgraded contract, right? I’m basically saying, why isn’t that enough and just call “proxy.getNumberOfDogs”

The fallback is supposed to route any function that wasn’t handled, correct? In our case, before we run upgrade, it points to the old contract, after we run upgrade it points to the new contract.

The explanation of fallback states that any non-existent function would route to the fallback function. And our fallback is supposed to handle passing that information to whatever contract address we specified/updated to.

So if the proxy.getNumberOfDogs() function doesn’t exist, shouldn’t it pass to the fallback and thereby get handled by our ‘assembly’ which would pass to whichever contract address we specified in the delegate call? (I know this doesn’t actually work… just saying why not)

Is there a way to directly call the fallback function internally?

Hope that makes sense; if not no worries I’m going to try and do some more research and testing and I’ll post my findings and let ya know!

filip · July 7, 2019, 10:55pm

Thank you for clarifying. Sorry for the late response, I’ve been taking some summer vacation.

“When we run “proxy.upgrade” we are “pointing” the fallback to the new upgraded contract, right? I’m basically saying, why isn’t that enough and just call “proxy.getNumberOfDogs””.

That is correct, yes. That is all we need to do. We don’t need to do anything else from the contracts perspective. Everything we do after that is for Truffle internally. Truffle won’t let us call a function if it doesn’t believe it exists. So that’s why I say in the video that we need to fool truffle to believe that function exists.

So to illustrate, we can’t call proxyDog.getNumberOfDogs() straight away. Because in truffle’s world, the proxyDog jacascript object contains no such function. So we would get a Javascript error that basically says that proxyDog contains no such function. But that is only a client error and has nothing to do with the smart contract.

In order to fix this, we fool truffle by running.

  var proxyDog = await Dogs.at(proxy.address);

We run the same thing after we have updated our proxy. In this case, we could do without it. But as a principle we wan’t to do it, because we might have added new functions in our DogsUpdated contract which truffle needs to know of.

I hope that made it clearer. Everything apart from proxy.upgrade (which is a solidity function) is meant to enable us to use truffle to call our fallback function correctly. It’s a frontend/client fix.

Eric · December 19, 2019, 6:42pm

I’m having this issue, can’t solve it, can’t go on… and dont know where to look more than google.

Error: Returned values aren’t valid, did it run Out of Gas? at PromiEvent (C:\Users\E\AppData\Roaming\npm\node_modules\truffle\build\webpack:\packages\contract\lib\promievent.js:9:1)
at TruffleContract.getNumberOfDogs (C:\Users\E\AppData\Roaming\npm\node_modules\truffle\build\webpack:\packages\contract\lib\execute.js:120:1)
at module.exports (C:\WEB\eth\proxyContract\migrations\2_deploy_proyx.js:10:35)
at process._tickCallback (internal/process/next_tick.js:68:7)

filip · December 23, 2019, 7:12am

Can you post your code here? And the truffle code you are executing? I need that in order to help you debug.

Eric · December 23, 2019, 11:54am

Sure!.
I have the error when I add this line in the 2_deploy_proxy.js;
var nrOfDogs = await proxyDog.getNumberOfDogs();

Dogs.sol

pragma solidity 0.5.12;

import “./Storage.sol”;

contract Dogs is Storage {

modifier onlyOwner(){

    require(msg.sender==owner);

    _;

}

constructor() public {

    owner = msg.sender;

}

function getNumberOfDogs() public view returns(uint256){

    return _uintStorage["Dogs"];

}



function setNumberOfDogs(uint256 _toSet) public {

    _uintStorage["Dogs"] = _toSet;

}

}

Eric · December 23, 2019, 11:50am

migrations.sol

pragma solidity >=0.4.21 <0.6.0;

contract Migrations {

address public owner;

uint public last_completed_migration;

constructor() public {

owner = msg.sender;

}

modifier restricted() {

if (msg.sender == owner) _;

}

function setCompleted(uint completed) public restricted {

last_completed_migration = completed;

}

function upgrade(address new_address) public restricted {

Migrations upgraded = Migrations(new_address);

upgraded.setCompleted(last_completed_migration);

}

Eric · December 23, 2019, 11:52am

proxy.sol
pragma solidity 0.5.12;

import “./Storage.sol”;

contract Proxy is Storage {

address currentAddress;

constructor (address _currentAddress) public {

    currentAddress = _currentAddress;

}

function upgrade(address _newAddress) public {

    currentAddress = _newAddress;

}

//fallback function

//redirect to current address

function() payable external { 

    address implementation;

    require(currentAddress !=address(0));

    bytes memory data = msg.data;

    assembly {

        let result := delegatecall(gas, implementation, add(data, 0x20), mload(data), 0, 0)

        let size := returndatasize

        let ptr := mload(0x40)

        returndatacopy(ptr, 0, size)

        switch result

        case 0 {revert(ptr, size)}

        default {return(ptr, size)}

    }

}

}

storage.sol
pragma solidity 0.5.12;

contract Storage {

mapping (string =>uint256) _uintStorage;

mapping (string =>address) _addressStorage;

mapping (string => bool) _boolStorage;

mapping (string => string) _stringStorage;

mapping (string => bytes4) _bytesStorage;

address public owner;

bool public _initialized;

}

2_deploy_proxy.js
const Dogs = artifacts.require(‘Dogs’);

const Proxy = artifacts.require(‘Proxy’);

module.exports = async function(deployer, network, accounts){

const dogs = await Dogs.new();

const proxy = await Proxy.new(dogs.address);

var proxyDog = await Dogs.at(proxy.address);

await proxyDog.setNumberOfDogs(10);

var nrOfDogs = await proxyDog.getNumberOfDogs();

//var nrOfDogs = await proxyDog.getNumberOfDogs();

console.log(proxyDog);

//console.log(nrOfDogs.toNumber());

}