[Flow EVM] prepare the StateDB and the Emulator to support batch run operations #5577

ramtinms · 2024-03-22T23:28:28Z

This PR

is part 1 of the changes needed for [Flow EVM] supporting evm.batchRun #5501
changes the stateDB
- adds the Reset method to clean up the transient part of the storage (logs, transient slots, ... ), this method can be used between transactions.
- separate Finalize part of the Commit method and add it as a separate method. Finalize, commit the changes from the Atree cache into the final ledger state. This has been added so when executing multiple transactions, we can save time on the serialization time of Atree.
updates the way we return errors from the emulator and handle them on the handler
- validation errors are also now part of the result and won't be returned by methods like RunTransaction.
- now any returned error from the emulator requires reverting the flow transaction (is a non-fatal but stopping error)
- and validation errors are handled by the handler
- this allows the returning of multiple results for a batch of transactions and only returns an error when we hit one that requires reverting the flow transaction (e.g. running out of computation).

Given that ValidationErrors are explicitly captured now, we don't need a wrapper.

…tor level

codecov-commenter · 2024-03-22T23:38:35Z

Codecov Report

Attention: Patch coverage is 56.29139% with 66 lines in your changes are missing coverage. Please review.

Project coverage is 55.65%. Comparing base (51a3fb6) to head (2a3c855).

Files	Patch %	Lines
fvm/evm/handler/handler.go	57.44%	33 Missing and 7 partials ⚠️
fvm/evm/types/result.go	13.33%	12 Missing and 1 partial ⚠️
fvm/evm/emulator/emulator.go	69.69%	7 Missing and 3 partials ⚠️
fvm/evm/emulator/state/stateDB.go	66.66%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #5577      +/-   ##
==========================================
- Coverage   55.67%   55.65%   -0.02%     
==========================================
  Files        1037     1037              
  Lines      101404   101377      -27     
==========================================
- Hits        56457    56423      -34     
- Misses      40607    40615       +8     
+ Partials     4340     4339       -1

