Skip to content

Conversation

@avivg-starkware
Copy link
Contributor

places

@avivg-starkware avivg-starkware marked this pull request as ready for review January 21, 2026 11:55
@reviewable-StarkWare
Copy link

This change is Reviewable

Copy link
Contributor Author

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

Copy link
Contributor

@einat-starkware einat-starkware left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


crates/apollo_transaction_converter/src/transaction_converter.rs line 229 at r1 (raw file):

            RpcTransaction::Invoke(RpcInvokeTransaction::V3(tx)) => {
                // Verify proof here; storage happens in the gateway after validation to avoid
                // storing proofs for rejected transactions.

Do you think it's necessary to mention the validators? When converting from consensus to internal consensus they also call this function, but the storage happens in the outer function?

Code quote:

                // Verify proof here; storage happens in the gateway after validation to avoid
                // storing proofs for rejected transactions.

@avivg-starkware avivg-starkware force-pushed the avivg/apollo_gateway/document_proof_verification_and_storage branch from 5be11fd to 98778b3 Compare January 21, 2026 12:39
Copy link
Contributor Author

@avivg-starkware avivg-starkware left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@avivg-starkware made 1 comment.
Reviewable status: 1 of 2 files reviewed, 1 unresolved discussion (waiting on @einat-starkware and @noaov1).


crates/apollo_transaction_converter/src/transaction_converter.rs line 229 at r1 (raw file):

Previously, einat-starkware wrote…

Do you think it's necessary to mention the validators? When converting from consensus to internal consensus they also call this function, but the storage happens in the outer function?

yes!:) changed

Copy link
Contributor

@einat-starkware einat-starkware left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

@einat-starkware reviewed 1 file and all commit messages, made 1 comment, and resolved 1 discussion.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @noaov1).

Copy link
Collaborator

@noaov1 noaov1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

@noaov1 reviewed 2 files and all commit messages, and made 1 comment.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @avivg-starkware).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants