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

zkApp composability RFC #303

Closed
mitschabaude opened this issue Jul 25, 2022 · 15 comments
Closed

zkApp composability RFC #303

mitschabaude opened this issue Jul 25, 2022 · 15 comments
Assignees
Labels
rfc Issues that contain some important documentation

Comments

@mitschabaude
Copy link
Member

mitschabaude commented Jul 25, 2022

zkApp composability RFC

"zkApp composability" refers to the ability to call zkApp methods from other zkApp methods. Technically, it uses the callData field on the zkApp party to connect the result of the called zkApp to the circuit/proof of the caller zkApp.

An initial spec by @mrmr1993 laying out the ideas for how to use callData can be found here: https://o1-labs.github.io/snapps-txns-reference-impl/target/doc/snapps_txn_reference_impl/call_data/index.html

We propose the following API for snarkyjs:

API

@method myMethod() {
  let calledContract = new CalledContract(address);
  let result = calledContract.calledMethod(arg);
  // .. do anything with result
}

That is, we can just call another method inside a smart contract method. That's the API 😃

NEW: To enable returning a result to another zkApp, the result type must be annotated with a TS type:

class CalledContract extends SmartContract {
  @method calledMethod(arg: UInt64): Bool { // <-- HERE
     // ...
  }
}

The return type annotation is captured by the decorator (like the argument types), and supports all circuit values.

Since it is easy to miss adding this, we take care to catch the case that

  • a method is called by another method, and returns something else than undefined
  • BUT no return type annotation exists

In this case, the following error is thrown:

Error: To return a result from calledMethod() inside another zkApp, you need to declare the return type.
This can be done by annotating the type at the end of the function signature. For example:

@method calledMethod(): Field {
  // ...
}

Note: Only types built out of `Field` are valid return types. This includes snarkyjs primitive types and custom CircuitValues.

zkApp calls can be arbitrarily nested! So, the calledMethod above could itself call another method.
(In theory, a method could even call itself! But we would need logical branching to make that work -- right now, it can't conditionally call itself, so calling itself would cause infinite recursion.)

How it works under the hood

At a high level, calling a zkApp does not mean that the called method is executed inside the caller's circuit. Instead, the callee should have its own party, with its own execution proof for the method that was called. This means that we have to be all the more careful to constrain the callee party in the caller's circuit, in a way that we fully prove the intended statement: "I called this method, on this zkApp, with these inputs, and got this result".

To make the caller prove that, we do the following:

  • In the callee circuit, we compute the following hash, and store the result in the callee's callData:
this.body.callData = Poseidon.hash([...inputs, ...outputs, methodIndex, blindingValue]);

--> inputs are the arguments of the method call, represented as an array of field elements
--> outputs is the return value of the method call, represented as an array of field elements
--> methodIndex identifies the method that is called, by an index starting with 0, among all the methods available on the called smart contract (the index is represented as a full Field)
--> blindingValue is a random Field that is made accessible to both the caller and callee circuits at proving time (it has the same value in both proofs!). In the circuit, blindingValue is represented as a witness with no further constraints.

  • In the caller circuit, we witness the party of the callee, plus the result of hashing the callee's own children (= the calls field of the callee's public input), plus the return value of the called method. Then, in the caller circuit we perform the same hash as before and compare it to the callee's callData:
// ... witness `callee` and `outputs` ...
let callData = Poseidon.hash([...inputs, ...outputs, methodIndex, blindingValue]);
callee.body.callData.assertEquals(callData);

--> this check proves that we called a method with a particular index, with particular inputs and output. To also prove that we call a particular zkApp, we have to add more checks (see next bullet point)
--> the blindingValue is needed to keep the inputs and outputs private. Note that the callData is sent to the network as part of the transaction, so it is public. If it would contain a hash of just the inputs and outputs, those could potentially be guessed and the guess checked against the hash, ruining user privacy. Since guessing the blindingValue is not possible, our approach keeps user inputs private.

  • In the caller's circuit, we also assert that the publicKey and tokenId on the callee are the ones provided by the user:
callee.body.publicKey.assertEquals(calleeInstance.self.body.publicKey);
callee.body.tokenId.assertEquals(calleeInstance.self.body.tokenId);

--> these checks make sure that the child party in the transaction, which belongs to the callee, has to refer to the same account which is defined by publicKey and tokenId. Thus, we're proving that we're calling a particular zkApp.

And that's all we do under the hood when you call another zkApp!

