Skip to content

Commit

Permalink
Ignore the message receipt if transaction is reverted (#2045)
Browse files Browse the repository at this point in the history
Include withdrawal message only if transaction is executed successfully,
otherwise it is possible to use same assets to create many withdrawals
with failed transactions.

The network is patched already with a private fix.

## Checklist
- [x] Breaking changes are clearly marked as such in the PR description
and changelog
- [x] New behavior is reflected in tests

### Before requesting review
- [x] I have reviewed the code myself

---------

Co-authored-by: Hannes Karppila <hannes.karppila@gmail.com>
Co-authored-by: Mitchell Turner <james.mitchell.turner@gmail.com>
  • Loading branch information
3 people authored Jul 27, 2024
1 parent b7e89e9 commit 1cfbb05
Show file tree
Hide file tree
Showing 3 changed files with 119 additions and 4 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
- "gas-price-threshold-percent" - the threshold percent for determining if the gas price will be increase or decreased
And the following CLI flags are serving a new purpose
- "min-gas-price" - the minimum gas price that the gas price algorithm will return

### Fixed

#### Breaking
- [2045](https://github.com/FuelLabs/fuel-core/pull/2045): Include withdrawal message only if transaction is executed successfully.
- [2041](https://github.com/FuelLabs/fuel-core/pull/2041): Add code for startup of the gas price algorithm updater so
the gas price db on startup is always in sync with the on chain db

Expand Down
108 changes: 107 additions & 1 deletion crates/fuel-core/src/executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,10 @@ mod tests {
RegId,
},
fuel_crypto::SecretKey,
fuel_merkle::sparse,
fuel_merkle::{
common::empty_sum_sha256,
sparse,
},
fuel_tx::{
consensus_parameters::gas::GasCostsValuesV1,
field::{
Expand Down Expand Up @@ -2528,6 +2531,109 @@ mod tests {
));
}

#[test]
fn withdrawal_message_included_in_header_for_successfully_executed_transaction() {
// Given
let amount_from_random_input = 1000;
let smo_tx = TransactionBuilder::script(
vec![
// The amount to send in coins.
op::movi(0x13, amount_from_random_input),
// Send the message output.
op::smo(0x0, 0x0, 0x0, 0x13),
op::ret(RegId::ONE),
]
.into_iter()
.collect(),
vec![],
)
.add_random_fee_input()
.script_gas_limit(1000000)
.finalize_as_transaction();

let block = PartialFuelBlock {
header: Default::default(),
transactions: vec![smo_tx],
};

// When
let ExecutionResult { block, .. } =
create_executor(Default::default(), Default::default())
.produce_and_commit(block)
.expect("block execution failed unexpectedly");
let result = create_executor(Default::default(), Default::default())
.validate_and_commit(&block)
.expect("block validation failed unexpectedly");

// Then
let Some(Receipt::MessageOut {
sender,
recipient,
amount,
nonce,
data,
..
}) = result.tx_status[0].result.receipts().first().cloned()
else {
panic!("Expected a MessageOut receipt");
};

// Reconstruct merkle message outbox merkle root and see that it matches
let mut mt = fuel_core_types::fuel_merkle::binary::in_memory::MerkleTree::new();
mt.push(
&Message::V1(MessageV1 {
sender,
recipient,
nonce,
amount,
data: data.unwrap_or_default(),
da_height: 1u64.into(),
})
.message_id()
.to_bytes(),
);
assert_eq!(block.header().message_outbox_root.as_ref(), mt.root());
}

#[test]
fn withdrawal_message_not_included_in_header_for_failed_transaction() {
// Given
let amount_from_random_input = 1000;
let smo_tx = TransactionBuilder::script(
vec![
// The amount to send in coins.
op::movi(0x13, amount_from_random_input),
// Send the message output.
op::smo(0x0, 0x0, 0x0, 0x13),
op::rvrt(0x0),
]
.into_iter()
.collect(),
vec![],
)
.add_random_fee_input()
.script_gas_limit(1000000)
.finalize_as_transaction();

let block = PartialFuelBlock {
header: Default::default(),
transactions: vec![smo_tx],
};

// When
let ExecutionResult { block, .. } =
create_executor(Default::default(), Default::default())
.produce_and_commit(block)
.expect("block execution failed unexpectedly");
create_executor(Default::default(), Default::default())
.validate_and_commit(&block)
.expect("block validation failed unexpectedly");

// Then
let empty_root = empty_sum_sha256();
assert_eq!(block.header().message_outbox_root.as_ref(), empty_root)
}

#[test]
fn get_block_height_returns_current_executing_block() {
let mut rng = StdRng::seed_from_u64(1234);
Expand Down
10 changes: 7 additions & 3 deletions crates/services/executor/src/executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1365,9 +1365,13 @@ where
.used_gas
.checked_add(used_gas)
.ok_or(ExecutorError::GasOverflow)?;
execution_data
.message_ids
.extend(receipts.iter().filter_map(|r| r.message_id()));

if !reverted {
execution_data
.message_ids
.extend(receipts.iter().filter_map(|r| r.message_id()));
}

let status = if reverted {
TransactionExecutionResult::Failed {
result: Some(state),
Expand Down

0 comments on commit 1cfbb05

Please sign in to comment.