Skip to content
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

bug(PXE): Transaction simulation should agree with inclusion if on same state #4781

Closed
Tracked by #5070
LHerskind opened this issue Feb 27, 2024 · 1 comment
Closed
Tracked by #5070
Assignees
Labels
A-internal-devex Area: Relates to the devex of internal teams building Aztec. T-bug Type: Bug. Something is broken.

Comments

@LHerskind
Copy link
Contributor

Currently, the simulation of a transaction can produce multiple of the same nullifiers without causing the simulation to fail, even though it would be impossible to actually include in a block.

The simulation of transaction T on state S should only pass if the transaction T can actually be added on state S as part of a block as well.

@LHerskind LHerskind added T-bug Type: Bug. Something is broken. A-internal-devex Area: Relates to the devex of internal teams building Aztec. labels Feb 27, 2024
@github-project-automation github-project-automation bot moved this to Todo in A3 Feb 27, 2024
@LHerskind LHerskind self-assigned this Feb 27, 2024
@LHerskind LHerskind changed the title feat: Transaction simulation should agree with inclusion if on same state feat(PXE): Transaction simulation should agree with inclusion if on same state Mar 9, 2024
@LHerskind LHerskind changed the title feat(PXE): Transaction simulation should agree with inclusion if on same state refactor(PXE): Transaction simulation should agree with inclusion if on same state Mar 9, 2024
@LHerskind LHerskind changed the title refactor(PXE): Transaction simulation should agree with inclusion if on same state bug(PXE): Transaction simulation should agree with inclusion if on same state Mar 9, 2024
@LHerskind LHerskind removed their assignment Mar 11, 2024
@sklppy88 sklppy88 self-assigned this Aug 6, 2024
@sklppy88 sklppy88 moved this from Todo to In Progress in A3 Aug 8, 2024
sklppy88 added a commit that referenced this issue Aug 9, 2024
…on fails (#7840)

This PR makes a simulation of a tx fail, if the tx cannot be included in
a block and added to the state.

e.g. If a simulation produces duplicate nullifiers, or nullifiers that
already exist in a state tree, the results of this simulation should not
be returned, but should warn users that the transaction simulated is
impossible to actually be added to a block due to being an invalid
transaction.

The method for achieving the above is that a new API on the node was
created, used for validating the correctness of the metadata and
side-effects produced by a transaction. A transaction is deemed valid if
and only if the transaction can be added to a block that can be used to
advance state.

Note: this update does not make this validation necessary, and defaults
to offline simulation. Offline simulation is previous non-validated
behavior, and is potentially useful if we ever move to a model where a
node is optional to a pxe.

Another note just for reference: there is weirdness in e2e_prover, that
fails the proof validation.

Resolves #4781.
sklppy88 added a commit that referenced this issue Aug 12, 2024
…on fails (#7840)

This PR makes a simulation of a tx fail, if the tx cannot be included in
a block and added to the state.

e.g. If a simulation produces duplicate nullifiers, or nullifiers that
already exist in a state tree, the results of this simulation should not
be returned, but should warn users that the transaction simulated is
impossible to actually be added to a block due to being an invalid
transaction.

The method for achieving the above is that a new API on the node was
created, used for validating the correctness of the metadata and
side-effects produced by a transaction. A transaction is deemed valid if
and only if the transaction can be added to a block that can be used to
advance state.

Note: this update does not make this validation necessary, and defaults
to offline simulation. Offline simulation is previous non-validated
behavior, and is potentially useful if we ever move to a model where a
node is optional to a pxe.

Another note just for reference: there is weirdness in e2e_prover, that
fails the proof validation.

Resolves #4781.
sklppy88 added a commit that referenced this issue Aug 12, 2024
…on fails (#7840)

This PR makes a simulation of a tx fail, if the tx cannot be included in
a block and added to the state.

e.g. If a simulation produces duplicate nullifiers, or nullifiers that
already exist in a state tree, the results of this simulation should not
be returned, but should warn users that the transaction simulated is
impossible to actually be added to a block due to being an invalid
transaction.

The method for achieving the above is that a new API on the node was
created, used for validating the correctness of the metadata and
side-effects produced by a transaction. A transaction is deemed valid if
and only if the transaction can be added to a block that can be used to
advance state.

Note: this update does not make this validation necessary, and defaults
to offline simulation. Offline simulation is previous non-validated
behavior, and is potentially useful if we ever move to a model where a
node is optional to a pxe.

Another note just for reference: there is weirdness in e2e_prover, that
fails the proof validation.

Resolves #4781.
sklppy88 added a commit that referenced this issue Aug 13, 2024
…on fails (#7840)

This PR makes a simulation of a tx fail, if the tx cannot be included in
a block and added to the state.

e.g. If a simulation produces duplicate nullifiers, or nullifiers that
already exist in a state tree, the results of this simulation should not
be returned, but should warn users that the transaction simulated is
impossible to actually be added to a block due to being an invalid
transaction.

The method for achieving the above is that a new API on the node was
created, used for validating the correctness of the metadata and
side-effects produced by a transaction. A transaction is deemed valid if
and only if the transaction can be added to a block that can be used to
advance state.

Note: this update does not make this validation necessary, and defaults
to offline simulation. Offline simulation is previous non-validated
behavior, and is potentially useful if we ever move to a model where a
node is optional to a pxe.

Another note just for reference: there is weirdness in e2e_prover, that
fails the proof validation.

Resolves #4781.
sklppy88 added a commit that referenced this issue Aug 13, 2024
fix: fixing private voting by correctly throwing an error if simulation fails (#7840)

This PR makes a simulation of a tx fail, if the tx cannot be included in
a block and added to the state.

e.g. If a simulation produces duplicate nullifiers, or nullifiers that
already exist in a state tree, the results of this simulation should not
be returned, but should warn users that the transaction simulated is
impossible to actually be added to a block due to being an invalid
transaction.

The method for achieving the above is that a new API on the node was
created, used for validating the correctness of the metadata and
side-effects produced by a transaction. A transaction is deemed valid if
and only if the transaction can be added to a block that can be used to
advance state.

Note: this update does not make this validation necessary, and defaults
to offline simulation. Offline simulation is previous non-validated
behavior, and is potentially useful if we ever move to a model where a
node is optional to a pxe.

Another note just for reference: there is weirdness in e2e_prover, that
fails the proof validation.

Resolves #4781.

Apply suggestions from code review

Co-authored-by: Nicolás Venturo <[email protected]>
sklppy88 added a commit that referenced this issue Aug 13, 2024
fix: fixing private voting by correctly throwing an error if simulation fails (#7840)

This PR makes a simulation of a tx fail, if the tx cannot be included in
a block and added to the state.

e.g. If a simulation produces duplicate nullifiers, or nullifiers that
already exist in a state tree, the results of this simulation should not
be returned, but should warn users that the transaction simulated is
impossible to actually be added to a block due to being an invalid
transaction.

The method for achieving the above is that a new API on the node was
created, used for validating the correctness of the metadata and
side-effects produced by a transaction. A transaction is deemed valid if
and only if the transaction can be added to a block that can be used to
advance state.

Note: this update does not make this validation necessary, and defaults
to offline simulation. Offline simulation is previous non-validated
behavior, and is potentially useful if we ever move to a model where a
node is optional to a pxe.

Another note just for reference: there is weirdness in e2e_prover, that
fails the proof validation.

Resolves #4781.

Apply suggestions from code review

Co-authored-by: Nicolás Venturo <[email protected]>
sklppy88 added a commit that referenced this issue Aug 18, 2024
fix: fixing private voting by correctly throwing an error if simulation fails (#7840)

This PR makes a simulation of a tx fail, if the tx cannot be included in
a block and added to the state.

e.g. If a simulation produces duplicate nullifiers, or nullifiers that
already exist in a state tree, the results of this simulation should not
be returned, but should warn users that the transaction simulated is
impossible to actually be added to a block due to being an invalid
transaction.

The method for achieving the above is that a new API on the node was
created, used for validating the correctness of the metadata and
side-effects produced by a transaction. A transaction is deemed valid if
and only if the transaction can be added to a block that can be used to
advance state.

Note: this update does not make this validation necessary, and defaults
to offline simulation. Offline simulation is previous non-validated
behavior, and is potentially useful if we ever move to a model where a
node is optional to a pxe.

Another note just for reference: there is weirdness in e2e_prover, that
fails the proof validation.

Resolves #4781.

Apply suggestions from code review

Co-authored-by: Nicolás Venturo <[email protected]>
sklppy88 added a commit that referenced this issue Aug 26, 2024
fix: fixing private voting by correctly throwing an error if simulation fails (#7840)

This PR makes a simulation of a tx fail, if the tx cannot be included in
a block and added to the state.

e.g. If a simulation produces duplicate nullifiers, or nullifiers that
already exist in a state tree, the results of this simulation should not
be returned, but should warn users that the transaction simulated is
impossible to actually be added to a block due to being an invalid
transaction.

The method for achieving the above is that a new API on the node was
created, used for validating the correctness of the metadata and
side-effects produced by a transaction. A transaction is deemed valid if
and only if the transaction can be added to a block that can be used to
advance state.

Note: this update does not make this validation necessary, and defaults
to offline simulation. Offline simulation is previous non-validated
behavior, and is potentially useful if we ever move to a model where a
node is optional to a pxe.

Another note just for reference: there is weirdness in e2e_prover, that
fails the proof validation.

Resolves #4781.

Apply suggestions from code review

Co-authored-by: Nicolás Venturo <[email protected]>
sklppy88 added a commit that referenced this issue Aug 27, 2024
fix: fixing private voting by correctly throwing an error if simulation fails (#7840)

This PR makes a simulation of a tx fail, if the tx cannot be included in
a block and added to the state.

e.g. If a simulation produces duplicate nullifiers, or nullifiers that
already exist in a state tree, the results of this simulation should not
be returned, but should warn users that the transaction simulated is
impossible to actually be added to a block due to being an invalid
transaction.

The method for achieving the above is that a new API on the node was
created, used for validating the correctness of the metadata and
side-effects produced by a transaction. A transaction is deemed valid if
and only if the transaction can be added to a block that can be used to
advance state.

Note: this update does not make this validation necessary, and defaults
to offline simulation. Offline simulation is previous non-validated
behavior, and is potentially useful if we ever move to a model where a
node is optional to a pxe.

Another note just for reference: there is weirdness in e2e_prover, that
fails the proof validation.

Resolves #4781.

Apply suggestions from code review

Co-authored-by: Nicolás Venturo <[email protected]>
sklppy88 added a commit that referenced this issue Aug 27, 2024
fix: fixing private voting by correctly throwing an error if simulation fails (#7840)

This PR makes a simulation of a tx fail, if the tx cannot be included in
a block and added to the state.

e.g. If a simulation produces duplicate nullifiers, or nullifiers that
already exist in a state tree, the results of this simulation should not
be returned, but should warn users that the transaction simulated is
impossible to actually be added to a block due to being an invalid
transaction.

The method for achieving the above is that a new API on the node was
created, used for validating the correctness of the metadata and
side-effects produced by a transaction. A transaction is deemed valid if
and only if the transaction can be added to a block that can be used to
advance state.

Note: this update does not make this validation necessary, and defaults
to offline simulation. Offline simulation is previous non-validated
behavior, and is potentially useful if we ever move to a model where a
node is optional to a pxe.

Another note just for reference: there is weirdness in e2e_prover, that
fails the proof validation.

Resolves #4781.

Apply suggestions from code review

Co-authored-by: Nicolás Venturo <[email protected]>
sklppy88 added a commit that referenced this issue Aug 27, 2024
fix: fixing private voting by correctly throwing an error if simulation fails (#7840)

This PR makes a simulation of a tx fail, if the tx cannot be included in
a block and added to the state.

e.g. If a simulation produces duplicate nullifiers, or nullifiers that
already exist in a state tree, the results of this simulation should not
be returned, but should warn users that the transaction simulated is
impossible to actually be added to a block due to being an invalid
transaction.

The method for achieving the above is that a new API on the node was
created, used for validating the correctness of the metadata and
side-effects produced by a transaction. A transaction is deemed valid if
and only if the transaction can be added to a block that can be used to
advance state.

Note: this update does not make this validation necessary, and defaults
to offline simulation. Offline simulation is previous non-validated
behavior, and is potentially useful if we ever move to a model where a
node is optional to a pxe.

Another note just for reference: there is weirdness in e2e_prover, that
fails the proof validation.

Resolves #4781.

Apply suggestions from code review

Co-authored-by: Nicolás Venturo <[email protected]>
sklppy88 added a commit that referenced this issue Aug 27, 2024
fix: fixing private voting by correctly throwing an error if simulation fails (#7840)

This PR makes a simulation of a tx fail, if the tx cannot be included in
a block and added to the state.

e.g. If a simulation produces duplicate nullifiers, or nullifiers that
already exist in a state tree, the results of this simulation should not
be returned, but should warn users that the transaction simulated is
impossible to actually be added to a block due to being an invalid
transaction.

The method for achieving the above is that a new API on the node was
created, used for validating the correctness of the metadata and
side-effects produced by a transaction. A transaction is deemed valid if
and only if the transaction can be added to a block that can be used to
advance state.

Note: this update does not make this validation necessary, and defaults
to offline simulation. Offline simulation is previous non-validated
behavior, and is potentially useful if we ever move to a model where a
node is optional to a pxe.

Another note just for reference: there is weirdness in e2e_prover, that
fails the proof validation.

Resolves #4781.

Apply suggestions from code review

Co-authored-by: Nicolás Venturo <[email protected]>
sklppy88 added a commit that referenced this issue Aug 28, 2024
fix: fixing private voting by correctly throwing an error if simulation fails (#7840)

This PR makes a simulation of a tx fail, if the tx cannot be included in
a block and added to the state.

e.g. If a simulation produces duplicate nullifiers, or nullifiers that
already exist in a state tree, the results of this simulation should not
be returned, but should warn users that the transaction simulated is
impossible to actually be added to a block due to being an invalid
transaction.

The method for achieving the above is that a new API on the node was
created, used for validating the correctness of the metadata and
side-effects produced by a transaction. A transaction is deemed valid if
and only if the transaction can be added to a block that can be used to
advance state.

Note: this update does not make this validation necessary, and defaults
to offline simulation. Offline simulation is previous non-validated
behavior, and is potentially useful if we ever move to a model where a
node is optional to a pxe.

Another note just for reference: there is weirdness in e2e_prover, that
fails the proof validation.

Resolves #4781.

Apply suggestions from code review

Co-authored-by: Nicolás Venturo <[email protected]>
sklppy88 added a commit that referenced this issue Aug 28, 2024
fix: fixing private voting by correctly throwing an error if simulation fails (#7840)

This PR makes a simulation of a tx fail, if the tx cannot be included in
a block and added to the state.

e.g. If a simulation produces duplicate nullifiers, or nullifiers that
already exist in a state tree, the results of this simulation should not
be returned, but should warn users that the transaction simulated is
impossible to actually be added to a block due to being an invalid
transaction.

The method for achieving the above is that a new API on the node was
created, used for validating the correctness of the metadata and
side-effects produced by a transaction. A transaction is deemed valid if
and only if the transaction can be added to a block that can be used to
advance state.

Note: this update does not make this validation necessary, and defaults
to offline simulation. Offline simulation is previous non-validated
behavior, and is potentially useful if we ever move to a model where a
node is optional to a pxe.

Another note just for reference: there is weirdness in e2e_prover, that
fails the proof validation.

Resolves #4781.

Apply suggestions from code review

Co-authored-by: Nicolás Venturo <[email protected]>
sklppy88 added a commit that referenced this issue Aug 28, 2024
fix: fixing private voting by correctly throwing an error if simulation fails (#7840)

This PR makes a simulation of a tx fail, if the tx cannot be included in
a block and added to the state.

e.g. If a simulation produces duplicate nullifiers, or nullifiers that
already exist in a state tree, the results of this simulation should not
be returned, but should warn users that the transaction simulated is
impossible to actually be added to a block due to being an invalid
transaction.

The method for achieving the above is that a new API on the node was
created, used for validating the correctness of the metadata and
side-effects produced by a transaction. A transaction is deemed valid if
and only if the transaction can be added to a block that can be used to
advance state.

Note: this update does not make this validation necessary, and defaults
to offline simulation. Offline simulation is previous non-validated
behavior, and is potentially useful if we ever move to a model where a
node is optional to a pxe.

Another note just for reference: there is weirdness in e2e_prover, that
fails the proof validation.

Resolves #4781.

Apply suggestions from code review

Co-authored-by: Nicolás Venturo <[email protected]>
sklppy88 added a commit that referenced this issue Aug 28, 2024
fix: fixing private voting by correctly throwing an error if simulation fails (#7840)

This PR makes a simulation of a tx fail, if the tx cannot be included in
a block and added to the state.

e.g. If a simulation produces duplicate nullifiers, or nullifiers that
already exist in a state tree, the results of this simulation should not
be returned, but should warn users that the transaction simulated is
impossible to actually be added to a block due to being an invalid
transaction.

The method for achieving the above is that a new API on the node was
created, used for validating the correctness of the metadata and
side-effects produced by a transaction. A transaction is deemed valid if
and only if the transaction can be added to a block that can be used to
advance state.

Note: this update does not make this validation necessary, and defaults
to offline simulation. Offline simulation is previous non-validated
behavior, and is potentially useful if we ever move to a model where a
node is optional to a pxe.

Another note just for reference: there is weirdness in e2e_prover, that
fails the proof validation.

Resolves #4781.

Apply suggestions from code review

Co-authored-by: Nicolás Venturo <[email protected]>
sklppy88 added a commit that referenced this issue Aug 28, 2024
fix: fixing private voting by correctly throwing an error if simulation fails (#7840)

This PR makes a simulation of a tx fail, if the tx cannot be included in
a block and added to the state.

e.g. If a simulation produces duplicate nullifiers, or nullifiers that
already exist in a state tree, the results of this simulation should not
be returned, but should warn users that the transaction simulated is
impossible to actually be added to a block due to being an invalid
transaction.

The method for achieving the above is that a new API on the node was
created, used for validating the correctness of the metadata and
side-effects produced by a transaction. A transaction is deemed valid if
and only if the transaction can be added to a block that can be used to
advance state.

Note: this update does not make this validation necessary, and defaults
to offline simulation. Offline simulation is previous non-validated
behavior, and is potentially useful if we ever move to a model where a
node is optional to a pxe.

Another note just for reference: there is weirdness in e2e_prover, that
fails the proof validation.

Resolves #4781.

Apply suggestions from code review

Co-authored-by: Nicolás Venturo <[email protected]>
sklppy88 added a commit that referenced this issue Aug 28, 2024
fix: fixing private voting by correctly throwing an error if simulation fails (#7840)

This PR makes a simulation of a tx fail, if the tx cannot be included in
a block and added to the state.

e.g. If a simulation produces duplicate nullifiers, or nullifiers that
already exist in a state tree, the results of this simulation should not
be returned, but should warn users that the transaction simulated is
impossible to actually be added to a block due to being an invalid
transaction.

The method for achieving the above is that a new API on the node was
created, used for validating the correctness of the metadata and
side-effects produced by a transaction. A transaction is deemed valid if
and only if the transaction can be added to a block that can be used to
advance state.

Note: this update does not make this validation necessary, and defaults
to offline simulation. Offline simulation is previous non-validated
behavior, and is potentially useful if we ever move to a model where a
node is optional to a pxe.

Another note just for reference: there is weirdness in e2e_prover, that
fails the proof validation.

Resolves #4781.

Apply suggestions from code review

Co-authored-by: Nicolás Venturo <[email protected]>
sklppy88 added a commit that referenced this issue Aug 28, 2024
fix: fixing private voting by correctly throwing an error if simulation fails (#7840)

This PR makes a simulation of a tx fail, if the tx cannot be included in
a block and added to the state.

e.g. If a simulation produces duplicate nullifiers, or nullifiers that
already exist in a state tree, the results of this simulation should not
be returned, but should warn users that the transaction simulated is
impossible to actually be added to a block due to being an invalid
transaction.

The method for achieving the above is that a new API on the node was
created, used for validating the correctness of the metadata and
side-effects produced by a transaction. A transaction is deemed valid if
and only if the transaction can be added to a block that can be used to
advance state.

Note: this update does not make this validation necessary, and defaults
to offline simulation. Offline simulation is previous non-validated
behavior, and is potentially useful if we ever move to a model where a
node is optional to a pxe.

Another note just for reference: there is weirdness in e2e_prover, that
fails the proof validation.

Resolves #4781.

Apply suggestions from code review

Co-authored-by: Nicolás Venturo <[email protected]>
sklppy88 added a commit that referenced this issue Aug 29, 2024
fix: fixing private voting by correctly throwing an error if simulation fails (#7840)

This PR makes a simulation of a tx fail, if the tx cannot be included in
a block and added to the state.

e.g. If a simulation produces duplicate nullifiers, or nullifiers that
already exist in a state tree, the results of this simulation should not
be returned, but should warn users that the transaction simulated is
impossible to actually be added to a block due to being an invalid
transaction.

The method for achieving the above is that a new API on the node was
created, used for validating the correctness of the metadata and
side-effects produced by a transaction. A transaction is deemed valid if
and only if the transaction can be added to a block that can be used to
advance state.

Note: this update does not make this validation necessary, and defaults
to offline simulation. Offline simulation is previous non-validated
behavior, and is potentially useful if we ever move to a model where a
node is optional to a pxe.

Another note just for reference: there is weirdness in e2e_prover, that
fails the proof validation.

Resolves #4781.

Apply suggestions from code review

Co-authored-by: Nicolás Venturo <[email protected]>
sklppy88 added a commit that referenced this issue Aug 29, 2024
fix: fixing private voting by correctly throwing an error if simulation fails (#7840)

This PR makes a simulation of a tx fail, if the tx cannot be included in
a block and added to the state.

e.g. If a simulation produces duplicate nullifiers, or nullifiers that
already exist in a state tree, the results of this simulation should not
be returned, but should warn users that the transaction simulated is
impossible to actually be added to a block due to being an invalid
transaction.

The method for achieving the above is that a new API on the node was
created, used for validating the correctness of the metadata and
side-effects produced by a transaction. A transaction is deemed valid if
and only if the transaction can be added to a block that can be used to
advance state.

Note: this update does not make this validation necessary, and defaults
to offline simulation. Offline simulation is previous non-validated
behavior, and is potentially useful if we ever move to a model where a
node is optional to a pxe.

Another note just for reference: there is weirdness in e2e_prover, that
fails the proof validation.

Resolves #4781.

Apply suggestions from code review

Co-authored-by: Nicolás Venturo <[email protected]>
sklppy88 added a commit that referenced this issue Aug 30, 2024
…#7951)

Part of #4781 by having parity between sequencer tx validation and node
tx validation.

Note that we are using the validators from the sequencer, and they
should match. We are omitting `phases` and `gas` tx validator which is
in the sequencer and not here is because those tx validators are
customizable by the sequencer and not uniform between all sequencers.

---------

Co-authored-by: Nicolás Venturo <[email protected]>
codygunton pushed a commit that referenced this issue Aug 30, 2024
…#7951)

Part of #4781 by having parity between sequencer tx validation and node
tx validation.

Note that we are using the validators from the sequencer, and they
should match. We are omitting `phases` and `gas` tx validator which is
in the sequencer and not here is because those tx validators are
customizable by the sequencer and not uniform between all sequencers.

---------

Co-authored-by: Nicolás Venturo <[email protected]>
@sklppy88
Copy link
Contributor

sklppy88 commented Sep 5, 2024

Completed via #7951

@sklppy88 sklppy88 closed this as completed Sep 5, 2024
@github-project-automation github-project-automation bot moved this from In Progress to Done in A3 Sep 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-internal-devex Area: Relates to the devex of internal teams building Aztec. T-bug Type: Bug. Something is broken.
Projects
Archived in project
Development

No branches or pull requests

2 participants