cartesi / rollups

Cartesi Rollups
30 stars 12 forks source link

Refactor deployment to remove Authority-History inter-dependency #28

Closed pedroargento closed 1 year ago

pedroargento commented 1 year ago

The current deployment scripts for the Authority and History contracts have an inter-dependency, where the order of deployment matters. This can cause issues when creating a new pair of Authority and History contracts, as they depend on the existence of each other. This creates a chicken-and-egg problem and can result in unnecessary storage writes and calldata usage.

Proposed Solution:

  1. Refactor the deployment scripts to remove the inter-dependency between the Authority constructor and the History constructor.

  2. Discuss and reach a consensus on the recommended way of deployment for creating a new pair of Authority and History contracts. One possible solution is to create the History contract with the parameter _owner set to msg.sender, then create the Authority contract with the History contract as a parameter, and finally call H.migrateToConsensus(A) to transfer ownership to the Authority contract.

  3. Consider removing the _owner parameter from the History constructor to save calldata in cases where most of the use cases involve creating a new pair of Authority and History contracts.

  4. Document the deployment dependency and recommended way of deployment in the documentation to guide developers.

  5. Remove the _history parameter from the Authority constructor after discussing and reaching a consensus on the changes to save a storage write.

Expected Outcome:

Refactoring the deployment scripts and removing the Authority-History inter-dependency will improve the efficiency of contract deployments and reduce unnecessary storage writes and calldata usage. Proper documentation and recommended way of deployment will guide developers in creating new pairs of Authority and History contracts without encountering the inter-dependency issue.

guidanoli commented 1 year ago

I think it would be beneficial to clarify that the interdependence between Authority and History lies on a deeper, conceptual level, which will not be removed by the refactor. Every Authority depends on a History contract to store and retrieve its claims, and every History contract has an owner (which can be any type of consensus, really!).

As you can see, the problem lies mostly on the design of the smart contracts, and—more specifically—their constructors. Therefore, the deployment scripts merely reflect how the smart contracts were designed.

Rather than removing the interdependence, the refactor offers savings in gas costs on unnecessary storage writes, and making the order of deployment unambiguous, by removing the _history parameter from the Authority contract. By doing so, we would also have to remove the history parameter from the ApplicationCreated event.

pedroargento commented 1 year ago

While we are at this, its also important to reorganize deployment scripts. Separate what is Cartesi common infrastructure (eg. Portals, Factory), dapp deployment, and validator infrastructure (History, Authority).

guidanoli commented 1 year ago

This has been superseeded by https://github.com/cartesi/rollups-contracts/issues/35.