safe-global / safe-singleton-factory

Singleton factory used by Safe related contracts
MIT License
71 stars 72 forks source link

Proposal to Automate Deployment of Safe Singleton Factory using GitHub Actions #530

Open mshakeg opened 3 weeks ago

mshakeg commented 3 weeks ago

I would like to propose an enhancement to the deployment process of the Safe Singleton Factory to new chains. Currently, the process requires intervention from the Safe team, which can introduce delays for deployments on new chains. To streamline this process and ensure timely deployments, I suggest automating the deployment using GitHub Actions.

Current Process:

The current workflow involves manually deploying the factory from the deployer account (0xE1CB04A0fA36DdD16a06ea828007E35e1a3cBC37). Although there is an existing GitHub Action that validates if the deployer account has sufficient funds and marks the issue as ready-to-deploy, the final deployment step still requires manual intervention.

Proposed Enhancement:

To fully automate the deployment process, I propose the following:

  1. Securely Load Mnemonic into GitHub Actions:

    • Store the deployer account's mnemonic as a secret in the GitHub repository.
    • Use this mnemonic within the GitHub Actions workflow to sign and send the deployment transaction.
  2. Enhance the Existing GitHub Action:

    • Extend the current GitHub Action to include steps for deploying the factory contract once an issue is marked as "ready-to-deploy".
    • The deployment step should use the stored mnemonic and the RPC URL to interact with the blockchain and deploy the contract using yarn estimate-compile and yarn submit.

Proposed Workflow:

name: "Validate the issue description and prepare the transaction"
on:
  issues:
    types: [opened, edited]
env:
  ISSUE_BODY: ${{ github.event.issue.body }}
  GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
  NUMBER: ${{ github.event.issue.number }}
  REPO: ${{ github.repository }}
  MNEMONIC: ${{ secrets.MNEMONIC }}

jobs:
  prepareTransaction:
    if: ${{ contains(github.event.issue.labels.*.name, 'new-chain') }}
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v4
        # Existing steps...
      - name: Run the validation script
        run: bash ./.github/scripts/validate_new_chain_request.sh
        # Existing env variables...

      - name: Extract RPC URL from Issue
        id: extract_rpc
        run: |
          rpc_url=$(echo "$ISSUE_BODY" | grep -E -o 'https?://[^ ]+' -m 1 | head -1)
          echo "RPC_URL=${rpc_url}" >> $GITHUB_ENV

      - name: Post Comment to the Issue
        # Existing steps...

  deployFactory:
    if: ${{ github.event.issue.labels.*.name == 'ready-to-deploy' }}
    runs-on: ubuntu-latest
    steps:
      - name: Checkout Repository
        uses: actions/checkout@v4

      - name: Deploy Safe Singleton Factory
        run: |
          yarn estimate-compile
          yarn submit
        env:
          MNEMONIC: ${{ secrets.MNEMONIC }}
          RPC_URL: ${{ steps.extract_rpc.outputs.RPC_URL }}

  alwaysExecuted:
    runs-on: ubuntu-latest
    steps:
      - run: echo "always executed"

By automating the deployment process, we can significantly reduce the time required to deploy the Safe Singleton Factory on new chains, ensuring a more efficient and responsive workflow. Ensure nonce 0 is used for the deployment tx.

FYI GPT4 drafted.

nlordell commented 3 weeks ago

Thank you very much for your proposal! While I agree, it would be nice to have it fully automated, we have been reluctant to upload private keys to GitHub actions and prefer the manual step.

mshakeg commented 3 weeks ago

Hi @nlordell is this particular mnemonic only used for the safe singleton factory? if so then I don't see the concern even if the private key is somehow leaked(which should hopefully never happen if proper access control to github secrets are setup). I just think with a new chain launching every other day, automating this will save you guys a bunch of time.

mmv08 commented 3 weeks ago

if so then I don't see the concern

if the key is leaked then it will be impossible to deploy safe contracts to canonical addresses to the chain, since the nonce will be used and the resulting factory address will be different.

mshakeg commented 3 weeks ago

@mmv08 agreed, what I'm getting at is I don't see what's exceptional about storing the key in gh secrets vs locally on devs machine, why is it more likely to be leaked in the former relative to the latter?

In fact if devs store the private key in plain text unencrypted then gh secrets is probably way more secure.

nlordell commented 3 weeks ago

In fact if devs store the private key in plain text unencrypted

Don't worry, we don't :stuck_out_tongue:.

why is it more likely to be leaked in the former relative to the latter?

I don't think it is more likely to be leaked, it is just increasing the probability of leaking the secret by adding a new system that has access to it. Also, there are some different attack vectors that need to be considered when using secrets in GH actions, and as @mmv08 mentioned this secret cannot be rotated if it is leaked.

Note that we have some proposals on how to make a completely permissionless CREATE2 deployer that will hopefully be made possible with EIP-7702: https://ethereum-magicians.org/t/eip-7702-set-eoa-account-code-for-one-transaction/19923/161?u=nlordell

mshakeg commented 3 weeks ago

@nlordell

Don't worry, we don't 😛.

Nice, what hardhat extension are y'all using to securely manage keys?

wrt EIP-7702, it sounds promising, but it also sounds like it'll only work on EVM chains that supports the proposed tx type, not sure how widely adopted this will be, but at least initially it won't work for almost all EVM chains, so we'll still have to rely on this factory to deploy canonical instances of Safe.

nlordell commented 3 weeks ago

Nice, what hardhat extension are y'all using to securely manage keys?

We use 1Password CLI in our flows.

but it also sounds like it'll only work on EVM chains that supports the proposed tx type

True :disappointed: - hoping 7702 takes off though :stuck_out_tongue:.