feat: add tests #1448

temaniarpit27 · 2026-01-07T09:46:15Z

🔄 Changes Summary

Add tests for backward and forward let

🐞 Issues

Closes Testing bats #1447

🔗 Related PRs

feat: add backward, forward let test e2e#231

## 🔄 Changes Summary This PR adds a function to rewind merkle tree to a previous root index. (This will be used for backwarding of `LocalExitTree`). ## ⚠️ Breaking Changes NA ## 📋 Config Updates NA ## ✅ Testing - 🤖 **Automatic**: `aggkit` CI

…tree accordingly

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot

Pull request overview

This pull request adds support for testing backward and forward Last Exit Tree (LET) operations by adjusting the deposit count semantics to use 1-indexed counting in events while maintaining 0-indexed internal storage.

Modified processor logic to subtract 1 from event deposit counts when processing BackwardLET and ForwardLET events
Updated test data to use 1-indexed deposit counts (deposit counts in events are now 1 higher than before)
Enhanced error handling for Kurtosis artifact downloads with detailed troubleshooting information
Updated E2E workflow to use newer commit references

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 10 comments.

Show a summary per file

File	Description
bridgesync/processor.go	Adjusted BackwardLET and ForwardLET deposit count handling to subtract 1 from event values to convert from 1-indexed to 0-indexed internal representation
bridgesync/processor_test.go	Updated test data to use 1-indexed deposit counts; removed unused math import and test cases for overlapping BackwardLET and invalid deposit count validation
scripts/local_config_helper	Added dynamic artifact name construction, enhanced error messages for artifact downloads, and renamed variable from zkevm_rollup_chain_id to l2_chain_id
test/run-local-e2e.sh	Fixed shell pattern to avoid matching bare `-`, corrected ENCLAVE_NAME for pessimistic tests, commented out tests except for backward/forward LET test with filter
.github/workflows/test-e2e.yml	Updated agglayer-e2e-ref commit hashes to newer version
.github/test_e2e_op_args_base.json	Enabled deploy_cdk_bridge_infra flag

bridgesync/processor_test.go

test/run-local-e2e.sh

scripts/local_config_helper

bridgesync/processor_test.go

Copilot

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 7 comments.

Copilot · 2026-01-10T06:05:41Z

test/run-local-e2e.sh

    single-l2-network-op-succinct)
        bats ./tests/aggkit/bridge-e2e.bats || exit 1
-        bats ./tests/aggkit/e2e-pp.bats || exit 1
        bats ./tests/aggkit/bridge-sovereign-chain-e2e.bats || exit 1


The removal of bats ./tests/aggkit/e2e-pp.bats test execution in the single-l2-network-op-succinct test type appears unrelated to the stated purpose of this PR (adding tests for backward and forward LET). This removal reduces test coverage for the op-succinct configuration. If this test is now obsolete or has been moved, this should be documented in the PR description. Otherwise, this appears to be an accidental deletion that reduces test coverage.

Copilot · 2026-01-10T06:05:41Z

scripts/local_config_helper

+        # Extract service number from AGGKIT_SERVICE (e.g., aggkit-001 -> 001)
+        local SERVICE_NUMBER=$(echo "$AGGKIT_SERVICE" | sed 's/.*-//')


The artifact name construction uses sed 's/.*-//' to extract the service number, which assumes the service name always ends with a hyphen followed by the number. This could fail if the AGGKIT_SERVICE variable has a different format (e.g., no hyphen, multiple hyphens in unexpected places, or trailing text after the number). Consider using a more robust pattern like sed -n 's/.*-$[0-9]\+$$/\1/p' to ensure only numeric suffixes are extracted, or validate the format before extraction.

Suggested change

# Extract service number from AGGKIT_SERVICE (e.g., aggkit-001 -> 001)

local SERVICE_NUMBER=$(echo "$AGGKIT_SERVICE" | sed 's/.*-//')