An important thing to note is that we add stuff to the callee circuit, although the called method didn't specify that it can be called. To make that work, we in fact add that stuff to every zkApp method. In particular, the callData field will always be populated, regardless of whether a zkApp is called or not.

Making all zkApps callable is a deliberate design decision. The alternative would be to add a special annotation, for example @callableMethod instead of @method, to tell snarkyjs that this method should add the callData computation to its circuit. That way, we would save a few constraints in methods that aren't callable. However, many zkApp developers would probably not consider the requirement of a special annotation at first, or overestimate the performance impact of adding it (it is really small, just 20-odd constraints for the extra Poseidon hash). To make the zkApp callable later, it would have to be re-deployed later. Also, we would have to ensure that a non-callable zkApp aren't called by accident, because that would leave the call inputs and output completely unconstrained, making the caller's proof spoof-able.

In my opinion, it's much better to spend a few constraints to make composability the default!

EDIT: Another decision I made is to not expose the callee's child parties to the caller's circuit. Instead, I just witness the relevant hash, leaving it up to the callee to do any checks on its children. The rough idea is that when you do a function call, you usually don't want to / shouldn't have to "look inside" that function and inspect inputs and outputs of nested function calls. I'm curious if there are considerations that I missed!

@jasongitmail
Copy link
Contributor

@method myMethod() {
  let calledContract = new CalledContract(address);
  let result = calledContract.calledMethod(arg);
  // .. do anything with result
}

That is, we can just call another method inside a smart contract method. That's the API 😃

Nice :)

--> methodIndex identifies the method that is called, by an index starting with 0, among all the methods available on the called smart contract (the index is represented as a full Field)

Do we think methodIndex is sufficient or should it be a methodDigest? If a digest, could it be computed at build time to have no performance implications anyway? I assume it could be

In my opinion, it's much better to spend a few constraints to make composability the default!

+1 Reasonable first step and probably the right long term choice too

@MartinMinkov
Copy link
Contributor

Awesome work! A few quick clarifying questions for me :D

At a high level, calling a zkApp does not mean that the called method is executed inside the caller's circuit. Instead, the callee should have its own party, with its own execution proof for the method that was called.

How does one construct the party for the callee? Will users have to construct this value before calling into a method or will it be auto-generated in some way?

is check proves that we called a method with a particular index, with particular inputs and output. To also prove that we call a particular zkApp, we have to add more checks (see next bullet point)

Just wanted to confirm my understanding that methods on a zkApp are stored in some sort of known order for methodIndex to be used. For example, if I'm calling into a zkApp for a certain method, I would just reference it by the order in which all the methods are declared?

The rough idea is that when you do a function call, you usually don't want to / shouldn't have to "look inside" that function and inspect inputs and outputs of nested function calls. I'm curious if there are considerations that I missed!

I'm wondering how this will affect token transfers in callable methods where one zkApp imposes some sort of strict layout of parties. As I understand it, a callee zkApp will be able to inspect the parties of its caller but not the other way around? If so, how does the callee have access to these parties?

@mitschabaude
Copy link
Member Author

How does one construct the party for the callee? Will users have to construct this value before calling into a method or will it be auto-generated in some way?

It's autogenerated! Here is a full code example, there's no extra logic apart from calling the other zkapp: https://github.com/o1-labs/snarkyjs/blob/2b30db84f90ce7b1e0f13983ebf8353634213619/src/examples/zkapps/composability.ts

Just wanted to confirm my understanding that methods on a zkApp are stored in some sort of known order for methodIndex to be used.

Yes, the methods are stored in a fixed order, and that order is also baked into the verification key when compiling. Order depends on the order that the @method decorators are called in, but that's an implementation detail

I'm wondering how this will affect token transfers in callable methods where one zkApp imposes some sort of strict layout of parties. As I understand it, a callee zkApp will be able to inspect the parties of its caller but not the other way around? If so, how does the callee have access to these parties?

Yes, for tokens the parties relationship between caller and callee is exactly reversed.. maybe we have to find a different API for calling token contracts, where we pass some sort of callback. In any case, as Matt pointed out on slack, for tokens the token contract actually has to inspect all the children's children etc, so that they can't transfer tokens without violating the token contract's rules. One simple (but probably too limiting?) way of achieving that would be to require that a token contract's children have no children themselves

@mitschabaude
Copy link
Member Author

mitschabaude commented Jul 25, 2022

