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

Rename Gateway library to Decrypt library #662

Open
immortal-tofu opened this issue Dec 18, 2024 · 13 comments
Open

Rename Gateway library to Decrypt library #662

immortal-tofu opened this issue Dec 18, 2024 · 13 comments
Assignees

Comments

@immortal-tofu
Copy link
Collaborator

To simplify wording and API of the Gateway:

Gateway library

        uint256[] memory cts = new uint256[](1);
        cts[0] = Gateway.toUint256(xBool);
        Decrypt.request(cts, this.callbackBool.selector, block.timestamp + 2 days);

The signatures are always verified: (we need to pick one option)

  • Option 1: by the Gateway contract but For now, we'll keep UUPS proxy on Gateway.
  • Option 2: by the dapp contract but it should be very simple to do it. Not sure it's doable.

GatewayCaller

Two options:

  • Rename GatewayCaller to something else: DecryptParams ? Decrypt? StoreDecryptParams?
  • Remove GatewayCaller to embed this storage directly through the library

We'd keep setGateway and others gatewayUrl when we are referring to the contract address or the URL.

@jatZama
Copy link
Member

jatZama commented Dec 18, 2024

In current design you cannot use block.timestamp + 2 days fyi because there is a current max delay constant set to 1 day in the contract: https://github.com/zama-ai/fhevm-backend/blob/main/contracts/gateway/GatewayContract.sol#L32
This was done to let the gateway service catch up for all missed requests until 24H before , in case it shuts down for any reason.
But this is why as I proposed in this issue we first need to rediscuss extensively many specification details for GatewayContract: see zama-ai/fhevm-backend#186

@immortal-tofu
Copy link
Collaborator Author

I just copied past an example from TestAsyncDecrypt and changed it, I probably didn't copy the good one. Ignore the + 2 days.

@jatZama
Copy link
Member

jatZama commented Dec 19, 2024

For Gateway Library: you said Option 2: by the dapp contract but it should be very simple to do it. Not sure it's doable. this is already implemented in a KMS agnostic way, so I say let's keep the option to the developers to use trustless case or not. This is already implemented and the most general way.

For GatewayCaller :
You cannot replace the GatewayCaller to a library, since this abstract contract is holding so many storage mappings: GatewayCaller so option 2 is impossible.

@jatZama
Copy link
Member

jatZama commented Dec 19, 2024

@PacificYield also suggested to rename the GatewayContract to simply Gateway. At first we could not do this because we already had the Gateway solidity library with same name, but since you want to rename the Gateway library to Decrypt then maybe we could then also rename GatewayContract to Gateway, wdyt?

@PacificYield
Copy link
Contributor

PacificYield commented Dec 19, 2024

Yes, I think we could rename Gateway.sol (the lib) to Decrypt.sol and rename GatewayContract to Gateway, since it is the gate between the offchain and the onchain worlds.

@immortal-tofu
Copy link
Collaborator Author

For Gateway Library: you said Option 2: by the dapp contract but it should be very simple to do it. Not sure it's doable. this is already implemented in a KMS agnostic way, so I say let's keep the option to the developers to use trustless case or not. This is already implemented and the most general way.

For GatewayCaller : You cannot replace the GatewayCaller to a library, since this abstract contract is holding so many storage mappings: GatewayCaller so option 2 is impossible.

We need to be opinionated there. We added an option to "choose" between trustless or not, I think we should go for trustless or "almost" trustless (= trustless if the Gateway contract is not updated). We need to simplify things. I'm open to proposition in that direction.

@jatZama
Copy link
Member

jatZama commented Dec 19, 2024

For Gateway Library: you said Option 2: by the dapp contract but it should be very simple to do it. Not sure it's doable. this is already implemented in a KMS agnostic way, so I say let's keep the option to the developers to use trustless case or not. This is already implemented and the most general way.
For GatewayCaller : You cannot replace the GatewayCaller to a library, since this abstract contract is holding so many storage mappings: GatewayCaller so option 2 is impossible.

We need to be opinionated there. We added an option to "choose" between trustless or not, I think we should go for trustless or "almost" trustless (= trustless if the Gateway contract is not updated). We need to simplify things. I'm open to proposition in that direction.

So we keep only what i call today the "trusted" decryption case, meaning the KMSVerifier is called only by the GatewayContract, never the dapp? and hence we make the GatewayContract immutable instead of upgradeable? I am ok with this but , this contradicts @mortendahl 's original specification since he explicitly wanted the dApp itself to check KMS singatures. Wdyt @mortendahl ?

@immortal-tofu
Copy link
Collaborator Author

immortal-tofu commented Dec 19, 2024

