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

Remove ink-as-dependency #1163

Closed
wants to merge 9 commits into from
Closed

Conversation

HCastano
Copy link
Contributor

@HCastano HCastano commented Mar 3, 2022

As part of the ink! RC7 release the ink! codegen started generating a ContractRef
struct alongside the Contract struct. Since then there has been no need for
ink-as-dependency because all contracts get a generated Ref struct which can be used
anywhere ink-as-dependency would've been used.

The release notes even mention that the Ref struct was meant to be the successor to the
ink-as-dependency concept.

MyContractRef is pretty much the same of what we had gotten with the old
ink-as-dependency. It is a typed thin-wrapper around an AccountId that is mirroring the
ink! smart contract's API and implemented traits.

This PR removes ink-as-dependency since it is no longer needed.

@HCastano
Copy link
Contributor Author

HCastano commented Mar 3, 2022

I'll fix the CI tomorrow 😔

@xgreenx
Copy link
Collaborator

xgreenx commented Mar 3, 2022

You can't just remove the ink-as-dependency feature because it is still used to prevent multiple call, deploy, and __ink_generate_metadata.

@Robbepop mentioned in that #665 the idea with in-as-root feature. All no mangle methods can be marked with that feature and cargo-contract can enable it during compilation.

@HCastano
Copy link
Contributor Author

HCastano commented Mar 4, 2022

You can't just remove the ink-as-dependency feature because it is still used to prevent multiple call, deploy, and __ink_generate_metadata.

@Robbepop mentioned in that #665 the idea with in-as-root feature. All no mangle methods can be marked with that feature and cargo-contract can enable it during compilation.

Hmm yeah, I figured the ContractRef impl was a bit more clever, but you're right we
still need some way to prevent multiple of those no_mangle calls.

So the main thing I really wanted to get rid of here was the
#[ink::contract(compile_as_dependency = ...)] syntax from the language.

I think we can still do that without removing the ink-as-dependency feature flag. I
don't really see a difference between that and ink-as-root, the feature will still need
to be enabled/disabled somewhere.

@xgreenx
Copy link
Collaborator

xgreenx commented Mar 4, 2022

I think we can still do that without removing the ink-as-dependency feature flag. I
don't really see a difference between that and ink-as-root, the feature will still need
to be enabled/disabled somewhere.

ink-as-root should be enabled only for the root contract, not for subcontracts, and cargo-contract can enable it because the developer calls cargo contract build for the root contract.

ink-as-dependency should be enabled for all subcontracts, and cargo-contract can't recognize what contract is sub contract=)

@HCastano
Copy link
Contributor Author

HCastano commented Mar 8, 2022

Okay so I'm gonna split this out into two PRs.

  1. Remove compile_as_dependency
  2. Replace ink-as-dependency with ink-as-root

For (1) I've opened #1168, and for (2) I've started playing around with something today, will
maybe open a PR tomorrow.

@HCastano HCastano closed this Mar 8, 2022
@HCastano HCastano deleted the hc-remove-ink-as-dependency branch July 29, 2022 17:01
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.

2 participants