Move the crypto contract outside of cadence

[tarakby]

Issue to be solved

The crypto contract offers crypto tools that are fully developed in the cadence language and don't have to be built-in in the language. It makes more sense to take the crypto contract outside of the Cadence repo and add it as an independent standard library core-contract.

This issue is a follow-up on the discussion in #1447 (comment)

Suggested Solution

Moving the Crypto contract out of Cadence will require:

• Moving the contract to the Flow core contracts repository (https://github.com/onflow/flow-core-contracts) (small)
• Move tests (small)
• Update Cadence to no longer handle contract specifically (small)
• Update flow-go / FVM to rewrite import (small)
• Update downstream dependencies (small)
• Deploy the contract on all chains (medium)

No additional work in the contract itself is required, we had already previously refactored it to be deployable like any other contract

[j1010001]

Given the effort required to complete this (moving this as a separate contract, updating all contracts to import this) it is unlikely we can complete it in C1.0. It might need to stay where it is.

[j1010001]

We might be able to do this in a non-breaking way, if we can make "import crypto" statement to work - then this would not be a breaking change.

[bluesign]

I feel like if something breaking will be done here, better to moving native types ( HashAlgorithm.etc ) inside crypto in a native way. Crypto being pure Cadence has no value for the future. If we later have add something that is not in pure Cadence ( which seems pretty likely in the crypto context ) at least we can prevent polluting global namespace.

[joshuahannan]

I'm happy to help with this if needed!

[turbolent]

Instead of migrating the state, we can maybe just add a function between Cadence and FVM, and keep resolving import Crypto to a particular address on a particular chain.

[turbolent]

@joshuahannan Thank you! I've ported the Crypto contract to the Flow Core Contracts repo here: onflow/flow-core-contracts#456

[turbolent]

@joshuahannan Now that onflow/flow-core-contracts#456 is merged, could you please help with deploying it on-chain to all networks?

[turbolent]

cc @j1010001 and @nvdtf

[joshuahannan]

@turbolent We want to deploy it to the Service Account, right? Should we also prep some sort of announcement about this new deployment?

[turbolent]

@joshuahannan Correct, the Crypto contract is assumed to be deployed to the Service Account, see https://github.com/onflow/flow-go/blob/c61e4aa9cb32882c1613700e87a7be8befe6f5d2/fvm/systemcontracts/system_contracts.go#L372.

IDK if we need an announcement for it, given the contract is just "moved", and that's rather just an implementation detail. Did we do it e.g. for the Burner contract?

[joshuahannan]

Good point. Probably don't need any announcement. I deployed on the testnet service account: https://testnet.flowview.app/account/0x8c5303eaa26202d6/contract

We'll need to schedule a multisig to deploy it on mainnet. What kind of testing do we need to do on testnet before we deploy to mainnet?

[turbolent]

Running the test case that is in the repo, https://github.com/onflow/flow-go/pull/6571/files#diff-b9fa8f5074fc5581ad11c32215b4ff4ecd8d79bb1a471ae692236631d3cdacadR3236-R3406, should be sufficient.

cc @tarakby for more if needed

[tarakby]

We should add failing cases tests, the current test only checks the happy path.
I can't add them right now but I'll create an issue. If that was the level of testing we had already, then we shouldn't block deploying the contracts (I've checked the contract implementation).

[tarakby]

Btw shouldn't the tests be added to the core-contracts repo? why are they in flow-go?
(I'm asking to know in which repo I should create the testing issue)

[turbolent]

@tarakby There are multiple tests, at different levels.

The Crypto contract got moved from Cadence to the Flow Core Contracts repo: https://github.com/onflow/flow-core-contracts/blob/master/contracts/Crypto.cdc, and there are existing unit tests: https://github.com/onflow/flow-core-contracts/blob/master/contracts/Crypto_test.cdc. Please feel free to extend them.

Then in flow-go there is the test linked above, which is an integration test. Agreed that this could be extended with additional tests, please feel free to add any.

@joshuahannan probably asked above about what testing should be done to verify the contract was properly deployed on-chain.

[tarakby]

Thank you @turbolent for clarifying, sorry I missed the core-contract repo tests, which look complete (include the failing tests).

@joshuahannan To make sure contracts are properly deployed, we usually use the https://github.com/onflow/flow-e2e-tests repo. There are already tests for other core-contracts so we should extend those to cover the new contract.

[tarakby]

As an update, I spent some time last week to add proper e2e tests but the repo is currently outdated, and getting it to work with the new flow-go/Cadence will take me some time.
I will need to get back to other priorities for now, and I've created two issues:

• https://github.com/onflow/flow-e2e-tests/issues/61
• https://github.com/onflow/flow-e2e-tests/issues/62
  I can get back to https://github.com/onflow/flow-e2e-tests/issues/62 when I have more time later in the cycle. For now, maybe we could do manual testing to check the contract deployed works?

[joshuahannan]

I just ran all the tests in https://github.com/onflow/flow-core-contracts/blob/master/tests/crypto_test.cdc on testnet and they all passed so it looks like we're good there

[joshuahannan]

We also just deployed to mainnet and the tests all passed there as well! 👍

[tarakby]

Nice, thank you @joshuahannan

[turbolent]

Now that the implementation in Cadence and flow-go is complete, and the contracts are deployed on the chains, this should be complete.