[Flow EVM] Optimize EVMEncodeABI by removing an ArrayValue iteration

[fxamacker]

Updates #8401

Currently, EVMEncodeABI iterates valuesArray (ArrayValue) twice. The first iteration is used to report ABI computation, and the second iteration is used to encode values.

This optimization combines ABI computation reporting and value encoding into one ArrayValue iteration.

Details

The optimization checks limits before each computation inside the loop. So the loop starts and it will exit early to prevent limits being exceeded during the loop.

• The happy path (entire loop is within limits) is optimized by looping once instead of twice.

• The unhappy path (entire loop doesn't fit within limits) is potentially less efficient because it is detected during the loop. So computations that are within limits are allowed during the loop, and the loop is exited early if the next computation won't fit within limits.

Caveats

The current code (without this PR) is not metering empty valuesArray, so that aspect is unaffected by this PR.

Elements are metered regardless of type and value, so an element that is an empty valuesArray would still be metered. This aspect is also unaffected by this PR.

Context

I found this quick hit, while looking for other (larger) cross-vm optimizations.

## Summary by CodeRabbit

* Refactor
* Enhanced computation intensity reporting for encoding operations to provide more granular, per-element metrics, enabling more detailed performance tracking and visibility.

EDIT: added Details and added Caveats.

[github-actions]

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

[coderabbitai]

📝 Walkthrough

Walkthrough

Modified ABI encoding computation reporting in fvm/evm/impl/abi.go to track intensity per individual array element instead of as a batch operation. The underlying iteration and encoding logic remain unchanged; only the granularity of how computational intensity is reported during ABI encoding has been adjusted.

Changes

Cohort / File(s)	Summary
ABI Encoding Computation Reporting fvm/evm/impl/abi.go	Shifted from batch-level reporting via reportArrayABIEncodingComputation to per-element reporting within the loop using reportABIEncodingComputation, triggering separate ComputationKindEVMEncodeABI intensity reports for each array element during encoding.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

• [Flow EVM] Implement ABI encoding/decoding for arrays of Solidity tuples #8371: Modifies array element handling within the same fvm/evm/impl/abi.go file, affecting tuple handling and type resolution alongside related array encoding logic.

Suggested labels

Improvement

Suggested reviewers

• turbolent
• zhangchiqing
• janezpodhostnik

Poem

🐰 Per-element, element-by-element, oh what a sight!
Array encoding now reports with granular might,
Batch no more, each value gets its due,
Computation intensity shines bright and true! ✨

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main optimization: removing an ArrayValue iteration in EVMEncodeABI, which matches the core change documented in the PR objectives.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fxamacker/remove-one-array-iteration-in-internal-evm-EncodeABI

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[codecov-commenter]

Codecov Report

❌ Patch coverage is 0% with 13 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
fvm/evm/impl/abi.go	0.00%	13 Missing ⚠️

📢 Thoughts on this report? Let us know!

[zhangchiqing]

👍

[turbolent]

This is potentially dangerous: We should always estimate and meter computation before performing computation

[fxamacker]

Hi @turbolent

This is potentially dangerous: We should always estimate and meter computation before performing computation

The optimization meters elements during the loop before each computation, which allows the "within-limits" scenario to iterate the loop just once (instead of twice).

Unfortunately, it sacrifices the "exceeds-limits" scenario because the loop is allowed to continue until metering stops it (since it doesn't know upfront the loop doesn't need to start).

So this optimization speeds up the "within-limits" scenario at the cost of the "exceeds-limits" scenario being less efficient.

I can close this PR if the tradeoff isn't worthwhile or risks cannot be mitigated.

Thoughts?

[bluesign]

I think the issue here is: empty array is encoded for free.

computation := uint64(2 * abiEncodingByteSize)
common.UseComputation(
  context,
  common.ComputationUsage{
	  Kind:      environment.ComputationKindEVMEncodeABI,
	  Intensity: computation,
  },
)

before Iterate should make it similar.

[fxamacker]

Hi @bluesign

I think the issue here is: empty array is encoded for free.

Are you referring to empty valuesArray not getting metered?

The current code (without this PR) is not metering empty valuesArray, so that aspect is unaffected by this PR.

Elements are metered regardless of type and value, so an element that is an empty array would still be metered. This aspect is also not affected by this PR.

BTW, I'm new to the metering-related codebase (onboarding and looking for existing tests/benchmarks). Please let me know if I miss anything.

[bluesign]

@fxamacker ah yeah you are right, it is almost identical. I thought it is calling previously reportABIEncodingComputation but it is actually reportArrayABIEncodingComputation

[janezpodhostnik]

In this case in the "exceeds-limits" scenario the change is minimal. Imagine the following case user has 100 computation left in transaction. and needs 200 computation for whole operation, where each loop of the operation takes 10 computation.

current logic:

• add 200 to computation. computation is now computation_limit + 100
• fail transaction before doing any further work
• charge fees. which can only happen up to: computation_limit
• so technically we charged the user for 100 computation units we never performed the computation for, because we knew that the operation would fail.
  new logic
• perform the loop 10 times. computation is now at computation_limit
• fail transaction
• charge fees at computation_limit. Same as before
• we performed the computation partially up to the users computation_limit and the user was charged for exactly that computation.

in the new logic we are performing some work we technically wouldn't need to, but that work is charged to the user, so that is fine. I think we should optimize for the happy path (which this does), but make sure that any work done is being charged as fees to the user (which it is).

[m-Peter]

LGTM!