l2geth: remove ovmsigner (#591)

* l2geth: remove ovmsigner

Also reduce the diff

Co-authored-by: smartcontracts

* l2geth: add changeset

.changeset/ten-keys-share.md

@@ -0,0 +1,5 @@
+---
+"@eth-optimism/l2geth": patch
+---
+
+Remove the OVMSigner

l2geth/core/types/transaction_signing.go

@@ -17,67 +17,16 @@
 package types
 
 import (
-	"bytes"
 	"crypto/ecdsa"
-	"encoding/binary"
 	"errors"
 	"fmt"
 	"math/big"
-	"strings"
 
-	"github.com/ethereum/go-ethereum/accounts/abi"
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/crypto"
 	"github.com/ethereum/go-ethereum/params"
-	"golang.org/x/crypto/sha3"
 )
 
-var codec abi.ABI
-
-func init() {
-	const abidata = `
-	[
-		{
-			"type": "function",
-			"name": "encode",
-			"constant": true,
-			"inputs": [
-				{
-					"name": "nonce",
-					"type": "uint256"
-				},
-				{
-					"name": "gasLimit",
-					"type": "uint256"
-				},
-				{
-					"name": "gasPrice",
-					"type": "uint256"
-				},
-				{
-					"name": "chainId",
-					"type": "uint256"
-				},
-				{
-					"name": "to",
-					"type": "address"
-				},
-				{
-					"name": "data",
-					"type": "bytes"
-				}
-			]
-		}
-	]
-	`
-
-	var err error
-	codec, err = abi.JSON(strings.NewReader(abidata))
-	if err != nil {
-		panic(fmt.Errorf("unable to create Eth Sign abi reader: %v", err))
-	}
-}
-
 var (
 	ErrInvalidChainId = errors.New("invalid chain id for signer")
 )
@@ -91,7 +40,16 @@ type sigCache struct {
 
 // MakeSigner returns a Signer based on the given chain config and block number.
 func MakeSigner(config *params.ChainConfig, blockNumber *big.Int) Signer {
-	return NewOVMSigner(config.ChainID)
+	var signer Signer
+	switch {
+	case config.IsEIP155(blockNumber):
+		signer = NewEIP155Signer(config.ChainID)
+	case config.IsHomestead(blockNumber):
+		signer = HomesteadSigner{}
+	default:
+		signer = FrontierSigner{}
+	}
+	return signer
 }
 
 // SignTx signs the transaction using the given signer and private key
@@ -144,97 +102,6 @@ type Signer interface {
 	Equal(Signer) bool
 }
 
-// OVMSigner implements Signers using the EIP155 rules along with a new
-// `eth_sign` based signature hash.
-type OVMSigner struct {
-	EIP155Signer
-}
-
-func NewOVMSigner(chainId *big.Int) OVMSigner {
-	signer := NewEIP155Signer(chainId)
-	return OVMSigner{signer}
-}
-
-func (s OVMSigner) Equal(s2 Signer) bool {
-	ovm, ok := s2.(OVMSigner)
-	return ok && ovm.chainId.Cmp(s.chainId) == 0
-}
-
-// Hash returns the hash to be signed by the sender.
-// It does not uniquely identify the transaction.
-func (s OVMSigner) Hash(tx *Transaction) common.Hash {
-	if tx.IsEthSignSighash() {
-		msg := s.OVMSignerTemplateSighashPreimage(tx)
-
-		hasher := sha3.NewLegacyKeccak256()
-		hasher.Write(msg[:])
-		digest := hasher.Sum(nil)
-
-		return common.BytesToHash(digest)
-	}
-
-	return rlpHash([]interface{}{
-		tx.data.AccountNonce,
-		tx.data.Price,
-		tx.data.GasLimit,
-		tx.data.Recipient,
-		tx.data.Amount,
-		tx.data.Payload,
-		s.chainId, uint(0), uint(0),
-	})
-}
-
-// Sender will ecrecover the public key that created the signature
-// and then hash the public key to create an address. In the
-// case of L1ToL2 transactions, Layer One did the authentication
-// for us so there is no signature involved. The concept of a "from"
-// is only required for bookkeeping within this codebase
-func (s OVMSigner) Sender(tx *Transaction) (common.Address, error) {
-	qo := tx.QueueOrigin()
-	if qo != nil && qo.Uint64() == uint64(QueueOriginL1ToL2) {
-		return common.Address{}, nil
-	}
-	if !tx.Protected() {
-		return HomesteadSigner{}.Sender(tx)
-	}
-	if tx.ChainId().Cmp(s.chainId) != 0 {
-		return common.Address{}, ErrInvalidChainId
-	}
-	V := new(big.Int).Sub(tx.data.V, s.chainIdMul)
-	V.Sub(V, big8)
-	return recoverPlain(s.Hash(tx), tx.data.R, tx.data.S, V, true)
-}
-
-// OVMSignerTemplateSighashPreimage creates the preimage for the `eth_sign` like
-// signature hash. The transaction is `ABI.encodePacked`.
-func (s OVMSigner) OVMSignerTemplateSighashPreimage(tx *Transaction) []byte {
-	data := []interface{}{
-		big.NewInt(int64(tx.data.AccountNonce)),
-		big.NewInt(int64(tx.data.GasLimit)),
-		tx.data.Price,
-		s.chainId,
-		*tx.data.Recipient,
-		tx.data.Payload,
-	}
-
-	ret, err := codec.Pack("encode", data...)
-	if err != nil {
-		panic(fmt.Errorf("unable to pack Eth Sign data: %v", err))
-	}
-
-	hasher := sha3.NewLegacyKeccak256()
-	// Slice off the function selector before hashing
-	hasher.Write(ret[4:])
-	digest := hasher.Sum(nil)
-
-	preimage := new(bytes.Buffer)
-	prefix := []byte("\x19Ethereum Signed Message:\n32")
-	binary.Write(preimage, binary.BigEndian, prefix)
-	binary.Write(preimage, binary.BigEndian, digest)
-
-	return preimage.Bytes()
-}
-
 // EIP155Transaction implements Signer using the EIP155 rules.
 type EIP155Signer struct {
 	chainId, chainIdMul *big.Int

l2geth/core/types/transaction_signing_test.go

@@ -136,100 +136,3 @@ func TestChainId(t *testing.T) {
 		t.Error("expected no error")
 	}
 }
-
-func TestOVMSigner(t *testing.T) {
-	key, _ := defaultTestKey()
-
-	tx := NewTransaction(0, common.Address{}, new(big.Int), 0, new(big.Int), nil)
-	txMeta := NewTransactionMeta(nil, 0, nil, SighashEthSign, QueueOriginSequencer, nil, nil, nil)
-	tx.SetTransactionMeta(txMeta)
-
-	var err error
-	tx, err = SignTx(tx, NewOVMSigner(big.NewInt(1)), key)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	_, err = Sender(NewOVMSigner(big.NewInt(2)), tx)
-	if err != ErrInvalidChainId {
-		t.Error("expected error:", ErrInvalidChainId)
-	}
-
-	_, err = Sender(NewOVMSigner(big.NewInt(1)), tx)
-	if err != nil {
-		t.Error("expected no error")
-	}
-}
-
-func TestOVMSignerHash(t *testing.T) {
-	signer := NewOVMSigner(big.NewInt(1))
-
-	txNil := NewTransaction(0, common.Address{}, new(big.Int), 0, new(big.Int), nil)
-	txEIP155 := NewTransaction(0, common.Address{}, new(big.Int), 0, new(big.Int), nil)
-
-	hashNil := signer.Hash(txNil)
-	hashEIP155 := signer.Hash(txEIP155)
-	if hashNil != hashEIP155 {
-		t.Errorf("Signature hashes should be equal: %s != %s", hashNil.Hex(), hashEIP155.Hex())
-	}
-
-	// The signature hash should be different when using `SighashEthSign`
-	txEthSign := NewTransaction(0, common.Address{}, new(big.Int), 0, new(big.Int), nil)
-	txMeta := NewTransactionMeta(nil, 0, nil, SighashEthSign, QueueOriginSequencer, nil, nil, nil)
-	txEthSign.SetTransactionMeta(txMeta)
-
-	hashEthSign := signer.Hash(txEthSign)
-	if hashEIP155 == hashEthSign {
-		t.Errorf("Signature hashes should not be equal: %s == %s", hashEIP155.Hex(), hashEthSign.Hex())
-	}
-}
-
-func TestOVMSignerSender(t *testing.T) {
-	// Create a keypair to sign transactions with and the corresponding address
-	// from the public key.
-	key, _ := crypto.GenerateKey()
-	addr := crypto.PubkeyToAddress(key.PublicKey)
-
-	// This test makes sure that both the EIP155 and EthSign signature hash
-	// codepaths work when using the OVMSigner.
-	signer := NewOVMSigner(big.NewInt(1))
-	var err error
-
-	// Create a transaction with EIP155 signature hash, sign the transaction,
-	// recover the address and assert that the address matches the key.
-	txEIP155 := NewTransaction(0, addr, new(big.Int), 0, new(big.Int), nil)
-
-	txEIP155, err = SignTx(txEIP155, signer, key)
-	if err != nil {
-		t.Errorf("No error expected")
-	}
-
-	recEIP155, err := signer.Sender(txEIP155)
-	if err != nil {
-		t.Errorf("No error expected")
-	}
-
-	if addr != recEIP155 {
-		t.Errorf("Recovered address doesn't match. Got %s, expected %s", recEIP155.Hex(), addr.Hex())
-	}
-
-	// Create a transaction with EthSign signature hash, sign the transaction,
-	// recover the address and assert that the address matches the key.
-	txEthSign := NewTransaction(0, addr, new(big.Int), 0, new(big.Int), nil)
-	txMeta := NewTransactionMeta(nil, 0, nil, SighashEthSign, QueueOriginSequencer, nil, nil, nil)
-	txEthSign.SetTransactionMeta(txMeta)
-
-	txEthSign, err = SignTx(txEthSign, signer, key)
-	if err != nil {
-		t.Errorf("No error expected")
-	}
-
-	recEthSign, err := signer.Sender(txEthSign)
-	if err != nil {
-		t.Errorf("No error expected")
-	}
-
-	if addr != recEthSign {
-		t.Errorf("Recovered address doesn't match. Got %s, expected %s", recEthSign.Hex(), addr.Hex())
-	}
-}

l2geth/core/types/transaction_test.go

@@ -208,7 +208,7 @@ func TestTransactionJSON(t *testing.T) {
 	if err != nil {
 		t.Fatalf("could not generate key: %v", err)
 	}
-	signer := NewOVMSigner(common.Big1)
+	signer := NewEIP155Signer(common.Big1)
 
 	transactions := make([]*Transaction, 0, 50)
 	for i := uint64(0); i < 25; i++ {

l2geth/internal/ethapi/api.go

@@ -1269,7 +1269,10 @@ type RPCTransaction struct {
 // newRPCTransaction returns a transaction that will serialize to the RPC
 // representation, with the given location metadata set (if available).
 func newRPCTransaction(tx *types.Transaction, blockHash common.Hash, blockNumber uint64, index uint64) *RPCTransaction {
-	var signer types.Signer = types.NewOVMSigner(tx.ChainId())
+	var signer types.Signer = types.FrontierSigner{}
+	if tx.Protected() {
+		signer = types.NewEIP155Signer(tx.ChainId())
+	}
 	from, _ := types.Sender(signer, tx)
 	v, r, s := tx.RawSignatureValues()
 
@@ -1497,7 +1500,7 @@ func (s *PublicTransactionPoolAPI) GetTransactionReceipt(ctx context.Context, ha
 
 	var signer types.Signer = types.FrontierSigner{}
 	if tx.Protected() {
-		signer = types.NewOVMSigner(tx.ChainId())
+		signer = types.NewEIP155Signer(tx.ChainId())
 	}
 	from, _ := types.Sender(signer, tx)
 
@@ -1815,7 +1818,7 @@ func (s *PublicTransactionPoolAPI) PendingTransactions() ([]*RPCTransaction, err
 	for _, tx := range pending {
 		var signer types.Signer = types.HomesteadSigner{}
 		if tx.Protected() {
-			signer = types.NewOVMSigner(tx.ChainId())
+			signer = types.NewEIP155Signer(tx.ChainId())
 		}
 		from, _ := types.Sender(signer, tx)
 		if _, exists := accounts[from]; exists {
@@ -1843,7 +1846,7 @@ func (s *PublicTransactionPoolAPI) Resend(ctx context.Context, sendArgs SendTxAr
 	for _, p := range pending {
 		var signer types.Signer = types.HomesteadSigner{}
 		if p.Protected() {
-			signer = types.NewOVMSigner(p.ChainId())
+			signer = types.NewEIP155Signer(p.ChainId())
 		}
 		wantSigHash := signer.Hash(matchTx)
 

l2geth/miner/worker.go

@@ -646,7 +646,7 @@ func (w *worker) makeCurrent(parent *types.Block, header *types.Header) error {
 		return err
 	}
 	env := &environment{
-		signer:    types.NewOVMSigner(w.chainConfig.ChainID),
+		signer:    types.NewEIP155Signer(w.chainConfig.ChainID),
 		state:     state,
 		ancestors: mapset.NewSet(),
 		family:    mapset.NewSet(),

l2geth/rollup/client.go

@@ -130,7 +130,7 @@ type RollupClient interface {
 // Client is an HTTP based RollupClient
 type Client struct {
 	client *resty.Client
-	signer *types.OVMSigner
+	signer *types.EIP155Signer
 }
 
 // TransactionResponse represents the response from the remote server when
@@ -152,7 +152,7 @@ func NewClient(url string, chainID *big.Int) *Client {
 	client := resty.New()
 	client.SetHostURL(url)
 	client.SetHeader("User-Agent", "sequencer")
-	signer := types.NewOVMSigner(chainID)
+	signer := types.NewEIP155Signer(chainID)
 
 	return &Client{
 		client: client,
@@ -271,7 +271,7 @@ func (c *Client) GetLatestEnqueue() (*types.Transaction, error) {
 
 // batchedTransactionToTransaction converts a transaction into a
 // types.Transaction that can be consumed by the SyncService
-func batchedTransactionToTransaction(res *transaction, signer *types.OVMSigner) (*types.Transaction, error) {
+func batchedTransactionToTransaction(res *transaction, signer *types.EIP155Signer) (*types.Transaction, error) {
 	// `nil` transactions are not found
 	if res == nil {
 		return nil, errElementNotFound
@@ -538,7 +538,7 @@ func (c *Client) GetTransactionBatch(index uint64) (*Batch, []*types.Transaction
 
 // parseTransactionBatchResponse will turn a TransactionBatchResponse into a
 // Batch and its corresponding types.Transactions
-func parseTransactionBatchResponse(txBatch *TransactionBatchResponse, signer *types.OVMSigner) (*Batch, []*types.Transaction, error) {
+func parseTransactionBatchResponse(txBatch *TransactionBatchResponse, signer *types.EIP155Signer) (*Batch, []*types.Transaction, error) {
 	if txBatch == nil {
 		return nil, nil, nil
 	}