For Gateway Library: you said Option 2: by the dapp contract but it should be very simple to do it. Not sure it's doable. this is already implemented in a KMS agnostic way, so I say let's keep the option to the developers to use trustless case or not. This is already implemented and the most general way.
For GatewayCaller : You cannot replace the GatewayCaller to a library, since this abstract contract is holding so many storage mappings: GatewayCaller so option 2 is impossible.

We need to be opinionated there. We added an option to "choose" between trustless or not, I think we should go for trustless or "almost" trustless (= trustless if the Gateway contract is not updated). We need to simplify things. I'm open to proposition in that direction.

So we keep only what i call today the "trusted" decryption case, meaning the KMSVerifier is called only by the GatewayContract, never the dapp? and hence we make the GatewayContract immutable instead of upgradeable? I am ok with this but , this contradicts @mortendahl 's original specification since he explicitly wanted the dApp itself to check KMS singatures. Wdyt @mortendahl ?

As I said, I'm ok with trustless (= dapp is verifying signature) or "semi trusted" (=Gateway contract verifies signature but could potentially updated).
When you use the gateway, you're a Zama customer. It's ok to have this "trust assumption" that the gateway won't be updated maliciously, like you trust Zama to not update the TFHEExecutor maliciously on the coprocessor.

Again, the idea is not to design "the best Gateway", the idea is to deliver a usable Gateway for ZWS.

@jatZama
Copy link
Member

jatZama commented Dec 19, 2024

For Gateway Library: you said Option 2: by the dapp contract but it should be very simple to do it. Not sure it's doable. this is already implemented in a KMS agnostic way, so I say let's keep the option to the developers to use trustless case or not. This is already implemented and the most general way.
For GatewayCaller : You cannot replace the GatewayCaller to a library, since this abstract contract is holding so many storage mappings: GatewayCaller so option 2 is impossible.

We need to be opinionated there. We added an option to "choose" between trustless or not, I think we should go for trustless or "almost" trustless (= trustless if the Gateway contract is not updated). We need to simplify things. I'm open to proposition in that direction.

So we keep only what i call today the "trusted" decryption case, meaning the KMSVerifier is called only by the GatewayContract, never the dapp? and hence we make the GatewayContract immutable instead of upgradeable? I am ok with this but , this contradicts @mortendahl 's original specification since he explicitly wanted the dApp itself to check KMS singatures. Wdyt @mortendahl ?

As I said, I'm ok with trustless (= dapp is verifying signature) or "semi trusted" (=Gateway contract verifies signature but could potentially updated). When you use the gateway, you're a Zama customer. It's ok to have this "trust assumption" that the gateway won't be updated maliciously, like you trust Zama to not update the TFHEExecutor maliciously on the coprocessor.

Again, the idea is not to design "the best Gateway", the idea is to deliver a usable Gateway for ZWS.

Is this an English "or" (exclusive) or a mathematics "or" (inclusive)?
Maybe another idea could be to rework how the trustless decryption (ie kms signatures verified inside dapp) happens exactly, because this part was only ever specified only by me very quickly back in June, and no one ever reviewed the initial design.

@immortal-tofu
Copy link
Collaborator Author

This is "or" exclusive. I want to get rid of the boolean option as described in the library ticket.

@mortendahl
Copy link

I am ok with this but , this contradicts @mortendahl 's original specification since he explicitly wanted the dApp itself to check KMS singatures. Wdyt @mortendahl ?

Yes I think it's fine, if it's causing big issues. If we ever notice that developers are asking for it, we can re-introduce the trustless.

@jatZama
Copy link
Member

jatZama commented Dec 19, 2024

what is the conclusion?
Gateway solidity library to be renamed -> Decrypt
GatewayContract solidity contract to be renamed -> Gateway
GatewayCaller to be renamed -> DecryptParams ?
Is this correct ?

What about the onlyGateway modifier defined in GatewayCaller / DecryptParams? should it be renamed onlyDecrypt as well?

Another issue related to trustless decryption is that it would be very helpful in case a dApp wants high availability and not assume Gateway will be always online to fulfil the request @mortendahl so I am not sure we could really neglect the trustless case long term. Otherwise, a way to avoid this issue short term is to remove theonlyRelayer modifier on the fulfillRequest function of Gateway contract, meaning anyone could submit the decryption request from KMS to the Gateway contract, but to make this meaningful it would also mean we would need to make the Gateway contract immutable instead of an upgradeable proxy.

@mortendahl
Copy link

Another issue related to trustless decryption is that it would be very helpful in case a dApp wants high availability and not assume Gateway will be always online to fulfil the request @mortendahl so I am not sure we could really neglect the trustless case long term.

I'm wondering if developers would not code this resilient into the dapp more directly. Ie, the ability to update which gateway service they use in case it stops running.

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

4 participants