Skip to content

Commit 3ab964c

Browse files
fix: Expired transaction event appears after commit (#5396)
* fix: remove extra expired reporting Signed-off-by: Lohachov Mykhailo <lohachov@soramitsu.co.jp> * test: add test for expired tx Signed-off-by: Lohachov Mykhailo <lohachov@soramitsu.co.jp> * chore(deps): bump coverallsapp/github-action from 2.3.4 to 2.3.6 (#5292) Bumps [coverallsapp/github-action](https://github.com/coverallsapp/github-action) from 2.3.4 to 2.3.6. - [Release notes](https://github.com/coverallsapp/github-action/releases) - [Commits](coverallsapp/github-action@v2.3.4...v2.3.6) --- updated-dependencies: - dependency-name: coverallsapp/github-action dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * fix: remove deleted workflow Signed-off-by: Lohachov Mykhailo <lohachov@soramitsu.co.jp> * fix: block interface changes Signed-off-by: Lohachov Mykhailo <lohachov@soramitsu.co.jp> --------- Signed-off-by: Lohachov Mykhailo <lohachov@soramitsu.co.jp> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
1 parent 3f039a3 commit 3ab964c

File tree

1 file changed

+49
-15
lines changed

1 file changed

+49
-15
lines changed

crates/iroha_core/src/queue.rs

Lines changed: 49 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ pub struct Failure {
7979
}
8080

8181
/// Will remove transaction from the queue on drop.
82-
/// See [`Queue::remove_stale_transaction`] for details.
82+
/// See [`Queue::remove_transaction`] for details.
8383
pub struct TransactionGuard {
8484
tx: AcceptedTransaction,
8585
queue: Arc<Queue>,
@@ -95,7 +95,7 @@ impl Deref for TransactionGuard {
9595

9696
impl Drop for TransactionGuard {
9797
fn drop(&mut self) {
98-
self.queue.remove_stale_transaction(&self.tx);
98+
self.queue.remove_transaction(&self.tx);
9999
}
100100
}
101101

@@ -172,10 +172,10 @@ impl Queue {
172172
}
173173

174174
fn check_tx(&self, tx: &AcceptedTransaction, state_view: &StateView) -> Result<(), Error> {
175-
if self.is_expired(tx) {
176-
Err(Error::Expired)
177-
} else if tx.is_in_blockchain(state_view) {
175+
if tx.is_in_blockchain(state_view) {
178176
Err(Error::InBlockchain)
177+
} else if self.is_expired(tx) {
178+
Err(Error::Expired)
179179
} else {
180180
Ok(())
181181
}
@@ -351,19 +351,10 @@ impl Queue {
351351
/// 3. When transaction is removed from [`Sumeragi::transaction_cache`]
352352
/// (either because it was expired, or because transaction is commited to blockchain),
353353
/// we should remove transaction from [`Queue::accepted_tx`].
354-
fn remove_stale_transaction(&self, tx: &AcceptedTransaction) {
354+
fn remove_transaction(&self, tx: &AcceptedTransaction) {
355355
let removed = self.txs.remove(&tx.as_ref().hash());
356356
if removed.is_some() {
357357
self.decrease_per_user_tx_count(tx.as_ref().authority());
358-
359-
if self.is_expired(tx) {
360-
let event = TransactionEvent {
361-
hash: tx.as_ref().hash(),
362-
block_height: None,
363-
status: TransactionStatus::Expired,
364-
};
365-
let _ = self.events_sender.send(event.into());
366-
}
367358
}
368359
}
369360

@@ -587,6 +578,49 @@ pub mod tests {
587578
assert_eq!(queue.txs.len(), 0);
588579
}
589580

581+
#[test]
582+
async fn push_expired_tx_already_in_blockchain() {
583+
let chain_id = ChainId::from("00000000-0000-0000-0000-000000000000");
584+
585+
let (alice_id, alice_keypair) = gen_account_in("wonderland");
586+
587+
let kura = Kura::blank_kura_for_testing();
588+
let query_handle = LiveQueryStore::start_test();
589+
let state = State::new(world_with_test_domains(), kura, query_handle);
590+
let (max_clock_drift, tx_limits) = {
591+
let state_view = state.world.view();
592+
let params = state_view.parameters();
593+
(params.sumeragi().max_clock_drift(), params.transaction)
594+
};
595+
let (time_handle, time_source) = TimeSource::new_mock(Duration::default());
596+
597+
let mut tx =
598+
TransactionBuilder::new_with_time_source(chain_id.clone(), alice_id, &time_source);
599+
tx.set_ttl(Duration::from_millis(100));
600+
let tx = tx.sign(alice_keypair.private_key());
601+
let tx = AcceptedTransaction::accept(tx, &chain_id, max_clock_drift, tx_limits)
602+
.expect("Failed to accept Transaction.");
603+
604+
let block_header = ValidBlock::new_dummy(&KeyPair::random().into_parts().1)
605+
.as_ref()
606+
.header();
607+
let mut state_block = state.block(block_header);
608+
state_block
609+
.transactions
610+
.insert_block_with_single_tx(tx.as_ref().hash(), nonzero!(1_usize));
611+
state_block.commit();
612+
let queue = Queue::test(config_factory(), &time_source);
613+
time_handle.advance(Duration::from_secs(100));
614+
assert!(matches!(
615+
queue.push(tx, state.view()),
616+
Err(Failure {
617+
err: Error::InBlockchain,
618+
..
619+
})
620+
));
621+
assert_eq!(queue.txs.len(), 0);
622+
}
623+
590624
#[test]
591625
async fn get_tx_drop_if_in_blockchain() {
592626
let max_txs_in_block = nonzero!(2_usize);

0 commit comments

Comments
 (0)