From 957e7f47a26fa2174efb433987f34b5109d0ebf5 Mon Sep 17 00:00:00 2001 From: Minhyuk Kim Date: Mon, 28 Oct 2024 14:29:05 -0600 Subject: [PATCH] Implement mock rpc in gossip_test and apply review --- op-node/p2p/gossip_test.go | 129 +++++++++++++++++++------ op-service/signer/blockpayload_args.go | 3 +- op-service/signer/client.go | 2 +- 3 files changed, 103 insertions(+), 31 deletions(-) diff --git a/op-node/p2p/gossip_test.go b/op-node/p2p/gossip_test.go index fc8ee98dd5b03..0831b8e90099b 100644 --- a/op-node/p2p/gossip_test.go +++ b/op-node/p2p/gossip_test.go @@ -3,32 +3,32 @@ package p2p import ( "bytes" "context" + "crypto/ecdsa" "fmt" "io" "math/big" "testing" "time" - "github.com/ethereum-optimism/optimism/op-e2e/e2eutils" "github.com/ethereum-optimism/optimism/op-node/rollup" "github.com/golang/snappy" - opsigner "github.com/ethereum-optimism/optimism/op-service/signer" - // "github.com/ethereum-optimism/optimism/op-service/eth" "github.com/ethereum-optimism/optimism/op-service/eth" + oprpc "github.com/ethereum-optimism/optimism/op-service/rpc" + opsigner "github.com/ethereum-optimism/optimism/op-service/signer" + "github.com/ethereum-optimism/optimism/op-service/testlog" "github.com/ethereum-optimism/optimism/op-service/testutils" + "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common/hexutil" "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/crypto" - "github.com/ethereum/go-ethereum/log" + "github.com/ethereum/go-ethereum/rpc" pubsub "github.com/libp2p/go-libp2p-pubsub" pubsub_pb "github.com/libp2p/go-libp2p-pubsub/pb" "github.com/libp2p/go-libp2p/core/peer" "github.com/stretchr/testify/require" - - "github.com/ethereum-optimism/optimism/op-service/testlog" ) func TestGuardGossipValidator(t *testing.T) { @@ -63,30 +63,30 @@ func TestVerifyBlockSignature(t *testing.T) { L2ChainID: big.NewInt(100), } peerId := peer.ID("foo") - secrets, err := e2eutils.DefaultMnemonicConfig.Secrets() + secrets, err := crypto.GenerateKey() require.NoError(t, err) msg := []byte("any msg") t.Run("Valid", func(t *testing.T) { - runCfg := &testutils.MockRuntimeConfig{P2PSeqAddress: crypto.PubkeyToAddress(secrets.SequencerP2P.PublicKey)} - signer := &PreparedSigner{Signer: NewLocalSigner(secrets.SequencerP2P)} + runCfg := &testutils.MockRuntimeConfig{P2PSeqAddress: crypto.PubkeyToAddress(secrets.PublicKey)} + signer := &PreparedSigner{Signer: NewLocalSigner(secrets)} sig, err := signer.Sign(context.Background(), SigningDomainBlocksV1, cfg.L2ChainID, msg) require.NoError(t, err) - result := verifyBlockSignature(logger, cfg, runCfg, peerId, sig[:65], msg) + result := verifyBlockSignature(logger, cfg, runCfg, peerId, sig[:], msg) require.Equal(t, pubsub.ValidationAccept, result) }) t.Run("WrongSigner", func(t *testing.T) { runCfg := &testutils.MockRuntimeConfig{P2PSeqAddress: common.HexToAddress("0x1234")} - signer := &PreparedSigner{Signer: NewLocalSigner(secrets.SequencerP2P)} + signer := &PreparedSigner{Signer: NewLocalSigner(secrets)} sig, err := signer.Sign(context.Background(), SigningDomainBlocksV1, cfg.L2ChainID, msg) require.NoError(t, err) - result := verifyBlockSignature(logger, cfg, runCfg, peerId, sig[:65], msg) + result := verifyBlockSignature(logger, cfg, runCfg, peerId, sig[:], msg) require.Equal(t, pubsub.ValidationReject, result) }) t.Run("InvalidSignature", func(t *testing.T) { - runCfg := &testutils.MockRuntimeConfig{P2PSeqAddress: crypto.PubkeyToAddress(secrets.SequencerP2P.PublicKey)} + runCfg := &testutils.MockRuntimeConfig{P2PSeqAddress: crypto.PubkeyToAddress(secrets.PublicKey)} sig := make([]byte, 65) result := verifyBlockSignature(logger, cfg, runCfg, peerId, sig, msg) require.Equal(t, pubsub.ValidationReject, result) @@ -94,50 +94,90 @@ func TestVerifyBlockSignature(t *testing.T) { t.Run("NoSequencer", func(t *testing.T) { runCfg := &testutils.MockRuntimeConfig{} - signer := &PreparedSigner{Signer: NewLocalSigner(secrets.SequencerP2P)} + signer := &PreparedSigner{Signer: NewLocalSigner(secrets)} sig, err := signer.Sign(context.Background(), SigningDomainBlocksV1, cfg.L2ChainID, msg) require.NoError(t, err) - result := verifyBlockSignature(logger, cfg, runCfg, peerId, sig[:65], msg) + result := verifyBlockSignature(logger, cfg, runCfg, peerId, sig[:], msg) require.Equal(t, pubsub.ValidationIgnore, result) }) } +type mockRemoteSigner struct { + priv *ecdsa.PrivateKey +} + +func (t *mockRemoteSigner) SignBlockPayload(args opsigner.BlockPayloadArgs) (hexutil.Bytes, error) { + signingHash, err := args.ToSigningHash() + if err != nil { + return nil, err + } + signature, err := crypto.Sign(signingHash[:], t.priv) + if err != nil { + return nil, err + } + return signature, nil +} + func TestVerifyBlockSignatureWithRemoteSigner(t *testing.T) { - t.Skipf("Implement mocking of client rpc calls") + secrets, err := crypto.GenerateKey() + require.NoError(t, err) + + remoteSigner := &mockRemoteSigner{secrets} + server := oprpc.NewServer( + "127.0.0.1", + 8545, + "test", + oprpc.WithAPIs([]rpc.API{ + { + Namespace: "opsigner", + Service: remoteSigner, + }, + }), + ) + + require.NoError(t, server.Start()) + defer func() { + _ = server.Stop() + }() logger := testlog.Logger(t, log.LevelCrit) cfg := &rollup.Config{ L2ChainID: big.NewInt(100), } + peerId := peer.ID("foo") - secrets, err := e2eutils.DefaultMnemonicConfig.Secrets() - require.NoError(t, err) msg := []byte("any msg") + signerCfg := opsigner.NewCLIConfig() + signerCfg.Endpoint = "http://127.0.0.1:8545" + signerCfg.TLSConfig.TLSKey = "" + signerCfg.TLSConfig.TLSCert = "" + signerCfg.TLSConfig.TLSCaCert = "" + t.Run("Valid", func(t *testing.T) { - runCfg := &testutils.MockRuntimeConfig{P2PSeqAddress: crypto.PubkeyToAddress(secrets.SequencerP2P.PublicKey)} - remoteSigner, err := NewRemoteSigner(logger, opsigner.NewCLIConfig()) + runCfg := &testutils.MockRuntimeConfig{P2PSeqAddress: crypto.PubkeyToAddress(secrets.PublicKey)} + remoteSigner, err := NewRemoteSigner(logger, signerCfg) require.NoError(t, err) signer := &PreparedSigner{Signer: remoteSigner} sig, err := signer.Sign(context.Background(), SigningDomainBlocksV1, cfg.L2ChainID, msg) require.NoError(t, err) - result := verifyBlockSignature(logger, cfg, runCfg, peerId, sig[:65], msg) + result := verifyBlockSignature(logger, cfg, runCfg, peerId, sig[:], msg) require.Equal(t, pubsub.ValidationAccept, result) }) t.Run("WrongSigner", func(t *testing.T) { runCfg := &testutils.MockRuntimeConfig{P2PSeqAddress: common.HexToAddress("0x1234")} - remoteSigner, err := NewRemoteSigner(logger, opsigner.NewCLIConfig()) + remoteSigner, err := NewRemoteSigner(logger, signerCfg) require.NoError(t, err) signer := &PreparedSigner{Signer: remoteSigner} sig, err := signer.Sign(context.Background(), SigningDomainBlocksV1, cfg.L2ChainID, msg) require.NoError(t, err) - result := verifyBlockSignature(logger, cfg, runCfg, peerId, sig[:65], msg) + result := verifyBlockSignature(logger, cfg, runCfg, peerId, sig[:], msg) require.Equal(t, pubsub.ValidationReject, result) }) t.Run("InvalidSignature", func(t *testing.T) { - runCfg := &testutils.MockRuntimeConfig{P2PSeqAddress: crypto.PubkeyToAddress(secrets.SequencerP2P.PublicKey)} + runCfg := &testutils.MockRuntimeConfig{P2PSeqAddress: crypto.PubkeyToAddress(secrets.PublicKey)} sig := make([]byte, 65) result := verifyBlockSignature(logger, cfg, runCfg, peerId, sig, msg) require.Equal(t, pubsub.ValidationReject, result) @@ -145,14 +185,45 @@ func TestVerifyBlockSignatureWithRemoteSigner(t *testing.T) { t.Run("NoSequencer", func(t *testing.T) { runCfg := &testutils.MockRuntimeConfig{} - remoteSigner, err := NewRemoteSigner(logger, opsigner.NewCLIConfig()) + remoteSigner, err := NewRemoteSigner(logger, signerCfg) require.NoError(t, err) signer := &PreparedSigner{Signer: remoteSigner} sig, err := signer.Sign(context.Background(), SigningDomainBlocksV1, cfg.L2ChainID, msg) require.NoError(t, err) - result := verifyBlockSignature(logger, cfg, runCfg, peerId, sig[:65], msg) + result := verifyBlockSignature(logger, cfg, runCfg, peerId, sig[:], msg) require.Equal(t, pubsub.ValidationIgnore, result) }) + + t.Run("RemoteSignerNoTLS", func(t *testing.T) { + signerCfg := opsigner.NewCLIConfig() + signerCfg.Endpoint = "http://127.0.0.1:8545" + signerCfg.TLSConfig.TLSKey = "invalid" + signerCfg.TLSConfig.TLSCert = "invalid" + signerCfg.TLSConfig.TLSCaCert = "invalid" + + _, err := NewRemoteSigner(logger, signerCfg) + require.Error(t, err) + }) + + t.Run("RemoteSignerInvalidEndpoint", func(t *testing.T) { + signerCfg := opsigner.NewCLIConfig() + signerCfg.Endpoint = "Invalid" + signerCfg.TLSConfig.TLSKey = "" + signerCfg.TLSConfig.TLSCert = "" + signerCfg.TLSConfig.TLSCaCert = "" + _, err := NewRemoteSigner(logger, signerCfg) + require.Error(t, err) + }) + + t.Run("RemoteSignerInvalidEndpoint", func(t *testing.T) { + signerCfg := opsigner.NewCLIConfig() + signerCfg.Endpoint = "Invalid" + signerCfg.TLSConfig.TLSKey = "" + signerCfg.TLSConfig.TLSCert = "" + signerCfg.TLSConfig.TLSCaCert = "" + _, err := NewRemoteSigner(logger, signerCfg) + require.Error(t, err) + }) } type MarshalSSZ interface { @@ -200,10 +271,10 @@ func TestBlockValidator(t *testing.T) { cfg := &rollup.Config{ L2ChainID: big.NewInt(100), } - secrets, err := e2eutils.DefaultMnemonicConfig.Secrets() + secrets, err := crypto.GenerateKey() require.NoError(t, err) - runCfg := &testutils.MockRuntimeConfig{P2PSeqAddress: crypto.PubkeyToAddress(secrets.SequencerP2P.PublicKey)} - signer := &PreparedSigner{Signer: NewLocalSigner(secrets.SequencerP2P)} + runCfg := &testutils.MockRuntimeConfig{P2PSeqAddress: crypto.PubkeyToAddress(secrets.PublicKey)} + signer := &PreparedSigner{Signer: NewLocalSigner(secrets)} // Params Set 2: Call the validation function peerID := peer.ID("foo") diff --git a/op-service/signer/blockpayload_args.go b/op-service/signer/blockpayload_args.go index 64e2acb1ffdae..8239bc0967d6b 100644 --- a/op-service/signer/blockpayload_args.go +++ b/op-service/signer/blockpayload_args.go @@ -40,7 +40,8 @@ func (args *BlockPayloadArgs) Check() error { return nil } -// ToSigningHash hashes +// ToSigningHash creates a signingHash from the block payload args. +// Uses the hashing scheme from https://github.com/ethereum-optimism/specs/blob/main/specs/protocol/rollup-node-p2p.md#block-signatures func (args *BlockPayloadArgs) ToSigningHash() (common.Hash, error) { if err := args.Check(); err != nil { return common.Hash{}, err diff --git a/op-service/signer/client.go b/op-service/signer/client.go index 1bbbed9f97d2e..05620960225ff 100644 --- a/op-service/signer/client.go +++ b/op-service/signer/client.go @@ -121,7 +121,7 @@ func (s *SignerClient) SignBlockPayload(ctx context.Context, args *BlockPayloadA return [65]byte{}, fmt.Errorf("opsigner_signBlockPayload failed: %w", err) } - if len(result) < 65 { + if len(result) != 65 { return [65]byte{}, fmt.Errorf("invalid signature: %s", result.String()) }