Skip to content

op-signer, op-node: fix block signer utils #14478

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
protolambda merged 2 commits into from
Feb 21, 2025
Merged

op-signer, op-node: fix block signer utils #14478

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bbdc860
op-service: fix block signer utils
protolambda Feb 20, 2025
93818dc
op-service: improve block signer
protolambda Feb 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion op-node/p2p/gossip.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (

"github.com/ethereum-optimism/optimism/op-node/rollup"
"github.com/ethereum-optimism/optimism/op-service/eth"
opsigner "github.com/ethereum-optimism/optimism/op-service/signer"
)

const (
Expand Down Expand Up @@ -569,7 +570,7 @@ func (p *publisher) PublishL2Payload(ctx context.Context, envelope *eth.Executio

data := buf.Bytes()
payloadData := data[65:]
sig, err := signer.Sign(ctx, SigningDomainBlocksV1, p.cfg.L2ChainID, payloadData)
sig, err := signer.Sign(ctx, SigningDomainBlocksV1, eth.ChainIDFromBig(p.cfg.L2ChainID), opsigner.PayloadHash(payloadData))
if err != nil {
return fmt.Errorf("failed to sign execution payload with signer: %w", err)
}
Expand Down
17 changes: 9 additions & 8 deletions op-node/p2p/gossip_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ func TestVerifyBlockSignature(t *testing.T) {
t.Run("Valid", func(t *testing.T) {
runCfg := &testutils.MockRuntimeConfig{P2PSeqAddress: crypto.PubkeyToAddress(secrets.PublicKey)}
signer := &PreparedSigner{Signer: NewLocalSigner(secrets)}
sig, err := signer.Sign(context.Background(), SigningDomainBlocksV1, cfg.L2ChainID, msg)
sig, err := signer.Sign(context.Background(), SigningDomainBlocksV1, eth.ChainIDFromBig(cfg.L2ChainID), opsigner.PayloadHash(msg))
require.NoError(t, err)
result := verifyBlockSignature(logger, cfg, runCfg, peerId, sig[:], msg)
require.Equal(t, pubsub.ValidationAccept, result)
Expand All @@ -80,7 +80,7 @@ func TestVerifyBlockSignature(t *testing.T) {
t.Run("WrongSigner", func(t *testing.T) {
runCfg := &testutils.MockRuntimeConfig{P2PSeqAddress: common.HexToAddress("0x1234")}
signer := &PreparedSigner{Signer: NewLocalSigner(secrets)}
sig, err := signer.Sign(context.Background(), SigningDomainBlocksV1, cfg.L2ChainID, msg)
sig, err := signer.Sign(context.Background(), SigningDomainBlocksV1, eth.ChainIDFromBig(cfg.L2ChainID), opsigner.PayloadHash(msg))
require.NoError(t, err)
result := verifyBlockSignature(logger, cfg, runCfg, peerId, sig[:], msg)
require.Equal(t, pubsub.ValidationReject, result)
Expand All @@ -96,7 +96,7 @@ func TestVerifyBlockSignature(t *testing.T) {
t.Run("NoSequencer", func(t *testing.T) {
runCfg := &testutils.MockRuntimeConfig{}
signer := &PreparedSigner{Signer: NewLocalSigner(secrets)}
sig, err := signer.Sign(context.Background(), SigningDomainBlocksV1, cfg.L2ChainID, msg)
sig, err := signer.Sign(context.Background(), SigningDomainBlocksV1, eth.ChainIDFromBig(cfg.L2ChainID), opsigner.PayloadHash(msg))
require.NoError(t, err)
result := verifyBlockSignature(logger, cfg, runCfg, peerId, sig[:], msg)
require.Equal(t, pubsub.ValidationIgnore, result)
Expand All @@ -108,10 +108,11 @@ type mockRemoteSigner struct {
}

func (t *mockRemoteSigner) SignBlockPayload(args opsigner.BlockPayloadArgs) (hexutil.Bytes, error) {
signingHash, err := args.ToSigningHash()
msg, err := args.Message()
if err != nil {
return nil, err
}
signingHash := msg.ToSigningHash()
signature, err := crypto.Sign(signingHash[:], t.priv)
if err != nil {
return nil, err
Expand Down Expand Up @@ -161,7 +162,7 @@ func TestVerifyBlockSignatureWithRemoteSigner(t *testing.T) {
remoteSigner, err := NewRemoteSigner(logger, signerCfg)
require.NoError(t, err)
signer := &PreparedSigner{Signer: remoteSigner}
sig, err := signer.Sign(context.Background(), SigningDomainBlocksV1, cfg.L2ChainID, msg)
sig, err := signer.Sign(context.Background(), SigningDomainBlocksV1, eth.ChainIDFromBig(cfg.L2ChainID), opsigner.PayloadHash(msg))
require.NoError(t, err)
result := verifyBlockSignature(logger, cfg, runCfg, peerId, sig[:], msg)
require.Equal(t, pubsub.ValidationAccept, result)
Expand All @@ -172,7 +173,7 @@ func TestVerifyBlockSignatureWithRemoteSigner(t *testing.T) {
remoteSigner, err := NewRemoteSigner(logger, signerCfg)
require.NoError(t, err)
signer := &PreparedSigner{Signer: remoteSigner}
sig, err := signer.Sign(context.Background(), SigningDomainBlocksV1, cfg.L2ChainID, msg)
sig, err := signer.Sign(context.Background(), SigningDomainBlocksV1, eth.ChainIDFromBig(cfg.L2ChainID), opsigner.PayloadHash(msg))
require.NoError(t, err)
result := verifyBlockSignature(logger, cfg, runCfg, peerId, sig[:], msg)
require.Equal(t, pubsub.ValidationReject, result)
Expand All @@ -190,7 +191,7 @@ func TestVerifyBlockSignatureWithRemoteSigner(t *testing.T) {
remoteSigner, err := NewRemoteSigner(logger, signerCfg)
require.NoError(t, err)
signer := &PreparedSigner{Signer: remoteSigner}
sig, err := signer.Sign(context.Background(), SigningDomainBlocksV1, cfg.L2ChainID, msg)
sig, err := signer.Sign(context.Background(), SigningDomainBlocksV1, eth.ChainIDFromBig(cfg.L2ChainID), opsigner.PayloadHash(msg))
require.NoError(t, err)
result := verifyBlockSignature(logger, cfg, runCfg, peerId, sig[:], msg)
require.Equal(t, pubsub.ValidationIgnore, result)
Expand Down Expand Up @@ -231,7 +232,7 @@ func createSignedP2Payload(payload MarshalSSZ, signer Signer, l2ChainID *big.Int
}
data := buf.Bytes()
payloadData := data[65:]
sig, err := signer.Sign(context.TODO(), SigningDomainBlocksV1, l2ChainID, payloadData)
sig, err := signer.Sign(context.TODO(), SigningDomainBlocksV1, eth.ChainIDFromBig(l2ChainID), opsigner.PayloadHash(payloadData))
if err != nil {
return nil, fmt.Errorf("failed to sign execution payload with signer: %w", err)
}
Expand Down
35 changes: 22 additions & 13 deletions op-node/p2p/signer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,25 +5,30 @@ import (
"crypto/ecdsa"
"errors"
"io"
"math/big"

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

"github.com/ethereum-optimism/optimism/op-node/rollup"
"github.com/ethereum-optimism/optimism/op-service/eth"
opsigner "github.com/ethereum-optimism/optimism/op-service/signer"
)

var SigningDomainBlocksV1 = [32]byte{}

type Signer interface {
Sign(ctx context.Context, domain [32]byte, chainID *big.Int, encodedMsg []byte) (sig *[65]byte, err error)
Sign(ctx context.Context, domain eth.Bytes32, chainID eth.ChainID, payloadHash common.Hash) (sig *[65]byte, err error)
io.Closer
}

func BlockSigningHash(cfg *rollup.Config, payloadBytes []byte) (common.Hash, error) {
return opsigner.NewBlockPayloadArgs(SigningDomainBlocksV1, cfg.L2ChainID, payloadBytes, nil).ToSigningHash()
msg := opsigner.BlockSigningMessage{
Domain: SigningDomainBlocksV1,
ChainID: eth.ChainIDFromBig(cfg.L2ChainID),
PayloadHash: opsigner.PayloadHash(payloadBytes),
}
return msg.ToSigningHash(), nil
}

// LocalSigner is suitable for testing
Expand All @@ -35,17 +40,16 @@ func NewLocalSigner(priv *ecdsa.PrivateKey) *LocalSigner {
return &LocalSigner{priv: priv}
}

func (s *LocalSigner) Sign(ctx context.Context, domain [32]byte, chainID *big.Int, encodedMsg []byte) (sig *[65]byte, err error) {
func (s *LocalSigner) Sign(ctx context.Context, domain eth.Bytes32, chainID eth.ChainID, payloadHash common.Hash) (sig *[65]byte, err error) {
if s.priv == nil {
return nil, errors.New("signer is closed")
}

blockPayloadArgs := opsigner.NewBlockPayloadArgs(domain, chainID, encodedMsg, nil)
signingHash, err := blockPayloadArgs.ToSigningHash()

if err != nil {
return nil, err
msg := opsigner.BlockSigningMessage{
Domain: domain,
ChainID: chainID,
PayloadHash: payloadHash,
}
signingHash := msg.ToSigningHash()
signature, err := crypto.Sign(signingHash[:], s.priv)
if err != nil {
return nil, err
Expand All @@ -72,14 +76,19 @@ func NewRemoteSigner(logger log.Logger, config opsigner.CLIConfig) (*RemoteSigne
return &RemoteSigner{signerClient, &senderAddress}, nil
}

func (s *RemoteSigner) Sign(ctx context.Context, domain [32]byte, chainID *big.Int, encodedMsg []byte) (sig *[65]byte, err error) {
func (s *RemoteSigner) Sign(ctx context.Context, domain eth.Bytes32, chainID eth.ChainID, payloadHash common.Hash) (sig *[65]byte, err error) {
if s.client == nil {
return nil, errors.New("signer is closed")
}

blockPayloadArgs := opsigner.NewBlockPayloadArgs(domain, chainID, encodedMsg, s.sender)
// We use V1 for now, since the server may not support V2 yet
blockPayloadArgs := &opsigner.BlockPayloadArgs{
Domain: domain,
ChainID: chainID.ToBig(),
PayloadHash: payloadHash[:],
SenderAddress: s.sender,
}
signature, err := s.client.SignBlockPayload(ctx, blockPayloadArgs)

if err != nil {
return nil, err
}
Expand Down
24 changes: 15 additions & 9 deletions op-node/p2p/signer_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,14 @@ func TestSigningHash_DifferentDomain(t *testing.T) {
}

payloadBytes := []byte("arbitraryData")
hash, err := opsigner.NewBlockPayloadArgs(SigningDomainBlocksV1, cfg.L2ChainID, payloadBytes, nil).ToSigningHash()
msg, err := opsigner.NewBlockPayloadArgs(SigningDomainBlocksV1, cfg.L2ChainID, payloadBytes, nil).Message()
require.NoError(t, err, "creating first signing hash")

hash2, err := opsigner.NewBlockPayloadArgs([32]byte{3}, cfg.L2ChainID, payloadBytes, nil).ToSigningHash()
msg2, err := opsigner.NewBlockPayloadArgs([32]byte{3}, cfg.L2ChainID, payloadBytes, nil).Message()
require.NoError(t, err, "creating second signing hash")

hash := msg.ToSigningHash()
hash2 := msg2.ToSigningHash()
require.NotEqual(t, hash, hash2, "signing hash should be different when domain is different")
}

Expand All @@ -33,27 +35,31 @@ func TestSigningHash_DifferentChainID(t *testing.T) {
}

payloadBytes := []byte("arbitraryData")
hash, err := opsigner.NewBlockPayloadArgs(SigningDomainBlocksV1, cfg1.L2ChainID, payloadBytes, nil).ToSigningHash()
msg, err := opsigner.NewBlockPayloadArgs(SigningDomainBlocksV1, cfg1.L2ChainID, payloadBytes, nil).Message()
require.NoError(t, err, "creating first signing hash")

hash2, err := opsigner.NewBlockPayloadArgs(SigningDomainBlocksV1, cfg2.L2ChainID, payloadBytes, nil).ToSigningHash()
msg2, err := opsigner.NewBlockPayloadArgs(SigningDomainBlocksV1, cfg2.L2ChainID, payloadBytes, nil).Message()
require.NoError(t, err, "creating second signing hash")

hash := msg.ToSigningHash()
hash2 := msg2.ToSigningHash()
require.NotEqual(t, hash, hash2, "signing hash should be different when chain ID is different")
}

func TestSigningHash_DifferentMessage(t *testing.T) {
func TestSigningHash_DifferentPayload(t *testing.T) {
cfg := &rollup.Config{
L2ChainID: big.NewInt(100),
}

hash, err := opsigner.NewBlockPayloadArgs(SigningDomainBlocksV1, cfg.L2ChainID, []byte("msg1"), nil).ToSigningHash()
msg, err := opsigner.NewBlockPayloadArgs(SigningDomainBlocksV1, cfg.L2ChainID, []byte("payload1"), nil).Message()
require.NoError(t, err, "creating first signing hash")

hash2, err := opsigner.NewBlockPayloadArgs(SigningDomainBlocksV1, cfg.L2ChainID, []byte("msg2"), nil).ToSigningHash()
msg2, err := opsigner.NewBlockPayloadArgs(SigningDomainBlocksV1, cfg.L2ChainID, []byte("payload2"), nil).Message()
require.NoError(t, err, "creating second signing hash")

require.NotEqual(t, hash, hash2, "signing hash should be different when message is different")
hash := msg.ToSigningHash()
hash2 := msg2.ToSigningHash()
require.NotEqual(t, hash, hash2, "signing hash should be different when payload is different")
}

func TestSigningHash_LimitChainID(t *testing.T) {
Expand All @@ -63,6 +69,6 @@ func TestSigningHash_LimitChainID(t *testing.T) {
cfg := &rollup.Config{
L2ChainID: chainID,
}
_, err := opsigner.NewBlockPayloadArgs(SigningDomainBlocksV1, cfg.L2ChainID, []byte("arbitraryData"), nil).ToSigningHash()
_, err := opsigner.NewBlockPayloadArgs(SigningDomainBlocksV1, cfg.L2ChainID, []byte("arbitraryData"), nil).Message()
require.ErrorContains(t, err, "chain_id is too large")
}
46 changes: 42 additions & 4 deletions op-service/eth/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,32 @@ func (ie InputError) Is(target error) bool {
return ok // we implement Unwrap, so we do not have to check the inner type now
}

// Bytes65 is a 65-byte long byte string, and encoded with 0x-prefix in hex.
// This can be used to represent encoded secp256k ethereum signatures.
type Bytes65 [65]byte

func (b *Bytes65) UnmarshalJSON(text []byte) error {
return hexutil.UnmarshalFixedJSON(reflect.TypeOf(b), text, b[:])
}

func (b *Bytes65) UnmarshalText(text []byte) error {
return hexutil.UnmarshalFixedText("Bytes65", text, b[:])
}

func (b Bytes65) MarshalText() ([]byte, error) {
return hexutil.Bytes(b[:]).MarshalText()
}

func (b Bytes65) String() string {
return hexutil.Encode(b[:])
}

// TerminalString implements log.TerminalStringer, formatting a string for console
// output during logging.
func (b Bytes65) TerminalString() string {
return fmt.Sprintf("0x%x..%x", b[:3], b[65-3:])
}

type Bytes32 [32]byte

func (b *Bytes32) UnmarshalJSON(text []byte) error {
Expand All @@ -92,7 +118,7 @@ func (b Bytes32) String() string {
// TerminalString implements log.TerminalStringer, formatting a string for console
// output during logging.
func (b Bytes32) TerminalString() string {
return fmt.Sprintf("%x..%x", b[:3], b[29:])
return fmt.Sprintf("0x%x..%x", b[:3], b[29:])
}

type Bytes8 [8]byte
Expand All @@ -116,7 +142,7 @@ func (b Bytes8) String() string {
// TerminalString implements log.TerminalStringer, formatting a string for console
// output during logging.
func (b Bytes8) TerminalString() string {
return fmt.Sprintf("%x", b[:])
return fmt.Sprintf("0x%x", b[:])
}

type Bytes96 [96]byte
Expand All @@ -140,7 +166,7 @@ func (b Bytes96) String() string {
// TerminalString implements log.TerminalStringer, formatting a string for console
// output during logging.
func (b Bytes96) TerminalString() string {
return fmt.Sprintf("%x..%x", b[:3], b[93:])
return fmt.Sprintf("0x%x..%x", b[:3], b[93:])
}

type Bytes256 [256]byte
Expand All @@ -164,7 +190,7 @@ func (b Bytes256) String() string {
// TerminalString implements log.TerminalStringer, formatting a string for console
// output during logging.
func (b Bytes256) TerminalString() string {
return fmt.Sprintf("%x..%x", b[:3], b[253:])
return fmt.Sprintf("0x%x..%x", b[:3], b[253:])
}

type Uint64Quantity = hexutil.Uint64
Expand Down Expand Up @@ -211,6 +237,14 @@ type ExecutionPayloadEnvelope struct {
RequestsHash *common.Hash `json:"requestsHash,omitempty"`
}

func (env *ExecutionPayloadEnvelope) ID() BlockID {
return env.ExecutionPayload.ID()
}

func (env *ExecutionPayloadEnvelope) String() string {
return fmt.Sprintf("envelope(%s)", env.ID())
}

type ExecutionPayload struct {
ParentHash common.Hash `json:"parentHash"`
FeeRecipient common.Address `json:"feeRecipient"`
Expand Down Expand Up @@ -242,6 +276,10 @@ func (payload *ExecutionPayload) ID() BlockID {
return BlockID{Hash: payload.BlockHash, Number: uint64(payload.BlockNumber)}
}

func (payload *ExecutionPayload) String() string {
return fmt.Sprintf("payload(%s)", payload.ID())
}

func (payload *ExecutionPayload) ParentID() BlockID {
n := uint64(payload.BlockNumber)
if n > 0 {
Expand Down
41 changes: 41 additions & 0 deletions op-service/eth/types_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ package eth
import (
"encoding/json"
"errors"
"fmt"
"math"
"strings"
"testing"

"github.com/ethereum/go-ethereum/common"
Expand Down Expand Up @@ -131,3 +133,42 @@ func TestStorageKey(t *testing.T) {
require.Equal(t, c.marshaled, []uint8(key)[:])
}
}

func TestBytes(t *testing.T) {
testBytesN(t, 8, &Bytes8{0: 1, 8 - 1: 2}, func() BytesN { return new(Bytes8) })
testBytesN(t, 32, &Bytes32{0: 1, 32 - 1: 2}, func() BytesN { return new(Bytes32) })
testBytesN(t, 65, &Bytes65{0: 1, 65 - 1: 2}, func() BytesN { return new(Bytes65) })
testBytesN(t, 96, &Bytes96{0: 1, 96 - 1: 2}, func() BytesN { return new(Bytes96) })
testBytesN(t, 256, &Bytes256{0: 1, 256 - 1: 2}, func() BytesN { return new(Bytes256) })
}

type BytesN interface {
String() string
TerminalString() string
UnmarshalJSON(text []byte) error
UnmarshalText(text []byte) error
MarshalText() ([]byte, error)
}

func testBytesN(t *testing.T, n int, x BytesN, alloc func() BytesN) {
t.Run(fmt.Sprintf("Bytes%d", n), func(t *testing.T) {
xStr := "0x01" + strings.Repeat("00", n-2) + "02"
require.Equal(t, xStr, x.String())
if n == 8 { // too short for dots
require.Equal(t, "0x0100000000000002", x.TerminalString())
} else {
require.Equal(t, "0x010000..000002", x.TerminalString())
}
out, err := x.MarshalText()
require.NoError(t, err)
require.Equal(t, xStr, string(out))

y := alloc()
require.NoError(t, y.UnmarshalText([]byte(xStr)))
require.Equal(t, x, y)

z := alloc()
require.NoError(t, z.UnmarshalJSON([]byte(fmt.Sprintf("%q", xStr))))
require.Equal(t, x, z)
})
}
Loading