Skip to content

Commit

Permalink
op-deployer: Safety and validation improvements (#12993)
Browse files Browse the repository at this point in the history
- Enables support for deploying tagged versions against new chains, but behind a huge warning that requires user input to bypass. Since our tagged release versions do not contain all implementations, using op-deployer in this way will deploy contracts that haven't been governance approved. Since this workflow is useful for development but bad for prod, I've added support for it but users have to bypass a large warning that describes the risks.
- Validates the hashes of tagged version artifacts after downloading them. This prevents users from downloading tampered versions of the artifacts from GCS.
  • Loading branch information
mslipper authored Nov 20, 2024
1 parent 0bfa930 commit b05ec5a
Show file tree
Hide file tree
Showing 9 changed files with 238 additions and 36 deletions.
51 changes: 49 additions & 2 deletions op-deployer/pkg/deployer/artifacts/downloader.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@ package artifacts
import (
"archive/tar"
"bufio"
"bytes"
"compress/gzip"
"context"
"crypto/sha256"
"errors"
"fmt"
"io"
Expand All @@ -15,6 +17,8 @@ import (
"strings"
"time"

"github.com/ethereum/go-ethereum/common"

"github.com/ethereum-optimism/optimism/op-deployer/pkg/deployer/standard"

"github.com/ethereum/go-ethereum/log"
Expand All @@ -41,15 +45,50 @@ func LogProgressor(lgr log.Logger) DownloadProgressor {
func Download(ctx context.Context, loc *Locator, progress DownloadProgressor) (foundry.StatDirFs, CleanupFunc, error) {
var u *url.URL
var err error
var checker integrityChecker
if loc.IsTag() {
u, err = standard.ArtifactsURLForTag(loc.Tag)
if err != nil {
return nil, nil, fmt.Errorf("failed to get standard artifacts URL for tag %s: %w", loc.Tag, err)
}

hash, err := standard.ArtifactsHashForTag(loc.Tag)
if err != nil {
return nil, nil, fmt.Errorf("failed to get standard artifacts hash for tag %s: %w", loc.Tag, err)
}

checker = &hashIntegrityChecker{hash: hash}
} else {
u = loc.URL
checker = &noopIntegrityChecker{}
}

return downloadURL(ctx, u, progress, checker)
}

type integrityChecker interface {
CheckIntegrity(data []byte) error
}

type hashIntegrityChecker struct {
hash common.Hash
}

func (h *hashIntegrityChecker) CheckIntegrity(data []byte) error {
hash := sha256.Sum256(data)
if hash != h.hash {
return fmt.Errorf("integrity check failed - expected: %x, got: %x", h.hash, hash)
}
return nil
}

type noopIntegrityChecker struct{}

func (noopIntegrityChecker) CheckIntegrity(data []byte) error {
return nil
}

func downloadURL(ctx context.Context, u *url.URL, progress DownloadProgressor, checker integrityChecker) (foundry.StatDirFs, CleanupFunc, error) {
switch u.Scheme {
case "http", "https":
req, err := http.NewRequestWithContext(ctx, http.MethodGet, u.String(), nil)
Expand Down Expand Up @@ -78,7 +117,16 @@ func Download(ctx context.Context, loc *Locator, progress DownloadProgressor) (f
total: resp.ContentLength,
}

gr, err := gzip.NewReader(pr)
data, err := io.ReadAll(pr)
if err != nil {
return nil, nil, fmt.Errorf("failed to read response body: %w", err)
}

if err := checker.CheckIntegrity(data); err != nil {
return nil, nil, fmt.Errorf("failed to check integrity: %w", err)
}

gr, err := gzip.NewReader(bytes.NewReader(data))
if err != nil {
return nil, nil, fmt.Errorf("failed to create gzip reader: %w", err)
}
Expand Down Expand Up @@ -111,7 +159,6 @@ type progressReader struct {
}

func (pr *progressReader) Read(p []byte) (int, error) {

n, err := pr.r.Read(p)
pr.curr += int64(n)
if pr.progress != nil && time.Since(pr.lastPrint) > 1*time.Second {
Expand Down
61 changes: 51 additions & 10 deletions op-deployer/pkg/deployer/artifacts/downloader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,12 @@ import (
"os"
"testing"

"github.com/ethereum/go-ethereum/common"

"github.com/stretchr/testify/require"
)

func TestDownloadArtifacts(t *testing.T) {
func TestDownloadArtifacts_MockArtifacts(t *testing.T) {
f, err := os.OpenFile("testdata/artifacts.tar.gz", os.O_RDONLY, 0o644)
require.NoError(t, err)
defer f.Close()
Expand All @@ -21,6 +23,9 @@ func TestDownloadArtifacts(t *testing.T) {
w.WriteHeader(http.StatusOK)
_, err := io.Copy(w, f)
require.NoError(t, err)
// Seek to beginning of file for next request
_, err = f.Seek(0, 0)
require.NoError(t, err)
}))
defer ts.Close()

Expand All @@ -31,14 +36,50 @@ func TestDownloadArtifacts(t *testing.T) {
URL: artifactsURL,
}

fs, cleanup, err := Download(ctx, loc, nil)
require.NoError(t, err)
require.NotNil(t, fs)
defer func() {
require.NoError(t, cleanup())
}()
t.Run("success", func(t *testing.T) {
fs, cleanup, err := Download(ctx, loc, nil)
require.NoError(t, err)
require.NotNil(t, fs)
defer func() {
require.NoError(t, cleanup())
}()

info, err := fs.Stat("WETH98.sol/WETH98.json")
require.NoError(t, err)
require.Greater(t, info.Size(), int64(0))
info, err := fs.Stat("WETH98.sol/WETH98.json")
require.NoError(t, err)
require.Greater(t, info.Size(), int64(0))
})

t.Run("bad integrity", func(t *testing.T) {
_, _, err := downloadURL(ctx, loc.URL, nil, &hashIntegrityChecker{
hash: common.Hash{'B', 'A', 'D'},
})
require.Error(t, err)
require.ErrorContains(t, err, "integrity check failed")
})

t.Run("ok integrity", func(t *testing.T) {
_, _, err := downloadURL(ctx, loc.URL, nil, &hashIntegrityChecker{
hash: common.HexToHash("0x0f814df0c4293aaaadd468ac37e6c92f0b40fd21df848076835cb2c21d2a516f"),
})
require.NoError(t, err)
})
}

func TestDownloadArtifacts_TaggedVersions(t *testing.T) {
tags := []string{
"op-contracts/v1.6.0",
"op-contracts/v1.7.0-beta.1+l2-contracts",
}
for _, tag := range tags {
t.Run(tag, func(t *testing.T) {
t.Parallel()

loc := MustNewLocatorFromTag(tag)
_, cleanup, err := Download(context.Background(), loc, nil)
t.Cleanup(func() {
require.NoError(t, cleanup())
})
require.NoError(t, err)
})
}
}
20 changes: 17 additions & 3 deletions op-deployer/pkg/deployer/broadcaster/gas_estimator.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,20 @@ import (
var (
// baseFeePadFactor = 50% as a divisor
baseFeePadFactor = big.NewInt(2)
// tipMulFactor = 20 as a multiplier
tipMulFactor = big.NewInt(20)
// tipMulFactor = 5 as a multiplier
tipMulFactor = big.NewInt(5)
// dummyBlobFee is a dummy value for the blob fee. Since this gas estimator will never
// post blobs, it's just set to 1.
dummyBlobFee = big.NewInt(1)
// maxTip is the maximum tip that can be suggested by this estimator.
maxTip = big.NewInt(50 * 1e9)
// minTip is the minimum tip that can be suggested by this estimator.
minTip = big.NewInt(1 * 1e9)
)

// DeployerGasPriceEstimator is a custom gas price estimator for use with op-deployer.
// It pads the base fee by 50% and multiplies the suggested tip by 20.
// It pads the base fee by 50% and multiplies the suggested tip by 5 up to a max of
// 50 gwei.
func DeployerGasPriceEstimator(ctx context.Context, client txmgr.ETHBackend) (*big.Int, *big.Int, *big.Int, error) {
chainHead, err := client.HeaderByNumber(ctx, nil)
if err != nil {
Expand All @@ -34,5 +39,14 @@ func DeployerGasPriceEstimator(ctx context.Context, client txmgr.ETHBackend) (*b
baseFeePad := new(big.Int).Div(chainHead.BaseFee, baseFeePadFactor)
paddedBaseFee := new(big.Int).Add(chainHead.BaseFee, baseFeePad)
paddedTip := new(big.Int).Mul(tip, tipMulFactor)

if paddedTip.Cmp(minTip) < 0 {
paddedTip.Set(minTip)
}

if paddedTip.Cmp(maxTip) > 0 {
paddedTip.Set(maxTip)
}

return paddedTip, paddedBaseFee, dummyBlobFee, nil
}
63 changes: 50 additions & 13 deletions op-deployer/pkg/deployer/integration_test/apply_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,21 +100,38 @@ func TestEndToEndApply(t *testing.T) {
l1Client, err := ethclient.Dial(rpcURL)
require.NoError(t, err)

depKey := new(deployerKey)
pk, err := crypto.HexToECDSA("ac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80")
require.NoError(t, err)

l1ChainID := new(big.Int).SetUint64(defaultL1ChainID)
dk, err := devkeys.NewMnemonicDevKeys(devkeys.TestMnemonic)
require.NoError(t, err)
pk, err := dk.Secret(depKey)
require.NoError(t, err)

l2ChainID1 := uint256.NewInt(1)
l2ChainID2 := uint256.NewInt(2)

loc, _ := testutil.LocalArtifacts(t)
intent, st := newIntent(t, l1ChainID, dk, l2ChainID1, loc, loc)
cg := ethClientCodeGetter(ctx, l1Client)

t.Run("initial chain", func(t *testing.T) {
t.Run("two chains one after another", func(t *testing.T) {
intent, st := newIntent(t, l1ChainID, dk, l2ChainID1, loc, loc)
cg := ethClientCodeGetter(ctx, l1Client)

require.NoError(t, deployer.ApplyPipeline(
ctx,
deployer.ApplyPipelineOpts{
L1RPCUrl: rpcURL,
DeployerPrivateKey: pk,
Intent: intent,
State: st,
Logger: lgr,
StateWriter: pipeline.NoopStateWriter(),
},
))

// create a new environment with wiped state to ensure we can continue using the
// state from the previous deployment
intent.Chains = append(intent.Chains, newChainIntent(t, dk, l1ChainID, l2ChainID2))

require.NoError(t, deployer.ApplyPipeline(
ctx,
deployer.ApplyPipelineOpts{
Expand All @@ -131,10 +148,12 @@ func TestEndToEndApply(t *testing.T) {
validateOPChainDeployment(t, cg, st, intent)
})

t.Run("subsequent chain", func(t *testing.T) {
// create a new environment with wiped state to ensure we can continue using the
// state from the previous deployment
intent.Chains = append(intent.Chains, newChainIntent(t, dk, l1ChainID, l2ChainID2))
t.Run("chain with tagged artifacts", func(t *testing.T) {
intent, st := newIntent(t, l1ChainID, dk, l2ChainID1, loc, loc)
cg := ethClientCodeGetter(ctx, l1Client)

intent.L1ContractsLocator = artifacts.DefaultL1ContractsLocator
intent.L2ContractsLocator = artifacts.DefaultL2ContractsLocator

require.NoError(t, deployer.ApplyPipeline(
ctx,
Expand All @@ -148,6 +167,7 @@ func TestEndToEndApply(t *testing.T) {
},
))

validateSuperchainDeployment(t, st, cg)
validateOPChainDeployment(t, cg, st, intent)
})
}
Expand Down Expand Up @@ -245,9 +265,26 @@ func testApplyExistingOPCM(t *testing.T, l1ChainID uint64, forkRPCUrl string, ve
{"DelayedWETH", releases.DelayedWETH.ImplementationAddress, st.ImplementationsDeployment.DelayedWETHImplAddress},
}
for _, tt := range implTests {
t.Run(tt.name, func(t *testing.T) {
require.Equal(t, tt.expAddr, tt.actAddr)
})
require.Equal(t, tt.expAddr, tt.actAddr, "unexpected address for %s", tt.name)
}

superchain, err := standard.SuperchainFor(l1ChainIDBig.Uint64())
require.NoError(t, err)

managerOwner, err := standard.ManagerOwnerAddrFor(l1ChainIDBig.Uint64())
require.NoError(t, err)

superchainTests := []struct {
name string
expAddr common.Address
actAddr common.Address
}{
{"ProxyAdmin", managerOwner, st.SuperchainDeployment.ProxyAdminAddress},
{"SuperchainConfig", common.Address(*superchain.Config.SuperchainConfigAddr), st.SuperchainDeployment.SuperchainConfigProxyAddress},
{"ProtocolVersions", common.Address(*superchain.Config.ProtocolVersionsAddr), st.SuperchainDeployment.ProtocolVersionsProxyAddress},
}
for _, tt := range superchainTests {
require.Equal(t, tt.expAddr, tt.actAddr, "unexpected address for %s", tt.name)
}

artifactsFSL2, cleanupL2, err := artifacts.Download(
Expand Down
8 changes: 5 additions & 3 deletions op-deployer/pkg/deployer/pipeline/implementations.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,12 @@ func DeployImplementations(env *Env, intent *state.Intent, st *state.State) erro
var err error
if intent.L1ContractsLocator.IsTag() && intent.DeploymentStrategy == state.DeploymentStrategyLive {
standardVersionsTOML, err = standard.L1VersionsDataFor(intent.L1ChainID)
if err != nil {
return fmt.Errorf("error getting standard versions TOML: %w", err)
if err == nil {
contractsRelease = intent.L1ContractsLocator.Tag
} else {
contractsRelease = "dev"
}
contractsRelease = intent.L1ContractsLocator.Tag

} else {
contractsRelease = "dev"
}
Expand Down
Loading

0 comments on commit b05ec5a

Please sign in to comment.