Skip to content

l1: use AnvilBaseLayer in multi-contract events test #7788

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

Conversation

giladchase
Copy link
Contributor

No description provided.

@reviewable-StarkWare
Copy link

This change is Reviewable

@giladchase giladchase marked this pull request as ready for review July 6, 2025 10:06
@giladchase giladchase force-pushed the gilad/07-03-l1_remove_out_of_scope_assertion_in_test branch from b00e27a to 88f1e71 Compare July 6, 2025 10:07
@giladchase giladchase force-pushed the gilad/07-03-l1_use_anvilbaselayer_in_multi-contract_events_test branch from 6395f25 to 2215d4e Compare July 6, 2025 10:07
@giladchase giladchase force-pushed the gilad/07-03-l1_remove_out_of_scope_assertion_in_test branch from 88f1e71 to e71f02a Compare July 28, 2025 12:57
@giladchase giladchase force-pushed the gilad/07-03-l1_use_anvilbaselayer_in_multi-contract_events_test branch from 2215d4e to 85bcb7d Compare July 28, 2025 12:57
@giladchase giladchase force-pushed the gilad/07-03-l1_remove_out_of_scope_assertion_in_test branch from e71f02a to 0860d6b Compare July 28, 2025 14:49
@giladchase giladchase force-pushed the gilad/07-03-l1_use_anvilbaselayer_in_multi-contract_events_test branch from 85bcb7d to 153c076 Compare July 28, 2025 14:49
Copy link
Contributor

@ArniStarkware ArniStarkware left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r1.
Reviewable status: all files reviewed (commit messages unreviewed), 1 unresolved discussion (waiting on @matan-starkware)


crates/papyrus_base_layer/src/base_layer_test.rs line 133 at r1 (raw file):

    // Deploy another instance of the contract to the same anvil instance.
    let other_contract = Starknet::deploy(this_contract.contract.provider().clone()).await.unwrap();

I suggest dedicating a line to the provider, otherwise it looks odd that we use "this_contract" to declare the other contract.
I wrote the original line. #sorry.

Wait - can we get the provider for anvil_base_layer - without passing through this_contract?

Suggestion:

    // Deploy another instance of the contract to the same anvil instance.
    let provider = this_contract.contract.provider().clone();
    let other_contract = Starknet::deploy(provider).await.unwrap();

@giladchase giladchase force-pushed the gilad/07-03-l1_use_anvilbaselayer_in_multi-contract_events_test branch from 153c076 to 8d90f94 Compare August 4, 2025 04:59
@giladchase giladchase force-pushed the gilad/07-03-l1_remove_out_of_scope_assertion_in_test branch 2 times, most recently from 9e547ae to f81844f Compare August 4, 2025 05:09
@giladchase giladchase force-pushed the gilad/07-03-l1_use_anvilbaselayer_in_multi-contract_events_test branch from 8d90f94 to a1bab6f Compare August 4, 2025 05:09
Copy link
Contributor Author

@giladchase giladchase left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed (commit messages unreviewed), 1 unresolved discussion (waiting on @ArniStarkware and @matan-starkware)


crates/papyrus_base_layer/src/base_layer_test.rs line 133 at r1 (raw file):
Done.

Wait - can we get the provider for anvil_base_layer - without passing through this_contract?

We'll need to call this, which needs a config and node URL, but the coupling between those and the contract are the reason why base layer abstraction exists, so trying to create a new contract by hand will duplicate it's constructor logic basically, I think.

@giladchase giladchase force-pushed the gilad/07-03-l1_use_anvilbaselayer_in_multi-contract_events_test branch from a1bab6f to 04a0c3c Compare August 4, 2025 05:50
Copy link
Contributor

@ArniStarkware ArniStarkware left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @matan-starkware)

@giladchase giladchase force-pushed the gilad/07-03-l1_remove_out_of_scope_assertion_in_test branch from f81844f to 63855a1 Compare August 4, 2025 12:30
@giladchase giladchase force-pushed the gilad/07-03-l1_use_anvilbaselayer_in_multi-contract_events_test branch 2 times, most recently from 92de0ef to f9536ef Compare August 4, 2025 12:50
@giladchase giladchase force-pushed the gilad/07-03-l1_remove_out_of_scope_assertion_in_test branch from 63855a1 to 65c5e4e Compare August 4, 2025 12:50
@giladchase giladchase force-pushed the gilad/07-03-l1_use_anvilbaselayer_in_multi-contract_events_test branch from f9536ef to b0f2c7a Compare August 4, 2025 13:51
@giladchase giladchase force-pushed the gilad/07-03-l1_remove_out_of_scope_assertion_in_test branch from 65c5e4e to 83f6ba2 Compare August 4, 2025 13:51
@giladchase giladchase force-pushed the gilad/07-03-l1_use_anvilbaselayer_in_multi-contract_events_test branch 2 times, most recently from d729f2a to f88d3af Compare August 5, 2025 08:09
@giladchase giladchase force-pushed the gilad/07-03-l1_remove_out_of_scope_assertion_in_test branch from 0803f98 to 8ad01f9 Compare August 5, 2025 08:09
@giladchase giladchase changed the base branch from gilad/07-03-l1_remove_out_of_scope_assertion_in_test to main August 5, 2025 10:36
@giladchase giladchase force-pushed the gilad/07-03-l1_use_anvilbaselayer_in_multi-contract_events_test branch from f88d3af to 6cf4085 Compare August 5, 2025 10:36
Copy link

graphite-app bot commented Aug 5, 2025

Merge activity

  • Aug 5, 10:37 AM UTC: Graphite rebased this pull request, because this pull request is set to merge when ready.

@giladchase giladchase force-pushed the gilad/07-03-l1_use_anvilbaselayer_in_multi-contract_events_test branch from 6cf4085 to 9fc8033 Compare August 5, 2025 12:05
@giladchase giladchase added this pull request to the merge queue Aug 5, 2025
Merged via the queue into main with commit fef36cd Aug 5, 2025
20 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Aug 8, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants