Commit 247b3992 authored by Josh Klopfenstein's avatar Josh Klopfenstein Committed by GitHub

Use local logger in data source (#11756)

There may be other places where a global logger is used, but this instance is particularly important because one message warns the user of a misconfiguration that otherwise manifests in a seemingly unrelated bug (rollbacks on unsafe block consolidation).
parent 31444eb3
...@@ -86,7 +86,7 @@ func (ds *BlobDataSource) open(ctx context.Context) ([]blobOrCalldata, error) { ...@@ -86,7 +86,7 @@ func (ds *BlobDataSource) open(ctx context.Context) ([]blobOrCalldata, error) {
return nil, NewTemporaryError(fmt.Errorf("failed to open blob data source: %w", err)) return nil, NewTemporaryError(fmt.Errorf("failed to open blob data source: %w", err))
} }
data, hashes := dataAndHashesFromTxs(txs, &ds.dsCfg, ds.batcherAddr) data, hashes := dataAndHashesFromTxs(txs, &ds.dsCfg, ds.batcherAddr, ds.log)
if len(hashes) == 0 { if len(hashes) == 0 {
// there are no blobs to fetch so we can return immediately // there are no blobs to fetch so we can return immediately
...@@ -115,13 +115,13 @@ func (ds *BlobDataSource) open(ctx context.Context) ([]blobOrCalldata, error) { ...@@ -115,13 +115,13 @@ func (ds *BlobDataSource) open(ctx context.Context) ([]blobOrCalldata, error) {
// dataAndHashesFromTxs extracts calldata and datahashes from the input transactions and returns them. It // dataAndHashesFromTxs extracts calldata and datahashes from the input transactions and returns them. It
// creates a placeholder blobOrCalldata element for each returned blob hash that must be populated // creates a placeholder blobOrCalldata element for each returned blob hash that must be populated
// by fillBlobPointers after blob bodies are retrieved. // by fillBlobPointers after blob bodies are retrieved.
func dataAndHashesFromTxs(txs types.Transactions, config *DataSourceConfig, batcherAddr common.Address) ([]blobOrCalldata, []eth.IndexedBlobHash) { func dataAndHashesFromTxs(txs types.Transactions, config *DataSourceConfig, batcherAddr common.Address, logger log.Logger) ([]blobOrCalldata, []eth.IndexedBlobHash) {
data := []blobOrCalldata{} data := []blobOrCalldata{}
var hashes []eth.IndexedBlobHash var hashes []eth.IndexedBlobHash
blobIndex := 0 // index of each blob in the block's blob sidecar blobIndex := 0 // index of each blob in the block's blob sidecar
for _, tx := range txs { for _, tx := range txs {
// skip any non-batcher transactions // skip any non-batcher transactions
if !isValidBatchTx(tx, config.l1Signer, config.batchInboxAddress, batcherAddr) { if !isValidBatchTx(tx, config.l1Signer, config.batchInboxAddress, batcherAddr, logger) {
blobIndex += len(tx.BlobHashes()) blobIndex += len(tx.BlobHashes())
continue continue
} }
......
...@@ -13,7 +13,9 @@ import ( ...@@ -13,7 +13,9 @@ import (
"github.com/ethereum/go-ethereum/crypto" "github.com/ethereum/go-ethereum/crypto"
"github.com/ethereum-optimism/optimism/op-service/eth" "github.com/ethereum-optimism/optimism/op-service/eth"
"github.com/ethereum-optimism/optimism/op-service/testlog"
"github.com/ethereum-optimism/optimism/op-service/testutils" "github.com/ethereum-optimism/optimism/op-service/testutils"
"github.com/ethereum/go-ethereum/log"
) )
func TestDataAndHashesFromTxs(t *testing.T) { func TestDataAndHashesFromTxs(t *testing.T) {
...@@ -23,6 +25,7 @@ func TestDataAndHashesFromTxs(t *testing.T) { ...@@ -23,6 +25,7 @@ func TestDataAndHashesFromTxs(t *testing.T) {
publicKey, _ := privateKey.Public().(*ecdsa.PublicKey) publicKey, _ := privateKey.Public().(*ecdsa.PublicKey)
batcherAddr := crypto.PubkeyToAddress(*publicKey) batcherAddr := crypto.PubkeyToAddress(*publicKey)
batchInboxAddr := testutils.RandomAddress(rng) batchInboxAddr := testutils.RandomAddress(rng)
logger := testlog.Logger(t, log.LvlInfo)
chainId := new(big.Int).SetUint64(rng.Uint64()) chainId := new(big.Int).SetUint64(rng.Uint64())
signer := types.NewCancunSigner(chainId) signer := types.NewCancunSigner(chainId)
...@@ -42,7 +45,7 @@ func TestDataAndHashesFromTxs(t *testing.T) { ...@@ -42,7 +45,7 @@ func TestDataAndHashesFromTxs(t *testing.T) {
} }
calldataTx, _ := types.SignNewTx(privateKey, signer, txData) calldataTx, _ := types.SignNewTx(privateKey, signer, txData)
txs := types.Transactions{calldataTx} txs := types.Transactions{calldataTx}
data, blobHashes := dataAndHashesFromTxs(txs, &config, batcherAddr) data, blobHashes := dataAndHashesFromTxs(txs, &config, batcherAddr, logger)
require.Equal(t, 1, len(data)) require.Equal(t, 1, len(data))
require.Equal(t, 0, len(blobHashes)) require.Equal(t, 0, len(blobHashes))
...@@ -57,14 +60,14 @@ func TestDataAndHashesFromTxs(t *testing.T) { ...@@ -57,14 +60,14 @@ func TestDataAndHashesFromTxs(t *testing.T) {
} }
blobTx, _ := types.SignNewTx(privateKey, signer, blobTxData) blobTx, _ := types.SignNewTx(privateKey, signer, blobTxData)
txs = types.Transactions{blobTx} txs = types.Transactions{blobTx}
data, blobHashes = dataAndHashesFromTxs(txs, &config, batcherAddr) data, blobHashes = dataAndHashesFromTxs(txs, &config, batcherAddr, logger)
require.Equal(t, 1, len(data)) require.Equal(t, 1, len(data))
require.Equal(t, 1, len(blobHashes)) require.Equal(t, 1, len(blobHashes))
require.Nil(t, data[0].calldata) require.Nil(t, data[0].calldata)
// try again with both the blob & calldata transactions and make sure both are picked up // try again with both the blob & calldata transactions and make sure both are picked up
txs = types.Transactions{blobTx, calldataTx} txs = types.Transactions{blobTx, calldataTx}
data, blobHashes = dataAndHashesFromTxs(txs, &config, batcherAddr) data, blobHashes = dataAndHashesFromTxs(txs, &config, batcherAddr, logger)
require.Equal(t, 2, len(data)) require.Equal(t, 2, len(data))
require.Equal(t, 1, len(blobHashes)) require.Equal(t, 1, len(blobHashes))
require.NotNil(t, data[1].calldata) require.NotNil(t, data[1].calldata)
...@@ -72,7 +75,7 @@ func TestDataAndHashesFromTxs(t *testing.T) { ...@@ -72,7 +75,7 @@ func TestDataAndHashesFromTxs(t *testing.T) {
// make sure blob tx to the batch inbox is ignored if not signed by the batcher // make sure blob tx to the batch inbox is ignored if not signed by the batcher
blobTx, _ = types.SignNewTx(testutils.RandomKey(), signer, blobTxData) blobTx, _ = types.SignNewTx(testutils.RandomKey(), signer, blobTxData)
txs = types.Transactions{blobTx} txs = types.Transactions{blobTx}
data, blobHashes = dataAndHashesFromTxs(txs, &config, batcherAddr) data, blobHashes = dataAndHashesFromTxs(txs, &config, batcherAddr, logger)
require.Equal(t, 0, len(data)) require.Equal(t, 0, len(data))
require.Equal(t, 0, len(blobHashes)) require.Equal(t, 0, len(blobHashes))
...@@ -81,7 +84,7 @@ func TestDataAndHashesFromTxs(t *testing.T) { ...@@ -81,7 +84,7 @@ func TestDataAndHashesFromTxs(t *testing.T) {
blobTxData.To = testutils.RandomAddress(rng) blobTxData.To = testutils.RandomAddress(rng)
blobTx, _ = types.SignNewTx(privateKey, signer, blobTxData) blobTx, _ = types.SignNewTx(privateKey, signer, blobTxData)
txs = types.Transactions{blobTx} txs = types.Transactions{blobTx}
data, blobHashes = dataAndHashesFromTxs(txs, &config, batcherAddr) data, blobHashes = dataAndHashesFromTxs(txs, &config, batcherAddr, logger)
require.Equal(t, 0, len(data)) require.Equal(t, 0, len(data))
require.Equal(t, 0, len(blobHashes)) require.Equal(t, 0, len(blobHashes))
} }
......
...@@ -79,7 +79,7 @@ func (ds *CalldataSource) Next(ctx context.Context) (eth.Data, error) { ...@@ -79,7 +79,7 @@ func (ds *CalldataSource) Next(ctx context.Context) (eth.Data, error) {
func DataFromEVMTransactions(dsCfg DataSourceConfig, batcherAddr common.Address, txs types.Transactions, log log.Logger) []eth.Data { func DataFromEVMTransactions(dsCfg DataSourceConfig, batcherAddr common.Address, txs types.Transactions, log log.Logger) []eth.Data {
out := []eth.Data{} out := []eth.Data{}
for _, tx := range txs { for _, tx := range txs {
if isValidBatchTx(tx, dsCfg.l1Signer, dsCfg.batchInboxAddress, batcherAddr) { if isValidBatchTx(tx, dsCfg.l1Signer, dsCfg.batchInboxAddress, batcherAddr, log) {
out = append(out, tx.Data()) out = append(out, tx.Data())
} }
} }
......
...@@ -93,19 +93,19 @@ type DataSourceConfig struct { ...@@ -93,19 +93,19 @@ type DataSourceConfig struct {
// isValidBatchTx returns true if: // isValidBatchTx returns true if:
// 1. the transaction has a To() address that matches the batch inbox address, and // 1. the transaction has a To() address that matches the batch inbox address, and
// 2. the transaction has a valid signature from the batcher address // 2. the transaction has a valid signature from the batcher address
func isValidBatchTx(tx *types.Transaction, l1Signer types.Signer, batchInboxAddr, batcherAddr common.Address) bool { func isValidBatchTx(tx *types.Transaction, l1Signer types.Signer, batchInboxAddr, batcherAddr common.Address, logger log.Logger) bool {
to := tx.To() to := tx.To()
if to == nil || *to != batchInboxAddr { if to == nil || *to != batchInboxAddr {
return false return false
} }
seqDataSubmitter, err := l1Signer.Sender(tx) // optimization: only derive sender if To is correct seqDataSubmitter, err := l1Signer.Sender(tx) // optimization: only derive sender if To is correct
if err != nil { if err != nil {
log.Warn("tx in inbox with invalid signature", "hash", tx.Hash(), "err", err) logger.Warn("tx in inbox with invalid signature", "hash", tx.Hash(), "err", err)
return false return false
} }
// some random L1 user might have sent a transaction to our batch inbox, ignore them // some random L1 user might have sent a transaction to our batch inbox, ignore them
if seqDataSubmitter != batcherAddr { if seqDataSubmitter != batcherAddr {
log.Warn("tx in inbox with unauthorized submitter", "addr", seqDataSubmitter, "hash", tx.Hash(), "err", err) logger.Warn("tx in inbox with unauthorized submitter", "addr", seqDataSubmitter, "hash", tx.Hash(), "err", err)
return false return false
} }
return true return true
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment