diff --git a/.changeset/good-clocks-relax.md b/.changeset/good-clocks-relax.md new file mode 100644 index 000000000000..daef47e2ef90 --- /dev/null +++ b/.changeset/good-clocks-relax.md @@ -0,0 +1,5 @@ +--- +'@eth-optimism/data-transport-layer': patch +--- + +Add better logging to DTL about shutoff block diff --git a/.changeset/olive-students-love.md b/.changeset/olive-students-love.md new file mode 100644 index 000000000000..694ca99b9537 --- /dev/null +++ b/.changeset/olive-students-love.md @@ -0,0 +1,5 @@ +--- +'@eth-optimism/contracts-bedrock': patch +--- + +Makes the Proxy contract inheritable by making functions (public virtual). diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index d3fd36188bf8..2172e08cfb2a 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -31,12 +31,13 @@ /op-proposer @ethereum-optimism/go-reviewers /op-program @ethereum-optimism/go-reviewers /op-service @ethereum-optimism/go-reviewers +/ops-bedrock @ethereum-optimism/go-reviewers + # Ops /.circleci @ethereum-optimism/infra-reviewers /.github @ethereum-optimism/infra-reviewers /ops @ethereum-optimism/infra-reviewers -/ops-bedrock @ethereum-optimism/infra-reviewers /op-signer @ethereum-optimism/infra-reviewers # Misc diff --git a/.github/mergify.yml b/.github/mergify.yml index 7d996c1edc80..76296581af90 100644 --- a/.github/mergify.yml +++ b/.github/mergify.yml @@ -29,6 +29,7 @@ pull_request_rules: queue: name: default method: merge + merge_bot_account: OptimismBot - name: Add merge train label conditions: - "queue-position >= 0" diff --git a/.gitmodules b/.gitmodules index 8802b2259132..e398bb691620 100644 --- a/.gitmodules +++ b/.gitmodules @@ -1,3 +1,8 @@ [submodule "tests"] path = l2geth/tests/testdata url = https://github.com/ethereum/tests +[submodule "packages/contracts-periphery/lib/multicall"] + path = packages/contracts-periphery/lib/multicall + url = https://github.com/mds1/multicall +[submodule "lib/multicall"] + branch = v3.1.0 diff --git a/README.md b/README.md index 12984d86916f..767fce6e619a 100644 --- a/README.md +++ b/README.md @@ -24,7 +24,7 @@ If you want to build Optimism, check out the [Protocol Specs](./specs/). ## Community -General discussion happens most frequently on the [Optimism discord](https://discord-gateway.optimism.io). +General discussion happens most frequently on the [Optimism discord](https://discord.gg/optimism). Governance discussion can also be found on the [Optimism Governance Forum](https://gov.optimism.io/). ## Contributing diff --git a/docs/op-stack/README.md b/docs/op-stack/README.md index 169f69f314ce..bc67b46d9f1b 100644 --- a/docs/op-stack/README.md +++ b/docs/op-stack/README.md @@ -1,6 +1,6 @@ # The OP Stack Docs -[![Discord](https://img.shields.io/discord/667044843901681675.svg?color=768AD4&label=discord&logo=https%3A%2F%2Fdiscordapp.com%2Fassets%2F8c9701b98ad4372b58f13fd9f65f966e.svg)](https://discord-gateway.optimism.io) +[![Discord](https://img.shields.io/discord/667044843901681675.svg?color=768AD4&label=discord&logo=https%3A%2F%2Fdiscordapp.com%2Fassets%2F8c9701b98ad4372b58f13fd9f65f966e.svg)](https://discord.gg/optimism) [![Twitter Follow](https://img.shields.io/twitter/follow/optimismPBC.svg?label=optimismPBC&style=social)](https://twitter.com/optimismPBC) The OP Stack is an open, collectively maintained development stack for blockchain ecosystems. diff --git a/docs/op-stack/src/.vuepress/theme/components/Anchor.js b/docs/op-stack/src/.vuepress/theme/components/Anchor.js index f05faf8cd342..a4e30894b9c9 100644 --- a/docs/op-stack/src/.vuepress/theme/components/Anchor.js +++ b/docs/op-stack/src/.vuepress/theme/components/Anchor.js @@ -57,7 +57,7 @@ export default Vue.extend({ "Support" ]), h("div", { class: "anchor-support-links" }, [ - h("a", { attrs: { href: "https://discord.optimism.io", target: "_blank" } }, [ + h("a", { attrs: { href: "https://discord.gg/optimism", target: "_blank" } }, [ h("div", [ h("i", { attrs: { class: "fab fa-discord" } }), " Discord community " diff --git a/docs/op-stack/src/docs/contribute.md b/docs/op-stack/src/docs/contribute.md index 8a5d41a5a2cb..b97f9d68947b 100644 --- a/docs/op-stack/src/docs/contribute.md +++ b/docs/op-stack/src/docs/contribute.md @@ -30,4 +30,4 @@ If you’re looking for other ways to get involved, here are a few options: - Grab an idea from the [project ideas list](https://github.com/ethereum-optimism/optimism-project-ideas) to and building - Suggest a new idea for the [project ideas list](https://github.com/ethereum-optimism/optimism-project-ideas) - Improve the [Optimism Community Hub](https://community.optimism.io/) [documentation](https://github.com/ethereum-optimism/community-hub) or [tutorials](https://github.com/ethereum-optimism/optimism-tutorial) -- Become an Optimism Ambassador, Support Nerd, and more in the [Optimism Discord](https://discord-gateway.optimism.io/) +- Become an Optimism Ambassador, Support Nerd, and more in the [Optimism Discord](https://discord.gg/optimism) diff --git a/go.mod b/go.mod index 94804df9ac87..61699a09bca4 100644 --- a/go.mod +++ b/go.mod @@ -6,7 +6,7 @@ require ( github.com/btcsuite/btcd v0.23.3 github.com/btcsuite/btcd/chaincfg/chainhash v1.0.1 github.com/decred/dcrd/dcrec/secp256k1/v4 v4.1.0 - github.com/docker/docker v20.10.21+incompatible + github.com/docker/docker v20.10.24+incompatible github.com/docker/go-connections v0.4.0 github.com/ethereum-optimism/go-ethereum-hdwallet v0.1.3 github.com/ethereum/go-ethereum v1.11.5 diff --git a/go.sum b/go.sum index 8b4ea79a580f..397437aaac3a 100644 --- a/go.sum +++ b/go.sum @@ -158,8 +158,8 @@ github.com/dgryski/go-farm v0.0.0-20190423205320-6a90982ecee2/go.mod h1:SqUrOPUn github.com/dlclark/regexp2 v1.7.0 h1:7lJfhqlPssTb1WQx4yvTHN0uElPEv52sbaECrAQxjAo= github.com/docker/distribution v2.8.1+incompatible h1:Q50tZOPR6T/hjNsyc9g8/syEs6bk8XXApsHjKukMl68= github.com/docker/distribution v2.8.1+incompatible/go.mod h1:J2gT2udsDAN96Uj4KfcMRqY0/ypR+oyYUYmja8H+y+w= -github.com/docker/docker v20.10.21+incompatible h1:UTLdBmHk3bEY+w8qeO5KttOhy6OmXWsl/FEet9Uswog= -github.com/docker/docker v20.10.21+incompatible/go.mod h1:eEKB0N0r5NX/I1kEveEz05bcu8tLC/8azJZsviup8Sk= +github.com/docker/docker v20.10.24+incompatible h1:Ugvxm7a8+Gz6vqQYQQ2W7GYq5EUPaAiuPgIfVyI3dYE= +github.com/docker/docker v20.10.24+incompatible/go.mod h1:eEKB0N0r5NX/I1kEveEz05bcu8tLC/8azJZsviup8Sk= github.com/docker/go-connections v0.4.0 h1:El9xVISelRB7BuFusrZozjnkIM5YnzCViNKohAFqRJQ= github.com/docker/go-connections v0.4.0/go.mod h1:Gbd7IOopHjR8Iph03tsViu4nIes5XhDvyHbTtUxmeec= github.com/docker/go-units v0.4.0/go.mod h1:fgPhTUdO+D/Jk86RDLlptpiXQzgHJF7gydDDbaIK4Dk= diff --git a/op-batcher/batcher/batch_submitter.go b/op-batcher/batcher/batch_submitter.go index 991e71fb0f91..1dd27aa905d3 100644 --- a/op-batcher/batcher/batch_submitter.go +++ b/op-batcher/batcher/batch_submitter.go @@ -51,9 +51,10 @@ func Main(version string, cliCtx *cli.Context) error { return err } } - defer batchSubmitter.StopIfRunning(context.Background()) ctx, cancel := context.WithCancel(context.Background()) + defer cancel() // Stop pprof and metrics only after main loop returns + defer batchSubmitter.StopIfRunning(context.Background()) pprofConfig := cfg.PprofConfig if pprofConfig.Enabled { @@ -106,7 +107,8 @@ func Main(version string, cliCtx *cli.Context) error { syscall.SIGQUIT, }...) <-interruptChannel - cancel() - _ = server.Stop() + if err := server.Stop(); err != nil { + l.Error("Error shutting down http server: %w", err) + } return nil } diff --git a/op-chain-ops/cmd/rollover/main.go b/op-chain-ops/cmd/rollover/main.go index c2510b7db7da..8c6a541c8037 100644 --- a/op-chain-ops/cmd/rollover/main.go +++ b/op-chain-ops/cmd/rollover/main.go @@ -9,21 +9,25 @@ import ( "sync" "time" - "github.com/ethereum/go-ethereum/common/hexutil" + "github.com/mattn/go-isatty" + "github.com/urfave/cli/v2" + + "github.com/ethereum-optimism/optimism/op-chain-ops/util" "github.com/ethereum-optimism/optimism/op-bindings/bindings" legacy_bindings "github.com/ethereum-optimism/optimism/op-bindings/legacy-bindings" - "github.com/ethereum/go-ethereum/ethclient" - "github.com/ethereum-optimism/optimism/op-chain-ops/util" "github.com/ethereum/go-ethereum/accounts/abi/bind" "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/common/hexutil" + "github.com/ethereum/go-ethereum/ethclient" "github.com/ethereum/go-ethereum/log" "github.com/ethereum/go-ethereum/rpc" - "github.com/urfave/cli/v2" ) func main() { + log.Root().SetHandler(log.StreamHandler(os.Stderr, log.TerminalFormat(isatty.IsTerminal(os.Stderr.Fd())))) + app := cli.NewApp() app.Name = "rollover" app.Usage = "Commands for assisting in the rollover of the system" @@ -149,6 +153,9 @@ func main() { return err } log.Info("Remaining deposits that must be submitted", "count", finalPending) + if finalPending.Cmp(common.Big0) == 0 { + log.Info("All deposits have been batch submitted") + } return nil }, }, @@ -183,11 +190,11 @@ func main() { log.Info("Waiting for CanonicalTransactionChain") wg.Add(1) - go waitForTotalElements(&wg, ctc, clients.L2Client) + go waitForTotalElements(&wg, ctc, clients.L2Client, "CanonicalTransactionChain") log.Info("Waiting for StateCommitmentChain") wg.Add(1) - go waitForTotalElements(&wg, scc, clients.L2Client) + go waitForTotalElements(&wg, scc, clients.L2Client, "StateCommitmentChain") wg.Wait() log.Info("All batches have been submitted") @@ -210,7 +217,7 @@ type RollupContract interface { } // waitForTotalElements will poll to see -func waitForTotalElements(wg *sync.WaitGroup, contract RollupContract, client *ethclient.Client) { +func waitForTotalElements(wg *sync.WaitGroup, contract RollupContract, client *ethclient.Client, name string) { defer wg.Done() for { @@ -228,9 +235,16 @@ func waitForTotalElements(wg *sync.WaitGroup, contract RollupContract, client *e } if totalElements.Uint64() == bn { + log.Info("Total elements matches block number", "name", name, "count", bn) return } - log.Info("Waiting for elements to be submitted", "count", totalElements.Uint64()-bn, "height", bn, "total-elements", totalElements.Uint64()) + log.Info( + "Waiting for elements to be submitted", + "name", name, + "count", totalElements.Uint64()-bn, + "height", bn, + "total-elements", totalElements.Uint64(), + ) time.Sleep(3 * time.Second) } diff --git a/op-chain-ops/util/util.go b/op-chain-ops/util/util.go index f82d5b625def..f164743bf27f 100644 --- a/op-chain-ops/util/util.go +++ b/op-chain-ops/util/util.go @@ -39,21 +39,21 @@ func NewClients(ctx *cli.Context) (*Clients, error) { l1RpcURL := ctx.String("l1-rpc-url") l1Client, err := ethclient.Dial(l1RpcURL) if err != nil { - return nil, err + return nil, fmt.Errorf("cannot dial L1: %w", err) } l1ChainID, err := l1Client.ChainID(context.Background()) if err != nil { - return nil, err + return nil, fmt.Errorf("cannot fetch L1 chainid: %w", err) } l2RpcURL := ctx.String("l2-rpc-url") l2Client, err := ethclient.Dial(l2RpcURL) if err != nil { - return nil, err + return nil, fmt.Errorf("cannot dial L2: %w", err) } l2ChainID, err := l2Client.ChainID(context.Background()) if err != nil { - return nil, err + return nil, fmt.Errorf("cannot fetch L2 chainid: %w", err) } l1RpcClient, err := rpc.DialContext(context.Background(), l1RpcURL) diff --git a/op-e2e/actions/l2_batcher_test.go b/op-e2e/actions/l2_batcher_test.go index d5c16342e392..bd7072e9bff5 100644 --- a/op-e2e/actions/l2_batcher_test.go +++ b/op-e2e/actions/l2_batcher_test.go @@ -447,7 +447,7 @@ func TestBigL2Txs(gt *testing.T) { require.NoError(t, err) gas, err := core.IntrinsicGas(data, nil, false, true, true, false) require.NoError(t, err) - if gas > engine.l2GasPool.Gas() { + if gas > engine.engineApi.RemainingBlockGas() { break } tx := types.MustSignNewTx(dp.Secrets.Alice, signer, &types.DynamicFeeTx{ diff --git a/op-e2e/actions/l2_engine.go b/op-e2e/actions/l2_engine.go index a0c9de6950c9..f5c2b422f594 100644 --- a/op-e2e/actions/l2_engine.go +++ b/op-e2e/actions/l2_engine.go @@ -3,12 +3,12 @@ package actions import ( "errors" + "github.com/ethereum-optimism/optimism/op-e2e/e2eutils" + "github.com/ethereum-optimism/optimism/op-program/l2/engineapi" "github.com/stretchr/testify/require" - "github.com/ethereum/go-ethereum/beacon/engine" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/core" - "github.com/ethereum/go-ethereum/core/state" "github.com/ethereum/go-ethereum/core/types" geth "github.com/ethereum/go-ethereum/eth" "github.com/ethereum/go-ethereum/eth/ethconfig" @@ -38,22 +38,10 @@ type L2Engine struct { rollupGenesis *rollup.Genesis // L2 evm / chain - l2Chain *core.BlockChain - l2Database ethdb.Database - l2Cfg *core.Genesis - l2Signer types.Signer - - // L2 block building data - l2BuildingHeader *types.Header // block header that we add txs to for block building - l2BuildingState *state.StateDB // state used for block building - l2GasPool *core.GasPool // track gas used of ongoing building - pendingIndices map[common.Address]uint64 // per account, how many txs from the pool were already included in the block, since the pool is lagging behind block mining. - l2Transactions []*types.Transaction // collects txs that were successfully included into current block build - l2Receipts []*types.Receipt // collect receipts of ongoing building - l2ForceEmpty bool // when no additional txs may be processed (i.e. when sequencer drift runs out) - l2TxFailed []*types.Transaction // log of failed transactions which could not be included - - payloadID engine.PayloadID // ID of payload that is currently being built + l2Chain *core.BlockChain + l2Signer types.Signer + + engineApi *engineapi.L2EngineAPI failL2RPC error // mock error } @@ -61,6 +49,38 @@ type L2Engine struct { type EngineOption func(ethCfg *ethconfig.Config, nodeCfg *node.Config) error func NewL2Engine(t Testing, log log.Logger, genesis *core.Genesis, rollupGenesisL1 eth.BlockID, jwtPath string, options ...EngineOption) *L2Engine { + n, ethBackend, apiBackend := newBackend(t, genesis, jwtPath, options) + engineApi := engineapi.NewL2EngineAPI(log, apiBackend) + chain := ethBackend.BlockChain() + genesisBlock := chain.Genesis() + eng := &L2Engine{ + log: log, + node: n, + eth: ethBackend, + rollupGenesis: &rollup.Genesis{ + L1: rollupGenesisL1, + L2: eth.BlockID{Hash: genesisBlock.Hash(), Number: genesisBlock.NumberU64()}, + L2Time: genesis.Timestamp, + }, + l2Chain: chain, + l2Signer: types.LatestSigner(genesis.Config), + engineApi: engineApi, + } + // register the custom engine API, so we can serve engine requests while having more control + // over sequencing of individual txs. + n.RegisterAPIs([]rpc.API{ + { + Namespace: "engine", + Service: eng.engineApi, + Authenticated: true, + }, + }) + require.NoError(t, n.Start(), "failed to start L2 op-geth node") + + return eng +} + +func newBackend(t e2eutils.TestingBase, genesis *core.Genesis, jwtPath string, options []EngineOption) (*node.Node, *geth.Ethereum, *engineApiBackend) { ethCfg := ðconfig.Config{ NetworkId: genesis.Config.ChainID.Uint64(), Genesis: genesis, @@ -89,33 +109,26 @@ func NewL2Engine(t Testing, log log.Logger, genesis *core.Genesis, rollupGenesis chain := backend.BlockChain() db := backend.ChainDb() - genesisBlock := chain.Genesis() - eng := &L2Engine{ - log: log, - node: n, - eth: backend, - rollupGenesis: &rollup.Genesis{ - L1: rollupGenesisL1, - L2: eth.BlockID{Hash: genesisBlock.Hash(), Number: genesisBlock.NumberU64()}, - L2Time: genesis.Timestamp, - }, - l2Chain: chain, - l2Database: db, - l2Cfg: genesis, - l2Signer: types.LatestSigner(genesis.Config), + apiBackend := &engineApiBackend{ + BlockChain: chain, + db: db, + genesis: genesis, } - // register the custom engine API, so we can serve engine requests while having more control - // over sequencing of individual txs. - n.RegisterAPIs([]rpc.API{ - { - Namespace: "engine", - Service: (*L2EngineAPI)(eng), - Authenticated: true, - }, - }) - require.NoError(t, n.Start(), "failed to start L2 op-geth node") + return n, backend, apiBackend +} - return eng +type engineApiBackend struct { + *core.BlockChain + db ethdb.Database + genesis *core.Genesis +} + +func (e *engineApiBackend) Database() ethdb.Database { + return e.db +} + +func (e *engineApiBackend) Genesis() *core.Genesis { + return e.genesis } func (s *L2Engine) EthClient() *ethclient.Client { @@ -158,39 +171,25 @@ func (e *L2Engine) ActL2RPCFail(t Testing) { // ActL2IncludeTx includes the next transaction from the given address in the block that is being built func (e *L2Engine) ActL2IncludeTx(from common.Address) Action { return func(t Testing) { - if e.l2BuildingHeader == nil { - t.InvalidAction("not currently building a block, cannot include tx from queue") - return - } - if e.l2ForceEmpty { + if e.engineApi.ForcedEmpty() { e.log.Info("Skipping including a transaction because e.L2ForceEmpty is true") - // t.InvalidAction("cannot include any sequencer txs") return } - i := e.pendingIndices[from] + i := e.engineApi.PendingIndices(from) txs, q := e.eth.TxPool().ContentFrom(from) if uint64(len(txs)) <= i { t.Fatalf("no pending txs from %s, and have %d unprocessable queued txs from this account", from, len(q)) } tx := txs[i] - if tx.Gas() > e.l2BuildingHeader.GasLimit { - t.Fatalf("tx consumes %d gas, more than available in L2 block %d", tx.Gas(), e.l2BuildingHeader.GasLimit) - } - if tx.Gas() > uint64(*e.l2GasPool) { - t.InvalidAction("action takes too much gas: %d, only have %d", tx.Gas(), uint64(*e.l2GasPool)) - return - } - e.pendingIndices[from] = i + 1 // won't retry the tx - e.l2BuildingState.SetTxContext(tx.Hash(), len(e.l2Transactions)) - receipt, err := core.ApplyTransaction(e.l2Cfg.Config, e.l2Chain, &e.l2BuildingHeader.Coinbase, - e.l2GasPool, e.l2BuildingState, e.l2BuildingHeader, tx, &e.l2BuildingHeader.GasUsed, *e.l2Chain.GetVMConfig()) - if err != nil { - e.l2TxFailed = append(e.l2TxFailed, tx) - t.Fatalf("failed to apply transaction to L2 block (tx %d): %v", len(e.l2Transactions), err) + err := e.engineApi.IncludeTx(tx, from) + if errors.Is(err, engineapi.ErrNotBuildingBlock) { + t.InvalidAction(err.Error()) + } else if errors.Is(err, engineapi.ErrUsesTooMuchGas) { + t.InvalidAction("included tx uses too much gas: %v", err) + } else if err != nil { + t.Fatalf("include tx: %v", err) } - e.l2Receipts = append(e.l2Receipts, receipt) - e.l2Transactions = append(e.l2Transactions, tx) } } diff --git a/op-e2e/actions/l2_engine_test.go b/op-e2e/actions/l2_engine_test.go index 5418d10b11e1..535a74e46b97 100644 --- a/op-e2e/actions/l2_engine_test.go +++ b/op-e2e/actions/l2_engine_test.go @@ -4,6 +4,8 @@ import ( "math/big" "testing" + "github.com/ethereum-optimism/optimism/op-program/l2/engineapi" + "github.com/ethereum-optimism/optimism/op-program/l2/engineapi/test" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/consensus/beacon" "github.com/ethereum/go-ethereum/consensus/ethash" @@ -187,3 +189,15 @@ func TestL2EngineAPIFail(gt *testing.T) { require.NoError(t, err) require.Equal(gt, sd.L2Cfg.ToBlock().Hash(), head.Hash(), "expecting engine to start at genesis") } + +func TestEngineAPITests(t *testing.T) { + test.RunEngineAPITests(t, func() engineapi.EngineBackend { + jwtPath := e2eutils.WriteDefaultJWT(t) + dp := e2eutils.MakeDeployParams(t, defaultRollupTestParams) + sd := e2eutils.Setup(t, dp, defaultAlloc) + n, _, apiBackend := newBackend(t, sd.L2Cfg, jwtPath, nil) + err := n.Start() + require.NoError(t, err) + return apiBackend + }) +} diff --git a/op-e2e/actions/l2_sequencer_test.go b/op-e2e/actions/l2_sequencer_test.go index 5a62617d730d..449ef463eb9b 100644 --- a/op-e2e/actions/l2_sequencer_test.go +++ b/op-e2e/actions/l2_sequencer_test.go @@ -98,7 +98,7 @@ func TestL2Sequencer_SequencerDrift(gt *testing.T) { // We passed the sequencer drift: we can still keep the old origin, but can't include any txs sequencer.ActL2KeepL1Origin(t) sequencer.ActL2StartBlock(t) - require.True(t, engine.l2ForceEmpty, "engine should not be allowed to include anything after sequencer drift is surpassed") + require.True(t, engine.engineApi.ForcedEmpty(), "engine should not be allowed to include anything after sequencer drift is surpassed") } // TestL2Sequencer_SequencerOnlyReorg regression-tests a Goerli halt where the sequencer diff --git a/op-node/p2p/sync.go b/op-node/p2p/sync.go index f6a4d050b8da..b3e06a951e96 100644 --- a/op-node/p2p/sync.go +++ b/op-node/p2p/sync.go @@ -202,11 +202,18 @@ type SyncClient struct { results chan syncResult + receivePayload receivePayloadFn + + // resource context: all peers and mainLoop tasks inherit this, and start shutting down once resCancel() is called. resCtx context.Context resCancel context.CancelFunc - receivePayload receivePayloadFn - wg sync.WaitGroup + // wait group: wait for the resources to close. Adding to this is only safe if the peersLock is held. + wg sync.WaitGroup + + // Don't allow anything to be added to the wait-group while, or after, we are shutting down. + // This is protected by peersLock. + closingPeers bool } func NewSyncClient(log log.Logger, cfg *rollup.Config, newStream newStreamFn, rcv receivePayloadFn, metrics SyncClientMetrics) *SyncClient { @@ -239,7 +246,9 @@ func NewSyncClient(log log.Logger, cfg *rollup.Config, newStream newStreamFn, rc } func (s *SyncClient) Start() { + s.peersLock.Lock() s.wg.Add(1) + s.peersLock.Unlock() go s.mainLoop() } @@ -250,6 +259,9 @@ func (s *SyncClient) AddPeer(id peer.ID) { s.log.Warn("cannot register peer for sync duties, peer was already registered", "peer", id) return } + if s.closingPeers { + return + } s.wg.Add(1) // add new peer routine ctx, cancel := context.WithCancel(s.resCtx) @@ -269,7 +281,12 @@ func (s *SyncClient) RemovePeer(id peer.ID) { delete(s.peers, id) } +// Close will shut down the sync client and all attached work, and block until shutdown is complete. +// This will block if the Start() has not created the main background loop. func (s *SyncClient) Close() error { + s.peersLock.Lock() + s.closingPeers = true + s.peersLock.Unlock() s.resCancel() s.wg.Wait() return nil diff --git a/op-node/p2p/sync_test.go b/op-node/p2p/sync_test.go index 68fd38c21e38..288419889657 100644 --- a/op-node/p2p/sync_test.go +++ b/op-node/p2p/sync_test.go @@ -2,10 +2,8 @@ package p2p import ( "context" - "math" "math/big" "testing" - "time" "github.com/libp2p/go-libp2p/core/host" "github.com/libp2p/go-libp2p/core/network" @@ -13,13 +11,14 @@ import ( mocknet "github.com/libp2p/go-libp2p/p2p/net/mock" "github.com/stretchr/testify/require" + "github.com/ethereum/go-ethereum" + "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/log" + "github.com/ethereum-optimism/optimism/op-node/eth" "github.com/ethereum-optimism/optimism/op-node/metrics" "github.com/ethereum-optimism/optimism/op-node/rollup" "github.com/ethereum-optimism/optimism/op-node/testlog" - "github.com/ethereum/go-ethereum" - "github.com/ethereum/go-ethereum/common" - "github.com/ethereum/go-ethereum/log" ) type mockPayloadFn func(n uint64) (*eth.ExecutionPayload, error) @@ -120,18 +119,12 @@ func TestSinglePeerSync(t *testing.T) { require.NoError(t, cl.RequestL2Range(ctx, l2Ref(10), l2Ref(20))) // and wait for the sync results to come in (in reverse order) - receiveCtx, receiveCancel := context.WithTimeout(ctx, time.Second*5) - defer receiveCancel() for i := uint64(19); i > 10; i-- { - select { - case p := <-received: - require.Equal(t, uint64(p.BlockNumber), i, "expecting payloads in order") - exp, ok := payloads[uint64(p.BlockNumber)] - require.True(t, ok, "expecting known payload") - require.Equal(t, exp.BlockHash, p.BlockHash, "expecting the correct payload") - case <-receiveCtx.Done(): - t.Fatal("did not receive all expected payloads within expected time") - } + p := <-received + require.Equal(t, uint64(p.BlockNumber), i, "expecting payloads in order") + exp, ok := payloads[uint64(p.BlockNumber)] + require.True(t, ok, "expecting known payload") + require.Equal(t, exp.BlockHash, p.BlockHash, "expecting the correct payload") } } @@ -202,34 +195,20 @@ func TestMultiPeerSync(t *testing.T) { // With such large range to request we are going to hit the rate-limits of B and C, // but that means we'll balance the work between the peers. - - // wait for the results to come in, based on the expected rate limit, divided by 2 (because we have 2 servers), with a buffer of 2 seconds - receiveCtx, receiveCancel := context.WithTimeout(ctx, time.Second*time.Duration(math.Ceil(float64((89-10)/peerServerBlocksRateLimit)))/2+time.Second*2) - defer receiveCancel() - for i := uint64(89); i > 10; i-- { - select { - case p := <-recvA: - exp, ok := payloads[uint64(p.BlockNumber)] - require.True(t, ok, "expecting known payload") - require.Equal(t, exp.BlockHash, p.BlockHash, "expecting the correct payload") - case <-receiveCtx.Done(): - t.Fatal("did not receive all expected payloads within expected time") - } - } + p := <-recvA + exp, ok := payloads[uint64(p.BlockNumber)] + require.True(t, ok, "expecting known payload") + require.Equal(t, exp.BlockHash, p.BlockHash, "expecting the correct payload") // now see if B can sync a range, and fill the gap with a re-request bl25 := payloads[25] // temporarily remove it from the available payloads. This will create a gap delete(payloads, uint64(25)) require.NoError(t, clB.RequestL2Range(ctx, l2Ref(20), l2Ref(30))) for i := uint64(29); i > 25; i-- { - select { - case p := <-recvB: - exp, ok := payloads[uint64(p.BlockNumber)] - require.True(t, ok, "expecting known payload") - require.Equal(t, exp.BlockHash, p.BlockHash, "expecting the correct payload") - case <-receiveCtx.Done(): - t.Fatal("did not receive all expected payloads within expected time") - } + p := <-recvB + exp, ok := payloads[uint64(p.BlockNumber)] + require.True(t, ok, "expecting known payload") + require.Equal(t, exp.BlockHash, p.BlockHash, "expecting the correct payload") } // the request for 25 should fail. See: // server: WARN peer requested unknown block by number num=25 @@ -239,16 +218,11 @@ func TestMultiPeerSync(t *testing.T) { payloads[25] = bl25 // And request a range again, 25 is there now, and 21-24 should follow quickly (some may already have been fetched and wait in quarantine) require.NoError(t, clB.RequestL2Range(ctx, l2Ref(20), l2Ref(26))) - receiveCtx, receiveCancel = context.WithTimeout(ctx, time.Second*10) - defer receiveCancel() + for i := uint64(25); i > 20; i-- { - select { - case p := <-recvB: - exp, ok := payloads[uint64(p.BlockNumber)] - require.True(t, ok, "expecting known payload") - require.Equal(t, exp.BlockHash, p.BlockHash, "expecting the correct payload") - case <-receiveCtx.Done(): - t.Fatal("did not receive all expected payloads within expected time") - } + p := <-recvB + exp, ok := payloads[uint64(p.BlockNumber)] + require.True(t, ok, "expecting known payload") + require.Equal(t, exp.BlockHash, p.BlockHash, "expecting the correct payload") } } diff --git a/op-e2e/actions/l2_engine_api.go b/op-program/l2/engineapi/l2_engine_api.go similarity index 63% rename from op-e2e/actions/l2_engine_api.go rename to op-program/l2/engineapi/l2_engine_api.go index 1555f775edea..77c42ba9da16 100644 --- a/op-e2e/actions/l2_engine_api.go +++ b/op-program/l2/engineapi/l2_engine_api.go @@ -1,4 +1,4 @@ -package actions +package engineapi import ( "context" @@ -11,26 +11,74 @@ import ( "github.com/ethereum/go-ethereum/beacon/engine" "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/consensus" "github.com/ethereum/go-ethereum/consensus/misc" "github.com/ethereum/go-ethereum/core" - "github.com/ethereum/go-ethereum/core/rawdb" "github.com/ethereum/go-ethereum/core/state" "github.com/ethereum/go-ethereum/core/types" + "github.com/ethereum/go-ethereum/core/vm" "github.com/ethereum/go-ethereum/log" + "github.com/ethereum/go-ethereum/params" "github.com/ethereum/go-ethereum/trie" "github.com/ethereum-optimism/optimism/op-node/eth" ) +type EngineBackend interface { + CurrentBlock() *types.Header + CurrentSafeBlock() *types.Header + CurrentFinalBlock() *types.Header + GetHeaderByHash(hash common.Hash) *types.Header + GetBlockByHash(hash common.Hash) *types.Block + GetBlock(hash common.Hash, number uint64) *types.Block + // GetHeader returns the header corresponding to the hash/number argument pair. + GetHeader(common.Hash, uint64) *types.Header + HasBlockAndState(hash common.Hash, number uint64) bool + GetCanonicalHash(n uint64) common.Hash + + GetVMConfig() *vm.Config + Config() *params.ChainConfig + // Engine retrieves the chain's consensus engine. + Engine() consensus.Engine + + StateAt(root common.Hash) (*state.StateDB, error) + + InsertBlockWithoutSetHead(block *types.Block) error + SetCanonical(head *types.Block) (common.Hash, error) + SetFinalized(header *types.Header) + SetSafe(header *types.Header) +} + // L2EngineAPI wraps an engine actor, and implements the RPC backend required to serve the engine API. // This re-implements some of the Geth API work, but changes the API backend so we can deterministically // build and control the L2 block contents to reach very specific edge cases as desired for testing. -type L2EngineAPI L2Engine +type L2EngineAPI struct { + log log.Logger + backend EngineBackend + + // L2 block building data + l2BuildingHeader *types.Header // block header that we add txs to for block building + l2BuildingState *state.StateDB // state used for block building + l2GasPool *core.GasPool // track gas used of ongoing building + pendingIndices map[common.Address]uint64 // per account, how many txs from the pool were already included in the block, since the pool is lagging behind block mining. + l2Transactions []*types.Transaction // collects txs that were successfully included into current block build + l2Receipts []*types.Receipt // collect receipts of ongoing building + l2ForceEmpty bool // when no additional txs may be processed (i.e. when sequencer drift runs out) + l2TxFailed []*types.Transaction // log of failed transactions which could not be included + + payloadID engine.PayloadID // ID of payload that is currently being built +} + +func NewL2EngineAPI(log log.Logger, backend EngineBackend) *L2EngineAPI { + return &L2EngineAPI{ + log: log, + backend: backend, + } +} var ( - STATUS_INVALID = ð.ForkchoiceUpdatedResult{PayloadStatus: eth.PayloadStatusV1{Status: eth.ExecutionInvalid}, PayloadID: nil} - STATUS_SYNCING = ð.ForkchoiceUpdatedResult{PayloadStatus: eth.PayloadStatusV1{Status: eth.ExecutionSyncing}, PayloadID: nil} - INVALID_TERMINAL_BLOCK = eth.PayloadStatusV1{Status: eth.ExecutionInvalid, LatestValidHash: &common.Hash{}} + STATUS_INVALID = ð.ForkchoiceUpdatedResult{PayloadStatus: eth.PayloadStatusV1{Status: eth.ExecutionInvalid}, PayloadID: nil} + STATUS_SYNCING = ð.ForkchoiceUpdatedResult{PayloadStatus: eth.PayloadStatusV1{Status: eth.ExecutionSyncing}, PayloadID: nil} ) // computePayloadId computes a pseudo-random payloadid, based on the parameters. @@ -53,16 +101,64 @@ func computePayloadId(headBlockHash common.Hash, params *eth.PayloadAttributes) return out } +func (ea *L2EngineAPI) RemainingBlockGas() uint64 { + return ea.l2GasPool.Gas() +} + +func (ea *L2EngineAPI) ForcedEmpty() bool { + return ea.l2ForceEmpty +} + +func (ea *L2EngineAPI) PendingIndices(from common.Address) uint64 { + return ea.pendingIndices[from] +} + +var ( + ErrNotBuildingBlock = errors.New("not currently building a block, cannot include tx from queue") + ErrExceedsGasLimit = errors.New("tx gas exceeds block gas limit") + ErrUsesTooMuchGas = errors.New("action takes too much gas") +) + +func (ea *L2EngineAPI) IncludeTx(tx *types.Transaction, from common.Address) error { + if ea.l2BuildingHeader == nil { + return ErrNotBuildingBlock + } + if ea.l2ForceEmpty { + ea.log.Info("Skipping including a transaction because e.L2ForceEmpty is true") + // t.InvalidAction("cannot include any sequencer txs") + return nil + } + + if tx.Gas() > ea.l2BuildingHeader.GasLimit { + return fmt.Errorf("%w tx gas: %d, block gas limit: %d", ErrExceedsGasLimit, tx.Gas(), ea.l2BuildingHeader.GasLimit) + } + if tx.Gas() > uint64(*ea.l2GasPool) { + return fmt.Errorf("%w: %d, only have %d", ErrUsesTooMuchGas, tx.Gas(), uint64(*ea.l2GasPool)) + } + + ea.pendingIndices[from] = ea.pendingIndices[from] + 1 // won't retry the tx + ea.l2BuildingState.SetTxContext(tx.Hash(), len(ea.l2Transactions)) + receipt, err := core.ApplyTransaction(ea.backend.Config(), ea.backend, &ea.l2BuildingHeader.Coinbase, + ea.l2GasPool, ea.l2BuildingState, ea.l2BuildingHeader, tx, &ea.l2BuildingHeader.GasUsed, *ea.backend.GetVMConfig()) + if err != nil { + ea.l2TxFailed = append(ea.l2TxFailed, tx) + return fmt.Errorf("invalid L2 block (tx %d): %w", len(ea.l2Transactions), err) + } + ea.l2Receipts = append(ea.l2Receipts, receipt) + ea.l2Transactions = append(ea.l2Transactions, tx) + return nil +} + func (ea *L2EngineAPI) startBlock(parent common.Hash, params *eth.PayloadAttributes) error { if ea.l2BuildingHeader != nil { ea.log.Warn("started building new block without ending previous block", "previous", ea.l2BuildingHeader, "prev_payload_id", ea.payloadID) } - parentHeader := ea.l2Chain.GetHeaderByHash(parent) + parentHeader := ea.backend.GetHeaderByHash(parent) if parentHeader == nil { return fmt.Errorf("uknown parent block: %s", parent) } - statedb, err := state.New(parentHeader.Root, state.NewDatabase(ea.l2Database), nil) + statedb, err := ea.backend.StateAt(parentHeader.Root) if err != nil { return fmt.Errorf("failed to init state db around block %s (state %s): %w", parent, parentHeader.Root, err) } @@ -78,7 +174,7 @@ func (ea *L2EngineAPI) startBlock(parent common.Hash, params *eth.PayloadAttribu MixDigest: common.Hash(params.PrevRandao), } - header.BaseFee = misc.CalcBaseFee(ea.l2Cfg.Config, parentHeader) + header.BaseFee = misc.CalcBaseFee(ea.backend.Config(), parentHeader) ea.l2BuildingHeader = header ea.l2BuildingState = statedb @@ -96,8 +192,8 @@ func (ea *L2EngineAPI) startBlock(parent common.Hash, params *eth.PayloadAttribu return fmt.Errorf("transaction %d is not valid: %w", i, err) } ea.l2BuildingState.SetTxContext(tx.Hash(), i) - receipt, err := core.ApplyTransaction(ea.l2Cfg.Config, ea.l2Chain, &ea.l2BuildingHeader.Coinbase, - ea.l2GasPool, ea.l2BuildingState, ea.l2BuildingHeader, &tx, &ea.l2BuildingHeader.GasUsed, *ea.l2Chain.GetVMConfig()) + receipt, err := core.ApplyTransaction(ea.backend.Config(), ea.backend, &ea.l2BuildingHeader.Coinbase, + ea.l2GasPool, ea.l2BuildingState, ea.l2BuildingHeader, &tx, &ea.l2BuildingHeader.GasUsed, *ea.backend.GetVMConfig()) if err != nil { ea.l2TxFailed = append(ea.l2TxFailed, &tx) return fmt.Errorf("failed to apply deposit transaction to L2 block (tx %d): %w", i, err) @@ -116,17 +212,8 @@ func (ea *L2EngineAPI) endBlock() (*types.Block, error) { ea.l2BuildingHeader = nil header.GasUsed = header.GasLimit - uint64(*ea.l2GasPool) - header.Root = ea.l2BuildingState.IntermediateRoot(ea.l2Cfg.Config.IsEIP158(header.Number)) + header.Root = ea.l2BuildingState.IntermediateRoot(ea.backend.Config().IsEIP158(header.Number)) block := types.NewBlock(header, ea.l2Transactions, nil, ea.l2Receipts, trie.NewStackTrie(nil)) - - // Write state changes to db - root, err := ea.l2BuildingState.Commit(ea.l2Cfg.Config.IsEIP158(header.Number)) - if err != nil { - return nil, fmt.Errorf("l2 state write error: %w", err) - } - if err := ea.l2BuildingState.Database().TrieDB().Commit(root, false); err != nil { - return nil, fmt.Errorf("l2 trie write error: %w", err) - } return block, nil } @@ -153,31 +240,16 @@ func (ea *L2EngineAPI) ForkchoiceUpdatedV1(ctx context.Context, state *eth.Forkc // Check whether we have the block yet in our database or not. If not, we'll // need to either trigger a sync, or to reject this forkchoice update for a // reason. - block := ea.l2Chain.GetBlockByHash(state.HeadBlockHash) + block := ea.backend.GetBlockByHash(state.HeadBlockHash) if block == nil { // TODO: syncing not supported yet return STATUS_SYNCING, nil } // Block is known locally, just sanity check that the beacon client does not // attempt to push us back to before the merge. - if block.Difficulty().BitLen() > 0 || block.NumberU64() == 0 { - var ( - td = ea.l2Chain.GetTd(state.HeadBlockHash, block.NumberU64()) - ptd = ea.l2Chain.GetTd(block.ParentHash(), block.NumberU64()-1) - ttd = ea.l2Chain.Config().TerminalTotalDifficulty - ) - if td == nil || (block.NumberU64() > 0 && ptd == nil) { - ea.log.Error("TDs unavailable for TTD check", "number", block.NumberU64(), "hash", state.HeadBlockHash, "td", td, "parent", block.ParentHash(), "ptd", ptd) - return STATUS_INVALID, errors.New("TDs unavailable for TDD check") - } - if td.Cmp(ttd) < 0 { - ea.log.Error("Refusing beacon update to pre-merge", "number", block.NumberU64(), "hash", state.HeadBlockHash, "diff", block.Difficulty(), "age", common.PrettyAge(time.Unix(int64(block.Time()), 0))) - return ð.ForkchoiceUpdatedResult{PayloadStatus: INVALID_TERMINAL_BLOCK, PayloadID: nil}, nil - } - if block.NumberU64() > 0 && ptd.Cmp(ttd) >= 0 { - ea.log.Error("Parent block is already post-ttd", "number", block.NumberU64(), "hash", state.HeadBlockHash, "diff", block.Difficulty(), "age", common.PrettyAge(time.Unix(int64(block.Time()), 0))) - return ð.ForkchoiceUpdatedResult{PayloadStatus: INVALID_TERMINAL_BLOCK, PayloadID: nil}, nil - } + // Note: Differs from op-geth implementation as pre-merge blocks are never supported here + if block.Difficulty().BitLen() > 0 { + return STATUS_INVALID, errors.New("pre-merge blocks not supported") } valid := func(id *engine.PayloadID) *eth.ForkchoiceUpdatedResult { return ð.ForkchoiceUpdatedResult{ @@ -185,16 +257,16 @@ func (ea *L2EngineAPI) ForkchoiceUpdatedV1(ctx context.Context, state *eth.Forkc PayloadID: id, } } - if rawdb.ReadCanonicalHash(ea.l2Database, block.NumberU64()) != state.HeadBlockHash { + if ea.backend.GetCanonicalHash(block.NumberU64()) != state.HeadBlockHash { // Block is not canonical, set head. - if latestValid, err := ea.l2Chain.SetCanonical(block); err != nil { + if latestValid, err := ea.backend.SetCanonical(block); err != nil { return ð.ForkchoiceUpdatedResult{PayloadStatus: eth.PayloadStatusV1{Status: eth.ExecutionInvalid, LatestValidHash: &latestValid}}, err } - } else if ea.l2Chain.CurrentBlock().Hash() == state.HeadBlockHash { + } else if ea.backend.CurrentBlock().Hash() == state.HeadBlockHash { // If the specified head matches with our local head, do nothing and keep // generating the payload. It's a special corner case that a few slots are // missing and we are requested to generate the payload in slot. - } else if ea.l2Chain.Config().Optimism == nil { // minor L2Engine API divergence: allow proposers to reorg their own chain + } else if ea.backend.Config().Optimism == nil { // minor L2Engine API divergence: allow proposers to reorg their own chain panic("engine not configured as optimism engine") } @@ -202,30 +274,30 @@ func (ea *L2EngineAPI) ForkchoiceUpdatedV1(ctx context.Context, state *eth.Forkc // chain final and completely in PoS mode. if state.FinalizedBlockHash != (common.Hash{}) { // If the finalized block is not in our canonical tree, somethings wrong - finalHeader := ea.l2Chain.GetHeaderByHash(state.FinalizedBlockHash) + finalHeader := ea.backend.GetHeaderByHash(state.FinalizedBlockHash) if finalHeader == nil { ea.log.Warn("Final block not available in database", "hash", state.FinalizedBlockHash) return STATUS_INVALID, engine.InvalidForkChoiceState.With(errors.New("final block not available in database")) - } else if rawdb.ReadCanonicalHash(ea.l2Database, finalHeader.Number.Uint64()) != state.FinalizedBlockHash { + } else if ea.backend.GetCanonicalHash(finalHeader.Number.Uint64()) != state.FinalizedBlockHash { ea.log.Warn("Final block not in canonical chain", "number", block.NumberU64(), "hash", state.HeadBlockHash) return STATUS_INVALID, engine.InvalidForkChoiceState.With(errors.New("final block not in canonical chain")) } // Set the finalized block - ea.l2Chain.SetFinalized(finalHeader) + ea.backend.SetFinalized(finalHeader) } // Check if the safe block hash is in our canonical tree, if not somethings wrong if state.SafeBlockHash != (common.Hash{}) { - safeHeader := ea.l2Chain.GetHeaderByHash(state.SafeBlockHash) + safeHeader := ea.backend.GetHeaderByHash(state.SafeBlockHash) if safeHeader == nil { ea.log.Warn("Safe block not available in database") return STATUS_INVALID, engine.InvalidForkChoiceState.With(errors.New("safe block not available in database")) } - if rawdb.ReadCanonicalHash(ea.l2Database, safeHeader.Number.Uint64()) != state.SafeBlockHash { + if ea.backend.GetCanonicalHash(safeHeader.Number.Uint64()) != state.SafeBlockHash { ea.log.Warn("Safe block not in canonical chain") return STATUS_INVALID, engine.InvalidForkChoiceState.With(errors.New("safe block not in canonical chain")) } // Set the safe block - ea.l2Chain.SetSafe(safeHeader) + ea.backend.SetSafe(safeHeader) } // If payload generation was requested, create a new block to be potentially // sealed by the beacon client. The payload will be requested later, and we @@ -270,7 +342,7 @@ func (ea *L2EngineAPI) NewPayloadV1(ctx context.Context, payload *eth.ExecutionP } // If we already have the block locally, ignore the entire execution and just // return a fake success. - if block := ea.l2Chain.GetBlockByHash(payload.BlockHash); block != nil { + if block := ea.backend.GetBlockByHash(payload.BlockHash); block != nil { ea.log.Warn("Ignoring already known beacon payload", "number", payload.BlockNumber, "hash", payload.BlockHash, "age", common.PrettyAge(time.Unix(int64(block.Time()), 0))) hash := block.Hash() return ð.PayloadStatusV1{Status: eth.ExecutionValid, LatestValidHash: &hash}, nil @@ -278,16 +350,23 @@ func (ea *L2EngineAPI) NewPayloadV1(ctx context.Context, payload *eth.ExecutionP // TODO: skipping invalid ancestor check (i.e. not remembering previously failed blocks) - parent := ea.l2Chain.GetBlock(block.ParentHash(), block.NumberU64()-1) + parent := ea.backend.GetBlock(block.ParentHash(), block.NumberU64()-1) if parent == nil { // TODO: hack, saying we accepted if we don't know the parent block. Might want to return critical error if we can't actually sync. return ð.PayloadStatusV1{Status: eth.ExecutionAccepted, LatestValidHash: nil}, nil } - if !ea.l2Chain.HasBlockAndState(block.ParentHash(), block.NumberU64()-1) { + + if block.Time() <= parent.Time() { + log.Warn("Invalid timestamp", "parent", block.Time(), "block", block.Time()) + return ea.invalid(errors.New("invalid timestamp"), parent.Header()), nil + } + + if !ea.backend.HasBlockAndState(block.ParentHash(), block.NumberU64()-1) { ea.log.Warn("State not available, ignoring new payload") return ð.PayloadStatusV1{Status: eth.ExecutionAccepted}, nil } - if err := ea.l2Chain.InsertBlockWithoutSetHead(block); err != nil { + log.Trace("Inserting block without sethead", "hash", block.Hash(), "number", block.Number) + if err := ea.backend.InsertBlockWithoutSetHead(block); err != nil { ea.log.Warn("NewPayloadV1: inserting block failed", "error", err) // TODO not remembering the payload as invalid return ea.invalid(err, parent.Header()), nil @@ -297,7 +376,7 @@ func (ea *L2EngineAPI) NewPayloadV1(ctx context.Context, payload *eth.ExecutionP } func (ea *L2EngineAPI) invalid(err error, latestValid *types.Header) *eth.PayloadStatusV1 { - currentHash := ea.l2Chain.CurrentBlock().Hash() + currentHash := ea.backend.CurrentBlock().Hash() if latestValid != nil { // Set latest valid hash to 0x0 if parent is PoW block currentHash = common.Hash{} diff --git a/op-program/l2/engineapi/test/l2_engine_api_tests.go b/op-program/l2/engineapi/test/l2_engine_api_tests.go new file mode 100644 index 000000000000..2d910cde5ad0 --- /dev/null +++ b/op-program/l2/engineapi/test/l2_engine_api_tests.go @@ -0,0 +1,299 @@ +package test + +import ( + "context" + "testing" + + "github.com/ethereum-optimism/optimism/op-node/eth" + "github.com/ethereum-optimism/optimism/op-node/rollup/derive" + "github.com/ethereum-optimism/optimism/op-node/testlog" + "github.com/ethereum-optimism/optimism/op-program/l2/engineapi" + "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/core/types" + "github.com/ethereum/go-ethereum/log" + "github.com/stretchr/testify/require" +) + +var gasLimit = eth.Uint64Quantity(30_000_000) +var feeRecipient = common.Address{} + +func RunEngineAPITests(t *testing.T, createBackend func() engineapi.EngineBackend) { + t.Run("CreateBlock", func(t *testing.T) { + api := newTestHelper(t, createBackend) + + block := api.addBlock() + api.assert.Equal(block.BlockHash, api.headHash(), "should create and import new block") + }) + + t.Run("IncludeRequiredTransactions", func(t *testing.T) { + api := newTestHelper(t, createBackend) + genesis := api.backend.CurrentBlock() + + txData, err := derive.L1InfoDeposit(1, eth.HeaderBlockInfo(genesis), eth.SystemConfig{}, true) + api.assert.NoError(err) + tx := types.NewTx(txData) + block := api.addBlock(tx) + api.assert.Equal(block.BlockHash, api.headHash(), "should create and import new block") + imported := api.backend.GetBlockByHash(block.BlockHash) + api.assert.Len(imported.Transactions(), 1, "should include transaction") + }) + + t.Run("IgnoreUpdateHeadToOlderBlock", func(t *testing.T) { + api := newTestHelper(t, createBackend) + genesisHash := api.headHash() + api.addBlock() + block := api.addBlock() + api.assert.Equal(block.BlockHash, api.headHash(), "should have extended chain") + + api.forkChoiceUpdated(genesisHash, genesisHash, genesisHash) + api.assert.Equal(block.BlockHash, api.headHash(), "should not have reset chain head") + }) + + t.Run("AllowBuildingOnOlderBlock", func(t *testing.T) { + api := newTestHelper(t, createBackend) + genesis := api.backend.CurrentBlock() + api.addBlock() + block := api.addBlock() + api.assert.Equal(block.BlockHash, api.headHash(), "should have extended chain") + + payloadID := api.startBlockBuilding(genesis, eth.Uint64Quantity(genesis.Time+3)) + api.assert.Equal(block.BlockHash, api.headHash(), "should not reset chain head when building starts") + + payload := api.getPayload(payloadID) + api.assert.Equal(genesis.Hash(), payload.ParentHash, "should have old block as parent") + + api.newPayload(payload) + api.forkChoiceUpdated(payload.BlockHash, genesis.Hash(), genesis.Hash()) + api.assert.Equal(payload.BlockHash, api.headHash(), "should reorg to block built on old parent") + }) + + t.Run("RejectInvalidBlockHash", func(t *testing.T) { + api := newTestHelper(t, createBackend) + + // Invalid because BlockHash won't be correct (among many other reasons) + block := ð.ExecutionPayload{} + r, err := api.engine.NewPayloadV1(api.ctx, block) + api.assert.NoError(err) + api.assert.Equal(eth.ExecutionInvalidBlockHash, r.Status) + }) + + t.Run("RejectBlockWithInvalidStateTransition", func(t *testing.T) { + api := newTestHelper(t, createBackend) + genesis := api.backend.CurrentBlock() + + // Build a valid block + payloadID := api.startBlockBuilding(genesis, eth.Uint64Quantity(genesis.Time+2)) + newBlock := api.getPayload(payloadID) + + // But then make it invalid by changing the state root + newBlock.StateRoot = eth.Bytes32(genesis.TxHash) + updateBlockHash(newBlock) + + r, err := api.engine.NewPayloadV1(api.ctx, newBlock) + api.assert.NoError(err) + api.assert.Equal(eth.ExecutionInvalid, r.Status) + }) + + t.Run("RejectBlockWithSameTimeAsParent", func(t *testing.T) { + api := newTestHelper(t, createBackend) + genesis := api.backend.CurrentBlock() + + payloadID := api.startBlockBuilding(genesis, eth.Uint64Quantity(genesis.Time)) + newBlock := api.getPayload(payloadID) + + r, err := api.engine.NewPayloadV1(api.ctx, newBlock) + api.assert.NoError(err) + api.assert.Equal(eth.ExecutionInvalid, r.Status) + }) + + t.Run("RejectBlockWithTimeBeforeParent", func(t *testing.T) { + api := newTestHelper(t, createBackend) + genesis := api.backend.CurrentBlock() + + payloadID := api.startBlockBuilding(genesis, eth.Uint64Quantity(genesis.Time-1)) + newBlock := api.getPayload(payloadID) + + r, err := api.engine.NewPayloadV1(api.ctx, newBlock) + api.assert.NoError(err) + api.assert.Equal(eth.ExecutionInvalid, r.Status) + }) + + t.Run("UpdateSafeAndFinalizedHead", func(t *testing.T) { + api := newTestHelper(t, createBackend) + + finalized := api.addBlock() + safe := api.addBlock() + head := api.addBlock() + + api.forkChoiceUpdated(head.BlockHash, safe.BlockHash, finalized.BlockHash) + api.assert.Equal(head.BlockHash, api.headHash(), "should update head block") + api.assert.Equal(safe.BlockHash, api.safeHash(), "should update safe block") + api.assert.Equal(finalized.BlockHash, api.finalHash(), "should update finalized block") + }) + + t.Run("RejectSafeHeadWhenNotAncestor", func(t *testing.T) { + api := newTestHelper(t, createBackend) + genesis := api.backend.CurrentBlock() + + api.addBlock() + chainA2 := api.addBlock() + chainA3 := api.addBlock() + + chainB1 := api.addBlockWithParent(genesis, eth.Uint64Quantity(genesis.Time+3)) + + result, err := api.engine.ForkchoiceUpdatedV1(api.ctx, ð.ForkchoiceState{ + HeadBlockHash: chainA3.BlockHash, + SafeBlockHash: chainB1.BlockHash, + FinalizedBlockHash: chainA2.BlockHash, + }, nil) + api.assert.ErrorContains(err, "Invalid forkchoice state", "should return error from forkChoiceUpdated") + api.assert.Equal(eth.ExecutionInvalid, result.PayloadStatus.Status, "forkChoiceUpdated should return invalid") + api.assert.Nil(result.PayloadID, "should not provide payload ID when invalid") + }) + + t.Run("RejectFinalizedHeadWhenNotAncestor", func(t *testing.T) { + api := newTestHelper(t, createBackend) + genesis := api.backend.CurrentBlock() + + api.addBlock() + chainA2 := api.addBlock() + chainA3 := api.addBlock() + + chainB1 := api.addBlockWithParent(genesis, eth.Uint64Quantity(genesis.Time+3)) + + result, err := api.engine.ForkchoiceUpdatedV1(api.ctx, ð.ForkchoiceState{ + HeadBlockHash: chainA3.BlockHash, + SafeBlockHash: chainA2.BlockHash, + FinalizedBlockHash: chainB1.BlockHash, + }, nil) + api.assert.ErrorContains(err, "Invalid forkchoice state", "should return error from forkChoiceUpdated") + api.assert.Equal(eth.ExecutionInvalid, result.PayloadStatus.Status, "forkChoiceUpdated should return invalid") + api.assert.Nil(result.PayloadID, "should not provide payload ID when invalid") + }) +} + +// Updates the block hash to the expected value based on the other fields in the payload +func updateBlockHash(newBlock *eth.ExecutionPayload) { + // And fix up the block hash + newHash, _ := newBlock.CheckBlockHash() + newBlock.BlockHash = newHash +} + +type testHelper struct { + t *testing.T + ctx context.Context + engine *engineapi.L2EngineAPI + backend engineapi.EngineBackend + assert *require.Assertions +} + +func newTestHelper(t *testing.T, createBackend func() engineapi.EngineBackend) *testHelper { + logger := testlog.Logger(t, log.LvlDebug) + ctx := context.Background() + backend := createBackend() + api := engineapi.NewL2EngineAPI(logger, backend) + test := &testHelper{ + t: t, + ctx: ctx, + engine: api, + backend: backend, + assert: require.New(t), + } + return test +} + +func (h *testHelper) headHash() common.Hash { + return h.backend.CurrentBlock().Hash() +} + +func (h *testHelper) safeHash() common.Hash { + return h.backend.CurrentSafeBlock().Hash() +} + +func (h *testHelper) finalHash() common.Hash { + return h.backend.CurrentFinalBlock().Hash() +} + +func (h *testHelper) Log(args ...any) { + h.t.Log(args...) +} + +func (h *testHelper) addBlock(txs ...*types.Transaction) *eth.ExecutionPayload { + head := h.backend.CurrentBlock() + return h.addBlockWithParent(head, eth.Uint64Quantity(head.Time+2), txs...) +} + +func (h *testHelper) addBlockWithParent(head *types.Header, timestamp eth.Uint64Quantity, txs ...*types.Transaction) *eth.ExecutionPayload { + prevHead := h.backend.CurrentBlock() + id := h.startBlockBuilding(head, timestamp, txs...) + + block := h.getPayload(id) + h.assert.Equal(timestamp, block.Timestamp, "should create block with correct timestamp") + h.assert.Equal(head.Hash(), block.ParentHash, "should have correct parent") + h.assert.Len(block.Transactions, len(txs)) + + h.newPayload(block) + + // Should not have changed the chain head yet + h.assert.Equal(prevHead, h.backend.CurrentBlock()) + + h.forkChoiceUpdated(block.BlockHash, head.Hash(), head.Hash()) + h.assert.Equal(block.BlockHash, h.backend.CurrentBlock().Hash()) + return block +} + +func (h *testHelper) forkChoiceUpdated(head common.Hash, safe common.Hash, finalized common.Hash) { + h.Log("forkChoiceUpdated", "head", head, "safe", safe, "finalized", finalized) + result, err := h.engine.ForkchoiceUpdatedV1(h.ctx, ð.ForkchoiceState{ + HeadBlockHash: head, + SafeBlockHash: safe, + FinalizedBlockHash: finalized, + }, nil) + h.assert.NoError(err) + h.assert.Equal(eth.ExecutionValid, result.PayloadStatus.Status, "forkChoiceUpdated should return valid") + h.assert.Nil(result.PayloadStatus.ValidationError, "should not have validation error when valid") + h.assert.Nil(result.PayloadID, "should not provide payload ID when block building not requested") +} + +func (h *testHelper) startBlockBuilding(head *types.Header, newBlockTimestamp eth.Uint64Quantity, txs ...*types.Transaction) *eth.PayloadID { + h.Log("Start block building", "head", head.Hash(), "timestamp", newBlockTimestamp) + var txData []eth.Data + for _, tx := range txs { + rlp, err := tx.MarshalBinary() + h.assert.NoError(err, "Failed to marshall tx %v", tx) + txData = append(txData, rlp) + } + result, err := h.engine.ForkchoiceUpdatedV1(h.ctx, ð.ForkchoiceState{ + HeadBlockHash: head.Hash(), + SafeBlockHash: head.Hash(), + FinalizedBlockHash: head.Hash(), + }, ð.PayloadAttributes{ + Timestamp: newBlockTimestamp, + PrevRandao: eth.Bytes32(head.MixDigest), + SuggestedFeeRecipient: feeRecipient, + Transactions: txData, + NoTxPool: true, + GasLimit: &gasLimit, + }) + h.assert.NoError(err) + h.assert.Equal(eth.ExecutionValid, result.PayloadStatus.Status) + id := result.PayloadID + h.assert.NotNil(id) + return id +} + +func (h *testHelper) getPayload(id *eth.PayloadID) *eth.ExecutionPayload { + h.Log("getPayload", "id", id) + block, err := h.engine.GetPayloadV1(h.ctx, *id) + h.assert.NoError(err) + h.assert.NotNil(block) + return block +} + +func (h *testHelper) newPayload(block *eth.ExecutionPayload) { + h.Log("newPayload", "hash", block.BlockHash) + r, err := h.engine.NewPayloadV1(h.ctx, block) + h.assert.NoError(err) + h.assert.Equal(eth.ExecutionValid, r.Status) + h.assert.Nil(r.ValidationError) +} diff --git a/op-service/metrics/event.go b/op-service/metrics/event.go index 8589bf654d6a..78143727189e 100644 --- a/op-service/metrics/event.go +++ b/op-service/metrics/event.go @@ -52,7 +52,6 @@ func NewEventVec(factory Factory, ns string, name string, displayName string, la Namespace: ns, Name: fmt.Sprintf("last_%s_unix", name), Help: fmt.Sprintf("Timestamp of last %s event", displayName), - }, - labelNames), + }, labelNames), } } diff --git a/op-service/txmgr/metrics/noop.go b/op-service/txmgr/metrics/noop.go index b08e5f20bd43..ee77357579d3 100644 --- a/op-service/txmgr/metrics/noop.go +++ b/op-service/txmgr/metrics/noop.go @@ -4,4 +4,9 @@ import "github.com/ethereum/go-ethereum/core/types" type NoopTxMetrics struct{} -func (*NoopTxMetrics) RecordL1GasFee(*types.Receipt) {} +func (*NoopTxMetrics) RecordNonce(uint64) {} +func (*NoopTxMetrics) RecordGasBumpCount(int) {} +func (*NoopTxMetrics) RecordTxConfirmationLatency(int64) {} +func (*NoopTxMetrics) TxConfirmed(*types.Receipt) {} +func (*NoopTxMetrics) TxPublished(string) {} +func (*NoopTxMetrics) RPCError() {} diff --git a/op-service/txmgr/metrics/tx_metrics.go b/op-service/txmgr/metrics/tx_metrics.go index 6201e13a9b3c..2ae036b33f81 100644 --- a/op-service/txmgr/metrics/tx_metrics.go +++ b/op-service/txmgr/metrics/tx_metrics.go @@ -9,11 +9,34 @@ import ( ) type TxMetricer interface { - RecordL1GasFee(receipt *types.Receipt) + RecordGasBumpCount(int) + RecordTxConfirmationLatency(int64) + RecordNonce(uint64) + TxConfirmed(*types.Receipt) + TxPublished(string) + RPCError() } type TxMetrics struct { - TxL1GasFee prometheus.Gauge + TxL1GasFee prometheus.Gauge + TxGasBump prometheus.Gauge + LatencyConfirmedTx prometheus.Gauge + currentNonce prometheus.Gauge + txPublishError *prometheus.CounterVec + publishEvent metrics.Event + confirmEvent metrics.EventVec + rpcError prometheus.Counter +} + +func receiptStatusString(receipt *types.Receipt) string { + switch receipt.Status { + case types.ReceiptStatusSuccessful: + return "success" + case types.ReceiptStatusFailed: + return "failed" + default: + return "unknown_status" + } } var _ TxMetricer = (*TxMetrics)(nil) @@ -26,9 +49,67 @@ func MakeTxMetrics(ns string, factory metrics.Factory) TxMetrics { Help: "L1 gas fee for transactions in GWEI", Subsystem: "txmgr", }), + TxGasBump: factory.NewGauge(prometheus.GaugeOpts{ + Namespace: ns, + Name: "tx_gas_bump", + Help: "Number of times a transaction gas needed to be bumped before it got included", + Subsystem: "txmgr", + }), + LatencyConfirmedTx: factory.NewGauge(prometheus.GaugeOpts{ + Namespace: ns, + Name: "tx_confirmed_latency_ms", + Help: "Latency of a confirmed transaction in milliseconds", + Subsystem: "txmgr", + }), + currentNonce: factory.NewGauge(prometheus.GaugeOpts{ + Namespace: ns, + Name: "current_nonce", + Help: "Current nonce of the from address", + Subsystem: "txmgr", + }), + txPublishError: factory.NewCounterVec(prometheus.CounterOpts{ + Namespace: ns, + Name: "tx_publish_error_count", + Help: "Count of publish errors. Labells are sanitized error strings", + Subsystem: "txmgr", + }, []string{"error"}), + confirmEvent: metrics.NewEventVec(factory, ns, "confirm", "tx confirm", []string{"status"}), + publishEvent: metrics.NewEvent(factory, ns, "publish", "tx publish"), + rpcError: factory.NewCounter(prometheus.CounterOpts{ + Namespace: ns, + Name: "rpc_error_count", + Help: "Temporrary: Count of RPC errors (like timeouts) that have occurrred", + Subsystem: "txmgr", + }), } } -func (t *TxMetrics) RecordL1GasFee(receipt *types.Receipt) { +func (t *TxMetrics) RecordNonce(nonce uint64) { + t.currentNonce.Set(float64(nonce)) +} + +// TxConfirmed records lots of information about the confirmed transaction +func (t *TxMetrics) TxConfirmed(receipt *types.Receipt) { + t.confirmEvent.Record(receiptStatusString(receipt)) t.TxL1GasFee.Set(float64(receipt.EffectiveGasPrice.Uint64() * receipt.GasUsed / params.GWei)) } + +func (t *TxMetrics) RecordGasBumpCount(times int) { + t.TxGasBump.Set(float64(times)) +} + +func (t *TxMetrics) RecordTxConfirmationLatency(latency int64) { + t.LatencyConfirmedTx.Set(float64(latency)) +} + +func (t *TxMetrics) TxPublished(errString string) { + if errString != "" { + t.txPublishError.WithLabelValues(errString).Inc() + } else { + t.publishEvent.Record() + } +} + +func (t *TxMetrics) RPCError() { + t.rpcError.Inc() +} diff --git a/op-service/txmgr/txmgr.go b/op-service/txmgr/txmgr.go index 90de074e931e..37711506cbbf 100644 --- a/op-service/txmgr/txmgr.go +++ b/op-service/txmgr/txmgr.go @@ -148,6 +148,7 @@ func (m *SimpleTxManager) Send(ctx context.Context, candidate TxCandidate) (*typ func (m *SimpleTxManager) craftTx(ctx context.Context, candidate TxCandidate) (*types.Transaction, error) { gasTipCap, basefee, err := m.suggestGasPriceCaps(ctx) if err != nil { + m.metr.RPCError() return nil, fmt.Errorf("failed to get gas price info: %w", err) } gasFeeCap := calcGasFeeCap(basefee, gasTipCap) @@ -157,8 +158,10 @@ func (m *SimpleTxManager) craftTx(ctx context.Context, candidate TxCandidate) (* defer cancel() nonce, err := m.backend.NonceAt(childCtx, m.cfg.From, nil) if err != nil { + m.metr.RPCError() return nil, fmt.Errorf("failed to get nonce: %w", err) } + m.metr.RecordNonce(nonce) rawTx := &types.DynamicFeeTx{ ChainID: m.chainID, @@ -216,6 +219,7 @@ func (m *SimpleTxManager) send(ctx context.Context, tx *types.Transaction) (*typ ticker := time.NewTicker(m.cfg.ResubmissionTimeout) defer ticker.Stop() + bumpCounter := 0 for { select { case <-ticker.C: @@ -231,12 +235,15 @@ func (m *SimpleTxManager) send(ctx context.Context, tx *types.Transaction) (*typ // Increase the gas price & submit the new transaction tx = m.increaseGasPrice(ctx, tx) wg.Add(1) + bumpCounter += 1 go sendTxAsync(tx) case <-ctx.Done(): return nil, ctx.Err() case receipt := <-receiptChan: + m.metr.RecordGasBumpCount(bumpCounter) + m.metr.TxConfirmed(receipt) return receipt, nil } } @@ -251,6 +258,7 @@ func (m *SimpleTxManager) publishAndWaitForTx(ctx context.Context, tx *types.Tra cCtx, cancel := context.WithTimeout(ctx, m.cfg.NetworkTimeout) defer cancel() + t := time.Now() err := m.backend.SendTransaction(cCtx, tx) sendState.ProcessSendError(err) @@ -259,19 +267,28 @@ func (m *SimpleTxManager) publishAndWaitForTx(ctx context.Context, tx *types.Tra switch { case errStringMatch(err, core.ErrNonceTooLow): log.Warn("nonce too low", "err", err) + m.metr.TxPublished("nonce_to_low") case errStringMatch(err, context.Canceled): + m.metr.RPCError() log.Warn("transaction send cancelled", "err", err) + m.metr.TxPublished("context_cancelled") case errStringMatch(err, txpool.ErrAlreadyKnown): log.Warn("resubmitted already known transaction", "err", err) + m.metr.TxPublished("tx_already_known") case errStringMatch(err, txpool.ErrReplaceUnderpriced): log.Warn("transaction replacement is underpriced", "err", err) + m.metr.TxPublished("tx_replacement_underpriced") case errStringMatch(err, txpool.ErrUnderpriced): log.Warn("transaction is underpriced", "err", err) + m.metr.TxPublished("tx_underpriced") default: + m.metr.RPCError() log.Error("unable to publish transaction", "err", err) + m.metr.TxPublished("unknown_error") } return } + m.metr.TxPublished("") log.Info("Transaction successfully published") // Poll for the transaction to be ready & then send the result to receiptChan @@ -282,7 +299,7 @@ func (m *SimpleTxManager) publishAndWaitForTx(ctx context.Context, tx *types.Tra } select { case receiptChan <- receipt: - m.metr.RecordL1GasFee(receipt) + m.metr.RecordTxConfirmationLatency(time.Since(t).Milliseconds()) default: } } @@ -314,9 +331,11 @@ func (m *SimpleTxManager) queryReceipt(ctx context.Context, txHash common.Hash, m.l.Trace("Transaction not yet mined", "hash", txHash) return nil } else if err != nil { + m.metr.RPCError() m.l.Info("Receipt retrieval failed", "hash", txHash, "err", err) return nil } else if receipt == nil { + m.metr.RPCError() m.l.Warn("Receipt and error are both nil", "hash", txHash) return nil } @@ -400,6 +419,7 @@ func (m *SimpleTxManager) suggestGasPriceCaps(ctx context.Context) (*big.Int, *b defer cancel() tip, err := m.backend.SuggestGasTipCap(cCtx) if err != nil { + m.metr.RPCError() return nil, nil, fmt.Errorf("failed to fetch the suggested gas tip cap: %w", err) } else if tip == nil { return nil, nil, errors.New("the suggested tip was nil") @@ -408,6 +428,7 @@ func (m *SimpleTxManager) suggestGasPriceCaps(ctx context.Context) (*big.Int, *b defer cancel() head, err := m.backend.HeaderByNumber(cCtx, nil) if err != nil { + m.metr.RPCError() return nil, nil, fmt.Errorf("failed to fetch the suggested basefee: %w", err) } else if head.BaseFee == nil { return nil, nil, errors.New("txmgr does not support pre-london blocks that do not have a basefee") diff --git a/op-service/txmgr/txmgr_test.go b/op-service/txmgr/txmgr_test.go index d6da3b4677ba..f170b5c08cc9 100644 --- a/op-service/txmgr/txmgr_test.go +++ b/op-service/txmgr/txmgr_test.go @@ -691,6 +691,7 @@ func TestWaitMinedReturnsReceiptAfterFailure(t *testing.T) { name: "TEST", backend: &borkedBackend, l: testlog.Logger(t, log.LvlCrit), + metr: &metrics.NoopTxMetrics{}, } // Don't mine the tx with the default backend. The failingBackend will @@ -726,6 +727,7 @@ func doGasPriceIncrease(t *testing.T, txTipCap, txFeeCap, newTip, newBaseFee int name: "TEST", backend: &borkedBackend, l: testlog.Logger(t, log.LvlCrit), + metr: &metrics.NoopTxMetrics{}, } tx := types.NewTx(&types.DynamicFeeTx{ @@ -829,6 +831,7 @@ func TestIncreaseGasPriceNotExponential(t *testing.T) { name: "TEST", backend: &borkedBackend, l: testlog.Logger(t, log.LvlCrit), + metr: &metrics.NoopTxMetrics{}, } tx := types.NewTx(&types.DynamicFeeTx{ GasTipCap: big.NewInt(10), diff --git a/packages/atst/README.md b/packages/atst/README.md index 5a038d0e45da..7cadfab87a42 100644 --- a/packages/atst/README.md +++ b/packages/atst/README.md @@ -63,7 +63,7 @@ Also some more hooks exported by the cli but these are likely the only ones you Please see our [contributing.md](https://github.com/ethereum-optimism/optimism/blob/develop/CONTRIBUTING.md). No contribution is too small. -Having your contribution denied feels bad. +Having your contribution denied feels bad. Please consider [opening an issue](https://github.com/ethereum-optimism/optimism/issues) before adding any new features or apis. @@ -73,5 +73,5 @@ If you have any problems, these resources could help you: - [sdk documentation](https://github.com/ethereum-optimism/optimism/blob/develop/packages/atst/docs/sdk.md) - [cli documentation](https://github.com/ethereum-optimism/optimism/blob/develop/packages/atst/docs/cli.md) -- [Optimism Discord](https://discord-gateway.optimism.io/) -- [Telegram group](https://t.me/+zwpJ8Ohqgl8yNjNh) +- [Optimism Discord](https://discord.gg/optimism) +- [Telegram group](https://t.me/+zwpJ8Ohqgl8yNjNh) diff --git a/packages/contracts-bedrock/contracts/deployment/SystemDictator.sol b/packages/contracts-bedrock/contracts/deployment/SystemDictator.sol index 0c6f4cbe4ee2..129dd9c08ba6 100644 --- a/packages/contracts-bedrock/contracts/deployment/SystemDictator.sol +++ b/packages/contracts-bedrock/contracts/deployment/SystemDictator.sol @@ -215,7 +215,7 @@ contract SystemDictator is OwnableUpgradeable { /** * @notice Configures the ProxyAdmin contract. */ - function step1() external onlyOwner step(1) { + function step1() public onlyOwner step(1) { // Set the AddressManager in the ProxyAdmin. config.globalConfig.proxyAdmin.setAddressManager(config.globalConfig.addressManager); @@ -260,7 +260,7 @@ contract SystemDictator is OwnableUpgradeable { * @notice Pauses the system by shutting down the L1CrossDomainMessenger and setting the * deposit halt flag to tell the Sequencer's DTL to stop accepting deposits. */ - function step2() external onlyOwner step(2) { + function step2() public onlyOwner step(2) { // Store the address of the old L1CrossDomainMessenger implementation. We will need this // address in the case that we have to exit early. oldL1CrossDomainMessenger = config.globalConfig.addressManager.getAddress( @@ -410,6 +410,14 @@ contract SystemDictator is OwnableUpgradeable { ); } + /** + * @notice Calls the first 2 steps of the migration process. + */ + function phase1() external onlyOwner { + step1(); + step2(); + } + /** * @notice Tranfers admin ownership to the final owner. */ diff --git a/packages/contracts-bedrock/contracts/universal/Proxy.sol b/packages/contracts-bedrock/contracts/universal/Proxy.sol index f2fead040b61..242cc7705569 100644 --- a/packages/contracts-bedrock/contracts/universal/Proxy.sol +++ b/packages/contracts-bedrock/contracts/universal/Proxy.sol @@ -84,7 +84,7 @@ contract Proxy { * * @param _implementation Address of the implementation contract. */ - function upgradeTo(address _implementation) external proxyCallIfNotAdmin { + function upgradeTo(address _implementation) public virtual proxyCallIfNotAdmin { _setImplementation(_implementation); } @@ -96,8 +96,9 @@ contract Proxy { * @param _data Calldata to delegatecall the new implementation with. */ function upgradeToAndCall(address _implementation, bytes calldata _data) - external + public payable + virtual proxyCallIfNotAdmin returns (bytes memory) { @@ -112,7 +113,7 @@ contract Proxy { * * @param _admin New owner of the proxy contract. */ - function changeAdmin(address _admin) external proxyCallIfNotAdmin { + function changeAdmin(address _admin) public virtual proxyCallIfNotAdmin { _changeAdmin(_admin); } @@ -121,7 +122,7 @@ contract Proxy { * * @return Owner address. */ - function admin() external proxyCallIfNotAdmin returns (address) { + function admin() public virtual proxyCallIfNotAdmin returns (address) { return _getAdmin(); } @@ -130,7 +131,7 @@ contract Proxy { * * @return Implementation address. */ - function implementation() external proxyCallIfNotAdmin returns (address) { + function implementation() public virtual proxyCallIfNotAdmin returns (address) { return _getImplementation(); } diff --git a/packages/contracts-bedrock/deploy-config/hardhat.json b/packages/contracts-bedrock/deploy-config/hardhat.json index cb40d0bbb13f..fddcadf1296a 100644 --- a/packages/contracts-bedrock/deploy-config/hardhat.json +++ b/packages/contracts-bedrock/deploy-config/hardhat.json @@ -2,6 +2,7 @@ "finalSystemOwner": "0x9965507D1a55bcC2695C58ba16FB37d819B0A4dc", "portalGuardian": "0x9965507D1a55bcC2695C58ba16FB37d819B0A4dc", "controller": "0xf39fd6e51aad88f6f4ce6ab8827279cfffb92266", + "proxyAdminOwner": "0x9965507D1a55bcC2695C58ba16FB37d819B0A4dc", "l1StartingBlockTag": "earliest", "l1ChainID": 900, @@ -16,12 +17,22 @@ "batchSenderAddress": "0x3C44CdDdB6a900fa2b585dd299e03d12FA4293BC", "l2OutputOracleSubmissionInterval": 6, - "l2OutputOracleStartingTimestamp": -1, + "l2OutputOracleStartingTimestamp": 0, + "l2OutputOracleStartingBlockNumber": 0, "l2OutputOracleProposer": "0x70997970C51812dc3A010C7d01b50e0d17dc79C8", "l2OutputOracleChallenger": "0x6925B8704Ff96DEe942623d6FB5e946EF5884b63", "l2GenesisBlockBaseFeePerGas": "0x3B9ACA00", - "baseFeeVaultRecipient": "0xBcd4042DE499D14e55001CcbB24a551F3b954096", + "l2GenesisBlockGasLimit": "0x17D7840", + + "baseFeeVaultRecipient": "0x9965507D1a55bcC2695C58ba16FB37d819B0A4dc", + "l1FeeVaultRecipient": "0x9965507D1a55bcC2695C58ba16FB37d819B0A4dc", + "sequencerFeeVaultRecipient": "0x9965507D1a55bcC2695C58ba16FB37d819B0A4dc", + + "governanceTokenName": "Optimism", + "governanceTokenSymbol": "OP", + "governanceTokenOwner": "0x9965507D1a55bcC2695C58ba16FB37d819B0A4dc", + "l1FeeVaultRecipient": "0x71bE63f3384f5fb98995898A86B02Fb2426c5788", "sequencerFeeVaultRecipient": "0xfabb0ac9d68b0b445fb7357272ff202c5651694a", diff --git a/packages/contracts-bedrock/deploy/020-SystemDictatorSteps-1.ts b/packages/contracts-bedrock/deploy/020-SystemDictatorSteps-1.ts index 480af8a7533b..e1f3980c4308 100644 --- a/packages/contracts-bedrock/deploy/020-SystemDictatorSteps-1.ts +++ b/packages/contracts-bedrock/deploy/020-SystemDictatorSteps-1.ts @@ -11,10 +11,8 @@ import { assertContractVariable, getContractsFromArtifacts, getDeploymentAddress, - doStep, - jsonifyTransaction, - getTenderlySimulationLink, - getCastCommand, + doOwnershipTransfer, + doPhase, } from '../src/deploy-utils' const uint128Max = ethers.BigNumber.from('0xffffffffffffffffffffffffffffffff') @@ -73,10 +71,13 @@ const deployFn: DeployFunction = async (hre) => { // Transfer ownership of the ProxyAdmin to the SystemDictator. if ((await ProxyAdmin.owner()) !== SystemDictator.address) { - console.log(`Setting ProxyAdmin owner to MSD`) - await ProxyAdmin.transferOwnership(SystemDictator.address) - } else { - console.log(`Proxy admin already owned by MSD`) + await doOwnershipTransfer({ + isLiveDeployer, + proxy: ProxyAdmin, + name: 'ProxyAdmin', + transferFunc: 'transferOwnership', + dictator: SystemDictator, + }) } // We don't need to transfer proxy addresses if we're already beyond the proxy transfer step. @@ -89,31 +90,13 @@ const deployFn: DeployFunction = async (hre) => { needsProxyTransfer && (await AddressManager.owner()) !== SystemDictator.address ) { - if (isLiveDeployer) { - console.log(`Setting AddressManager owner to MSD`) - await AddressManager.transferOwnership(SystemDictator.address) - } else { - const tx = await AddressManager.populateTransaction.transferOwnership( - SystemDictator.address - ) - console.log(`Please transfer AddressManager owner to MSD`) - console.log(`AddressManager address: ${AddressManager.address}`) - console.log(`MSD address: ${SystemDictator.address}`) - console.log(`JSON:`) - console.log(jsonifyTransaction(tx)) - console.log(getCastCommand(tx)) - console.log(await getTenderlySimulationLink(SystemDictator.provider, tx)) - } - - // Wait for the ownership transfer to complete. - await awaitCondition( - async () => { - const owner = await AddressManager.owner() - return owner === SystemDictator.address - }, - 5000, - 1000 - ) + await doOwnershipTransfer({ + isLiveDeployer, + proxy: AddressManager, + name: 'AddressManager', + transferFunc: 'transferOwnership', + dictator: SystemDictator, + }) } else { console.log(`AddressManager already owned by the SystemDictator`) } @@ -125,35 +108,13 @@ const deployFn: DeployFunction = async (hre) => { from: ethers.constants.AddressZero, })) !== SystemDictator.address ) { - if (isLiveDeployer) { - console.log(`Setting L1StandardBridge owner to MSD`) - await L1StandardBridgeProxyWithSigner.setOwner(SystemDictator.address) - } else { - const tx = await L1StandardBridgeProxy.populateTransaction.setOwner( - SystemDictator.address - ) - console.log(`Please transfer L1StandardBridge (proxy) owner to MSD`) - console.log( - `L1StandardBridgeProxy address: ${L1StandardBridgeProxy.address}` - ) - console.log(`MSD address: ${SystemDictator.address}`) - console.log(`JSON:`) - console.log(jsonifyTransaction(tx)) - console.log(getCastCommand(tx)) - console.log(await getTenderlySimulationLink(SystemDictator.provider, tx)) - } - - // Wait for the ownership transfer to complete. - await awaitCondition( - async () => { - const owner = await L1StandardBridgeProxy.callStatic.getOwner({ - from: ethers.constants.AddressZero, - }) - return owner === SystemDictator.address - }, - 5000, - 1000 - ) + await doOwnershipTransfer({ + isLiveDeployer, + proxy: L1StandardBridgeProxyWithSigner, + name: 'L1StandardBridgeProxy', + transferFunc: 'setOwner', + dictator: SystemDictator, + }) } else { console.log(`L1StandardBridge already owned by MSD`) } @@ -165,47 +126,58 @@ const deployFn: DeployFunction = async (hre) => { from: ethers.constants.AddressZero, })) !== SystemDictator.address ) { - if (isLiveDeployer) { - console.log(`Setting L1ERC721Bridge owner to MSD`) - await L1ERC721BridgeProxyWithSigner.changeAdmin(SystemDictator.address) - } else { - const tx = await L1ERC721BridgeProxy.populateTransaction.changeAdmin( - SystemDictator.address - ) - console.log(`Please transfer L1ERC721Bridge (proxy) owner to MSD`) - console.log(`L1ERC721BridgeProxy address: ${L1ERC721BridgeProxy.address}`) - console.log(`MSD address: ${SystemDictator.address}`) - console.log(`JSON:`) - console.log(jsonifyTransaction(tx)) - console.log(getCastCommand(tx)) - console.log(await getTenderlySimulationLink(SystemDictator.provider, tx)) - } - - // Wait for the ownership transfer to complete. - await awaitCondition( - async () => { - const owner = await L1ERC721BridgeProxy.callStatic.admin({ - from: ethers.constants.AddressZero, - }) - return owner === SystemDictator.address - }, - 5000, - 1000 - ) + await doOwnershipTransfer({ + isLiveDeployer, + proxy: L1ERC721BridgeProxyWithSigner, + name: 'L1ERC721BridgeProxy', + transferFunc: 'changeAdmin', + dictator: SystemDictator, + }) } else { console.log(`L1ERC721Bridge already owned by MSD`) } - // Step 1 is a freebie, it doesn't impact the system. - await doStep({ + // Wait for the ownership transfers to complete before continuing. + await awaitCondition( + async (): Promise => { + const proxyAdminOwner = await ProxyAdmin.owner() + const addressManagerOwner = await AddressManager.owner() + const l1StandardBridgeOwner = + await L1StandardBridgeProxy.callStatic.getOwner({ + from: ethers.constants.AddressZero, + }) + const l1Erc721BridgeOwner = await L1ERC721BridgeProxy.callStatic.admin({ + from: ethers.constants.AddressZero, + }) + + return ( + proxyAdminOwner === SystemDictator.address && + addressManagerOwner === SystemDictator.address && + l1StandardBridgeOwner === SystemDictator.address && + l1Erc721BridgeOwner === SystemDictator.address + ) + }, + 5000, + 1000 + ) + + await doPhase({ isLiveDeployer, SystemDictator, - step: 1, + phase: 1, message: ` + Phase 1 includes the following steps: + Step 1 will configure the ProxyAdmin contract, you can safely execute this step at any time without impacting the functionality of the rest of the system. + + Step 2 will stop deposits and withdrawals via the L1CrossDomainMessenger and will stop the + DTL from syncing new deposits via the CTC, effectively shutting down the legacy system. Once + this step has been executed, you should immediately begin the L2 migration process. If you + need to restart the system, run exit1() followed by finalize(). `, checks: async () => { + // Step 1 checks await assertContractVariable( ProxyAdmin, 'addressManager', @@ -264,21 +236,8 @@ const deployFn: DeployFunction = async (hre) => { assert(config.systemTxMaxGas === 1_000_000) assert(ethers.utils.parseUnits('1', 'gwei').eq(config.minimumBaseFee)) assert(config.maximumBaseFee.eq(uint128Max)) - }, - }) - // Step 2 shuts down the system. - await doStep({ - isLiveDeployer, - SystemDictator, - step: 2, - message: ` - Step 2 will stop deposits and withdrawals via the L1CrossDomainMessenger and will stop the - DTL from syncing new deposits via the CTC, effectively shutting down the legacy system. Once - this step has been executed, you should immediately begin the L2 migration process. If you - need to restart the system, run exit1() followed by finalize(). - `, - checks: async () => { + // Step 2 checks const messenger = await AddressManager.getAddress( 'OVM_L1CrossDomainMessenger' ) diff --git a/packages/contracts-bedrock/deploy/021-SystemDictatorSteps-2.ts b/packages/contracts-bedrock/deploy/021-SystemDictatorSteps-2.ts index fa44f1be7099..97356572820e 100644 --- a/packages/contracts-bedrock/deploy/021-SystemDictatorSteps-2.ts +++ b/packages/contracts-bedrock/deploy/021-SystemDictatorSteps-2.ts @@ -10,11 +10,11 @@ import '@nomiclabs/hardhat-ethers' import { assertContractVariable, getContractsFromArtifacts, - jsonifyTransaction, + printJsonTransaction, isStep, doStep, - getTenderlySimulationLink, - getCastCommand, + printTenderlySimulationLink, + printCastCommand, } from '../src/deploy-utils' const deployFn: DeployFunction = async (hre) => { @@ -206,10 +206,9 @@ const deployFn: DeployFunction = async (hre) => { ) ) console.log(`MSD address: ${SystemDictator.address}`) - console.log(`JSON:`) - console.log(jsonifyTransaction(tx)) - console.log(getCastCommand(tx)) - console.log(await getTenderlySimulationLink(SystemDictator.provider, tx)) + printJsonTransaction(tx) + printCastCommand(tx) + await printTenderlySimulationLink(SystemDictator.provider, tx) } await awaitCondition( @@ -318,10 +317,9 @@ const deployFn: DeployFunction = async (hre) => { const tx = await OptimismPortal.populateTransaction.unpause() console.log(`Please unpause the OptimismPortal...`) console.log(`OptimismPortal address: ${OptimismPortal.address}`) - console.log(`JSON:`) - console.log(jsonifyTransaction(tx)) - console.log(getCastCommand(tx)) - console.log(await getTenderlySimulationLink(SystemDictator.provider, tx)) + printJsonTransaction(tx) + printCastCommand(tx) + await printTenderlySimulationLink(SystemDictator.provider, tx) } await awaitCondition( @@ -348,10 +346,9 @@ const deployFn: DeployFunction = async (hre) => { const tx = await SystemDictator.populateTransaction.finalize() console.log(`Please finalize deployment...`) console.log(`MSD address: ${SystemDictator.address}`) - console.log(`JSON:`) - console.log(jsonifyTransaction(tx)) - console.log(getCastCommand(tx)) - console.log(await getTenderlySimulationLink(SystemDictator.provider, tx)) + printJsonTransaction(tx) + printCastCommand(tx) + await printTenderlySimulationLink(SystemDictator.provider, tx) } await awaitCondition( diff --git a/packages/contracts-bedrock/scripts/slither.sh b/packages/contracts-bedrock/scripts/slither.sh index e05a2653f6b9..f8f7d9586b4a 100755 --- a/packages/contracts-bedrock/scripts/slither.sh +++ b/packages/contracts-bedrock/scripts/slither.sh @@ -3,7 +3,11 @@ rm -rf artifacts forge-artifacts # See slither.config.json for slither settings -if [ -n "$TRIAGE_MODE" ]; then +if [[ -z "$TRIAGE_MODE" ]]; then + echo "Running slither" + slither . +else + echo "Running slither in triage mode" # Slither's triage mode will run an 'interview' in the terminal, allowing you to review each of # its findings, and specify which should be ignored in future runs of slither. This will update # (or create) the slither.db.json file. This DB is a cleaner alternative to adding slither-disable @@ -20,6 +24,4 @@ if [ -n "$TRIAGE_MODE" ]; then mv $DB $TEMP_DB jq 'walk(if type == "object" then del(.filename_absolute) else . end)' $TEMP_DB > $DB rm -f $TEMP_DB -else - slither . fi diff --git a/packages/contracts-bedrock/slither.config.json b/packages/contracts-bedrock/slither.config.json index f6de8a76d6f2..42eba89015fb 100644 --- a/packages/contracts-bedrock/slither.config.json +++ b/packages/contracts-bedrock/slither.config.json @@ -1,5 +1,5 @@ { - "detectors_to_exclude": "assembly-usage,block-timestamp,naming-convention,solc-version", + "detectors_to_exclude": "assembly-usage,block-timestamp,naming-convention,solc-version,low-level-calls", "exclude_informational": true, "exclude_low": true, "exclude_medium": true, diff --git a/packages/contracts-bedrock/src/deploy-utils.ts b/packages/contracts-bedrock/src/deploy-utils.ts index 5f57a171f8af..cf579569ce22 100644 --- a/packages/contracts-bedrock/src/deploy-utils.ts +++ b/packages/contracts-bedrock/src/deploy-utils.ts @@ -305,20 +305,56 @@ export const getDeploymentAddress = async ( * @param tx Ethers transaction object. * @returns JSON-ified transaction object. */ -export const jsonifyTransaction = (tx: ethers.PopulatedTransaction): string => { - return JSON.stringify( - { - from: tx.from, - to: tx.to, - data: tx.data, - value: tx.value, - chainId: tx.chainId, - }, - null, - 2 +export const printJsonTransaction = (tx: ethers.PopulatedTransaction): void => { + console.log( + 'JSON transaction parameters:\n' + + JSON.stringify( + { + from: tx.from, + to: tx.to, + data: tx.data, + value: tx.value, + chainId: tx.chainId, + }, + null, + 2 + ) ) } +/** + * Mini helper for transferring a Proxy to the MSD + * + * @param opts Options for executing the step. + * @param opts.isLiveDeployer True if the deployer is live. + * @param opts.proxy proxy contract. + * @param opts.dictator dictator contract. + */ +export const doOwnershipTransfer = async (opts: { + isLiveDeployer?: boolean + proxy: ethers.Contract + name: string + transferFunc: string + dictator: ethers.Contract +}): Promise => { + if (opts.isLiveDeployer) { + console.log(`Setting ${opts.name} owner to MSD`) + await opts.proxy[opts.transferFunc](opts.dictator.address) + } else { + const tx = await opts.proxy.populateTransaction[opts.transferFunc]( + opts.dictator.address + ) + console.log(` + Please transfer ${opts.name} (proxy) owner to MSD + - ${opts.name} address: ${opts.proxy.address} + - MSD address: ${opts.dictator.address} + `) + printJsonTransaction(tx) + printCastCommand(tx) + await printTenderlySimulationLink(opts.dictator.provider, tx) + } +} + /** * Mini helper for checking if the current step is a target step. * @@ -333,6 +369,25 @@ export const isStep = async ( return (await dictator.currentStep()) === step } +/** + * Mini helper for checking if the current step is the first step in target phase. + * + * @param dictator SystemDictator contract. + * @param phase Target phase. + * @returns True if the current step is the first step in target phase. + */ +export const isStartOfPhase = async ( + dictator: ethers.Contract, + phase: number +): Promise => { + const phaseToStep = { + 1: 1, + 2: 3, + 3: 6, + } + return (await dictator.currentStep()) === phaseToStep[phase] +} + /** * Mini helper for executing a given step. * @@ -350,7 +405,8 @@ export const doStep = async (opts: { message: string checks: () => Promise }): Promise => { - if (!(await isStep(opts.SystemDictator, opts.step))) { + const isStepVal = await isStep(opts.SystemDictator, opts.step) + if (!isStepVal) { console.log(`Step already completed: ${opts.step}`) return } @@ -368,11 +424,8 @@ export const doStep = async (opts: { ]() console.log(`Please execute step ${opts.step}...`) console.log(`MSD address: ${opts.SystemDictator.address}`) - console.log(`JSON:`) - console.log(jsonifyTransaction(tx)) - console.log( - await getTenderlySimulationLink(opts.SystemDictator.provider, tx) - ) + printJsonTransaction(tx) + await printTenderlySimulationLink(opts.SystemDictator.provider, tx) } // Wait for the step to complete. @@ -389,36 +442,91 @@ export const doStep = async (opts: { } /** - * Returns a direct link to a Tenderly simulation. + * Mini helper for executing a given phase. + * + * @param opts Options for executing the step. + * @param opts.isLiveDeployer True if the deployer is live. + * @param opts.SystemDictator SystemDictator contract. + * @param opts.step Step to execute. + * @param opts.message Message to print before executing the step. + * @param opts.checks Checks to perform after executing the step. + */ +export const doPhase = async (opts: { + isLiveDeployer?: boolean + SystemDictator: ethers.Contract + phase: number + message: string + checks: () => Promise +}): Promise => { + const isStart = await isStartOfPhase(opts.SystemDictator, opts.phase) + if (!isStart) { + console.log(`Start of phase ${opts.phase} already completed`) + return + } + + // Extra message to help the user understand what's going on. + console.log(opts.message) + + // Either automatically or manually execute the step. + if (opts.isLiveDeployer) { + console.log(`Executing phase ${opts.phase}...`) + await opts.SystemDictator[`phase${opts.phase}`]() + } else { + const tx = await opts.SystemDictator.populateTransaction[ + `phase${opts.phase}` + ]() + console.log(`Please execute phase ${opts.phase}...`) + console.log(`MSD address: ${opts.SystemDictator.address}`) + printJsonTransaction(tx) + await printTenderlySimulationLink(opts.SystemDictator.provider, tx) + } + + // Wait for the step to complete. + await awaitCondition( + async () => { + return isStartOfPhase(opts.SystemDictator, opts.phase + 1) + }, + 30000, + 1000 + ) + + // Perform post-step checks. + await opts.checks() +} + +/** + * Prints a direct link to a Tenderly simulation. * * @param provider Ethers Provider. * @param tx Ethers transaction object. - * @returns the url of the tenderly simulation. */ -export const getTenderlySimulationLink = async ( +export const printTenderlySimulationLink = async ( provider: ethers.providers.Provider, tx: ethers.PopulatedTransaction -): Promise => { +): Promise => { if (process.env.TENDERLY_PROJECT && process.env.TENDERLY_USERNAME) { - return `https://dashboard.tenderly.co/${process.env.TENDERLY_PROJECT}/${ - process.env.TENDERLY_USERNAME - }/simulator/new?${new URLSearchParams({ - network: (await provider.getNetwork()).chainId.toString(), - contractAddress: tx.to, - rawFunctionInput: tx.data, - from: tx.from, - }).toString()}` + console.log( + `https://dashboard.tenderly.co/${process.env.TENDERLY_PROJECT}/${ + process.env.TENDERLY_USERNAME + }/simulator/new?${new URLSearchParams({ + network: (await provider.getNetwork()).chainId.toString(), + contractAddress: tx.to, + rawFunctionInput: tx.data, + from: tx.from, + }).toString()}` + ) } } /** - * Returns a cast commmand for submitting a given transaction. + * Prints a cast commmand for submitting a given transaction. * * @param tx Ethers transaction object. - * @returns the cast command */ -export const getCastCommand = (tx: ethers.PopulatedTransaction): string => { +export const printCastCommand = (tx: ethers.PopulatedTransaction): void => { if (process.env.CAST_COMMANDS) { - return `cast send ${tx.to} ${tx.data} --from ${tx.from} --value ${tx.value}` + console.log( + `cast send ${tx.to} ${tx.data} --from ${tx.from} --value ${tx.value}` + ) } } diff --git a/packages/contracts-periphery/.gitmodules b/packages/contracts-periphery/.gitmodules new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/packages/contracts-periphery/config/deploy/ethereum.ts b/packages/contracts-periphery/config/deploy/ethereum.ts index 883b69161db2..b5c55ef20cd1 100644 --- a/packages/contracts-periphery/config/deploy/ethereum.ts +++ b/packages/contracts-periphery/config/deploy/ethereum.ts @@ -6,6 +6,8 @@ const config: DeployConfig = { optimistName: '', optimistSymbol: '', attestorAddress: '', + optimistInviterName: '', + optimistInviterInviteGranter: '', } export default config diff --git a/packages/contracts-periphery/config/deploy/goerli.ts b/packages/contracts-periphery/config/deploy/goerli.ts index b6f5dc418459..19f7299f29bb 100644 --- a/packages/contracts-periphery/config/deploy/goerli.ts +++ b/packages/contracts-periphery/config/deploy/goerli.ts @@ -6,6 +6,8 @@ const config: DeployConfig = { optimistName: 'Optimist', optimistSymbol: 'OPTIMIST', attestorAddress: '0x60c5C9c98bcBd0b0F2fD89B24c16e533BaA8CdA3', + optimistInviterInviteGranter: '0x60c5C9c98bcBd0b0F2fD89B24c16e533BaA8CdA3', + optimistInviterName: 'OptimistInviter', } export default config diff --git a/packages/contracts-periphery/config/deploy/hardhat.ts b/packages/contracts-periphery/config/deploy/hardhat.ts index 5748379cd363..4cadd97f4d2b 100644 --- a/packages/contracts-periphery/config/deploy/hardhat.ts +++ b/packages/contracts-periphery/config/deploy/hardhat.ts @@ -6,6 +6,8 @@ const config: DeployConfig = { optimistName: 'OP Citizenship', optimistSymbol: 'OPNFT', attestorAddress: '0x70997970c51812dc3a010c7d01b50e0d17dc79c8', + optimistInviterInviteGranter: '0x70997970c51812dc3a010c7d01b50e0d17dc79c8', + optimistInviterName: 'OptimistInviter', } export default config diff --git a/packages/contracts-periphery/config/deploy/mainnet.ts b/packages/contracts-periphery/config/deploy/mainnet.ts index b6f5dc418459..19f7299f29bb 100644 --- a/packages/contracts-periphery/config/deploy/mainnet.ts +++ b/packages/contracts-periphery/config/deploy/mainnet.ts @@ -6,6 +6,8 @@ const config: DeployConfig = { optimistName: 'Optimist', optimistSymbol: 'OPTIMIST', attestorAddress: '0x60c5C9c98bcBd0b0F2fD89B24c16e533BaA8CdA3', + optimistInviterInviteGranter: '0x60c5C9c98bcBd0b0F2fD89B24c16e533BaA8CdA3', + optimistInviterName: 'OptimistInviter', } export default config diff --git a/packages/contracts-periphery/config/deploy/optimism-goerli.ts b/packages/contracts-periphery/config/deploy/optimism-goerli.ts index 7e89cdc3a84b..bb8d30fe55a2 100644 --- a/packages/contracts-periphery/config/deploy/optimism-goerli.ts +++ b/packages/contracts-periphery/config/deploy/optimism-goerli.ts @@ -6,6 +6,8 @@ const config: DeployConfig = { optimistName: 'Optimist', optimistSymbol: 'OPTIMIST', attestorAddress: '0x8F0EBDaA1cF7106bE861753B0f9F5c0250fE0819', + optimistInviterInviteGranter: '0x8F0EBDaA1cF7106bE861753B0f9F5c0250fE0819', + optimistInviterName: 'OptimistInviter', } export default config diff --git a/packages/contracts-periphery/config/deploy/optimism.ts b/packages/contracts-periphery/config/deploy/optimism.ts index b6f5dc418459..19f7299f29bb 100644 --- a/packages/contracts-periphery/config/deploy/optimism.ts +++ b/packages/contracts-periphery/config/deploy/optimism.ts @@ -6,6 +6,8 @@ const config: DeployConfig = { optimistName: 'Optimist', optimistSymbol: 'OPTIMIST', attestorAddress: '0x60c5C9c98bcBd0b0F2fD89B24c16e533BaA8CdA3', + optimistInviterInviteGranter: '0x60c5C9c98bcBd0b0F2fD89B24c16e533BaA8CdA3', + optimistInviterName: 'OptimistInviter', } export default config diff --git a/packages/contracts-periphery/contracts/foundry-tests/MulticallContractCompiler.t.sol b/packages/contracts-periphery/contracts/foundry-tests/MulticallContractCompiler.t.sol new file mode 100644 index 000000000000..a2868358da32 --- /dev/null +++ b/packages/contracts-periphery/contracts/foundry-tests/MulticallContractCompiler.t.sol @@ -0,0 +1,11 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.0; + +import { Multicall3 } from "multicall/src/Multicall3.sol"; + +/** + * Just exists so we can compile this contract. + */ +contract MulticallContractCompiler { + +} diff --git a/packages/contracts-periphery/contracts/foundry-tests/Optimist.t.sol b/packages/contracts-periphery/contracts/foundry-tests/Optimist.t.sol index b6525e73d1fe..653b6853cca0 100644 --- a/packages/contracts-periphery/contracts/foundry-tests/Optimist.t.sol +++ b/packages/contracts-periphery/contracts/foundry-tests/Optimist.t.sol @@ -1,61 +1,205 @@ //SPDX-License-Identifier: MIT -pragma solidity 0.8.15; +pragma solidity >=0.6.2 <0.9.0; /* Testing utilities */ import { Test } from "forge-std/Test.sol"; import { AttestationStation } from "../universal/op-nft/AttestationStation.sol"; import { Optimist } from "../universal/op-nft/Optimist.sol"; +import { OptimistAllowlist } from "../universal/op-nft/OptimistAllowlist.sol"; +import { OptimistInviter } from "../universal/op-nft/OptimistInviter.sol"; +import { OptimistInviterHelper } from "../testing/helpers/OptimistInviterHelper.sol"; import { Strings } from "@openzeppelin/contracts/utils/Strings.sol"; import { IERC721 } from "@openzeppelin/contracts/token/ERC721/IERC721.sol"; +interface IMulticall3 { + struct Call3 { + address target; + bool allowFailure; + bytes callData; + } + + struct Result { + bool success; + bytes returnData; + } + + function aggregate3(Call3[] calldata calls) + external + payable + returns (Result[] memory returnData); +} + contract Optimist_Initializer is Test { event Transfer(address indexed from, address indexed to, uint256 indexed tokenId); event Initialized(uint8); + event AttestationCreated( + address indexed creator, + address indexed about, + bytes32 indexed key, + bytes val + ); - address constant alice_admin = address(128); - address constant bob = address(256); - address constant sally = address(512); string constant name = "Optimist name"; string constant symbol = "OPTIMISTSYMBOL"; string constant base_uri = "https://storageapi.fleek.co/6442819a1b05-bucket/optimist-nft/attributes"; AttestationStation attestationStation; Optimist optimist; + OptimistAllowlist optimistAllowlist; + OptimistInviter optimistInviter; - function attestBaseuri(string memory _baseUri) internal { + // Helps with EIP-712 signature generation + OptimistInviterHelper optimistInviterHelper; + + // To test multicall for claiming and minting in one call + IMulticall3 multicall3; + + address internal carol_baseURIAttestor; + address internal alice_allowlistAttestor; + address internal eve_inviteGranter; + address internal ted_coinbaseAttestor; + address internal bob; + address internal sally; + + /** + * @notice BaseURI attestor sets the baseURI of the Optimist NFT. + */ + function _attestBaseURI(string memory _baseUri) internal { + bytes32 baseURIAttestationKey = optimist.BASE_URI_ATTESTATION_KEY(); AttestationStation.AttestationData[] memory attestationData = new AttestationStation.AttestationData[](1); attestationData[0] = AttestationStation.AttestationData( address(optimist), - bytes32("optimist.base-uri"), + baseURIAttestationKey, bytes(_baseUri) ); - vm.prank(alice_admin); + + vm.expectEmit(true, true, true, true, address(attestationStation)); + emit AttestationCreated( + carol_baseURIAttestor, + address(optimist), + baseURIAttestationKey, + bytes(_baseUri) + ); + vm.prank(carol_baseURIAttestor); + attestationStation.attest(attestationData); + } + + /** + * @notice Allowlist attestor creates an attestation for an address. + */ + function _attestAllowlist(address _about) internal { + bytes32 attestationKey = optimistAllowlist.OPTIMIST_CAN_MINT_ATTESTATION_KEY(); + AttestationStation.AttestationData[] + memory attestationData = new AttestationStation.AttestationData[](1); + // we are using true but it can be any non empty value + attestationData[0] = AttestationStation.AttestationData({ + about: _about, + key: attestationKey, + val: bytes("true") + }); + + vm.expectEmit(true, true, true, true, address(attestationStation)); + emit AttestationCreated(alice_allowlistAttestor, _about, attestationKey, bytes("true")); + + vm.prank(alice_allowlistAttestor); attestationStation.attest(attestationData); + + assertTrue(optimist.isOnAllowList(_about)); } - function attestAllowlist(address _about) internal { + /** + * @notice Coinbase Quest attestor creates an attestation for an address. + */ + function _attestCoinbaseQuest(address _about) internal { + bytes32 attestationKey = optimistAllowlist.COINBASE_QUEST_ELIGIBLE_ATTESTATION_KEY(); AttestationStation.AttestationData[] memory attestationData = new AttestationStation.AttestationData[](1); // we are using true but it can be any non empty value attestationData[0] = AttestationStation.AttestationData({ about: _about, - key: bytes32("optimist.can-mint"), + key: attestationKey, val: bytes("true") }); - vm.prank(alice_admin); + + vm.expectEmit(true, true, true, true, address(attestationStation)); + emit AttestationCreated(ted_coinbaseAttestor, _about, attestationKey, bytes("true")); + + vm.prank(ted_coinbaseAttestor); attestationStation.attest(attestationData); + + assertTrue(optimist.isOnAllowList(_about)); + } + + /** + * @notice Issues invite, then claims it using the claimer's address. + */ + function _inviteAndClaim(address _about) internal { + uint256 inviterPrivateKey = 0xbeefbeef; + address inviter = vm.addr(inviterPrivateKey); + + address[] memory addresses = new address[](1); + addresses[0] = inviter; + + vm.prank(eve_inviteGranter); + + // grant invites to Inviter; + optimistInviter.setInviteCounts(addresses, 3); + + // issue a new invite + OptimistInviter.ClaimableInvite memory claimableInvite = optimistInviterHelper + .getClaimableInviteWithNewNonce(inviter); + + // EIP-712 sign with Inviter's private key + + (uint8 v, bytes32 r, bytes32 s) = vm.sign( + inviterPrivateKey, + optimistInviterHelper.getDigest(claimableInvite) + ); + bytes memory signature = abi.encodePacked(r, s, v); + + bytes32 hashedCommit = keccak256(abi.encode(_about, signature)); + + // commit the invite + vm.prank(_about); + optimistInviter.commitInvite(hashedCommit); + + // wait minimum commitment period + vm.warp(optimistInviter.MIN_COMMITMENT_PERIOD() + block.timestamp); + + // reveal and claim the invite + optimistInviter.claimInvite(_about, claimableInvite, signature); + + assertTrue(optimist.isOnAllowList(_about)); + } + + /** + * @notice Mocks the allowlistAttestor to always return true for a given address. + */ + function _mockAllowlistTrueFor(address _claimer) internal { + vm.mockCall( + address(optimistAllowlist), + abi.encodeWithSelector(OptimistAllowlist.isAllowedToMint.selector, _claimer), + abi.encode(true) + ); + + assertTrue(optimist.isOnAllowList(_claimer)); + } + + /** + * @notice Returns address as uint256. + */ + function _getTokenId(address _owner) internal pure returns (uint256) { + return uint256(uint160(address(_owner))); } function setUp() public { - // Give alice and bob and sally some ETH - vm.deal(alice_admin, 1 ether); - vm.deal(bob, 1 ether); - vm.deal(sally, 1 ether); - - vm.label(alice_admin, "alice_admin"); - vm.label(bob, "bob"); - vm.label(sally, "sally"); + carol_baseURIAttestor = makeAddr("carol_baseURIAttestor"); + alice_allowlistAttestor = makeAddr("alice_allowlistAttestor"); + eve_inviteGranter = makeAddr("eve_inviteGranter"); + ted_coinbaseAttestor = makeAddr("ted_coinbaseAttestor"); + bob = makeAddr("bob"); + sally = makeAddr("sally"); _initializeContracts(); } @@ -63,99 +207,207 @@ contract Optimist_Initializer is Test { attestationStation = new AttestationStation(); vm.expectEmit(true, true, false, false); emit Initialized(1); - optimist = new Optimist(name, symbol, alice_admin, attestationStation); + + optimistInviter = new OptimistInviter({ + _inviteGranter: eve_inviteGranter, + _attestationStation: attestationStation + }); + + optimistInviter.initialize("OptimistInviter"); + + // Initialize the helper which helps sign EIP-712 signatures + optimistInviterHelper = new OptimistInviterHelper(optimistInviter, "OptimistInviter"); + + optimistAllowlist = new OptimistAllowlist({ + _attestationStation: attestationStation, + _allowlistAttestor: alice_allowlistAttestor, + _coinbaseQuestAttestor: ted_coinbaseAttestor, + _optimistInviter: address(optimistInviter) + }); + + optimist = new Optimist({ + _name: name, + _symbol: symbol, + _baseURIAttestor: carol_baseURIAttestor, + _attestationStation: attestationStation, + _optimistAllowlist: optimistAllowlist + }); + + // address test = deployCode("Multicall3.sol"); + multicall3 = IMulticall3(deployCode("Multicall3.sol")); } } contract OptimistTest is Optimist_Initializer { - function test_optimist_initialize() external { + /** + * @notice Check that constructor and initializer parameters are correctly set. + */ + function test_initialize_success() external { // expect name to be set assertEq(optimist.name(), name); // expect symbol to be set assertEq(optimist.symbol(), symbol); // expect attestationStation to be set assertEq(address(optimist.ATTESTATION_STATION()), address(attestationStation)); - assertEq(optimist.ATTESTOR(), alice_admin); - assertEq(optimist.version(), "1.0.0"); + assertEq(optimist.BASE_URI_ATTESTOR(), carol_baseURIAttestor); + assertEq(optimist.version(), "2.0.0"); } /** - * @dev Bob should be able to mint an NFT if he is allowlisted - * by the attestation station and has a balance of 0 + * @notice Bob should be able to mint an NFT if he is allowlisted + * by the allowlistAttestor and has a balance of 0. */ - function test_optimist_mint_happy_path() external { + function test_mint_afterAllowlistAttestation_succeeds() external { // bob should start with 0 balance assertEq(optimist.balanceOf(bob), 0); - // whitelist bob - attestAllowlist(bob); + // allowlist bob + _attestAllowlist(bob); - uint256 tokenId = uint256(uint160(bob)); + assertTrue(optimistAllowlist.isAllowedToMint(bob)); + + // Check that the OptimistAllowlist is checked + bytes memory data = abi.encodeWithSelector(optimistAllowlist.isAllowedToMint.selector, bob); + vm.expectCall(address(optimistAllowlist), data); + + // mint an NFT and expect mint transfer event to be emitted vm.expectEmit(true, true, true, true); - emit Transfer(address(0), bob, tokenId); + emit Transfer(address(0), bob, _getTokenId(bob)); + vm.prank(bob); + optimist.mint(bob); - bytes memory data = abi.encodeWithSelector( - attestationStation.attestations.selector, - alice_admin, - bob, - bytes32("optimist.can-mint") - ); - vm.expectCall(address(attestationStation), data); - // mint an NFT + // expect the NFT to be owned by bob + assertEq(optimist.ownerOf(_getTokenId(bob)), bob); + assertEq(optimist.balanceOf(bob), 1); + } + + /** + * @notice Bob should be able to mint an NFT if he claimed an invite through OptimistInviter + * and has a balance of 0. + */ + function test_mint_afterInviteClaimed_succeeds() external { + // bob should start with 0 balance + assertEq(optimist.balanceOf(bob), 0); + + // bob claims an invite + _inviteAndClaim(bob); + + assertTrue(optimistAllowlist.isAllowedToMint(bob)); + + // Check that the OptimistAllowlist is checked + bytes memory data = abi.encodeWithSelector(optimistAllowlist.isAllowedToMint.selector, bob); + vm.expectCall(address(optimistAllowlist), data); + + // mint an NFT and expect mint transfer event to be emitted + vm.expectEmit(true, true, true, true); + emit Transfer(address(0), bob, _getTokenId(bob)); vm.prank(bob); optimist.mint(bob); + // expect the NFT to be owned by bob - assertEq(optimist.ownerOf(256), bob); + assertEq(optimist.ownerOf(_getTokenId(bob)), bob); assertEq(optimist.balanceOf(bob), 1); } /** - * @dev Sally should be able to mint a token on behalf of bob + * @notice Bob should be able to mint an NFT if he has an attestation from Coinbase Quest + * attestor and has a balance of 0. */ - function test_optimist_mint_secondary_minter() external { - attestAllowlist(bob); + function test_mint_afterCoinbaseQuestAttestation_succeeds() external { + // bob should start with 0 balance + assertEq(optimist.balanceOf(bob), 0); - bytes memory data = abi.encodeWithSelector( - attestationStation.attestations.selector, - alice_admin, - bob, - bytes32("optimist.can-mint") - ); - vm.expectCall(address(attestationStation), data); + // bob receives attestation from Coinbase Quest attestor + _attestCoinbaseQuest(bob); + + assertTrue(optimistAllowlist.isAllowedToMint(bob)); + + // Check that the OptimistAllowlist is checked + bytes memory data = abi.encodeWithSelector(optimistAllowlist.isAllowedToMint.selector, bob); + vm.expectCall(address(optimistAllowlist), data); + + // mint an NFT and expect mint transfer event to be emitted + vm.expectEmit(true, true, true, true); + emit Transfer(address(0), bob, _getTokenId(bob)); + vm.prank(bob); + optimist.mint(bob); + + // expect the NFT to be owned by bob + assertEq(optimist.ownerOf(_getTokenId(bob)), bob); + assertEq(optimist.balanceOf(bob), 1); + } + + /** + * @notice Multiple valid attestations should allow Bob to mint. + */ + function test_mint_afterMultipleAttestations_succeeds() external { + // bob should start with 0 balance + assertEq(optimist.balanceOf(bob), 0); + + // bob receives attestation from Coinbase Quest attestor + _attestCoinbaseQuest(bob); + + // allowlist bob + _attestAllowlist(bob); + + // bob claims an invite + _inviteAndClaim(bob); + + assertTrue(optimistAllowlist.isAllowedToMint(bob)); + + // Check that the OptimistAllowlist is checked + bytes memory data = abi.encodeWithSelector(optimistAllowlist.isAllowedToMint.selector, bob); + vm.expectCall(address(optimistAllowlist), data); - uint256 tokenId = uint256(uint160(bob)); + // mint an NFT and expect mint transfer event to be emitted vm.expectEmit(true, true, true, true); - emit Transfer(address(0), bob, tokenId); + emit Transfer(address(0), bob, _getTokenId(bob)); + vm.prank(bob); + optimist.mint(bob); + + // expect the NFT to be owned by bob + assertEq(optimist.ownerOf(_getTokenId(bob)), bob); + assertEq(optimist.balanceOf(bob), 1); + } + + /** + * @notice Sally should be able to mint a token on behalf of bob. + */ + function test_mint_secondaryMinter_succeeds() external { + _mockAllowlistTrueFor(bob); + + vm.expectEmit(true, true, true, true); + emit Transfer(address(0), bob, _getTokenId(bob)); // mint as sally instead of bob vm.prank(sally); optimist.mint(bob); // expect the NFT to be owned by bob - assertEq(optimist.ownerOf(256), bob); + assertEq(optimist.ownerOf(_getTokenId(bob)), bob); assertEq(optimist.balanceOf(bob), 1); } /** - * @dev Bob should not be able to mint an NFT if he is not whitelisted + * @notice Bob should not be able to mint an NFT if he is not allowlisted. */ - function test_optimist_mint_no_attestation() external { + function test_mint_forNonAllowlistedClaimer_reverts() external { vm.prank(bob); vm.expectRevert("Optimist: address is not on allowList"); optimist.mint(bob); } /** - * @dev Bob's tx should revert if he already minted + * @notice Bob's tx should revert if he already minted. */ - function test_optimist_mint_already_minted() external { - attestAllowlist(bob); + function test_mint_forAlreadyMintedClaimer_reverts() external { + _attestAllowlist(bob); // mint initial nft with bob vm.prank(bob); optimist.mint(bob); // expect the NFT to be owned by bob - assertEq(optimist.ownerOf(256), bob); + assertEq(optimist.ownerOf(_getTokenId(bob)), bob); assertEq(optimist.balanceOf(bob), 1); // attempt to mint again @@ -164,82 +416,52 @@ contract OptimistTest is Optimist_Initializer { } /** - * @dev The baseURI should be set by attestation station - * by the owner of contract alice_admin + * @notice The baseURI should be set by attestation station by the baseURIAttestor. */ - function test_optimist_baseURI() external { - attestBaseuri(base_uri); + function test_baseURI_returnsCorrectBaseURI_succeeds() external { + _attestBaseURI(base_uri); bytes memory data = abi.encodeWithSelector( attestationStation.attestations.selector, - alice_admin, + carol_baseURIAttestor, address(optimist), - bytes32("optimist.base-uri") + optimist.BASE_URI_ATTESTATION_KEY() ); vm.expectCall(address(attestationStation), data); - vm.prank(alice_admin); + vm.prank(carol_baseURIAttestor); // assert baseURI is set assertEq(optimist.baseURI(), base_uri); } /** - * @dev The tokenURI should return the token uri - * for a minted token + * @notice tokenURI should return the token uri for a minted token. */ - function test_optimist_token_uri() external { - attestAllowlist(bob); + function test_tokenURI_returnsCorrectTokenURI_succeeds() external { // we are using true but it can be any non empty value - attestBaseuri(base_uri); + _attestBaseURI(base_uri); // mint an NFT + _mockAllowlistTrueFor(bob); vm.prank(bob); optimist.mint(bob); // assert tokenURI is set assertEq(optimist.baseURI(), base_uri); assertEq( - optimist.tokenURI(256), - // solhint-disable-next-line max-line-length - "https://storageapi.fleek.co/6442819a1b05-bucket/optimist-nft/attributes/0x0000000000000000000000000000000000000100.json" - ); - } - - /** - * @dev Should return a boolean of if the address is whitelisted - */ - function test_optimist_is_on_allow_list() external { - attestAllowlist(bob); - - bytes memory data = abi.encodeWithSelector( - attestationStation.attestations.selector, - alice_admin, - bob, - bytes32("optimist.can-mint") - ); - vm.expectCall(address(attestationStation), data); - // assert bob is whitelisted - assertEq(optimist.isOnAllowList(bob), true); - data = abi.encodeWithSelector( - attestationStation.attestations.selector, - alice_admin, - sally, - bytes32("optimist.can-mint") + optimist.tokenURI(_getTokenId(bob)), + "https://storageapi.fleek.co/6442819a1b05-bucket/optimist-nft/attributes/0x1d96f2f6bef1202e4ce1ff6dad0c2cb002861d3e.json" ); - vm.expectCall(address(attestationStation), data); - // assert sally is not whitelisted - assertEq(optimist.isOnAllowList(sally), false); } /** - * @dev Should return the token id of the owner + * @notice Should return the token id of the owner. */ - function test_optimist_token_id_of_owner() external { - // whitelist bob + function test_tokenIdOfAddress_returnsOwnerID_succeeds() external { uint256 willTokenId = 1024; address will = address(1024); - attestAllowlist(will); + _mockAllowlistTrueFor(will); optimist.mint(will); @@ -247,10 +469,10 @@ contract OptimistTest is Optimist_Initializer { } /** - * @dev It should revert if anybody attemps token transfer + * @notice transferFrom should revert since Optimist is a SBT. */ - function test_optimist_sbt_transfer() external { - attestAllowlist(bob); + function test_transferFrom_reverts() external { + _mockAllowlistTrueFor(bob); // mint as bob vm.prank(bob); @@ -259,23 +481,23 @@ contract OptimistTest is Optimist_Initializer { // attempt to transfer to sally vm.expectRevert(bytes("Optimist: soul bound token")); vm.prank(bob); - optimist.transferFrom(bob, sally, 256); + optimist.transferFrom(bob, sally, _getTokenId(bob)); // attempt to transfer to sally vm.expectRevert(bytes("Optimist: soul bound token")); vm.prank(bob); - optimist.safeTransferFrom(bob, sally, 256); + optimist.safeTransferFrom(bob, sally, _getTokenId(bob)); // attempt to transfer to sally vm.expectRevert(bytes("Optimist: soul bound token")); vm.prank(bob); - optimist.safeTransferFrom(bob, sally, 256, bytes("0x")); + optimist.safeTransferFrom(bob, sally, _getTokenId(bob), bytes("0x")); } /** - * @dev It should revert if anybody attemps approve + * @notice approve should revert since Optimist is a SBT. */ - function test_optimist_sbt_approve() external { - attestAllowlist(bob); + function test_approve_reverts() external { + _mockAllowlistTrueFor(bob); // mint as bob vm.prank(bob); @@ -284,16 +506,38 @@ contract OptimistTest is Optimist_Initializer { // attempt to approve sally vm.prank(bob); vm.expectRevert("Optimist: soul bound token"); - optimist.approve(address(attestationStation), 256); + optimist.approve(address(attestationStation), _getTokenId(bob)); - assertEq(optimist.getApproved(256), address(0)); + assertEq(optimist.getApproved(_getTokenId(bob)), address(0)); } /** - * @dev It should be able to burn token + * @notice setApprovalForAll should revert since Optimist is a SBT. */ - function test_optimist_burn() external { - attestAllowlist(bob); + function test_setApprovalForAll_reverts() external { + _mockAllowlistTrueFor(bob); + + // mint as bob + vm.prank(bob); + optimist.mint(bob); + vm.prank(alice_allowlistAttestor); + vm.expectRevert(bytes("Optimist: soul bound token")); + optimist.setApprovalForAll(alice_allowlistAttestor, true); + + // expect approval amount to stil be 0 + assertEq(optimist.getApproved(_getTokenId(bob)), address(0)); + // isApprovedForAll should return false + assertEq( + optimist.isApprovedForAll(alice_allowlistAttestor, alice_allowlistAttestor), + false + ); + } + + /** + * @notice Only owner should be able to burn token. + */ + function test_burn_byOwner_succeeds() external { + _mockAllowlistTrueFor(bob); // mint as bob vm.prank(bob); @@ -301,37 +545,103 @@ contract OptimistTest is Optimist_Initializer { // burn as bob vm.prank(bob); - optimist.burn(256); + optimist.burn(_getTokenId(bob)); // expect bob to have no balance now assertEq(optimist.balanceOf(bob), 0); } /** - * @dev setApprovalForAll should revert as sbt + * @notice Non-owner attempting to burn token should revert. */ - function test_optimist_set_approval_for_all() external { - attestAllowlist(bob); + function test_burn_byNonOwner_reverts() external { + _mockAllowlistTrueFor(bob); // mint as bob vm.prank(bob); optimist.mint(bob); - vm.prank(alice_admin); - vm.expectRevert(bytes("Optimist: soul bound token")); - optimist.setApprovalForAll(alice_admin, true); - // expect approval amount to stil be 0 - assertEq(optimist.getApproved(256), address(0)); - // isApprovedForAll should return false - assertEq(optimist.isApprovedForAll(alice_admin, alice_admin), false); + vm.expectRevert("ERC721: caller is not token owner nor approved"); + // burn as Sally + vm.prank(sally); + optimist.burn(_getTokenId(bob)); + + // expect bob to have still have the token + assertEq(optimist.balanceOf(bob), 1); } /** - * @dev should support erc721 interface + * @notice Should support ERC-721 interface. */ - function test_optimist_supports_interface() external { + function test_supportsInterface_returnsCorrectInterfaceForERC721_succeeds() external { bytes4 iface721 = type(IERC721).interfaceId; - // check that it supports erc721 interface + // check that it supports ERC-721 interface assertEq(optimist.supportsInterface(iface721), true); } + + /** + * @notice Checking that multi-call using the invite & claim flow works correctly, since the + * frontend will be making multicalls to improve UX. The OptimistInviter.claimInvite + * and Optimist.mint will be batched + */ + function test_multicall_batchingClaimAndMint_succeeds() external { + uint256 inviterPrivateKey = 0xbeefbeef; + address inviter = vm.addr(inviterPrivateKey); + + address[] memory addresses = new address[](1); + addresses[0] = inviter; + + vm.prank(eve_inviteGranter); + + // grant invites to Inviter; + optimistInviter.setInviteCounts(addresses, 3); + + // issue a new invite + OptimistInviter.ClaimableInvite memory claimableInvite = optimistInviterHelper + .getClaimableInviteWithNewNonce(inviter); + + // EIP-712 sign with Inviter's private key + + (uint8 v, bytes32 r, bytes32 s) = vm.sign( + inviterPrivateKey, + optimistInviterHelper.getDigest(claimableInvite) + ); + bytes memory signature = abi.encodePacked(r, s, v); + + bytes32 hashedCommit = keccak256(abi.encode(bob, signature)); + + // commit the invite + vm.prank(bob); + optimistInviter.commitInvite(hashedCommit); + + // wait minimum commitment period + vm.warp(optimistInviter.MIN_COMMITMENT_PERIOD() + block.timestamp); + + IMulticall3.Call3[] memory calls = new IMulticall3.Call3[](2); + + // First call is to claim the invite, receiving the attestation + calls[0] = IMulticall3.Call3({ + target: address(optimistInviter), + callData: abi.encodeWithSelector( + optimistInviter.claimInvite.selector, + bob, + claimableInvite, + signature + ), + allowFailure: false + }); + + // Second call is to mint the Optimist NFT + calls[1] = IMulticall3.Call3({ + target: address(optimist), + callData: abi.encodeWithSelector(optimist.mint.selector, bob), + allowFailure: false + }); + + multicall3.aggregate3(calls); + + assertTrue(optimist.isOnAllowList(bob)); + assertEq(optimist.ownerOf(_getTokenId(bob)), bob); + assertEq(optimist.balanceOf(bob), 1); + } } diff --git a/packages/contracts-periphery/contracts/foundry-tests/OptimistAllowlist.t.sol b/packages/contracts-periphery/contracts/foundry-tests/OptimistAllowlist.t.sol new file mode 100644 index 000000000000..0c98cc6dfa57 --- /dev/null +++ b/packages/contracts-periphery/contracts/foundry-tests/OptimistAllowlist.t.sol @@ -0,0 +1,284 @@ +//SPDX-License-Identifier: MIT +pragma solidity 0.8.15; + +/* Testing utilities */ +import { Test } from "forge-std/Test.sol"; +import { AttestationStation } from "../universal/op-nft/AttestationStation.sol"; +import { OptimistAllowlist } from "../universal/op-nft/OptimistAllowlist.sol"; +import { OptimistInviter } from "../universal/op-nft/OptimistInviter.sol"; +import { OptimistInviterHelper } from "../testing/helpers/OptimistInviterHelper.sol"; +import { OptimistConstants } from "../universal/op-nft/libraries/OptimistConstants.sol"; + +contract OptimistAllowlist_Initializer is Test { + event AttestationCreated( + address indexed creator, + address indexed about, + bytes32 indexed key, + bytes val + ); + address internal alice_allowlistAttestor; + address internal sally_coinbaseQuestAttestor; + address internal ted; + + uint256 internal bobPrivateKey; + address internal bob; + + AttestationStation attestationStation; + OptimistAllowlist optimistAllowlist; + OptimistInviter optimistInviter; + + // Helps with EIP-712 signature generation + OptimistInviterHelper optimistInviterHelper; + + function setUp() public { + alice_allowlistAttestor = makeAddr("alice_allowlistAttestor"); + sally_coinbaseQuestAttestor = makeAddr("sally_coinbaseQuestAttestor"); + ted = makeAddr("ted"); + + bobPrivateKey = 0xB0B0B0B0; + bob = vm.addr(bobPrivateKey); + vm.label(bob, "bob"); + + // Give alice and bob and sally some ETH + vm.deal(alice_allowlistAttestor, 1 ether); + vm.deal(sally_coinbaseQuestAttestor, 1 ether); + vm.deal(bob, 1 ether); + vm.deal(ted, 1 ether); + + _initializeContracts(); + } + + function attestAllowlist(address _about) internal { + AttestationStation.AttestationData[] + memory attestationData = new AttestationStation.AttestationData[](1); + // we are using true but it can be any non empty value + attestationData[0] = AttestationStation.AttestationData({ + about: _about, + key: optimistAllowlist.OPTIMIST_CAN_MINT_ATTESTATION_KEY(), + val: bytes("true") + }); + vm.prank(alice_allowlistAttestor); + attestationStation.attest(attestationData); + } + + function attestCoinbaseQuest(address _about) internal { + AttestationStation.AttestationData[] + memory attestationData = new AttestationStation.AttestationData[](1); + // we are using true but it can be any non empty value + attestationData[0] = AttestationStation.AttestationData({ + about: _about, + key: optimistAllowlist.COINBASE_QUEST_ELIGIBLE_ATTESTATION_KEY(), + val: bytes("true") + }); + vm.prank(sally_coinbaseQuestAttestor); + attestationStation.attest(attestationData); + } + + function inviteAndClaim(address claimer) internal { + address[] memory addresses = new address[](1); + addresses[0] = bob; + + vm.prank(alice_allowlistAttestor); + + // grant invites to Bob; + optimistInviter.setInviteCounts(addresses, 3); + + // issue a new invite + OptimistInviter.ClaimableInvite memory claimableInvite = optimistInviterHelper + .getClaimableInviteWithNewNonce(bob); + + // EIP-712 sign with Bob's private key + bytes memory signature = _getSignature( + bobPrivateKey, + optimistInviterHelper.getDigest(claimableInvite) + ); + + bytes32 hashedCommit = keccak256(abi.encode(claimer, signature)); + + // commit the invite + vm.prank(claimer); + optimistInviter.commitInvite(hashedCommit); + + // wait minimum commitment period + vm.warp(optimistInviter.MIN_COMMITMENT_PERIOD() + block.timestamp); + + // reveal and claim the invite + optimistInviter.claimInvite(claimer, claimableInvite, signature); + } + + /** + * @notice Get signature as a bytes blob, since SignatureChecker takes arbitrary signature blobs. + * + */ + function _getSignature(uint256 _signingPrivateKey, bytes32 _digest) + internal + pure + returns (bytes memory) + { + (uint8 v, bytes32 r, bytes32 s) = vm.sign(_signingPrivateKey, _digest); + + bytes memory signature = abi.encodePacked(r, s, v); + return signature; + } + + function _initializeContracts() internal { + attestationStation = new AttestationStation(); + + optimistInviter = new OptimistInviter(alice_allowlistAttestor, attestationStation); + optimistInviter.initialize("OptimistInviter"); + + optimistAllowlist = new OptimistAllowlist( + attestationStation, + alice_allowlistAttestor, + sally_coinbaseQuestAttestor, + address(optimistInviter) + ); + + optimistInviterHelper = new OptimistInviterHelper(optimistInviter, "OptimistInviter"); + } +} + +contract OptimistAllowlistTest is OptimistAllowlist_Initializer { + function test_constructor_succeeds() external { + // expect attestationStation to be set + assertEq(address(optimistAllowlist.ATTESTATION_STATION()), address(attestationStation)); + assertEq(optimistAllowlist.ALLOWLIST_ATTESTOR(), alice_allowlistAttestor); + assertEq(optimistAllowlist.COINBASE_QUEST_ATTESTOR(), sally_coinbaseQuestAttestor); + assertEq(address(optimistAllowlist.OPTIMIST_INVITER()), address(optimistInviter)); + + assertEq(optimistAllowlist.version(), "1.0.0"); + } + + /** + * @notice Base case, a account without any relevant attestations should not be able to mint. + */ + function test_isAllowedToMint_withoutAnyAttestations_fails() external { + assertFalse(optimistAllowlist.isAllowedToMint(bob)); + } + + /** + * @notice After receiving a valid allowlist attestation, the account should be able to mint. + */ + function test_isAllowedToMint_fromAllowlistAttestor_succeeds() external { + attestAllowlist(bob); + assertTrue(optimistAllowlist.isAllowedToMint(bob)); + } + + /** + * @notice After receiving a valid attestation from the Coinbase Quest attestor, + * the account should be able to mint. + */ + function test_isAllowedToMint_fromCoinbaseQuestAttestor_succeeds() external { + attestCoinbaseQuest(bob); + assertTrue(optimistAllowlist.isAllowedToMint(bob)); + } + + /** + * @notice Account that received an attestation from the OptimistInviter contract by going + * through the claim invite flow should be able to mint. + */ + function test_isAllowedToMint_fromInvite_succeeds() external { + inviteAndClaim(bob); + assertTrue(optimistAllowlist.isAllowedToMint(bob)); + } + + /** + * @notice Attestation from the wrong allowlist attestor should not allow minting. + */ + function test_isAllowedToMint_fromWrongAllowlistAttestor_fails() external { + // Ted is not the allowlist attestor + vm.prank(ted); + attestationStation.attest( + bob, + optimistAllowlist.OPTIMIST_CAN_MINT_ATTESTATION_KEY(), + bytes("true") + ); + assertFalse(optimistAllowlist.isAllowedToMint(bob)); + } + + /** + * @notice Coinbase quest attestation from wrong attestor should not allow minting. + */ + function test_isAllowedToMint_fromWrongCoinbaseQuestAttestor_fails() external { + // Ted is not the coinbase quest attestor + vm.prank(ted); + attestationStation.attest( + bob, + optimistAllowlist.COINBASE_QUEST_ELIGIBLE_ATTESTATION_KEY(), + bytes("true") + ); + assertFalse(optimistAllowlist.isAllowedToMint(bob)); + } + + /** + * @notice Claiming an invite on the non-official OptimistInviter contract should not allow + * minting. + */ + function test_isAllowedToMint_fromWrongOptimistInviter_fails() external { + vm.prank(ted); + attestationStation.attest( + bob, + OptimistConstants.OPTIMIST_CAN_MINT_FROM_INVITE_ATTESTATION_KEY, + bytes("true") + ); + assertFalse(optimistAllowlist.isAllowedToMint(bob)); + } + + /** + * @notice Having multiple signals, even if one is invalid, should still allow minting. + */ + function test_isAllowedToMint_withMultipleAttestations_succeeds() external { + attestAllowlist(bob); + attestCoinbaseQuest(bob); + inviteAndClaim(bob); + + assertTrue(optimistAllowlist.isAllowedToMint(bob)); + + // A invalid attestation, as Ted is not allowlist attestor + vm.prank(ted); + attestationStation.attest( + bob, + optimistAllowlist.OPTIMIST_CAN_MINT_ATTESTATION_KEY(), + bytes("true") + ); + + // Since Bob has at least one valid attestation, he should be allowed to mint + assertTrue(optimistAllowlist.isAllowedToMint(bob)); + } + + /** + * @notice Having falsy attestation value should not allow minting. + */ + function test_isAllowedToMint_fromAllowlistAttestorWithFalsyValue_fails() external { + // First sends correct attestation + attestAllowlist(bob); + + bytes32 key = optimistAllowlist.OPTIMIST_CAN_MINT_ATTESTATION_KEY(); + vm.expectEmit(true, true, true, false); + emit AttestationCreated(alice_allowlistAttestor, bob, key, bytes("dsafsds")); + + // Invalidates existing attestation + vm.prank(alice_allowlistAttestor); + attestationStation.attest(bob, key, bytes("")); + + assertFalse(optimistAllowlist.isAllowedToMint(bob)); + } + + /** + * @notice Having falsy attestation value from Coinbase attestor should not allow minting. + */ + function test_isAllowedToMint_fromCoinbaseQuestAttestorWithFalsyValue_fails() external { + // First sends correct attestation + attestAllowlist(bob); + + bytes32 key = optimistAllowlist.OPTIMIST_CAN_MINT_ATTESTATION_KEY(); + vm.expectEmit(true, true, true, true); + emit AttestationCreated(alice_allowlistAttestor, bob, key, bytes("")); + + // Invalidates existing attestation + vm.prank(alice_allowlistAttestor); + attestationStation.attest(bob, key, bytes("")); + + assertFalse(optimistAllowlist.isAllowedToMint(bob)); + } +} diff --git a/packages/contracts-periphery/contracts/universal/op-nft/Optimist.sol b/packages/contracts-periphery/contracts/universal/op-nft/Optimist.sol index 3089c73e6351..85d749df600d 100644 --- a/packages/contracts-periphery/contracts/universal/op-nft/Optimist.sol +++ b/packages/contracts-periphery/contracts/universal/op-nft/Optimist.sol @@ -6,40 +6,54 @@ import { ERC721BurnableUpgradeable } from "@openzeppelin/contracts-upgradeable/token/ERC721/extensions/ERC721BurnableUpgradeable.sol"; import { AttestationStation } from "./AttestationStation.sol"; +import { OptimistAllowlist } from "./OptimistAllowlist.sol"; import { Strings } from "@openzeppelin/contracts/utils/Strings.sol"; /** * @author Optimism Collective * @author Gitcoin - * @title Optimist + * @title Optimist * @notice A Soul Bound Token for real humans only(tm). */ contract Optimist is ERC721BurnableUpgradeable, Semver { + /** + * @notice Attestation key used by the attestor to attest the baseURI. + */ + bytes32 public constant BASE_URI_ATTESTATION_KEY = bytes32("optimist.base-uri"); + + /** + * @notice Attestor who attests to baseURI. + */ + address public immutable BASE_URI_ATTESTOR; + /** * @notice Address of the AttestationStation contract. */ AttestationStation public immutable ATTESTATION_STATION; /** - * @notice Attestor who attests to baseURI and allowlist. + * @notice Address of the OptimistAllowlist contract. */ - address public immutable ATTESTOR; + OptimistAllowlist public immutable OPTIMIST_ALLOWLIST; /** - * @custom:semver 1.0.0 + * @custom:semver 2.0.0 * @param _name Token name. * @param _symbol Token symbol. - * @param _attestor Address of the attestor. + * @param _baseURIAttestor Address of the baseURI attestor. * @param _attestationStation Address of the AttestationStation contract. + * @param _optimistAllowlist Address of the OptimistAllowlist contract */ constructor( string memory _name, string memory _symbol, - address _attestor, - AttestationStation _attestationStation - ) Semver(1, 0, 0) { - ATTESTOR = _attestor; + address _baseURIAttestor, + AttestationStation _attestationStation, + OptimistAllowlist _optimistAllowlist + ) Semver(2, 0, 0) { + BASE_URI_ATTESTOR = _baseURIAttestor; ATTESTATION_STATION = _attestationStation; + OPTIMIST_ALLOWLIST = _optimistAllowlist; initialize(_name, _symbol); } @@ -76,7 +90,7 @@ contract Optimist is ERC721BurnableUpgradeable, Semver { string( abi.encodePacked( ATTESTATION_STATION.attestations( - ATTESTOR, + BASE_URI_ATTESTOR, address(this), bytes32("optimist.base-uri") ) @@ -105,17 +119,15 @@ contract Optimist is ERC721BurnableUpgradeable, Semver { } /** - * @notice Checks whether a given address is allowed to mint the Optimist NFT yet. Since the - * Optimist NFT will also be used as part of the Citizens House, mints are currently - * restricted. Eventually anyone will be able to mint. + * @notice Checks OptimistAllowlist to determine whether a given address is allowed to mint + * the Optimist NFT. Since the Optimist NFT will also be used as part of the + * Citizens House, mints are currently restricted. Eventually anyone will be able + * to mint. * * @return Whether or not the address is allowed to mint yet. */ function isOnAllowList(address _recipient) public view returns (bool) { - return - ATTESTATION_STATION - .attestations(ATTESTOR, _recipient, bytes32("optimist.can-mint")) - .length > 0; + return OPTIMIST_ALLOWLIST.isAllowedToMint(_recipient); } /** diff --git a/packages/contracts-periphery/contracts/universal/op-nft/OptimistAllowlist.sol b/packages/contracts-periphery/contracts/universal/op-nft/OptimistAllowlist.sol new file mode 100644 index 000000000000..c3df49e2077c --- /dev/null +++ b/packages/contracts-periphery/contracts/universal/op-nft/OptimistAllowlist.sol @@ -0,0 +1,159 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.15; + +import { Semver } from "@eth-optimism/contracts-bedrock/contracts/universal/Semver.sol"; +import { AttestationStation } from "./AttestationStation.sol"; +import { OptimistConstants } from "./libraries/OptimistConstants.sol"; + +/** + * @title OptimistAllowlist + * @notice Source of truth for whether an address is able to mint an Optimist NFT. + isAllowedToMint function checks various signals to return boolean value for whether an + address is eligible or not. + */ +contract OptimistAllowlist is Semver { + /** + * @notice Attestation key used by the AllowlistAttestor to manually add addresses to the + * allowlist. + */ + bytes32 public constant OPTIMIST_CAN_MINT_ATTESTATION_KEY = bytes32("optimist.can-mint"); + + /** + * @notice Attestation key used by Coinbase to issue attestations for Quest participants. + */ + bytes32 public constant COINBASE_QUEST_ELIGIBLE_ATTESTATION_KEY = + bytes32("coinbase.quest-eligible"); + + /** + * @notice Address of the AttestationStation contract. + */ + AttestationStation public immutable ATTESTATION_STATION; + + /** + * @notice Attestor that issues 'optimist.can-mint' attestations. + */ + address public immutable ALLOWLIST_ATTESTOR; + + /** + * @notice Attestor that issues 'coinbase.quest-eligible' attestations. + */ + address public immutable COINBASE_QUEST_ATTESTOR; + + /** + * @notice Address of OptimistInviter contract that issues 'optimist.can-mint-from-invite' + * attestations. + */ + address public immutable OPTIMIST_INVITER; + + /** + * @custom:semver 1.0.0 + * + * @param _attestationStation Address of the AttestationStation contract. + * @param _allowlistAttestor Address of the allowlist attestor. + * @param _coinbaseQuestAttestor Address of the Coinbase Quest attestor. + * @param _optimistInviter Address of the OptimistInviter contract. + */ + constructor( + AttestationStation _attestationStation, + address _allowlistAttestor, + address _coinbaseQuestAttestor, + address _optimistInviter + ) Semver(1, 0, 0) { + ATTESTATION_STATION = _attestationStation; + ALLOWLIST_ATTESTOR = _allowlistAttestor; + COINBASE_QUEST_ATTESTOR = _coinbaseQuestAttestor; + OPTIMIST_INVITER = _optimistInviter; + } + + /** + * @notice Checks whether a given address is allowed to mint the Optimist NFT yet. Since the + * Optimist NFT will also be used as part of the Citizens House, mints are currently + * restricted. Eventually anyone will be able to mint. + * + * Currently, address is allowed to mint if it satisfies any of the following: + * 1) Has a valid 'optimist.can-mint' attestation from the allowlist attestor. + * 2) Has a valid 'coinbase.quest-eligible' attestation from Coinbase Quest attestor + * 3) Has a valid 'optimist.can-mint-from-invite' attestation from the OptimistInviter + * contract. + * + * @param _claimer Address to check. + * + * @return Whether or not the address is allowed to mint yet. + */ + function isAllowedToMint(address _claimer) public view returns (bool) { + return + _hasAttestationFromAllowlistAttestor(_claimer) || + _hasAttestationFromCoinbaseQuestAttestor(_claimer) || + _hasAttestationFromOptimistInviter(_claimer); + } + + /** + * @notice Checks whether an address has a valid 'optimist.can-mint' attestation from the + * allowlist attestor. + * + * @param _claimer Address to check. + * + * @return Whether or not the address has a valid attestation. + */ + function _hasAttestationFromAllowlistAttestor(address _claimer) internal view returns (bool) { + // Expected attestation value is bytes32("true") + return + _hasValidAttestation(ALLOWLIST_ATTESTOR, _claimer, OPTIMIST_CAN_MINT_ATTESTATION_KEY); + } + + /** + * @notice Checks whether an address has a valid attestation from the Coinbase attestor. + * + * @param _claimer Address to check. + * + * @return Whether or not the address has a valid attestation. + */ + function _hasAttestationFromCoinbaseQuestAttestor(address _claimer) + internal + view + returns (bool) + { + // Expected attestation value is bytes32("true") + return + _hasValidAttestation( + COINBASE_QUEST_ATTESTOR, + _claimer, + COINBASE_QUEST_ELIGIBLE_ATTESTATION_KEY + ); + } + + /** + * @notice Checks whether an address has a valid attestation from the OptimistInviter contract. + * + * @param _claimer Address to check. + * + * @return Whether or not the address has a valid attestation. + */ + function _hasAttestationFromOptimistInviter(address _claimer) internal view returns (bool) { + // Expected attestation value is the inviter's address + return + _hasValidAttestation( + OPTIMIST_INVITER, + _claimer, + OptimistConstants.OPTIMIST_CAN_MINT_FROM_INVITE_ATTESTATION_KEY + ); + } + + /** + * @notice Checks whether an address has a valid truthy attestation. + * Any attestation val other than bytes32("") is considered truthy. + * + * @param _creator Address that made the attestation. + * @param _about Address attestation is about. + * @param _key Key of the attestation. + * + * @return Whether or not the address has a valid truthy attestation. + */ + function _hasValidAttestation( + address _creator, + address _about, + bytes32 _key + ) internal view returns (bool) { + return ATTESTATION_STATION.attestations(_creator, _about, _key).length > 0; + } +} diff --git a/packages/contracts-periphery/deploy/op-nft/AttestationStationProxy.ts b/packages/contracts-periphery/deploy/op-nft/AttestationStationProxy.ts index 9e391ed184d0..769033ab2cfd 100644 --- a/packages/contracts-periphery/deploy/op-nft/AttestationStationProxy.ts +++ b/packages/contracts-periphery/deploy/op-nft/AttestationStationProxy.ts @@ -61,9 +61,9 @@ const deployFn: DeployFunction = async (hre: HardhatRuntimeEnvironment) => { addr ) - const implementation = await Proxy.callStatic.implementation({ - from: ethers.constants.AddressZero, - }) + const implementation = await Proxy.connect( + ethers.constants.AddressZero + ).callStatic.implementation() console.log(`implementation is set to ${implementation}`) if ( getAddress(implementation) !== @@ -82,9 +82,9 @@ const deployFn: DeployFunction = async (hre: HardhatRuntimeEnvironment) => { } const l2ProxyOwnerAddress = deployConfig.l2ProxyOwnerAddress - const admin = await Proxy.callStatic.admin({ - from: ethers.constants.AddressZero, - }) + const admin = await Proxy.connect( + ethers.constants.AddressZero + ).callStatic.admin() console.log(`admin is set to ${admin}`) if (getAddress(admin) !== getAddress(l2ProxyOwnerAddress)) { console.log('admin not set correctly') @@ -99,7 +99,7 @@ const deployFn: DeployFunction = async (hre: HardhatRuntimeEnvironment) => { console.log('Contract deployment complete') await assertContractVariable(Proxy, 'admin', l2ProxyOwnerAddress) - await assertContractVariable(AttestationStation, 'version', '1.0.0') + await assertContractVariable(AttestationStation, 'version', '1.1.0') } deployFn.tags = ['AttestationStationProxy', 'OptimistEnvironment'] diff --git a/packages/contracts-periphery/deploy/op-nft/OptimistInviterImpl.ts b/packages/contracts-periphery/deploy/op-nft/OptimistInviterImpl.ts new file mode 100644 index 000000000000..7971385fca3f --- /dev/null +++ b/packages/contracts-periphery/deploy/op-nft/OptimistInviterImpl.ts @@ -0,0 +1,43 @@ +/* Imports: External */ +import { DeployFunction } from 'hardhat-deploy/dist/types' +import { HardhatRuntimeEnvironment } from 'hardhat/types' + +import '@nomiclabs/hardhat-ethers' +import '@eth-optimism/hardhat-deploy-config' +import 'hardhat-deploy' +import type { DeployConfig } from '../../src' + +const deployFn: DeployFunction = async (hre: HardhatRuntimeEnvironment) => { + const deployConfig = hre.deployConfig as DeployConfig + + const { deployer } = await hre.getNamedAccounts() + + console.log(`Deploying OptimistInviter implementation with ${deployer}`) + + const Deployment__AttestationStation = await hre.deployments.get( + 'AttestationStationProxy' + ) + const attestationStationAddress = Deployment__AttestationStation.address + + console.log(`Using ${attestationStationAddress} as the ATTESTATION_STATION`) + console.log( + `Using ${deployConfig.optimistInviterInviteGranter} as INVITE_GRANTER` + ) + + const { deploy } = await hre.deployments.deterministic('OptimistInviter', { + salt: hre.ethers.utils.solidityKeccak256(['string'], ['OptimistInviter']), + from: deployer, + args: [ + deployConfig.optimistInviterInviteGranter, + attestationStationAddress, + ], + log: true, + }) + + await deploy() +} + +deployFn.tags = ['OptimistInviter', 'OptimistEnvironment'] +deployFn.dependencies = ['AttestationStationProxy'] + +export default deployFn diff --git a/packages/contracts-periphery/deploy/op-nft/OptimistInviterProxy.ts b/packages/contracts-periphery/deploy/op-nft/OptimistInviterProxy.ts new file mode 100644 index 000000000000..d9c019c55cd2 --- /dev/null +++ b/packages/contracts-periphery/deploy/op-nft/OptimistInviterProxy.ts @@ -0,0 +1,195 @@ +/* Imports: External */ +import assert from 'assert' + +import { DeployFunction } from 'hardhat-deploy/dist/types' +import { HardhatRuntimeEnvironment } from 'hardhat/types' +import '@eth-optimism/hardhat-deploy-config' +import '@nomiclabs/hardhat-ethers' +import 'hardhat-deploy' +import { assertContractVariable } from '@eth-optimism/contracts-bedrock/src/deploy-utils' +import { ethers, utils } from 'ethers' + +import type { DeployConfig } from '../../src' + +const { getAddress } = utils + +// Required conditions before deploying - Specified in `deployFn.dependencies` +// - AttestationStationProxy is deployed and points to the correct implementation +// - OptimistInviterImpl is deployed +// +// Steps +// 1. Deploy OptimistInviterProxy +// 2. Point the newly deployed proxy to the implementation, if it hasn't been done already +// 3. Update the admin of the proxy to the l2ProxyOwnerAddress, if it hasn't been done already +// 4. Basic sanity checks for contract variables + +const deployFn: DeployFunction = async (hre: HardhatRuntimeEnvironment) => { + const deployConfig = hre.deployConfig as DeployConfig + + // Deployer should be set in hardhat.config.ts + const { deployer } = await hre.getNamedAccounts() + + // We want the ability to deploy to a deterministic address, so we need the init bytecode to be + // consistent across deployments. The ddd will quickly transfer the ownership of the Proxy to a + // multisig after deployment. + // + // We need a consistent ddd, since the Proxy takes a `_admin` constructor argument, which + // affects the init bytecode and hence deployed address. + const ddd = deployConfig.ddd + + if (getAddress(deployer) !== getAddress(ddd)) { + // Not a hard requirement. We can deploy with any account and just set the `_admin` to the + // ddd, but requiring that the deployer is the same as the ddd minimizes number of hot wallets + // we need to keep track of during deployment. + throw new Error('Must deploy with the ddd') + } + + // Get the up to date deployment of the OptimistInviter contract + const Deployment__OptimistInviterImpl = await hre.deployments.get( + 'OptimistInviter' + ) + + console.log(`Deploying OptimistInviterProxy with ${deployer}`) + + // Deploys the Proxy.sol contract with the `_admin` constructor param set to the ddd (=== deployer). + const { deploy } = await hre.deployments.deterministic( + 'OptimistInviterProxy', + { + salt: hre.ethers.utils.solidityKeccak256( + ['string'], + ['OptimistInviterProxy'] + ), + contract: 'Proxy', + from: deployer, + args: [deployer], + log: true, + } + ) + + // Deploy the Proxy contract + await deploy() + + const Deployment__OptimistInviterProxy = await hre.deployments.get( + 'OptimistInviterProxy' + ) + console.log( + `OptimistProxy deployed to ${Deployment__OptimistInviterProxy.address}` + ) + + // Deployed Proxy.sol contract + const Proxy = await hre.ethers.getContractAt( + 'Proxy', + Deployment__OptimistInviterProxy.address + ) + + // Deployed Proxy.sol contract with the OptimistInviter interface + const OptimistInviter = await hre.ethers.getContractAt( + 'OptimistInviter', + Deployment__OptimistInviterProxy.address + ) + + // Gets the current implementation address the proxy is pointing to. + // callStatic is used since the `Proxy.implementation()` is not a view function and ethers will + // try to make a transaction if we don't use callStatic. Using the zero address as `from` lets us + // call functions on the proxy and not trigger the delegatecall. See Proxy.sol proxyCallIfNotAdmin + // modifier for more details. + const implementation = await Proxy.connect( + ethers.constants.AddressZero + ).callStatic.implementation() + console.log(`implementation set to ${implementation}`) + + if ( + getAddress(implementation) !== + getAddress(Deployment__OptimistInviterImpl.address) + ) { + // If the proxy isn't pointing to the correct implementation, we need to set it to the correct + // one, then call initialize() in the proxy's context. + console.log( + 'implementation not set to OptimistInviter implementation contract' + ) + console.log( + `Setting implementation to ${Deployment__OptimistInviterImpl.address}` + ) + + const name = deployConfig.optimistInviterName + + // Create the calldata for the call to `initialize()` + const calldata = OptimistInviter.interface.encodeFunctionData( + 'initialize', + [name] + ) + + // ethers.Signer for the ddd + const dddSigner = await hre.ethers.provider.getSigner(deployer) + + // Point the proxy to the deployed OptimistInviter implementation contract, + // and call `initialize()` in the proxy's context + const tx = await Proxy.connect(dddSigner).upgradeToAndCall( + Deployment__OptimistInviterImpl.address, + calldata + ) + const receipt = await tx.wait() + console.log(`implementation set in ${receipt.transactionHash}`) + } else { + console.log( + 'implementation already set to OptimistInviter implementation contract' + ) + } + + const l2ProxyOwnerAddress = deployConfig.l2ProxyOwnerAddress + + // Get the current proxy admin address + const admin = await Proxy.connect( + ethers.constants.AddressZero + ).callStatic.admin() + + console.log(`admin currently set to ${admin}`) + + if (getAddress(admin) !== getAddress(l2ProxyOwnerAddress)) { + // If the proxy admin isn't the l2ProxyOwnerAddress, we need to update it + // We're assuming that the proxy admin is the ddd right now. + console.log('admin is not set to the l2ProxyOwnerAddress') + console.log(`Setting admin to ${l2ProxyOwnerAddress}`) + + // ethers.Signer for the ddd + const dddSigner = await hre.ethers.provider.getSigner(deployer) + + // change admin to the l2ProxyOwnerAddress + const tx = await Proxy.connect(dddSigner).changeAdmin(l2ProxyOwnerAddress) + const receipt = await tx.wait() + console.log(`admin set in ${receipt.transactionHash}`) + } else { + console.log('admin already set to proxy owner address') + } + + const Deployment__AttestationStation = await hre.deployments.get( + 'AttestationStationProxy' + ) + + await assert( + getAddress( + await Proxy.connect(ethers.constants.AddressZero).callStatic.admin() + ) === getAddress(l2ProxyOwnerAddress) + ) + + await assertContractVariable(OptimistInviter, 'version', '1.0.0') + + await assertContractVariable( + OptimistInviter, + 'INVITE_GRANTER', + deployConfig.optimistInviterInviteGranter + ) + + await assertContractVariable( + OptimistInviter, + 'ATTESTATION_STATION', + Deployment__AttestationStation.address + ) + + await assertContractVariable(OptimistInviter, 'EIP712_VERSION', '1.0.0') +} + +deployFn.tags = ['OptimistInviterProxy', 'OptimistEnvironment'] +deployFn.dependencies = ['AttestationStationProxy', 'OptimistInviter'] + +export default deployFn diff --git a/packages/contracts-periphery/deploy/op-nft/OptimistProxy.ts b/packages/contracts-periphery/deploy/op-nft/OptimistProxy.ts index 71c403610063..1575b97f373a 100644 --- a/packages/contracts-periphery/deploy/op-nft/OptimistProxy.ts +++ b/packages/contracts-periphery/deploy/op-nft/OptimistProxy.ts @@ -48,9 +48,9 @@ const deployFn: DeployFunction = async (hre: HardhatRuntimeEnvironment) => { Deployment__OptimistProxy.address ) - const implementation = await Proxy.callStatic.implementation({ - from: ethers.constants.AddressZero, - }) + const implementation = await Proxy.connect( + ethers.constants.AddressZero + ).callStatic.implementation() console.log(`implementation set to ${implementation}`) if (getAddress(implementation) !== getAddress(Deployment__Optimist.address)) { console.log('implementation not set to Optimist contract') @@ -75,9 +75,9 @@ const deployFn: DeployFunction = async (hre: HardhatRuntimeEnvironment) => { } const l2ProxyOwnerAddress = deployConfig.l2ProxyOwnerAddress - const admin = await Proxy.callStatic.admin({ - from: ethers.constants.AddressZero, - }) + const admin = await Proxy.connect( + ethers.constants.AddressZero + ).callStatic.admin() console.log(`admin set to ${admin}`) if (getAddress(admin) !== getAddress(l2ProxyOwnerAddress)) { console.log('detected admin is not set') @@ -96,7 +96,7 @@ const deployFn: DeployFunction = async (hre: HardhatRuntimeEnvironment) => { await assertContractVariable(Proxy, 'admin', l2ProxyOwnerAddress) await assertContractVariable(Optimist, 'name', deployConfig.optimistName) - await assertContractVariable(Optimist, 'verson', '1.0.0') + await assertContractVariable(Optimist, 'version', '1.0.0') await assertContractVariable(Optimist, 'symbol', deployConfig.optimistSymbol) await assertContractVariable( Optimist, diff --git a/packages/contracts-periphery/foundry.toml b/packages/contracts-periphery/foundry.toml index 6d2ab931a011..c2ba2a5128c0 100644 --- a/packages/contracts-periphery/foundry.toml +++ b/packages/contracts-periphery/foundry.toml @@ -16,9 +16,13 @@ remappings = [ '@rari-capital/solmate/=node_modules/@rari-capital/solmate', 'forge-std/=node_modules/forge-std/src', 'ds-test/=node_modules/ds-test/src', + 'multicall/=lib/multicall', '@openzeppelin/contracts/=node_modules/@openzeppelin/contracts/', '@openzeppelin/contracts-upgradeable/=node_modules/@openzeppelin/contracts-upgradeable/', '@eth-optimism/contracts-bedrock/=../../node_modules/@eth-optimism/contracts-bedrock', ] # The metadata hash can be removed from the bytecode by setting "none" bytecode_hash = "none" +libs = ["node_modules", "lib"] +# Required to use `deployCode` to deploy the multicall contract which has incompatible version +fs_permissions = [{ access = "read", path = "./forge-artifacts/Multicall3.sol/Multicall3.json"}] diff --git a/packages/contracts-periphery/lib/multicall b/packages/contracts-periphery/lib/multicall new file mode 160000 index 000000000000..a1fa0644fa41 --- /dev/null +++ b/packages/contracts-periphery/lib/multicall @@ -0,0 +1 @@ +Subproject commit a1fa0644fa412cd3237ef7081458ecb2ffad7dbe diff --git a/packages/contracts-periphery/src/config/deploy.ts b/packages/contracts-periphery/src/config/deploy.ts index f5ff0e732671..a11a251a66d2 100644 --- a/packages/contracts-periphery/src/config/deploy.ts +++ b/packages/contracts-periphery/src/config/deploy.ts @@ -29,10 +29,20 @@ export interface DeployConfig { optimistSymbol: string /** - * Address of the priviledged attestor for the Optimist contract. + * Address of the privileged attestor for the Optimist contract. */ attestorAddress: string + /** + * Address of the privileged account for the OptimistInviter contract that can grant invites. + */ + optimistInviterInviteGranter: string + + /** + * Name of OptimistInviter contract, used for the EIP712 domain separator. + */ + optimistInviterName: string + /** * Address of the owner of the proxies on L2. There will be a ProxyAdmin deployed as a predeploy * after bedrock, so the owner of proxies should be updated to that after the upgrade. @@ -63,6 +73,12 @@ export const configSpec: DeployConfigSpec = { attestorAddress: { type: 'address', }, + optimistInviterInviteGranter: { + type: 'address', + }, + optimistInviterName: { + type: 'string', + }, l2ProxyOwnerAddress: { type: 'address', }, diff --git a/packages/data-transport-layer/src/services/l1-ingestion/service.ts b/packages/data-transport-layer/src/services/l1-ingestion/service.ts index d6fcac9ebaaf..2eef4dd39979 100644 --- a/packages/data-transport-layer/src/services/l1-ingestion/service.ts +++ b/packages/data-transport-layer/src/services/l1-ingestion/service.ts @@ -277,6 +277,12 @@ export class L1IngestionService extends BaseService { depositTargetL1Block, handleEventsTransactionEnqueued ) + } else { + this.logger.info('Deposit shutoff reached', { + depositTargetL1Block, + highestSyncedL1Block, + depositShutoffBlock, + }) } await this._syncEvents(