Clean up unsafe usages, tweak rust naming (#2294)

* Finalize OP_RETURN default rules

* Cleanup coinbase rewards, getblock

* Cleanup reverse

* Revert "Finalize OP_RETURN default rules"

This reverts commit 87d70f4.

* Add ToUniValue

* Include vminfo on getblock

* Add support for Coinbase

* Multi-layered verbosity

* Refine ToUniValue methods

* Reduce noise

* Avoid confusing names for non UTXO rewards

* Further cleanups

* Minor format cleanup

* Use version terminology

* Mark items as unsafe, rename queue

* More changes

* More changes

* And more

* And then some more

* fmt

* Resolve lints

* And more unsafe marks

* More changes, use results, add CrossBoundaryResVal variants

* Fix lints

* Restore apply/reverse coinbase behaviour

---------

Co-authored-by: Bushstar <[email protected]>

lib/ain-evm/src/core.rs

@@ -181,11 +181,16 @@ impl EVMCoreService {
     /// # Returns
     ///
     /// Returns the signed tx, tx prepay gas fees and the gas used to call the tx.
-    pub fn validate_raw_tx(
+    ///
+    /// # Safety
+    ///
+    /// Result cannot be used safety unless cs_main lock is taken on C++ side
+    /// across all usages. Note: To be replaced with a proper lock flow later.
+    ///
+    pub unsafe fn validate_raw_tx(
         &self,
         tx: &str,
         queue_id: u64,
-        use_context: bool,
     ) -> Result<ValidateTxInfo, Box<dyn Error>> {
         debug!("[validate_raw_tx] raw transaction : {:#?}", tx);
         let signed_tx = SignedTx::try_from(tx)
@@ -256,7 +261,8 @@ impl EVMCoreService {
             return Err(format_err!("gas limit higher than MAX_GAS_PER_BLOCK").into());
         }
 
-        let used_gas = if use_context {
+        let use_queue = queue_id != 0;
+        let used_gas = if use_queue {
             let TxResponse { used_gas, .. } = self.call(EthCallArgs {
                 caller: Some(signed_tx.sender),
                 to: signed_tx.to(),
@@ -272,11 +278,11 @@ impl EVMCoreService {
         };
 
         // Validate total gas usage in queued txs exceeds block size
-        if use_context {
+        if use_queue {
             debug!("[validate_raw_tx] used_gas: {:#?}", used_gas);
             let total_current_gas_used = self
                 .tx_queues
-                .get_total_gas_used(queue_id)
+                .get_total_gas_used_in(queue_id)
                 .unwrap_or_default();
 
             if total_current_gas_used + U256::from(used_gas) > MAX_GAS_PER_BLOCK {
@@ -303,7 +309,13 @@ impl EVMCoreService {
 
 // Transaction queue methods
 impl EVMCoreService {
-    pub fn add_balance(
+    ///
+    /// # Safety
+    ///
+    /// Result cannot be used safety unless cs_main lock is taken on C++ side
+    /// across all usages. Note: To be replaced with a proper lock flow later.
+    ///
+    pub unsafe fn add_balance(
         &self,
         queue_id: u64,
         address: H160,
@@ -312,11 +324,17 @@ impl EVMCoreService {
     ) -> Result<(), EVMError> {
         let queue_tx = QueueTx::SystemTx(SystemTx::EvmIn(BalanceUpdate { address, amount }));
         self.tx_queues
-            .queue_tx(queue_id, queue_tx, hash, U256::zero(), U256::zero())?;
+            .push_in(queue_id, queue_tx, hash, U256::zero(), U256::zero())?;
         Ok(())
     }
 
-    pub fn sub_balance(
+    ///
+    /// # Safety
+    ///
+    /// Result cannot be used safety unless cs_main lock is taken on C++ side
+    /// across all usages. Note: To be replaced with a proper lock flow later.
+    ///
+    pub unsafe fn sub_balance(
         &self,
         queue_id: u64,
         address: H160,
@@ -338,21 +356,43 @@ impl EVMCoreService {
         } else {
             let queue_tx = QueueTx::SystemTx(SystemTx::EvmOut(BalanceUpdate { address, amount }));
             self.tx_queues
-                .queue_tx(queue_id, queue_tx, hash, U256::zero(), U256::zero())?;
+                .push_in(queue_id, queue_tx, hash, U256::zero(), U256::zero())?;
             Ok(())
         }
     }
 
-    pub fn get_queue_id(&self) -> u64 {
-        self.tx_queues.get_queue_id()
+    ///
+    /// # Safety
+    ///
+    /// Result cannot be used safety unless cs_main lock is taken on C++ side
+    /// across all usages. Note: To be replaced with a proper lock flow later.
+    ///
+    pub unsafe fn create_queue(&self) -> u64 {
+        self.tx_queues.create()
     }
 
-    pub fn remove(&self, queue_id: u64) {
+    ///
+    /// # Safety
+    ///
+    /// Result cannot be used safety unless cs_main lock is taken on C++ side
+    /// across all usages. Note: To be replaced with a proper lock flow later.
+    ///
+    pub unsafe fn remove_queue(&self, queue_id: u64) {
         self.tx_queues.remove(queue_id);
     }
 
-    pub fn remove_txs_by_sender(&self, queue_id: u64, address: H160) -> Result<(), EVMError> {
-        self.tx_queues.remove_txs_by_sender(queue_id, address)?;
+    ///
+    /// # Safety
+    ///
+    /// Result cannot be used safety unless cs_main lock is taken on C++ side
+    /// across all usages. Note: To be replaced with a proper lock flow later.
+    ///
+    pub unsafe fn remove_txs_by_sender_in(
+        &self,
+        queue_id: u64,
+        address: H160,
+    ) -> Result<(), EVMError> {
+        self.tx_queues.remove_by_sender_in(queue_id, address)?;
         Ok(())
     }
 
@@ -375,14 +415,20 @@ impl EVMCoreService {
     /// # Returns
     ///
     /// Returns the next valid nonce as a `U256`. Defaults to U256::zero()
-    pub fn get_next_valid_nonce_in_queue(
+    ///
+    /// # Safety
+    ///
+    /// Result cannot be used safety unless cs_main lock is taken on C++ side
+    /// across all usages. Note: To be replaced with a proper lock flow later.
+    ///
+    pub unsafe fn get_next_valid_nonce_in_queue(
         &self,
         queue_id: u64,
         address: H160,
     ) -> Result<U256, QueueError> {
         let nonce = self
             .tx_queues
-            .get_next_valid_nonce(queue_id, address)?
+            .get_next_valid_nonce_in(queue_id, address)?
             .unwrap_or_else(|| {
                 let latest_block = self
                     .storage

lib/ain-evm/src/evm.rs

@@ -99,20 +99,26 @@ impl EVMServices {
         }
     }
 
-    pub fn construct_block(
+    ///
+    /// # Safety
+    ///
+    /// Result cannot be used safety unless cs_main lock is taken on C++ side
+    /// across all usages. Note: To be replaced with a proper lock flow later.
+    ///
+    pub unsafe fn construct_block_in_queue(
         &self,
         queue_id: u64,
         difficulty: u32,
         beneficiary: H160,
         timestamp: u64,
         dvm_block_number: u64,
     ) -> Result<FinalizedBlockInfo, Box<dyn Error>> {
-        let tx_queue = self.core.tx_queues.get_queue(queue_id)?;
+        let tx_queue = self.core.tx_queues.get(queue_id)?;
         let mut queue = tx_queue.data.lock().unwrap();
-        let queue_len = queue.transactions.len();
-        let mut all_transactions = Vec::with_capacity(queue_len);
-        let mut failed_transactions = Vec::with_capacity(queue_len);
-        let mut receipts_v3: Vec<ReceiptV3> = Vec::with_capacity(queue_len);
+        let queue_txs_len = queue.transactions.len();
+        let mut all_transactions = Vec::with_capacity(queue_txs_len);
+        let mut failed_transactions = Vec::with_capacity(queue_txs_len);
+        let mut receipts_v3: Vec<ReceiptV3> = Vec::with_capacity(queue_txs_len);
         let mut total_gas_used = 0u64;
         let mut total_gas_fees = U256::zero();
         let mut logs_bloom: Bloom = Bloom::default();
@@ -340,9 +346,15 @@ impl EVMServices {
         })
     }
 
-    pub fn finalize_block(&self, queue_id: u64) -> Result<(), Box<dyn Error>> {
+    ///
+    /// # Safety
+    ///
+    /// Result cannot be used safety unless cs_main lock is taken on C++ side
+    /// across all usages. Note: To be replaced with a proper lock flow later.
+    ///
+    pub unsafe fn commit_queue(&self, queue_id: u64) -> Result<(), Box<dyn Error>> {
         {
-            let tx_queue = self.core.tx_queues.get_queue(queue_id)?;
+            let tx_queue = self.core.tx_queues.get(queue_id)?;
             let queue = tx_queue.data.lock().unwrap();
             let Some(BlockData { block, receipts }) = queue.block_data.clone() else {
                 return Err(format_err!("no constructed EVM block exist in queue id").into());
@@ -383,7 +395,13 @@ impl EVMServices {
         Ok(())
     }
 
-    pub fn queue_tx(
+    ///
+    /// # Safety
+    ///
+    /// Result cannot be used safety unless cs_main lock is taken on C++ side
+    /// across all usages. Note: To be replaced with a proper lock flow later.
+    ///
+    pub unsafe fn push_tx_in_queue(
         &self,
         queue_id: u64,
         tx: QueueTx,
@@ -399,7 +417,7 @@ impl EVMServices {
 
         self.core
             .tx_queues
-            .queue_tx(queue_id, tx.clone(), hash, gas_used, base_fee)?;
+            .push_in(queue_id, tx.clone(), hash, gas_used, base_fee)?;
 
         if let QueueTx::SignedTx(signed_tx) = tx {
             self.filters.add_tx_to_filters(signed_tx.transaction.hash());

lib/ain-evm/src/txqueue.rs

@@ -37,7 +37,13 @@ impl TransactionQueueMap {
 
     /// `get_queue_id` generates a unique random ID, creates a new `TransactionQueue` for that ID,
     /// and then returns the ID.
-    pub fn get_queue_id(&self) -> u64 {
+    ///
+    /// # Safety
+    ///
+    /// Result cannot be used safety unless cs_main lock is taken on C++ side
+    /// across all usages. Note: To be replaced with a proper lock flow later.
+    ///
+    pub unsafe fn create(&self) -> u64 {
         let mut rng = rand::thread_rng();
         loop {
             let queue_id = rng.gen();
@@ -56,7 +62,13 @@ impl TransactionQueueMap {
 
     /// Try to remove and return the `TransactionQueue` associated with the provided
     /// queue ID.
-    pub fn remove(&self, queue_id: u64) -> Option<Arc<TransactionQueue>> {
+    ///
+    /// # Safety
+    ///
+    /// Result cannot be used safety unless cs_main lock is taken on C++ side
+    /// across all usages. Note: To be replaced with a proper lock flow later.
+    ///
+    pub unsafe fn remove(&self, queue_id: u64) -> Option<Arc<TransactionQueue>> {
         self.queues.write().unwrap().remove(&queue_id)
     }
 
@@ -69,7 +81,12 @@ impl TransactionQueueMap {
     ///
     /// Returns `QueueError::NoSuchQueue` if no queue is associated with the given queue ID.
     ///
-    pub fn get_queue(&self, queue_id: u64) -> Result<Arc<TransactionQueue>> {
+    /// # Safety
+    ///
+    /// Result cannot be used safety unless cs_main lock is taken on C++ side
+    /// across all usages. Note: To be replaced with a proper lock flow later.
+    ///
+    pub unsafe fn get(&self, queue_id: u64) -> Result<Arc<TransactionQueue>> {
         Ok(Arc::clone(
             self.queues
                 .read()
@@ -93,7 +110,12 @@ impl TransactionQueueMap {
     /// previous nonce of transactions from the same sender in the queue.
     /// Returns `QueueError::InvalidFee` if the fee calculation overflows.
     ///
-    pub fn queue_tx(
+    /// # Safety
+    ///
+    /// Result cannot be used safety unless cs_main lock is taken on C++ side
+    /// across all usages. Note: To be replaced with a proper lock flow later.
+    ///
+    pub unsafe fn push_in(
         &self,
         queue_id: u64,
         tx: QueueTx,
@@ -112,12 +134,23 @@ impl TransactionQueueMap {
     ///
     /// Returns `QueueError::NoSuchQueue` if no queue is associated with the given queue ID.
     ///
-    pub fn remove_txs_by_sender(&self, queue_id: u64, sender: H160) -> Result<()> {
+    /// # Safety
+    ///
+    /// Result cannot be used safety unless cs_main lock is taken on C++ side
+    /// across all usages. Note: To be replaced with a proper lock flow later.
+    ///
+    pub unsafe fn remove_by_sender_in(&self, queue_id: u64, sender: H160) -> Result<()> {
         self.with_transaction_queue(queue_id, |queue| queue.remove_txs_by_sender(sender))
     }
 
-    pub fn get_tx_queue_items(&self, queue_id: u64) -> Result<Vec<QueueTxItem>> {
-        self.with_transaction_queue(queue_id, TransactionQueue::get_tx_queue_items)
+    ///
+    /// # Safety
+    ///
+    /// Result cannot be used safety unless cs_main lock is taken on C++ side
+    /// across all usages. Note: To be replaced with a proper lock flow later.
+    ///
+    pub unsafe fn get_txs_cloned_in(&self, queue_id: u64) -> Result<Vec<QueueTxItem>> {
+        self.with_transaction_queue(queue_id, TransactionQueue::get_queue_txs_cloned)
     }
 
     /// `get_next_valid_nonce` returns the next valid nonce for the account with the provided address
@@ -131,19 +164,33 @@ impl TransactionQueueMap {
     /// Returns None when the address does not have any transaction queued or
     /// Some(nonce) with the next valid nonce (current + 1) for the associated address
     ///
-    pub fn get_next_valid_nonce(&self, queue_id: u64, address: H160) -> Result<Option<U256>> {
+    /// # Safety
+    ///
+    /// Result cannot be used safety unless cs_main lock is taken on C++ side
+    /// across all usages. Note: To be replaced with a proper lock flow later.
+    ///
+    pub unsafe fn get_next_valid_nonce_in(
+        &self,
+        queue_id: u64,
+        address: H160,
+    ) -> Result<Option<U256>> {
         self.with_transaction_queue(queue_id, |queue| queue.get_next_valid_nonce(address))
     }
 
-    pub fn get_total_gas_used(&self, queue_id: u64) -> Result<U256> {
+    /// # Safety
+    ///
+    /// Result cannot be used safety unless cs_main lock is taken on C++ side
+    /// across all usages. Note: To be replaced with a proper lock flow later.
+    ///
+    pub unsafe fn get_total_gas_used_in(&self, queue_id: u64) -> Result<U256> {
         self.with_transaction_queue(queue_id, |queue| queue.get_total_gas_used())
     }
 
     /// Apply the closure to the queue associated with the queue ID.
     /// # Errors
     ///
     /// Returns `QueueError::NoSuchQueue` if no queue is associated with the given queue ID.
-    pub fn with_transaction_queue<T, F>(&self, queue_id: u64, f: F) -> Result<T>
+    unsafe fn with_transaction_queue<T, F>(&self, queue_id: u64, f: F) -> Result<T>
     where
         F: FnOnce(&TransactionQueue) -> T,
     {
@@ -271,7 +318,7 @@ impl TransactionQueue {
         data.account_nonces.remove(&sender);
     }
 
-    pub fn get_tx_queue_items(&self) -> Vec<QueueTxItem> {
+    pub fn get_queue_txs_cloned(&self) -> Vec<QueueTxItem> {
         self.data.lock().unwrap().transactions.clone()
     }