Do we think methodIndex is sufficient or should it be a methodDigest? If a digest, could it be computed at build time to have no performance implications anyway? I assume it could be

That's an excellent question! We should really discuss what behavior we want in the case a called zkApp updates its verification key. I see two possibilities:

  1. The intention of the caller is: "I call this particular method on this particular zkApp". If the called zkApp updates its methods, by changing the verification key, the caller zkApp still continues to work without updating, and just uses the updated method on the callee contract. If the update on the called zkApp is malicious, users might be at risk immediately. (EDIT: only if the code that's shipped to users is updated though. Because all execution is still on the user side, also of the callee, so its client-side JS has to agree with its verification key)

  2. The intention of the caller is: "I call this particular method on this particular zkApp, and it has to be at this particular snapshot". If the called zkApp changes its verification key, the caller zkApp will stop working until it is updated as well. This also means that a malicious update which affects the method digest of the called circuit can be caught by a diligent developer, before it can affect users of the caller zkApp.

With taking the methodIndex as identifier, I was going for behavior (1). However, now I no longer think that the index alone does the best job there, because it can happen that the callee is updated by inserting another @method, and then the index would no longer point at the intended function. That's why in the PR, I now implemented (1) as follows: Instead of the method index, I take the string "${methodIndex};${methodName}" and convert that to a Field (using Encoding.stringToFields). I think this captures the intention of calling a method with a particular name.

With regards to a possible implementation of (2), it should be possible to create some sort of digest of the callee method at compile time. We'd want to include that method digest as a constant in the callee circuit though, which creates a cyclic dependency: We can't digest the method circuit before using that digest in the circuit itself. I assume we can come up with a reasonable workaround though. Like, create a digest of the same circuit, with the constant which represents the method digest replaced by zero.

The main question to resolve, however, is which behavior we want: (1) or (2). This is a nuanced discussion and inputs are highly welcome! I'm slightly tending to (1), for the following reason: There are many ways of launching a supply chain attack on dependants of your snarkyjs smart contract without affecting the method digest. For example, an npm package that is included which contains the callee smart contact could modify snarkyjs, such that the method digest doesn't change while the real behavior and verification key do change in ways that harm users. Besides that, there are probably 100 ways how an imported npm package could pwn the developers and users of the caller contract. So, IMO failing on an update of the method digest makes the developer experience of calling other zkApps much worse without notably improving the security of users; it might even give a false sense of security. Real diligence would mean restricting updates to the entire JS code contained in called zkApp packages. This can be done, for example, by fixing the version of the dependency in package.json (not allowing patch release updates). This also achieves what fixing the method digest in the circuit gives you -- users can't create proofs for the callee from an outdated verification key -- but in addition is also blocking all those other ways a malicious package update could create harm.

@mitschabaude
Copy link
Member Author

I have another argument for not forcing the caller to update the verification key when the callee changes: A substantial fraction of zkApps might want to set their permissions so that their verification key is non-updatable -- to remove any doubts about the trustworthiness of the zkApp. Making this decision of freezing your zkApp would be made near impossible if you have dependencies that can update, if this would break your zkApp forever.

@ymekuria
Copy link
Contributor

@method myMethod() {
  let calledContract = new CalledContract(address);
  let result = calledContract.calledMethod(arg);
  // .. do anything with result
}

This simple API is great!

Making all zkApps callable is a deliberate design decision.

This feels like the right approach.

For example, an npm package that is included which contains the callee smart contact could modify snarkyjs, such that the method digest doesn't change while the real behavior and verification key do change in ways that harm users. Besides that, there are probably 100 ways how an imported npm package could pwn the developers and users of the caller contract.

This made me think about how users will use this API to interact with other contracts. In your opinion @mitschabaude or anyone else, do you think composing smart contracts by importing them as npm packages(similar to how we guide to add them to a UI) will enable developers to easily use this api and be a good experience?

@mitschabaude
Copy link
Member Author

mitschabaude commented Jul 26, 2022

do you think composing smart contracts by importing them as npm packages(similar to how we guide to add them to a UI) will enable developers to easily use this api and be a good experience?

Yeah I think this would give a good experience!

Slightly tangential: I wonder if long-term, we should come up with something else than npm for dependency management, like leveraging the on-chain zkappUri to store the smart contract code plus metadata, which could potentially be made more secure / transparent / targeted to smart contract development than npm.

But it is nice that we already have a package manager which everyone knows how to use! So maybe the zkapp metadata idea is complementary to that and not replacing it.

@ymekuria
Copy link
Contributor

Slightly tangential: I wonder if long-term, we should come up with something else than npm for dependency management, like leveraging the on-chain zkappUri to store the smart contract code plus metadata, which could potentially be made more secure / transparent / targeted to smart contract development than npm.

I like the potential of using the zkappUri to store contract code in the future. We should all discuss this possibility further.

@mimoo
Copy link
Contributor

mimoo commented Jul 27, 2022

This is a really well written doc! I have some clarification questions on the RFC:

party

I'm a huge noob but I still don't really get why it's important that the callee has its own party. Is there some doc on "parties" in zkapps somewhere?

plus the result of hashing the callee's own children (= the calls field of the callee's public input)

