Merge pull request #1791 from eqlabs/chris/mandatory-execution-resources

refactor: common execution resources are mandatory

crates/common/src/receipt.rs

@@ -4,7 +4,7 @@ use crate::prelude::*;
 pub struct Receipt {
     pub actual_fee: Option<Fee>,
     pub events: Vec<crate::event::Event>,
-    pub execution_resources: Option<ExecutionResources>,
+    pub execution_resources: ExecutionResources,
     pub l2_to_l1_messages: Vec<L2ToL1Message>,
     pub execution_status: ExecutionStatus,
     pub transaction_hash: TransactionHash,
@@ -36,7 +36,7 @@ pub struct ExecutionResources {
     pub builtin_instance_counter: BuiltinCounters,
     pub n_steps: u64,
     pub n_memory_holes: u64,
-    pub data_availability: Option<ExecutionDataAvailability>,
+    pub data_availability: ExecutionDataAvailability,
 }
 
 #[derive(Clone, Debug, Default, PartialEq, Eq)]

crates/gateway-types/src/reply.rs

@@ -347,7 +347,7 @@ pub(crate) mod transaction {
                 builtin_instance_counter: value.builtin_instance_counter.into(),
                 n_steps: value.n_steps,
                 n_memory_holes: value.n_memory_holes,
-                data_availability: value.data_availability.map(Into::into),
+                data_availability: value.data_availability.unwrap_or_default().into(),
             }
         }
     }
@@ -358,7 +358,7 @@ pub(crate) mod transaction {
                 builtin_instance_counter: value.builtin_instance_counter.into(),
                 n_steps: value.n_steps,
                 n_memory_holes: value.n_memory_holes,
-                data_availability: value.data_availability.map(Into::into),
+                data_availability: Some(value.data_availability.into()),
             }
         }
     }
@@ -640,7 +640,7 @@ pub(crate) mod transaction {
             Self {
                 actual_fee,
                 events,
-                execution_resources: execution_resources.map(Into::into),
+                execution_resources: Some(execution_resources.into()),
                 l1_to_l2_consumed_message: None,
                 l2_to_l1_messages: l2_to_l1_messages.into_iter().map(Into::into).collect(),
                 transaction_hash,
@@ -671,7 +671,7 @@ pub(crate) mod transaction {
             common::Receipt {
                 actual_fee,
                 events,
-                execution_resources: execution_resources.map(Into::into),
+                execution_resources: execution_resources.unwrap_or_default().into(),
                 l2_to_l1_messages: l2_to_l1_messages.into_iter().map(Into::into).collect(),
                 transaction_hash,
                 transaction_index,

crates/p2p/src/client/types.rs

@@ -19,7 +19,6 @@ use pathfinder_common::{
     SequencerAddress, StarknetVersion, StateCommitment, Tip, TransactionCommitment,
     TransactionHash, TransactionNonce, TransactionSignatureElem, TransactionVersion,
 };
-use pathfinder_crypto::Felt;
 
 /// We don't want to introduce circular dependencies between crates
 /// and we need to work around for the orphan rule - implement conversion fns for types ourside our crate.
@@ -127,7 +126,7 @@ impl
 #[derive(Clone, Default, Debug, PartialEq)]
 pub struct Receipt {
     pub actual_fee: Option<Fee>,
-    pub execution_resources: Option<ExecutionResources>,
+    pub execution_resources: ExecutionResources,
     pub l2_to_l1_messages: Vec<L2ToL1Message>,
     pub execution_status: ExecutionStatus,
     pub transaction_hash: TransactionHash,
@@ -162,58 +161,49 @@ impl TryFrom<p2p_proto::receipt::Receipt> for Receipt {
             | Declare(DeclareTransactionReceipt { common })
             | L1Handler(L1HandlerTransactionReceipt { common, .. })
             | Deploy(DeployTransactionReceipt { common, .. })
-            | DeployAccount(DeployAccountTransactionReceipt { common, .. }) => {
-                let data_availability = (common.execution_resources.l1_gas != Felt::ZERO
-                    || common.execution_resources.l1_data_gas != Felt::ZERO)
-                    .then_some(ExecutionDataAvailability {
+            | DeployAccount(DeployAccountTransactionReceipt { common, .. }) => Ok(Self {
+                transaction_hash: TransactionHash(common.transaction_hash.0),
+                actual_fee: Some(Fee(common.actual_fee)),
+                execution_resources: ExecutionResources {
+                    builtin_instance_counter: BuiltinCounters {
+                        output_builtin: common.execution_resources.builtins.output.into(),
+                        pedersen_builtin: common.execution_resources.builtins.pedersen.into(),
+                        range_check_builtin: common.execution_resources.builtins.range_check.into(),
+                        ecdsa_builtin: common.execution_resources.builtins.ecdsa.into(),
+                        bitwise_builtin: common.execution_resources.builtins.bitwise.into(),
+                        ec_op_builtin: common.execution_resources.builtins.ec_op.into(),
+                        keccak_builtin: common.execution_resources.builtins.keccak.into(),
+                        poseidon_builtin: common.execution_resources.builtins.poseidon.into(),
+                        segment_arena_builtin: 0,
+                    },
+                    n_steps: common.execution_resources.steps.into(),
+                    n_memory_holes: common.execution_resources.memory_holes.into(),
+                    data_availability: ExecutionDataAvailability {
                         l1_gas: GasPrice::from(common.execution_resources.l1_gas).0,
                         l1_data_gas: GasPrice::from(common.execution_resources.l1_data_gas).0,
-                    });
-                Ok(Self {
-                    transaction_hash: TransactionHash(common.transaction_hash.0),
-                    actual_fee: Some(Fee(common.actual_fee)),
-                    execution_resources: Some(ExecutionResources {
-                        builtin_instance_counter: BuiltinCounters {
-                            output_builtin: common.execution_resources.builtins.output.into(),
-                            pedersen_builtin: common.execution_resources.builtins.pedersen.into(),
-                            range_check_builtin: common
-                                .execution_resources
-                                .builtins
-                                .range_check
-                                .into(),
-                            ecdsa_builtin: common.execution_resources.builtins.ecdsa.into(),
-                            bitwise_builtin: common.execution_resources.builtins.bitwise.into(),
-                            ec_op_builtin: common.execution_resources.builtins.ec_op.into(),
-                            keccak_builtin: common.execution_resources.builtins.keccak.into(),
-                            poseidon_builtin: common.execution_resources.builtins.poseidon.into(),
-                            segment_arena_builtin: 0,
-                        },
-                        n_steps: common.execution_resources.steps.into(),
-                        n_memory_holes: common.execution_resources.memory_holes.into(),
-                        data_availability,
-                    }),
-                    l2_to_l1_messages: common
-                        .messages_sent
-                        .into_iter()
-                        .map(|x| L2ToL1Message {
-                            from_address: ContractAddress(x.from_address),
-                            payload: x
-                                .payload
-                                .into_iter()
-                                .map(L2ToL1MessagePayloadElem)
-                                .collect(),
-                            to_address: EthereumAddress(x.to_address.0),
-                        })
-                        .collect(),
-                    execution_status: if common.revert_reason.is_empty() {
-                        ExecutionStatus::Succeeded
-                    } else {
-                        ExecutionStatus::Reverted {
-                            reason: common.revert_reason,
-                        }
                     },
-                })
-            }
+                },
+                l2_to_l1_messages: common
+                    .messages_sent
+                    .into_iter()
+                    .map(|x| L2ToL1Message {
+                        from_address: ContractAddress(x.from_address),
+                        payload: x
+                            .payload
+                            .into_iter()
+                            .map(L2ToL1MessagePayloadElem)
+                            .collect(),
+                        to_address: EthereumAddress(x.to_address.0),
+                    })
+                    .collect(),
+                execution_status: if common.revert_reason.is_empty() {
+                    ExecutionStatus::Succeeded
+                } else {
+                    ExecutionStatus::Reverted {
+                        reason: common.revert_reason,
+                    }
+                },
+            }),
         }
     }
 }

crates/pathfinder/src/p2p_network/sync_handlers/conv.rs

@@ -206,8 +206,8 @@ impl ToDto<p2p_proto::receipt::Receipt> for (Transaction, Receipt) {
                 })
                 .collect(),
             execution_resources: {
-                let e = self.1.execution_resources.unwrap_or_default();
-                let da = e.data_availability.unwrap_or_default();
+                let e = self.1.execution_resources;
+                let da = e.data_availability;
                 // Assumption: the values are small enough to fit into u32
                 ExecutionResources {
                     builtins: BuiltinCounter {

crates/rpc/src/lib.rs

@@ -451,16 +451,16 @@ pub mod test_utils {
             }),
         };
         let mut receipt0 = Receipt {
-            execution_resources: Some(ExecutionResources {
+            execution_resources: ExecutionResources {
                 builtin_instance_counter: BuiltinCounters {
                     output_builtin: 33,
                     pedersen_builtin: 32,
                     ..Default::default()
                 },
                 n_memory_holes: 5,
                 n_steps: 10,
-                data_availability: None,
-            }),
+                data_availability: Default::default(),
+            },
             transaction_hash: txn0.hash,
             ..Default::default()
         };
@@ -626,20 +626,20 @@ pub mod test_utils {
                         keys: vec![event_key_bytes!(b"pending key 2")],
                     },
                 ],
-                execution_resources: Some(ExecutionResources::default()),
+                execution_resources: ExecutionResources::default(),
                 transaction_hash: transactions[0].hash,
                 transaction_index: TransactionIndex::new_or_panic(0),
                 ..Default::default()
             },
             Receipt {
-                execution_resources: Some(ExecutionResources::default()),
+                execution_resources: ExecutionResources::default(),
                 transaction_hash: transactions[1].hash,
                 transaction_index: TransactionIndex::new_or_panic(1),
                 ..Default::default()
             },
             // Reverted and without events
             Receipt {
-                execution_resources: Some(ExecutionResources::default()),
+                execution_resources: ExecutionResources::default(),
                 transaction_hash: transactions[2].hash,
                 transaction_index: TransactionIndex::new_or_panic(2),
                 execution_status: ExecutionStatus::Reverted {

crates/rpc/src/v06/method/get_transaction_receipt.rs

@@ -484,7 +484,7 @@ pub mod types {
                     .collect(),
                 events: receipt.events.into_iter().map(Event::from).collect(),
                 revert_reason,
-                execution_resources: receipt.execution_resources.unwrap_or_default().into(),
+                execution_resources: receipt.execution_resources.into(),
                 execution_status: receipt.execution_status.into(),
                 finality_status,
             };
@@ -624,9 +624,7 @@ pub mod types {
                 events: receipt.events.into_iter().map(Event::from).collect(),
                 revert_reason,
                 execution_status: receipt.execution_status.into(),
-                execution_resources: ExecutionResourcesProperties::from(
-                    receipt.execution_resources.unwrap_or_default(),
-                ),
+                execution_resources: receipt.execution_resources.into(),
                 finality_status: FinalityStatus::AcceptedOnL2,
             };
 
@@ -855,7 +853,7 @@ mod tests {
                             },
                             n_memory_holes: 5,
                             n_steps: 10,
-                            data_availability: None,
+                            data_availability: Default::default(),
                         }
                         .into(),
                     }
@@ -906,7 +904,7 @@ mod tests {
                             },
                             n_memory_holes: 5,
                             n_steps: 10,
-                            data_availability: None,
+                            data_availability: Default::default(),
                         }
                         .into(),
                     }
