Fix revert reason encoding (#3420)

jangko · web-flow · commit a7d5ff3eac5b · 2025-06-22T21:13:08.000Z
diff --git a/execution_chain/evm/evm_errors.nim b/execution_chain/evm/evm_errors.nim
@@ -14,9 +14,8 @@ import
   std/tables,
   results,
   stint,
-  stew/[assign2, byteutils],
-  eth/common/hashes,
-  web3/encoding
+  stew/byteutils,
+  eth/common/hashes
 
 export
   results
@@ -101,20 +100,42 @@ const panicReasons = {
   0x51: "uninitialized function",
 }.toTable
 
+func decodeU256(input: openArray[byte]): Opt[UInt256] =
+  if input.len < 32:
+    return Opt.none(UInt256)
+  Opt.some(UInt256.fromBytesBE(input.toOpenArray(0, 31)))
+
+func decodeString(input: openArray[byte]): Opt[string] =
+  let
+    offset256 = ?decodeU256(input)
+    offset = offset256.truncate(int)
+
+  if offset >= input.len:
+    return Opt.none(string)
+
+  let
+    len256 = ?decodeU256(input.toOpenArray(offset, input.len-1))
+    len = len256.truncate(int)
+    dataOffset = offset + 32
+
+  if dataOffset + len >= input.len:
+    return Opt.none(string)
+
+  ok(string.fromBytes(input.toOpenArray(dataOffset, dataOffset + len - 1)))
+
 # UnpackRevert resolves the abi-encoded revert reason. According to the solidity
 # spec https://solidity.readthedocs.io/en/latest/control-structures.html#revert,
 # the provided revert reason is abi-encoded as if it were a call to function
 # `Error(string)` or `Panic(uint256)`.
-proc unpackRevertReason*(data: openArray[byte], reason: var string) =
+proc unpackRevertReason*(data: openArray[byte]): Opt[string] =
   if data.len() < 4:
-    reason = ""
-    return
+    return Opt.none(string)
 
   let selector = data[0..3]
 
   if selector == revertSelector:
-    discard decode(data.toOpenArray(4, data.len() - 1), 0, 0, reason)
+    return decodeString(data.toOpenArray(4, data.len() - 1))
   elif selector == panicSelector:
-    var reasonCode: UInt256
-    discard decode(data.toOpenArray(4, data.len() - 1), 0, 0, reasonCode)
-    assign(reason, panicReasons.getOrDefault(reasonCode.truncate(int)))
+    let reasonCode = decodeU256(data.toOpenArray(4, data.len() - 1)).valueOr:
+      return Opt.none(string)
+    return ok(panicReasons.getOrDefault(reasonCode.truncate(int)))
diff --git a/execution_chain/evm/interpreter/op_handlers/oph_sysops.nim b/execution_chain/evm/interpreter/op_handlers/oph_sysops.nim
@@ -64,13 +64,12 @@ proc revertOp(cpt: VmCpt): EvmResultVoid =
 
   cpt.memory.extend(pos, len)
   assign(cpt.output, cpt.memory.read(pos, len))
-
-  var revertReason: string
-  unpackRevertReason(cpt.output, revertReason)
+  
+  let revertReason = unpackRevertReason(cpt.output)
 
   let revertMsg =
-    if revertReason.len() > 0:
-      "execution reverted: " & revertReason
+    if revertReason.isSome:
+      "execution reverted: " & revertReason.value
     else:
       "execution reverted"
 
diff --git a/tests/test_evm_errors.nim b/tests/test_evm_errors.nim
@@ -14,13 +14,23 @@ suite "EVM errors tests":
   test "unpack revert reason data":
     let data = "0x08c379a000000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000018556e69737761705632526f757465723a20455850495245440000000000000000"
 
-    var revertReason: string
-    unpackRevertReason(data.hexToSeqByte(), revertReason)
+    let revertReason = unpackRevertReason(data.hexToSeqByte()).expect("something")
     check revertReason == "UniswapV2Router: EXPIRED"
 
+  test "unpack revert reason data missing bytes":
+    let data = "0x08c379a0000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000018556e69737761705632526f757465723a20455850495245440000000000000000"
+
+    let revertReason = unpackRevertReason(data.hexToSeqByte())
+    check revertReason == Opt.none(string)
+    
   test "unpack panic reason data":
     let data = "0x4e487b710000000000000000000000000000000000000000000000000000000000000032"
 
-    var revertReason: string
-    unpackRevertReason(data.hexToSeqByte(), revertReason)
+    var revertReason = unpackRevertReason(data.hexToSeqByte()).expect("something")
     check revertReason == "out-of-bounds access of an array or bytesN"
+
+  test "unpack panic reason data missing byte":
+    let data = "0x4e487b7100000000000000000000000000000000000000000000000000000000000032"
+
+    var revertReason = unpackRevertReason(data.hexToSeqByte())
+    check revertReason == Opt.none(string)
