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

refactor(mempool_test_utils): add non mut account_with_id, rename previous implementation #2801

Merged
merged 1 commit into from
Dec 30, 2024

Conversation

yair-starkware
Copy link
Contributor

No description provided.

@reviewable-StarkWare
Copy link

This change is Reviewable

Copy link

graphite-app bot commented Dec 19, 2024

Graphite Automations

"Yair - Auto-assign" took an action on this PR • (12/19/24)

1 assignee was added to this PR based on Yair's automation.

@yair-starkware yair-starkware force-pushed the yair/changing_test_scenarios branch from e02ed63 to 861c70f Compare December 19, 2024 09:26
@yair-starkware yair-starkware force-pushed the yair/refactor_account_with_id branch from da0ded7 to 7322a80 Compare December 19, 2024 09:26
@yair-starkware yair-starkware force-pushed the yair/changing_test_scenarios branch from 861c70f to 85383c4 Compare December 19, 2024 10:21
@yair-starkware yair-starkware force-pushed the yair/refactor_account_with_id branch from 7322a80 to a51b3af Compare December 19, 2024 10:21
@yair-starkware yair-starkware force-pushed the yair/changing_test_scenarios branch from 85383c4 to 471426d Compare December 19, 2024 10:23
@yair-starkware yair-starkware force-pushed the yair/refactor_account_with_id branch from a51b3af to c1f3c9c Compare December 19, 2024 10:23
@yair-starkware yair-starkware force-pushed the yair/changing_test_scenarios branch from 471426d to 10faaa6 Compare December 19, 2024 10:50
@yair-starkware yair-starkware force-pushed the yair/refactor_account_with_id branch from c1f3c9c to 0aaaacb Compare December 19, 2024 10:50
@yair-starkware yair-starkware force-pushed the yair/changing_test_scenarios branch from 10faaa6 to 9eca0c3 Compare December 22, 2024 12:14
@yair-starkware yair-starkware force-pushed the yair/refactor_account_with_id branch from 0aaaacb to 3379aa0 Compare December 26, 2024 13:10
@yair-starkware yair-starkware changed the base branch from yair/changing_test_scenarios to main December 26, 2024 13:10
Copy link

Artifacts upload workflows:

@yair-starkware yair-starkware force-pushed the yair/refactor_account_with_id branch from 3379aa0 to 56942a2 Compare December 26, 2024 13:57
Copy link
Collaborator

@alonh5 alonh5 left a comment

Choose a reason for hiding this comment

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

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


crates/mempool_test_utils/src/starknet_api_test_utils.rs line 209 at r1 (raw file):

    }

    pub fn account_with_id(&self, account_id: AccountId) -> &AccountTransactionGenerator {

Why do you need both?

@yair-starkware
Copy link
Contributor Author

crates/mempool_test_utils/src/starknet_api_test_utils.rs line 209 at r1 (raw file):

Previously, alonh5 (Alon Haramati) wrote…

Why do you need both?

We need to hold the funding account and the account to be deployed simultaneously. It is not possible if they require a mutable ref of self

Copy link
Collaborator

@alonh5 alonh5 left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @yair-starkware)


crates/mempool_test_utils/src/starknet_api_test_utils.rs line 209 at r1 (raw file):

Previously, yair-starkware (Yair) wrote…

We need to hold the funding account and the account to be deployed simultaneously. It is not possible if they require a mutable ref of self

Got it, thanks!

@yair-starkware yair-starkware added this pull request to the merge queue Dec 30, 2024
Merged via the queue into main with commit 0055360 Dec 30, 2024
8 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Jan 1, 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