Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clean up unsafe usages, tweak rust naming #2294

Merged
merged 27 commits into from
Aug 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
87d70f4
Finalize OP_RETURN default rules
prasannavl Aug 5, 2023
8b3539e
Cleanup coinbase rewards, getblock
prasannavl Aug 6, 2023
bb5bf45
Cleanup reverse
prasannavl Aug 6, 2023
c7936ad
Revert "Finalize OP_RETURN default rules"
prasannavl Aug 6, 2023
5c22a62
Add ToUniValue
prasannavl Aug 6, 2023
4e33856
Include vminfo on getblock
prasannavl Aug 6, 2023
de0b6bd
Add support for Coinbase
prasannavl Aug 6, 2023
ef42120
Multi-layered verbosity
prasannavl Aug 6, 2023
b93b71d
Refine ToUniValue methods
prasannavl Aug 6, 2023
8ef5844
Reduce noise
prasannavl Aug 6, 2023
f7eb0e7
Avoid confusing names for non UTXO rewards
prasannavl Aug 7, 2023
0fbe762
Further cleanups
prasannavl Aug 7, 2023
fafe2f1
Minor format cleanup
prasannavl Aug 7, 2023
4afec3f
Use version terminology
prasannavl Aug 7, 2023
6c816d0
Mark items as unsafe, rename queue
prasannavl Aug 7, 2023
421ac97
More changes
prasannavl Aug 7, 2023
22fe4e3
More changes
prasannavl Aug 7, 2023
d849a22
And more
prasannavl Aug 7, 2023
248353d
And then some more
prasannavl Aug 7, 2023
cbd13b8
fmt
prasannavl Aug 7, 2023
9c9d9e1
Resolve lints
prasannavl Aug 7, 2023
376b897
And more unsafe marks
prasannavl Aug 7, 2023
9cfa962
More changes, use results, add CrossBoundaryResVal variants
prasannavl Aug 7, 2023
40c8f34
Fix lints
prasannavl Aug 7, 2023
ef163e3
Restore apply/reverse coinbase behaviour
Bushstar Aug 7, 2023
6feba73
Merge branch 'master' into pvl/unsafe-rs
prasannavl Aug 7, 2023
7d8180d
Merge branch 'master' into pvl/unsafe-rs
prasannavl Aug 7, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 62 additions & 16 deletions lib/ain-evm/src/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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(),
Expand All @@ -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 {
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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(())
}

Expand All @@ -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
Expand Down
38 changes: 28 additions & 10 deletions lib/ain-evm/src/evm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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,
Expand All @@ -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());
Expand Down
69 changes: 58 additions & 11 deletions lib/ain-evm/src/txqueue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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)
}

Expand All @@ -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()
Expand All @@ -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,
Expand All @@ -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
Expand All @@ -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,
{
Expand Down Expand Up @@ -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()
}

Expand Down
Loading