Skip to content

Commit 1073a8f

Browse files
committed
[TransactionOrchestrator] enhance retries on transient failures (#24649)
- Process transactions in a separate task in TO, to avoid cancelled by user dropping the request. - Retry transient errors in the background. - Avoid casting reject votes because of consensus overload, against transactions already in consensus. This should reduce the chance of rejecting valid transactions. CI
1 parent 74d4f61 commit 1073a8f

File tree

6 files changed

+235
-77
lines changed

6 files changed

+235
-77
lines changed

crates/sui-core/src/authority.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1281,6 +1281,7 @@ impl AuthorityState {
12811281
&self,
12821282
epoch_store: &Arc<AuthorityPerEpochStore>,
12831283
transaction: &VerifiedTransaction,
1284+
enforce_live_input_objects: bool,
12841285
) -> SuiResult<()> {
12851286
if !epoch_store
12861287
.get_reconfig_state_read_lock_guard()
@@ -1313,6 +1314,17 @@ impl AuthorityState {
13131314
.into());
13141315
}
13151316

1317+
if enforce_live_input_objects && live_object.version() < obj_ref.1 {
1318+
// Returns a non-retriable error when the input object version is required to be live.
1319+
return Err(SuiErrorKind::UserInputError {
1320+
error: UserInputError::ObjectVersionUnavailableForConsumption {
1321+
provided_obj_ref: *obj_ref,
1322+
current_version: live_object.version(),
1323+
},
1324+
}
1325+
.into());
1326+
}
1327+
13161328
// If version matches, verify digest also matches
13171329
if obj_ref.1 == live_object.version() && obj_ref.2 != live_object.digest() {
13181330
return Err(SuiErrorKind::UserInputError {

crates/sui-core/src/consensus_validator.rs

Lines changed: 9 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,11 @@ use tracing::{debug, info, instrument, warn};
2222
use crate::{
2323
authority::{AuthorityState, authority_per_epoch_store::AuthorityPerEpochStore},
2424
checkpoints::CheckpointServiceNotify,
25-
consensus_adapter::ConsensusOverloadChecker,
25+
consensus_adapter::{ConsensusOverloadChecker, NoopConsensusOverloadChecker},
2626
};
2727

28-
/// Allows verifying the validity of transactions
28+
/// Validates transactions from consensus and votes on whether to execute the transactions
29+
/// based on their validity and the current state of the authority.
2930
#[derive(Clone)]
3031
pub struct SuiTxValidator {
3132
authority_state: Arc<AuthorityState>,
@@ -37,7 +38,6 @@ pub struct SuiTxValidator {
3738
impl SuiTxValidator {
3839
pub fn new(
3940
authority_state: Arc<AuthorityState>,
40-
consensus_overload_checker: Arc<dyn ConsensusOverloadChecker>,
4141
checkpoint_service: Arc<dyn CheckpointServiceNotify + Send + Sync>,
4242
metrics: Arc<SuiTxValidatorMetrics>,
4343
) -> Self {
@@ -46,6 +46,8 @@ impl SuiTxValidator {
4646
"SuiTxValidator constructed for epoch {}",
4747
epoch_store.epoch()
4848
);
49+
// Intentionally do not check consensus overload, because this is validating transactions already in consensus.
50+
let consensus_overload_checker = Arc::new(NoopConsensusOverloadChecker {});
4951
Self {
5052
authority_state,
5153
consensus_overload_checker,
@@ -330,9 +332,8 @@ mod tests {
330332
use crate::{
331333
authority::test_authority_builder::TestAuthorityBuilder,
332334
checkpoints::CheckpointServiceNoop,
333-
consensus_adapter::{
334-
NoopConsensusOverloadChecker,
335-
consensus_tests::{test_certificates, test_gas_objects, test_user_transaction},
335+
consensus_adapter::consensus_tests::{
336+
test_certificates, test_gas_objects, test_user_transaction,
336337
},
337338
consensus_validator::{SuiTxValidator, SuiTxValidatorMetrics},
338339
};
@@ -364,12 +365,8 @@ mod tests {
364365
.unwrap();
365366

366367
let metrics = SuiTxValidatorMetrics::new(&Default::default());
367-
let validator = SuiTxValidator::new(
368-
state.clone(),
369-
Arc::new(NoopConsensusOverloadChecker {}),
370-
Arc::new(CheckpointServiceNoop {}),
371-
metrics,
372-
);
368+
let validator =
369+
SuiTxValidator::new(state.clone(), Arc::new(CheckpointServiceNoop {}), metrics);
373370
let res = validator.verify_batch(&[&first_transaction_bytes]);
374371
assert!(res.is_ok(), "{res:?}");
375372

@@ -481,7 +478,6 @@ mod tests {
481478

482479
let validator = SuiTxValidator::new(
483480
state.clone(),
484-
Arc::new(NoopConsensusOverloadChecker {}),
485481
Arc::new(CheckpointServiceNoop {}),
486482
SuiTxValidatorMetrics::new(&Default::default()),
487483
);
@@ -565,7 +561,6 @@ mod tests {
565561

566562
let validator = SuiTxValidator::new(
567563
state.clone(),
568-
Arc::new(NoopConsensusOverloadChecker {}),
569564
Arc::new(CheckpointServiceNoop {}),
570565
SuiTxValidatorMetrics::new(&Default::default()),
571566
);
@@ -619,7 +614,6 @@ mod tests {
619614

620615
let validator = SuiTxValidator::new(
621616
state.clone(),
622-
Arc::new(NoopConsensusOverloadChecker {}),
623617
Arc::new(CheckpointServiceNoop {}),
624618
SuiTxValidatorMetrics::new(&Default::default()),
625619
);
@@ -680,7 +674,6 @@ mod tests {
680674
.unwrap();
681675
let validator = SuiTxValidator::new(
682676
state.clone(),
683-
Arc::new(NoopConsensusOverloadChecker {}),
684677
Arc::new(CheckpointServiceNoop {}),
685678
SuiTxValidatorMetrics::new(&Default::default()),
686679
);

0 commit comments

Comments
 (0)