evm: Save state for failed transactions (#2116)

* Save state for failed TXs

* Add test

* Add intermediate3 fork

lib/ain-cpp-imports/src/bridge.rs

@@ -19,5 +19,6 @@ pub mod ffi {
         fn getHighestBlock() -> i32;
         fn getCurrentHeight() -> i32;
         fn pastChangiIntermediateHeight2() -> bool;
+        fn pastChangiIntermediateHeight3() -> bool;
     }
 }

lib/ain-cpp-imports/src/lib.rs

@@ -58,6 +58,9 @@ mod ffi {
     pub fn pastChangiIntermediateHeight2() -> bool {
         unimplemented!("{}", UNIMPL_MSG)
     }
+    pub fn pastChangiIntermediateHeight3() -> bool {
+        unimplemented!("{}", UNIMPL_MSG)
+    }
 }
 
 pub fn get_chain_id() -> Result<u64, Box<dyn Error>> {
@@ -139,5 +142,10 @@ pub fn past_changi_intermediate_height_2_height() -> Result<bool, Box<dyn Error>
     Ok(height)
 }
 
+pub fn past_changi_intermediate_height_3_height() -> Result<bool, Box<dyn Error>> {
+    let height = ffi::pastChangiIntermediateHeight3();
+    Ok(height)
+}
+
 #[cfg(test)]
 mod tests {}

lib/ain-evm/src/executor.rs

