Added events to verifyState and finishUpgrade

nucypher/blockchain/eth/sol/source/contracts/Issuer.sol

@@ -173,7 +173,8 @@ contract Issuer is Upgradeable {
         return totalSupply - Math.max(currentSupply1, currentSupply2);
     }
 
-    function verifyState(address _testTarget) public onlyWhileUpgrading {
+    function verifyState(address _testTarget) public {
+        super.verifyState(_testTarget);
         require(address(uint160(delegateGet(_testTarget, "token()"))) == address(token));
         require(delegateGet(_testTarget, "miningCoefficient()") == miningCoefficient);
         require(delegateGet(_testTarget, "lockedPeriodsCoefficient()") == lockedPeriodsCoefficient);
@@ -185,7 +186,8 @@ contract Issuer is Upgradeable {
         require(delegateGet(_testTarget, "totalSupply()") == totalSupply);
     }
 
-    function finishUpgrade(address _target) public onlyWhileUpgrading {
+    function finishUpgrade(address _target) public {
+        super.finishUpgrade(_target);
         Issuer issuer = Issuer(_target);
         token = issuer.token();
         miningCoefficient = issuer.miningCoefficient();

nucypher/blockchain/eth/sol/source/contracts/MinersEscrow.sol

@@ -1165,7 +1165,7 @@ contract MinersEscrow is Issuer {
         }
     }
 
-    function verifyState(address _testTarget) public onlyWhileUpgrading {
+    function verifyState(address _testTarget) public {
         super.verifyState(_testTarget);
         require(uint16(delegateGet(_testTarget, "minLockedPeriods()")) == minLockedPeriods);
         require(delegateGet(_testTarget, "minAllowableLockedTokens()") == minAllowableLockedTokens);
@@ -1211,7 +1211,7 @@ contract MinersEscrow is Issuer {
         }
     }
 
-    function finishUpgrade(address _target) public onlyWhileUpgrading {
+    function finishUpgrade(address _target) public {
         super.finishUpgrade(_target);
         MinersEscrow escrow = MinersEscrow(_target);
         minLockedPeriods = escrow.minLockedPeriods();

nucypher/blockchain/eth/sol/source/contracts/MiningAdjudicator.sol

@@ -160,7 +160,8 @@ contract MiningAdjudicator is Upgradeable {
         }
     }
 
-    function verifyState(address _testTarget) public onlyWhileUpgrading {
+    function verifyState(address _testTarget) public {
+        super.verifyState(_testTarget);
         require(address(delegateGet(_testTarget, "escrow()")) == address(escrow));
         require(SignatureVerifier.HashAlgorithm(uint256(delegateGet(_testTarget, "hashAlgorithm()"))) == hashAlgorithm);
         require(delegateGet(_testTarget, "basePenalty()") == basePenalty);
@@ -174,7 +175,8 @@ contract MiningAdjudicator is Upgradeable {
         require(delegateGet(_testTarget, "evaluatedCFrags(bytes32)", evaluationCFragHash) != 0);
     }
 
-    function finishUpgrade(address _target) public onlyWhileUpgrading {
+    function finishUpgrade(address _target) public {
+        super.finishUpgrade(_target);
         MiningAdjudicator targetContract = MiningAdjudicator(_target);
         escrow = targetContract.escrow();
         hashAlgorithm = targetContract.hashAlgorithm();

nucypher/blockchain/eth/sol/source/contracts/PolicyManager.sol

@@ -499,7 +499,8 @@ contract PolicyManager is Upgradeable {
         }
     }
 
-    function verifyState(address _testTarget) public onlyWhileUpgrading {
+    function verifyState(address _testTarget) public {
+        super.verifyState(_testTarget);
         require(address(uint160(delegateGet(_testTarget, "escrow()"))) == address(escrow));
         require(uint32(delegateGet(_testTarget, "secondsPerPeriod()")) == secondsPerPeriod);
         Policy storage policy = policies[RESERVED_POLICY_ID];
@@ -531,7 +532,8 @@ contract PolicyManager is Upgradeable {
             bytes32(bytes20(RESERVED_NODE)), bytes32(uint256(11)))) == nodeInfo.rewardDelta[11]);
     }
 
-    function finishUpgrade(address _target) public onlyWhileUpgrading {
+    function finishUpgrade(address _target) public {
+        super.finishUpgrade(_target);
         PolicyManager policyManager = PolicyManager(_target);
         escrow = policyManager.escrow();
         secondsPerPeriod = policyManager.secondsPerPeriod();

nucypher/blockchain/eth/sol/source/contracts/proxy/Dispatcher.sol

@@ -61,6 +61,7 @@ contract Dispatcher is Upgradeable {
         require(address(uint160(delegateGet(_testTarget, "target()"))) == target);
         require(address(uint160(delegateGet(_testTarget, "previousTarget()"))) == previousTarget);
         require(bytes32(delegateGet(_testTarget, "secretHash()")) == secretHash);
+        require(uint8(delegateGet(_testTarget, "isUpgrade()")) == isUpgrade);
     }
 
     /**

nucypher/blockchain/eth/sol/source/contracts/proxy/Upgradeable.sol

@@ -12,6 +12,9 @@ import "zeppelin/ownership/Ownable.sol";
 **/
 contract Upgradeable is Ownable {
 
+    event StateVerified(address indexed testTarget, address sender);
+    event UpgradeFinished(address indexed target, address sender);
+
     /**
     * @dev Contracts at the target must reserve the same location in storage for this address as in Dispatcher
     * Stored data actually lives in the Dispatcher
@@ -32,7 +35,7 @@ contract Upgradeable is Ownable {
     /**
     * @dev Upgrade status. Explicit `uint8` type is used instead of `bool` to save gas by excluding 0 value
     **/
-    uint8 isUpgrade;
+    uint8 public isUpgrade;
 
     /** Constants for `isUpgrade` field **/
     uint8 constant UPGRADE_FALSE = 1;
@@ -52,13 +55,17 @@ contract Upgradeable is Ownable {
     * @dev Method for verifying storage state.
     * Should check that new target contract returns right storage value
     **/
-    function verifyState(address _testTarget) public /*onlyWhileUpgrading*/;
+    function verifyState(address _testTarget) public onlyWhileUpgrading {
+        emit StateVerified(_testTarget, msg.sender);
+    }
 
     /**
     * @dev Copy values from the new target to the current storage
     * @param _target New target contract address
     **/
-    function finishUpgrade(address _target) public /*onlyWhileUpgrading*/;
+    function finishUpgrade(address _target) public onlyWhileUpgrading {
+        emit UpgradeFinished(_target, msg.sender);
+    }
 
     /**
     * @dev Base method to get data

tests/blockchain/eth/contracts/base/test_dispatcher.py

@@ -44,9 +44,11 @@ def test_dispatcher(testerchain):
     contract3_lib, _ = testerchain.interface.deploy_contract('ContractV3', 2)
     contract2_bad_lib, _ = testerchain.interface.deploy_contract('ContractV2Bad')
     dispatcher, _ = testerchain.interface.deploy_contract('Dispatcher', contract1_lib.address, secret_hash)
+    assert contract1_lib.address == dispatcher.functions.target().call()
 
     upgrades = dispatcher.events.Upgraded.createFilter(fromBlock=0)
-    assert contract1_lib.address == dispatcher.functions.target().call()
+    state_verifications = dispatcher.events.StateVerified.createFilter(fromBlock=0)
+    upgrade_finishings = dispatcher.events.UpgradeFinished.createFilter(fromBlock=0)
 
     events = upgrades.get_all_entries()
     assert 1 == len(events)
@@ -55,6 +57,12 @@ def test_dispatcher(testerchain):
     assert contract1_lib.address == event_args['to']
     assert creator == event_args['owner']
 
+    events = upgrade_finishings.get_all_entries()
+    assert 1 == len(events)
+    event_args = events[0]['args']
+    assert contract1_lib.address == event_args['target']
+    assert creator == event_args['sender']
+
     # Assign dispatcher address as contract.
     # In addition to the interface can be used ContractV1, ContractV2 or ContractV3 ABI
     contract_instance = testerchain.interface.w3.eth.contract(
@@ -137,12 +145,23 @@ def test_dispatcher(testerchain):
 
     events = upgrades.get_all_entries()
     assert 2 == len(events)
-
     event_args = events[1]['args']
     assert contract1_lib.address == event_args['from']
     assert contract2_lib.address == event_args['to']
     assert creator == event_args['owner']
 
+    events = state_verifications.get_all_entries()
+    assert 1 == len(events)
+    event_args = events[0]['args']
+    assert contract2_lib.address == event_args['testTarget']
+    assert creator == event_args['sender']
+
+    events = upgrade_finishings.get_all_entries()
+    assert 2 == len(events)
+    event_args = events[1]['args']
+    assert contract2_lib.address == event_args['target']
+    assert creator == event_args['sender']
+
     # Check values and methods after upgrade
     assert 20 == contract_instance.functions.returnValue().call()
     assert 5 == contract_instance.functions.getStorageValue().call()
@@ -237,6 +256,18 @@ def test_dispatcher(testerchain):
     assert contract1_lib.address == event_args['to']
     assert creator == event_args['owner']
 
+    events = state_verifications.get_all_entries()
+    assert 2 == len(events)
+    event_args = events[1]['args']
+    assert contract2_lib.address == event_args['testTarget']
+    assert creator == event_args['sender']
+
+    events = upgrade_finishings.get_all_entries()
+    assert 3 == len(events)
+    event_args = events[2]['args']
+    assert contract1_lib.address == event_args['target']
+    assert creator == event_args['sender']
+
     # Can't upgrade to the bad version
     with pytest.raises((TransactionFailed, ValueError)):
         tx = dispatcher.functions.upgrade(contract2_bad_lib.address, secret3, secret_hash).transact({'from': creator})
@@ -286,8 +317,6 @@ def test_dispatcher(testerchain):
     assert 2 == contract_instance.functions.storageValueToCheck().call()
     assert 'Hello' == contract_instance.functions.getDynamicallySizedValue().call()
 
-    # bug? with duplicate entries
-    upgrades = dispatcher.events.Upgraded.createFilter(fromBlock=0)
     events = upgrades.get_all_entries()
     assert 4 == len(events)
     event_args = events[2]['args']
@@ -299,6 +328,24 @@ def test_dispatcher(testerchain):
     assert contract3_lib.address == event_args['to']
     assert creator == event_args['owner']
 
+    events = state_verifications.get_all_entries()
+    assert 4 == len(events)
+    event_args = events[2]['args']
+    assert contract2_lib.address == event_args['testTarget']
+    assert creator == event_args['sender']
+    event_args = events[3]['args']
+    assert contract3_lib.address == event_args['testTarget']
+    assert creator == event_args['sender']
+
+    events = upgrade_finishings.get_all_entries()
+    assert 5 == len(events)
+    event_args = events[3]['args']
+    assert contract2_lib.address == event_args['target']
+    assert creator == event_args['sender']
+    event_args = events[4]['args']
+    assert contract3_lib.address == event_args['target']
+    assert creator == event_args['sender']
+
     # Create and check events
     tx = contract_instance.functions.createEvent(22).transact({'from': creator})
     testerchain.wait_for_receipt(tx)

tests/blockchain/eth/contracts/base/test_issuer.py

@@ -235,3 +235,24 @@ def test_upgrading(testerchain, token):
         tx = dispatcher.functions.upgrade(contract_library_bad.address, secret, secret2_hash)\
             .transact({'from': creator})
         testerchain.wait_for_receipt(tx)
+
+    events = dispatcher.events.StateVerified.createFilter(fromBlock=0).get_all_entries()
+    assert 2 == len(events)
+    event_args = events[0]['args']
+    assert contract_library_v2.address == event_args['testTarget']
+    assert creator == event_args['sender']
+    event_args = events[1]['args']
+    assert contract_library_v2.address == event_args['testTarget']
+    assert creator == event_args['sender']
+
+    events = dispatcher.events.UpgradeFinished.createFilter(fromBlock=0).get_all_entries()
+    assert 3 == len(events)
+    event_args = events[0]['args']
+    assert contract_library_v1.address == event_args['target']
+    assert creator == event_args['sender']
+    event_args = events[1]['args']
+    assert contract_library_v2.address == event_args['target']
+    assert creator == event_args['sender']
+    event_args = events[2]['args']
+    assert contract_library_v1.address == event_args['target']
+    assert creator == event_args['sender']

tests/blockchain/eth/contracts/contracts/IssuerTestSet.sol

@@ -67,9 +67,6 @@ contract IssuerBad is Upgradeable {
 //    uint256 public currentSupply1;
     uint256 public currentSupply2;
 
-    function verifyState(address) public {}
-    function finishUpgrade(address) public {}
-
 }
 
 
@@ -102,7 +99,7 @@ contract IssuerV2Mock is Issuer {
         valueToCheck = _valueToCheck;
     }
 
-    function verifyState(address _testTarget) public onlyWhileUpgrading {
+    function verifyState(address _testTarget) public {
         super.verifyState(_testTarget);
         require(delegateGet(_testTarget, "valueToCheck()") == valueToCheck);
     }

tests/blockchain/eth/contracts/contracts/MinersEscrowTestSet.sol

@@ -78,7 +78,7 @@ contract MinersEscrowV2Mock is MinersEscrow {
         valueToCheck = _valueToCheck;
     }
 
-    function verifyState(address _testTarget) public onlyWhileUpgrading {
+    function verifyState(address _testTarget) public {
         super.verifyState(_testTarget);
         require(delegateGet(_testTarget, "valueToCheck()") == valueToCheck);
     }
@@ -86,6 +86,7 @@ contract MinersEscrowV2Mock is MinersEscrow {
     function finishUpgrade(address _target) public onlyWhileUpgrading {
         MinersEscrowV2Mock escrow = MinersEscrowV2Mock(_target);
         valueToCheck = escrow.valueToCheck();
+        emit UpgradeFinished(_target, msg.sender);
     }
 }
 

tests/blockchain/eth/contracts/contracts/MiningAdjudicatorTestSet.sol

@@ -57,9 +57,6 @@ contract MiningAdjudicatorBad is Upgradeable {
     mapping (bytes32 => bool) public evaluatedCFrags;
     mapping (address => uint256) public penaltyHistory;
 
-    function verifyState(address) public {}
-    function finishUpgrade(address) public {}
-
 }
 
 
@@ -94,7 +91,7 @@ contract MiningAdjudicatorV2Mock is MiningAdjudicator {
         valueToCheck = _valueToCheck;
     }
 
-    function verifyState(address _testTarget) public onlyWhileUpgrading {
+    function verifyState(address _testTarget) public {
         super.verifyState(_testTarget);
         require(uint256(delegateGet(_testTarget, "valueToCheck()")) == valueToCheck);
     }

tests/blockchain/eth/contracts/contracts/PolicyManagerTestSet.sol

@@ -34,7 +34,7 @@ contract PolicyManagerV2Mock is PolicyManager {
         valueToCheck = _valueToCheck;
     }
 
-    function verifyState(address _testTarget) public onlyWhileUpgrading {
+    function verifyState(address _testTarget) public {
         super.verifyState(_testTarget);
         require(delegateGet(_testTarget, "valueToCheck()") == valueToCheck);
     }

tests/blockchain/eth/contracts/contracts/proxy/ContractV1.sol

@@ -124,7 +124,8 @@ contract ContractV1 is ContractInterface, Upgradeable {
         return mappingStructures[_index].arrayValues[_arrayIndex];
     }
 
-    function verifyState(address _testTarget) public onlyWhileUpgrading {
+    function verifyState(address _testTarget) public {
+        super.verifyState(_testTarget);
         require(delegateGet(_testTarget, "storageValue()") == storageValue);
         bytes memory value = delegateGetBytes(_testTarget, "dynamicallySizedValue()");
         require(value.length == bytes(dynamicallySizedValue).length &&
@@ -193,7 +194,8 @@ contract ContractV1 is ContractInterface, Upgradeable {
         }
     }
 
-    function finishUpgrade(address _target) public onlyWhileUpgrading {
+    function finishUpgrade(address _target) public {
+        super.finishUpgrade(_target);
         storageValue = ContractV1(_target).storageValue();
     }
 

tests/blockchain/eth/contracts/contracts/proxy/ContractV2.sol

@@ -133,7 +133,8 @@ contract ContractV2 is ContractInterface, Upgradeable {
         return mappingStructures[_index].arrayValues;
     }
 
-    function verifyState(address _testTarget) public onlyWhileUpgrading {
+    function verifyState(address _testTarget) public {
+        super.verifyState(_testTarget);
         require(delegateGet(_testTarget, "storageValue()") == storageValue);
         bytes memory value = delegateGetBytes(_testTarget, "dynamicallySizedValue()");
         require(value.length == bytes(dynamicallySizedValue).length &&
@@ -222,7 +223,8 @@ contract ContractV2 is ContractInterface, Upgradeable {
         }
     }
 
-    function finishUpgrade(address _target) public onlyWhileUpgrading {
+    function finishUpgrade(address _target) public {
+        super.finishUpgrade(_target);
         storageValueToCheck = ContractV2(_target).storageValueToCheck();
     }
 

tests/blockchain/eth/contracts/contracts/proxy/ContractV3.sol

@@ -18,7 +18,7 @@ contract ContractV3 is ContractV2 {
         anotherStorageValue = _value * 2;
     }
 
-    function verifyState(address _testTarget) public onlyWhileUpgrading {
+    function verifyState(address _testTarget) public {
         super.verifyState(_testTarget);
         require(delegateGet(_testTarget, "anotherStorageValue()") == anotherStorageValue);
     }

tests/blockchain/eth/contracts/main/miners_escrow/test_miners_escrow_additional.py

@@ -150,6 +150,27 @@ def test_upgrading(testerchain, token):
             .transact({'from': creator})
         testerchain.wait_for_receipt(tx)
 
+    events = dispatcher.events.StateVerified.createFilter(fromBlock=0).get_all_entries()
+    assert 2 == len(events)
+    event_args = events[0]['args']
+    assert contract_library_v2.address == event_args['testTarget']
+    assert creator == event_args['sender']
+    event_args = events[1]['args']
+    assert contract_library_v2.address == event_args['testTarget']
+    assert creator == event_args['sender']
+
+    events = dispatcher.events.UpgradeFinished.createFilter(fromBlock=0).get_all_entries()
+    assert 3 == len(events)
+    event_args = events[0]['args']
+    assert contract_library_v1.address == event_args['target']
+    assert creator == event_args['sender']
+    event_args = events[1]['args']
+    assert contract_library_v2.address == event_args['target']
+    assert creator == event_args['sender']
+    event_args = events[2]['args']
+    assert contract_library_v1.address == event_args['target']
+    assert creator == event_args['sender']
+
 
 @pytest.mark.slow
 def test_re_stake(testerchain, token, escrow_contract):

tests/blockchain/eth/contracts/main/mining_adjudicator/test_mining_adjudicator.py

@@ -560,3 +560,24 @@ def test_upgrading(testerchain):
         tx = dispatcher.functions.upgrade(contract_library_bad.address, secret, secret2_hash) \
             .transact({'from': creator})
         testerchain.wait_for_receipt(tx)
+
+    events = dispatcher.events.StateVerified.createFilter(fromBlock=0).get_all_entries()
+    assert 2 == len(events)
+    event_args = events[0]['args']
+    assert contract_library_v2.address == event_args['testTarget']
+    assert creator == event_args['sender']
+    event_args = events[1]['args']
+    assert contract_library_v2.address == event_args['testTarget']
+    assert creator == event_args['sender']
+
+    events = dispatcher.events.UpgradeFinished.createFilter(fromBlock=0).get_all_entries()
+    assert 3 == len(events)
+    event_args = events[0]['args']
+    assert contract_library_v1.address == event_args['target']
+    assert creator == event_args['sender']
+    event_args = events[1]['args']
+    assert contract_library_v2.address == event_args['target']
+    assert creator == event_args['sender']
+    event_args = events[2]['args']
+    assert contract_library_v1.address == event_args['target']
+    assert creator == event_args['sender']

tests/blockchain/eth/contracts/main/policy_manager/test_policy_manager.py

@@ -404,3 +404,24 @@ def test_upgrading(testerchain):
         tx = dispatcher.functions.upgrade(contract_library_bad.address, secret, secret2_hash)\
             .transact({'from': creator})
         testerchain.wait_for_receipt(tx)
+
+    events = dispatcher.events.StateVerified.createFilter(fromBlock=0).get_all_entries()
+    assert 2 == len(events)
+    event_args = events[0]['args']
+    assert contract_library_v2.address == event_args['testTarget']
+    assert creator == event_args['sender']
+    event_args = events[1]['args']
+    assert contract_library_v2.address == event_args['testTarget']
+    assert creator == event_args['sender']
+
+    events = dispatcher.events.UpgradeFinished.createFilter(fromBlock=0).get_all_entries()
+    assert 3 == len(events)
+    event_args = events[0]['args']
+    assert contract_library_v1.address == event_args['target']
+    assert creator == event_args['sender']
+    event_args = events[1]['args']
+    assert contract_library_v2.address == event_args['target']
+    assert creator == event_args['sender']
+    event_args = events[2]['args']
+    assert contract_library_v1.address == event_args['target']
+    assert creator == event_args['sender']