refactor!: call reserved system contract is forbidden

core/api/src/jsonrpc/error.rs

@@ -51,6 +51,8 @@ pub enum RpcError {
     InvalidFromBlockAndToBlockUnion,
     #[display(fmt = "Invalid filter id {}", _0)]
     CannotFindFilterId(u64),
+    #[display(fmt = "Not allow to call system contract address")]
+    CallSystemContract,
 
     #[display(fmt = "EVM error {}", "decode_revert_msg(&_0.ret)")]
     Evm(TxResp),
@@ -88,6 +90,7 @@ impl RpcError {
             RpcError::InvalidRewardPercentiles(_, _) => -40020,
             RpcError::InvalidFromBlockAndToBlockUnion => -40021,
             RpcError::CannotFindFilterId(_) => -40022,
+            RpcError::CallSystemContract => -40023,
 
             RpcError::Evm(_) => -49998,
             RpcError::Internal(_) => -49999,
@@ -129,6 +132,7 @@ impl From<RpcError> for ErrorObjectOwned {
                 ErrorObject::owned(err_code, err, none_data)
             }
             RpcError::CannotFindFilterId(_) => ErrorObject::owned(err_code, err, none_data),
+            RpcError::CallSystemContract => ErrorObject::owned(err_code, err, none_data),
 
             RpcError::Evm(resp) => {
                 ErrorObject::owned(err_code, err.clone(), Some(vm_err(resp.clone())))

core/api/src/jsonrpc/impl/web3.rs

@@ -1,13 +1,14 @@
 use std::{sync::Arc, time::Duration};
 
+use core_executor::is_call_system_script;
 use jsonrpsee::core::RpcResult;
 
 use common_apm::metrics_rpc;
 use protocol::traits::{APIAdapter, Context};
 use protocol::types::{
     Block, BlockNumber, Bytes, EthAccountProof, Hash, Header, Hex, Proposal, Receipt,
-    SignedTransaction, TxResp, UnverifiedTransaction, BASE_FEE_PER_GAS, H160, H256,
-    MAX_FEE_HISTORY, MAX_RPC_GAS_CAP, MIN_TRANSACTION_GAS_LIMIT, U256,
+    SignedTransaction, TransactionAction, TxResp, UnverifiedTransaction, BASE_FEE_PER_GAS, H160,
+    H256, MAX_FEE_HISTORY, MAX_RPC_GAS_CAP, MIN_TRANSACTION_GAS_LIMIT, U256,
 };
 use protocol::{
     async_trait, codec::ProtocolCodec, lazy::PROTOCOL_VERSION, tokio::time::sleep, ProtocolResult,
@@ -419,6 +420,14 @@ impl<Adapter: APIAdapter + 'static> Web3RpcServer for Web3RpcImpl<Adapter> {
             return Err(RpcError::GasLimitIsTooLarge.into());
         }
 
+        if let Some(call_addr) = req.to {
+            if is_call_system_script(&TransactionAction::Call(call_addr))
+                .map_err(|e| RpcError::Internal(e.to_string()))?
+            {
+                return Err(RpcError::CallSystemContract.into());
+            }
+        }
+
         let number = self.get_block_number_by_id(block_id).await?;
 
         let data_bytes = req
@@ -453,6 +462,14 @@ impl<Adapter: APIAdapter + 'static> Web3RpcServer for Web3RpcImpl<Adapter> {
             }
         }
 
+        if let Some(call_addr) = req.to {
+            if is_call_system_script(&TransactionAction::Call(call_addr))
+                .map_err(|e| RpcError::Internal(e.to_string()))?
+            {
+                return Err(RpcError::CallSystemContract.into());
+            }
+        }
+
         let num = match number {
             Some(BlockId::Num(n)) => Some(n.as_u64()),
             _ => None,

core/executor/src/lib.rs

@@ -11,6 +11,7 @@ pub use crate::adapter::{
     AxonExecutorApplyAdapter, AxonExecutorReadOnlyAdapter, MPTTrie, RocksTrieDB,
 };
 pub use crate::system_contract::{
+    is_call_system_script,
     metadata::{MetadataHandle, HARDFORK_INFO},
     DataProvider,
 };
@@ -34,8 +35,8 @@ use protocol::types::{
 use crate::precompiles::build_precompile_set;
 use crate::system_contract::{
     after_block_hook, before_block_hook, system_contract_dispatch,
-    CKB_LIGHT_CLIENT_CONTRACT_ADDRESS, HEADER_CELL_ROOT_KEY, IMAGE_CELL_CONTRACT_ADDRESS,
-    METADATA_CONTRACT_ADDRESS, METADATA_ROOT_KEY, NATIVE_TOKEN_CONTRACT_ADDRESS,
+    CKB_LIGHT_CLIENT_CONTRACT_ADDRESS, HEADER_CELL_ROOT_KEY, METADATA_CONTRACT_ADDRESS,
+    METADATA_ROOT_KEY,
 };
 
 lazy_static::lazy_static! {
@@ -491,20 +492,6 @@ impl AxonExecutor {
     }
 }
 
-pub fn is_call_system_script(action: &TransactionAction) -> bool {
-    let system_contracts = [
-        NATIVE_TOKEN_CONTRACT_ADDRESS,
-        METADATA_CONTRACT_ADDRESS,
-        CKB_LIGHT_CLIENT_CONTRACT_ADDRESS,
-        IMAGE_CELL_CONTRACT_ADDRESS,
-    ];
-
-    match action {
-        TransactionAction::Call(addr) => system_contracts.contains(addr),
-        TransactionAction::Create => false,
-    }
-}
-
 pub fn is_transaction_call(action: &TransactionAction, addr: &H160) -> bool {
     action == &TransactionAction::Call(*addr)
 }

core/executor/src/system_contract/error.rs

@@ -3,7 +3,7 @@ use std::io;
 use ethers::abi::AbiError;
 use thiserror::Error;
 
-use protocol::{ProtocolError, ProtocolErrorKind};
+use protocol::{types::H160, ProtocolError, ProtocolErrorKind};
 
 #[derive(Error, Debug)]
 pub enum SystemScriptError {
@@ -84,6 +84,9 @@ pub enum SystemScriptError {
 
     #[error("Metadata version is discontinuous")]
     MetadataVersionDiscontinuity,
+
+    #[error("Call a reserved system contract address {0}")]
+    ReservedAddress(H160),
 }
 
 impl From<SystemScriptError> for ProtocolError {

core/executor/src/system_contract/mod.rs

@@ -9,6 +9,7 @@ pub mod metadata;
 pub use crate::system_contract::ckb_light_client::{
     CkbLightClientContract, CKB_LIGHT_CLIENT_CONTRACT_ADDRESS,
 };
+use crate::system_contract::error::SystemScriptError;
 pub use crate::system_contract::image_cell::{ImageCellContract, IMAGE_CELL_CONTRACT_ADDRESS};
 pub use crate::system_contract::metadata::{
     check_ckb_related_info_exist, MetadataContract, METADATA_CONTRACT_ADDRESS,
@@ -29,7 +30,8 @@ use rocksdb::DB;
 
 use protocol::traits::{CkbDataProvider, ExecutorAdapter};
 use protocol::types::{
-    Bytes, HardforkInfoInner, Hasher, Metadata, SignedTransaction, TxResp, H160, H256,
+    Bytes, HardforkInfoInner, Hasher, Metadata, SignedTransaction, TransactionAction, TxResp, H160,
+    H256,
 };
 use protocol::{ckb_blake2b_256, ProtocolResult};
 
@@ -45,6 +47,12 @@ pub const fn system_contract_address(addr: u8) -> H160 {
         0xff, 0xff, 0xff, 0xff, addr,
     ])
 }
+const SYSTEM_CONTRACT_ADDRESSES_SET: [H160; 4] = [
+    NATIVE_TOKEN_CONTRACT_ADDRESS,
+    METADATA_CONTRACT_ADDRESS,
+    CKB_LIGHT_CLIENT_CONTRACT_ADDRESS,
+    IMAGE_CELL_CONTRACT_ADDRESS,
+];
 const HEADER_CELL_DB_CACHE_SIZE: usize = 200;
 const METADATA_DB_CACHE_SIZE: usize = 10;
 
@@ -304,3 +312,59 @@ impl DataProvider {
         DataProvider { root }
     }
 }
+
+pub fn is_call_system_script(action: &TransactionAction) -> ProtocolResult<bool> {
+    let call_addr = match action {
+        TransactionAction::Call(addr) => addr,
+        TransactionAction::Create => return Ok(false),
+    };
+
+    // The first 19 bytes of the address are 0xff, which means that the address
+    // follows system contract address format.
+    if call_addr.0.iter().take(19).all(|i| i == &0xff) {
+        if SYSTEM_CONTRACT_ADDRESSES_SET.contains(call_addr) {
+            return Ok(true);
+        }
+
+        // Call a reserved system contract address returns error.
+        return Err(SystemScriptError::ReservedAddress(*call_addr).into());
+    }
+
+    // The address is not a system contract address.
+    Ok(false)
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_is_call_system_contract() {
+        let action = TransactionAction::Create;
+        assert!(!is_call_system_script(&action).unwrap());
+
+        let addr = H160::from_low_u64_be(0x1);
+        let action = TransactionAction::Call(addr);
+        assert!(!is_call_system_script(&action).unwrap());
+
+        let addr = NATIVE_TOKEN_CONTRACT_ADDRESS;
+        let action = TransactionAction::Call(addr);
+        assert!(is_call_system_script(&action).unwrap());
+
+        let addr = METADATA_CONTRACT_ADDRESS;
+        let action = TransactionAction::Call(addr);
+        assert!(is_call_system_script(&action).unwrap());
+
+        let addr = CKB_LIGHT_CLIENT_CONTRACT_ADDRESS;
+        let action = TransactionAction::Call(addr);
+        assert!(is_call_system_script(&action).unwrap());
+
+        let addr = IMAGE_CELL_CONTRACT_ADDRESS;
+        let action = TransactionAction::Call(addr);
+        assert!(is_call_system_script(&action).unwrap());
+
+        let addr = system_contract_address(0x4);
+        let action = TransactionAction::Call(addr);
+        assert!(is_call_system_script(&action).is_err());
+    }
+}

core/mempool/src/adapter/mod.rs

@@ -372,7 +372,7 @@ where
         ctx: Context,
         tx: &SignedTransaction,
     ) -> ProtocolResult<U256> {
-        if is_call_system_script(tx.transaction.unsigned.action()) {
+        if is_call_system_script(tx.transaction.unsigned.action())? {
             return self.check_system_script_tx_authorization(ctx, tx).await;
         }
 

core/mempool/src/lib.rs

@@ -183,7 +183,7 @@ where
     Adapter: MemPoolAdapter + 'static,
 {
     async fn insert(&self, ctx: Context, tx: SignedTransaction) -> ProtocolResult<()> {
-        let is_call_system_script = is_call_system_script(tx.transaction.unsigned.action());
+        let is_call_system_script = is_call_system_script(tx.transaction.unsigned.action())?;
 
         log::debug!(
             "[mempool]: is call system script {:?}",
@@ -306,7 +306,7 @@ where
 
             for (signed_tx, check_nonce) in txs.into_iter().zip(check_nonces.into_iter()) {
                 let is_call_system_script =
-                    is_call_system_script(signed_tx.transaction.unsigned.action());
+                    is_call_system_script(signed_tx.transaction.unsigned.action())?;
                 if is_call_system_script {
                     self.pool.insert_system_script_tx(signed_tx)?;
                 } else {