Improve GatewayContract design

[jatZama]

We have to discuss further the specifications of GatewayContract, here is a (non-exhaustive) list of the suggestions to improve its design:

1/ Don't make it upgradable, this is actually needed for a real trustless version - to avoid the problems with requestID logic in the upgradeable case - instead the dapp developer could always optionally add a setter function in his dApp to change the gateway address if he really wish to. Remember this is acceptable since GatewayContract is not part of the core fhevm protocol.

2/ Also note that to get a real trustless decryption method example, the onlyGateway modifier inside the callback of the dApp could be dropped - even if the dev still uses the GatewayContract for high availability and simpler integration.

3/ Don't use the onlyRelayer modifier on GatewayContract:fulfillRequest , especially if we implement 0/ this will be needed, to ensure decryption can always be available for the dApp (no "relayer censorship"is possible), and because no more owner of contract is needed since it is not a proxy and we should remove the addRelayer function, if we agree to implement this.

4/ Remove payable modifier from GatewayContract:fulfillRequest : in this case it would make sense to also remove the msgValue from the GatewayContract:requestDecryption function and from EventDecryption event -> breaking changes on gateway service then because of new function and event selectors. We also discussed this point already with @manoranjith. It reduces risks, and I don't think there is any good use case which really needs a payable callback.

5/ We should write clear specifications for Gateway payment, Ghazi asked many questions around this unsolved issue, but it is still unclear if some onchain payment from the requesting dApp/user should happen and how to handle it exactly. This point is also closely related the offchain logic to avoid DDOS, maybe use something more sensible than the today hardcoded gasLimit of 750_000. This point is a huge topic in itself.

6/ Minor improvement: use custom errors instead of revert strings in requires, to save some gas. This is not specific to GatewayContract btw, we have same issue in many core contracts well.

7/ Minor improvement: rename the EventDecryption to something more explicit, such as DecryptionRequest maybe. breaking change as well on Gateway service if we do this btw

8/ Should we increase the MAX_DELAY constant to something bigger than the current value of 1 day?

[PacificYield]