Flag	Coverage Δ
unittests	`55.65% <56.29%> (-0.02%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

ramtinms · 2024-03-25T19:38:44Z

fvm/evm/types/codeFinder.go

-	// if is a EVM validation error first unwrap one level
-	if IsEVMValidationError(err) {
-		// check local errors
-		switch err {
-		case ErrInvalidBalance:
-			return ValidationErrCodeInvalidBalance
-		case ErrInsufficientComputation:
-			return ValidationErrCodeInsufficientComputation
-		case ErrUnAuthroizedMethodCall:
-			return ValidationErrCodeUnAuthroizedMethodCall
-		case ErrWithdrawBalanceRounding:
-			return ValidationErrCodeWithdrawBalanceRounding
-		}
-		err = errors.Unwrap(err)
-	}
-


These are not part of validation errors anymore, these all requires the flow transaction to revert

ramtinms · 2024-03-25T19:39:09Z

fvm/evm/types/errors.go

-	// invalid balance is provided (e.g. negative value)
-	ValidationErrCodeInvalidBalance ErrorCode = 101
-	// insufficient computation is left in the flow transaction
-	ValidationErrCodeInsufficientComputation ErrorCode = 102
-	// unauthroized method call
-	ValidationErrCodeUnAuthroizedMethodCall ErrorCode = 103
-	// withdraw balance is prone to rounding error
-	ValidationErrCodeWithdrawBalanceRounding ErrorCode = 104


Not part of the validation errors and the codes are not used.

sideninja · 2024-03-27T18:15:18Z

fvm/evm/emulator/emulator.go

@@ -244,10 +268,15 @@ func (proc *procedure) withdrawFrom(
 		return res, err
 	}

+	// if any error (invalid or vm) on the internal call, revert and don't commit any change
+	if res.Invalid() || res.Failed() {
+		return res, types.ErrInternalDirecCallFailed


is it intentional to swallow the exact error? should we wrap it?

It was intentional to stop the flow transaction, but now I think we might be able to send the result upward and do everything on the handler.

I did more looking in it, the reason I think it might be a better option to make an intentional error for now is the direct call is also invisible to the user (we form it). we might revisit this in the future.

fvm/evm/emulator/emulator.go

sideninja · 2024-03-27T18:20:16Z

fvm/evm/types/emulator.go

+// Any error returned by any of the methods (e.g. stateDB errors) if non-fatal stops the outer flow transaction
+// if fatal stops the node.
+// EVM validation errors and EVM execution errors are part of the returned result
+// and should be handled separately.


much better design ❤️

sideninja · 2024-03-27T18:26:32Z

fvm/evm/types/result.go

@@ -49,6 +56,9 @@ func (res *Result) VMErrorString() string {
 // and for each log, Address, Topics, Data (consensus fields)
 // During execution we also do fill in BlockNumber, TxIndex, Index (event index)
 func (res *Result) Receipt() *gethTypes.Receipt {
+	if res.Invalid() {


I wonder if not having a receipt for invalid transactions is correct since they do consume gas?

Thats an EVM-expected behaviour that we can't change easily. For example, in the case of a nonce mismatch (ahead of state), there should never be a state change or receipt otherwise the 3rd party apps will break after resubmission of the transaction when the nonce becomes the right number.

sideninja

Nice!

janezpodhostnik

LGTM 🚀

janezpodhostnik · 2024-03-27T20:11:23Z

fvm/evm/types/result.go

+// given we charge the fees on flow transaction and we
+// are doing on chain validation we can/should charge the
+// user for the validation fee.
+const InvalidTransactionGasCost = 1_000


This is an ok solution, but I think it might be better if we introduce a new computationKind for failed transactions that can then be dynamically calibrated.

Good idea, I'll add an issue for it.

ramtinms added 5 commits March 22, 2024 11:18

update statedb with reset and finalize

82bc147

update results to include validation error and handle errors on emula…

d121cfe

…tor level

fix ret error

aec93fa

update handler to deal with validation errors

0aaa755

update emulator test

bd15ac1

ramtinms added 5 commits March 25, 2024 11:48

improve emulator error handling

9c1b0f7

improve error handling and safety

146b3a8

improve naming

10fb7e3

no need to wrap validation errors anymore

616cca8

remove unnecessary error codes

53b6c82

ramtinms marked this pull request as ready for review March 25, 2024 19:37

ramtinms requested a review from janezpodhostnik as a code owner March 25, 2024 19:37

ramtinms requested a review from sideninja March 25, 2024 19:37

ramtinms commented Mar 25, 2024

View reviewed changes

add a saftey check on empty result

636c182

ramtinms changed the title ~~[Flow EVM] prepare StateDB and Emulator to support batch run operations~~ [Flow EVM] prepare the StateDB and the Emulator to support batch run operations Mar 25, 2024

sideninja reviewed Mar 27, 2024

View reviewed changes

fvm/evm/emulator/emulator.go Outdated Show resolved Hide resolved

sideninja reviewed Mar 27, 2024

View reviewed changes

sideninja approved these changes Mar 27, 2024

View reviewed changes

apply pr feedback

9fa6ebc

janezpodhostnik approved these changes Mar 27, 2024

View reviewed changes

ramtinms and others added 2 commits March 27, 2024 15:58

typo

43daee7

Merge branch 'master' into ramtin/5501-batch-run-part1

2a3c855

ramtinms enabled auto-merge March 27, 2024 23:05

ramtinms added this pull request to the merge queue Mar 28, 2024

Merged via the queue into master with commit 5922cda Mar 28, 2024
55 checks passed

ramtinms deleted the ramtin/5501-batch-run-part1 branch March 28, 2024 00:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Flow EVM] prepare the StateDB and the Emulator to support batch run operations #5577

[Flow EVM] prepare the StateDB and the Emulator to support batch run operations #5577

ramtinms commented Mar 22, 2024 •

edited

Loading

codecov-commenter commented Mar 22, 2024 •

edited

Loading

ramtinms Mar 25, 2024

ramtinms Mar 25, 2024

sideninja Mar 27, 2024

ramtinms Mar 27, 2024

ramtinms Mar 27, 2024

sideninja Mar 27, 2024

sideninja Mar 27, 2024

ramtinms Mar 27, 2024

sideninja left a comment

janezpodhostnik left a comment

janezpodhostnik Mar 27, 2024

ramtinms Mar 27, 2024

[Flow EVM] prepare the StateDB and the Emulator to support batch run operations #5577

[Flow EVM] prepare the StateDB and the Emulator to support batch run operations #5577

Conversation

ramtinms commented Mar 22, 2024 • edited Loading

codecov-commenter commented Mar 22, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sideninja left a comment

Choose a reason for hiding this comment

janezpodhostnik left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ramtinms commented Mar 22, 2024 •

edited

Loading

codecov-commenter commented Mar 22, 2024 •

edited

Loading