Skip to content

Commit 43d1fd7

Browse files
akhi3030shreyan-gupta
authored andcommitted
refactor: some cleanups in process_tx_internal (near#13003)
- Remove some more `?` by using alternative means to get the data. - reduce indentation by inserting explicit `return`s. This makes it a bit easier to understand what the code is doing. Still I think there is room for improvement but bit by bit.
1 parent 6fd75cf commit 43d1fd7

File tree

1 file changed

+53
-54
lines changed

1 file changed

+53
-54
lines changed

chain/client/src/client.rs

+53-54
Original file line numberDiff line numberDiff line change
@@ -2242,6 +2242,7 @@ impl Client {
22422242
self.shard_tracker.care_about_shard(me, &head.last_block_hash, shard_id, true);
22432243
let will_care_about_shard =
22442244
self.shard_tracker.will_care_about_shard(me, &head.last_block_hash, shard_id, true);
2245+
22452246
if care_about_shard || will_care_about_shard {
22462247
let shard_uid = shard_id_to_uid(self.epoch_manager.as_ref(), shard_id, &epoch_id)?;
22472248
let state_root = match self.chain.get_chunk_extra(&head.last_block_hash, &shard_uid) {
@@ -2267,69 +2268,67 @@ impl Client {
22672268
receiver_congestion_info,
22682269
) {
22692270
debug!(target: "client", ?err, "Invalid tx");
2270-
Ok(ProcessTxResponse::InvalidTx(err))
2271-
} else if check_only {
2272-
Ok(ProcessTxResponse::ValidTx)
2273-
} else {
2274-
// Transactions only need to be recorded if the node is a validator.
2275-
if me.is_some() {
2276-
match self
2277-
.chunk_producer
2278-
.sharded_tx_pool
2279-
.insert_transaction(shard_uid, tx.clone())
2280-
{
2281-
InsertTransactionResult::Success => {
2282-
trace!(target: "client", ?shard_uid, tx_hash = ?tx.get_hash(), "Recorded a transaction.");
2283-
}
2284-
InsertTransactionResult::Duplicate => {
2285-
trace!(target: "client", ?shard_uid, tx_hash = ?tx.get_hash(), "Duplicate transaction, not forwarding it.");
2286-
return Ok(ProcessTxResponse::ValidTx);
2287-
}
2288-
InsertTransactionResult::NoSpaceLeft => {
2289-
if is_forwarded {
2290-
trace!(target: "client", ?shard_uid, tx_hash = ?tx.get_hash(), "Transaction pool is full, dropping the transaction.");
2291-
} else {
2292-
trace!(target: "client", ?shard_uid, tx_hash = ?tx.get_hash(), "Transaction pool is full, trying to forward the transaction.");
2293-
}
2271+
return Ok(ProcessTxResponse::InvalidTx(err));
2272+
}
2273+
if check_only {
2274+
return Ok(ProcessTxResponse::ValidTx);
2275+
}
2276+
// Transactions only need to be recorded if the node is a validator.
2277+
if me.is_some() {
2278+
match self.chunk_producer.sharded_tx_pool.insert_transaction(shard_uid, tx.clone())
2279+
{
2280+
InsertTransactionResult::Success => {
2281+
trace!(target: "client", ?shard_uid, tx_hash = ?tx.get_hash(), "Recorded a transaction.");
2282+
}
2283+
InsertTransactionResult::Duplicate => {
2284+
trace!(target: "client", ?shard_uid, tx_hash = ?tx.get_hash(), "Duplicate transaction, not forwarding it.");
2285+
return Ok(ProcessTxResponse::ValidTx);
2286+
}
2287+
InsertTransactionResult::NoSpaceLeft => {
2288+
if is_forwarded {
2289+
trace!(target: "client", ?shard_uid, tx_hash = ?tx.get_hash(), "Transaction pool is full, dropping the transaction.");
2290+
} else {
2291+
trace!(target: "client", ?shard_uid, tx_hash = ?tx.get_hash(), "Transaction pool is full, trying to forward the transaction.");
22942292
}
22952293
}
22962294
}
2295+
}
22972296

2298-
// Active validator:
2299-
// possibly forward to next epoch validators
2300-
// Not active validator:
2301-
// forward to current epoch validators,
2302-
// possibly forward to next epoch validators
2303-
if self.active_validator(shard_id, signer)? {
2304-
trace!(target: "client", account = ?me, ?shard_id, tx_hash = ?tx.get_hash(), is_forwarded, "Recording a transaction.");
2305-
metrics::TRANSACTION_RECEIVED_VALIDATOR.inc();
2306-
2307-
if !is_forwarded {
2308-
self.possibly_forward_tx_to_next_epoch(tx, signer)?;
2309-
}
2310-
Ok(ProcessTxResponse::ValidTx)
2311-
} else if !is_forwarded {
2312-
trace!(target: "client", ?shard_id, tx_hash = ?tx.get_hash(), "Forwarding a transaction.");
2313-
metrics::TRANSACTION_RECEIVED_NON_VALIDATOR.inc();
2314-
self.forward_tx(&epoch_id, tx, signer)?;
2315-
Ok(ProcessTxResponse::RequestRouted)
2316-
} else {
2317-
trace!(target: "client", ?shard_id, tx_hash = ?tx.get_hash(), "Non-validator received a forwarded transaction, dropping it.");
2318-
metrics::TRANSACTION_RECEIVED_NON_VALIDATOR_FORWARDED.inc();
2319-
Ok(ProcessTxResponse::NoResponse)
2297+
// Active validator:
2298+
// possibly forward to next epoch validators
2299+
// Not active validator:
2300+
// forward to current epoch validators,
2301+
// possibly forward to next epoch validators
2302+
if self.active_validator(shard_id, signer)? {
2303+
trace!(target: "client", account = ?me, ?shard_id, tx_hash = ?tx.get_hash(), is_forwarded, "Recording a transaction.");
2304+
metrics::TRANSACTION_RECEIVED_VALIDATOR.inc();
2305+
2306+
if !is_forwarded {
2307+
self.possibly_forward_tx_to_next_epoch(tx, signer)?;
23202308
}
2309+
return Ok(ProcessTxResponse::ValidTx);
23212310
}
2322-
} else if check_only {
2323-
Ok(ProcessTxResponse::DoesNotTrackShard)
2324-
} else if is_forwarded {
2311+
if !is_forwarded {
2312+
trace!(target: "client", ?shard_id, tx_hash = ?tx.get_hash(), "Forwarding a transaction.");
2313+
metrics::TRANSACTION_RECEIVED_NON_VALIDATOR.inc();
2314+
self.forward_tx(&epoch_id, tx, signer)?;
2315+
return Ok(ProcessTxResponse::RequestRouted);
2316+
}
2317+
trace!(target: "client", ?shard_id, tx_hash = ?tx.get_hash(), "Non-validator received a forwarded transaction, dropping it.");
2318+
metrics::TRANSACTION_RECEIVED_NON_VALIDATOR_FORWARDED.inc();
2319+
return Ok(ProcessTxResponse::NoResponse);
2320+
}
2321+
2322+
if check_only {
2323+
return Ok(ProcessTxResponse::DoesNotTrackShard);
2324+
}
2325+
if is_forwarded {
23252326
// Received forwarded transaction but we are not tracking the shard
23262327
debug!(target: "client", ?me, ?shard_id, tx_hash = ?tx.get_hash(), "Received forwarded transaction but no tracking shard");
2327-
Ok(ProcessTxResponse::NoResponse)
2328-
} else {
2329-
// We are not tracking this shard, so there is no way to validate this tx. Just rerouting.
2330-
self.forward_tx(&epoch_id, tx, signer)?;
2331-
Ok(ProcessTxResponse::RequestRouted)
2328+
return Ok(ProcessTxResponse::NoResponse);
23322329
}
2330+
// We are not tracking this shard, so there is no way to validate this tx. Just rerouting.
2331+
self.forward_tx(&epoch_id, tx, signer).map(|()| ProcessTxResponse::RequestRouted)
23332332
}
23342333

23352334
/// Determine if I am a validator in next few blocks for specified shard, assuming epoch doesn't change.

0 commit comments

Comments
 (0)