Two questions from me.

  function requestDecryption(
      uint256[] calldata ctsHandles,
      bytes4 callbackSelector,
      uint256 msgValue,
      uint256 maxTimestamp,
      bool passSignaturesToCaller
  ) external virtual returns (uint256 initialCounter) {

What is the rationale to use maxTimestamp instead of maxBlockNumber for request decryption?

How about renaming GatewayContract to GatewayRequestHandler? It seems a bit odd to have a suffix "Contract" in the name of the contract.

[immortal-tofu]

1, 2 and 3:
Gateway Contract will be upgradable, this is too complicated to make it permanent at this stage. I agree that we need a mechanism where anyone can send decryption if they provide proper signatures, so onlyRelayer must disappear.

We have two options:

• The Gateway contract is verifying the signatures.
• The dapp is verifying the signatures

For the second point (The dapp is verifying the signatures), I'm wondering if it's not possible to embed the GatewayContract into the dapp directly. you need to call the proxy on the contract implemented by GatewayCaller:

abstract contract GatewayCaller {
    function fulfillRequest(
        uint256 requestID,
        bytes memory decryptedCts,
        bytes[] memory signatures
    ) {
       ...
       (will call myCallback)
    }
}

contract MyDapp is GatewayCaller {
  myCallback(uint256 requestId, bool value) internal {
    ...
  }
}

Maybe with this model, we can even pass stored parameters (stored with GatewayCaller) directly as parameter of the function.

[immortal-tofu]

I agree for 4/

For 5/, this is out of scope: decryption are free for now but will be charged and monitored by ZWS offchain. They'll probably ignore request for a non-paying users.

No opinion on 6/7/8

[jatZama]

Two questions from me.

  function requestDecryption(
      uint256[] calldata ctsHandles,
      bytes4 callbackSelector,
      uint256 msgValue,
      uint256 maxTimestamp,
      bool passSignaturesToCaller
  ) external virtual returns (uint256 initialCounter) {

What is the rationale to use maxTimestamp instead of maxBlockNumber for request decryption?

How about renaming GatewayContract to GatewayRequestHandler? It seems a bit odd to have a suffix "Contract" in the name of the contract.

maxBlockNumber is a bad idea because it is less precise -and also less human interpretable- than maxTimestamp. For eg if a slot is missed by a validator on ethereum, you can have a lot of variability (if a block is missed, timestamp increases 24sec instead of 12s between 2 blocks). This can have very serious impact in DeFi.
For second point we agreed to rename the contract to Gateway.

[jatZama]

1, 2 and 3: Gateway Contract will be upgradable, this is too complicated to make it permanent at this stage. I agree that we need a mechanism where anyone can send decryption if they provide proper signatures, so onlyRelayer must disappear.

We have two options:

• The Gateway contract is verifying the signatures.
• The dapp is verifying the signatures

For the second point (The dapp is verifying the signatures), I'm wondering if it's not possible to embed the GatewayContract into the dapp directly. you need to call the proxy on the contract implemented by GatewayCaller:

abstract contract GatewayCaller {
function fulfillRequest(
uint256 requestID,
bytes memory decryptedCts,
bytes[] memory signatures
) {
...
(will call myCallback)
}
}
contract MyDapp is GatewayCaller {
myCallback(uint256 requestId, bool value) internal {
...
}
}
Maybe with this model, we can even pass stored parameters (stored with GatewayCaller) directly as parameter of the function.

I don't understand your second point to " embed the GatewayContract into the dapp directly." I need more details.
Are you planning to drop the Gateway contract entirely? How would this work to make the Gateway service aware a decryption was requested? I guess this could work but only if the dApp developer himself launches his own gateway service for his own dapp, so he would track his own requestID, emit his own DecryptionRequest event etc.

[immortal-tofu]

The idea would be to keep the gateway contract "just" to emit events, but the cts list, and requestId are handled by the dapp.
It would imply a change on the gateway service of course.

The dapp would contains:

• request IDs
• cts to decrypt
• The requestFulfill function

The Gateway contract contains:

• A route to emit an event and store metadata (which contract to call to get cts and to fulfillRequest)

To init a decryption, you'd call a dapp function requestDecryption which will create a request id, store cts list and call the gateway contract (or decrypt contract) transmitting the requestId.
The offchain gateway service knows that by calling dapp.getCts(requestId) it will get the list of cts to decrypt. The offchain service receive decryption and signatures, and call dapp's fulfillRequests with these informations.

These methods (fulFillRequest, requestDecryption, getCts) are included in what we call today GatewayCaller. Of course these are not definitive name, it's just here to illustrate the workflow.

[immortal-tofu]

With this method, we could have 100% trustless since the fulfillRequest is in the dapp.
Note: If you're concerned by requestId relative to dapp, we can have a requestId generator in the gateway contract which returns an unique id.

[jatZama]

The idea would be to keep the gateway contract "just" to emit events, but the cts list, and requestId are handled by the dapp. It would imply a change on the gateway service of course.

The dapp would contains:

• request IDs
• cts to decrypt
• The requestFulfill function

The Gateway contract contains:

• A route to emit an event and store metadata (which contract to call to get cts and to fulfillRequest)

To init a decryption, you'd call a dapp function requestDecryption which will create a request id, store cts list and call the gateway contract (or decrypt contract) transmitting the requestId. The offchain gateway service knows that by calling dapp.getCts(requestId) it will get the list of cts to decrypt. The offchain service receive decryption and signatures, and call dapp's fulfillRequests with these informations.

These methods (fulFillRequest, requestDecryption, getCts) are included in what we call today GatewayCaller. Of course these are not definitive name, it's just here to illustrate the workflow.

Implementation of what you suggest is done in this PR : #217
Note that requestID is generated inside the DecryptionOracle contract (new name you decided for replacing GatewayContract) by hashing the dApp address with a global counter stored in DecryptionOracle. For this method to be 100% trustless we would need DecryptionOracle to be immutable, but you decided on Slack to keep it upgradeable for now.
Note also there was no need to implement a getCts function since the requested handles to be decrypted are already emitted inside the DecryptionRequest event.