Skip to content

Commit f5ba331

Browse files
authored
fix(ethexe): remove dispatches for panic injected messages with uncharged gas (#5256)
1 parent cf5ce50 commit f5ba331

File tree

2 files changed

+41
-28
lines changed

2 files changed

+41
-28
lines changed

ethexe/processor/src/tests.rs

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1297,6 +1297,7 @@ async fn executable_balance_injected_panic_not_charged() {
12971297
.await
12981298
.unwrap();
12991299

1300+
// Promises still returns for panic injected txs.
13001301
let panic_promise = promise_receiver
13011302
.recv()
13021303
.await
@@ -1309,14 +1310,12 @@ async fn executable_balance_injected_panic_not_charged() {
13091310
SimpleExecutionError::UserspacePanic
13101311
))
13111312
);
1313+
assert_eq!(panic_promise.reply.payload, b"\xE0\x80\x80");
13121314

13131315
let to_users = handler.transitions.current_messages();
1314-
assert_eq!(to_users.len(), 2);
13151316

1316-
let message = &to_users[1].1;
1317-
assert_eq!(message.destination, user_id);
1318-
// Check that panic indeed happened
1319-
assert_eq!(&message.payload[..3], b"\xE0\x80\x80");
1317+
// Only one message to user, because injected message panic and do not charge gas.
1318+
assert_eq!(to_users.len(), 1);
13201319

13211320
// Check that executable balance is unchanged
13221321
let exec_balance_after = handler.program_state(actor_id).executable_balance;
@@ -1346,12 +1345,7 @@ async fn executable_balance_injected_panic_not_charged() {
13461345
.unwrap();
13471346

13481347
let to_users = transitions.current_messages();
1349-
assert_eq!(to_users.len(), 3);
1350-
1351-
let message = &to_users[2].1;
1352-
assert_eq!(message.destination, user_id);
1353-
// Check that panic indeed happened
1354-
assert_eq!(&message.payload[..3], b"\xE0\x80\x80");
1348+
assert_eq!(to_users.len(), 2);
13551349

13561350
// Check that executable balance decreased on canonical message
13571351
let exec_balance_after = ProcessingHandler::new(processor.db.clone(), transitions)

ethexe/runtime/common/src/journal.rs

Lines changed: 36 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,10 @@ use crate::{
55
Program, ProgramState, Storage,
66
},
77
};
8-
use alloc::{collections::BTreeMap, vec::Vec};
8+
use alloc::{
9+
collections::{BTreeMap, BTreeSet},
10+
vec::Vec,
11+
};
912
use core::{mem, num::NonZero, panic};
1013
use core_processor::common::{DispatchOutcome, JournalHandler, JournalNote};
1114
use ethexe_common::{
@@ -507,6 +510,10 @@ where
507510
let notes_count = journal.len();
508511
let mut skipped_notes = 0;
509512

513+
// The set of panic injected messages for which we do not charge executable balance.
514+
// Dispatches for these messages will not be include into filtered journal notes.
515+
let mut messages_to_skip = BTreeSet::new();
516+
510517
let filtered: Vec<_> = journal
511518
.filter_map(|note| {
512519
match note {
@@ -530,15 +537,19 @@ where
530537
allocations_update.insert(program_id, allocations);
531538
}
532539
JournalNote::GasBurned {
533-
message_id: _,
540+
message_id,
534541
amount,
535542
is_panic,
536543
} => {
537544
self.gas_allowance_counter.charge(amount);
538545

539546
// Special case for panicked `Injected` messages with gas spent less than the threshold.
540-
if !is_panic || self.should_charge_exec_balance_on_panic(amount) {
541-
self.charge_exec_balance(amount);
547+
match (is_panic, self.should_charge_exec_balance_on_panic(amount)) {
548+
(true, false) => {
549+
// Message panic and we do not charge exec balance - do not include to journal.
550+
messages_to_skip.insert(message_id);
551+
}
552+
_ => self.charge_exec_balance(amount),
542553
}
543554
}
544555
note @ JournalNote::StopProcessing {
@@ -554,20 +565,28 @@ where
554565
// * SendDispatch to self
555566
// * SendValue to self
556567
note => {
557-
if let JournalNote::SendDispatch { dispatch, .. } = &note {
558-
// TODO: #5227 delay must be taken into account
559-
self.limiter.outgoing_messages =
560-
self.limiter.outgoing_messages.saturating_sub(1);
561-
self.limiter.outgoing_messages_bytes =
562-
self.limiter.outgoing_messages_bytes.saturating_sub(
563-
u32::try_from(dispatch.payload_bytes().len())
564-
.expect("payload size is too big for u32"),
565-
);
566-
567-
if dispatch.is_reply() && self.call_reply {
568-
self.limiter.call_replies =
569-
self.limiter.call_replies.saturating_sub(1);
568+
match &note {
569+
JournalNote::SendDispatch { message_id, .. }
570+
if messages_to_skip.contains(message_id) =>
571+
{
572+
return None;
573+
}
574+
JournalNote::SendDispatch { dispatch, .. } => {
575+
// TODO: #5227 delay must be taken into account
576+
self.limiter.outgoing_messages =
577+
self.limiter.outgoing_messages.saturating_sub(1);
578+
self.limiter.outgoing_messages_bytes =
579+
self.limiter.outgoing_messages_bytes.saturating_sub(
580+
u32::try_from(dispatch.payload_bytes().len())
581+
.expect("payload size is too big for u32"),
582+
);
583+
584+
if dispatch.is_reply() && self.call_reply {
585+
self.limiter.call_replies =
586+
self.limiter.call_replies.saturating_sub(1);
587+
}
570588
}
589+
_ => {}
571590
}
572591

573592
skipped_notes += 1;

0 commit comments

Comments
 (0)