apollo_protobuf: split InvokeV3 to InvokeV3WithProof and InvokeV3WithoutProof

[avivg-starkware]

No description provided.

[reviewable-StarkWare]

This change is 

[avivg-starkware]

• apollo_protobuf: split InvokeV3 to InvokeV3WithProof and InvokeV3WithoutProof #11538 👈 (View in Graphite)
• main

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

[avivg-starkware]

@ShahakShama , I saw there's a separate repo for p2p :
 starknet-p2p-specs

Are there any appropriate changes that need to be made there?

@avivg-starkware made 8 comments.
Reviewable status: 0 of 8 files reviewed, 7 unresolved discussions (waiting on @ShahakShama).

---

crates/apollo_protobuf/src/converters/rpc_transaction.rs line 178 at r1 (raw file):

        (snapi_invoke, proof).into()
    }
}

Alternatively, we can skip the conversion from and into <(InvokeTransactionV3Common, Proof)>

It's here mainly for cleaner separation & aligning with the Declare scheme.

Suggestion:

impl TryFrom<protobuf::InvokeV3WithProof> for RpcInvokeTransactionV3 {
    type Error = ProtobufConversionError;
    fn try_from(value: protobuf::InvokeV3WithProof) -> Result<Self, Self::Error> {
        let common = InvokeTransactionV3Common::try_from(value.common.ok_or(
            ProtobufConversionError::MissingField {
                field_description: "InvokeV3WithProof::common",
            },
        )?)?;
        let proof = Proof::from(value.proof);

        Ok(Self {
            resource_bounds: match common.resource_bounds {
                ValidResourceBounds::AllResources(resource_bounds) => resource_bounds,
                _ => {
                    return Err(DEPRECATED_RESOURCE_BOUNDS_ERROR);
                }
            },
            tip: common.tip,
            signature: common.signature,
            nonce: common.nonce,
            sender_address: common.sender_address,
            calldata: common.calldata,
            nonce_data_availability_mode: common.nonce_data_availability_mode,
            fee_data_availability_mode: common.fee_data_availability_mode,
            paymaster_data: common.paymaster_data,
            account_deployment_data: common.account_deployment_data,
            proof_facts: common.proof_facts,
            proof,
        })
    }
}

impl From<RpcInvokeTransactionV3> for protobuf::InvokeV3WithProof {
    fn from(mut value: RpcInvokeTransactionV3) -> Self {
        let proof = Arc::unwrap_or_clone(std::mem::take(&mut value.proof).0);
        let common = InvokeTransactionV3Common {
            resource_bounds: ValidResourceBounds::AllResources(value.resource_bounds),
            tip: value.tip,
            signature: value.signature,
            nonce: value.nonce,
            sender_address: value.sender_address,
            calldata: value.calldata,
            nonce_data_availability_mode: value.nonce_data_availability_mode,
            fee_data_availability_mode: value.fee_data_availability_mode,
            paymaster_data: value.paymaster_data,
            account_deployment_data: value.account_deployment_data,
            proof_facts: value.proof_facts,
        };
        Self { common: Some(common.into()), proof }
    }
}

---

crates/apollo_protobuf/src/protobuf/protoc_output.rs line 284 at r1 (raw file):

#[allow(clippy::derive_partial_eq_without_eq)]
#[derive(Clone, PartialEq, ::prost::Message)]
pub struct InvokeV3Common {

Not sure whether we'd like to perhaps make this as "without proof" and not have a "common"

And then remove the InvokeV3WithoutProof defined in the file below.
And make the InvokeV3WithProof as below

(I find the current cleanest)

Suggestion (i):

pub struct InvokeV3WithoutProof  

Code snippet (ii):

pub struct InvokeV3WithProof {
    #[prost(message, optional, tag = "1")]
    pub invoke_without_proof: ::core::option::Option<InvokeV3WithoutProof>,
    #[prost(uint32, repeated, tag = "2")]
    pub proof: ::prost::alloc::vec::Vec<u32>,
}

---

crates/apollo_protobuf/src/protobuf/protoc_output.rs line 308 at r1 (raw file):

    pub proof_facts: ::prost::alloc::vec::Vec<Felt252>,
}
/// see <https://external.integration.starknet.io/feeder_gateway/get_transaction?transactionHash=0x41906f1c314cca5f43170ea75d3b1904196a10101190d2b12a41cc61cfd17c>

@ShahakShama - could you elaborate on that link?
Is this a link to a matching FGW tx? I tried searching online and got bad access.

Code quote:

/// see <https://external.integration.starknet.io/feeder_gateway/get_transaction?transactionHash=0x41906f1c314cca5f43170ea75d3b1904196a10101190d2b12a41cc61cfd17c>

---

crates/apollo_protobuf/src/proto/p2p/proto/transaction.proto line 64 at r1 (raw file):

}

// see https://external.integration.starknet.io/feeder_gateway/get_transaction?transactionHash=0x41906f1c314cca5f43170ea75d3b1904196a10101190d2b12a41cc61cfd17c

@ShahakShama - could you elaborate on that link?
Is this a link to a matching FGW tx? I tried searching online and got bad access.

Code quote:

// see https://external.integration.starknet.io/feeder_gateway/get_transaction?transactionHash=0x41906f1c314cca5f43170ea75d3b1904196a10101190d2b12a41cc61cfd17c

---

crates/apollo_protobuf/src/transaction.rs line 149 at r1 (raw file):

}

