Skip to content

Commit

Permalink
Revert buggy miner code and fix segfault (#2221)
Browse files Browse the repository at this point in the history
* Revert "Fix miner crash and refactor transaction removal (#2215)"

This reverts commit 01fb687.

* Fix segfault bug

* Isolate EVM miner processing

* fix lint

* Revert test runner

* Fixes to total fees and gas used

* Remove logging

* Fix merge errors

* Fixes to match rust build triplet and target triplet

* Fix lint

* Fix comments

* Fix merge bug

* Refactor remove txs from sender

* Use multimap for replace by fee

* Fix flaky test

---------

Co-authored-by: Niven <[email protected]>
Co-authored-by: Prasanna Loganathar <[email protected]>
Co-authored-by: Jouzo <[email protected]>
4 people authored Jul 26, 2023
1 parent 9a6e2fb commit aaa4c71
Showing 6 changed files with 161 additions and 143 deletions.
14 changes: 6 additions & 8 deletions lib/ain-evm/src/evm.rs
Original file line number Diff line number Diff line change
@@ -36,8 +36,6 @@ pub struct EVMServices {

pub struct FinalizedBlockInfo {
pub block_hash: [u8; 32],
// TODO: There's no reason for this to be hex encoded and de-coded back again
// We can just send the array of 256 directly, same as block hash.
pub failed_transactions: Vec<String>,
pub total_burnt_fees: U256,
pub total_priority_fees: U256,
@@ -145,8 +143,8 @@ impl EVMServices {

let mut executor = AinExecutor::new(&mut backend);

for (queue_tx, hash) in self.core.tx_queues.get_cloned_vec(queue_id) {
match queue_tx {
for queue_item in self.core.tx_queues.get_cloned_vec(queue_id) {
match queue_item.queue_tx {
QueueTx::SignedTx(signed_tx) => {
if ain_cpp_imports::past_changi_intermediate_height_4_height() {
let nonce = executor.get_nonce(&signed_tx.sender);
@@ -172,7 +170,7 @@ impl EVMServices {
);

if !exit_reason.is_succeed() {
failed_transactions.push(hex::encode(hash));
failed_transactions.push(hex::encode(queue_item.tx_hash));
}

let gas_fee = calculate_gas_fee(&signed_tx, U256::from(used_gas), base_fee)?;
@@ -190,7 +188,7 @@ impl EVMServices {
);
if let Err(e) = executor.add_balance(address, amount) {
debug!("[finalize_block] EvmIn failed with {e}");
failed_transactions.push(hex::encode(hash));
failed_transactions.push(hex::encode(queue_item.tx_hash));
}
}
QueueTx::BridgeTx(BridgeTx::EvmOut(BalanceUpdate { address, amount })) => {
@@ -201,7 +199,7 @@ impl EVMServices {

if let Err(e) = executor.sub_balance(address, amount) {
debug!("[finalize_block] EvmOut failed with {e}");
failed_transactions.push(hex::encode(hash));
failed_transactions.push(hex::encode(queue_item.tx_hash));
}
}
}
@@ -272,7 +270,7 @@ impl EVMServices {
match self.core.tx_queues.get_total_fees(queue_id) {
Some(total_fees) => {
if (total_burnt_fees + total_priority_fees) != U256::from(total_fees) {
return Err(anyhow!("EVM block rejected because block total fees != (burnt fees + priority fees). Burnt fees: {}, priority fees: {}", total_burnt_fees, total_priority_fees).into());
return Err(anyhow!("EVM block rejected because block total fees != (burnt fees + priority fees). Burnt fees: {}, priority fees: {}, total fees: {}", total_burnt_fees, total_priority_fees, total_fees).into());
}
}
None => {
80 changes: 50 additions & 30 deletions lib/ain-evm/src/txqueue.rs
Original file line number Diff line number Diff line change
@@ -94,21 +94,21 @@ impl TransactionQueueMap {
.unwrap()
.get(&queue_id)
.ok_or(QueueError::NoSuchContext)
.map(|queue| queue.queue_tx((tx, hash), gas_used, base_fee))?
.map(|queue| queue.queue_tx(tx, hash, gas_used, base_fee))?
}

/// `drain_all` returns all transactions from the `TransactionQueue` associated with the
/// provided queue ID, removing them from the queue. Transactions are returned in the
/// order they were added.
pub fn drain_all(&self, queue_id: u64) -> Vec<QueueTxWithNativeHash> {
pub fn drain_all(&self, queue_id: u64) -> Vec<QueueTxItem> {
self.queues
.read()
.unwrap()
.get(&queue_id)
.map_or(Vec::new(), TransactionQueue::drain_all)
}

pub fn get_cloned_vec(&self, queue_id: u64) -> Vec<QueueTxWithNativeHash> {
pub fn get_cloned_vec(&self, queue_id: u64) -> Vec<QueueTxItem> {
self.queues
.read()
.unwrap()
@@ -173,14 +173,21 @@ pub enum QueueTx {
BridgeTx(BridgeTx),
}

type QueueTxWithNativeHash = (QueueTx, NativeTxHash);
#[derive(Debug, Clone)]
pub struct QueueTxItem {
pub queue_tx: QueueTx,
pub tx_hash: NativeTxHash,
pub tx_fee: u64,
pub gas_used: u64,
}

/// The `TransactionQueue` holds a queue of transactions and a map of account nonces.
/// The `TransactionQueueData` holds a queue of transactions with a map of the account nonces,
/// the total gas fees and the total gas used by the transactions.
/// It's used to manage and process transactions for different accounts.
///
#[derive(Debug, Default)]
struct TransactionQueueData {
transactions: Vec<QueueTxWithNativeHash>,
transactions: Vec<QueueTxItem>,
account_nonces: HashMap<H160, U256>,
total_fees: u64,
total_gas_used: u64,
@@ -216,28 +223,27 @@ impl TransactionQueue {
data.transactions.clear();
}

pub fn drain_all(&self) -> Vec<QueueTxWithNativeHash> {
pub fn drain_all(&self) -> Vec<QueueTxItem> {
let mut data = self.data.lock().unwrap();
data.total_fees = 0u64;
data.total_gas_used = 0u64;

data.transactions
.drain(..)
.collect::<Vec<QueueTxWithNativeHash>>()
data.transactions.drain(..).collect::<Vec<QueueTxItem>>()
}

pub fn get_cloned_vec(&self) -> Vec<QueueTxWithNativeHash> {
pub fn get_cloned_vec(&self) -> Vec<QueueTxItem> {
self.data.lock().unwrap().transactions.clone()
}

pub fn queue_tx(
&self,
tx: QueueTxWithNativeHash,
tx: QueueTx,
tx_hash: NativeTxHash,
gas_used: u64,
base_fee: U256,
) -> Result<(), QueueError> {
let mut gas_fee: u64 = 0;
let mut data = self.data.lock().unwrap();
if let QueueTx::SignedTx(signed_tx) = &tx.0 {
if let QueueTx::SignedTx(signed_tx) = &tx {
if let Some(nonce) = data.account_nonces.get(&signed_tx.sender) {
if signed_tx.nonce() != nonce + 1 {
return Err(QueueError::InvalidNonce((signed_tx.clone(), *nonce)));
@@ -246,15 +252,20 @@ impl TransactionQueue {
data.account_nonces
.insert(signed_tx.sender, signed_tx.nonce());

let gas_fee = match calculate_gas_fee(signed_tx, gas_used.into(), base_fee) {
gas_fee = match calculate_gas_fee(signed_tx, gas_used.into(), base_fee) {
Ok(fee) => fee.as_u64(),
Err(_) => return Err(QueueError::InvalidFee),
};

data.total_fees += gas_fee;
data.total_gas_used += gas_used;
}
data.transactions.push(tx);
data.transactions.push(QueueTxItem {
queue_tx: tx,
tx_hash,
tx_fee: gas_fee,
gas_used,
});
Ok(())
}

@@ -264,13 +275,22 @@ impl TransactionQueue {

pub fn remove_txs_by_sender(&self, sender: H160) {
let mut data = self.data.lock().unwrap();
data.transactions.retain(|(tx, _)| {
let tx_sender = match tx {
let mut fees_to_remove = 0;
let mut gas_used_to_remove = 0;
data.transactions.retain(|item| {
let tx_sender = match &item.queue_tx {
QueueTx::SignedTx(tx) => tx.sender,
QueueTx::BridgeTx(tx) => tx.sender(),
};
tx_sender != sender
if tx_sender == sender {
fees_to_remove += item.tx_fee;
gas_used_to_remove += item.gas_used;
return false;
}
true
});
data.total_fees -= fees_to_remove;
data.total_gas_used -= gas_used_to_remove;
data.account_nonces.remove(&sender);
}

@@ -341,10 +361,10 @@ mod tests {
// Nonce 0, sender 0xe61a3a6eb316d773c773f4ce757a542f673023c6
let tx3 = QueueTx::SignedTx(Box::new(SignedTx::try_from("f869808502540be400832dc6c0943e338e722607a8c1eab615579ace4f6dedfa19fa80840adb1a9a2aa03d28d24808c3de08c606c5544772ded91913f648ad56556f181905208e206c85a00ecd0ba938fb89fc4a17ea333ea842c7305090dee9236e2b632578f9e5045cb3").unwrap()));

queue.queue_tx((tx1, H256::from_low_u64_be(1).into()), 0u64, U256::zero())?;
queue.queue_tx((tx2, H256::from_low_u64_be(2).into()), 0u64, U256::zero())?;
queue.queue_tx(tx1, H256::from_low_u64_be(1).into(), 0u64, U256::zero())?;
queue.queue_tx(tx2, H256::from_low_u64_be(2).into(), 0u64, U256::zero())?;
// Should fail as nonce 2 is already queued for this sender
let queued = queue.queue_tx((tx3, H256::from_low_u64_be(3).into()), 0u64, U256::zero());
let queued = queue.queue_tx(tx3, H256::from_low_u64_be(3).into(), 0u64, U256::zero());
assert!(matches!(queued, Err(QueueError::InvalidNonce { .. })));
Ok(())
}
@@ -368,11 +388,11 @@ mod tests {
// Nonce 0, sender 0xe61a3a6eb316d773c773f4ce757a542f673023c6
let tx4 = QueueTx::SignedTx(Box::new(SignedTx::try_from("f869808502540be400832dc6c0943e338e722607a8c1eab615579ace4f6dedfa19fa80840adb1a9a2aa03d28d24808c3de08c606c5544772ded91913f648ad56556f181905208e206c85a00ecd0ba938fb89fc4a17ea333ea842c7305090dee9236e2b632578f9e5045cb3").unwrap()));

queue.queue_tx((tx1, H256::from_low_u64_be(1).into()), 0u64, U256::zero())?;
queue.queue_tx((tx2, H256::from_low_u64_be(2).into()), 0u64, U256::zero())?;
queue.queue_tx((tx3, H256::from_low_u64_be(3).into()), 0u64, U256::zero())?;
queue.queue_tx(tx1, H256::from_low_u64_be(1).into(), 0u64, U256::zero())?;
queue.queue_tx(tx2, H256::from_low_u64_be(2).into(), 0u64, U256::zero())?;
queue.queue_tx(tx3, H256::from_low_u64_be(3).into(), 0u64, U256::zero())?;
// Should fail as nonce 2 is already queued for this sender
let queued = queue.queue_tx((tx4, H256::from_low_u64_be(4).into()), 0u64, U256::zero());
let queued = queue.queue_tx(tx4, H256::from_low_u64_be(4).into(), 0u64, U256::zero());
assert!(matches!(queued, Err(QueueError::InvalidNonce { .. })));
Ok(())
}
@@ -393,10 +413,10 @@ mod tests {
// Nonce 2, sender 0x6bc42fd533d6cb9d973604155e1f7197a3b0e703
let tx4 = QueueTx::SignedTx(Box::new(SignedTx::try_from("f869028502540be400832dc6c0943e338e722607a8c1eab615579ace4f6dedfa19fa80840adb1a9a2aa09588b47d2cd3f474d6384309cca5cb8e360cb137679f0a1589a1c184a15cb27ca0453ddbf808b83b279cac3226b61a9d83855aba60ae0d3a8407cba0634da7459d").unwrap()));

queue.queue_tx((tx1, H256::from_low_u64_be(1).into()), 0u64, U256::zero())?;
queue.queue_tx((tx2, H256::from_low_u64_be(2).into()), 0u64, U256::zero())?;
queue.queue_tx((tx3, H256::from_low_u64_be(3).into()), 0u64, U256::zero())?;
queue.queue_tx((tx4, H256::from_low_u64_be(4).into()), 0u64, U256::zero())?;
queue.queue_tx(tx1, H256::from_low_u64_be(1).into(), 0u64, U256::zero())?;
queue.queue_tx(tx2, H256::from_low_u64_be(2).into(), 0u64, U256::zero())?;
queue.queue_tx(tx3, H256::from_low_u64_be(3).into(), 0u64, U256::zero())?;
queue.queue_tx(tx4, H256::from_low_u64_be(4).into(), 0u64, U256::zero())?;
Ok(())
}
}
Loading

0 comments on commit aaa4c71

Please sign in to comment.