Fluffy: eth_createAccessList fixes and improvements (#3247)

bhartnett · web-flow · commit 91850aa8d346 · 2025-04-29T21:21:43.000+08:00
diff --git a/fluffy/evm/async_evm.nim b/fluffy/evm/async_evm.nim
@@ -8,7 +8,7 @@
 {.push raises: [].}
 
 import
-  std/sets,
+  std/[sets, algorithm],
   stew/byteutils,
   chronos,
   chronicles,
@@ -141,10 +141,7 @@ template toCallResult(evmResult: EvmResult[CallResult]): Result[CallResult, stri
         "EVM execution failed: " & $e.code
     )
 
-  if callResult.error.len() > 0:
-    err("EVM execution failed: " & callResult.error)
-  else:
-    ok(callResult)
+  ok(callResult)
 
 proc callFetchingState(
     evm: AsyncEvm,
@@ -277,8 +274,7 @@ proc callFetchingState(
         vmState.ledger.setCode(q.address, code)
         fetchedCode.incl(q.address)
     except CatchableError as e:
-      # TODO: why do the above futures throw a CatchableError and not CancelledError?
-      raiseAssert(e.msg)
+      raise newException(CancelledError, e.msg)
 
   evmResult.toCallResult()
 
@@ -335,12 +331,19 @@ proc call*(
   defer:
     txFrame.dispose() # always dispose state changes
 
-  let vmState = evm.setupVmState(txFrame, header)
-  await evm.callFetchingState(vmState, header, tx, optimisticStateFetch)
+  let
+    vmState = evm.setupVmState(txFrame, header)
+    callResult =
+      ?(await evm.callFetchingState(vmState, header, tx, optimisticStateFetch))
+
+  if callResult.error.len() > 0:
+    err("EVM execution failed: " & callResult.error)
+  else:
+    ok(callResult)
 
 proc createAccessList*(
     evm: AsyncEvm, header: Header, tx: TransactionArgs, optimisticStateFetch = true
-): Future[Result[(transactions.AccessList, GasInt), string]] {.
+): Future[Result[(transactions.AccessList, Opt[string], GasInt), string]] {.
     async: (raises: [CancelledError])
 .} =
   let
@@ -375,11 +378,27 @@ proc createAccessList*(
   txWithAl.accessList = Opt.some(al.getAccessList())
     # converts to transactions.AccessList
 
-  let finalCallResult = ?evm.call(vmState, header, txWithAl)
+  let
+    finalCallResult = ?evm.call(vmState, header, txWithAl)
+    error =
+      if finalCallResult.error.len() > 0:
+        Opt.some(finalCallResult.error)
+      else:
+        Opt.none(string)
+
+  # Sort the access list
+  var accessList = txWithAl.accessList.get(@[])
+  for a in accessList.mitems():
+    a.storageKeys.sort(
+      proc(x, y: Bytes32): int =
+        cmp(x.data, y.data)
+    )
+  accessList.sort(
+    proc(x, y: AccessPair): int =
+      cmp(x.address.data, y.address.data)
+  )
 
-  # TODO: Do we need to use the estimate gas calculation here to get a more acturate
-  # estimation of the gas requirement?
-  ok((txWithAl.accessList.get(@[]), finalCallResult.gasUsed))
+  ok((accessList, error, finalCallResult.gasUsed))
 
 proc estimateGas*(
     evm: AsyncEvm, header: Header, tx: TransactionArgs, optimisticStateFetch = true
diff --git a/fluffy/rpc/rpc_eth_api.nim b/fluffy/rpc/rpc_eth_api.nim
@@ -486,7 +486,7 @@ proc installEthApiHandlers*(
     ## quantityTag: integer block number, or the string "latest", "earliest" or "pending",
     ##   see the default block parameter.
     ## Returns: the access list object which contains the addresses and storage keys which
-    ##   are read by the transaction.
+    ##   are read and written by the transaction.
 
     if tx.to.isNone():
       raise newException(ValueError, "to address is required")
@@ -502,12 +502,13 @@ proc installEthApiHandlers*(
       optimisticStateFetch = optimisticStateFetch.valueOr:
         true
 
-    let (accessList, gasUsed) = (
+    let (accessList, error, gasUsed) = (
       await evm.createAccessList(header, tx, optimisticStateFetch)
     ).valueOr:
       raise newException(ValueError, error)
 
-    return AccessListResult(accessList: accessList, gasUsed: gasUsed.Quantity)
+    return
+      AccessListResult(accessList: accessList, error: error, gasUsed: gasUsed.Quantity)
 
   rpcServer.rpc("eth_estimateGas") do(
     tx: TransactionArgs, quantityTag: RtBlockIdentifier, optimisticStateFetch: Opt[bool]
diff --git a/fluffy/tests/evm/test_async_evm.nim b/fluffy/tests/evm/test_async_evm.nim
@@ -186,13 +186,14 @@ suite "Async EVM":
           input: Opt.some(testCase.txArgs.input.hexToSeqByte()),
         )
 
-      let (accessList, gasUsed) = (
+      let (accessList, error, gasUsed) = (
         waitFor evm.createAccessList(header, tx, optimisticStateFetch = true)
       ).expect("success")
 
       check:
         accessList.len() == testCase.expected.accessList.len()
         gasUsed > 0
+        error.isNone()
 
       for accessPair in accessList:
         let
@@ -220,13 +221,14 @@ suite "Async EVM":
           input: Opt.some(testCase.txArgs.input.hexToSeqByte()),
         )
 
-      let (accessList, gasUsed) = (
+      let (accessList, error, gasUsed) = (
         waitFor evm.createAccessList(header, tx, optimisticStateFetch = false)
       ).expect("success")
 
       check:
         accessList.len() == testCase.expected.accessList.len()
         gasUsed > 0
+        error.isNone()
 
       for accessPair in accessList:
         let
