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 fallback message and constructor handlers #739

Closed
Robbepop opened this issue Mar 19, 2021 · 6 comments · Fixed by #1020
Closed

Allow fallback message and constructor handlers #739

Robbepop opened this issue Mar 19, 2021 · 6 comments · Fixed by #1020
Assignees
Labels
B-design Designing a new component, interface or functionality. C-discussion An issue for discussion for a given topic.

Comments

@Robbepop
Copy link
Collaborator

Robbepop commented Mar 19, 2021

Towards a future with upgradeable ink! smart contracts.

Motivation & Related Work

In order to create an upgradeable contract on Ethereum contract authors usually have the following architecture of 2 or 3 different contracts interacting with each other:

  1. Forwarder: This contract receives all messages from outside and simply forwards them blindly to the contract at the address A which it stores on its own contract storage. The contract at address A normally is the logic layer of the set of contracts where all the interesting things are happening. It is also the part that is usually upgraded. Upgrading in this scheme relinks the address A from the Forwarder contract to a new version of the logic layer contract. If there is a Data contract the Forwarder contract also needs to store its address so that it can notify the Data layer about an upgrade for the Logic layer. This is required so that the Data layer can protect accesses from outside.
  2. Logic: This contract contains all the logic of the contract. It has several constructors and messages to expose its API to the outside world. Interactions should only ever happen through the harded Forwarder contract and the logic layer necessarily needs to protect itself from accesses besides that one. Optionally a contract author can decide to put their data into yet another data contract. If this is the case the Logic contract needs to invoke messages of the Data contract in order to interact with any persisting storage data and it will have absolutely no storage to itself. When upgrading a contract only the Logic layer is going to be swapped out while the Forwarder and Data layers are kept. The Forwarder simply needs to relink to the new Logic layer contract address though and the new Logic contract needs to keep the original links to the old Data contract so that all storage is kept as is.
  3. Data: The data layer is only concerned about storing and loading persisted storage data required by the logic layer. It should offer a proper API for the Logic layer contract and must protect itself from accesses other than the Logic layer contract or the Forwarder contract. Upgrading a contract should never swap out the Data layer contract since this would be an extraordinarily costly operation to do. Keeping and persisting the state between upgrades is critical.

Upgradeability in ink!

The above scheme is also applicable to ink! smart contract. The described Logic and Data layers work pretty much the same as described above. However, for the Forwarder layer ink! is missing a feature to allow for wildcard message or constructor selectors. Meaning that ink! always requires the contract author to define all valid messages and constructors up-front and upon contract execution all other messages and constructors (with non-matching selectors) will be declined. Therefore with the current state of ink! the Forwarder contract is required to mirror the API exposed by the Logic layer contract. Also this solution does not allow for changing the API of the Logic layer, e.g. adding new messages or constructors to it or adding optional arguments to existing messages or constructors because this would necessitate adjustments in the Forwarder contract. However, the Forwarder layer is immutable entirely. So adjusting the Forwader layer in this architecture is impossible. Therefore changing the API of the Logic layer contract is impossible.

However, with respect to contract upgradeability adjusting the Logic layer API is going to be very important in many cases. Therefore ink! needs to provide a way to allow for this scenario. This is best done by allowing ink! contracts to accept wildcard selectors. ink! did not support this feature so far since the exact same feature can be problematic as it has been designed in Ethereum.

Design

Syntactically we will model the new behavior as always through attribute proc. macros.
Since we already have an #[ink(selector = N)] attribute proc. macro working with ink! messages and constructors it is the natural candidate to allow a contract to accept wildcard selectors. This could be done with a syntax like #[ink(selector = _)]. There can only ever be exactly one #[ink(selector = _)] ink! message and constructor respectively defined for the same contract. This being an opt-in feature will guard us against problems that can happen in Ethereum smart contracts and equips this feature with the specific purpose for being used in Forwarder smart contracts.

The signature of an ink! message with the #[ink(selector = _)] attribute will be as follows:

#[ink(selector = _)]
fn my_message(&[mut] self, selector: [u8; 4]);

However, the above signature is missing out on 2 things:

  1. How to retrieve the input that is going to be forwarded to the Logic layer?
  2. How to return the output from the Logic layer back to the caller of the Forward layer?
  3. How to define APIs to relink the Logic and Data layer contracts?

Solution for problem 1

For problem number 1. we could add another parameter to the signature:

#[ink(selector = _)]
fn my_message(&[mut] self, selector: [u8; 4], input: Vec<u8>);