@@ -133,7 +133,9 @@ impl<'backend> Executor for AinExecutor<'backend> {
         let (values, logs) = executor.into_state().deconstruct();
         let logs = logs.into_iter().collect::<Vec<_>>();
 
-        if exit_reason.is_succeed() {
+        let past_changi_intermediate3 = ain_cpp_imports::past_changi_intermediate_height_3_height().expect("Unable to get Changi intermediate3 height");
+
+        if (!past_changi_intermediate3 && exit_reason.is_succeed()) || (past_changi_intermediate3) {
             ApplyBackend::apply(self.backend, values, logs.clone(), true);
             self.backend.commit();
         }

src/chainparams.cpp

@@ -143,6 +143,7 @@ class CMainParams : public CChainParams {
         consensus.NextNetworkUpgradeHeight = std::numeric_limits<int>::max();
         consensus.ChangiIntermediateHeight = std::numeric_limits<int>::max();
         consensus.ChangiIntermediateHeight2 = std::numeric_limits<int>::max();
+        consensus.ChangiIntermediateHeight3 = std::numeric_limits<int>::max();
 
         consensus.pos.diffLimit = uint256S("00000fffffffffffffffffffffffffffffffffffffffffffffffffffffffffff");
 //        consensus.pos.nTargetTimespan = 14 * 24 * 60 * 60; // two weeks
@@ -640,6 +641,7 @@ class CChangiParams : public CChainParams {
         consensus.NextNetworkUpgradeHeight = 1586750;
         consensus.ChangiIntermediateHeight = 1717800;
         consensus.ChangiIntermediateHeight2 = 1717493;
+        consensus.ChangiIntermediateHeight3 = std::numeric_limits<int>::max();
 
         consensus.pos.diffLimit = uint256S("00000fffffffffffffffffffffffffffffffffffffffffffffffffffffffffff");
         consensus.pos.nTargetTimespan = 5 * 60; // 5 min == 10 blocks
@@ -858,6 +860,7 @@ class CDevNetParams : public CChainParams {
         consensus.NextNetworkUpgradeHeight = 1586750;
         consensus.ChangiIntermediateHeight = std::numeric_limits<int>::max();
         consensus.ChangiIntermediateHeight2 = std::numeric_limits<int>::max();
+        consensus.ChangiIntermediateHeight3 = std::numeric_limits<int>::max();
 
         consensus.pos.diffLimit = uint256S("00000fffffffffffffffffffffffffffffffffffffffffffffffffffffffffff");
         consensus.pos.nTargetTimespan = 5 * 60; // 5 min == 10 blocks
@@ -1079,6 +1082,7 @@ class CRegTestParams : public CChainParams {
         consensus.NextNetworkUpgradeHeight = 10000000;
         consensus.ChangiIntermediateHeight = 10000000;
         consensus.ChangiIntermediateHeight2 = 10000000;
+        consensus.ChangiIntermediateHeight3 = 10000000;
 
         consensus.pos.diffLimit = uint256S("00000fffffffffffffffffffffffffffffffffffffffffffffffffffffffffff");
         consensus.pos.nTargetTimespan = 14 * 24 * 60 * 60; // two weeks
@@ -1327,6 +1331,7 @@ void SetupCommonArgActivationParams(Consensus::Params &consensus) {
     UpdateHeightValidation("Next Network Upgrade", "-nextnetworkupgradeheight", consensus.NextNetworkUpgradeHeight);
     UpdateHeightValidation("Changi Intermediate", "-changiintermediateheight", consensus.ChangiIntermediateHeight);
     UpdateHeightValidation("Changi Intermediate2", "-changiintermediate2height", consensus.ChangiIntermediateHeight2);
+    UpdateHeightValidation("Changi Intermediate3", "-changiintermediate3height", consensus.ChangiIntermediateHeight3);
 
     if (gArgs.GetBoolArg("-simulatemainnet", false)) {
         consensus.pos.nTargetTimespan = 5 * 60; // 5 min == 10 blocks

src/consensus/params.h

@@ -104,6 +104,7 @@ struct Params {
     int NextNetworkUpgradeHeight;
     int ChangiIntermediateHeight; // To be changed to NextNetworkUpgradeHeight on mainnet release
     int ChangiIntermediateHeight2; // To be changed to NextNetworkUpgradeHeight on mainnet release
+    int ChangiIntermediateHeight3; // To be changed to NextNetworkUpgradeHeight on mainnet release
 
     /** Foundation share after AMK, normalized to COIN = 100% */
     CAmount foundationShareDFIP1;

src/ffi/ffiexports.cpp

@@ -209,9 +209,16 @@ int getHighestBlock() {
 }
 
 int getCurrentHeight() {
+    LOCK(cs_main);
     return ::ChainActive().Height() ? (int) ::ChainActive().Height() : -1;
 }
 
 bool pastChangiIntermediateHeight2() {
+    LOCK(cs_main);
     return ::ChainActive().Height() >= Params().GetConsensus().ChangiIntermediateHeight2;
 }
+
+bool pastChangiIntermediateHeight3() {
+    LOCK(cs_main);
+    return ::ChainActive().Height() >= Params().GetConsensus().ChangiIntermediateHeight3;
+}

src/ffi/ffiexports.h

@@ -20,5 +20,6 @@ rust::string getStateInputJSON();
 int getHighestBlock();
 int getCurrentHeight();
 bool pastChangiIntermediateHeight2();
+bool pastChangiIntermediateHeight3();
 
 #endif // DEFI_FFI_FFIEXPORTS_H

src/init.cpp

@@ -514,6 +514,7 @@ void SetupServerArgs()
     gArgs.AddArg("-nextnetworkupgradeheight", "Next Network Upgrade fork activation height (regtest only)", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::CHAINPARAMS);
     gArgs.AddArg("-changiintermediateheight", "Changi Intermediate fork activation height (regtest only)", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::CHAINPARAMS);
     gArgs.AddArg("-changiintermediate2height", "Changi Intermediate2 fork activation height (regtest only)", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::CHAINPARAMS);
+    gArgs.AddArg("-changiintermediate3height", "Changi Intermediate3 fork activation height (regtest only)", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::CHAINPARAMS);
     gArgs.AddArg("-jellyfish_regtest", "Configure the regtest network for jellyfish testing", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::OPTIONS);
     gArgs.AddArg("-regtest-skip-loan-collateral-validation", "Skip loan collateral check for jellyfish testing", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::OPTIONS);
     gArgs.AddArg("-regtest-minttoken-simulate-mainnet", "Simulate mainnet for minttokens on regtest -  default behavior on regtest is to allow anyone to mint mintable tokens for ease of testing", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::OPTIONS);

test/functional/contracts/Reverter.sol

@@ -0,0 +1,17 @@
+// SPDX-License-Identifier: GPL-3.0
+
+pragma solidity >=0.8.2 <0.9.0;
+
+contract Reverter {
+    uint256 number;
+
+     function tryRevert() public {
+        number += 1;
+
+        revert('Function reverted');
+    }
+
+    function trySuccess() public {
+        number += 1;
+    }
+}

test/functional/feature_evm_contracts.py

@@ -20,7 +20,7 @@ def set_test_params(self):
              '-fortcanningheight=82', '-fortcanninghillheight=84', '-fortcanningroadheight=86',
              '-fortcanningcrunchheight=88', '-fortcanningspringheight=90', '-fortcanninggreatworldheight=94',
              '-fortcanningepilogueheight=96', '-grandcentralheight=101', '-nextnetworkupgradeheight=105',
-             '-subsidytest=1', '-txindex=1'],
+             '-changiintermediate3height=105', '-subsidytest=1', '-txindex=1'],
         ]
 
     def setup(self):
@@ -34,14 +34,38 @@ def setup(self):
         self.nodes[0].setgov({"ATTRIBUTES": {'v0/params/feature/evm': 'true'}})
         self.nodes[0].generate(1)
 
+    def failed_tx_should_increment_nonce(self, key_pair, node):
+        from web3 import Web3
+        web3 = Web3(Web3.HTTPProvider(node.get_evm_rpc()))
+
+        evm_contract = EVMContract.from_file("Reverter.sol", "Reverter").compile()
+        contract = node.evm.deploy_compiled_contract(key_pair, evm_contract)
+
+        # for successful TX
+        before_tx_count = web3.eth.get_transaction_count(key_pair.address)
+
+        node.evm.sign_and_send(contract.functions.trySuccess(), key_pair)
+        after_tx_count = web3.eth.get_transaction_count(key_pair.address)
+
+        assert_equal(before_tx_count + 1, after_tx_count)
+
+        # for failed TX
+        before_tx_count = web3.eth.get_transaction_count(key_pair.address)
+
+        node.evm.sign_and_send(contract.functions.tryRevert(), key_pair)
+        after_tx_count = web3.eth.get_transaction_count(key_pair.address)
+
+        assert_equal(before_tx_count + 1, after_tx_count)
+
     def run_test(self):
         node = self.nodes[0]
         self.setup()
 
         key_pair = KeyPair.from_node(node)
         address = key_pair.address
 
-        node.transferdomain([{"src": {"address":self.address, "amount":"50@DFI", "domain": 2}, "dst":{"address":address, "amount":"50@DFI", "domain": 3}}])
+        node.transferdomain([{"src": {"address": self.address, "amount": "50@DFI", "domain": 2},
+                              "dst": {"address": address, "amount": "50@DFI", "domain": 3}}])
         node.generate(1)
 
         evm_contract = EVMContract.from_file("SimpleStorage.sol", "Test").compile()
@@ -53,6 +77,8 @@ def run_test(self):
         # get variable
         assert_equal(contract.functions.retrieve().call(), 10)
 
+        self.failed_tx_should_increment_nonce(key_pair, node)
+
 
 if __name__ == '__main__':
     EVMTest().main()