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.

Sign up for GitHub

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

Jump to bottom

chore(avm): handle parsing error #10203

Merged
merged 4 commits into from
Nov 27, 2024
Merged

chore(avm): handle parsing error #10203

merged 4 commits into from
Nov 27, 2024

Conversation

jeanmon
Copy link
Contributor

@jeanmon jeanmon commented Nov 26, 2024

Resolves #9770

@jeanmon jeanmon force-pushed the jm/9770-parsing-error-handling branch from 5f07b44 to b7bf44a Compare November 26, 2024 10:33
@jeanmon jeanmon marked this pull request as ready for review November 26, 2024 10:38
IlyasRidhuan
IlyasRidhuan requested changes Nov 26, 2024
View reviewed changes
Copy link
Contributor

@IlyasRidhuan IlyasRidhuan left a comment

Choose a reason for hiding this comment

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

Blocking until this at least to stop the merge conflicts hell

fcarreiro
fcarreiro reviewed Nov 26, 2024
View reviewed changes
Copy link
Contributor

@fcarreiro fcarreiro left a comment

Choose a reason for hiding this comment

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

Let's maybe also wait for @IlyasRidhuan 's PRs to go in first since he's getting many conflicts and he's in the critical path.

yarn-project/simulator/src/avm/errors.ts Outdated Show resolved Hide resolved
yarn-project/simulator/src/avm/errors.ts Outdated
*/
export class InvalidOpcodeError extends AvmExecutionError {
constructor(opcode: Opcode) {
super(`Opcode ${Opcode[opcode]} (0x${opcode.toString(16)}) not implemented`);
Copy link
Contributor

Choose a reason for hiding this comment

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

BTW, I know this was there already but it's not ideal. It works ok if we have an opcode in the list that is not implemented, but it does not work if you encounter a value which is not any of the existing opcodes (and you'd want to catch that as well).

yarn-project/simulator/src/avm/serialization/bytecode_serialization.ts Outdated Show resolved Hide resolved
yarn-project/simulator/src/avm/serialization/bytecode_serialization.ts Show resolved Hide resolved
yarn-project/simulator/src/avm/opcodes/memory.ts
@@ -59,16 +59,19 @@ export class Set extends Instruction {
private value: bigint | number,
) {
super();
TaggedMemory.checkIsValidTag(inTag);
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm I see. Ideally we'd do this at deserialization time, but there we don't know if something is a tag so we cant. I guess it's fine to put this here :)

Copy link
Contributor Author

@jeanmon jeanmon Nov 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@fcarreiro Otherwise, I could have defined a Tag type in the serialization layer. I can still do that if you prefer. I decided to not be too intrusive with the current code.

@jeanmon jeanmon force-pushed the jm/9770-parsing-error-handling branch from b7bf44a to eba6ec4 Compare November 26, 2024 16:26
@fcarreiro fcarreiro self-requested a review November 26, 2024 17:01
@jeanmon jeanmon force-pushed the jm/9770-parsing-error-handling branch from eba6ec4 to 3d1a27e Compare November 27, 2024 11:00
@IlyasRidhuan IlyasRidhuan self-requested a review November 27, 2024 11:03
@jeanmon jeanmon merged commit 3c623fc into master Nov 27, 2024
72 checks passed
@jeanmon jeanmon deleted the jm/9770-parsing-error-handling branch November 27, 2024 12:04
@AztecBot AztecBot mentioned this pull request Nov 27, 2024
Merged
critesjosh pushed a commit that referenced this pull request Nov 27, 2024
@AztecBot
chore(master): Release 0.65.1 (#10219)
62fc917 
🤖 I have created a release *beep* *boop*
---


<details><summary>aztec-package: 0.65.1</summary>

##
[0.65.1](aztec-package-v0.65.0...aztec-package-v0.65.1)
(2024-11-27)


### Miscellaneous

* Delete old serialization methods
([#9951](#9951))
([10d3f6f](10d3f6f))
</details>

<details><summary>barretenberg.js: 0.65.1</summary>

##
[0.65.1](barretenberg.js-v0.65.0...barretenberg.js-v0.65.1)
(2024-11-27)


### Features

* Speed up transaction execution
([#10172](#10172))
([da265b6](da265b6))


### Bug Fixes

* Add pako as a dependency in bb.js
([#10186](#10186))
([b773c14](b773c14))
</details>

<details><summary>aztec-packages: 0.65.1</summary>

##
[0.65.1](aztec-packages-v0.65.0...aztec-packages-v0.65.1)
(2024-11-27)


### Features

* Add total mana used to header
([#9868](#9868))
([2478d19](2478d19))
* Assert metrics in network tests
([#10215](#10215))
([9380c0f](9380c0f))
* Avm inserts nullifiers from private
([#10129](#10129))
([3fc0c7c](3fc0c7c))
* Burn congestion fee
([#10231](#10231))
([20a33f2](20a33f2))
* Configure world state block history
([#10216](#10216))
([01eb392](01eb392))
* Integrate fee into rollup
([#10176](#10176))
([12744d6](12744d6))
* Speed up transaction execution
([#10172](#10172))
([da265b6](da265b6))
* Using current gas prices in cli-wallet
([#10105](#10105))
([15ffeea](15ffeea))


### Bug Fixes

* Add pako as a dependency in bb.js
([#10186](#10186))
([b773c14](b773c14))
* **avm:** Execution test ordering
([#10226](#10226))
([49b4a6c](49b4a6c))
* Deploy preview master
([#10227](#10227))
([321a175](321a175))
* Use current base fee for public fee payment
([#10230](#10230))
([f081d80](f081d80))


### Miscellaneous

* Add traces and histograms to avm simulator
([#10233](#10233))
([e83726d](e83726d)),
closes
[#10146](#10146)
* Avm-proving and avm-integration tests do not require simulator to
export function with jest mocks
([#10228](#10228))
([f28fcdb](f28fcdb))
* **avm:** Handle parsing error
([#10203](#10203))
([3c623fc](3c623fc)),
closes
[#9770](#9770)
* **avm:** Zero initialization in avm public inputs and execution test
fixes
([#10238](#10238))
([0c7c4c9](0c7c4c9))
* Bump timeout for after-hook for data store test again
([#10240](#10240))
([52047f0](52047f0))
* CIVC VK
([#10223](#10223))
([089c34c](089c34c))
* Declare global types
([#10206](#10206))
([7b2e343](7b2e343))
* Delete old serialization methods
([#9951](#9951))
([10d3f6f](10d3f6f))
* Fix migration notes
([#10252](#10252))
([05bdcd5](05bdcd5))
* Pull out some sync changes
([#10245](#10245))
([1bfc15e](1bfc15e))
* Remove docs from sync
([#10241](#10241))
([eeea0aa](eeea0aa))
* Replace relative paths to noir-protocol-circuits
([e7690ca](e7690ca))
</details>

<details><summary>barretenberg: 0.65.1</summary>

##
[0.65.1](barretenberg-v0.65.0...barretenberg-v0.65.1)
(2024-11-27)


### Features

* Add total mana used to header
([#9868](#9868))
([2478d19](2478d19))
* Configure world state block history
([#10216](#10216))
([01eb392](01eb392))
* Speed up transaction execution
([#10172](#10172))
([da265b6](da265b6))


### Bug Fixes

* **avm:** Execution test ordering
([#10226](#10226))
([49b4a6c](49b4a6c))


### Miscellaneous

* **avm:** Handle parsing error
([#10203](#10203))
([3c623fc](3c623fc)),
closes
[#9770](#9770)
* **avm:** Zero initialization in avm public inputs and execution test
fixes
([#10238](#10238))
([0c7c4c9](0c7c4c9))
* CIVC VK
([#10223](#10223))
([089c34c](089c34c))
* Pull out some sync changes
([#10245](#10245))
([1bfc15e](1bfc15e))
</details>

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
AztecBot added a commit to AztecProtocol/barretenberg that referenced this pull request Nov 28, 2024
@AztecBot
chore(master): Release 0.65.1 (#10219)
14f7045 
🤖 I have created a release *beep* *boop*
---


<details><summary>aztec-package: 0.65.1</summary>

##
[0.65.1](AztecProtocol/aztec-packages@aztec-package-v0.65.0...aztec-package-v0.65.1)
(2024-11-27)


### Miscellaneous

* Delete old serialization methods
([#9951](AztecProtocol/aztec-packages#9951))
([10d3f6f](AztecProtocol/aztec-packages@10d3f6f))
</details>

<details><summary>barretenberg.js: 0.65.1</summary>

##
[0.65.1](AztecProtocol/aztec-packages@barretenberg.js-v0.65.0...barretenberg.js-v0.65.1)
(2024-11-27)


### Features

* Speed up transaction execution
([#10172](AztecProtocol/aztec-packages#10172))
([da265b6](AztecProtocol/aztec-packages@da265b6))


### Bug Fixes

* Add pako as a dependency in bb.js
([#10186](AztecProtocol/aztec-packages#10186))
([b773c14](AztecProtocol/aztec-packages@b773c14))
</details>

<details><summary>aztec-packages: 0.65.1</summary>

##
[0.65.1](AztecProtocol/aztec-packages@aztec-packages-v0.65.0...aztec-packages-v0.65.1)
(2024-11-27)


### Features

* Add total mana used to header
([#9868](AztecProtocol/aztec-packages#9868))
([2478d19](AztecProtocol/aztec-packages@2478d19))
* Assert metrics in network tests
([#10215](AztecProtocol/aztec-packages#10215))
([9380c0f](AztecProtocol/aztec-packages@9380c0f))
* Avm inserts nullifiers from private
([#10129](AztecProtocol/aztec-packages#10129))
([3fc0c7c](AztecProtocol/aztec-packages@3fc0c7c))
* Burn congestion fee
([#10231](AztecProtocol/aztec-packages#10231))
([20a33f2](AztecProtocol/aztec-packages@20a33f2))
* Configure world state block history
([#10216](AztecProtocol/aztec-packages#10216))
([01eb392](AztecProtocol/aztec-packages@01eb392))
* Integrate fee into rollup
([#10176](AztecProtocol/aztec-packages#10176))
([12744d6](AztecProtocol/aztec-packages@12744d6))
* Speed up transaction execution
([#10172](AztecProtocol/aztec-packages#10172))
([da265b6](AztecProtocol/aztec-packages@da265b6))
* Using current gas prices in cli-wallet
([#10105](AztecProtocol/aztec-packages#10105))
([15ffeea](AztecProtocol/aztec-packages@15ffeea))


### Bug Fixes

* Add pako as a dependency in bb.js
([#10186](AztecProtocol/aztec-packages#10186))
([b773c14](AztecProtocol/aztec-packages@b773c14))
* **avm:** Execution test ordering
([#10226](AztecProtocol/aztec-packages#10226))
([49b4a6c](AztecProtocol/aztec-packages@49b4a6c))
* Deploy preview master
([#10227](AztecProtocol/aztec-packages#10227))
([321a175](AztecProtocol/aztec-packages@321a175))
* Use current base fee for public fee payment
([#10230](AztecProtocol/aztec-packages#10230))
([f081d80](AztecProtocol/aztec-packages@f081d80))


### Miscellaneous

* Add traces and histograms to avm simulator
([#10233](AztecProtocol/aztec-packages#10233))
([e83726d](AztecProtocol/aztec-packages@e83726d)),
closes
[#10146](AztecProtocol/aztec-packages#10146)
* Avm-proving and avm-integration tests do not require simulator to
export function with jest mocks
([#10228](AztecProtocol/aztec-packages#10228))
([f28fcdb](AztecProtocol/aztec-packages@f28fcdb))
* **avm:** Handle parsing error
([#10203](AztecProtocol/aztec-packages#10203))
([3c623fc](AztecProtocol/aztec-packages@3c623fc)),
closes
[#9770](AztecProtocol/aztec-packages#9770)
* **avm:** Zero initialization in avm public inputs and execution test
fixes
([#10238](AztecProtocol/aztec-packages#10238))
([0c7c4c9](AztecProtocol/aztec-packages@0c7c4c9))
* Bump timeout for after-hook for data store test again
([#10240](AztecProtocol/aztec-packages#10240))
([52047f0](AztecProtocol/aztec-packages@52047f0))
* CIVC VK
([#10223](AztecProtocol/aztec-packages#10223))
([089c34c](AztecProtocol/aztec-packages@089c34c))
* Declare global types
([#10206](AztecProtocol/aztec-packages#10206))
([7b2e343](AztecProtocol/aztec-packages@7b2e343))
* Delete old serialization methods
([#9951](AztecProtocol/aztec-packages#9951))
([10d3f6f](AztecProtocol/aztec-packages@10d3f6f))
* Fix migration notes
([#10252](AztecProtocol/aztec-packages#10252))
([05bdcd5](AztecProtocol/aztec-packages@05bdcd5))
* Pull out some sync changes
([#10245](AztecProtocol/aztec-packages#10245))
([1bfc15e](AztecProtocol/aztec-packages@1bfc15e))
* Remove docs from sync
([#10241](AztecProtocol/aztec-packages#10241))
([eeea0aa](AztecProtocol/aztec-packages@eeea0aa))
* Replace relative paths to noir-protocol-circuits
([e7690ca](AztecProtocol/aztec-packages@e7690ca))
</details>

<details><summary>barretenberg: 0.65.1</summary>

##
[0.65.1](AztecProtocol/aztec-packages@barretenberg-v0.65.0...barretenberg-v0.65.1)
(2024-11-27)


### Features

* Add total mana used to header
([#9868](AztecProtocol/aztec-packages#9868))
([2478d19](AztecProtocol/aztec-packages@2478d19))
* Configure world state block history
([#10216](AztecProtocol/aztec-packages#10216))
([01eb392](AztecProtocol/aztec-packages@01eb392))
* Speed up transaction execution
([#10172](AztecProtocol/aztec-packages#10172))
([da265b6](AztecProtocol/aztec-packages@da265b6))


### Bug Fixes

* **avm:** Execution test ordering
([#10226](AztecProtocol/aztec-packages#10226))
([49b4a6c](AztecProtocol/aztec-packages@49b4a6c))


### Miscellaneous

* **avm:** Handle parsing error
([#10203](AztecProtocol/aztec-packages#10203))
([3c623fc](AztecProtocol/aztec-packages@3c623fc)),
closes
[#9770](AztecProtocol/aztec-packages#9770)
* **avm:** Zero initialization in avm public inputs and execution test
fixes
([#10238](AztecProtocol/aztec-packages#10238))
([0c7c4c9](AztecProtocol/aztec-packages@0c7c4c9))
* CIVC VK
([#10223](AztecProtocol/aztec-packages#10223))
([089c34c](AztecProtocol/aztec-packages@089c34c))
* Pull out some sync changes
([#10245](AztecProtocol/aztec-packages#10245))
([1bfc15e](AztecProtocol/aztec-packages@1bfc15e))
</details>

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@IlyasRidhuan IlyasRidhuan IlyasRidhuan approved these changes

@fcarreiro fcarreiro fcarreiro approved these changes

@dbanks12 dbanks12 Awaiting requested review from dbanks12 dbanks12 is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Handle parsing errors gracefully by raising error flag and returning a proof
3 participants
@jeanmon @fcarreiro @IlyasRidhuan