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

instantiate_v2 with additional limit parameters #2123

Merged
merged 22 commits into from
Feb 27, 2024
Merged

Conversation

ascjones
Copy link
Collaborator

@ascjones ascjones commented Feb 21, 2024

Requires version of pallet-contracts including instantiate_v2 host function. Introduced in this commit

Following up on #2077.

There is a new host function instantiate_v2 which allows passing both Weight parts: ref_time_limit and proof_time_limit and the storage_deposit_limit. The legacy instantiate function only provides the single gas_limit parameter, which is used as the value for ref_time_limit.

The instantiate extrinsic already requires these, so this just brings the cross-contract instantiation API into line with the extrinsic.

Migration

If your target node does not yet support instantiate_v2, then you can continue to use the original v1 host function, but it requires calling the instantiate_v1 method on the builder e.g.

let create_params = build_create::<OtherContractRef>()
  .instantiate_v1()
  .code_hash(Hash::from([0x42; 32]))
  .gas_limit(500_000_000)

Otherwise if you do not change your code, it will attempt to call the new instantiate_v2 function, with no compiler warnings unless you are using gas_limit which is replaced by ref_time_limit.

@ascjones ascjones changed the title WIP instantiate_v2 instantiate_v2 cross contract instantiation with additional limit parameters Feb 26, 2024
@ascjones ascjones changed the title instantiate_v2 cross contract instantiation with additional limit parameters instantiate_v2 with additional limit parameters Feb 26, 2024
@ascjones ascjones marked this pull request as ready for review February 26, 2024 16:43
Comment on lines +572 to +576
let enc_input = scoped.take_encoded(params.exec_input());
// We support `AccountId` types with an encoding that requires up to
// 1024 bytes. Beyond that limit ink! contracts will trap for now.
// In the default configuration encoded `AccountId` require 32 bytes.
let out_address = &mut scoped.take(1024);
Copy link
Contributor

Choose a reason for hiding this comment

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

We can't make use of AccountId::max_encoded_len() here instead of hard coding 1024?

Copy link
Collaborator Author

@ascjones ascjones Feb 27, 2024

Choose a reason for hiding this comment

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

It's a good idea. We will need to:

  1. Enable max-encoded-len feature
  2. Add some bounds to the traits and impls
  3. Derive MaxEncodedLen for AccountId

It's not that much to do, but I'm inclined to make that a separate PR to this, so we can properly test it and see e.g. contract size differences if any.

This block of code I simply copied from the existing instantiate_contract, so we are confident it works well.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@ascjones ascjones enabled auto-merge (squash) February 27, 2024 09:40
@ascjones ascjones merged commit 6bc816b into master Feb 27, 2024
24 checks passed
@ascjones ascjones deleted the aj/instantiate-v2 branch February 27, 2024 11:07
Copy link

🦑 📈 ink! Example Contracts ‒ Changes Report 📉 🦑

These are the results when building the integration-tests/* contracts from this branch with cargo-contract and comparing them to ink! master:

Contract Upstream Size (kB) PR Size (kB) Diff (kB) Diff (%) Change
call-builder-return-value 9.237 9.237 0 0
call-runtime 2.061 2.061 0 0
combined-extension 2.137 2.12 -0.017 -0.795508 📉
conditional-compilation 1.49 1.49 0 0
contract-storage 7.568 7.568 0 0
contract-terminate 1.329 1.329 0 0
contract-transfer 1.689 1.689 0 0
cross-contract-calls 7.73 7.73 0 0
cross-contract-calls/other-contract 1.583 1.583 0 0
custom-allocator 7.775 7.775 0 0
custom-environment 2.146 2.146 0 0
dns 7.318 7.318 0 0
e2e-call-runtime 1.296 1.296 0 0
e2e-runtime-only-backend 1.881 1.881 0 0
erc1155 14.308 14.308 0 0
erc20 6.918 6.918 0 0
erc721 10.007 10.007 0 0
events 5.258 5.258 0 0
flipper 1.639 1.639 0 0
incrementer 1.504 1.504 0 0
lang-err-integration-tests/call-builder-delegate 2.638 2.638 0 0
lang-err-integration-tests/call-builder 5.567 5.567 0 0
lang-err-integration-tests/constructors-return-value 1.985 1.985 0 0
lang-err-integration-tests/contract-ref 5.058 5.058 0 0
lang-err-integration-tests/integration-flipper 1.815 1.815 0 0
lazyvec-integration-test 4.616 4.616 0 0
mapping-integration-tests 7.999 7.999 0 0
mother 12.741 12.741 0 0
multi-contract-caller 6.645 6.645 0 0
multi-contract-caller/accumulator 1.378 1.378 0 0
multi-contract-caller/adder 1.912 1.912 0 0
multi-contract-caller/subber 1.932 1.932 0 0
multisig 21.821 21.821 0 0
payment-channel 5.659 5.659 0 0
psp22-extension 7.071 7.071 0 0
rand-extension 2.965 2.965 0 0
sr25519-verification 1.142 1.142 0 0
static-buffer 2.536 2.536 0 0
trait-dyn-cross-contract-calls 2.887 2.887 0 0
trait-dyn-cross-contract-calls/contracts/incrementer 1.545 1.545 0 0
trait-erc20 7.294 7.294 0 0
trait-flipper 1.49 1.49 0 0
trait-incrementer 1.614 1.614 0 0
upgradeable-contracts/delegator 3.928 3.928 0 0
upgradeable-contracts/delegator/delegatee 1.609 1.609 0 0
upgradeable-contracts/delegator/delegatee2 1.609 1.609 0 0
upgradeable-contracts/set-code-hash-migration 1.743 1.743 0 0
upgradeable-contracts/set-code-hash-migration/migration 1.45 1.45 0 0
upgradeable-contracts/set-code-hash-migration/updated-incrementer 1.897 1.897 0 0
upgradeable-contracts/set-code-hash 1.743 1.743 0 0
upgradeable-contracts/set-code-hash/updated-incrementer 1.721 1.721 0 0
wildcard-selector 2.846 2.846 0 0

Link to the run | Last update: Tue Feb 27 12:30:26 CET 2024

This was referenced Feb 28, 2024
@SkymanOne SkymanOne added the Breaking change This PR of issue introduces a breaking change label Mar 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Breaking change This PR of issue introduces a breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants