EVM: Validate nonce ordering in finalize_block

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

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

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

@@ -61,6 +61,9 @@ mod ffi {
     pub fn pastChangiIntermediateHeight3() -> bool {
         unimplemented!("{}", UNIMPL_MSG)
     }
+    pub fn pastChangiIntermediateHeight4() -> bool {
+        unimplemented!("{}", UNIMPL_MSG)
+    }
 }
 
 pub fn get_chain_id() -> Result<u64, Box<dyn Error>> {
@@ -145,5 +148,9 @@ pub fn past_changi_intermediate_height_3_height() -> bool {
     ffi::pastChangiIntermediateHeight3()
 }
 
+pub fn past_changi_intermediate_height_4_height() -> bool {
+    ffi::pastChangiIntermediateHeight4()
+}
+
 #[cfg(test)]
 mod tests {}

lib/ain-evm/src/backend.rs

@@ -70,7 +70,7 @@ impl EVMBackend {
         storage: I,
         reset_storage: bool,
     ) -> Result<Account> {
-        let account = self.get_account(address).unwrap_or(Account {
+        let account = self.get_account(&address).unwrap_or(Account {
             nonce: U256::zero(),
             balance: U256::zero(),
             storage_root: H256::zero(),
@@ -175,12 +175,18 @@ impl EVMBackend {
 }
 
 impl EVMBackend {
-    pub fn get_account(&self, address: H160) -> Option<Account> {
+    pub fn get_account(&self, address: &H160) -> Option<Account> {
         self.state
             .get(address.as_bytes())
             .unwrap_or(None)
             .and_then(|addr| Account::decode(&Rlp::new(addr.as_bytes_ref())).ok())
     }
+
+    pub fn get_nonce(&self, address: &H160) -> U256 {
+        self.get_account(address)
+            .map(|acc| acc.nonce)
+            .unwrap_or_default()
+    }
 }
 
 impl Backend for EVMBackend {
@@ -241,7 +247,7 @@ impl Backend for EVMBackend {
 
     fn basic(&self, address: H160) -> Basic {
         trace!(target: "backend", "[EVMBackend] basic for address {:x?}", address);
-        self.get_account(address)
+        self.get_account(&address)
             .map(|account| Basic {
                 balance: account.balance,
                 nonce: account.nonce,
@@ -251,14 +257,14 @@ impl Backend for EVMBackend {
 
     fn code(&self, address: H160) -> Vec<u8> {
         trace!(target: "backend", "[EVMBackend] code for address {:x?}", address);
-        self.get_account(address)
+        self.get_account(&address)
             .and_then(|account| self.storage.get_code_by_hash(account.code_hash))
             .unwrap_or_default()
     }
 
     fn storage(&self, address: H160, index: H256) -> H256 {
         trace!(target: "backend", "[EVMBackend] Getting storage for address {:x?} at index {:x?}", address, index);
-        self.get_account(address)
+        self.get_account(&address)
             .and_then(|account| {
                 self.trie_store
                     .trie_db
@@ -336,7 +342,7 @@ impl BridgeBackend for EVMBackend {
 
     fn sub_balance(&mut self, address: H160, amount: U256) -> Result<()> {
         let account = self
-            .get_account(address)
+            .get_account(&address)
             .ok_or(EVMBackendError::NoSuchAccount(address))?;
 
         if account.balance < amount {

lib/ain-evm/src/evm.rs

@@ -371,7 +371,7 @@ impl EVMHandler {
             Arc::clone(&self.storage),
             Vicinity::default(),
         )?;
-        Ok(backend.get_account(address))
+        Ok(backend.get_account(&address))
     }
 
     pub fn get_code(&self, address: H160, block_number: U256) -> Result<Option<Vec<u8>>, EVMError> {

lib/ain-evm/src/executor.rs

@@ -21,8 +21,6 @@ pub struct AinExecutor<'backend> {
     backend: &'backend mut EVMBackend,
 }
 
-impl<'backend> AinExecutor<'backend> {}
-
 impl<'backend> AinExecutor<'backend> {
     pub fn new(backend: &'backend mut EVMBackend) -> Self {
         Self { backend }
@@ -39,6 +37,10 @@ impl<'backend> AinExecutor<'backend> {
     pub fn commit(&mut self) -> H256 {
         self.backend.commit()
     }
+
+    pub fn get_nonce(&self, address: &H160) -> U256 {
+        self.backend.get_nonce(address)
+    }
 }
 
 impl<'backend> Executor for AinExecutor<'backend> {

lib/ain-evm/src/handler.rs

@@ -130,6 +130,13 @@ impl Handlers {
         for (queue_tx, hash) in self.evm.tx_queues.get_cloned_vec(context) {
             match queue_tx {
                 QueueTx::SignedTx(signed_tx) => {
+                    if ain_cpp_imports::past_changi_intermediate_height_4_height() {
+                        let nonce = executor.get_nonce(&signed_tx.sender);
+                        if signed_tx.nonce() != nonce {
+                            return Err(anyhow!("EVM block rejected for invalid nonce. Address {} nonce {}, signed_tx nonce: {}", signed_tx.sender, nonce, signed_tx.nonce()).into());
+                        }
+                    }
+
                     let (
                         TxResponse {
                             exit_reason,

lib/ain-rs-exports/src/lib.rs

@@ -30,6 +30,7 @@ pub mod ffi {
         priv_key: [u8; 32],
     }
 
+    #[derive(Default)]
     pub struct FinalizeBlockResult {
         block_hash: [u8; 32],
         failed_transactions: Vec<String>,
@@ -81,7 +82,8 @@ pub mod ffi {
             native_tx_hash: [u8; 32],
         ) -> Result<bool>;
 
-        fn evm_finalize(
+        fn evm_try_finalize(
+            result: &mut CrossBoundaryResult,
             context: u64,
             update_state: bool,
             difficulty: u32,
@@ -407,30 +409,38 @@ fn evm_try_queue_tx(
 /// # Errors
 ///
 /// Returns an Error if there is an error restoring the state trie.
+/// Returns an Error if the block has invalid TXs, viz. out of order nonces
 ///
 /// # Returns
 ///
 /// Returns a `FinalizeBlockResult` containing the block hash, failed transactions, and miner fee on success.
-fn evm_finalize(
+fn evm_try_finalize(
+    result: &mut CrossBoundaryResult,
     context: u64,
     update_state: bool,
     difficulty: u32,
     miner_address: [u8; 20],
     timestamp: u64,
 ) -> Result<ffi::FinalizeBlockResult, Box<dyn Error>> {
     let eth_address = H160::from(miner_address);
-    let (block_hash, failed_txs, gas_used) = RUNTIME.handlers.finalize_block(
-        context,
-        update_state,
-        difficulty,
-        eth_address,
-        timestamp,
-    )?;
-    Ok(ffi::FinalizeBlockResult {
-        block_hash,
-        failed_transactions: failed_txs,
-        miner_fee: gas_used,
-    })
+    match RUNTIME
+        .handlers
+        .finalize_block(context, update_state, difficulty, eth_address, timestamp)
+    {
+        Ok((block_hash, failed_txs, gas_used)) => {
+            result.ok = true;
+            Ok(ffi::FinalizeBlockResult {
+                block_hash,
+                failed_transactions: failed_txs,
+                miner_fee: gas_used,
+            })
+        }
+        Err(e) => {
+            result.ok = false;
+            result.reason = e.to_string();
+            Ok(ffi::FinalizeBlockResult::default())
+        }
+    }
 }
 
 pub fn preinit() {

src/chainparams.cpp

@@ -144,6 +144,8 @@ class CMainParams : public CChainParams {
         consensus.ChangiIntermediateHeight = std::numeric_limits<int>::max();
         consensus.ChangiIntermediateHeight2 = std::numeric_limits<int>::max();
         consensus.ChangiIntermediateHeight3 = std::numeric_limits<int>::max();
+        consensus.ChangiIntermediateHeight3 = std::numeric_limits<int>::max();
+        consensus.ChangiIntermediateHeight4 = std::numeric_limits<int>::max();
 
         consensus.pos.diffLimit = uint256S("00000fffffffffffffffffffffffffffffffffffffffffffffffffffffffffff");
 //        consensus.pos.nTargetTimespan = 14 * 24 * 60 * 60; // two weeks
@@ -642,6 +644,7 @@ class CChangiParams : public CChainParams {
         consensus.ChangiIntermediateHeight = 1717800;
         consensus.ChangiIntermediateHeight2 = 1717493;
         consensus.ChangiIntermediateHeight3 = 1730100;
+        consensus.ChangiIntermediateHeight4 = std::numeric_limits<int>::max();
 
         consensus.pos.diffLimit = uint256S("00000fffffffffffffffffffffffffffffffffffffffffffffffffffffffffff");
         consensus.pos.nTargetTimespan = 5 * 60; // 5 min == 10 blocks
@@ -861,6 +864,7 @@ class CDevNetParams : public CChainParams {
         consensus.ChangiIntermediateHeight = std::numeric_limits<int>::max();
         consensus.ChangiIntermediateHeight2 = std::numeric_limits<int>::max();
         consensus.ChangiIntermediateHeight3 = std::numeric_limits<int>::max();
+        consensus.ChangiIntermediateHeight4 = std::numeric_limits<int>::max();
 
         consensus.pos.diffLimit = uint256S("00000fffffffffffffffffffffffffffffffffffffffffffffffffffffffffff");
         consensus.pos.nTargetTimespan = 5 * 60; // 5 min == 10 blocks
@@ -1083,6 +1087,7 @@ class CRegTestParams : public CChainParams {
         consensus.ChangiIntermediateHeight = 10000000;
         consensus.ChangiIntermediateHeight2 = 10000000;
         consensus.ChangiIntermediateHeight3 = 10000000;
+        consensus.ChangiIntermediateHeight4 = std::numeric_limits<int>::max();
 
         consensus.pos.diffLimit = uint256S("00000fffffffffffffffffffffffffffffffffffffffffffffffffffffffffff");
         consensus.pos.nTargetTimespan = 14 * 24 * 60 * 60; // two weeks

src/consensus/params.h

@@ -105,6 +105,7 @@ struct Params {
     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
+    int ChangiIntermediateHeight4; // To be changed to NextNetworkUpgradeHeight on mainnet release
 
     /** Foundation share after AMK, normalized to COIN = 100% */
     CAmount foundationShareDFIP1;

src/ffi/ffiexports.cpp

@@ -222,3 +222,8 @@ bool pastChangiIntermediateHeight3() {
     LOCK(cs_main);
     return ::ChainActive().Height() >= Params().GetConsensus().ChangiIntermediateHeight3;
 }
+
+bool pastChangiIntermediateHeight4() {
+    LOCK(cs_main);
+    return ::ChainActive().Height() >= Params().GetConsensus().ChangiIntermediateHeight3;
+}

src/ffi/ffiexports.h

@@ -21,5 +21,6 @@ int getHighestBlock();
 int getCurrentHeight();
 bool pastChangiIntermediateHeight2();
 bool pastChangiIntermediateHeight3();
+bool pastChangiIntermediateHeight4();
 
-#endif // DEFI_FFI_FFIEXPORTS_H
+#endif  // DEFI_FFI_FFIEXPORTS_H

src/masternodes/validation.cpp

@@ -2424,7 +2424,8 @@ static void ProcessEVMQueue(const CBlock &block, const CBlockIndex *pindex, CCus
             minerAddress = GetScriptForDestination(dest);
         }
 
-        const auto blockResult = evm_finalize(evmContext, false, block.nBits, beneficiary, block.GetBlockTime());
+        CrossBoundaryResult result;
+        const auto blockResult = evm_try_finalize(result, evmContext, false, block.nBits, beneficiary, block.GetBlockTime());
         auto evmBlockHash = std::vector<uint8_t>(blockResult.block_hash.begin(), blockResult.block_hash.end());
         std::reverse(evmBlockHash.begin(), evmBlockHash.end());
 

src/miner.cpp

@@ -275,7 +275,8 @@ std::unique_ptr<CBlockTemplate> BlockAssembler::CreateNewBlock(const CScript& sc
     if (IsEVMEnabled(nHeight, mnview, consensus)) {
         std::array<uint8_t, 20> beneficiary{};
         std::copy(nodePtr->ownerAuthAddress.begin(), nodePtr->ownerAuthAddress.end(), beneficiary.begin());
-        auto blockResult = evm_finalize(evmContext, false, pos::GetNextWorkRequired(pindexPrev, pblock->nTime, consensus), beneficiary, blockTime);
+        CrossBoundaryResult result;
+        auto blockResult = evm_try_finalize(result, evmContext, false, pos::GetNextWorkRequired(pindexPrev, pblock->nTime, consensus), beneficiary, blockTime);
         evm_discard_context(evmContext);
 
         const auto blockHash = std::vector<uint8_t>(blockResult.block_hash.begin(), blockResult.block_hash.end());

src/validation.cpp

@@ -3309,7 +3309,12 @@ bool CChainState::ConnectTip(CValidationState& state, const CChainParams& chainp
         nTime3 = GetTimeMicros(); nTimeConnectTotal += nTime3 - nTime2;
         LogPrint(BCLog::BENCH, "  - Connect total: %.2fms [%.2fs (%.2fms/blk)]\n", (nTime3 - nTime2) * MILLI, nTimeConnectTotal * MICRO, nTimeConnectTotal * MILLI / nBlocksTotal);
         if (IsEVMEnabled(pindexNew->nHeight, mnview, chainparams.GetConsensus())) {
-            evm_finalize(evmContext, true, blockConnecting.nBits, beneficiary, blockConnecting.GetBlockTime());
+            CrossBoundaryResult result;
+            evm_try_finalize(result, evmContext, true, blockConnecting.nBits, beneficiary, blockConnecting.GetBlockTime());
+            if (!result.ok && pindexNew->nHeight >= Params().GetConsensus().ChangiIntermediateHeight4) {
+                return error("%s: ConnectBlock %s failed, %s", __func__, pindexNew->GetBlockHash().ToString(), result.reason.c_str());
+            }
+
         }
         bool flushed = view.Flush() && mnview.Flush();
         assert(flushed);