Merge contract repositories

[jannikluhn]

Currently, we have two separate contract repositories: shutter-network/shop-contracts and shutter-network/gnosh-contracts. We should merge them into one (e.g. shutter-network/contracts) and get rid of unused contracts. In particular:

• shop-contracts:
  • KeyperSetManager, KeyperSet, KeyBroadcast and EonKeyPublish is used in both the SHOP and the GNOSH implementation
  • Inbox is used in the SHOP implementation
• gnosh-contracts:
  • KeyperSetManager, KeyperSet, KeyBroadcast and EonKeyPublish are unused and can be removed
  • Sequencer and ValidatorRegistry are used in the GNOSH implementation
  • The interfaces IKeyperSetManager, IKeyperSet, IKeybroadcast, IValidatorRegistry and ISequencer are references to the GNOSH spec and should be preserved
• We have tests and scripts in both repositories. There is a probably a lot of overlap that we only need to preserve once. For the scripts, we should especially have two separate deploy scripts for shop and gnosh as well as a script to add a new keyper set.
• The two repos have automatically generated go bindings that are used as dependencies in rolling-shutter and other repos. They get generated in slightly different ways (see gen/gen.go in shop-contracts and gen_bindings.sh in gnosh-contracts). I personally prefer the latter, but others may disagree. We should end up with either a single or two separate packages. A single is probably preferable for simplicity.
• In shop-contracts there's some predeploy code that probably should be preserved, but I'm not entirely sure if we still need it.

[konradkonrad]

Created a repository with combined main branch history: https://github.com/shutter-network/contracts

[konradkonrad]

Question @jannikluhn :

• The interfaces IKeyperSetManager, IKeyperSet, IKeybroadcast, IValidatorRegistry and ISequencer are references to the GNOSH spec and should be preserved

By keeping only the shop implementations, these interfaces are now unused. Is this intentional? Should the common implementation use the interfaces?

[jannikluhn]

I guess for clarity it would be good

• to explicitly implement the interfaces in the contracts
• to use the interfaces when we reference contracts, but don't care about the concrete implementation (e.g. when the KeyperSetManager references the KeyperSet)

[konradkonrad]

Hmm, for KeyperSetManager this leads to some trouble. By using the shop implementation and implementing the gnosh interface IKeyperSetManager, we end up with these domain-incompatible implementations:

…
contract KeyperSetManager is RestrictedPausable, IKeyperSetManager {
…

    function getKeyperSetIndexByBlock(
        uint64 blockNumber
    ) external view returns (uint64) {
        for (uint256 i = keyperSets.length; i > 0; i--) {
            if (keyperSets[i - 1].activationBlock <= blockNumber) {
                return uint64(i - 1);
            }
        }
        revert NoActiveKeyperSet();
    }

    function getKeyperSetActivationBlock(
        uint64 index
    ) external view returns (uint64) {
        return keyperSets[index].activationBlock;
    }

    function getKeyperSetAddress(uint64 index) external view returns (address) {
        return keyperSets[index].contractAddress;
    }

       function getKeyperSetIndexBySlot(
        uint64 slot
    ) external view returns (uint64) {
        for (uint256 i = activationSlots.length; i > 0; i--) {
            if (activationSlots[i - 1] <= slot) {
                return uint64(i - 1);
            }
        }
        revert NoActiveKeyperSet();
    }

    function getKeyperSetAddress(uint64 index) external view returns (address) {
        return contracts[index];
    }

    function getKeyperSetActivationSlot(
        uint64 index
    ) external view returns (uint64) {
        return activationSlots[index];
    }

[jannikluhn]

Apparently, IKeyperSetManager is outdated. It didn't do the slot -> block transition which the SHOP implementation did. It should be easy to update. Haven't checked the other interface yet.

[konradkonrad]

@ulope can you state snapshutter requirements for the common contracts repository?

[konradkonrad]

@ezdac Can you give some input on what is absolutely necessary to be kept from shop for codegen/predeploys? See

• https://github.com/shutter-network/contracts/tree/main/shop_orig vs
• https://github.com/shutter-network/contracts/blob/main/gen_shop_bindings.sh

[ulope]

@ulope can you state snapshutter requirements for the common contracts repository?

The snapshot integration doesn't have requirements on the contracts per-se. It only uses the contracts as far as required for Keyper set management.

In the old (currently still in use) implementation the Snapshot hub interface "pretends" to be the (now removed) collator, since that was the easiest way to allow it to issue and sign decryption requests.

[ezdac]

@ezdac Can you give some input on what is absolutely necessary to be kept from shop for codegen/predeploys? See

The codegen bindings are all necessary, however you can re-use the overlapping common code packages as long as the contracts used in both are interchangeable (which they seem to be). This means you only have to keep the Inbox source code and integrate that in the abigen code (which you are already doing). Maybe it would be good to add a prefix so that it's calledOptimismInbox to have a clear namespace separator.

The "predeploy" code can be kept as is. The important part here is the contracts.gen.go and contracts.go files. But this is only a mapping of addresses and is independent of the source / abi of the contracts. The ABI is used in the optimism predeploy code, but there the aformentioned bindings are used to import the ABI. The whole predeploy code generation is a convenience, but represents more a hammer searching for a nail.