apollo_gateway: set the proof when adding a transaction

[einat-starkware]

No description provided.

[reviewable-StarkWare]

This change is 

[einat-starkware]

• apollo_integration_tests: add txs with proof #11729
• apollo_transaction_converter: use proof manager to convert between transactions #11707
• apollo_transaction_converter: move transaction converter to its own crate #11705
• apollo_proof_manager: log verify and store times #11471
• apollo_gateway: only set proofs if they have passed tx validation in gateway #11703
• apollo_gateway: set the proof when adding a transaction #11638 👈 (View in Graphite)
• apollo_gateway: create GCS client and write proof to GCS #11497
• apollo_gateway: add large storage writer #11362
• apollo_gateway_config: create proof archive writer config #11478
• apollo_gateway: create large storage writer trait #11361
• apollo_gateway: use proof manager #11141 : 1 other dependent PR (#11147 )
• apollo_batcher: delete proof validator and move proof verification to transaction converter #11421 : 1 other dependent PR (#11157 )
• apollo_proof_manager: add proof manager client to the transaction converter #11044
• apollo_node: integrate proof manager server #11043
• apollo_node: integrate proof manager client into node #11042
• apollo_proof_manager: add metrics file #11041
• apollo_http_server: turn off unsupported tx version test #11625
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[meship-starkware]

@meship-starkware reviewed 1 file and all commit messages, and made 3 comments.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @einat-starkware and @noaov1).

---

crates/apollo_gateway/src/gateway.rs line 137 at r1 (raw file):

        let proof_data = if let RpcTransaction::Invoke(RpcInvokeTransaction::V3(ref tx)) = tx {
            if !tx.proof_facts.is_empty() {
                Some((tx.proof_facts.clone(), tx.proof.clone()))

The proof clone can be pretty heavy, no? Can you check the option of wrapping the Proof with Arc?
I think this is good enough for the first version

Code quote:

tx.proof.clone()

---

crates/apollo_gateway/src/gateway.rs line 143 at r1 (raw file):

        } else {
            None
        };

Consider, I think it's better than the nested if-else, but it might be slightly less readable.

Suggestion:

let proof_data = match tx {
    RpcTransaction::Invoke(RpcInvokeTransaction::V3(tx))
        if !tx.proof_facts.is_empty() =>
    {
        Some((tx.proof_facts.clone(), tx.proof.clone()))
    }
    _ => None,
};

---

crates/apollo_gateway/src/gateway.rs line 174 at r1 (raw file):

            tokio::spawn(async move {
                if let Err(e) = proof_archive_writer.set_proof(proof_facts, proof).await {
                    warn!("Failed to archive proof to GCS: {}", e);

How would this show in the logs? I think this should be an error log. Also, do you know if this component is part of the core sequencer pod or will it have its own pod?

Code quote:

 warn!("Failed to archive proof to GCS: {}", e);

[noaov1]

@noaov1 reviewed 1 file and all commit messages, and made 1 comment.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @einat-starkware and @meship-starkware).

---

crates/apollo_gateway/src/gateway.rs line 174 at r1 (raw file):

Previously, meship-starkware (Meshi Peled) wrote…

How would this show in the logs? I think this should be an error log. Also, do you know if this component is part of the core sequencer pod or will it have its own pod?

I agree

[einat-starkware]

@einat-starkware made 3 comments.
Reviewable status: 0 of 1 files reviewed, 3 unresolved discussions (waiting on @meship-starkware and @noaov1).

---

crates/apollo_gateway/src/gateway.rs line 137 at r1 (raw file):

Previously, meship-starkware (Meshi Peled) wrote…

The proof clone can be pretty heavy, no? Can you check the option of wrapping the Proof with Arc?
I think this is good enough for the first version

Proof is already wrapped in Arc :)

---

crates/apollo_gateway/src/gateway.rs line 143 at r1 (raw file):

Previously, meship-starkware (Meshi Peled) wrote…

Consider, I think it's better than the nested if-else, but it might be slightly less readable.

I agree, switched to use match :)

---

crates/apollo_gateway/src/gateway.rs line 174 at r1 (raw file):

Previously, noaov1 (Noa Oved) wrote…

I agree

The proof archive is just in the Gateway pod (as it is only part of the gateway), the proof manager is in the core sequencer pod.

[noaov1]

@noaov1 reviewed 1 file and all commit messages, made 1 comment, and resolved 1 discussion.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @meship-starkware).

[einat-starkware]

@einat-starkware resolved 2 discussions.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @meship-starkware).

[ArniStarkware]

crates/apollo_gateway/src/gateway.rs line 173 at r2 (raw file):

        if let Some((proof_facts, proof)) = proof_data {
            let proof_archive_writer = self.proof_archive_writer.clone();
            tokio::spawn(async move {

why tokio::spawn? Can't we simply call the proof_archive_writer?

Code quote:

 tokio::spawn(async move {

[einat-starkware]

@einat-starkware made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion.

---

crates/apollo_gateway/src/gateway.rs line 173 at r2 (raw file):

Previously, ArniStarkware (Arnon Hod) wrote…

why tokio::spawn? Can't we simply call the proof_archive_writer?

Writing to GCS can be slow and we don't want to hold up the gateway so we want to do it asynchronously - the rest of the system doesn't depend on when we finish writing to GCS, it just needs to happen eventually