rpc: Return error on tx revert in SimulateTransactions

[Exca-DK]

The third time is a charm 😅

closes #1108

[codecov]

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (e1125b6) 72.76% compared to head (3d6aaad) 74.21%.

❗ Current head 3d6aaad differs from pull request most recent head 4a98a21. Consider uploading reports for the commit 4a98a21 to get more accurate results

Files	Patch %	Lines
rpc/handlers.go	0.00%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1380      +/-   ##
==========================================
+ Coverage   72.76%   74.21%   +1.45%     
==========================================
  Files          96       91       -5     
  Lines        9943     9316     -627     
==========================================
- Hits         7235     6914     -321     
+ Misses       2167     1859     -308     
- Partials      541      543       +2     

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

[omerfirmak]

Sorry to break it to you but this behaviour is no longer needed, it was decided upon to just return the trace for a reverted transaction.

[omerfirmak]

Looks like I talked too soon :)

[omerfirmak]

Sorry, but things have changes since you implemented this, so I have to ask for another rewrite :/
We currently associate errors with transaction indexes, so that caller knows which txn failed in cairoVmExecute(). We can use this existing infrastructure to implement the same things for reverted transactions as well.

juno/vm/vm.go

Lines 76 to 80 in 3248d59

	func JunoReportError(readerHandle C.uintptr_t, txnIndex C.long, str *C.char) {
	context := unwrapContext(readerHandle)
	context.errTxnIndex = int64(txnIndex)
	context.err = C.GoString(str)
	}

Let's add a new flag to cairoVmExecute() that makes it treat reverted txns as errors. It could be called errorOnRevert. So instead of reporting it separately as a reverted txn like you do here, we will report it like a regular error similar to how we do it here and return early from the cairoVmExecute().

errorOnRevert should be set to true only if we are running a txn for fee estimation purposes. Otherwise it should be set to false.

[omerfirmak]

Had to go ahead and implement it myself due to time constraints.
Please feel free to take a look and comment #1518

[Exca-DK]

No worries. Few hours time-constraint will usually end up in such scenarios since I am working on it in my spare time 😄