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

Allow deriving storage traits for foreign types #1863

Open
athei opened this issue Jul 30, 2023 · 1 comment
Open

Allow deriving storage traits for foreign types #1863

athei opened this issue Jul 30, 2023 · 1 comment
Labels
A-ink_lang [ink_lang] Work item A-ink_storage [ink_storage] Work Item C-discussion An issue for discussion for a given topic.

Comments

@athei
Copy link
Contributor

athei commented Jul 30, 2023

In to place a type into contract storage we require it implement certain storage specific traits. Those are meant to be derived
and not implemented. This is a problem if a contract wants to store foreign types as it cannot use the derive macros on them.

After looking through or examples the affected traits seem to be:

  • scale::Decode
  • scale::Encode
  • scale_info::TypeInfo
  • ink::storage::traits::StorageLayout

The scale and scale info types are a bit more common but still parity specific and meant to be derived.

Please note that we don't know how common this case is and if we are willing to put in the work to support this for what might be a niche feature. Needs a discussion.

Suggested Solution

Presently, when deriving a type we require all contained types of a struct (or enum) to implement the type we are deriving in order to forward to them. In our case where the type is foreign we can't derive. The workaround is to newtype and manually add a forwarding impl. This is clearly not a good solution especially if the foreign type contains further foreign types. The boilerplate adds up.

To automate this we need to write a derive macro that derives the trait it is deriving on struct A for all types contained in A that don't already implement that trait. A recursive derive so to say:

//! Foreign crate

/// Doesn't implement any storage traits even though some contained types might.
#[derive(Clone, Display, Debug)]
struct StoreMe {
    all: crate::HasAllStorageTraits,
    onlyScale: crate::OnlyScaleTraits,
    nothing: crate::NoStorageTraits,
}

//! Contract

/// User will write this. Macro iterates over all contained types and emits a newtype + derive.
///
///    Specifies for which types to derive and for which embedded types an additional derive needs to be emitted.
///    This is needed for our storage types which are not implemented on the inner type.
///    This requires that the trait is actually deriveable for the type mentioned in `for`.
/// Please note that this looks so complicated because the example tries to cover all cases. With sensible
/// defaults we can probably make the common case pretty concise.
#[foreign_derive(
    [
        {traits = [StorageLayout, TypeInfo], for = [foreign::NoStorageTraits, foreign::OnlyScaleTraits]},
        {traits = [Encode, Decode], for = [foreign::NoStorageTraits]},
    ],
)
#[derive(Clone, Display, Debug)]
struct StoreMe(foreign::StoreMe);


// Macro will replace this with

/// Normal derive works because we emit derives for the contained types.
#[derive(StorageLayout, TypeInfo, Encode, Decode)]
struct StoreMe(foreign::StoreMe);

impl Deref for StoreMe {
    Target = foreign::StoreMe;
    ...
}

impl DerefMut for StoreMe {
    Target = foreign::StoreMe;
    ...
}

impl From<foreign::StoreMe> for StoreMe { .... }

impl From<StoreMe> for foreign::StoreMe { .... }

/// Without arguments just generate basic Deref and Into impls.
#[foreign_derive]
#[derive(StorageLayout, TypeInfo)]
struct OnlyScaleTraits(foreign::OnlyScaleTraits)
#[foreign_derive]
#[derive(StorageLayout, TypeInfo, Encode, Decode)]
struct NoStorageTraits(foreign::NoStorageTraits)
@xgreenx
Copy link
Collaborator

xgreenx commented Jul 30, 2023

The derive macro usually iterates over each field of the structure and recursively calls the corresponding method, assuming that fields implement all required traits.

But in the case of OnlyScaleTraits and NoStorageTraits, it is not valid. Deriving scale::Encode will throw an error that the foreign::NoStorageTraits doesn't implement this trait. Usually, you implement the trait manually by accessing the inner fields of the foreign::NoStorageTraits type. But the macro can't get information about inner fields, so it is impossible to automate=(

@SkymanOne SkymanOne added A-ink_storage [ink_storage] Work Item C-discussion An issue for discussion for a given topic. A-ink_lang [ink_lang] Work item labels Aug 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ink_lang [ink_lang] Work item A-ink_storage [ink_storage] Work Item C-discussion An issue for discussion for a given topic.
Projects
None yet
Development

No branches or pull requests

3 participants