feat: add historical library tests and remove old inclusion proof contract / related tests #12215

sklppy88 · 2025-02-24T10:14:35Z

Adding txe tests to the historical apis, and removing inclusion proofs contract with corresponding (flaky) e2e test

sklppy88 · 2025-02-24T10:14:51Z

feat: add historical library tests and remove old inclusion proof contract / related tests #12215 👈 (View in Graphite)
fix: make block number in txe make sense #11807 : 1 other dependent PR (#11711 )
master

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

sklppy88 · 2025-02-24T11:55:33Z

noir-projects/aztec-nr/aztec/src/test/mocks/mock_note.nr

@@ -71,18 +71,24 @@ impl NoteInterface for MockNote {
 pub(crate) struct MockNoteBuilder {
    value: Field,
    contract_address: Option<AztecAddress>,
+    nonce: Option<Field>,


This was modified to allow for creating deterministic note hashes.

sklppy88 · 2025-02-24T12:08:51Z

noir-projects/aztec-nr/aztec/src/history/contract_inclusion.nr

@@ -3,7 +3,7 @@ use dep::protocol_types::{
    hash::compute_siloed_nullifier,
 };

-trait ProveContractDeployment {
+pub trait ProveContractDeployment {


These were exposed to be used in the counter-contracts due to not being able to test this in aztec-nr.

They should be exposed anyway as the history library and the whole of Aztec.nr is to be consumed from contracts.

sklppy88 · 2025-02-24T12:35:34Z

noir-projects/aztec-nr/aztec/src/history/mod.nr

-mod nullifier_inclusion;
-mod nullifier_non_inclusion;
-mod public_storage;
+pub mod contract_inclusion;


Exposing all these as well to remove warnings

sklppy88 · 2025-02-24T12:37:24Z

noir-projects/noir-contracts/contracts/test_contract/src/main.nr

+    }
+
+    #[private]
+    fn test_note_inclusion(owner: AztecAddress, storage_slot: Field) {


We are adding this here because it was the only function from the nuked inclusion proofs contract that was not present but had functionality in a used e2e test.

benesjan

Looks good. The globals made it way more readable.

Feel free to merge once you address my last comments.

noir-projects/aztec-nr/aztec/src/history/public_storage/test.nr

noir-projects/aztec-nr/aztec/src/history/test.nr

benesjan · 2025-02-27T16:25:40Z

noir-projects/aztec-nr/aztec/src/test/helpers/test_environment.nr

@@ -14,6 +14,11 @@ use crate::oracle::{
 use protocol_types::constants::PUBLIC_DISPATCH_SELECTOR;
 use protocol_types::traits::Packable;

+// This is the first nullifier emitted from the TXe. It is an arbitrary number assigned in the TXe that is
+// larger than the first prefilled subtree of the nullifier tree. We use know FIRST_NULLIFIER_EMITTED_IN_TXE exists


Suggested change

// larger than the first prefilled subtree of the nullifier tree. We use know FIRST_NULLIFIER_EMITTED_IN_TXE exists

// larger than the first prefilled subtree of the nullifier tree. We know FIRST_NULLIFIER_EMITTED_IN_TXE exists

Where does TXE emit this? I cannot find that the global variable used when emitting it. Would use it for that if possible.

It's emitted in the first block that gets automined, txe_oracle.ts:678/760. I didn't want to make a constant that's used across both ts / nr as I thought that would be a bit overkill / not protocol enshrined as a constant.

TBH I find this quite problematic as this breaks the tests if that random value injected in txe_oracle.ts changes. I think the best thing here would be to just not use it and insert our own nullifier in the tests. That shouldn't be a problem, no?

Yeah you make a good point. I've added an issue tag here to #12226 and will address in a downstack PR as soon as this is in. Will nuke this completely and not make the assumption that this nullifier will exist.

This has been addressed in #12360.

noir-projects/noir-contracts/contracts/docs_example_contract/src/main.nr

benesjan · 2025-02-27T16:31:02Z

noir-projects/noir-contracts/contracts/test_contract/src/test.nr

+    (&mut env, contract_address, owner)
+}
+
+// These tests need to be here instead of a saner place like aztec-nr/aztec/src/history/contract_inclusion/test.nr


I would add a comment to noir-projects/aztec-nr/aztec/src/history/contract_inclusion.nr that it's tested here since the tests are in a bit of a random location.

Good point, have added a comment in that location !

benesjan · 2025-02-27T16:32:53Z

noir-projects/noir-contracts/contracts/test_contract/src/test.nr

+    let (env, contract_address) = setup();
+
+    let context = env.private();
+    let header_at_block_before_deployment = context.get_block_header_at(2);


I know the header name makes it pretty clear but anyway would assign smt. like CONTRACT_DEPLOYED_AT, I would check it matches in the setup when deploying and then use the value here.

Yep you're 100% correct. Have addressed.

benesjan · 2025-02-27T16:32:59Z

noir-projects/noir-contracts/contracts/test_contract/src/test.nr

+    let (env, contract_address) = setup();
+
+    let context = env.private();
+    let header_at_block_before_deployment = context.get_block_header_at(2);


Same as above would replace the "2"

Thank you, addressed !

benesjan · 2025-02-27T16:33:03Z

noir-projects/noir-contracts/contracts/test_contract/src/test.nr

+    let (env, contract_address) = setup();
+
+    let context = env.private();
+    let header_at_block_before_deployment = context.get_block_header_at(2);


Same as above would replace the "2"

Thank you, addressed !

Co-authored-by: Jan Beneš <janbenes1234@gmail.com>

sklppy88 changed the title ~~init~~ feat: add historical library tests and remove old inclusion proof contract / related tests Feb 24, 2025

sklppy88 force-pushed the ek/feat/add-historical-tests-aztec-nr branch 25 times, most recently from e720b2a to 393d16c Compare February 24, 2025 12:43

sklppy88 commented Feb 24, 2025

View reviewed changes

sklppy88 marked this pull request as ready for review February 24, 2025 12:44

sklppy88 force-pushed the ek/feat/add-historical-tests-aztec-nr branch from 393d16c to d8b7add Compare February 24, 2025 13:03

This was referenced Feb 27, 2025

draft: contracts with no private / public functions should not fail #11743

Closed

fix: make block number in txe make sense #11807

Merged

sklppy88 force-pushed the ek/feat/add-historical-tests-aztec-nr branch 2 times, most recently from 3df202b to 2b5c42c Compare February 27, 2025 14:54

sklppy88 changed the base branch from ek/fix/make-block-number-in-txe-make-sense to graphite-base/12215 February 27, 2025 15:12

sklppy88 force-pushed the ek/feat/add-historical-tests-aztec-nr branch from 2b5c42c to 0522cb7 Compare February 27, 2025 15:12

sklppy88 force-pushed the graphite-base/12215 branch from 8966263 to d74c4a6 Compare February 27, 2025 15:12

sklppy88 changed the base branch from graphite-base/12215 to ek/fix/make-block-number-in-txe-make-sense February 27, 2025 15:12

sklppy88 changed the base branch from ek/fix/make-block-number-in-txe-make-sense to graphite-base/12215 February 27, 2025 15:35

sklppy88 requested a review from benesjan February 27, 2025 15:36

sklppy88 force-pushed the ek/feat/add-historical-tests-aztec-nr branch from 0522cb7 to 1a5b205 Compare February 27, 2025 15:58

sklppy88 force-pushed the graphite-base/12215 branch from d74c4a6 to 8181149 Compare February 27, 2025 15:58

sklppy88 changed the base branch from graphite-base/12215 to master February 27, 2025 15:58

benesjan approved these changes Feb 27, 2025

View reviewed changes

sklppy88 and others added 8 commits February 27, 2025 16:46

init

c724b3f

addressing comments

84e7350

addressing more feedback

f0be1ee

Update noir-projects/aztec-nr/aztec/src/history/public_storage/test.nr

478c074

Co-authored-by: Jan Beneš <janbenes1234@gmail.com>

Update noir-projects/aztec-nr/aztec/src/history/public_storage/test.nr

9c29d3c

Co-authored-by: Jan Beneš <janbenes1234@gmail.com>

Update noir-projects/aztec-nr/aztec/src/history/test.nr

ff619fc

Co-authored-by: Jan Beneš <janbenes1234@gmail.com>

Update noir-projects/aztec-nr/aztec/src/history/test.nr

68bd62a

Co-authored-by: Jan Beneš <janbenes1234@gmail.com>

Update noir-projects/aztec-nr/aztec/src/test/helpers/test_environment.nr

a4a8851

Co-authored-by: Jan Beneš <janbenes1234@gmail.com>

sklppy88 force-pushed the ek/feat/add-historical-tests-aztec-nr branch 2 times, most recently from 4737290 to 2113ba0 Compare February 27, 2025 17:27

addressing more comments

53e8d21

sklppy88 force-pushed the ek/feat/add-historical-tests-aztec-nr branch from 2113ba0 to 53e8d21 Compare February 27, 2025 18:14

sklppy88 merged commit a301f72 into master Feb 27, 2025
10 checks passed

sklppy88 deleted the ek/feat/add-historical-tests-aztec-nr branch February 27, 2025 18:44

This was referenced Feb 28, 2025

fix: remove FIRST_NULLIFIER_EMITTED_IN_TXE #12359

Closed

fix: remove first nullifier emitted in txe #12360

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add historical library tests and remove old inclusion proof contract / related tests #12215

feat: add historical library tests and remove old inclusion proof contract / related tests #12215

sklppy88 commented Feb 24, 2025 •

edited

Loading

sklppy88 commented Feb 24, 2025 •

edited

Loading

sklppy88 Feb 24, 2025

sklppy88 Feb 24, 2025

benesjan Feb 25, 2025

sklppy88 Feb 24, 2025

sklppy88 Feb 24, 2025

benesjan left a comment

benesjan Feb 27, 2025

sklppy88 Feb 27, 2025

benesjan Feb 27, 2025

sklppy88 Feb 27, 2025 •

edited

Loading

sklppy88 Feb 28, 2025

benesjan Feb 27, 2025

sklppy88 Feb 27, 2025

benesjan Feb 27, 2025

sklppy88 Feb 27, 2025

benesjan Feb 27, 2025

sklppy88 Feb 27, 2025

benesjan Feb 27, 2025

sklppy88 Feb 27, 2025

	// larger than the first prefilled subtree of the nullifier tree. We use know FIRST_NULLIFIER_EMITTED_IN_TXE exists
	// larger than the first prefilled subtree of the nullifier tree. We know FIRST_NULLIFIER_EMITTED_IN_TXE exists

feat: add historical library tests and remove old inclusion proof contract / related tests #12215

feat: add historical library tests and remove old inclusion proof contract / related tests #12215

Conversation

sklppy88 commented Feb 24, 2025 • edited Loading

sklppy88 commented Feb 24, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

benesjan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sklppy88 Feb 27, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sklppy88 commented Feb 24, 2025 •

edited

Loading

sklppy88 commented Feb 24, 2025 •

edited

Loading

sklppy88 Feb 27, 2025 •

edited

Loading