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

Some problems about ink-as-dependency #625

Closed
yjhmelody opened this issue Jan 6, 2021 · 12 comments
Closed

Some problems about ink-as-dependency #625

yjhmelody opened this issue Jan 6, 2021 · 12 comments
Labels
C-question Further information is requested

Comments

@yjhmelody
Copy link
Contributor

yjhmelody commented Jan 6, 2021

I find when I cargo expand the ink code, add feature ink-as-dependency will only gen #[cfg(feature = "ink-as-dependency")] version. If I remove the feature, it will only gen #[cfg(not(feature = "ink-as-dependency"))] version code.

I think the ink-as-dependency is not clear enough, for example,like delegator, it enables feature ink-as-dependency for cross calling contract.
But I think ink-as-dependency could be two semantics: call other contracts on chain or call other contract libs in compiling time.

I try to find the way to do the second thing. I remove other contracts' feature ink-as-dependency and code the following:

    #[ink(storage)]
    pub struct Delegator {
        /// Says which of adder or subber is currently in use.
        which: Which,
        /// The accumulator smart contract.
        accumulator: Lazy<Accumulator>,
    }

    impl Delegator {
        /// Instantiate a delegator with the given sub-contract codes.
        #[ink(constructor)]
        pub fn new(
            init_value: i32,
        ) -> Self {
            let total_balance = Self::env().balance();
            let accumulator = Accumulator::new(init_value);

            Self {
                which: Which::Adder,
                accumulator: Lazy::new(accumulator),
            }
        }

but I got following error:

--> ink/examples/delegator2/lib.rs:83:40
   |
83 |                 accumulator: Lazy::new(accumulator),
   |                                        ^^^^^^^^^^^ expected struct `Accumulator`, found struct `CreateBuilder`
   |
   = note: expected struct `Accumulator`
              found struct `CreateBuilder<DefaultEnvironment, Unset<ink_env::Hash>, Unset<u64>, Unset<u128>, Set<ExecutionInput<ArgumentList<ink_env::call::utils::Argument<i32>, ArgumentList<ArgumentListEnd, ArgumentListEnd>>>>, Accumulator>`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0308`.
error: could not compile `delegator`

To learn more, run the command again with --verbose.

I feel strange about this. Because I think all original codes are reserved, and storage attr impls such as StorageLayout.
accumulator: Lazy<Accumulator>'s type is correct, but the new function returned a unexpected type ?

Is ink considering this part of the design?

By the way, I think that generating two mods is better than the condition compiling.
One as an cross calling contract, and one as a combinatorial library.

@yjhmelody yjhmelody changed the title Some promblem about ink-as-dependency Some problems about ink-as-dependency Jan 6, 2021
@Robbepop
Copy link
Collaborator

Robbepop commented Jan 6, 2021

The crate feature ink-as-dependency generates code that is highly specific to using the resulting smart contract as direct dependency of another contract. That's it's only purpose. If you want to upload it to a chain you need to compile it without this flag. You never want to use both versions (ink-as-dependency and standalone compilation) at the same time for the same ink! smart contract so generating both does not make real sense to me.

I am happy about you elaborating further what your exact requirements are and why the current system is problematic to you.

@yjhmelody
Copy link
Contributor Author

Maybe it is my presentation problem. The design of conditional compilation is actually no problem. But at present, if ink-as-dependency is not added, it cannot be compiled normally as a dependent library.
I just want to resuse contract logic(standalone compilation as you say), but meet some compiling problems.

Such as:

 = note: /usr/bin/ld: 
/ink/examples/delegator2/target/debug/deps/libaccumulator.rlib(accumulator.yfmqwprpepvcyb5.rcgu.o): in function `__ink_generate_metadata':
          /ink/examples/delegator2/accumulator/lib.rs:20: multiple definition of `__ink_generate_metadata'; /ink/examples/delegator2/target/debug/deps/delegator.4ga5an339vtqm0o6.rcgu.o:/ink/examples/delegator2
/lib.rs:19: first defined here
          /usr/bin/ld: /ink/examples/delegator2/target/debug/deps/libaccumulator.rlib(accumulator.yfmqwprpepvcyb5.rcgu.o): in function `call':
          /mnt/d/work/parity/ink/examples/delegator2/accumulator/lib.rs:20: multiple definition of `call'; /ink/examples/delegator2/target/debug/deps/delegator.4ga5an339vtqm0o6.rcgu.o:/ink/examples/delegator2/lib.rs:19: first d
efined here
          /usr/bin/ld: 
/ink/examples/delegator2/target/debug/deps/libaccumulator.rlib(accumulator.yfmqwprpepvcyb5.rcgu.o): in function `deploy':
          /ink/examples/delegator2/accumulator/lib.rs:20: multiple definition of `deploy'; /ink/examples/delegator2/target/debug/deps/delegator.4ga5an339vtqm0o6.rcgu.o:/ink/examples/delegator2/lib.rs:19: first
 defined here
          collect2: error: ld returned 1 exit status

Such call or __ink_generate_metadata has no_mangle attr. So we cannot reuse these code, because of linking errors.
The problem I mentioned before is occured when use without ink-as-dependency like this problem , but now I can not reproduce that promblem.

@yjhmelody
Copy link
Contributor Author

And this name ink-as-dependency cannot distinguish between standalone compilation and cross-contract call.
But this is really not a big problem.

@yjhmelody
Copy link
Contributor Author

And without ink-as-dependency, Delegator has not deriving Debug and Clone . People who want to export the contract to be standalone compilation also need to write some conditional compilation code.

@Robbepop
Copy link
Collaborator

Robbepop commented Jan 11, 2021

And without ink-as-dependency, Delegator has not deriving Debug and Clone . People who want to export the contract to be standalone compilation also need to write some conditional compilation code.

This is true unfortunately but I do not see any way to go around this problem. ink! targets Rust developers and knowing about conditional compilation and cfg is kind of a basic knowledge there at least if limited to attributes.

@yjhmelody
Copy link
Contributor Author

Such call or __ink_generate_metadata has no_mangle attr. So we cannot reuse these code, because of linking errors.

What about the linking error?
The generated code does not handle this situation well

@Robbepop
Copy link
Collaborator

Robbepop commented Jan 12, 2021

Maybe it is my presentation problem. The design of conditional compilation is actually no problem. But at present, if ink-as-dependency is not added, it cannot be compiled normally as a dependent library.

Well, that's how it is meant to be used.

I just want to resuse contract logic(standalone compilation as you say), but meet some compiling problems.

Can you tell me what you really need this for? Debugging? Analysis?
Maybe if there is a real useful use case we could add some flags to ink! to disable metadata generation through __ink_generate_metadata.

Have you tried turning of std crate feature for all but one of the compiled contracts?

@yjhmelody
Copy link
Contributor Author

yjhmelody commented Jan 15, 2021

Can you tell me what you really need this for? Debugging? Analysis?

I want to inherit other contract just like solidity. It's composed to be only one contract rather than needed to call another contract.

We know rust have no inheritance, but I want to embed other contract`s storage to reuse them. The data structure that can be reused now is only defined by ink itself.

I set default-features = false for all other contract libs already. The problem is no_mangle attr.

If you try to write such examples, you will meet the same problems.

@yjhmelody
Copy link
Contributor Author

yjhmelody commented Jan 21, 2021

Any plan or idea about the problem?
I think the __ink_generate_metadata/deploy/call need another feature to disable/enable.

@atenjin
Copy link

atenjin commented Feb 5, 2021

@Robbepop
The basic situation is to reuse existed contracts.
For example in Solidity:
Solidity provide inheritance, thus if someone want to extend or override something in a existed ERC20 contracts (for example the implementation of ERC20 in OpenZeppin-Contracts), he just inherent the contract and add self function.

However, rust do not have inheritance, thus, if someone wanna reuse existed contracts, the best way may be combine the existed contract in self contract, for example, as a storage in contract:

    #[ink(storage)]
    pub struct Delegator {
        /// Says which of adder or subber is currently in use.
        which: Which,
        /// The accumulator smart contract.
        accumulator: Lazy<Accumulator>,
    }

and the Accumulator crate in Cargo.toml do not have ink-as-dependency feature.

In this way, is "call other contract libs in compiling time" (I think this description is not accurate)

But all in all, what we want is someone may wanna reuse some other contracts code(include storage, or message). Though I know, rust is not solidity, imitating all features in Solidity is not necessary, I think if the ink contract could reuse some other contracts in some way, is a good feature. For nobody wanna implement a ERC20 need to copy the code from other contracts and implement the ERC20 trait...For example, if I just wanna add a mint function for ERC20 example contract, currently, I could only copy the code and add the function.

@HCastano HCastano added the C-question Further information is requested label Jun 15, 2021
@HCastano
Copy link
Contributor

@yjhmelody I'm gonna close this in favour of #631 and #776 since the original question around the usage of ink-as-dependency has been answered.

@Robbepop
Copy link
Collaborator

Note that #811 actually fixes parts of this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-question Further information is requested
Projects
None yet
Development

No branches or pull requests

4 participants