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

Fix contracts integration test #679

Merged
merged 4 commits into from
Oct 3, 2022
Merged

Fix contracts integration test #679

merged 4 commits into from
Oct 3, 2022

Conversation

ascjones
Copy link
Contributor

@ascjones ascjones commented Oct 3, 2022

Prior to paritytech/substrate#12383, setting proof_size to 0 worked for all extrinsics. Following that a proper value needed to be supplied. However u64::MAX was too high as it breached the limit for normal class transactions weight (I think about 0.75 * MAX_BLOCK_WEIGHT).

Interestingly 0 still worked for instantiate_with_code and call, but the fix was to set it to u64::MAX / 2.

@lexnv
Copy link
Collaborator

lexnv commented Oct 3, 2022

I was trying the same thing, then decided to adjust the ref_time to fit the contract:

1_250_000_000_000 out of gas

1_280_000_000_000 out of gas

1_290_000_000_000 out of gas

1_295_000_000_000 out of gas

1_299_000_000_000 block limit

1_300_000_000_000 block limit

1_310_000_000_000 block limit

1_375_000_000_000 block limit

But couldn't find any appropriate values that would accept our dummy contract, which is strange.

I also waited for 2 blocks between the instantiate_with_code call and instantiate without any effect, based on the comment from substrate about exhausted transactions: Invalidity might also be temporary. In case of ExhaustsResources the transaction does not fit to the current block, but it might be okay for the next one.

@ascjones ascjones mentioned this pull request Oct 3, 2022
@ascjones
Copy link
Contributor Author

ascjones commented Oct 3, 2022

The issue appears to have been introduced in paritytech/substrate#12383

@lexnv
Copy link
Collaborator

lexnv commented Oct 3, 2022

Replicating the test using the old API resulted in the same behavior:

        use crate::node_runtime::runtime_types::sp_weights::OldWeight;

        let instantiate_tx = node_runtime::tx().contracts().instantiate_old_weight(
            100_000_000_000_000_000, // endowment
            OldWeight(
                1_295_000_000_000,
            ), // gas_limit
            None,                    // storage_deposit_limit
            code_hash,
            data,
            salt,
        );

Copy link
Collaborator

@lexnv lexnv left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this! LGTM!

@ascjones ascjones merged commit 432df58 into master Oct 3, 2022
@ascjones ascjones deleted the aj/fix-contract-tests branch October 3, 2022 17:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants