Skip to content

Commit

Permalink
Pr comment changes
Browse files Browse the repository at this point in the history
- refactored handling of missing receipts
- cut down on a vec iteration in looking up transaction pending/queued
  • Loading branch information
maxconway committed Jan 27, 2025
1 parent a3eeff6 commit 8993108
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 24 deletions.
4 changes: 2 additions & 2 deletions zilliqa/src/api/types/zil.rs
Original file line number Diff line number Diff line change
Expand Up @@ -727,7 +727,7 @@ pub enum TransactionState {
impl TransactionStatusResponse {
pub fn new(
tx: VerifiedTransaction,
receipt: TransactionReceipt,
success: bool,
block: Option<Block>,
state: TransactionState,
) -> Result<Self> {
Expand Down Expand Up @@ -830,7 +830,7 @@ impl TransactionStatusResponse {
nonce: nonce.to_string(),
sender_addr: sender_pub_key,
signature,
success: receipt.success,
success,
to_addr: to_addr.to_hex(),
version: version.to_string(),
})
Expand Down
35 changes: 14 additions & 21 deletions zilliqa/src/api/zilliqa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ use crate::{
exec::zil_contract_address,
message::Block,
node::Node,
pool::TxAddResult,
pool::{PendingOrQueued, TxAddResult},
schnorr,
scilla::{split_storage_key, storage_key, ParamValue},
state::Code,
Expand Down Expand Up @@ -1487,18 +1487,16 @@ fn get_transaction_status(
"Txn Hash not found".to_string(),
jsonrpc_error_data.clone(),
))?;
let receipt =
node.get_transaction_receipt(hash)?
.ok_or(jsonrpsee::types::ErrorObject::owned(
RPCErrorCode::RpcDatabaseError as i32,
"Txn receipt not found".to_string(),
jsonrpc_error_data.clone(),
))?;
let receipt = node.get_transaction_receipt(hash)?;

let block = node.get_block(receipt.block_hash)?;
let (block, success) = if let Some(receipt) = &receipt {
(node.get_block(receipt.block_hash)?, receipt.success)
} else {
(None, false)
};

// Determine transaction state
let state = if !receipt.errors.is_empty() {
let state = if receipt.is_some_and(|receipt| !receipt.errors.is_empty()) {
TransactionState::Error
} else {
match &block {
Expand All @@ -1513,20 +1511,15 @@ fn get_transaction_status(
TransactionState::Pending
}
}
None => {
let mempool = node.consensus.txpool_content()?;
if mempool.queued.iter().any(|x| x.hash == hash) {
TransactionState::Queued
} else if mempool.pending.iter().any(|x| x.hash == hash) {
TransactionState::Pending
} else {
TransactionState::Error
}
}
None => match node.consensus.get_pending_or_queued(hash)? {
Some(PendingOrQueued::Pending) => TransactionState::Pending,
Some(PendingOrQueued::Queued) => TransactionState::Queued,
None => panic!("Transaction not found in block or pending/queued"),
},
}
};

TransactionStatusResponse::new(transaction, receipt, block, state)
TransactionStatusResponse::new(transaction, success, block, state)
}

#[cfg(test)]
Expand Down
7 changes: 6 additions & 1 deletion zilliqa/src/consensus.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ use crate::{
Vote, MAX_COMMITTEE_SIZE,
},
node::{MessageSender, NetworkMessage, OutgoingMessageFailure},
pool::{TransactionPool, TxAddResult, TxPoolContent},
pool::{PendingOrQueued, TransactionPool, TxAddResult, TxPoolContent},
range_map::RangeMap,
state::State,
time::SystemTime,
Expand Down Expand Up @@ -975,6 +975,11 @@ impl Consensus {
self.transaction_pool.preview_content(&self.state)
}

pub fn get_pending_or_queued(&self, tx_hash: Hash) -> Result<Option<PendingOrQueued>> {
self.transaction_pool
.get_pending_or_queued(&self.state, tx_hash)
}

pub fn pending_transaction_count(&self, account: Address) -> u64 {
let current_nonce = self.state.must_get_account(account).nonce;

Expand Down
22 changes: 22 additions & 0 deletions zilliqa/src/pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,12 @@ pub enum TxAddResult {
SameNonceButLowerGasPrice,
}

/// For transaction status returns
pub enum PendingOrQueued {
Pending,
Queued,
}

impl TxAddResult {
pub fn was_added(&self) -> bool {
matches!(self, Self::AddedToMempool)
Expand Down Expand Up @@ -163,6 +169,22 @@ impl TransactionPool {
Ok(None)
}

/// Returns whether the transaction is pending or queued
pub fn get_pending_or_queued(
&self,
state: &State,
tx_hash: Hash,
) -> Result<Option<PendingOrQueued>> {
let pending_txns = self.pending_transactions(state)?;
if pending_txns.iter().any(|txn| txn.hash == tx_hash) {
Ok(Some(PendingOrQueued::Pending))
} else if self.hash_to_index.contains_key(&tx_hash) {
Ok(Some(PendingOrQueued::Queued))
} else {
Ok(None)
}
}

/// Returns a list of txns that are pending for inclusion in the next block
pub fn pending_transactions(&self, state: &State) -> Result<Vec<&VerifiedTransaction>> {
// Keeps track of [account, cumulative_txns_cost]
Expand Down

0 comments on commit 8993108

Please sign in to comment.