# Extract numeric service number from AGGKIT_SERVICE (e.g., aggkit-001 -> 001)

local SERVICE_NUMBER

SERVICE_NUMBER=$(echo "$AGGKIT_SERVICE" | sed -n 's/.*-$[0-9]\+$$/\1/p')

if [ -z "$SERVICE_NUMBER" ]; then

log_fatal "AGGKIT_SERVICE ('$AGGKIT_SERVICE') must end with a numeric suffix like 'aggkit-001'"

fi

Copilot · 2026-01-10T06:05:41Z

bridgesync/processor.go

 	}

-	newDepositCount := uint32(event.PreviousDepositCount.Uint64()) + 1
+	newDepositCount := uint32(event.PreviousDepositCount.Uint64())


The change from uint32(event.PreviousDepositCount.Uint64()) + 1 to uint32(event.PreviousDepositCount.Uint64()) appears to change the semantics of how deposit counts are interpreted. This is a critical change that affects how bridges are indexed. The PreviousDepositCount now directly becomes the newDepositCount, rather than being incremented. Ensure this is the correct interpretation according to the contract's event semantics and that all consumers of this field are aware of this change in behavior.

Suggested change

newDepositCount := uint32(event.PreviousDepositCount.Uint64())

newDepositCount := uint32(event.PreviousDepositCount.Uint64()) + 1

Copilot · 2026-01-10T06:05:42Z

bridgesync/processor.go

+		prev, err := aggkitcommon.SafeUint64(new(big.Int).Sub(backwardLET.PreviousDepositCount, big.NewInt(1)))
 		if err != nil {
 			return fmt.Errorf("invalid previous deposit count: %w", err)
 		}

-		next, err := aggkitcommon.SafeUint64(backwardLET.NewDepositCount)
+		next, err := aggkitcommon.SafeUint64(new(big.Int).Sub(backwardLET.NewDepositCount, big.NewInt(1)))


The subtraction of 1 from both PreviousDepositCount and NewDepositCount in BackwardLET processing represents a significant semantic change. The adjustment is applied before converting to uint64 and storing in the database. This appears to change the interpretation of deposit counts from the contract events. Verify that this adjustment correctly aligns with the updated contract specification and that the query deposit_count > $1 AND deposit_count <= $2 in the restore logic still works correctly with the adjusted values.

Can you explain why we are decreasing the (previous and next) deposit counts by one? I fail to understand the logic.

Copilot · 2026-01-10T06:05:42Z

bridgesync/processor.go

+			adjustedCount := new(big.Int).Sub(event.BackwardLET.NewDepositCount, big.NewInt(1))
+			newDepositCount, leafIndex, err := normalizeDepositCount(adjustedCount)


The adjustment of NewDepositCount by subtracting 1 before calling normalizeDepositCount changes the interpretation of BackwardLET events. This adjustment will affect which bridges are archived and deleted, as it changes the threshold value passed to archiveAndDeleteBridgesAbove. Ensure this aligns with the contract's actual behavior and that the corresponding forward LET handling has been updated consistently.

Suggested change

adjustedCount := new(big.Int).Sub(event.BackwardLET.NewDepositCount, big.NewInt(1))

newDepositCount, leafIndex, err := normalizeDepositCount(adjustedCount)

newDepositCount, leafIndex, err := normalizeDepositCount(event.BackwardLET.NewDepositCount)

Copilot · 2026-01-10T06:05:42Z

bridgesync/processor_test.go

-			},
-			processBlockErrMsg: "invalid deposit count: value=4294967296 exceeds uint32 max",
-		},
 		{


The removal of test cases for invalid deposit counts (negative values and values exceeding uint32 range) eliminates important edge case testing. These tests verified that the system properly handles malformed or invalid contract events. Without these tests, there's no verification that the system will reject or properly handle invalid deposit count values that could potentially be emitted from a compromised or buggy contract. Consider keeping these tests or replacing them with equivalent validation tests that work with the new deposit count interpretation.

Copilot · 2026-01-10T06:05:43Z

bridgesync/processor_test.go

 			targetDepositCount:    8,
 			skipBlocks:            []uint64{2, 3}, // all the bridges from these blocks were backwarded
 			archivedDepositCounts: []uint32{3, 4, 5},
 		},


The removal of the "overlapping backward let events" test case removes coverage for an important scenario where multiple BackwardLET events might interact. This test case verified that the system correctly handles consecutive BackwardLET events that operate on overlapping deposit count ranges. Without this test, there's less confidence that the refactored deposit count handling correctly manages these complex scenarios. Consider whether this scenario is still relevant with the new deposit count interpretation and, if so, restore or replace this test case.

test/run-local-e2e.sh

.github/test_e2e_op_args_base.json

Stefan-Ethernal · 2026-01-12T12:51:38Z

bridgesync/processor.go

+		prev, err := aggkitcommon.SafeUint64(new(big.Int).Sub(backwardLET.PreviousDepositCount, big.NewInt(1)))
 		if err != nil {
 			return fmt.Errorf("invalid previous deposit count: %w", err)
 		}

-		next, err := aggkitcommon.SafeUint64(backwardLET.NewDepositCount)
+		next, err := aggkitcommon.SafeUint64(new(big.Int).Sub(backwardLET.NewDepositCount, big.NewInt(1)))


Can you explain why we are decreasing the (previous and next) deposit counts by one? I fail to understand the logic.

Stefan-Ethernal · 2026-01-12T14:03:34Z

bridgesync/processor.go

 			}

-			newDepositCount, leafIndex, err := normalizeDepositCount(event.BackwardLET.NewDepositCount)
+			adjustedCount := new(big.Int).Sub(event.BackwardLET.NewDepositCount, big.NewInt(1))


Does the NewDepositCount get decremented by 1, because leaf with the index NewDepositCount gets rollbacked from the tree as well?

If that's the case, would it make more sense if we would delete all the nodes that has the index greater than or equal to NewDepositCount (also adjust the comments of the functions), instead of decrementing. WDYT?

so basically what smart contract emits is deposit count which is 1 indexed (starts from 1 and not 0)
we here consider deposit count related to bridges which is 0 indexed (starts from 0)
so we need to manage accordingly here

sonarqubecloud · 2026-01-15T13:07:33Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

Stefan-Ethernal and others added 30 commits December 12, 2025 07:09

remove unused var

38c00a1

backward let event indexing scaffolding

e9834af

introduce new migration and refactor migrations resolution to be generic

003112a

TestBuildAppender increase coverage

c3dbef4

delete bridges on backward let

928cf5f

update exit tree on backward LET

aea7e8f

formatting

6ae4adb

remove bridges whose deposit count is greater than new deposit count

b2dccc5

remove InvalidClaim struct

aa2cd0c

correctly determine leaf index

a9c7035

archive bridges to bridge_archive table and handle reorgs

44774fa

increase code coverage, ignore double insertions to bridge_archive table

275aed8

Merge branch 'develop' into feat/index-backward-let

4d4d220

address 1st round of comments

ecc3ded

reverse the steps, first delete the bridges and then update the exit …

f98e350

…tree accordingly

Apply suggestions from code review

a74ca6e

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

fix sql syntax error

b2035c2

leaf index is the same as deposit count

38f1fbe

invoke BackwardLET string function

752f986

remove redundant check

7c941ab

fix sql syntax in migration file (2nd part)

7ffebfd

reinsert exit tree leaves on reorg of backward let event

2b1ce28

add bridge source column and add ordering by deposit_count

ba08508

move the new sql migrations to bridgesync0012 migration file

e4c8f62

formatting

3600cd6

Merge branch 'develop' into feat/index-backward-let

dba8654

TestProcessor_BackwardLET

c900f0a

increase code coverage in common pkg

2e39f39