Note that input needs to be of type Vec<u8> since it is not known at this point what the arguments are encoded in, nor how much bytes are to be expected. The problem with this solution is that it requires heap allocation as soon as there is a single byte in the input which can cause some unnecessary Wasm binary bloat and maybe even some inefficiencies that could be avoided. So far the ink! implementation tries its best to avoid heap allocations where possible.

Another solution is to not introduce the input argument to the signature and instead provide a low-level API to forward the input to some address, e.g. ink_env::forward_input_to_address(address: Address) -> !. The problem with this API is that it must be called before any interaction with SEAL is happening. Also it ideally never returns which brings me to the 2. point from above.

Solution for problem 2

If the Forwarder contract successfully calls the Logic layer then how do we handle the return value from the Logic layer? It somehow must be propagated back to the caller of the Forwarder contract. Ideally the return value would not be required to walk back through the Forwarder contract since that would just cause unnecessary overhead. Instead SEAL would ideally provide to call a contract in tail position so that SEAL would relink the caller to the caller of the tail call caller with the same effects that a tail call in a real computation would have. @athei is this possible?

Solution to problem 3

The wildcard annotated ink! message will only ever be called if the other defined messages with their particular selectors do not match the incoming selector. This kind of shadows some portions of wildcard selectors but should not be a major problem all in all. This way it is possible to define messages such as fn upgrade_to(logic: Address) in order to relink both Forwarder layer and Data layer contracts in order to use the new Logic layer contract.

@athei
Copy link
Contributor

athei commented Mar 19, 2021

Yes this would be totally doable. We would need to add a new version of seal_call though. I can imagine a new seal_call that adds a flag parameter for future extensibility. The initially supported flags could be:

FORWARD_INPUT - Forward the input of current functions to the callee. Panics if input was already read.
TAILCALL - Do not return from the call but rather return the result of the callee to the callers caller.
DENY_REENTRANCY - Panic if the callee (the contract) is already on the call stack.

@Robbepop
Copy link
Collaborator Author

Robbepop commented Mar 19, 2021

Yes this would be totally doable. We would need to add a new version of seal_call though. I can imagine a new seal_call that adds a flag parameter for future extensibility. The initially supported flags could be:

FORWARD_INPUT - Forward the input of current functions to the callee. Panics if input was already read.
TAILCALL - Do not return from the call but rather return the result of the callee to the callers caller.
DENY_REENTRANCY - Panic if the callee (the contract) is already on the call stack.

This is even better than what I have had in mind!
I especially love the DENY_REENTRANCY flag since it prevents yet another issue that we had on our radar.

If ink! settles on using the new seal_call API I wonder though what would be a sane default for DENY_REENTRANCY. 🤔 Also I would maybe rename DENY_REENTRANCY to ALLOW_REENTRANCY to not invent yet another negative config.

@athei
Copy link
Contributor

athei commented Mar 19, 2021

Reentrancy should be denied by default.

@cmichi
Copy link
Collaborator

cmichi commented Mar 19, 2021

A question is also how we would handle the ABI. For a frictionless UX it should be possible to use either

  • the metadata from the logic contract when interacting with the forwarder contract
  • as well as the metadata from the forwarder contract when trying to re-link forwarder and logic.

But then cargo-contract would need to have an option for generating forwarder.contract with the metadata of the logic contract instead.

Also we should then detect if the selectors of forwarder overlap with logic and warn about it.

@Robbepop
Copy link
Collaborator Author

Also we should then detect if the selectors of forwarder overlap with logic and warn about it.

This is a good idea and should ideally be implemented somewhere in the cargo-contract CLI tool.

I have not yet made extensive thoughts about the metadata problems but you are right in that we ideally provide a merged concept of contract metadata. Maybe this could also be implemented in the Canvas UI so that it is possible to append messages and constructor metadata to some live contract in order to simplify calling it through UI. Not sure what is going to be the best solution. We have to make sure that metadata is kept separate though so that updating the Logic layer does not invalidate the Forwarder metadata in any way.

@xgreenx
Copy link
Collaborator

xgreenx commented May 31, 2021

I want to add some clarification for point 1(Forwarder).
In the case of Solidity, the Proxy contract is forwarding input arguments to the delegate call function.

This function takes the code of contract A and executes it in the context of the forwarder contract. It means that we apply modifications to the storage of the forwarder but using the logic from contract A.

We need only implement delegate call on pallet level, default function and we will be able to have upgradable contracts.

Also, we need to define rules for an upgradable contract like it has been done here.

These rules are based on the realization of storage data.
In the case of Solidity OpenZeppein suggest having some gap slots. In the case of the substrate, we can try to handle it during decode/encode.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B-design Designing a new component, interface or functionality. C-discussion An issue for discussion for a given topic.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants