feat: add tests #1448
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -1405,12 +1405,12 @@ func (p *processor) restoreBackwardLETBridges(tx dbtypes.Txer, backwardLETs []*B | |||||||
| ` | ||||||||
|
|
||||||||
| for _, backwardLET := range backwardLETs { | ||||||||
| 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(new(big.Int).Sub(backwardLET.NewDepositCount, big.NewInt(1))) | ||||||||
|
Comment on lines
+1408
to
+1413
There was a problem hiding this comment. 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
Contributor
There was a problem hiding this comment. Can you explain why we are decreasing the (previous and next) deposit counts by one? I fail to understand the logic. |
||||||||
| if err != nil { | ||||||||
| return fmt.Errorf("invalid new deposit count: %w", err) | ||||||||
| } | ||||||||
|
|
@@ -1590,7 +1590,8 @@ func (p *processor) ProcessBlock(ctx context.Context, block sync.Block) error { | |||||||
| return err | ||||||||
| } | ||||||||
|
|
||||||||
| adjustedCount := new(big.Int).Sub(event.BackwardLET.NewDepositCount, big.NewInt(1)) | ||||||||
|
Contributor
There was a problem hiding this comment. Does the 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
Contributor
Author
There was a problem hiding this comment. so basically what smart contract emits is deposit count which is 1 indexed (starts from 1 and not 0) |
||||||||
| newDepositCount, leafIndex, err := normalizeDepositCount(adjustedCount) | ||||||||
|
Comment on lines
+1593
to
+1594
There was a problem hiding this comment. 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
|
||||||||
| if err != nil { | ||||||||
| return err | ||||||||
| } | ||||||||
|
|
@@ -1754,7 +1755,7 @@ func (p *processor) handleForwardLETEvent(tx dbtypes.Txer, event *ForwardLET, bl | |||||||
| return 0, fmt.Errorf("failed to decode new leaves in forward LET: %w", err) | ||||||||
| } | ||||||||
|
|
||||||||
| newDepositCount := uint32(event.PreviousDepositCount.Uint64()) | ||||||||
|
There was a problem hiding this comment. The change from
Suggested change
|
||||||||
| newBlockPos := event.BlockPos | ||||||||
| if blockPos != nil { | ||||||||
| newBlockPos = *blockPos | ||||||||
|
|
||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,7 +5,6 @@ import ( | |
| "encoding/json" | ||
| "errors" | ||
| "fmt" | ||
| "math/big" | ||
| "os" | ||
| "path" | ||
|
|
@@ -487,8 +486,8 @@ var ( | |
| Event{BackwardLET: &BackwardLET{ | ||
| BlockNum: 5, | ||
| BlockPos: 4, | ||
| PreviousDepositCount: big.NewInt(4), | ||
| NewDepositCount: big.NewInt(3), | ||
| PreviousRoot: common.HexToHash("0x15cd4b94cacc2cf50d055e1adb5fbfe5cd95485e121a5c411d73e263f2a66685"), | ||
| NewRoot: common.HexToHash("0xa03113d9ce128863f29479689c82d0b37ebc9432c569c3a57f22d6c008256c5b"), | ||
| }}, | ||
|
|
@@ -5447,8 +5446,8 @@ func TestProcessor_BackwardLET(t *testing.T) { | |
| Event{BackwardLET: &BackwardLET{ | ||
| BlockNum: uint64(len(blocks) + 1), | ||
| BlockPos: 0, | ||
| PreviousDepositCount: big.NewInt(4), | ||
| NewDepositCount: big.NewInt(3), | ||
| PreviousRoot: common.HexToHash("0x9ba667158a062be548e5c1b2e8a9a2ad03b693e562535b0723880627c6664b02"), | ||
| NewRoot: common.HexToHash("0xa9d31ebbb97c7cd7c7103bee8af7d0b4c83771939baba0b415b0f94c4c39fd84"), | ||
| }}, | ||
|
|
@@ -5471,8 +5470,8 @@ func TestProcessor_BackwardLET(t *testing.T) { | |
| Event{BackwardLET: &BackwardLET{ | ||
| BlockNum: uint64(len(blocks) + 1), | ||
| BlockPos: 0, | ||
| PreviousDepositCount: big.NewInt(6), | ||
| NewDepositCount: big.NewInt(1), | ||
| PreviousRoot: common.HexToHash("0x9ba667158a062be548e5c1b2e8a9a2ad03b693e562535b0723880627c6664b02"), | ||
| NewRoot: common.HexToHash("0x283c52c3d10a22d01f95f5bcab5e823675c9855bd40b1e82f32b0437b3b6a446"), | ||
| }}, | ||
|
|
@@ -5495,8 +5494,8 @@ func TestProcessor_BackwardLET(t *testing.T) { | |
| Event{BackwardLET: &BackwardLET{ | ||
| BlockNum: uint64(len(blocks) + 1), | ||
| BlockPos: 0, | ||
| PreviousDepositCount: big.NewInt(6), | ||
| NewDepositCount: big.NewInt(5), | ||
| PreviousRoot: common.HexToHash("0x9ba667158a062be548e5c1b2e8a9a2ad03b693e562535b0723880627c6664b02"), | ||
| NewRoot: common.HexToHash("0x44e1bf8449ecec2b8b1d123fab00d33c9acb308e590605adf5f6e2de4d1c1133"), | ||
| }}, | ||
|
|
@@ -5520,8 +5519,8 @@ func TestProcessor_BackwardLET(t *testing.T) { | |
| Event{BackwardLET: &BackwardLET{ | ||
| BlockNum: uint64(len(blocks) + 1), | ||
| BlockPos: 0, | ||
| PreviousDepositCount: big.NewInt(6), | ||
| NewDepositCount: big.NewInt(3), | ||
| PreviousRoot: common.HexToHash("0x9ba667158a062be548e5c1b2e8a9a2ad03b693e562535b0723880627c6664b02"), | ||
| NewRoot: common.HexToHash("0xa9d31ebbb97c7cd7c7103bee8af7d0b4c83771939baba0b415b0f94c4c39fd84"), | ||
| }}, | ||
|
|
@@ -5536,44 +5535,6 @@ func TestProcessor_BackwardLET(t *testing.T) { | |
| skipBlocks: []uint64{2, 3}, // all the bridges from these blocks were backwarded | ||
| archivedDepositCounts: []uint32{3, 4, 5}, | ||
| }, | ||
|
There was a problem hiding this comment. 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. |
||
| { | ||
| name: "backward let on empty bridge table", | ||
| setupBlocks: func() []sync.Block { | ||
|
|
@@ -5585,52 +5546,14 @@ func TestProcessor_BackwardLET(t *testing.T) { | |
| Event{BackwardLET: &BackwardLET{ | ||
| BlockNum: 1, | ||
| BlockPos: 0, | ||
| PreviousDepositCount: big.NewInt(7), | ||
| NewDepositCount: big.NewInt(4), | ||
| }}, | ||
| }, | ||
| }} | ||
| }, | ||
| targetDepositCount: 0, | ||
| }, | ||
| { | ||
|
There was a problem hiding this comment. 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. |
||
| name: "backward let after a couple of bridges + reorg backward let", | ||
| setupBlocks: func() []sync.Block { | ||
|
|
@@ -5642,8 +5565,8 @@ func TestProcessor_BackwardLET(t *testing.T) { | |
| Event{BackwardLET: &BackwardLET{ | ||
| BlockNum: 4, | ||
| BlockPos: 0, | ||
| PreviousDepositCount: big.NewInt(6), | ||
| NewDepositCount: big.NewInt(3), | ||
| PreviousRoot: common.HexToHash("0x9ba667158a062be548e5c1b2e8a9a2ad03b693e562535b0723880627c6664b02"), | ||
| NewRoot: common.HexToHash("0xa9d31ebbb97c7cd7c7103bee8af7d0b4c83771939baba0b415b0f94c4c39fd84"), | ||
| }}, | ||
|
|
@@ -5668,8 +5591,8 @@ func TestProcessor_BackwardLET(t *testing.T) { | |
| Event{BackwardLET: &BackwardLET{ | ||
| BlockNum: uint64(len(blocks) + 1), | ||
| BlockPos: 0, | ||
| PreviousDepositCount: big.NewInt(6), | ||
| NewDepositCount: big.NewInt(3), | ||
| PreviousRoot: common.HexToHash("0x9ba667158a062be548e5c1b2e8a9a2ad03b693e562535b0723880627c6664b02"), | ||
| NewRoot: common.HexToHash("0xa9d31ebbb97c7cd7c7103bee8af7d0b4c83771939baba0b415b0f94c4c39fd84"), | ||
| }}, | ||
|
|
@@ -5888,7 +5811,7 @@ func TestHandleForwardLETEvent(t *testing.T) { | |
| BlockPos: 5, | ||
| BlockTimestamp: 1234567890, | ||
| TxnHash: common.HexToHash("0xabc123"), | ||
| PreviousDepositCount: big.NewInt(int64(initialDepositCount + 1)), | ||
| PreviousRoot: initialRoot, | ||
| NewDepositCount: big.NewInt(int64(initialDepositCount + 1)), | ||
|
temaniarpit27 marked this conversation as resolved.
|
||
| NewLeaves: encodedLeaves, | ||
|
|
@@ -5993,7 +5916,7 @@ func TestHandleForwardLETEvent(t *testing.T) { | |
| BlockPos: 10, | ||
| BlockTimestamp: 1234567900, | ||
| TxnHash: common.HexToHash("0xdef456"), | ||
| PreviousDepositCount: big.NewInt(int64(initialDepositCount + 1)), | ||
| PreviousRoot: initialRoot, | ||
| NewDepositCount: big.NewInt(int64(initialDepositCount + uint32(len(leaves)))), | ||
|
temaniarpit27 marked this conversation as resolved.
|
||
| NewLeaves: encodedLeaves, | ||
|
|
@@ -6092,7 +6015,7 @@ func TestHandleForwardLETEvent(t *testing.T) { | |
| BlockPos: 20, | ||
| BlockTimestamp: 1234567950, | ||
| TxnHash: common.HexToHash("0xforward789"), | ||
| PreviousDepositCount: big.NewInt(int64(initialDepositCount + 1)), | ||
| PreviousRoot: initialRoot, | ||
| NewDepositCount: big.NewInt(int64(initialDepositCount + 1)), | ||
|
temaniarpit27 marked this conversation as resolved.
|
||
| NewLeaves: encodedLeaves, | ||
|
|
@@ -6201,7 +6124,7 @@ func TestHandleForwardLETEvent(t *testing.T) { | |
| BlockPos: 30, | ||
| BlockTimestamp: 1234567999, | ||
| TxnHash: common.HexToHash("0xforward999"), | ||
| PreviousDepositCount: big.NewInt(int64(initialDepositCount + 1)), | ||
| PreviousRoot: initialRoot, | ||
| NewDepositCount: big.NewInt(int64(initialDepositCount + 1)), | ||
|
temaniarpit27 marked this conversation as resolved.
|
||
| NewLeaves: encodedLeaves, | ||
|
|
@@ -6262,7 +6185,7 @@ func TestHandleForwardLETEvent(t *testing.T) { | |
| BlockPos: 5, | ||
| BlockTimestamp: 1234567890, | ||
| TxnHash: common.HexToHash("0xabc123"), | ||
| PreviousDepositCount: big.NewInt(int64(initialDepositCount + 1)), | ||
|
temaniarpit27 marked this conversation as resolved.
|
||
| PreviousRoot: common.HexToHash("0xWRONG"), // Wrong root | ||
| NewDepositCount: big.NewInt(int64(initialDepositCount + 1)), | ||
| NewRoot: common.HexToHash("0x999"), | ||
|
|
@@ -6319,7 +6242,7 @@ func TestHandleForwardLETEvent(t *testing.T) { | |
| BlockPos: 5, | ||
| BlockTimestamp: 1234567890, | ||
| TxnHash: common.HexToHash("0xabc123"), | ||
| PreviousDepositCount: big.NewInt(int64(initialDepositCount + 1)), | ||
|
temaniarpit27 marked this conversation as resolved.
|
||
| PreviousRoot: initialRoot, | ||
| NewDepositCount: big.NewInt(int64(initialDepositCount + 1)), | ||
| NewRoot: common.HexToHash("0xWRONG"), // Wrong new root | ||
|
|
@@ -6352,7 +6275,7 @@ func TestHandleForwardLETEvent(t *testing.T) { | |
| BlockPos: 5, | ||
| BlockTimestamp: 1234567890, | ||
| TxnHash: common.HexToHash("0xabc123"), | ||
| PreviousDepositCount: big.NewInt(int64(initialDepositCount + 1)), | ||
| PreviousRoot: initialRoot, | ||
| NewDepositCount: big.NewInt(int64(initialDepositCount + 1)), | ||
|
temaniarpit27 marked this conversation as resolved.
|
||
| NewRoot: common.Hash{}, | ||
|
|
@@ -6407,7 +6330,7 @@ func TestHandleForwardLETEvent(t *testing.T) { | |
| BlockPos: 5, | ||
| BlockTimestamp: 1234567890, | ||
| TxnHash: common.HexToHash("0xabc123"), | ||
| PreviousDepositCount: big.NewInt(int64(initialDepositCount + 1)), | ||
| PreviousRoot: initialRoot, | ||
| NewDepositCount: big.NewInt(int64(initialDepositCount + 1)), | ||
|
temaniarpit27 marked this conversation as resolved.
|
||
| NewLeaves: encodedLeaves, | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.