TestProcessor_BackwardLET, backward LET on empty bridge table

72fa419

temaniarpit27 added 4 commits January 8, 2026 20:56

Merge branch 'develop' into arpit/1447

afca6a5

update tests

82b9c89

update tests

65a195a

update ref

b7600ba

temaniarpit27 requested review from Stefan-Ethernal, Copilot and goran-ethernal January 9, 2026 03:03

temaniarpit27 marked this pull request as ready for review January 9, 2026 03:03

Copilot started reviewing on behalf of temaniarpit27 January 9, 2026 03:03 View session

Copilot AI reviewed Jan 9, 2026

View reviewed changes

temaniarpit27 added 2 commits January 9, 2026 10:08

update ref

3b3aa60

update ref

dc758c7

temaniarpit27 requested a review from rachit77 January 9, 2026 05:35

temaniarpit27 added 3 commits January 9, 2026 15:22

update ref

1e64edb

update ref

825293b

update ref

ccd81db

temaniarpit27 requested a review from Copilot January 10, 2026 06:01

Copilot started reviewing on behalf of temaniarpit27 January 10, 2026 06:01 View session

Copilot AI reviewed Jan 10, 2026

View reviewed changes

temaniarpit27 force-pushed the arpit/1447 branch from ab225bf to ccd81db Compare January 12, 2026 05:17

rachit77 reviewed Jan 12, 2026

View reviewed changes

test/run-local-e2e.sh Show resolved Hide resolved

rachit77 reviewed Jan 12, 2026

View reviewed changes

.github/test_e2e_op_args_base.json Outdated Show resolved Hide resolved

rachit77 approved these changes Jan 12, 2026

View reviewed changes

Stefan-Ethernal reviewed Jan 12, 2026

View reviewed changes

temaniarpit27 added 5 commits January 13, 2026 13:39

update ref

aabeb3b

update ref

7ff6314

update ref

d700fd8

update ref

0656fbb

update ref

bd49e83

		# Extract service number from AGGKIT_SERVICE (e.g., aggkit-001 -> 001)
		local SERVICE_NUMBER=$(echo "$AGGKIT_SERVICE" \| sed 's/.*-//')

-        # Extract service number from AGGKIT_SERVICE (e.g., aggkit-001 -> 001)
-        local SERVICE_NUMBER=$(echo "$AGGKIT_SERVICE" | sed 's/.*-//')
+        # Extract numeric service number from AGGKIT_SERVICE (e.g., aggkit-001 -> 001)
+        local SERVICE_NUMBER
+        SERVICE_NUMBER=$(echo "$AGGKIT_SERVICE" | sed -n 's/.*-\([0-9]\+\)$/\1/p')
+        if [ -z "$SERVICE_NUMBER" ]; then
+            log_fatal "AGGKIT_SERVICE ('$AGGKIT_SERVICE') must end with a numeric suffix like 'aggkit-001'"
+        fi

	newDepositCount := uint32(event.PreviousDepositCount.Uint64())
	newDepositCount := uint32(event.PreviousDepositCount.Uint64()) + 1

		adjustedCount := new(big.Int).Sub(event.BackwardLET.NewDepositCount, big.NewInt(1))
		newDepositCount, leafIndex, err := normalizeDepositCount(adjustedCount)

feat: add tests #1448

Are you sure you want to change the base?

feat: add tests #1448

Uh oh!

Conversation

temaniarpit27 commented Jan 7, 2026

🔄 Changes Summary

🐞 Issues

🔗 Related PRs

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 10, 2026

Choose a reason for hiding this comment

Uh oh!

Stefan-Ethernal Jan 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 10, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Stefan-Ethernal Jan 12, 2026

Choose a reason for hiding this comment

Uh oh!

Stefan-Ethernal Jan 12, 2026

Choose a reason for hiding this comment

Uh oh!

temaniarpit27 Jan 13, 2026

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Jan 15, 2026

Quality Gate passed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants