Skip to content
This repository has been archived by the owner on Aug 21, 2024. It is now read-only.

fix(transaction): change deploy_account default version to be V3 #2064

Merged

Conversation

meship-starkware
Copy link
Contributor

@meship-starkware meship-starkware commented Jul 10, 2024

This change is Reviewable

Copy link
Contributor Author

@meship-starkware meship-starkware 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: 0 of 4 files reviewed, 2 unresolved discussions


crates/blockifier/src/concurrency/versioned_state_test.rs line 234 at r1 (raw file):

    let constructor_calldata = calldata![ctor_grind_arg, ctor_storage_arg];
    let deploy_tx_args = deploy_account_tx_args! {
        class_hash,

it only worked here without the resource bounds


crates/blockifier/src/transaction/transactions_test.rs line 1399 at r1 (raw file):

    let mut nonce_manager = NonceManager::default();
    let undeclared_hash = class_hash!("0xdeadbeef");
    let deploy_account = deploy_account_tx(

It only worked here without the resource bounds.

@meship-starkware meship-starkware changed the title fix(transactions): change deploy_account default version to be V3 fix(transaction): change deploy_account default version to be V3 Jul 10, 2024
@meship-starkware meship-starkware force-pushed the meship/make_deploy_account_V3_as_deafult branch from 0263ed4 to 90e6d71 Compare July 10, 2024 07:31
@meship-starkware meship-starkware force-pushed the meship/make_deploy_account_V3_as_deafult branch from 90e6d71 to 8d17056 Compare July 10, 2024 12:44
Copy link
Collaborator

@noaov1 noaov1 left a comment

Choose a reason for hiding this comment

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

Please rebase over maon-v0.13.2

Reviewed 4 of 4 files at r2, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @avi-starkware and @meship-starkware)


crates/blockifier/src/concurrency/versioned_state_test.rs line 226 at r2 (raw file):

            version: TransactionVersion::ONE,
        },
        &mut NonceManager::default(),

Suggestion:

    let deploy_account_tx_1 = deploy_account_tx(
        deploy_account_tx_args! {
            class_hash: account_without_validation.get_class_hash(),
            resource_bounds: l1_resource_bounds(u64::from(!zero_bounds), DEFAULT_STRK_L1_GAS_PRICE),
        },
        &mut NonceManager::default(),

crates/blockifier/src/concurrency/versioned_state_test.rs line 246 at r2 (raw file):

    let deployed_account_balance_key = get_fee_token_var_address(account_address);
    let fee_token_address = chain_info.fee_token_address(&FeeType::Strk);

Can you please extract it from account_tx_2?

Code quote:

   let fee_token_address = chain_info.fee_token_address(&FeeType::Strk);

crates/blockifier/src/concurrency/versioned_state_test.rs line 261 at r2 (raw file):

        s.spawn(move || {
            let result = account_tx_2.execute(&mut state_2, &block_context_2, true, true);
            result.unwrap();

Why?

Code quote:

            let result = account_tx_2.execute(&mut state_2, &block_context_2, true, true);
            result.unwrap();

@meship-starkware meship-starkware force-pushed the meship/make_deploy_account_V3_as_deafult branch from 8d17056 to e386bd4 Compare July 10, 2024 13:05
@meship-starkware meship-starkware changed the base branch from main to main-v0.13.2 July 10, 2024 13:05
Copy link
Contributor Author

@meship-starkware meship-starkware 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: 3 of 4 files reviewed, 3 unresolved discussions (waiting on @avi-starkware and @noaov1)


crates/blockifier/src/concurrency/versioned_state_test.rs line 261 at r2 (raw file):

Previously, noaov1 (Noa Oved) wrote…

Why?

for Debug will revert

@meship-starkware meship-starkware force-pushed the meship/make_deploy_account_V3_as_deafult branch from e386bd4 to 0f3d33e Compare July 10, 2024 13:18
Copy link
Contributor Author

@meship-starkware meship-starkware 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: 1 of 4 files reviewed, 1 unresolved discussion (waiting on @avi-starkware and @noaov1)


crates/blockifier/src/concurrency/versioned_state_test.rs line 226 at r2 (raw file):

            version: TransactionVersion::ONE,
        },
        &mut NonceManager::default(),

Done.

Copy link
Collaborator

@noaov1 noaov1 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 r3, 2 of 2 files at r4, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @avi-starkware)

@meship-starkware meship-starkware merged commit f8dec81 into main-v0.13.2 Jul 10, 2024
11 checks passed
@meship-starkware meship-starkware deleted the meship/make_deploy_account_V3_as_deafult branch July 10, 2024 13:33
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.

2 participants