@@ -931,7 +929,7 @@ mod tests {
             },
             n_steps: 9,
             n_memory_holes: 10,
-            data_availability: None,
+            data_availability: Default::default(),
         };
 
         let into = ExecutionResourcesProperties::from(original.clone());

crates/storage/src/connection/transaction.rs

@@ -385,6 +385,7 @@ pub(crate) mod dto {
         pub builtin_instance_counter: BuiltinCounters,
         pub n_steps: u64,
         pub n_memory_holes: u64,
+        // TODO make these mandatory once some new release makes resyncing necessary
         pub l1_gas: Option<u128>,
         pub l1_data_gas: Option<u128>,
     }
@@ -397,12 +398,12 @@ pub(crate) mod dto {
                 n_memory_holes: value.n_memory_holes,
                 data_availability: match (value.l1_gas, value.l1_data_gas) {
                     (Some(l1_gas), Some(l1_data_gas)) => {
-                        Some(pathfinder_common::receipt::ExecutionDataAvailability {
+                        pathfinder_common::receipt::ExecutionDataAvailability {
                             l1_gas,
                             l1_data_gas,
-                        })
+                        }
                     }
-                    _ => None,
+                    _ => Default::default(),
                 },
             }
         }
@@ -414,8 +415,8 @@ pub(crate) mod dto {
                 builtin_instance_counter: (&value.builtin_instance_counter).into(),
                 n_steps: value.n_steps,
                 n_memory_holes: value.n_memory_holes,
-                l1_gas: value.data_availability.as_ref().map(|x| x.l1_gas),
-                l1_data_gas: value.data_availability.as_ref().map(|x| x.l1_data_gas),
+                l1_gas: Some(value.data_availability.l1_gas),
+                l1_data_gas: Some(value.data_availability.l1_data_gas),
             }
         }
     }
@@ -615,7 +616,7 @@ pub(crate) mod dto {
             Self {
                 actual_fee: value.actual_fee,
                 events: value.events.clone(),
-                execution_resources: value.execution_resources.as_ref().map(Into::into),
+                execution_resources: Some((&value.execution_resources).into()),
                 // We don't care about this field anymore.
                 l1_to_l2_consumed_message: None,
                 l2_to_l1_messages: value.l2_to_l1_messages.iter().map(Into::into).collect(),
@@ -647,7 +648,7 @@ pub(crate) mod dto {
             common::Receipt {
                 actual_fee,
                 events,
-                execution_resources: execution_resources.as_ref().map(Into::into),
+                execution_resources: (&execution_resources.unwrap_or_default()).into(),
                 l2_to_l1_messages: l2_to_l1_messages.into_iter().map(Into::into).collect(),
                 transaction_hash,
                 transaction_index,

crates/storage/src/test_utils.rs

@@ -2,7 +2,7 @@ use crate::EmittedEvent;
 
 use super::Storage;
 use pathfinder_common::macro_prelude::*;
-use pathfinder_common::receipt::{ExecutionResources, Receipt};
+use pathfinder_common::receipt::{ExecutionDataAvailability, ExecutionResources, Receipt};
 use pathfinder_common::transaction::{
     DeclareTransactionV0V1, DeployTransaction, EntryPointType, InvokeTransactionV0, Transaction,
     TransactionVariant,
@@ -123,12 +123,15 @@ pub(crate) fn create_transactions_and_receipts() -> [(Transaction, Receipt); NUM
             } else {
                 vec![]
             },
-            execution_resources: Some(ExecutionResources {
+            execution_resources: ExecutionResources {
                 builtin_instance_counter: Default::default(),
                 n_steps: i as u64 + 987,
                 n_memory_holes: i as u64 + 1177,
-                data_availability: None,
-            }),
+                data_availability: ExecutionDataAvailability {
+                    l1_gas: i as u128 + 124,
+                    l1_data_gas: i as u128 + 457,
+                },
+            },
             transaction_hash: tx.hash,
             transaction_index: TransactionIndex::new_or_panic(i as u64 + 2311),
             ..Default::default()