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

[rc8] examples/erc721 build upload/deployment fails with ContractTrapped #1108

Closed
agryaznov opened this issue Jan 22, 2022 · 10 comments
Closed

Comments

@agryaznov
Copy link
Contributor

This error appears during uploading the contract through PolkadotJS Apps, however, it does not seem to be UI related, as it is clear that DispatchError is being returned from runtime:

Module: {
    index: 8
    error: 10
  }

(which is ContractTrapped from pallet_contracts)

The bug

examples/erc721 contract taken from ink! v3.0.0-rc8 builds successfully with cargo-contract 0.17

Uploading that build through PolkadotJS Apps v0.102.2 connected to substrate-contracts-node v0.5.0 fails with ContractTrapped

@xgreenx
Copy link
Collaborator

xgreenx commented Jan 23, 2022

After the #1065, the constructor should be payable to be able to get a deposit for a contract.
image

@cmichi @athei Hmm, seems that the transferred balance will be always not zero during the instantiation of the contract. When constructor can be not payable? Maybe contract-pallet should subtract the deposit(minimum balance) from transferred value during instantiation?

@athei
Copy link
Contributor

athei commented Jan 24, 2022

With the new automatic storage deposits we don't use the the transferred value in order to have the minimum balance. What will happen is that the required deposit is automatically transferred to the contracts account and is reserved there.

This is what allows us to have non payable constructors: The minimum deposit is handled by the pallet and the transferred value is only needed (or allowed) to be non zero if the contract logic requires it (payable constructor). Essentially constructors and messages behave exactly the same there now.

@athei
Copy link
Contributor

athei commented Jan 24, 2022

Just tested with everything on master and apps v0.103.1. Deploy works for me. What did you enter as value?

@cmichi
Copy link
Collaborator

cmichi commented Jan 24, 2022

@agryaznov I also can't reproduce the error. I installed substrate-contracts-node with --locked.

@agryaznov
Copy link
Contributor Author

The value was set to 1000 during deployment, which seems to be the reason of ContractTrapped as ERC721 constructor became non-payable by default (after storage deposits introduced with #10082 PR)...

With value = 0 the contract is being deployed successfully.

I think this will be an often mistake made by users who would likely specify non-zero endowment value on contract deployment, just on habit of a way how it used to work before.

@HCastano
Copy link
Contributor

With value = 0 the contract is being deployed successfully.

I think this will be an often mistake made by users who would likely specify non-zero endowment value on contract deployment, just on habit of a way how it used to work before.

This won't be a problem in the future with polkadot-js/apps#6645. The UI won't show the value field if a constructor isn't marked explicitly as payable.

@athei
Copy link
Contributor

athei commented Jan 24, 2022

Yeah so everything working as intended. The constructor isn't payable and therefore a ContractTrapped is exactly what we want here.

@athei athei closed this as completed Jan 24, 2022
@xgreenx
Copy link
Collaborator

xgreenx commented Jan 24, 2022

When I'm submitting the transaction about deployment with value zero it throw an contracts.NewContractNotFunded.
If I set the value 1 dot, it fails with contract trapped because the constructor is not payable. How do I need to instantiate the contract and pass 1 dot as endowment?

For testing I installed the node by the next command:

cargo install contracts-node --git https://github.com/paritytech/substrate-contracts-node.git --force --locked

@athei
Copy link
Contributor

athei commented Jan 24, 2022

That can't be because this error doesn't exist anymore. So you are most likely testing against an older node than you think you are.

@xgreenx
Copy link
Collaborator

xgreenx commented Jan 24, 2022

That can't be because this error doesn't exist anymore. So you are most likely testing against an older node than you think you are.

Yep, you are right, I had two nodes at the same time. With a new one everything works

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

No branches or pull requests

5 participants