what are the "children" of the callee?

also, what is the ramification of not including the verifier index/key in the hash we're doing on both sides? I'm wondering how things look like in terms of proofs when we call another zkapp, do we first create a proof running the other zkapp with a specific set of inputs (and the public input of the callee zkapp contains both the output and the calldata) and then verify that proof + use its public input within the caller zkapp? If this is the case then aren't we limited in the number of other zkapp calls we do?

@mimoo
Copy link
Contributor

mimoo commented Jul 27, 2022

ok reading the comments I'm wondering if your "${methodIndex};${methodName}" is:

  • resistant to simple updates of the callee: if the methodIndex change, then it won't work anymore
  • enough against breaking changes: it sounds like the callee should at least keep the same argument types and return type. This is what solidity does IIRC when it computes its method id (it's a hash over the name of the function, and the types of its arguments and return value)

so considering that, why not compute methodIndex like Ethereum does (and include name + types of arguments + type of return value)?

@mitschabaude
Copy link
Member Author

I'm a huge noob but I still don't really get why it's important that the callee has its own party. Is there some doc on "parties" in zkapps somewhere?

A "party" is just an account update: A set of changes and events to a single account, authorized by a proof which has to verify against that account's verification key. The callee is a smart contract method, so by design it creates such an account update. So, calling another zkApp is about much more than obtaining a return value and doing something with it -- it's also about composing smart contracts, which can move balances, change on-chain state etc. For all that, the callee has to include its own account update.

I think the currently best general doc on parties is https://o1-labs.github.io/snapps-txns-reference-impl/target/doc/snapps_txn_reference_impl/index.html

plus the result of hashing the callee's own children (= the calls field of the callee's public input)

what are the "children" of the callee?

The account updates of a transaction form a nested structure -- they're a list of trees (a "forest"). In our example, the caller is a top-level element of that list, and the callee is one of the caller's children. But the callee also has it's own list of children (which could be empty).

I'll try to explain what the purpose of this nesting is. It has to do with the public input of a zkApp proof.

In a zkApp proof, the public input consists of two field elements:

  1. a hash of the zkApp's own account update
  2. a hash of the entire tree of account updates below the zkApp's own update

So, this is the information we can reason about / make assertions about in a zkApp proof. In particular, a zkApp proof will reason about its own account update, and at the end, it will hash its update (in the circuit) and assert that it's equal to the first public input. This is how the account update gets authorized by the proof.

Similarly, you can authorize whatever the account updates below you in your tree are doing -- your children, their children, etc. Because you can hash them in your circuit and compare the result to your second public input!

So that's the purpose of nesting account updates -- enabling some zkApps to make statements about what other zkApps are doing. In this composability API, the caller has to make statements about the callee's callData, publicKey and tokenId, which are all contained in the callee's account update; that's why we make the callee a child of the caller. What I'm saying in the quote above is just that we don't care about making assertions about the callee's own children / the account updates deeper down in the tree.

also, what is the ramification of not including the verifier index/key in the hash we're doing on both sides? I'm wondering how things look like in terms of proofs when we call another zkapp, do we first create a proof running the other zkapp with a specific set of inputs (and the public input of the callee zkapp contains both the output and the calldata) and then verify that proof + use its public input within the caller zkapp? If this is the case then aren't we limited in the number of other zkapp calls we do?

We don't verify the callee's proof in the caller circuit. This is an alternative model, which is lighter-weight on the client and heavier on the server: Both the callee's and caller's proof have to be verified independently by the Mina node. They're just connected via their public inputs.

In particular, the callee's verification key is not a dependency for the caller; it can be changed without changing the caller (on-chain). However, the client-side code that produces the caller + callee proof has to be up to date with the callee's verification key, of course, otherwise we couldn't create its proof (or, we'd create a proof that fails).

