Different behaviors after upgrading smart contract using Proxy pattern

Hi,

I’m trying to understand how Upgradeable contract using Proxy pattern works. From my understanding, upgradeable contract using proxy pattern splits regular contract into three separated parts: Proxy, Implementation and State. When upgrading, only new implementation is upgraded whereas its state is unchanged. Proxy contract will ‘delegate call’ to a new implementation whenever a contract’s function is called. However, this behavior is not correct when I test some basic contracts using truffle and OpenZeppelin Upgrades Plugins. In case 1 below, old implementation can also modify state variables while it is failed to do so in case 2. Would you please explain why it has this different behavior?

:1234: Code to reproduce

Environment:
Truffle v5.3.0 (core: 5.3.0)
Solidity v0.5.16 (solc-js)
Node v15.12.0
Web3.js v1.2.9

  • Case 1: ‘Upgrade2’ contract has all functions of ‘Upgrade1’ contract and has an additional function ‘getBalanceOf()’
pragma solidity >=0.5.0 <0.8.0;

contract Upgrade1 {
    mapping(address => uint) balances;

    function deposit() external payable {
        balances[msg.sender] = msg.value;
    }

    function getBalance() external view returns (uint) {
        return address(this).balance;
    }
}

contract Upgrade2 {
    mapping(address => uint) balances;

    function deposit() external payable {
        balances[msg.sender] = msg.value;
    }

    function getBalance() external view returns (uint) {
        return address(this).balance;
    }

    function getBalanceOf(address _acc) external view returns (uint) {
        return balances[_acc];
    }
}
  • Case 2: A function ‘getBalance()’ is modified in ‘Upgrade4’ contract
pragma solidity >=0.5.0 <0.8.0;

contract Upgrade3 {
    mapping(address => uint) balances;

    function deposit() external payable {
        balances[msg.sender] = msg.value;
    }

    function getBalance() external view returns (uint) {
        return address(this).balance;
    }
}

contract Upgrade4 {
    mapping(address => uint) balances;

    function deposit() external payable {
        balances[msg.sender] = msg.value;
    }

    function getBalance(address _addr) external view returns (uint) {
        return balances[_addr];
    }
}

Migrations:

const {deployProxy, upgradeProxy} = require('@openzeppelin/truffle-upgrades');
const Upgrade1 = artifacts.require('Upgrade1');
const Upgrade2 = artifacts.require("Upgrade2");
const Upgrade3 = artifacts.require('Upgrade3');
const Upgrade4 = artifacts.require("Upgrade4");

module.exports = async function(deployer) {
    var upgrade1 = await deployProxy(Upgrade1, {deployer});
    await upgradeProxy(upgrade1.address, Upgrade2, {deployer});
    var upgrade3 = await deployProxy(Upgrade3, {deployer});
    await upgradeProxy(upgrade3.address, Upgrade4, {deployer});
}

Test:

const { deployProxy, upgradeProxy } = require("@openzeppelin/truffle-upgrades");

const Upgrade1 = artifacts.require("Upgrade1");
const Upgrade2 = artifacts.require("Upgrade2");
const Upgrade3 = artifacts.require('Upgrade3');
const Upgrade4 = artifacts.require("Upgrade4");

contract('Upgradeable contracts test', () => {
    let up1, up2, up3, up4, accounts;
    before(async () => {
        up1 = await deployProxy(Upgrade1);
        up2 = await upgradeProxy(up1.address, Upgrade2);
        up3 = await deployProxy(Upgrade3);
        up4 = await upgradeProxy(up3.address, Upgrade4);
        accounts = await web3.eth.getAccounts();
    });

    it('Contracts', async () => {
        console.log("Contract 1: ", up1.address);
        console.log("Contract 2: ", up2.address);
        console.log("Contract 3: ", up3.address);
        console.log("Contract 4: ", up4.address);
    })

    it('Deposit to Upgrade1 Contract', async() => {
        await up1.deposit({from: accounts[1], value: 1000});
        await up1.deposit({from: accounts[2], value: 5000});
        var balanceCont = await up1.getBalance();
        console.log('Contract Balance: ', web3.utils.BN(balanceCont).toNumber());
    });

    it('Upgrade2 Contract - Success', async() => {
        await up2.deposit({from: accounts[1], value: 1000});
        await up2.deposit({from: accounts[2], value: 5000});
        var balanceCont = await up2.getBalance();
        console.log('Contract Balance: ', web3.utils.BN(balanceCont).toNumber());
    });

    it('Deposit to Upgrade3 Contract', async() => {
        await up3.deposit({from: accounts[1], value: 1000});
        await up3.deposit({from: accounts[2], value: 5000});
        var balanceCont = await up3.getBalance();
        console.log('Contract Balance: ', web3.utils.BN(balanceCont).toNumber());
    });

    it('Upgrade4 Contract - Success', async() => {
        await up4.deposit({from: accounts[1], value: 1000});
        await up4.deposit({from: accounts[2], value: 5000}); 
        var balanceCont = await up4.getBalance(accounts[2]);
        console.log('Contract Balance: ', web3.utils.BN(balanceCont).toNumber());
    });
});

Results:

Contract: Upgradeable contracts test
Contract 1:  0x7146590Cc1354f48E2ab8bf09b29e0fE68454EcD
Contract 2:  0x7146590Cc1354f48E2ab8bf09b29e0fE68454EcD
Contract 3:  0x6C258784467Ab08f4563512412F18842637a2c5e
Contract 4:  0x6C258784467Ab08f4563512412F18842637a2c5e
    ✓ Contracts
