fix: reorg detector and syncer issues v0.4.2 (0xPolygon#271)

* fix: downloader

* fix: driver handling closing download channel

* fix: e2e tests

* fix: new logic

* fix: improved logic

* fix: remove aggsender password

aggsender/aggsender_test.go

@@ -59,7 +59,6 @@ func TestConfigString(t *testing.T) {
 		"BlockGetInterval: 10s\n" +
 		"CheckSettledInterval: 20s\n" +
 		"AggsenderPrivateKeyPath: /path/to/key\n" +
-		"AggsenderPrivateKeyPassword: password\n" +
 		"URLRPCL2: http://l2.rpc.url\n" +
 		"BlockFinality: latestBlock\n" +
 		"EpochNotificationPercentage: 50\n" +

aggsender/config.go

@@ -38,7 +38,6 @@ func (c Config) String() string {
 		"BlockGetInterval: " + c.BlockGetInterval.String() + "\n" +
 		"CheckSettledInterval: " + c.CheckSettledInterval.String() + "\n" +
 		"AggsenderPrivateKeyPath: " + c.AggsenderPrivateKey.Path + "\n" +
-		"AggsenderPrivateKeyPassword: " + c.AggsenderPrivateKey.Password + "\n" +
 		"URLRPCL2: " + c.URLRPCL2 + "\n" +
 		"BlockFinality: " + c.BlockFinality + "\n" +
 		"EpochNotificationPercentage: " + fmt.Sprintf("%d", c.EpochNotificationPercentage) + "\n" +

bridgesync/bridgesync.go

@@ -43,6 +43,7 @@ func NewL1(
 	retryAfterErrorPeriod time.Duration,
 	maxRetryAttemptsAfterError int,
 	originNetwork uint32,
+	finalizedBlockType etherman.BlockNumberFinality,
 ) (*BridgeSync, error) {
 	return newBridgeSync(
 		ctx,
@@ -59,6 +60,7 @@ func NewL1(
 		maxRetryAttemptsAfterError,
 		originNetwork,
 		false,
+		finalizedBlockType,
 	)
 }
 
@@ -76,6 +78,7 @@ func NewL2(
 	retryAfterErrorPeriod time.Duration,
 	maxRetryAttemptsAfterError int,
 	originNetwork uint32,
+	finalizedBlockType etherman.BlockNumberFinality,
 ) (*BridgeSync, error) {
 	return newBridgeSync(
 		ctx,
@@ -92,6 +95,7 @@ func NewL2(
 		maxRetryAttemptsAfterError,
 		originNetwork,
 		true,
+		finalizedBlockType,
 	)
 }
 
@@ -110,6 +114,7 @@ func newBridgeSync(
 	maxRetryAttemptsAfterError int,
 	originNetwork uint32,
 	syncFullClaims bool,
+	finalizedBlockType etherman.BlockNumberFinality,
 ) (*BridgeSync, error) {
 	processor, err := newProcessor(dbPath, l1OrL2ID)
 	if err != nil {
@@ -147,6 +152,7 @@ func newBridgeSync(
 		appender,
 		[]common.Address{bridge},
 		rh,
+		finalizedBlockType,
 	)
 	if err != nil {
 		return nil, err

bridgesync/bridgesync_test.go

@@ -52,6 +52,7 @@ func TestNewLx(t *testing.T) {
 		retryAfterErrorPeriod,
 		maxRetryAttemptsAfterError,
 		originNetwork,
+		blockFinalityType,
 	)
 
 	assert.NoError(t, err)
@@ -72,6 +73,7 @@ func TestNewLx(t *testing.T) {
 		retryAfterErrorPeriod,
 		maxRetryAttemptsAfterError,
 		originNetwork,
+		blockFinalityType,
 	)
 
 	assert.NoError(t, err)

bridgesync/e2e_test.go

@@ -29,7 +29,7 @@ func TestBridgeEventE2E(t *testing.T) {
 	go rd.Start(ctx) //nolint:errcheck
 
 	testClient := helpers.TestClient{ClientRenamed: client.Client()}
-	syncer, err := bridgesync.NewL1(ctx, dbPathSyncer, setup.EBZkevmBridgeAddr, 10, etherman.LatestBlock, rd, testClient, 0, time.Millisecond*10, 0, 0, 1)
+	syncer, err := bridgesync.NewL1(ctx, dbPathSyncer, setup.EBZkevmBridgeAddr, 10, etherman.LatestBlock, rd, testClient, 0, time.Millisecond*10, 0, 0, 1, etherman.SafeBlock)
 	require.NoError(t, err)
 
 	go syncer.Start(ctx)

claimsponsor/e2e_test.go

@@ -26,7 +26,7 @@ func TestE2EL1toEVML2(t *testing.T) {
 	env := aggoraclehelpers.SetupAggoracleWithEVMChain(t)
 	dbPathBridgeSyncL1 := path.Join(t.TempDir(), "file::memory:?cache=shared")
 	testClient := helpers.TestClient{ClientRenamed: env.L1Client.Client()}
-	bridgeSyncL1, err := bridgesync.NewL1(ctx, dbPathBridgeSyncL1, env.BridgeL1Addr, 10, etherman.LatestBlock, env.ReorgDetector, testClient, 0, time.Millisecond*10, 0, 0, 1)
+	bridgeSyncL1, err := bridgesync.NewL1(ctx, dbPathBridgeSyncL1, env.BridgeL1Addr, 10, etherman.LatestBlock, env.ReorgDetector, testClient, 0, time.Millisecond*10, 0, 0, 1, etherman.SafeBlock)
 	require.NoError(t, err)
 	go bridgeSyncL1.Start(ctx)
 

cmd/run.go

@@ -547,6 +547,7 @@ func runL1InfoTreeSyncerIfNeeded(
 		cfg.L1InfoTreeSync.RetryAfterErrorPeriod.Duration,
 		cfg.L1InfoTreeSync.MaxRetryAttemptsAfterError,
 		l1infotreesync.FlagNone,
+		etherman.FinalizedBlock,
 	)
 	if err != nil {
 		log.Fatal(err)
@@ -731,6 +732,7 @@ func runBridgeSyncL1IfNeeded(
 		cfg.RetryAfterErrorPeriod.Duration,
 		cfg.MaxRetryAttemptsAfterError,
 		cfg.OriginNetwork,
+		etherman.FinalizedBlock,
 	)
 	if err != nil {
 		log.Fatalf("error creating bridgeSyncL1: %s", err)
@@ -764,6 +766,7 @@ func runBridgeSyncL2IfNeeded(
 		cfg.RetryAfterErrorPeriod.Duration,
 		cfg.MaxRetryAttemptsAfterError,
 		cfg.OriginNetwork,
+		etherman.LatestBlock,
 	)
 	if err != nil {
 		log.Fatalf("error creating bridgeSyncL2: %s", err)

etherman/types.go

@@ -103,6 +103,10 @@ const (
 	EarliestBlock  = BlockNumberFinality("EarliestBlock")
 )
 
+func (b BlockNumberFinality) IsFinalized() bool {
+	return b == FinalizedBlock
+}
+
 func (b *BlockNumberFinality) ToBlockNum() (*big.Int, error) {
 	switch *b {
 	case FinalizedBlock:

l1infotreesync/e2e_test.go

@@ -66,8 +66,8 @@ func TestE2E(t *testing.T) {
 	rdm.On("AddBlockToTrack", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil)
 
 	client, auth, gerAddr, verifyAddr, gerSc, verifySC := newSimulatedClient(t)
-	syncer, err := l1infotreesync.New(ctx, dbPath, gerAddr, verifyAddr, 10, etherman.LatestBlock, rdm, client.Client(), time.Millisecond, 0, 100*time.Millisecond, 3,
-		l1infotreesync.FlagAllowWrongContractsAddrs)
+	syncer, err := l1infotreesync.New(ctx, dbPath, gerAddr, verifyAddr, 10, etherman.LatestBlock, rdm, client.Client(), time.Millisecond, 0, 100*time.Millisecond, 25,
+		l1infotreesync.FlagAllowWrongContractsAddrs, etherman.SafeBlock)
 	require.NoError(t, err)
 
 	go syncer.Start(ctx)
@@ -165,7 +165,7 @@ func TestWithReorgs(t *testing.T) {
 	require.NoError(t, rd.Start(ctx))
 
 	syncer, err := l1infotreesync.New(ctx, dbPathSyncer, gerAddr, verifyAddr, 10, etherman.LatestBlock, rd, client.Client(), time.Millisecond, 0, time.Second, 25,
-		l1infotreesync.FlagAllowWrongContractsAddrs)
+		l1infotreesync.FlagAllowWrongContractsAddrs, etherman.SafeBlock)
 	require.NoError(t, err)
 	go syncer.Start(ctx)
 
@@ -228,9 +228,6 @@ func TestWithReorgs(t *testing.T) {
 	// Block 4, 5, 6 after the fork
 	commitBlocks(t, client, 3, time.Millisecond*500)
 
-	// Make sure syncer is up to date
-	waitForSyncerToCatchUp(ctx, t, syncer, client)
-
 	// Assert rollup exit root after the fork - should be zero since there are no events in the block after the fork
 	expectedRollupExitRoot, err = verifySC.GetRollupExitRoot(&bind.CallOpts{Pending: false})
 	require.NoError(t, err)
@@ -244,11 +241,12 @@ func TestWithReorgs(t *testing.T) {
 	require.NoError(t, err)
 	time.Sleep(time.Millisecond * 500)
 
+	commitBlocks(t, client, 1, time.Millisecond*100)
+
 	// create some events and update the trees
 	updateL1InfoTreeAndRollupExitTree(2, 1)
 
-	// Block 4, 5, 6, 7 after the fork
-	commitBlocks(t, client, 4, time.Millisecond*100)
+	commitBlocks(t, client, 1, time.Millisecond*100)
 
 	// Make sure syncer is up to date
 	waitForSyncerToCatchUp(ctx, t, syncer, client)
@@ -283,7 +281,7 @@ func TestStressAndReorgs(t *testing.T) {
 	require.NoError(t, rd.Start(ctx))
 
 	syncer, err := l1infotreesync.New(ctx, dbPathSyncer, gerAddr, verifyAddr, 10, etherman.LatestBlock, rd, client.Client(), time.Millisecond, 0, time.Second, 100,
-		l1infotreesync.FlagAllowWrongContractsAddrs)
+		l1infotreesync.FlagAllowWrongContractsAddrs, etherman.SafeBlock)
 	require.NoError(t, err)
 	go syncer.Start(ctx)
 
@@ -323,7 +321,7 @@ func TestStressAndReorgs(t *testing.T) {
 		}
 	}
 
-	commitBlocks(t, client, 1, time.Millisecond*10)
+	commitBlocks(t, client, 11, time.Millisecond*10)
 
 	waitForSyncerToCatchUp(ctx, t, syncer, client)
 

l1infotreesync/l1infotreesync.go

@@ -47,6 +47,7 @@ func New(
 	retryAfterErrorPeriod time.Duration,
 	maxRetryAttemptsAfterError int,
 	flags CreationFlags,
+	finalizedBlockType etherman.BlockNumberFinality,
 ) (*L1InfoTreeSync, error) {
 	processor, err := newProcessor(dbPath)
 	if err != nil {
@@ -83,6 +84,7 @@ func New(
 		appender,
 		[]common.Address{globalExitRoot, rollupManager},
 		rh,
+		finalizedBlockType,
 	)
 	if err != nil {
 		return nil, err

l1infotreesync/processor.go

@@ -277,7 +277,9 @@ func (p *processor) Reorg(ctx context.Context, firstReorgedBlock uint64) error {
 		p.halted = false
 		p.haltedReason = ""
 	}
+
 	shouldRollback = false
+
 	return nil
 }
 

lastgersync/evmdownloader.go

@@ -49,7 +49,7 @@ func newDownloader(
 
 	return &downloader{
 		EVMDownloaderImplementation: sync.NewEVMDownloaderImplementation(
-			"lastgersync", l2Client, blockFinality, waitForNewBlocksPeriod, nil, nil, nil, rh,
+			"lastgersync", l2Client, blockFinality, waitForNewBlocksPeriod, nil, nil, nil, rh, blockFinality,
 		),
 		l2Client:       l2Client,
 		gerContract:    gerContract,