@mitschabaude
Copy link
Member Author

mitschabaude commented Jul 27, 2022

ok reading the comments I'm wondering if your "${methodIndex};${methodName}" is:

* _resistant to simple updates of the callee_: if the methodIndex change, then it won't work anymore

* _enough against breaking changes_: it sounds like the callee should at least keep the same argument types and return type. This is what solidity does IIRC when it computes its method id (it's a hash over the name of the function, and the types of its arguments and return value)

so considering that, why not compute methodIndex like Ethereum does (and include name + types of arguments + type of return value)?

That's very good input! I agree that the methodIndex is unnecessarily constraining, I'll leave it out and just include the full name converted to a list of field elements (previously I cut if off to one field element, that's why I thought I need the index for uniqueness, but there's no reason to restrict the name to one field element.)

Re types: Right now, at least the cumulative length in field elements of argument + return types is baked into the hash. However, this is very ambiguous. I propose that we do the following instead:

  • convert each argument $i$ to an array of field elements $X_i$ with length $n_i$
  • let the argument's hash input be $X_a := [n_0, ... X_0, n_1,... X_1, ...]$
  • let the total argument length be $n_a = n_0 + n_1 + ...$ (can be zero)
  • similarly, convert the return value to fields $X_r$ with length $n_r$ (can be zero).
  • let the method id be a list of field elements $X_m$ with length $n_m$
  • let $b$ be the blindingValue
  • compute $\mathrm{hash}([n_a, ...X_a, n_r, ...X_r, n_m, ...X_m, b])$

By including the lengths in the hash, we fix the number of arguments, length of each argument in fields, length of return value in fields, and the method name. (I learned this from your book @mimoo 😁 )

I don't think we can/should attach richer information than "length in fields" about any type, because a type doesn't necessarily have a name attached to it. From the circuit point of view, types really are a certain fields layout, plus some associated assertions that are added to the circuit, like the booleanness check for Bool. I don't think there's a good way of serializing the assertions though, fields layout seems to do a good job.

@mimoo
Copy link
Contributor

mimoo commented Aug 1, 2022

a hash of the zkApp's own account update
a hash of the entire tree of account updates below the zkApp's own update

interesting, is there a limit in account updates per transactions, because it takes room in a public input somewhere or something? Or perhaps this is information that's used in a proof and discarded? I guess I should just read the spec you linked to as this is starting to get unrelated :)

In this composability API, the caller has to make statements about the callee's callData, publicKey and tokenId

I'm guessing the tokenId of a zkapp is passed as public input, so it's important that we're in a model where the nested zkapp proof is verifier by the network and calls are glued by the network. (Otherwise I might lie about the callee's tokenId and they might lie about it too right?)

(I learned this from your book @mimoo 😁 )

wow nice :D

I don't think we can/should attach richer information than "length in fields" about any type, because a type doesn't necessarily have a name attached to it. From the circuit point of view, types really are a certain fields layout, plus some associated assertions that are added to the circuit, like the booleanness check for Bool. I don't think there's a good way of serializing the assertions though, fields layout seems to do a good job.

That's true, but they also don't have several arguments or even a return value after being compiled, it's just one single array of field elements. So IIUC, the decision is about where we want to put the line and break when changes occur.

I agree though that type names are probably going to be a pain if you're in a situation where you call a zkapp written in snarkyjs that calls a zkapp written in another language whenre type names have a different spelling convention.

In general the proposition looks good to me!

@mitschabaude
Copy link
Member Author

interesting, is there a limit in account updates per transactions, because it takes room in a public input somewhere or something?

there's no limit in general / on the protocol side, because the public input is just the two hashes, i.e. it has a fixed size of 2 field elements.

limits on children of a particular zkapp could be caused if the zkapp fully (re-)computes those hashes in its circuit. for the composability API, we don't make any assumptions about the callee's children, since we do not recompute its children's hashes in the caller circuit.

@bkase
Copy link
Member

bkase commented Aug 2, 2022

Just catching up here -- (1) makes sense to me; as you've said, clients can ask dependencies to become non-upgradable if they are worried by this. Plus upgradeability of dependent contract calls enables libraries to publish security fixes.

Also the security tradeoffs with the latest "smart contract method name mangling" algorithm sound good to me too 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rfc Issues that contain some important documentation
Projects
None yet
Development

No branches or pull requests

6 participants