_authorizeUpgrade() seems to be called twice when upgrading contract. Is this a bug or a feature?

Hi there,

I was experimenting with UUPS upgradeability and doing so, implementing some sort of version counter for my contract upgrades that would be automatically incremented whenever I upgrade my contract (provided that the call to increment the version always stays the same from one upgrade to the other). And I noticed that the version would be bumped more times than the number of upgrades I was pushing. So I write a couple of contracts to see what was going on:

ContractV1.sol

pragma solidity ^0.8.0;

import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
import "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol";

contract ContractV1 is Initializable, UUPSUpgradeable {

	uint256 public constructorCounter;
	uint256 public initializeCounter;
	uint256 public authorizeUpgradeCounter;

	/// @custom:oz-upgrades-unsafe-allow constructor
	constructor() initializer {
		constructorCounter+=1;
	}

	function initialize() public initializer {
		initializeCounter+=1;
	}

	function _authorizeUpgrade(address) internal override {
		authorizeUpgradeCounter+=1;
	}

}

ContractV2.sol

pragma solidity ^0.8.0;

import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
import "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol";

contract ContractV2 is Initializable, UUPSUpgradeable {

	uint256 public constructorCounter;
	uint256 public initializeCounter;
	uint256 public authorizeUpgradeCounter;

	/// @custom:oz-upgrades-unsafe-allow constructor
	constructor() initializer {
		constructorCounter+=10;
	}

	function initialize() public initializer {
		initializeCounter+=10;
	}

	function _authorizeUpgrade(address) internal override {
		authorizeUpgradeCounter+=10;
	}

}

ContractV3.sol

pragma solidity ^0.8.0;

import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
import "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol";

contract ContractV3 is Initializable, UUPSUpgradeable {

	uint256 public constructorCounter;
	uint256 public initializeCounter;
	uint256 public authorizeUpgradeCounter;

	/// @custom:oz-upgrades-unsafe-allow constructor
	constructor() initializer {
		constructorCounter+=100;
	}

	function initialize() public initializer {
		initializeCounter+=100;
	}

	function _authorizeUpgrade(address) internal override {
		authorizeUpgradeCounter+=100;
	}

}

And I wrote a little test to deploy ContractV1, then upgrade it to ContractV2 and ContractV3 successively:

testUpgradeability.js

let factory1 = await ethers.getContractFactory('ContractV1');
let proxy = await upgrades.deployProxy(factory1, { kind: 'uups' });
await proxy.deployed();

console.log("============ BEFORE UPGRADES ============")
console.log("constructorCounter = " + (await proxy.constructorCounter()).toNumber())
console.log("initializeCounter = " + (await proxy.initializeCounter()).toNumber())
console.log("authorizeUpgradeCounter = " + (await proxy.authorizeUpgradeCounter()).toNumber())

let factory2 = await ethers.getContractFactory('ContractV2');
proxy = await upgrades.upgradeProxy(proxy.address, factory2);

console.log("=========== AFTER 1ST UPGRADE ===========")
console.log("constructorCounter = " + (await proxy.constructorCounter()).toNumber())
console.log("initializeCounter = " + (await proxy.initializeCounter()).toNumber())
console.log("authorizeUpgradeCounter = " + (await proxy.authorizeUpgradeCounter()).toNumber())

let factory3 = await ethers.getContractFactory('ContractV3');
proxy = await upgrades.upgradeProxy(proxy.address, factory3);

console.log("=========== AFTER 2ND UPGRADE ===========")
console.log("constructorCounter = " + (await proxy.constructorCounter()).toNumber())
console.log("initializeCounter = " + (await proxy.initializeCounter()).toNumber())
console.log("authorizeUpgradeCounter = " + (await proxy.authorizeUpgradeCounter()).toNumber())

And this is what I am getting out:

============ BEFORE UPGRADES ============
constructorCounter = 0
initializeCounter = 1
authorizeUpgradeCounter = 0
=========== AFTER 1ST UPGRADE ===========
constructorCounter = 0
initializeCounter = 1
authorizeUpgradeCounter = 11
=========== AFTER 2ND UPGRADE ===========
constructorCounter = 0
initializeCounter = 1
authorizeUpgradeCounter = 121

This tells me 3 things:

  1. The constructors are never called, which is expected.
  2. The initialize function is called once during deployment, and never after that, which is also expected.
  3. The _authorizeUpgrade seems to not be called during deployment. However, during an upgrade, it seems to be called on both the latest contract before the upgrade and the new contract that is being pushed. Is this a bug or is there a logical explanation to this?

(On a side note, I already had this intuition of a multiple call to _authorizeUpgrade when I tried to add the initializer modifier to it, and I all the sudden I could not push upgrade anymore because "Initializable: contract is already initialized".)

_authorizeUpgrade is called twice as part of what we call a "rollback test" to ensure that the upgrade mechanism is not broken by an upgrade.

This is likely going to change for the next release because we're reviewing this rollback test mechanism, but you currently have to work assuming that _authorizeUpgrade will be called twice.

Thanks @frangio. For the moment I'll adjust my upgrade functions and hardcode the new values rather than incrementing them. Do you have a rough timeline for this change to be applied?

Possibly 1.5 months from now there will be a new release candidate.