From fdd7127e6144724e3155278c7ca33c21f0def130 Mon Sep 17 00:00:00 2001 From: KaoImin Date: Mon, 27 Nov 2023 15:24:06 +0800 Subject: [PATCH 1/3] refactor!: call reserved system contract is forbidden --- core/api/src/jsonrpc/error.rs | 4 ++ core/api/src/jsonrpc/impl/web3.rs | 17 ++++++ core/executor/src/lib.rs | 19 +------ core/executor/src/system_contract/error.rs | 5 +- core/executor/src/system_contract/mod.rs | 66 +++++++++++++++++++++- core/mempool/src/adapter/mod.rs | 2 +- core/mempool/src/lib.rs | 4 +- 7 files changed, 96 insertions(+), 21 deletions(-) diff --git a/core/api/src/jsonrpc/error.rs b/core/api/src/jsonrpc/error.rs index 5237f0a39..6738d7463 100644 --- a/core/api/src/jsonrpc/error.rs +++ b/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 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()))) diff --git a/core/api/src/jsonrpc/impl/web3.rs b/core/api/src/jsonrpc/impl/web3.rs index 625de63de..2331a2350 100644 --- a/core/api/src/jsonrpc/impl/web3.rs +++ b/core/api/src/jsonrpc/impl/web3.rs @@ -1,5 +1,6 @@ use std::{sync::Arc, time::Duration}; +use core_executor::is_call_system_script; use jsonrpsee::core::RpcResult; use common_apm::metrics_rpc; @@ -419,6 +420,14 @@ impl Web3RpcServer for Web3RpcImpl { 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 Web3RpcServer for Web3RpcImpl { } } + 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, diff --git a/core/executor/src/lib.rs b/core/executor/src/lib.rs index 913875e50..253c6c5d4 100644 --- a/core/executor/src/lib.rs +++ b/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) } diff --git a/core/executor/src/system_contract/error.rs b/core/executor/src/system_contract/error.rs index 6262e690a..e109c6aef 100644 --- a/core/executor/src/system_contract/error.rs +++ b/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 for ProtocolError { diff --git a/core/executor/src/system_contract/mod.rs b/core/executor/src/system_contract/mod.rs index 881882560..87e294edc 100644 --- a/core/executor/src/system_contract/mod.rs +++ b/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 { + 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()); + } +} diff --git a/core/mempool/src/adapter/mod.rs b/core/mempool/src/adapter/mod.rs index 5b5e4c957..ede5e6750 100644 --- a/core/mempool/src/adapter/mod.rs +++ b/core/mempool/src/adapter/mod.rs @@ -372,7 +372,7 @@ where ctx: Context, tx: &SignedTransaction, ) -> ProtocolResult { - 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; } diff --git a/core/mempool/src/lib.rs b/core/mempool/src/lib.rs index fd13212a0..c11817939 100644 --- a/core/mempool/src/lib.rs +++ b/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 { From 1df720ef5773132723d3b2436320b49dcf0f69ac Mon Sep 17 00:00:00 2001 From: KaoImin Date: Mon, 27 Nov 2023 15:40:11 +0800 Subject: [PATCH 2/3] refactor some code --- core/api/src/jsonrpc/impl/web3.rs | 14 +++++++------- core/executor/src/lib.rs | 2 +- core/executor/src/system_contract/mod.rs | 4 ++++ 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/core/api/src/jsonrpc/impl/web3.rs b/core/api/src/jsonrpc/impl/web3.rs index 2331a2350..11fa9d4ae 100644 --- a/core/api/src/jsonrpc/impl/web3.rs +++ b/core/api/src/jsonrpc/impl/web3.rs @@ -1,9 +1,9 @@ use std::{sync::Arc, time::Duration}; -use core_executor::is_call_system_script; use jsonrpsee::core::RpcResult; use common_apm::metrics_rpc; +use core_executor::is_system_contract_address_format; use protocol::traits::{APIAdapter, Context}; use protocol::types::{ Block, BlockNumber, Bytes, EthAccountProof, Hash, Header, Hex, Proposal, Receipt, @@ -421,9 +421,7 @@ impl Web3RpcServer for Web3RpcImpl { } if let Some(call_addr) = req.to { - if is_call_system_script(&TransactionAction::Call(call_addr)) - .map_err(|e| RpcError::Internal(e.to_string()))? - { + if is_system_contract_address_format(&call_addr) { return Err(RpcError::CallSystemContract.into()); } } @@ -463,9 +461,7 @@ impl Web3RpcServer for Web3RpcImpl { } if let Some(call_addr) = req.to { - if is_call_system_script(&TransactionAction::Call(call_addr)) - .map_err(|e| RpcError::Internal(e.to_string()))? - { + if is_system_contract_address_format(&call_addr) { return Err(RpcError::CallSystemContract.into()); } } @@ -1028,6 +1024,10 @@ impl Web3RpcServer for Web3RpcImpl { storage_position: Vec, number: BlockId, ) -> RpcResult { + if is_system_contract_address_format(&address) { + return Err(RpcError::CallSystemContract.into()); + } + let number = self.get_block_number_by_id(Some(number)).await?; let header = self diff --git a/core/executor/src/lib.rs b/core/executor/src/lib.rs index 253c6c5d4..d4c3c44a0 100644 --- a/core/executor/src/lib.rs +++ b/core/executor/src/lib.rs @@ -11,7 +11,7 @@ pub use crate::adapter::{ AxonExecutorApplyAdapter, AxonExecutorReadOnlyAdapter, MPTTrie, RocksTrieDB, }; pub use crate::system_contract::{ - is_call_system_script, + is_call_system_script, is_system_contract_address_format, metadata::{MetadataHandle, HARDFORK_INFO}, DataProvider, }; diff --git a/core/executor/src/system_contract/mod.rs b/core/executor/src/system_contract/mod.rs index 87e294edc..780458de6 100644 --- a/core/executor/src/system_contract/mod.rs +++ b/core/executor/src/system_contract/mod.rs @@ -313,6 +313,10 @@ impl DataProvider { } } +pub fn is_system_contract_address_format(addr: &H160) -> bool { + addr.0.iter().take(19).all(|i| i == &0xff) +} + pub fn is_call_system_script(action: &TransactionAction) -> ProtocolResult { let call_addr = match action { TransactionAction::Call(addr) => addr, From 0fe15ac483c460de9c877a56663f4880a6078baf Mon Sep 17 00:00:00 2001 From: KaoImin Date: Mon, 27 Nov 2023 18:36:48 +0800 Subject: [PATCH 3/3] perf: improve is system contract address format --- core/executor/src/system_contract/mod.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/core/executor/src/system_contract/mod.rs b/core/executor/src/system_contract/mod.rs index 780458de6..148b06056 100644 --- a/core/executor/src/system_contract/mod.rs +++ b/core/executor/src/system_contract/mod.rs @@ -47,6 +47,11 @@ pub const fn system_contract_address(addr: u8) -> H160 { 0xff, 0xff, 0xff, 0xff, addr, ]) } +const SYSTEM_CONTRACT_ADDRESSES_PREFIX: [u8; 19] = [ + 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, // 0xff * 8 + 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, // 0xff * 8 + 0xff, 0xff, 0xff, // 0xff * 3 +]; const SYSTEM_CONTRACT_ADDRESSES_SET: [H160; 4] = [ NATIVE_TOKEN_CONTRACT_ADDRESS, METADATA_CONTRACT_ADDRESS, @@ -314,7 +319,7 @@ impl DataProvider { } pub fn is_system_contract_address_format(addr: &H160) -> bool { - addr.0.iter().take(19).all(|i| i == &0xff) + addr.0[0..19] == SYSTEM_CONTRACT_ADDRESSES_PREFIX } pub fn is_call_system_script(action: &TransactionAction) -> ProtocolResult { @@ -325,7 +330,7 @@ pub fn is_call_system_script(action: &TransactionAction) -> ProtocolResult // 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 call_addr.0[0..19] == SYSTEM_CONTRACT_ADDRESSES_PREFIX { if SYSTEM_CONTRACT_ADDRESSES_SET.contains(call_addr) { return Ok(true); }