chore: minor aggoracle cleanups (#238)

* feat: remove sender address from aggoracle

* fix: localize ticker and stop it. Renames

* feat: split GER processing into small functions

* name functions consistently in ChainSender interface

aggoracle/chaingersender/evm.go

@@ -43,7 +43,6 @@ type EVMChainGERSender struct {
 	logger              *log.Logger
 	gerContract         *pessimisticglobalexitroot.Pessimisticglobalexitroot
 	gerAddr             common.Address
-	sender              common.Address
 	client              EthClienter
 	ethTxMan            EthTxManager
 	gasOffset           uint64
@@ -56,13 +55,12 @@ type EVMConfig struct {
 	ChainIDL2            uint64              `mapstructure:"ChainIDL2"`
 	GasOffset            uint64              `mapstructure:"GasOffset"`
 	WaitPeriodMonitorTx  cfgTypes.Duration   `mapstructure:"WaitPeriodMonitorTx"`
-	SenderAddr           common.Address      `mapstructure:"SenderAddr"`
 	EthTxManager         ethtxmanager.Config `mapstructure:"EthTxManager"`
 }
 
 func NewEVMChainGERSender(
 	logger *log.Logger,
-	l2GlobalExitRoot, sender common.Address,
+	l2GlobalExitRoot common.Address,
 	l2Client EthClienter,
 	ethTxMan EthTxManager,
 	gasOffset uint64,
@@ -77,43 +75,49 @@ func NewEVMChainGERSender(
 		logger:              logger,
 		gerContract:         gerContract,
 		gerAddr:             l2GlobalExitRoot,
-		sender:              sender,
 		client:              l2Client,
 		ethTxMan:            ethTxMan,
 		gasOffset:           gasOffset,
 		waitPeriodMonitorTx: waitPeriodMonitorTx,
 	}, nil
 }
 
-func (c *EVMChainGERSender) IsGERAlreadyInjected(ger common.Hash) (bool, error) {
+func (c *EVMChainGERSender) IsGERInjected(ger common.Hash) (bool, error) {
 	timestamp, err := c.gerContract.GlobalExitRootMap(&bind.CallOpts{Pending: false}, ger)
 	if err != nil {
 		return false, fmt.Errorf("error calling gerContract.GlobalExitRootMap: %w", err)
 	}
 
-	return timestamp.Cmp(big.NewInt(0)) != 0, nil
+	return timestamp.Cmp(common.Big0) != 0, nil
 }
 
-func (c *EVMChainGERSender) UpdateGERWaitUntilMined(ctx context.Context, ger common.Hash) error {
-	abi, err := pessimisticglobalexitroot.PessimisticglobalexitrootMetaData.GetAbi()
+func (c *EVMChainGERSender) InjectGER(ctx context.Context, ger common.Hash) error {
+	ticker := time.NewTicker(c.waitPeriodMonitorTx)
+	defer ticker.Stop()
+
+	gerABI, err := pessimisticglobalexitroot.PessimisticglobalexitrootMetaData.GetAbi()
 	if err != nil {
 		return err
 	}
-	data, err := abi.Pack("updateGlobalExitRoot", ger)
+
+	updateGERTxInput, err := gerABI.Pack("updateGlobalExitRoot", ger)
 	if err != nil {
 		return err
 	}
-	id, err := c.ethTxMan.Add(ctx, &c.gerAddr, big.NewInt(0), data, c.gasOffset, nil)
+
+	id, err := c.ethTxMan.Add(ctx, &c.gerAddr, big.NewInt(0), updateGERTxInput, c.gasOffset, nil)
 	if err != nil {
 		return err
 	}
 	for {
-		time.Sleep(c.waitPeriodMonitorTx)
+		<-ticker.C
+
 		c.logger.Debugf("waiting for tx %s to be mined", id.Hex())
 		res, err := c.ethTxMan.Result(ctx, id)
 		if err != nil {
 			c.logger.Error("error calling ethTxMan.Result: ", err)
 		}
+
 		switch res.Status {
 		case ethtxtypes.MonitoredTxStatusCreated,
 			ethtxtypes.MonitoredTxStatusSent:

aggoracle/e2e_test.go

@@ -36,7 +36,7 @@ func runTest(
 		time.Sleep(time.Millisecond * 150)
 		expectedGER, err := gerL1Contract.GetLastGlobalExitRoot(&bind.CallOpts{Pending: false})
 		require.NoError(t, err)
-		isInjected, err := sender.IsGERAlreadyInjected(expectedGER)
+		isInjected, err := sender.IsGERInjected(expectedGER)
 		require.NoError(t, err)
 		require.True(t, isInjected, fmt.Sprintf("iteration %d, GER: %s", i, common.Bytes2Hex(expectedGER[:])))
 	}

aggoracle/oracle.go

@@ -3,6 +3,7 @@ package aggoracle
 import (
 	"context"
 	"errors"
+	"fmt"
 	"math/big"
 	"time"
 
@@ -19,17 +20,17 @@ type L1InfoTreer interface {
 }
 
 type ChainSender interface {
-	IsGERAlreadyInjected(ger common.Hash) (bool, error)
-	UpdateGERWaitUntilMined(ctx context.Context, ger common.Hash) error
+	IsGERInjected(ger common.Hash) (bool, error)
+	InjectGER(ctx context.Context, ger common.Hash) error
 }
 
 type AggOracle struct {
-	logger        *log.Logger
-	ticker        *time.Ticker
-	l1Client      ethereum.ChainReader
-	l1Info        L1InfoTreer
-	chainSender   ChainSender
-	blockFinality *big.Int
+	logger            *log.Logger
+	waitPeriodNextGER time.Duration
+	l1Client          ethereum.ChainReader
+	l1Info            L1InfoTreer
+	chainSender       ChainSender
+	blockFinality     *big.Int
 }
 
 func New(
@@ -40,81 +41,96 @@ func New(
 	blockFinalityType etherman.BlockNumberFinality,
 	waitPeriodNextGER time.Duration,
 ) (*AggOracle, error) {
-	ticker := time.NewTicker(waitPeriodNextGER)
 	finality, err := blockFinalityType.ToBlockNum()
 	if err != nil {
 		return nil, err
 	}
 
 	return &AggOracle{
-		logger:        logger,
-		ticker:        ticker,
-		l1Client:      l1Client,
-		l1Info:        l1InfoTreeSyncer,
-		chainSender:   chainSender,
-		blockFinality: finality,
+		logger:            logger,
+		chainSender:       chainSender,
+		l1Client:          l1Client,
+		l1Info:            l1InfoTreeSyncer,
+		blockFinality:     finality,
+		waitPeriodNextGER: waitPeriodNextGER,
 	}, nil
 }
 
 func (a *AggOracle) Start(ctx context.Context) {
-	var (
-		blockNumToFetch uint64
-		gerToInject     common.Hash
-		err             error
-	)
+	ticker := time.NewTicker(a.waitPeriodNextGER)
+	defer ticker.Stop()
+
+	var blockNumToFetch uint64
+
 	for {
 		select {
-		case <-a.ticker.C:
-			blockNumToFetch, gerToInject, err = a.getLastFinalisedGER(ctx, blockNumToFetch)
-			if err != nil {
-				switch {
-				case errors.Is(err, l1infotreesync.ErrBlockNotProcessed):
-					a.logger.Debugf("syncer is not ready for the block %d", blockNumToFetch)
-
-				case errors.Is(err, db.ErrNotFound):
-					blockNumToFetch = 0
-					a.logger.Debugf("syncer has not found any GER until block %d", blockNumToFetch)
-
-				default:
-					a.logger.Error("error calling getLastFinalisedGER: ", err)
-				}
-
-				continue
-			}
-			if alreadyInjected, err := a.chainSender.IsGERAlreadyInjected(gerToInject); err != nil {
-				a.logger.Error("error calling isGERAlreadyInjected: ", err)
-				continue
-			} else if alreadyInjected {
-				a.logger.Debugf("GER %s already injected", gerToInject.Hex())
-				continue
+		case <-ticker.C:
+			if err := a.processLatestGER(ctx, &blockNumToFetch); err != nil {
+				a.handleGERProcessingError(err, blockNumToFetch)
 			}
-			a.logger.Infof("injecting new GER: %s", gerToInject.Hex())
-			if err := a.chainSender.UpdateGERWaitUntilMined(ctx, gerToInject); err != nil {
-				a.logger.Errorf("error calling updateGERWaitUntilMined, when trying to inject GER %s: %v", gerToInject.Hex(), err)
-				continue
-			}
-			a.logger.Infof("GER %s injected", gerToInject.Hex())
 		case <-ctx.Done():
 			return
 		}
 	}
 }
 
-// getLastFinalisedGER tries to return a finalised GER:
-// If blockNumToFetch != 0: it will try to fetch it until the given block
-// Else it will ask the L1 client for the latest finalised block and use that
-// If it fails to get the GER from the syncer, it will retunr the block number that used to query
-func (a *AggOracle) getLastFinalisedGER(ctx context.Context, blockNumToFetch uint64) (uint64, common.Hash, error) {
-	if blockNumToFetch == 0 {
+// processLatestGER fetches the latest finalized GER, checks if it is already injected and injects it if not
+func (a *AggOracle) processLatestGER(ctx context.Context, blockNumToFetch *uint64) error {
+	// Fetch the latest GER
+	blockNum, gerToInject, err := a.getLastFinalizedGER(ctx, *blockNumToFetch)
+	if err != nil {
+		return err
+	}
+
+	// Update the block number for the next iteration
+	*blockNumToFetch = blockNum
+
+	alreadyInjected, err := a.chainSender.IsGERInjected(gerToInject)
+	if err != nil {
+		return fmt.Errorf("error checking if GER is already injected: %w", err)
+	}
+	if alreadyInjected {
+		a.logger.Debugf("GER %s already injected", gerToInject.Hex())
+		return nil
+	}
+
+	a.logger.Infof("injecting new GER: %s", gerToInject.Hex())
+	if err := a.chainSender.InjectGER(ctx, gerToInject); err != nil {
+		return fmt.Errorf("error injecting GER %s: %w", gerToInject.Hex(), err)
+	}
+
+	a.logger.Infof("GER %s is injected successfully", gerToInject.Hex())
+	return nil
+}
+
+// handleGERProcessingError handles global exit root processing error
+func (a *AggOracle) handleGERProcessingError(err error, blockNumToFetch uint64) {
+	switch {
+	case errors.Is(err, l1infotreesync.ErrBlockNotProcessed):
+		a.logger.Debugf("syncer is not ready for the block %d", blockNumToFetch)
+	case errors.Is(err, db.ErrNotFound):
+		a.logger.Debugf("syncer has not found any GER until block %d", blockNumToFetch)
+	default:
+		a.logger.Error("unexpected error processing GER: ", err)
+	}
+}
+
+// getLastFinalizedGER tries to return a finalised GER:
+// If targetBlockNum != 0: it will try to fetch it until the given block
+// Else it will ask the L1 client for the latest finalised block and use that.
+// If it fails to get the GER from the syncer, it will return the block number that used to query
+func (a *AggOracle) getLastFinalizedGER(ctx context.Context, targetBlockNum uint64) (uint64, common.Hash, error) {
+	if targetBlockNum == 0 {
 		header, err := a.l1Client.HeaderByNumber(ctx, a.blockFinality)
 		if err != nil {
 			return 0, common.Hash{}, err
 		}
-		blockNumToFetch = header.Number.Uint64()
+		targetBlockNum = header.Number.Uint64()
 	}
-	info, err := a.l1Info.GetLatestInfoUntilBlock(ctx, blockNumToFetch)
+
+	info, err := a.l1Info.GetLatestInfoUntilBlock(ctx, targetBlockNum)
 	if err != nil {
-		return blockNumToFetch, common.Hash{}, err
+		return targetBlockNum, common.Hash{}, err
 	}
 
 	return 0, info.GlobalExitRoot, nil

claimsponsor/e2e_test.go

@@ -56,7 +56,7 @@ func TestE2EL1toEVML2(t *testing.T) {
 		time.Sleep(time.Millisecond * 300)
 		expectedGER, err := env.GERL1Contract.GetLastGlobalExitRoot(&bind.CallOpts{Pending: false})
 		require.NoError(t, err)
-		isInjected, err := env.AggOracleSender.IsGERAlreadyInjected(expectedGER)
+		isInjected, err := env.AggOracleSender.IsGERInjected(expectedGER)
 		require.NoError(t, err)
 		require.True(t, isInjected, fmt.Sprintf("iteration %d, GER: %s", i, common.Bytes2Hex(expectedGER[:])))
 

cmd/run.go

@@ -343,7 +343,6 @@ func createAggoracle(
 		sender, err = chaingersender.NewEVMChainGERSender(
 			logger,
 			cfg.AggOracle.EVMSender.GlobalExitRootL2Addr,
-			cfg.AggOracle.EVMSender.SenderAddr,
 			l2Client,
 			ethTxManager,
 			cfg.AggOracle.EVMSender.GasOffset,

config/default.go

@@ -228,7 +228,6 @@ WaitPeriodNextGER="100ms"
 		ChainIDL2=1337
 		GasOffset=0
 		WaitPeriodMonitorTx="100ms"
-		SenderAddr="0x70997970c51812dc3a010c7d01b50e0d17dc79c8"
 		[AggOracle.EVMSender.EthTxManager]
 				FrequencyToMonitorTxs = "1s"
 				WaitTxToBeMined = "2s"

lastgersync/e2e_test.go

@@ -44,7 +44,7 @@ func TestE2E(t *testing.T) {
 		time.Sleep(time.Millisecond * 150)
 		expectedGER, err := env.GERL1Contract.GetLastGlobalExitRoot(&bind.CallOpts{Pending: false})
 		require.NoError(t, err)
-		isInjected, err := env.AggOracleSender.IsGERAlreadyInjected(expectedGER)
+		isInjected, err := env.AggOracleSender.IsGERInjected(expectedGER)
 		require.NoError(t, err)
 		require.True(t, isInjected, fmt.Sprintf("iteration %d, GER: %s", i, common.Bytes2Hex(expectedGER[:])))
 

test/helpers/e2e.go

@@ -58,8 +58,12 @@ func NewE2EEnvWithEVML2(t *testing.T) *AggoracleWithEVMChainEnv {
 	t.Helper()
 
 	ctx := context.Background()
-	l1Client, syncer, gerL1Contract, gerL1Addr, bridgeL1Contract, bridgeL1Addr, authL1, rdL1, bridgeL1Sync := CommonSetup(t)
-	sender, l2Client, gerL2Contract, gerL2Addr, bridgeL2Contract, bridgeL2Addr, authL2, ethTxManMockL2, bridgeL2Sync, rdL2 := L2SetupEVM(t)
+	l1Client, syncer, gerL1Contract, gerL1Addr,
+		bridgeL1Contract, bridgeL1Addr, authL1, rdL1, bridgeL1Sync := CommonSetup(t)
+
+	sender, l2Client, gerL2Contract, gerL2Addr,
+		bridgeL2Contract, bridgeL2Addr, authL2,
+		ethTxManMockL2, bridgeL2Sync, rdL2 := L2SetupEVM(t)
 	oracle, err := aggoracle.New(
 		log.GetDefaultLogger(), sender,
 		l1Client.Client(), syncer,
@@ -137,16 +141,32 @@ func CommonSetup(t *testing.T) (
 		time.Millisecond, 0, periodRetry, retries, l1infotreesync.FlagAllowWrongContractsAddrs,
 	)
 	require.NoError(t, err)
+
 	go l1InfoTreeSync.Start(ctx)
 
+	const (
+		syncBlockChunks        = 10
+		waitForNewBlocksPeriod = 10 * time.Millisecond
+		originNetwork          = 1
+		initialBlock           = 0
+		retryPeriod            = 0
+		retriesCount           = 0
+	)
+
 	// Bridge sync
 	testClient := TestClient{ClientRenamed: l1Client.Client()}
 	dbPathBridgeSyncL1 := path.Join(t.TempDir(), "BridgeSyncL1.sqlite")
-	bridgeL1Sync, err := bridgesync.NewL1(ctx, dbPathBridgeSyncL1, bridgeL1Addr, 10, etherman.LatestBlock, rdL1, testClient, 0, time.Millisecond*10, 0, 0, 1, false) //nolint:mnd
+	bridgeL1Sync, err := bridgesync.NewL1(
+		ctx, dbPathBridgeSyncL1, bridgeL1Addr,
+		syncBlockChunks, etherman.LatestBlock, rdL1, testClient,
+		initialBlock, waitForNewBlocksPeriod, retryPeriod,
+		retriesCount, originNetwork, false)
 	require.NoError(t, err)
+
 	go bridgeL1Sync.Start(ctx)
 
-	return l1Client, l1InfoTreeSync, gerL1Contract, gerL1Addr, bridgeL1Contract, bridgeL1Addr, authL1, rdL1, bridgeL1Sync
+	return l1Client, l1InfoTreeSync, gerL1Contract, gerL1Addr,
+		bridgeL1Contract, bridgeL1Addr, authL1, rdL1, bridgeL1Sync
 }
 
 func L2SetupEVM(t *testing.T) (
@@ -167,7 +187,7 @@ func L2SetupEVM(t *testing.T) (
 	ethTxManMock := NewEthTxManMock(t, l2Client, authL2)
 	sender, err := chaingersender.NewEVMChainGERSender(
 		log.GetDefaultLogger(),
-		gerL2Addr, authL2.From, l2Client.Client(), ethTxManMock, 0, time.Millisecond*50, //nolint:mnd
+		gerL2Addr, l2Client.Client(), ethTxManMock, 0, time.Millisecond*50, //nolint:mnd
 	)
 	require.NoError(t, err)
 	ctx := context.Background()
@@ -181,8 +201,23 @@ func L2SetupEVM(t *testing.T) (
 	// Bridge sync
 	dbPathL2BridgeSync := path.Join(t.TempDir(), "BridgeSyncL2.sqlite")
 	testClient := TestClient{ClientRenamed: l2Client.Client()}
-	bridgeL2Sync, err := bridgesync.NewL2(ctx, dbPathL2BridgeSync, bridgeL2Addr, 10, etherman.LatestBlock, rdL2, testClient, 0, time.Millisecond*10, 0, 0, 1, false) //nolint:mnd
+
+	const (
+		syncBlockChunks        = 10
+		waitForNewBlocksPeriod = 10 * time.Millisecond
+		originNetwork          = 1
+		initialBlock           = 0
+		retryPeriod            = 0
+		retriesCount           = 0
+	)
+
+	bridgeL2Sync, err := bridgesync.NewL2(
+		ctx, dbPathL2BridgeSync, bridgeL2Addr, syncBlockChunks,
+		etherman.LatestBlock, rdL2, testClient,
+		initialBlock, waitForNewBlocksPeriod, retryPeriod,
+		retriesCount, originNetwork, false)
 	require.NoError(t, err)
+
 	go bridgeL2Sync.Start(ctx)
 
 	return sender, l2Client, gerL2Sc, gerL2Addr, bridgeL2Sc, bridgeL2Addr, authL2, ethTxManMock, bridgeL2Sync, rdL2

test/helpers/reorg.go

@@ -11,7 +11,8 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-// commitBlocks commits the specified number of blocks with the given client and waits for the specified duration after each block
+// commitBlocks commits the specified number of blocks with the given client
+// and waits for the specified duration after each block
 func CommitBlocks(t *testing.T, client *simulated.Backend, numBlocks int, waitDuration time.Duration) {
 	t.Helper()