Contract Balance:  6000
    ✓ Deposit to Upgrade1 Contract (252ms)
Contract Balance:  12000
    ✓ Upgrade2 Contract - Success (236ms)
    1) Deposit to Upgrade3 Contract
    > No events were emitted
Contract Balance:  5000
    ✓ Upgrade4 Contract - Success (248ms)


  4 passing (4s)
  1 failing

  1) Contract: Upgradeable contracts test
       Deposit to Upgrade3 Contract:
     Error: Returned error: VM Exception while processing transaction: revert
      at Context.<anonymous> (test/2_Upgrade.js:42:37)
      at runMicrotasks (<anonymous>)
      at processTicksAndRejections (node:internal/process/task_queues:94:5)

It’s because in Case 2

function getBalance() external view returns (uint) {
        return address(this).balance;
    }

is changed to

function getBalance(address _addr) external view returns (uint) {
        return balances[_addr];
    }

The state has changed so you can’t do it in your tests like you want

it('Deposit to Upgrade3 Contract', async() => {

The code doesn’t know where to go when it tries to deposit. It thinks it’s at it’s old state, but when it tries it sees a new state and errors out.

In

before(async () => {
        up1 = await deployProxy(Upgrade1);
        up2 = await upgradeProxy(up1.address, Upgrade2);
        up3 = await deployProxy(Upgrade3);
        up4 = await upgradeProxy(up3.address, Upgrade4);
        accounts = await web3.eth.getAccounts();
    });

You are doing this before every test, so contract 3 is upgraded when you do this test.

Thanks for your reply @Tsushima_Yoshiko. First of all, I’m using ‘before’ not ‘beforeEach’. Hence, it should be called only one time. As I understood, Proxy contract will ‘delegate call’ to a new implementation since an instance, being called, is a Proxy contract. So, I don’t understand why Proxy contract acts differently in these two cases. In Case 1, ‘Upgrade2’ contract append functions in the ‘Upgrade1’ contract while in case 2, ‘Upgrade4’ overwrite functions in the ‘Upgrade3’. If this is an expected behavior, I just wonder how Proxy contract detects when append / overwrite occurs

Oh yeah you’re right, but perhaps somehow doing all of this in before will cause that to happen regardless?

The upgrade has already happened because you upgraded it in the before
meaning by the time you go to Upgrade 3’s

function getBalance() external view returns (uint) {
        return address(this).balance;
    }

It’s state has already changed and no longer exists in the way it was before.

The proxy will delegate the call, not the actual contract. I think up3 is still targeting the old implementation. It might be a proxy, but it’s still the old version of the proxy that is targeting Contract 3. Does that make sense?

up4 = await upgradeProxy(up3.address, Upgrade4);
up4 has the Proxy that is pointing to the new implementation.

You’re right. I’ve figured it out what I was wrong. Thanks

const { deployProxy, upgradeProxy } = require("@openzeppelin/truffle-upgrades");

const Upgrade1 = artifacts.require("Upgrade1");
const Upgrade2 = artifacts.require("Upgrade2");
const Upgrade3 = artifacts.require('Upgrade3');
const Upgrade4 = artifacts.require("Upgrade4");

contract('Upgradeable contracts test', () => {
    let up1, up2, up3, up4, accounts;
    before(async () => {
        up1 = await deployProxy(Upgrade1);
        up2 = await upgradeProxy(up1.address, Upgrade2);
        up3 = await deployProxy(Upgrade3);
        up4 = await upgradeProxy(up3.address, Upgrade4);
        accounts = await web3.eth.getAccounts();
    });

    it('Deposit to Upgrade1 Contract', async() => {
        await up1.deposit({from: accounts[1], value: 1000});
        await up1.deposit({from: accounts[2], value: 5000});
        var balanceCont = await up1.getBalance();
        console.log('Contract Balance: ', web3.utils.BN(balanceCont).toNumber());
    });

    it('Upgrade2 Contract - Success', async() => {
        await up2.deposit({from: accounts[1], value: 1000});
        await up2.deposit({from: accounts[2], value: 5000});
        var balanceCont = await up2.getBalance();
        console.log('Contract Balance: ', web3.utils.BN(balanceCont).toNumber());
        //  Even though Balance of `Upgrade2` is 12000 
        //  Balance of Account 2 is still 5000 
        var balanceAcc = await up2.getBalanceOf(accounts[2]);
        console.log('Balance of Account 2: ', web3.utils.BN(balanceAcc).toNumber());
    });

    it('Deposit to Upgrade3 Contract', async() => {
        await up3.deposit({from: accounts[1], value: 1000});
        await up3.deposit({from: accounts[2], value: 5000});
        // var balanceCont = await up3.getBalance();
        // console.log('Contract Balance: ', web3.utils.BN(balanceCont).toNumber());
    });

    it('Upgrade4 Contract - Success', async() => {
        await up4.deposit({from: accounts[1], value: 1000});
        await up4.deposit({from: accounts[2], value: 5000}); 
        var balanceCont = await up4.getBalance(accounts[2]);
        console.log('Contract Balance: ', web3.utils.BN(balanceCont).toNumber());
    });
});

Result:

Contract Balance:  6000
    ✓ Deposit to Upgrade1 Contract (148ms)
Contract Balance:  12000
Balance of Account 2:  5000
    ✓ Upgrade2 Contract - Success (244ms)
    ✓ Deposit to Upgrade3 Contract (149ms)
Contract Balance:  5000
    ✓ Upgrade4 Contract - Success (157ms)