-
Notifications
You must be signed in to change notification settings - Fork 53
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adding support for declare v0, l1 handler and l1->l2 messages #321
Changes from 1 commit
789f457
d448b65
b842ae6
3da2f1a
2938bef
b8c554c
62e174f
4c16c8e
7f31dde
50dcc7a
f9650f5
334c258
7c98ebd
bc05e4f
7c63396
1485eca
c857b79
8e510c9
0ca66af
501eaf8
21d12f5
2ad4d19
fc16ff3
2209885
ed5b6c0
eaf5411
f21baad
77038a2
7669670
51054f8
b7a2a3e
8ed287c
638c645
859e50c
2fa3b46
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,7 +4,7 @@ use starknet_core::types::{ | |
DeclareTransactionResult, DeployAccountTransactionResult, InvokeTransactionResult, | ||
}; | ||
use starknet_providers::{Provider, ProviderError}; | ||
|
||
use mp_transactions::BroadcastedDeclareTransactionV0; | ||
use crate::{bail_internal_server_error, errors::StarknetRpcApiError}; | ||
|
||
use super::AddTransactionProvider; | ||
|
@@ -21,6 +21,12 @@ impl<P: Provider + Send + Sync> ForwardToProvider<P> { | |
|
||
#[async_trait] | ||
impl<P: Provider + Send + Sync> AddTransactionProvider for ForwardToProvider<P> { | ||
|
||
async fn add_declare_v0_transaction(&self, declare_v0_transaction: BroadcastedDeclareTransactionV0) -> RpcResult<DeclareTransactionResult> { | ||
// panic here, because we can't really forward it to the real FGW, or shall we enable it so that another madara full node is able to use it? | ||
// maybe a flag for this? as discussed | ||
unimplemented!() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Err(StarknetRpcApiError::UnimplementedMethod.into()) we really dont want to panic, it'll close down the entire node |
||
} | ||
async fn add_declare_transaction( | ||
&self, | ||
declare_transaction: BroadcastedDeclareTransaction, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,11 +3,28 @@ use starknet_core::types::{ | |
BroadcastedDeclareTransaction, BroadcastedDeployAccountTransaction, BroadcastedInvokeTransaction, | ||
DeclareTransactionResult, DeployAccountTransactionResult, InvokeTransactionResult, | ||
}; | ||
|
||
use mp_transactions::BroadcastedDeclareTransactionV0; | ||
use crate::{versions::v0_7_1::StarknetWriteRpcApiV0_7_1Server, Starknet}; | ||
|
||
#[async_trait] | ||
impl StarknetWriteRpcApiV0_7_1Server for Starknet { | ||
/// Submit a new declare transaction to be added to the chain | ||
/// | ||
/// # Arguments | ||
/// | ||
/// * `declare_v0_transaction` - the declare v0 transaction to be added to the chain | ||
/// | ||
/// # Returns | ||
/// | ||
/// * `declare_transaction_result` - the result of the declare transaction | ||
async fn add_declare_v0_transaction( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this should be in a separate node operator rpc endpoint, we really dont want to expose this to the outside world There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this has been resolved |
||
&self, | ||
declare_transaction: BroadcastedDeclareTransactionV0, | ||
) -> RpcResult<DeclareTransactionResult> { | ||
log::info!("add_declare_v0_transaction: {:?}", declare_transaction); | ||
Ok(self.add_transaction_provider.add_declare_v0_transaction(declare_transaction).await?) | ||
} | ||
|
||
/// Submit a new declare transaction to be added to the chain | ||
/// | ||
/// # Arguments | ||
|
Trantorian1 marked this conversation as resolved.
Show resolved
Hide resolved
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
use std::sync::Arc; | ||
|
||
use crate::{into_starknet_api::TransactionApiError, Transaction, TransactionWithHash}; | ||
use crate::{BroadcastedDeclareTransactionV0, into_starknet_api::TransactionApiError, Transaction, TransactionWithHash}; | ||
use blockifier::{execution::errors::ContractClassError, transaction::errors::TransactionExecutionError}; | ||
use mp_chain_config::StarknetVersion; | ||
use mp_class::{ | ||
|
@@ -30,6 +30,52 @@ pub enum BroadcastedToBlockifierError { | |
CompiledClassHashMismatch { expected: Felt, compilation: Felt }, | ||
} | ||
|
||
pub fn broadcasted_to_blockifier_v0( | ||
apoorvsadana marked this conversation as resolved.
Show resolved
Hide resolved
|
||
transaction: BroadcastedDeclareTransactionV0, | ||
chain_id: Felt, | ||
starknet_version: StarknetVersion, | ||
) -> Result< | ||
(blockifier::transaction::transaction_execution::Transaction, Option<ConvertedClass>), | ||
BroadcastedToBlockifierError, | ||
> { | ||
let (class_info, class_hash, extra_class_info) = { | ||
let compressed_legacy_class: CompressedLegacyContractClass = (*transaction.contract_class).clone().into(); | ||
let class_hash = compressed_legacy_class.compute_class_hash().unwrap(); | ||
log::debug!("Computed legacy class hash: {:?}", class_hash); | ||
let compressed_legacy_class: CompressedLegacyContractClass = (*transaction.contract_class).clone().into(); | ||
let class_blockifier = compressed_legacy_class | ||
.to_blockifier_class() | ||
.map_err(BroadcastedToBlockifierError::CompilationFailed)?; | ||
let class_info = LegacyClassInfo { contract_class: Arc::new(compressed_legacy_class) }; | ||
|
||
( | ||
Some(blockifier::execution::contract_class::ClassInfo::new(&class_blockifier, 0, 0)?), | ||
Some(class_hash), | ||
Some(ConvertedClass::Legacy(LegacyConvertedClass { class_hash, info: class_info })), | ||
) }; | ||
|
||
let is_query = transaction.is_query; | ||
let TransactionWithHash { transaction, hash } = | ||
TransactionWithHash::from_broadcasted_v0(transaction, chain_id, starknet_version, class_hash); | ||
let deployed_address = match &transaction { | ||
Transaction::DeployAccount(tx) => Some(tx.calculate_contract_address()), | ||
_ => None, | ||
}; | ||
let transaction: starknet_api::transaction::Transaction = transaction.try_into()?; | ||
|
||
Ok(( | ||
blockifier::transaction::transaction_execution::Transaction::from_api( | ||
transaction, | ||
TransactionHash(hash), | ||
class_info, | ||
None, | ||
deployed_address.map(|address| address.try_into().unwrap()), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. uses expect to explain that this should not panic There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. actually we don't need it for declare v0 but it was missing in other case so added there |
||
is_query, | ||
)?, | ||
extra_class_info, | ||
)) | ||
} | ||
|
||
pub fn broadcasted_to_blockifier( | ||
transaction: starknet_core::types::BroadcastedTransaction, | ||
chain_id: Felt, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
use std::sync::Arc; | ||
use mp_convert::ToFelt; | ||
use starknet_api::transaction::TransactionVersion; | ||
use starknet_types_core::{felt::Felt, hash::StarkHash}; | ||
|
@@ -12,7 +13,8 @@ mod to_starknet_core; | |
|
||
pub mod compute_hash; | ||
pub mod utils; | ||
pub use broadcasted_to_blockifier::{broadcasted_to_blockifier, BroadcastedToBlockifierError}; | ||
pub use broadcasted_to_blockifier::{broadcasted_to_blockifier, BroadcastedToBlockifierError, broadcasted_to_blockifier_v0}; | ||
use mp_class::CompressedLegacyContractClass; | ||
|
||
const SIMULATE_TX_VERSION_OFFSET: Felt = Felt::from_hex_unchecked("0x100000000000000000000000000000000"); | ||
|
||
|
@@ -38,6 +40,20 @@ impl TransactionWithHash { | |
} | ||
} | ||
|
||
#[derive(Debug, Eq, PartialEq, serde::Serialize, serde::Deserialize)] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe de/serialization tests There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. added |
||
pub struct BroadcastedDeclareTransactionV0 { | ||
/// The address of the account contract sending the declaration transaction | ||
pub sender_address: Felt, | ||
/// The maximal fee that can be charged for including the transaction | ||
pub max_fee: Felt, | ||
/// Signature | ||
pub signature: Vec<Felt>, | ||
/// The class to be declared | ||
pub contract_class: Arc<CompressedLegacyContractClass>, | ||
/// If set to `true`, uses a query-only transaction version that's invalid for execution | ||
pub is_query: bool, | ||
} | ||
|
||
#[derive(Debug, Clone, PartialEq, Eq, serde::Serialize, serde::Deserialize)] | ||
pub enum Transaction { | ||
Invoke(InvokeTransaction), | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
might be worth to put a comment here to say why we need declare v0 txs in the first place