Refactor

[RogerPodacter]

Note

Moves L1 deposit-building logic into EthTransaction, unifies transfer construction/signatures (including multi-transfer), and updates the Solidity contract and tests to the new function name and parameter order.

• Core (Ruby):
  • Deposit Building: Build ethscription deposits directly from EthTransaction via build_ethscription_deposits, processing calldata and ordered events (CreateEthscription, ESIP-1/2).
  • Transfers API: Consolidate factories into EthscriptionTransaction.build_create_ethscription and build_transfer with unified transfer_ids array; remove legacy validation; normalize address/hash helpers.
  • Calldata/Selectors: Update function selectors and calldata encoding:
    • Single: transferEthscription(address,bytes32).
    • Multiple: transferEthscriptions(address,bytes32[]) (note param order: address first).
  • Utilities: Add ByteString#bytesize; add UTF-8 input decoding with ESIP-7 support; event ordering helpers.
• Contracts (Solidity):
  • Rename transferMultipleEthscriptions(bytes32[],address) to transferEthscriptions(address,bytes32[]); use storage refs; minor refactors in transfer functions.
• Tests:
  • Update multi-transfer tests to new function name and argument order.
  • Add integration spec verifying selector choice and calldata for single vs multiple transfers.

^{Written by Cursor Bugbot for commit 8210799. This will update automatically on new commits. Configure here.}

[Copilot]

Pull Request Overview

This PR refactors ethscription deposit building and transfer handling to consolidate logic into EthTransaction and unify the transfer API. It updates the Solidity contract function name and parameter order to align with the new Ruby implementation.

Key Changes:

• Moved deposit-building logic from EthscriptionTransactionBuilder into EthTransaction#build_ethscription_deposits
• Unified transfer construction into a single build_transfer factory method that accepts both single and multiple IDs
• Renamed Solidity function from transferMultipleEthscriptions(bytes32[],address) to transferEthscriptions(address,bytes32[]) with parameter order change (address first)

Reviewed Changes

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

Show a summary per file

File	Description
spec/models/ethscription_transaction_builder_spec.rb	Updated test description to use string instead of class reference
spec/integration/transfer_selector_spec.rb	Added integration tests verifying correct function selector and calldata encoding for single vs multiple transfers
lib/byte_string.rb	Added bytesize method to ByteString class
contracts/test/EthscriptionsMultiTransfer.t.sol	Updated test names and function calls to use renamed transferEthscriptions with new parameter order
contracts/src/Ethscriptions.sol	Renamed transferMultipleEthscriptions to transferEthscriptions, swapped parameter order, changed to storage references
app/models/ethscription_transaction_builder.rb	Refactored to use unified transfer factory based on ID count
app/models/ethscription_transaction.rb	Consolidated factory methods, unified transfer_ids prop, updated function signatures and calldata encoding
app/models/eth_transaction.rb	Moved deposit-building logic from builder class into this model with event and calldata processing

---

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

[Copilot]

The method transfer_multiple_ethscriptions no longer exists. According to the refactor in ethscription_transaction.rb, this should call build_transfer instead, which now handles both single and multiple transfers.

Suggested change

	# Use appropriate factory method based on count
	transaction = if ids.length == 1
	EthscriptionTransaction.transfer_ethscription(
	**base_params,
	ethscription_id: ids.first
	)
	else
	EthscriptionTransaction.transfer_multiple_ethscriptions(
	**base_params,
	ethscription_ids: ids
	)
	end
	# Use unified factory method for single or multiple transfers
	transaction = EthscriptionTransaction.build_transfer(
	**base_params,
	ethscription_ids: ids
	)

[Copilot]

The build_create_ethscription factory method now returns nil when DataUri.valid? fails (line 48 in ethscription_transaction.rb). This will add nil values to @transactions, which could cause issues. Add a guard to only append non-nil transactions: @transactions << transaction if transaction.

Suggested change

	@transactions << transaction
	@transactions << transaction if transaction

[Copilot]

[nitpick] Similar to the creation case, this appends the transaction without checking if it's nil. Since build_transfer always returns a transaction object currently, this may be safe, but for consistency and defensive programming, consider adding a nil check: @transactions << transaction if transaction.

Suggested change

	@transactions << transaction
	@transactions << transaction if transaction

[Copilot]

The build_create_ethscription method can return nil if the content_uri is invalid. This will add nil to @transactions. Add a guard: @transactions << transaction if transaction.

Suggested change

	@transactions << transaction
	@transactions << transaction if transaction

[Copilot]

Changed from memory to storage for a single field access (ethscriptionNumber). Since only one field is read, using memory would be more gas-efficient as it avoids the storage pointer overhead. Consider reverting to memory unless multiple fields are accessed.

Suggested change

	Ethscription storage etsc = ethscriptions[transactionHash];
	uint256 tokenId = etsc.ethscriptionNumber;
	uint256 tokenId = ethscriptions[transactionHash].ethscriptionNumber;

[Copilot]

Changed from memory to storage but only ethscriptionNumber field is accessed. Using memory would be more gas-efficient for single field reads. Consider reverting to memory unless multiple fields need to be accessed.

Suggested change

	Ethscription storage etsc = ethscriptions[transactionHashes[i]];
	Ethscription memory etsc = ethscriptions[transactionHashes[i]];

[Copilot]

Pull Request Overview

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

---

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

[Copilot]

[nitpick] The private keyword is placed after the public method build_ethscription_deposits, but several other public methods (transaction_hash, is_success?, ethscription_source_hash) are defined before the private section. Consider moving all public methods before the private keyword to maintain consistent visibility organization.

[Copilot]

Instance variable @transactions is initialized in build_ethscription_deposits but could cause issues if the same EthTransaction instance is used concurrently or if the method is called multiple times. Consider using a local variable instead or documenting this constraint.

Suggested change

	@transactions = []
	# 1. Process calldata (try as creation, then as transfer)
	process_calldata
	# 2. Process events (creations and transfers)
	process_events
	@transactions.compact
	end
	private
	def process_calldata
	return unless to_address.present?
	try_calldata_creation
	try_calldata_transfer
	transactions = []
	# 1. Process calldata (try as creation, then as transfer)
	process_calldata(transactions)
	# 2. Process events (creations and transfers)
	process_events(transactions)
	transactions.compact
	end
	private
	def process_calldata(transactions)
	# ... (implementation not shown, but replace @transactions with transactions)
	end
	return unless to_address.present?
	try_calldata_creation(transactions)
	try_calldata_transfer(transactions)