From b2f75647266d322da50a755f48bd344145a22cf0 Mon Sep 17 00:00:00 2001 From: ron-starkware Date: Wed, 17 Jun 2026 17:23:05 +0300 Subject: [PATCH] apollo_transaction_converter: version-gate deploy-account address derivation Deploy-account contract addresses are derived inside the shared private helper convert_rpc_tx_to_internal, reached from two public converter methods: convert_rpc_tx_to_internal_rpc_tx (gateway tx ingestion) and convert_consensus_tx_to_internal_consensus_tx (consensus proposal validation). Both now take an AddressDerivationHash parameter, resolved at the call site from the expected (latest) versioned constants (address_derivation_hash()). Both call sites - apollo_gateway and apollo_consensus_orchestrator - already depend on blockifier, so the hash is resolved where it's used rather than where the converter is built; TransactionConverter::new is unchanged, so the batcher/consensus-manager/ mempool-p2p construction sites are untouched and no inert default is threaded through them. InternalRpcDeployAccountTransaction's tx hash now reuses the contract address stored during conversion instead of recomputing it, so the version-gated address flows into the hash. Dormant on its own: the hash is Pedersen for all versions until the activation flag is enabled. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/test_utils.rs | 16 +++-- .../src/validate_proposal.rs | 23 +++++-- crates/apollo_gateway/src/gateway.rs | 25 +++++-- crates/apollo_gateway/src/gateway_test.rs | 12 ++-- .../src/transaction_converter.rs | 14 ++-- .../src/transaction_converter_test.rs | 65 ++++++++++++++++--- crates/starknet_api/src/rpc_transaction.rs | 11 +++- 7 files changed, 122 insertions(+), 44 deletions(-) diff --git a/crates/apollo_consensus_orchestrator/src/test_utils.rs b/crates/apollo_consensus_orchestrator/src/test_utils.rs index 799593f380b..17f51924d75 100644 --- a/crates/apollo_consensus_orchestrator/src/test_utils.rs +++ b/crates/apollo_consensus_orchestrator/src/test_utils.rs @@ -64,7 +64,7 @@ use starknet_api::block::{ }; use starknet_api::block_hash::block_hash_calculator::{BlockHeaderCommitments, PartialBlockHash}; use starknet_api::consensus_transaction::{ConsensusTransaction, InternalConsensusTransaction}; -use starknet_api::core::{ChainId, ContractAddress, Nonce}; +use starknet_api::core::{AddressDerivationHash, ChainId, ContractAddress, Nonce}; use starknet_api::data_availability::L1DataAvailabilityMode; use starknet_api::execution_resources::GasAmount; use starknet_api::felt; @@ -115,10 +115,12 @@ pub(crate) static INTERNAL_TX_BATCH: LazyLock> .iter() .cloned() .map(|tx| { - let (internal_tx, _) = block_on( - TRANSACTION_CONVERTER.convert_consensus_tx_to_internal_consensus_tx(tx), - ) - .unwrap(); + let (internal_tx, _) = + block_on(TRANSACTION_CONVERTER.convert_consensus_tx_to_internal_consensus_tx( + tx, + AddressDerivationHash::Pedersen, + )) + .unwrap(); internal_tx }) .collect() @@ -308,9 +310,9 @@ impl TestDeps { .returning(|_| Ok(tx.clone())); self.transaction_converter .expect_convert_consensus_tx_to_internal_consensus_tx() - .withf(move |internal_tx| internal_tx == tx) + .withf(move |internal_tx, _| internal_tx == tx) // TODO(Einat): Add verification handle when client-side proving is tested in the validate proposal test. - .returning(|_| Ok((internal_tx.clone(), None))); + .returning(|_, _| Ok((internal_tx.clone(), None))); } } diff --git a/crates/apollo_consensus_orchestrator/src/validate_proposal.rs b/crates/apollo_consensus_orchestrator/src/validate_proposal.rs index f02ecbd0dbe..f23779a950d 100644 --- a/crates/apollo_consensus_orchestrator/src/validate_proposal.rs +++ b/crates/apollo_consensus_orchestrator/src/validate_proposal.rs @@ -24,6 +24,9 @@ use apollo_state_sync_types::communication::SharedStateSyncClient; use apollo_time::time::{Clock, ClockExt, DateTime}; use apollo_transaction_converter::{TransactionConverterTrait, VerifyAndStoreProofTask}; use apollo_versioned_constants::VersionedConstants; +// The address-derivation flag lives on blockifier's (full) versioned constants, not the +// lighter apollo_versioned_constants one used above for gas margins. +use blockifier::blockifier_versioned_constants::VersionedConstants as BlockifierVersionedConstants; use futures::channel::mpsc; use futures::StreamExt; use starknet_api::block::{BlockNumber, GasPrice, StarknetVersion}; @@ -564,13 +567,19 @@ async fn handle_proposal_part( // TODO(guyn): check that the length of txs and the number of batches we receive is not // so big it would fill up the memory (in case of a malicious proposal) debug!("Received transaction batch with {} txs", txs.len()); - let conversion_results = - futures::future::join_all(txs.into_iter().map(|tx| { - transaction_converter.convert_consensus_tx_to_internal_consensus_tx(tx) - })) - .await - .into_iter() - .collect::, _>>(); + // Derive deploy-account addresses with the latest versioned constants' hash, matching + // the proposer's gateway (which also predicts the scheme via latest_constants) and the + // block's execution. Relies on the latest version being active fleet-wide -- same + // decision-A boundary hazard documented in apollo_gateway::gateway. + let address_derivation_hash = + BlockifierVersionedConstants::latest_constants().address_derivation_hash(); + let conversion_results = futures::future::join_all(txs.into_iter().map(|tx| { + transaction_converter + .convert_consensus_tx_to_internal_consensus_tx(tx, address_derivation_hash) + })) + .await + .into_iter() + .collect::, _>>(); let conversion_results = match conversion_results { Ok(results) => results, Err(e) => { diff --git a/crates/apollo_gateway/src/gateway.rs b/crates/apollo_gateway/src/gateway.rs index 0ee7e537dda..5d529f02a7b 100644 --- a/crates/apollo_gateway/src/gateway.rs +++ b/crates/apollo_gateway/src/gateway.rs @@ -28,6 +28,7 @@ use apollo_transaction_converter::{ VerificationHandle, }; use async_trait::async_trait; +use blockifier::blockifier_versioned_constants::VersionedConstants; use blockifier::state::contract_class_manager::ContractClassManager; use starknet_api::executable_transaction::AccountTransaction; use starknet_api::rpc_transaction::{ @@ -39,6 +40,7 @@ use starknet_api::rpc_transaction::{ }; use starknet_api::transaction::fields::{Proof, ProofFacts, TransactionSignature}; use starknet_api::transaction::TransactionHash; +use starknet_api::versioned_constants_logic::VersionedConstantsTrait; use starknet_types_core::felt::Felt; use tokio::task::JoinHandle; use tokio::time::timeout; @@ -418,13 +420,22 @@ impl< (InternalRpcTransaction, AccountTransaction, Option<(ProofFacts, Proof)>), StarknetError, > { - let (internal_tx, verification_handle) = - self.transaction_converter.convert_rpc_tx_to_internal_rpc_tx(tx).await.map_err( - |e| { - warn!("Failed to convert RPC transaction to internal RPC transaction: {}", e); - transaction_converter_err_to_deprecated_gw_err(tx_signature, e) - }, - )?; + // Derive deploy-account addresses with the latest versioned constants' hash. The gateway + // must predict the derivation scheme of the (not-yet-assigned) landing block; this matches + // execution once the latest version is active fleet-wide. A tx submitted across a + // derivation-scheme activation boundary (Pedersen->Blake2 at 0.14.4) may be predicted with + // the wrong scheme and rejected -- the accepted decision-A boundary hazard (no separate + // opt-in tx version). + let address_derivation_hash = + VersionedConstants::latest_constants().address_derivation_hash(); + let (internal_tx, verification_handle) = self + .transaction_converter + .convert_rpc_tx_to_internal_rpc_tx(tx, address_derivation_hash) + .await + .map_err(|e| { + warn!("Failed to convert RPC transaction to internal RPC transaction: {}", e); + transaction_converter_err_to_deprecated_gw_err(tx_signature, e) + })?; // Await the verification task immediately. let proof_data = self diff --git a/crates/apollo_gateway/src/gateway_test.rs b/crates/apollo_gateway/src/gateway_test.rs index 38c7cd20411..b3ba4029722 100644 --- a/crates/apollo_gateway/src/gateway_test.rs +++ b/crates/apollo_gateway/src/gateway_test.rs @@ -52,7 +52,7 @@ use blockifier_test_utils::contracts::FeatureContract; use clap::Command; use mempool_test_utils::starknet_api_test_utils::{contract_class, declare_tx}; use metrics_exporter_prometheus::PrometheusBuilder; -use mockall::predicate::eq; +use mockall::predicate::{always, eq}; use rstest::{fixture, rstest}; use starknet_api::core::{ClassHash, ContractAddress, Nonce}; use starknet_api::executable_transaction::AccountTransaction; @@ -299,8 +299,8 @@ fn setup_transaction_converter_mock( mock_transaction_converter .expect_convert_rpc_tx_to_internal_rpc_tx() .once() - .with(eq(rpc_tx)) - .return_once(move |_| Ok((internal_tx, verification_handle))); + .with(eq(rpc_tx), always()) + .return_once(move |_, _| Ok((internal_tx, verification_handle))); let internal_tx = tx_args.get_internal_tx(); let executable_tx = tx_args.get_executable_tx(); @@ -341,8 +341,8 @@ fn setup_transaction_converter_mock_with_failed_verification( mock_transaction_converter .expect_convert_rpc_tx_to_internal_rpc_tx() .once() - .with(eq(rpc_tx)) - .return_once(move |_| Ok((internal_tx, verification_handle))); + .with(eq(rpc_tx), always()) + .return_once(move |_, _| Ok((internal_tx, verification_handle))); // Note: Unlike in the successful case, we don't set up // expect_convert_internal_rpc_tx_to_executable_tx because the verification failure will @@ -677,7 +677,7 @@ async fn test_transaction_converter_error( mock_dependencies .mock_transaction_converter .expect_convert_rpc_tx_to_internal_rpc_tx() - .return_once(|_| expect_internal_rpc_tx_result); + .return_once(|_, _| expect_internal_rpc_tx_result); mock_dependencies .mock_transaction_converter .expect_convert_internal_rpc_tx_to_executable_tx() diff --git a/crates/apollo_transaction_converter/src/transaction_converter.rs b/crates/apollo_transaction_converter/src/transaction_converter.rs index b7f0305d303..4ea5887ddae 100644 --- a/crates/apollo_transaction_converter/src/transaction_converter.rs +++ b/crates/apollo_transaction_converter/src/transaction_converter.rs @@ -82,6 +82,7 @@ pub trait TransactionConverterTrait: Send + Sync { async fn convert_consensus_tx_to_internal_consensus_tx( &self, tx: ConsensusTransaction, + address_derivation_hash: AddressDerivationHash, ) -> TransactionConverterResult<(InternalConsensusTransaction, Option)>; async fn convert_internal_rpc_tx_to_rpc_tx( @@ -92,6 +93,7 @@ pub trait TransactionConverterTrait: Send + Sync { async fn convert_rpc_tx_to_internal_rpc_tx( &self, tx: RpcTransaction, + address_derivation_hash: AddressDerivationHash, ) -> TransactionConverterResult<(InternalRpcTransaction, Option)>; async fn convert_internal_rpc_tx_to_executable_tx( @@ -184,11 +186,13 @@ impl TransactionConverterTrait for TransactionConverter { async fn convert_consensus_tx_to_internal_consensus_tx( &self, tx: ConsensusTransaction, + address_derivation_hash: AddressDerivationHash, ) -> TransactionConverterResult<(InternalConsensusTransaction, Option)> { match tx { ConsensusTransaction::RpcTransaction(tx) => { - let (internal_tx, proof_data) = self.convert_rpc_tx_to_internal(tx).await?; + let (internal_tx, proof_data) = + self.convert_rpc_tx_to_internal(tx, address_derivation_hash).await?; let task = proof_data.map(|(proof_facts, proof)| { self.spawn_verify_and_store_proof(proof_facts, proof) }); @@ -256,8 +260,10 @@ impl TransactionConverterTrait for TransactionConverter { async fn convert_rpc_tx_to_internal_rpc_tx( &self, tx: RpcTransaction, + address_derivation_hash: AddressDerivationHash, ) -> TransactionConverterResult<(InternalRpcTransaction, Option)> { - let (internal_tx, proof_data) = self.convert_rpc_tx_to_internal(tx).await?; + let (internal_tx, proof_data) = + self.convert_rpc_tx_to_internal(tx, address_derivation_hash).await?; let verification_handle = proof_data .map(|(proof_facts, proof)| self.spawn_proof_verification(proof_facts, proof)) .transpose()?; @@ -334,6 +340,7 @@ impl TransactionConverter { async fn convert_rpc_tx_to_internal( &self, tx: RpcTransaction, + address_derivation_hash: AddressDerivationHash, ) -> TransactionConverterResult<(InternalRpcTransaction, Option<(ProofFacts, Proof)>)> { let (tx_without_hash, proof_data) = match tx { RpcTransaction::Invoke(RpcInvokeTransaction::V3(tx)) => { @@ -376,8 +383,7 @@ impl TransactionConverter { ) } RpcTransaction::DeployAccount(RpcDeployAccountTransaction::V3(tx)) => { - let contract_address = - tx.calculate_contract_address(AddressDerivationHash::Pedersen)?; + let contract_address = tx.calculate_contract_address(address_derivation_hash)?; ( InternalRpcTransactionWithoutTxHash::DeployAccount( InternalRpcDeployAccountTransaction { diff --git a/crates/apollo_transaction_converter/src/transaction_converter_test.rs b/crates/apollo_transaction_converter/src/transaction_converter_test.rs index b3d063578bd..a7be0875882 100644 --- a/crates/apollo_transaction_converter/src/transaction_converter_test.rs +++ b/crates/apollo_transaction_converter/src/transaction_converter_test.rs @@ -7,6 +7,7 @@ use blockifier::context::ChainInfo; use blockifier_test_utils::cairo_versions::{CairoVersion, RunnableCairo1}; use mempool_test_utils::starknet_api_test_utils::{ declare_tx, + deploy_account_tx, invoke_tx, invoke_tx_client_side_proving, }; @@ -14,8 +15,14 @@ use mockall::predicate::eq; use rstest::{fixture, rstest}; use starknet_api::compiled_class_hash; use starknet_api::consensus_transaction::ConsensusTransaction; +use starknet_api::core::AddressDerivationHash; use starknet_api::executable_transaction::ValidateCompiledClassHashError; -use starknet_api::rpc_transaction::{RpcDeclareTransaction, RpcTransaction}; +use starknet_api::rpc_transaction::{ + InternalRpcTransaction, + InternalRpcTransactionWithoutTxHash, + RpcDeclareTransaction, + RpcTransaction, +}; use starknet_api::test_utils::{path_in_resources, read_json_file}; use starknet_api::transaction::fields::{Proof, ProofFacts}; @@ -104,8 +111,10 @@ async fn test_compiled_class_hash_mismatch() { ChainInfo::create_for_testing().chain_id, ); - let err = - transaction_converter.convert_rpc_tx_to_internal_rpc_tx(declare_tx).await.unwrap_err(); + let err = transaction_converter + .convert_rpc_tx_to_internal_rpc_tx(declare_tx, AddressDerivationHash::Pedersen) + .await + .unwrap_err(); let expected_code = TransactionConverterError::ValidateCompiledClassHashError( ValidateCompiledClassHashError::CompiledClassHashMismatch { computed_class_hash: other_compiled_class_hash, @@ -136,8 +145,10 @@ async fn test_proof_verification_called_for_invoke_v3_with_proof_facts( let transaction_converter = create_transaction_converter(mock_proof_manager_client); - let (_internal_tx, verification_handle) = - transaction_converter.convert_rpc_tx_to_internal_rpc_tx(invoke_tx).await.unwrap(); + let (_internal_tx, verification_handle) = transaction_converter + .convert_rpc_tx_to_internal_rpc_tx(invoke_tx, AddressDerivationHash::Pedersen) + .await + .unwrap(); await_verification_handle(verification_handle).await; } @@ -151,8 +162,10 @@ async fn test_proof_verification_skipped_for_invoke_v3_without_proof_facts() { let mock_proof_manager_client = MockProofManagerClient::new(); let transaction_converter = create_transaction_converter(mock_proof_manager_client); - let (_internal_tx, verification_handle) = - transaction_converter.convert_rpc_tx_to_internal_rpc_tx(invoke_tx).await.unwrap(); + let (_internal_tx, verification_handle) = transaction_converter + .convert_rpc_tx_to_internal_rpc_tx(invoke_tx, AddressDerivationHash::Pedersen) + .await + .unwrap(); assert!(verification_handle.is_none()); } @@ -189,7 +202,10 @@ async fn test_consensus_tx_to_internal_with_proof_facts_verifies_and_sets_proof( let transaction_converter = create_transaction_converter(mock_proof_manager_client); let (_internal_tx, verify_and_store_proof_task) = transaction_converter - .convert_consensus_tx_to_internal_consensus_tx(consensus_tx) + .convert_consensus_tx_to_internal_consensus_tx( + consensus_tx, + AddressDerivationHash::Pedersen, + ) .await .unwrap(); @@ -221,8 +237,10 @@ async fn test_convert_internal_rpc_tx_to_rpc_tx_with_proof(proof_facts: ProofFac let transaction_converter = create_transaction_converter(mock_proof_manager_client); - let (internal_tx, verification_handle) = - transaction_converter.convert_rpc_tx_to_internal_rpc_tx(rpc_tx.clone()).await.unwrap(); + let (internal_tx, verification_handle) = transaction_converter + .convert_rpc_tx_to_internal_rpc_tx(rpc_tx.clone(), AddressDerivationHash::Pedersen) + .await + .unwrap(); await_verification_handle(verification_handle).await; @@ -231,3 +249,30 @@ async fn test_convert_internal_rpc_tx_to_rpc_tx_with_proof(proof_facts: ProofFac assert_eq!(rpc_tx, rpc_tx_from_internal); } + +/// The deploy-account contract address (and the tx hash that embeds it) must be derived with the +/// AddressDerivationHash passed to the conversion, not a fixed scheme. +#[rstest] +#[tokio::test] +async fn test_deploy_account_conversion_uses_address_derivation_hash() { + let rpc_tx = deploy_account_tx(); + let transaction_converter = create_transaction_converter(MockProofManagerClient::new()); + + let (pedersen_internal, _) = transaction_converter + .convert_rpc_tx_to_internal_rpc_tx(rpc_tx.clone(), AddressDerivationHash::Pedersen) + .await + .unwrap(); + let (blake_internal, _) = transaction_converter + .convert_rpc_tx_to_internal_rpc_tx(rpc_tx, AddressDerivationHash::Blake2) + .await + .unwrap(); + + let deploy_account_address = |internal: &InternalRpcTransaction| match &internal.tx { + InternalRpcTransactionWithoutTxHash::DeployAccount(tx) => tx.contract_address, + _ => panic!("Expected a deploy-account transaction."), + }; + + // The requested hash flows through to the derived address and to the tx hash that embeds it. + assert_ne!(deploy_account_address(&pedersen_internal), deploy_account_address(&blake_internal),); + assert_ne!(pedersen_internal.tx_hash, blake_internal.tx_hash); +} diff --git a/crates/starknet_api/src/rpc_transaction.rs b/crates/starknet_api/src/rpc_transaction.rs index 7ac7a656b03..39c92132dc9 100644 --- a/crates/starknet_api/src/rpc_transaction.rs +++ b/crates/starknet_api/src/rpc_transaction.rs @@ -94,10 +94,15 @@ impl TransactionHasher for InternalRpcDeployAccountTransaction { chain_id: &ChainId, transaction_version: &TransactionVersion, ) -> Result { + // Use the contract address already derived during conversion (with the version-gated hash) + // rather than recomputing it, so the tx hash embeds the same address execution will deploy. match &self.tx { - RpcDeployAccountTransaction::V3(tx) => { - tx.calculate_transaction_hash(chain_id, transaction_version) - } + RpcDeployAccountTransaction::V3(tx) => get_deploy_account_transaction_v3_hash( + tx, + chain_id, + transaction_version, + self.contract_address, + ), } } }