Decouple validation from import by RogerPodacter · Pull Request #108 · ethscriptions-protocol/ethscriptions-indexer

RogerPodacter · 2025-09-22T16:11:28Z

No description provided.

Remove old Rails files

Copilot

Pull Request Overview

This PR decouples validation from the import process by implementing asynchronous validation using SolidQueue. It removes synchronous validation from the block importer and introduces a separate validation job system that processes L1 blocks independently.

Refactored block validation to use asynchronous job processing via SolidQueue
Replaced custom ValidationResult class with ActiveRecord model for database persistence
Simplified block import process to handle single blocks instead of batches

Reviewed Changes

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

Show a summary per file

File	Description
lib/l1_rpc_prefetcher.rb	Removed API data prefetching and reduced thread configuration
lib/block_validator.rb	Simplified validation interface and replaced custom result class with OpenStruct
app/services/eth_block_importer.rb	Refactored to single-block imports with async validation via jobs
app/models/validation_result.rb	New ActiveRecord model for persisting validation results
app/jobs/validation_job.rb	New job for async validation processing
app/jobs/gap_detection_job.rb	New job for detecting and filling validation gaps
config/*	Rails 8 upgrade and SolidQueue configuration
db/*	Database schema and migrations for validation results

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-09-22T16:12:53Z

+      # Handle case where block doesn't exist yet (normal when caught up)
+      if block.nil?
+        Rails.logger.debug "Block #{block_number} not yet available on L1"
+        return { error: :not_ready, block_number: block_number }


[nitpick] The returned hash includes :block_number but this key is not used in the calling code. Consider removing the unused key or document why it's needed for future use.

Suggested change

return { error: :not_ready, block_number: block_number }

return { error: :not_ready }

Copilot · 2025-09-22T16:12:53Z

    reset_validation_state

-    Rails.logger.info "Validating L1 block #{l1_block_number} with #{l2_block_hashes.size} L2 blocks"
+    # Rails.logger.info "Validating L1 block #{l1_block_number} with #{l2_block_hashes.size} L2 blocks"


Commented-out code should be removed rather than left in the codebase. If this logging is needed conditionally, implement proper conditional logging instead.

Suggested change

# Rails.logger.info "Validating L1 block #{l1_block_number} with #{l2_block_hashes.size} L2 blocks"

Rails.logger.info "Validating L1 block #{l1_block_number} with #{l2_block_hashes.size} L2 blocks" if ENV['DEBUG'] == '1'

Copilot · 2025-09-22T16:12:53Z

-    elsif ENV['VALIDATE_IMPORT'] == 'true'
-      logger.warn "Validation requested but not enabled in importer initialization"
+    # Queue validation job if validation is enabled
+    if ENV.fetch('VALIDATION_ENABLED').casecmp?('true')


ENV.fetch('VALIDATION_ENABLED') will raise an error if the environment variable is not set, since no default value is provided. This should include a default value like ENV.fetch('VALIDATION_ENABLED', 'false') to match the pattern used elsewhere in the codebase.

Suggested change

if ENV.fetch('VALIDATION_ENABLED').casecmp?('true')

if ENV.fetch('VALIDATION_ENABLED', 'false').casecmp?('true')

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

Copilot

Pull Request Overview

Copilot reviewed 20 out of 21 changed files in this pull request and generated 5 comments.

Comments suppressed due to low confidence (1)

app/services/eth_block_importer.rb:1

The validation_duration calculation is incorrect as it subtracts Time.current from itself, which will always result in 0. This should likely use a start time variable that was captured at the beginning of the validation.

class EthBlockImporter

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-09-22T17:08:50Z

    # Only enqueue a reasonable number at once to avoid overwhelming the promise system
-    # We'll enqueue more as blocks are consumed
-    max_to_enqueue = [@threads * 20, 100].min  # At most 20x thread count or 100
+    max_to_enqueue = [@threads * 10, 50].min


The magic numbers 10 and 50 should be documented or extracted as named constants to explain their purpose in the enqueueing algorithm.

Copilot · 2025-09-22T17:08:51Z

+      creation_results: [],
+      transfer_results: [],
+      storage_results: []


The @debug_data structure defined here uses different keys (creation_results, transfer_results, storage_results) than what's used in the reset_validation_state method (creation_comparisons, transfer_comparisons, storage_checks, event_comparisons). This inconsistency could lead to bugs.

Suggested change

creation_results: [],

transfer_results: [],

storage_results: []

creation_comparisons: [],

transfer_comparisons: [],

storage_checks: [],

event_comparisons: []

Copilot · 2025-09-22T17:08:51Z

+      creation_comparisons: [],
+      transfer_comparisons: [],
+      storage_checks: [],
+      event_comparisons: []


The @debug_data structure keys are inconsistent with the initialization at lines 11-15. The keys should match between initialization and reset to avoid potential runtime errors.

Suggested change

creation_comparisons: [],

transfer_comparisons: [],

storage_checks: [],

event_comparisons: []

creation_results: [],

transfer_results: [],

storage_results: []

Copilot · 2025-09-22T17:08:51Z

+    where("JSON_EXTRACT(validation_stats, '$.validation_details.expected_creations') > 0 OR JSON_EXTRACT(validation_stats, '$.validation_details.expected_transfers') > 0 OR JSON_EXTRACT(validation_stats, '$.validation_details.storage_checks') > 0")
+  }
+


This complex JSON query should be extracted to a named scope or method to improve readability and maintainability. The repeated JSON_EXTRACT calls make the query difficult to understand and modify.

Suggested change

where("JSON_EXTRACT(validation_stats, '$.validation_details.expected_creations') > 0 OR JSON_EXTRACT(validation_stats, '$.validation_details.expected_transfers') > 0 OR JSON_EXTRACT(validation_stats, '$.validation_details.storage_checks') > 0")

}

where(activity_json_condition)

}

# Returns the SQL condition for checking activity in validation_stats JSON

def self.activity_json_condition

<<~SQL.squish

JSON_EXTRACT(validation_stats, '$.validation_details.expected_creations') > 0

OR JSON_EXTRACT(validation_stats, '$.validation_details.expected_transfers') > 0

OR JSON_EXTRACT(validation_stats, '$.validation_details.storage_checks') > 0

SQL

end

Copilot · 2025-09-22T17:08:52Z

+
+      # Search backwards from current L2 tip to find blocks from this L1 block
+      # This is expensive but necessary for gap filling
+      (0..latest_l2_block_number).reverse_each do |l2_block_num|


This loop iterates through potentially millions of blocks in reverse order, making RPC calls for each one. This could be extremely slow and resource-intensive. Consider implementing a more efficient search strategy, such as binary search or caching L1-to-L2 block mappings.

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

RogerPodacter · 2025-09-22T17:30:00Z

bugbot run

Copilot

Pull Request Overview

Copilot reviewed 21 out of 22 changed files in this pull request and generated 4 comments.

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-09-22T17:31:49Z

+  def validate_l1_block(l1_block_number, l2_block_hashes)
    reset_validation_state
+    validation_start_time = Time.current

-    Rails.logger.info "Validating L1 block #{l1_block_number} with #{l2_block_hashes.size} L2 blocks"
+    Rails.logger.debug "Validating L1 block #{l1_block_number} with #{l2_block_hashes.size} L2 blocks"

-    # Use prefetched data if provided, otherwise fetch from API
-    expected = expected_override || fetch_expected_data(l1_block_number)
+    # Fetch expected data from API
+    expected = fetch_expected_data(l1_block_number)


The method signature has changed from accepting an optional expected_override parameter to no longer accepting it. This is a breaking change that could affect existing callers expecting the previous interface.

Copilot · 2025-09-22T17:31:50Z


-    result = ValidationResult.new(
+    # Return comprehensive debugging information
+    result = OpenStruct.new(


Returning an OpenStruct instead of a proper class (like the previous ValidationResult class) reduces type safety and IDE support. Consider keeping a structured result class for better maintainability.

Copilot · 2025-09-22T17:31:50Z

-  def import_batch_size
-    [blocks_behind, ENV.fetch('BLOCK_IMPORT_BATCH_SIZE', 2).to_i].min
-  end
+  # Removed batch processing - now imports one block at a time


This comment describes what was removed rather than explaining the current implementation. Consider updating to describe the current single-block import approach and its benefits.

Suggested change

# Removed batch processing - now imports one block at a time

# Imports one block at a time for improved reliability and easier handling of re-orgs

Copilot · 2025-09-22T17:31:50Z

+    [imported_ethscriptions_blocks, [eth_block]]
  end

+  # Legacy batch import method removed - use import_single_block instead


This comment refers to removed code that no longer exists. Since this is a comment about legacy code, it should be removed to avoid confusion.

Suggested change

# Legacy batch import method removed - use import_single_block instead

RogerPodacter added 2 commits September 22, 2025 10:29

Merge pull request #107 from ethscriptions-protocol/evm-backend-demo

64a1ae8

Remove old Rails files

Decouple validation from import

fd95c00

RogerPodacter requested a review from Copilot September 22, 2025 16:11

Copilot AI reviewed Sep 22, 2025

View reviewed changes

This comment was marked as outdated.

Sign in to view

RogerPodacter and others added 2 commits September 22, 2025 12:43

Update app/jobs/gap_detection_job.rb

a233ccd

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

Improve validation error visibility

65acb7c

RogerPodacter requested a review from Copilot September 22, 2025 17:07

Copilot AI reviewed Sep 22, 2025

View reviewed changes

This comment was marked as outdated.

Sign in to view

Update lib/l1_rpc_prefetcher.rb

f8cbc48

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

This comment was marked as outdated.

Sign in to view

Fixes

b90f970

RogerPodacter requested a review from Copilot September 22, 2025 17:29

Copilot AI reviewed Sep 22, 2025

View reviewed changes

This comment was marked as outdated.

Sign in to view

Improve logging, add misc files

8e8f10e

RogerPodacter merged commit 521a4b7 into evm-backend-demo Sep 22, 2025
2 checks passed

RogerPodacter deleted the decouple branch September 22, 2025 18:08

	return { error: :not_ready, block_number: block_number }
	return { error: :not_ready }

	# Rails.logger.info "Validating L1 block #{l1_block_number} with #{l2_block_hashes.size} L2 blocks"
	Rails.logger.info "Validating L1 block #{l1_block_number} with #{l2_block_hashes.size} L2 blocks" if ENV['DEBUG'] == '1'

	if ENV.fetch('VALIDATION_ENABLED').casecmp?('true')
	if ENV.fetch('VALIDATION_ENABLED', 'false').casecmp?('true')

		where("JSON_EXTRACT(validation_stats, '$.validation_details.expected_creations') > 0 OR JSON_EXTRACT(validation_stats, '$.validation_details.expected_transfers') > 0 OR JSON_EXTRACT(validation_stats, '$.validation_details.storage_checks') > 0")
		}

	# Removed batch processing - now imports one block at a time
	# Imports one block at a time for improved reliability and easier handling of re-orgs

Uh oh!

Conversation

RogerPodacter commented Sep 22, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Copilot AI Sep 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 22, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Copilot AI Sep 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 22, 2025

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

RogerPodacter commented Sep 22, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Copilot AI Sep 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 22, 2025

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants