Skip to content

Commit

Permalink
Fix for the race condition with tx status and receipts (FuelLabs#1658)
Browse files Browse the repository at this point in the history
Example of the failed CI
https://github.com/FuelLabs/fuel-core/actions/runs/7865306004/job/21471566710?pr=1656

The change moves receipts into the `TransationStatus` to avoid requests
to the database. The `TxPool` directly listens for blocks and
transaction statuses to notify the user via subscription. But when it
notifies the users, the off-chain database can still be outdated,
leading to empty receipts.

This PR fixes the race condition for the receipts. However, the problem
still exists for the end user if they want to fetch some updated
information from the off-chain database. I created a separate issue to
track it: FuelLabs#1659.

It is the fix for the FuelLabs#1656
  • Loading branch information
xgreenx authored Feb 12, 2024
1 parent 5cbe7e2 commit b7e1c6e
Show file tree
Hide file tree
Showing 32 changed files with 256 additions and 318 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 16,7 @@ concurrency:

env:
CARGO_TERM_COLOR: always
RUST_VERSION: 1.73.0
RUST_VERSION: 1.74.0
NIGHTLY_RUST_VERSION: nightly-2023-10-29
RUSTFLAGS: -D warnings
REGISTRY: ghcr.io
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/dependencies.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 16,7 @@ defaults:
env:
# So cargo doesn't complain about unstable features
RUSTC_BOOTSTRAP: 1
RUST_VERSION: 1.73.0
RUST_VERSION: 1.74.0
PR_TITLE: Weekly `cargo update`
PR_MESSAGE: |
Automation to keep dependencies in `Cargo.lock` current.
Expand Down
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 10,7 @@ Description of the upcoming release here.

### Changed

- [#1658](https://github.com/FuelLabs/fuel-core/pull/1658): Removed `Receipts` table. Instead, receipts are part of the `TransactionStatuses` table.
- [#1650](https://github.com/FuelLabs/fuel-core/pull/1650): Add api endpoint for getting estimates for future gas prices
- [#1649](https://github.com/FuelLabs/fuel-core/pull/1649): Add api endpoint for getting latest gas price
- [#1600](https://github.com/FuelLabs/fuel-core/pull/1640): Upgrade to fuel-vm 0.45.0
Expand All @@ -29,6 30,9 @@ Description of the upcoming release here.
- [#1636](https://github.com/FuelLabs/fuel-core/pull/1636): Add more docs to GraphQL DAP API.

#### Breaking
- [#1658](https://github.com/FuelLabs/fuel-core/pull/1658): Receipts are part of the transaction status.
Removed `reason` from the `TransactionExecutionResult::Failed`. It can be calculated based on the program state and receipts.
Also, it is not possible to fetch `receipts` from the `Transaction` directly anymore. Instead, you need to fetch `status` and its receipts.
- [#1646](https://github.com/FuelLabs/fuel-core/pull/1646): Remove redundant receipts from queries.
- [#1639](https://github.com/FuelLabs/fuel-core/pull/1639): Make Merkle metadata, i.e. `SparseMerkleMetadata` and `DenseMerkleMetadata` type version-able enums
- [#1632](https://github.com/FuelLabs/fuel-core/pull/1632): Make `Message` type a version-able enum
Expand Down
30 changes: 18 additions & 12 deletions benches/benches/block_target_gas.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 72,7 @@ use fuel_core_types::{
checked_transaction::EstimatePredicates,
consts::WORD_SIZE,
},
services::executor::TransactionExecutionResult,
};
use rand::SeedableRng;
use utils::{
Expand Down Expand Up @@ -352,11 353,9 @@ fn run_with_service_with_extra_inputs(
extra_outputs: Vec<Output>,
) {
group.bench_function(id, |b| {

b.to_async(rt).iter(|| {
let shared = service.shared.clone();


let mut tx_builder = fuel_core_types::fuel_tx::TransactionBuilder::script(
script.clone().into_iter().collect(),
script_data.clone(),
Expand All @@ -382,7 381,11 @@ fn run_with_service_with_extra_inputs(
TxPointer::default(),
*contract_id,
);
let contract_output = Output::contract(input_count as u8, Bytes32::zeroed(), Bytes32::zeroed());
let contract_output = Output::contract(
input_count as u8,
Bytes32::zeroed(),
Bytes32::zeroed(),
);

tx_builder
.add_input(contract_input)
Expand All @@ -397,9 400,13 @@ fn run_with_service_with_extra_inputs(
tx_builder.add_output(*output);
}
let mut tx = tx_builder.finalize_as_transaction();
tx.estimate_predicates(&shared.config.chain_conf.consensus_parameters.clone().into()).unwrap();
tx.estimate_predicates(
&shared.config.chain_conf.consensus_parameters.clone().into(),
)
.unwrap();
async move {
let tx_id = tx.id(&shared.config.chain_conf.consensus_parameters.chain_id);
let tx_id =
tx.id(&shared.config.chain_conf.consensus_parameters.chain_id);

let mut sub = shared.block_importer.block_importer.subscribe();
shared
Expand All @@ -415,13 422,12 @@ fn run_with_service_with_extra_inputs(
assert_eq!(res.sealed_block.entity.transactions().len(), 2);
assert_eq!(res.tx_status[0].id, tx_id);

let fuel_core_types::services::executor::TransactionExecutionResult::Failed {
reason,
..
} = &res.tx_status[0].result
else {
panic!("The execution should fails with out of gas")
};
let TransactionExecutionResult::Failed { result, receipts } =
&res.tx_status[0].result
else {
panic!("The execution should fails with out of gas")
};
let reason = TransactionExecutionResult::reason(receipts, result);
if !reason.contains("OutOfGas") {
panic!("The test failed because of {}", reason);
}
Expand Down
9 changes: 6 additions & 3 deletions bin/e2e-test-client/src/tests/script.rs
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 179,7 @@ async fn _dry_runs(

let tx_statuses = query?;
for (tx_status, tx) in tx_statuses.iter().zip(transactions.iter()) {
if tx_status.receipts.is_empty() {
if tx_status.result.receipts().is_empty() {
return Err(
format!("Receipts are empty for query_number {query_number}").into(),
)
Expand All @@ -189,10 189,13 @@ async fn _dry_runs(
if expect == DryRunResult::Successful {
assert!(matches!(
&tx_status.result,
TransactionExecutionResult::Success { result: _result }
TransactionExecutionResult::Success {
result: _result,
..
}
));
assert!(matches!(
tx_status.receipts.last(),
tx_status.result.receipts().last(),
Some(Receipt::ScriptResult {
result: ScriptExecutionResult::Success,
..
Expand Down
2 changes: 1 addition & 1 deletion ci_checks.sh
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 3,7 @@
# The script runs almost all CI checks locally.
#
# Requires installed:
# - Rust `1.73.0`
# - Rust `1.74.0`
# - Nightly rust formatter
# - `cargo install cargo-sort`

Expand Down
3 changes: 2 additions & 1 deletion crates/client/assets/schema.sdl
Original file line number Diff line number Diff line change
Expand Up @@ -258,10 258,12 @@ union DependentCost = LightOperation | HeavyOperation
type DryRunFailureStatus {
programState: ProgramState
reason: String!
receipts: [Receipt!]!
}

type DryRunSuccessStatus {
programState: ProgramState
receipts: [Receipt!]!
}

type DryRunTransactionExecutionStatus {
Expand Down Expand Up @@ -973,7 975,6 @@ type Transaction {
witnesses: [HexString!]
receiptsRoot: Bytes32
status: TransactionStatus
receipts: [Receipt!]
script: HexString
scriptData: HexString
bytecodeWitnessIndex: Int
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,49 12,82 @@ mutation($txs: [HexString!]!, $utxoValidation: Boolean) {
returnType
data
}
receipts {
param1
param2
amount
assetId
gas
digest
contract {
id
}
is
pc
ptr
ra
rb
rc
rd
reason
receiptType
to {
id
}
toAddress
val
len
result
gasUsed
data
sender
recipient
nonce
contractId
subId
}
}
... on DryRunFailureStatus {
reason
programState {
returnType
data
}
receipts {
param1
param2
amount
assetId
gas
digest
contract {
id
}
is
pc
ptr
ra
rb
rc
rd
reason
receiptType
to {
id
}
toAddress
val
len
result
gasUsed
data
sender
recipient
nonce
contractId
subId
}
}
}
receipts {
param1
param2
amount
assetId
gas
digest
contract {
id
}
is
pc
ptr
ra
rb
rc
rd
reason
receiptType
to {
id
}
toAddress
val
len
result
gasUsed
data
sender
recipient
nonce
contractId
subId
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -196,40 196,6 @@ query($id: TransactionId!) {
}
}
witnesses
receipts {
param1
param2
amount
assetId
gas
digest
contract {
id
}
is
pc
ptr
ra
rb
rc
rd
reason
receiptType
to {
id
}
toAddress
val
len
result
gasUsed
data
sender
recipient
nonce
contractId
subId
}
script
scriptData
policies {
Expand Down
30 changes: 16 additions & 14 deletions crates/client/src/client/schema/tx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -217,14 217,25 @@ impl TryFrom<DryRunTransactionStatus> for TransactionExecutionResult {
fn try_from(status: DryRunTransactionStatus) -> Result<Self, Self::Error> {
Ok(match status {
DryRunTransactionStatus::SuccessStatus(s) => {
let receipts = s
.receipts
.into_iter()
.map(|receipt| receipt.try_into())
.collect::<Result<Vec<fuel_tx::Receipt>, _>>()?;
TransactionExecutionResult::Success {
result: s.program_state.map(TryInto::try_into).transpose()?,
receipts,
}
}
DryRunTransactionStatus::FailureStatus(s) => {
let receipts = s
.receipts
.into_iter()
.map(|receipt| receipt.try_into())
.collect::<Result<Vec<fuel_tx::Receipt>, _>>()?;
TransactionExecutionResult::Failed {
result: s.program_state.map(TryInto::try_into).transpose()?,
reason: s.reason,
receipts,
}
}
DryRunTransactionStatus::Unknown => {
Expand All @@ -238,21 249,21 @@ impl TryFrom<DryRunTransactionStatus> for TransactionExecutionResult {
#[cynic(schema_path = "./assets/schema.sdl")]
pub struct DryRunSuccessStatus {
pub program_state: Option<ProgramState>,
pub receipts: Vec<Receipt>,
}

#[derive(cynic::QueryFragment, Debug)]
#[cynic(schema_path = "./assets/schema.sdl")]
pub struct DryRunFailureStatus {
pub reason: String,
pub program_state: Option<ProgramState>,
pub receipts: Vec<Receipt>,
}

#[derive(cynic::QueryFragment, Debug)]
#[cynic(schema_path = "./assets/schema.sdl")]
pub struct DryRunTransactionExecutionStatus {
pub id: TransactionId,
pub status: DryRunTransactionStatus,
pub receipts: Vec<Receipt>,
}

impl TryFrom<DryRunTransactionExecutionStatus> for TransactionExecutionStatus {
Expand All @@ -261,17 272,8 @@ impl TryFrom<DryRunTransactionExecutionStatus> for TransactionExecutionStatus {
fn try_from(schema: DryRunTransactionExecutionStatus) -> Result<Self, Self::Error> {
let id = schema.id.into();
let status = schema.status.try_into()?;
let receipts = schema
.receipts
.into_iter()
.map(|receipt| receipt.try_into())
.collect::<Result<Vec<fuel_tx::Receipt>, _>>()?;

Ok(TransactionExecutionStatus {
id,
result: status,
receipts,
})

Ok(TransactionExecutionStatus { id, result: status })
}
}

Expand Down
Loading

0 comments on commit b7e1c6e

Please sign in to comment.