-
Notifications
You must be signed in to change notification settings - Fork 168
fix: defer precompile claims in LocalTransactionProver
#3851
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
Changes from 3 commits
4f4dfa3
c2b5508
17366d4
85c61b2
807905a
1888a50
cfbc7ac
9ec3742
7f9be93
9356b4c
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 |
|---|---|---|
|
|
@@ -341,11 +341,14 @@ impl ProposedBatch { | |
| /// Creates a new [`ProposedBatch`] from the provided parts, verifying every transaction's | ||
| /// execution proof against the transaction kernel. | ||
| /// | ||
| /// Transactions whose precompile claims are still outstanding are accepted: verification checks | ||
| /// that their deferred witness matches their VM proof, and the batch prover settles the claims | ||
| /// of all transactions in the batch with a single precompile proof. | ||
|
Contributor
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. nit:
Contributor
Author
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. I'm working on a follow-up PR that I intend to stack on this one that implements merging the precompiles during batch building, so I would skip describing the planned settlement here. |
||
| /// | ||
| /// # Errors | ||
| /// | ||
| /// Returns an error for any of the batch-validation conditions documented on `new_batch_inner`, | ||
| /// if a transaction's proof fails to verify or does not meet `proof_security_level`, or if the | ||
| /// proof has an outstanding precompile obligation. | ||
| /// or if a transaction's proof fails to verify or does not meet `proof_security_level`. | ||
| pub fn new( | ||
| transactions: Vec<Arc<ProvenTransaction>>, | ||
| reference_block_header: BlockHeader, | ||
|
|
@@ -362,17 +365,14 @@ impl ProposedBatch { | |
|
|
||
| let verifier = TransactionVerifier::new(proof_security_level); | ||
| for tx in batch.transactions() { | ||
| let verification_outcome = verifier.verify(tx).map_err(|source| { | ||
| // The outcome may carry an outstanding precompile obligation, which the batch prover | ||
| // settles for all transactions at once. | ||
| let _verification_outcome = verifier.verify(tx).map_err(|source| { | ||
|
PhilippGackstatter marked this conversation as resolved.
|
||
| ProposedBatchError::TransactionVerificationFailed { | ||
| transaction_id: tx.id(), | ||
| source, | ||
| } | ||
| })?; | ||
| if !verification_outcome.is_complete() { | ||
| return Err(ProposedBatchError::IncompleteTransactionProof { | ||
| transaction_id: tx.id(), | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| Ok(batch) | ||
|
|
@@ -511,13 +511,6 @@ impl Deserializable for ProposedBatch { | |
| .map(Arc::new) | ||
| .collect::<Vec<Arc<ProvenTransaction>>>(); | ||
|
|
||
| if let Some(tx) = transactions.iter().find(|tx| !tx.proof().is_complete()) { | ||
| return Err(DeserializationError::InvalidValue(format!( | ||
| "transaction {} has an outstanding precompile obligation", | ||
| tx.id() | ||
| ))); | ||
| } | ||
|
|
||
| let block_header = BlockHeader::read_from(source)?; | ||
| let partial_blockchain = PartialBlockchain::read_from(source)?; | ||
| let unauthenticated_note_proofs = | ||
|
|
@@ -546,9 +539,14 @@ mod tests { | |
| use crate::Word; | ||
| use crate::account::{AccountType, AccountUpdateDetails}; | ||
| use crate::transaction::{InputNoteCommitment, OutputNote, ProvenTransaction, TxAccountUpdate}; | ||
|
|
||
| #[test] | ||
| fn proposed_batch_serialization() -> anyhow::Result<()> { | ||
| use crate::vm::ExecutionProof; | ||
|
|
||
| /// A proposed batch round-trips whether or not its transactions still owe precompile work, | ||
| /// since settling that work is the batch prover's job. | ||
| #[rstest::rstest] | ||
| #[case::complete(crate::testing::dummy_execution_proof())] | ||
| #[case::deferred(crate::testing::dummy_deferred_execution_proof())] | ||
| fn proposed_batch_serialization(#[case] proof: ExecutionProof) -> anyhow::Result<()> { | ||
| // create partial blockchain with 3 blocks - i.e., 2 peaks | ||
| let mut mmr = Mmr::default(); | ||
| for i in 0..3 { | ||
|
|
@@ -575,7 +573,6 @@ mod tests { | |
| let block_num = reference_block_header.block_num(); | ||
| let block_ref = reference_block_header.commitment(); | ||
| let expiration_block_num = reference_block_header.block_num() + 1; | ||
| let proof = crate::testing::dummy_execution_proof(); | ||
|
|
||
| let account_update = TxAccountUpdate::new( | ||
| account_id, | ||
|
|
@@ -587,7 +584,7 @@ mod tests { | |
| .context("failed to build account update")?; | ||
|
|
||
| let tx = ProvenTransaction::new( | ||
| account_update.clone(), | ||
| account_update, | ||
| Vec::<InputNoteCommitment>::new(), | ||
| Vec::<OutputNote>::new(), | ||
| block_num, | ||
|
|
@@ -599,8 +596,8 @@ mod tests { | |
|
|
||
| let batch = ProposedBatch::new_unverified( | ||
| vec![Arc::new(tx)], | ||
| reference_block_header.clone(), | ||
| partial_blockchain.clone(), | ||
| reference_block_header, | ||
| partial_blockchain, | ||
| BTreeMap::new(), | ||
| ) | ||
| .context("failed to propose batch")?; | ||
|
|
@@ -620,34 +617,6 @@ mod tests { | |
| assert_eq!(batch.input_notes, batch2.input_notes); | ||
| assert_eq!(batch.output_notes, batch2.output_notes); | ||
|
|
||
| let tx = ProvenTransaction::new( | ||
| account_update, | ||
| Vec::<InputNoteCommitment>::new(), | ||
| Vec::<OutputNote>::new(), | ||
| block_num, | ||
| block_ref, | ||
| expiration_block_num, | ||
| crate::testing::dummy_deferred_execution_proof(), | ||
| ) | ||
| .context("failed to build deferred proven transaction")?; | ||
| let transaction_id = tx.id(); | ||
| let batch = ProposedBatch::new_unverified( | ||
| vec![Arc::new(tx)], | ||
| reference_block_header, | ||
| partial_blockchain, | ||
| BTreeMap::new(), | ||
| ) | ||
| .context("failed to propose deferred batch")?; | ||
|
|
||
| let error = ProposedBatch::read_from_bytes(&batch.to_bytes()).unwrap_err(); | ||
| let expected_error = | ||
| format!("transaction {transaction_id} has an outstanding precompile obligation"); | ||
| assert_matches::assert_matches!( | ||
| error, | ||
| DeserializationError::InvalidValue(message) | ||
| if message == expected_error | ||
| ); | ||
|
|
||
| Ok(()) | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.