Skip to content

Commit f34e016

Browse files
apollo_transaction_converter: return a verification handle when (#12239)
converting transactions
1 parent ad67408 commit f34e016

File tree

7 files changed

+175
-69
lines changed

7 files changed

+175
-69
lines changed

crates/apollo_consensus_orchestrator/src/test_utils.rs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -102,8 +102,11 @@ pub(crate) static INTERNAL_TX_BATCH: LazyLock<Vec<InternalConsensusTransaction>>
102102
.iter()
103103
.cloned()
104104
.map(|tx| {
105-
block_on(TRANSACTION_CONVERTER.convert_consensus_tx_to_internal_consensus_tx(tx))
106-
.unwrap()
105+
let (internal_tx, _) = block_on(
106+
TRANSACTION_CONVERTER.convert_consensus_tx_to_internal_consensus_tx(tx),
107+
)
108+
.unwrap();
109+
internal_tx
107110
})
108111
.collect()
109112
});
@@ -281,7 +284,8 @@ impl TestDeps {
281284
self.transaction_converter
282285
.expect_convert_consensus_tx_to_internal_consensus_tx()
283286
.withf(move |internal_tx| internal_tx == tx)
284-
.returning(|_| Ok(internal_tx.clone()));
287+
// TODO(Einat): Add verification handle when client-side proving is tested in the validate proposal test.
288+
.returning(|_| Ok((internal_tx.clone(), None)));
285289
}
286290
}
287291

