Skip to content

chore: check artifact consistency#10271

Merged
LHerskind merged 2 commits intomasterfrom
lh/artifact-hash-consistency
Nov 28, 2024
Merged

chore: check artifact consistency#10271
LHerskind merged 2 commits intomasterfrom
lh/artifact-hash-consistency

Conversation

@LHerskind
Copy link
Contributor

@LHerskind LHerskind commented Nov 28, 2024 •
edited
Loading

When noir was updated it could lead to the artifact-hash changing which would break a few tests checking against a snapshot.

This pr alters the check such that it is instead checking that multiple computations agree with each other, but no longer checking that it match the checkpoint.

@LHerskind Graphite App
Copy link
Contributor Author

LHerskind commented Nov 28, 2024 •
edited
Loading

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

@LHerskind LHerskind marked this pull request as ready for review November 28, 2024 12:52
@spalladino
chore: Fix bad merge in integration l1 publisher
e0f8cd2 
Fixes build after a bad merge from #10163.
@LHerskind LHerskind changed the base branch from master to palla/fix-l1-publisher-bad-merge November 28, 2024 13:05
@LHerskind LHerskind force-pushed the lh/artifact-hash-consistency branch from eab0f8a to 3a17f1e Compare November 28, 2024 13:05
sklppy88
sklppy88 approved these changes Nov 28, 2024
View reviewed changes
Copy link
Contributor

@sklppy88 sklppy88 left a comment

Choose a reason for hiding this comment

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

LGTM

@spalladino
Copy link
Contributor

spalladino commented Nov 28, 2024

I'm not sure if the calculates the test contract artifact hash multiple times to ensure deterministic hashing test would do what we expect, since I'd expect the operation could be deterministic when run under the same conditions (eg this test) but not necessarily when run eg on a different platform. But I'm really in favor of killing the snapshot, so go ahead!

Base automatically changed from palla/fix-l1-publisher-bad-merge to master November 28, 2024 13:17
@LHerskind
Copy link
Contributor Author

LHerskind commented Nov 28, 2024

I'm not sure if the calculates the test contract artifact hash multiple times to ensure deterministic hashing test would do what we expect, since I'd expect the operation could be deterministic when run under the same conditions (eg this test) but not necessarily when run eg on a different platform. But I'm really in favor of killing the snapshot, so go ahead!

Agree, but think the current will at least catch a subset of them, so getting the snapshot away not seems worthwhile 😆

@LHerskind LHerskind merged commit 6a49405 into master Nov 28, 2024
@LHerskind LHerskind deleted the lh/artifact-hash-consistency branch November 28, 2024 14:10
@AztecBot AztecBot mentioned this pull request Nov 28, 2024
Merged
ludamad pushed a commit that referenced this pull request Nov 28, 2024
@AztecBot
chore(master): Release 0.65.2 (#10258)
10754db 
🤖 I have created a release *beep* *boop*
---


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

##
[0.65.2](aztec-package-v0.65.1...aztec-package-v0.65.2)
(2024-11-28)


### Features

* New proving broker
([#10174](#10174))
([6fd5fc1](6fd5fc1))
</details>

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

##
[0.65.2](barretenberg.js-v0.65.1...barretenberg.js-v0.65.2)
(2024-11-28)


### Miscellaneous

* **barretenberg.js:** Synchronize aztec-packages versions
</details>

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

##
[0.65.2](aztec-packages-v0.65.1...aztec-packages-v0.65.2)
(2024-11-28)


### Features

* Fee foresight support
([#10262](#10262))
([9e19244](9e19244))
* New proving broker
([#10174](#10174))
([6fd5fc1](6fd5fc1))
* Sequential insertion in indexed trees
([#10111](#10111))
([bfd9fa6](bfd9fa6))
* Swap polys to facilitate dynamic trace overflow
([#9976](#9976))
([b7b282c](b7b282c))


### Bug Fixes

* Don't store indices of zero leaves.
([#10270](#10270))
([c22be8b](c22be8b))
* Expect proper duplicate nullifier error patterns in e2e tests
([#10256](#10256))
([4ee8344](4ee8344))


### Miscellaneous

* Check artifact consistency
([#10271](#10271))
([6a49405](6a49405))
* Dont import things that themselves import jest in imported functions
([#10260](#10260))
([9440c1c](9440c1c))
* Fix bad merge in integration l1 publisher
([#10272](#10272))
([b5a6aa4](b5a6aa4))
* Fixing sol warnings
([#10276](#10276))
([3d113b2](3d113b2))
* Pull out sync changes
([#10274](#10274))
([391a6b7](391a6b7))
* Pull value merger code from sync
([#10080](#10080))
([3392629](3392629))
* Remove default gas settings
([#10163](#10163))
([c9a4d88](c9a4d88))
* Replace relative paths to noir-protocol-circuits
([654d801](654d801))
* Teardown context in prover coordination test
([#10257](#10257))
([7ea3888](7ea3888))
</details>

<details><summary>barretenberg: 0.65.2</summary>

##
[0.65.2](barretenberg-v0.65.1...barretenberg-v0.65.2)
(2024-11-28)


### Features

* Sequential insertion in indexed trees
([#10111](#10111))
([bfd9fa6](bfd9fa6))
* Swap polys to facilitate dynamic trace overflow
([#9976](#9976))
([b7b282c](b7b282c))


### Bug Fixes

* Don't store indices of zero leaves.
([#10270](#10270))
([c22be8b](c22be8b))


### Miscellaneous

* Pull value merger code from sync
([#10080](#10080))
([3392629](3392629))
</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 29, 2024
@AztecBot
chore(master): Release 0.65.2 (#10258)
bef9e48 
🤖 I have created a release *beep* *boop*
---


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

##
[0.65.2](AztecProtocol/aztec-packages@aztec-package-v0.65.1...aztec-package-v0.65.2)
(2024-11-28)


### Features

* New proving broker
([#10174](AztecProtocol/aztec-packages#10174))
([6fd5fc1](AztecProtocol/aztec-packages@6fd5fc1))
</details>

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

##
[0.65.2](AztecProtocol/aztec-packages@barretenberg.js-v0.65.1...barretenberg.js-v0.65.2)
(2024-11-28)


### Miscellaneous

* **barretenberg.js:** Synchronize aztec-packages versions
</details>

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

##
[0.65.2](AztecProtocol/aztec-packages@aztec-packages-v0.65.1...aztec-packages-v0.65.2)
(2024-11-28)


### Features

* Fee foresight support
([#10262](AztecProtocol/aztec-packages#10262))
([9e19244](AztecProtocol/aztec-packages@9e19244))
* New proving broker
([#10174](AztecProtocol/aztec-packages#10174))
([6fd5fc1](AztecProtocol/aztec-packages@6fd5fc1))
* Sequential insertion in indexed trees
([#10111](AztecProtocol/aztec-packages#10111))
([bfd9fa6](AztecProtocol/aztec-packages@bfd9fa6))
* Swap polys to facilitate dynamic trace overflow
([#9976](AztecProtocol/aztec-packages#9976))
([b7b282c](AztecProtocol/aztec-packages@b7b282c))


### Bug Fixes

* Don't store indices of zero leaves.
([#10270](AztecProtocol/aztec-packages#10270))
([c22be8b](AztecProtocol/aztec-packages@c22be8b))
* Expect proper duplicate nullifier error patterns in e2e tests
([#10256](AztecProtocol/aztec-packages#10256))
([4ee8344](AztecProtocol/aztec-packages@4ee8344))


### Miscellaneous

* Check artifact consistency
([#10271](AztecProtocol/aztec-packages#10271))
([6a49405](AztecProtocol/aztec-packages@6a49405))
* Dont import things that themselves import jest in imported functions
([#10260](AztecProtocol/aztec-packages#10260))
([9440c1c](AztecProtocol/aztec-packages@9440c1c))
* Fix bad merge in integration l1 publisher
([#10272](AztecProtocol/aztec-packages#10272))
([b5a6aa4](AztecProtocol/aztec-packages@b5a6aa4))
* Fixing sol warnings
([#10276](AztecProtocol/aztec-packages#10276))
([3d113b2](AztecProtocol/aztec-packages@3d113b2))
* Pull out sync changes
([#10274](AztecProtocol/aztec-packages#10274))
([391a6b7](AztecProtocol/aztec-packages@391a6b7))
* Pull value merger code from sync
([#10080](AztecProtocol/aztec-packages#10080))
([3392629](AztecProtocol/aztec-packages@3392629))
* Remove default gas settings
([#10163](AztecProtocol/aztec-packages#10163))
([c9a4d88](AztecProtocol/aztec-packages@c9a4d88))
* Replace relative paths to noir-protocol-circuits
([654d801](AztecProtocol/aztec-packages@654d801))
* Teardown context in prover coordination test
([#10257](AztecProtocol/aztec-packages#10257))
([7ea3888](AztecProtocol/aztec-packages@7ea3888))
</details>

<details><summary>barretenberg: 0.65.2</summary>

##
[0.65.2](AztecProtocol/aztec-packages@barretenberg-v0.65.1...barretenberg-v0.65.2)
(2024-11-28)


### Features

* Sequential insertion in indexed trees
([#10111](AztecProtocol/aztec-packages#10111))
([bfd9fa6](AztecProtocol/aztec-packages@bfd9fa6))
* Swap polys to facilitate dynamic trace overflow
([#9976](AztecProtocol/aztec-packages#9976))
([b7b282c](AztecProtocol/aztec-packages@b7b282c))


### Bug Fixes

* Don't store indices of zero leaves.
([#10270](AztecProtocol/aztec-packages#10270))
([c22be8b](AztecProtocol/aztec-packages@c22be8b))


### Miscellaneous

* Pull value merger code from sync
([#10080](AztecProtocol/aztec-packages#10080))
([3392629](AztecProtocol/aztec-packages@3392629))
</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

@sklppy88 sklppy88 sklppy88 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@LHerskind @spalladino @sklppy88