cartesi / rollups

Cartesi Rollups
30 stars 12 forks source link

Add `Authority` factory #64

Closed pedroargento closed 1 year ago

pedroargento commented 1 year ago

resolves cartesi/rollups-contracts#25 Changes:

TODO:

pedroargento commented 1 year ago

I had trouble generating the output proofs on my machine because of some conflicting node versions.

ZzzzHui commented 1 year ago

I created a PR to suggest some changes :)

tuler commented 1 year ago

What is the plan for History setup?

guidanoli commented 1 year ago

What is the plan for History setup?

@tuler I would suggest we create a factory for each history we implement. Currently, there is only one history implementation, which only accepts individual claims, but we envision other history implementations in the future. This would allow for optimal modularity: you can deploy any kind of consensus, which uses any kind of history.

What do you think? FYI @DaniOrtegaB @pedroargento @ZzzzHui

tuler commented 1 year ago

@guidanoli I was hoping to replace the code below with a single transaction.

    const Authority = await deployments.deploy("SunodoAuthority", {
        ...opts,
        contract: "Authority",
        args: [deployer, InputBox.address],
    });

    const History = await deployments.deploy("SunodoHistory", {
        ...opts,
        contract: "History",
        args: [Authority.address],
    });

    const authority = Authority__factory.connect(Authority.address, signer);
    const currentHistory = await authority.getHistory();
    if (currentHistory != History.address) {
        const tx = await authority.setHistory(History.address);
        const receipt = await tx.wait();
        if (receipt.status == 0) {
            throw Error(`Could not link Authority to history: ${tx.hash}`);
        }
    }
guidanoli commented 1 year ago

@guidanoli I was hoping to replace the code below with a single transaction.

For this (very) common case, where we use Authority linked to a simple History, we could create a contract that does all that. It would receive both factory addresses on the constructor and make them immutable (embedding them in the final byte code), and have an entry point that would instantiate an Authority and a History contract, and link them. We could even have a deterministic deployment that uses the same salt. What do you think?

pedroargento commented 1 year ago

If we create a History factory maybe its time to make a better abstraction for what a factory is. The Dapp Factory and Authority Factory share a lot of code.

guidanoli commented 1 year ago

If we create a History factory maybe its time to make a better abstraction for what a factory is. The Dapp Factory and Authority Factory share a lot of code.

Unfortunately, I don't think we are able to create these high-level typed abstractions in Solidity like you probably could in compilation-time-heavy languages like C++ or Rust. For example, defining a Factory<T> generic contract is not possible in the latest version of Solidity (0.8.20).

pedroargento commented 1 year ago

I see, it makes sense... So the only thing we could repurpose would be the deterministic address calculation, right?

guidanoli commented 1 year ago

I see, it makes sense... So the only thing we could repurpose would be the deterministic address calculation, right?

Oh, you're right. We could create a library of some sorts that has this deterministic address calculation routine. Let us see also if there is anything like these already implemented so we don't need to reinvent the wheel. :-)

guidanoli commented 1 year ago

@pedroargento It seems like OpenZeppelin has a Create2 library we can use. :-)

tuler commented 1 year ago

I think it's easier to wire the InputBox in the factory instead of passing as an arg to new newAuthority

guidanoli commented 1 year ago

Rebased and fixed commit scopes (onchain-contracts :arrow_right: contracts).

guidanoli commented 1 year ago

Ran yarn prepack.

guidanoli commented 1 year ago

Rebased and organized commits.

guidanoli commented 1 year ago

This PR has been migrated to https://github.com/cartesi/rollups-contracts/pull/26.