crates/apollo_consensus_orchestrator/src/validate_proposal.rs

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ use apollo_l1_gas_price_types::L1GasPriceProviderClient;
2020
use apollo_protobuf::consensus::{ConsensusBlockInfo, ProposalFin, ProposalPart, TransactionBatch};
2121
use apollo_state_sync_types::communication::StateSyncClient;
2222
use apollo_time::time::{Clock, ClockExt, DateTime};
23-
use apollo_transaction_converter::TransactionConverterTrait;
23+
use apollo_transaction_converter::{TransactionConverterTrait, VerificationHandle};
2424
use futures::channel::mpsc;
2525
use futures::StreamExt;
2626
use starknet_api::block::{BlockNumber, GasPrice};
@@ -414,22 +414,47 @@ async fn handle_proposal_part(
414414
// TODO(guyn): check that the length of txs and the number of batches we receive is not
415415
// so big it would fill up the memory (in case of a malicious proposal)
416416
debug!("Received transaction batch with {} txs", txs.len());
417-
let txs =
417+
let conversion_results =
418418
futures::future::join_all(txs.into_iter().map(|tx| {
419419
transaction_converter.convert_consensus_tx_to_internal_consensus_tx(tx)
420420
}))
421421
.await
422422
.into_iter()
423423
.collect::<Result<Vec<_>, _>>();
424-
let txs = match txs {
425-
Ok(txs) => txs,
424+
let conversion_results = match conversion_results {
425+
Ok(results) => results,
426426
Err(e) => {
427427
return HandledProposalPart::Failed(format!(
428428
"Failed to convert transactions. Stopping the build of the current \
429429
proposal. {e:?}"
430430
));
431431
}
432432
};
433+
434+
// Separate internal transactions from verification handles and collect verification
435+
// handles that are not None.
436+
let (txs, handles): (
437+
Vec<InternalConsensusTransaction>,
438+
Vec<Option<VerificationHandle>>,
439+
) = conversion_results.into_iter().unzip();
440+
for handle in handles.into_iter().flatten() {
441+
let proof_facts_hash = handle.proof_facts.hash();
442+
match handle.verification_task.await {
443+
Err(e) => {
444+
return HandledProposalPart::Failed(format!(
445+
"Proof verification task panicked for proof facts hash: \
446+
{proof_facts_hash:?}: {e}"
447+
));
448+
}
449+
Ok(Err(e)) => {
450+
return HandledProposalPart::Failed(format!(
451+
"Proof verification failed for proof facts hash: \
452+
{proof_facts_hash:?}: {e}"
453+
));
454+
}
455+
Ok(Ok(())) => {}
456+
}
457+
}
433458
debug!(
434459
"Converted transactions to internal representation. hashes={:?}",
435460
txs.iter().map(|tx| tx.tx_hash()).collect::<Vec<TransactionHash>>()

crates/apollo_gateway/src/gateway.rs

Lines changed: 41 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -22,18 +22,21 @@ use apollo_network_types::network_types::BroadcastedMessageMetadata;
2222
use apollo_proc_macros::sequencer_latency_histogram;
2323
use apollo_proof_manager_types::SharedProofManagerClient;
2424
use apollo_state_sync_types::communication::SharedStateSyncClient;
25-
use apollo_transaction_converter::{TransactionConverter, TransactionConverterTrait};
25+
use apollo_transaction_converter::{
26+
TransactionConverter,
27+
TransactionConverterTrait,
28+
VerificationHandle,
29+
};
2630
use async_trait::async_trait;
2731
use blockifier::state::contract_class_manager::ContractClassManager;
2832
use starknet_api::executable_transaction::AccountTransaction;
2933
use starknet_api::rpc_transaction::{
3034
InternalRpcTransaction,
3135
InternalRpcTransactionWithoutTxHash,
3236
RpcDeclareTransaction,
33-
RpcInvokeTransaction,
3437
RpcTransaction,
3538
};
36-
use starknet_api::transaction::fields::TransactionSignature;
39+
use starknet_api::transaction::fields::{Proof, ProofFacts, TransactionSignature};
3740
use tracing::{debug, error, info, warn};
3841

3942
use crate::errors::{
@@ -167,18 +170,6 @@ impl<
167170
let mut metric_counters = GatewayMetricHandle::new(&tx, &p2p_message_metadata);
168171
metric_counters.count_transaction_received();
169172

170-
// Extract proof data early for potential archiving later.
171-
let proof_data = match tx {
172-
RpcTransaction::Invoke(RpcInvokeTransaction::V3(ref tx)) => {
173-
if !tx.proof_facts.is_empty() {
174-
Some((tx.proof_facts.clone(), tx.proof.clone()))
175-
} else {
176-
None
177-
}
178-
}
179-
_ => None,
180-
};
181-
182173
if let RpcTransaction::Declare(ref declare_tx) = tx {
183174
if let Err(e) = self.check_declare_permissions(declare_tx) {
184175
metric_counters.record_add_tx_failure(&e);
@@ -190,7 +181,7 @@ impl<
190181
self.stateless_tx_validator.validate(&tx)?;
191182

192183
let tx_signature = tx.signature().clone();
193-
let (internal_tx, executable_tx) =
184+
let (internal_tx, executable_tx, proof_data) =
194185
self.convert_rpc_tx_to_internal_and_executable_txs(tx, &tx_signature).await?;
195186

196187
let mut stateful_transaction_validator = self
@@ -284,15 +275,24 @@ impl<
284275
&self,
285276
tx: RpcTransaction,
286277
tx_signature: &TransactionSignature,
287-
) -> Result<(InternalRpcTransaction, AccountTransaction), StarknetError> {
288-
let internal_tx =
278+
) -> Result<
279+
(InternalRpcTransaction, AccountTransaction, Option<(ProofFacts, Proof)>),
280+
StarknetError,
281+
> {
282+
let (internal_tx, verification_handle) =
289283
self.transaction_converter.convert_rpc_tx_to_internal_rpc_tx(tx).await.map_err(
290284
|e| {
291285
warn!("Failed to convert RPC transaction to internal RPC transaction: {}", e);
292286
transaction_converter_err_to_deprecated_gw_err(tx_signature, e)
293287
},
294288
)?;
295289

290+
// Extract proof data from verification handle before awaiting to be used later to store the
291+
// data.
292+
let proof_data = self
293+
.await_verification_task_and_extract_proof_data(verification_handle, tx_signature)
294+
.await?;
295+
296296
let executable_tx = self
297297
.transaction_converter
298298
.convert_internal_rpc_tx_to_executable_tx(internal_tx.clone())
@@ -302,7 +302,29 @@ impl<
302302
transaction_converter_err_to_deprecated_gw_err(tx_signature, e)
303303
})?;
304304

305-
Ok((internal_tx, executable_tx))
305+
Ok((internal_tx, executable_tx, proof_data))
306+
}
307+
async fn await_verification_task_and_extract_proof_data(
308+
&self,
309+
verification_handle: Option<VerificationHandle>,
310+
tx_signature: &TransactionSignature,
311+
) -> Result<Option<(ProofFacts, Proof)>, StarknetError> {
312+
let proof_data = verification_handle
313+
.as_ref()
314+
.map(|handle| (handle.proof_facts.clone(), handle.proof.clone()));
315+
316+
// Await the verification task immediately.
317+
if let Some(handle) = verification_handle {
318+
let verification_result = handle.verification_task.await.map_err(|e| {
319+
warn!("Proof verification task join error: {}", e);
320+
StarknetError::internal_with_logging("Proof verification task join error:", &e)
321+
})?;
322+
verification_result.map_err(|e| {
323+
warn!("Proof verification failed: {}", e);
324+
transaction_converter_err_to_deprecated_gw_err(tx_signature, e)
325+
})?;
326+
}
327+
Ok(proof_data)
306328
}
307329
}
308330

crates/apollo_gateway/src/gateway_test.rs

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ use apollo_transaction_converter::{
3737
MockTransactionConverterTrait,
3838
TransactionConverterError,
3939
TransactionConverterResult,
40+
VerificationHandle,
4041
};
4142
use blockifier::blockifier::config::ContractClassManagerConfig;
4243
use blockifier::context::ChainInfo;
@@ -266,11 +267,30 @@ fn setup_transaction_converter_mock(
266267
) {
267268
let rpc_tx = tx_args.get_rpc_tx();
268269
let internal_tx = tx_args.get_internal_tx();
270+
271+
// Create verification handle if the transaction has proof facts.
272+
let verification_handle =
273+
if let RpcTransaction::Invoke(RpcInvokeTransaction::V3(ref invoke_tx)) = rpc_tx {
274+
if !invoke_tx.proof_facts.is_empty() {
275+
// Create a simple task that just returns Ok (verification is mocked).
276+
let verification_task = tokio::spawn(async { Ok(()) });
277+
Some(VerificationHandle {
278+
proof_facts: invoke_tx.proof_facts.clone(),
279+
proof: invoke_tx.proof.clone(),
280+
verification_task,
281+
})
282+
} else {
283+
None
284+
}
285+
} else {
286+
None
287+
};
288+
269289
mock_transaction_converter
270290
.expect_convert_rpc_tx_to_internal_rpc_tx()
271291
.once()
272292
.with(eq(rpc_tx))
273-
.return_once(move |_| Ok(internal_tx));
293+
.return_once(move |_| Ok((internal_tx, verification_handle)));
274294

275295
let internal_tx = tx_args.get_internal_tx();
276296
let executable_tx = tx_args.get_executable_tx();
@@ -461,12 +481,15 @@ async fn test_add_tx_positive(
461481
Ok(executable_invoke_tx(invoke_args())),
462482
)]
463483
#[case::internal_to_executable_fails(
464-
Ok(invoke_args().get_internal_tx()),
484+
Ok((invoke_args().get_internal_tx(), None)),
465485
Err(TransactionConverterError::ClassNotFound { class_hash: ClassHash::default() })
466486
)]
467487
#[tokio::test]
468488
async fn test_transaction_converter_error(
469-
#[case] expect_internal_rpc_tx_result: TransactionConverterResult<InternalRpcTransaction>,
489+
#[case] expect_internal_rpc_tx_result: TransactionConverterResult<(
490+
InternalRpcTransaction,
491+
Option<VerificationHandle>,
492+
)>,
470493
#[case] expect_executable_tx_result: TransactionConverterResult<AccountTransaction>,
471494
mut mock_dependencies: MockDependencies,
472495
) {

crates/apollo_transaction_converter/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,4 +9,5 @@ pub use transaction_converter::{
99
TransactionConverterError,
1010
TransactionConverterResult,
1111
TransactionConverterTrait,
12+
VerificationHandle,
1213
};

0 commit comments

Comments
 (0)