impl TryFrom<protobuf::InvokeV3Common> for InvokeTransactionV3Common {

function same as to:
impl TryFrom<protobuf::InvokeV3> for InvokeTransactionV3 {

Code quote:

impl TryFrom<protobuf::InvokeV3Common> for InvokeTransactionV3Common {

---

crates/apollo_protobuf/src/transaction.rs line 220 at r1 (raw file):

}

impl From<InvokeTransactionV3Common> for protobuf::InvokeV3Common {

function same as:
impl From<InvokeTransactionV3> for protobuf::InvokeV3 {

Code quote:

impl From<InvokeTransactionV3Common> for protobuf::InvokeV3Common {

---

crates/apollo_protobuf/src/converters/transaction.rs line 633 at r1 (raw file):

        Self { common: Some(common.into()) }
    }
}

Similar to above, but from the other direction:
To be more explicit, we can add those

Suggestion:

impl TryFrom<protobuf::transaction_in_block::InvokeV3WithoutProof> for InvokeTransactionV3Common {
    type Error = ProtobufConversionError;
    fn try_from(
        value: protobuf::transaction_in_block::InvokeV3WithoutProof,
    ) -> Result<Self, Self::Error> {
        InvokeTransactionV3Common::try_from(
            value.common.ok_or(missing("InvokeV3WithoutProof::common"))?,
        )
    }
}

impl From<InvokeTransactionV3Common> for protobuf::transaction_in_block::InvokeV3WithoutProof {
    fn from(value: InvokeTransactionV3Common) -> Self {
        Self { common: Some(value.into()) }
    }
}

impl TryFrom<protobuf::transaction_in_block::InvokeV3WithoutProof> for InvokeTransactionV3 {
    type Error = ProtobufConversionError;
    fn try_from(
        value: protobuf::transaction_in_block::InvokeV3WithoutProof,
    ) -> Result<Self, Self::Error> {
        let common: InvokeTransactionV3Common = value.try_into()?;

        Ok(Self {
            resource_bounds: common.resource_bounds,
            tip: common.tip,
            signature: common.signature,
            nonce: common.nonce,
            sender_address: common.sender_address,
            calldata: common.calldata,
            nonce_data_availability_mode: common.nonce_data_availability_mode,
            fee_data_availability_mode: common.fee_data_availability_mode,
            paymaster_data: common.paymaster_data,
            account_deployment_data: common.account_deployment_data,
            proof_facts: common.proof_facts,
        })
    }
}

impl From<InvokeTransactionV3> for protobuf::transaction_in_block::InvokeV3WithoutProof {
    fn from(value: InvokeTransactionV3) -> Self {
        let common = InvokeTransactionV3Common {
            resource_bounds: value.resource_bounds,
            tip: value.tip,
            signature: value.signature,
            nonce: value.nonce,
            sender_address: value.sender_address,
            calldata: value.calldata,
            nonce_data_availability_mode: value.nonce_data_availability_mode,
            fee_data_availability_mode: value.fee_data_availability_mode,
            paymaster_data: value.paymaster_data,
            account_deployment_data: value.account_deployment_data,
            proof_facts: value.proof_facts,
        };
        common.into()
    }
}

[ShahakShama]

@ShahakShama reviewed 8 files and all commit messages, and made 5 comments.
Reviewable status: all files reviewed, 10 unresolved discussions (waiting on @avivg-starkware, @meship-starkware, and @noaov1).

---

crates/apollo_protobuf/src/proto/p2p/proto/transaction.proto line 64 at r1 (raw file):

Previously, avivg-starkware wrote…

@ShahakShama - could you elaborate on that link?
Is this a link to a matching FGW tx? I tried searching online and got bad access.

This is a transaction that has the invoke structure. I think it can stay the same assuming that once integration is upgraded it will have proof_facts for this tx as well

---

crates/apollo_protobuf/src/protobuf/protoc_output.rs line 308 at r1 (raw file):

Previously, avivg-starkware wrote…

@ShahakShama - could you elaborate on that link?
Is this a link to a matching FGW tx? I tried searching online and got bad access.

Then this link is broken. If you want to do scoutgirl then you can find a new invoke tx but you don't need to :)

---

crates/apollo_protobuf/src/converters/rpc_transaction.rs line 115 at r1 (raw file):

}

impl TryFrom<protobuf::InvokeV3WithProof> for (InvokeTransactionV3Common, Proof) {

Remove this and have the conversion to RpcInvokeTransactionV3 directly

---

crates/apollo_protobuf/src/transaction.rs line 135 at r1 (raw file):

#[derive(Clone, Debug, Deserialize, Eq, Hash, Ord, PartialEq, PartialOrd, Serialize)]
pub(crate) struct InvokeTransactionV3Common {

Isn't this the same as snapi's tx? If yes, remove this

---

crates/apollo_protobuf/src/proto/p2p/proto/transaction.proto line 50 at r1 (raw file):

}

message InvokeV3Common {

Unlike declare, here this is the full invoke v3 type. IMO you should rename this back to InvokeV3

[ShahakShama]

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

[avivg-starkware]

@avivg-starkware made 2 comments and resolved 1 discussion.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @meship-starkware and @noaov1).

---

crates/apollo_protobuf/src/converters/rpc_transaction.rs line 115 at r1 (raw file):

Previously, ShahakShama wrote…

Remove this and have the conversion to RpcInvokeTransactionV3 directly

Done.

---

crates/apollo_protobuf/src/transaction.rs line 135 at r1 (raw file):

Previously, ShahakShama wrote…

Isn't this the same as snapi's tx? If yes, remove this

Done.