fix: don't alter metadata state during eth_estimateGas (#1732)

src/chains/ethereum/ethereum/src/helpers/gas-estimator.ts

@@ -82,7 +82,9 @@ const binSearch = async (generateVM, runArgs, result, callback) => {
   const isEnoughGas = async gas => {
     const vm = generateVM(); // Generate fresh VM
     runArgs.tx.gasLimit = new BN(gas.toArrayLike(Buffer));
+    await vm.stateManager.checkpoint();
     const result = await vm.runTx(runArgs).catch(vmerr => ({ vmerr }));
+    await vm.stateManager.revert();
     return !result.vmerr && !result.execResult.exceptionError;
   };
 
@@ -245,8 +247,9 @@ const exactimate = async (vm, runArgs, callback) => {
     const gas = context.getCost();
     return gas.cost.add(gas.sixtyFloorths);
   };
-
+  await vm.stateManager.checkpoint();
   const result = await vm.runTx(runArgs).catch(vmerr => ({ vmerr }));
+  await vm.stateManager.revert();
   const vmerr = result.vmerr;
   if (vmerr) {
     return callback(vmerr);

src/chains/ethereum/ethereum/tests/forking/forking.test.ts

@@ -504,25 +504,27 @@ describe("forking", function () {
     }
 
     function set(provider: EthereumProvider, key: number, value: number) {
-      const encodedKey = Quantity.from(key)
-        .toBuffer()
-        .toString("hex")
-        .padStart(64, "0");
-      const encodedValue = Quantity.from(value)
-        .toBuffer()
-        .toString("hex")
-        .padStart(64, "0");
-
-      return provider.send("eth_sendTransaction", [
-        {
-          from: remoteAccounts[0],
-          to: contractAddress,
-          data: `0x${
-            methods[`setValueFor(uint8,uint256)`]
-          }${encodedKey}${encodedValue}`,
-          gas: `0x${(3141592).toString(16)}`
-        }
-      ]);
+      const tx = makeTxForSet(key, value) as any;
+      tx.gas = `0x${(3141592).toString(16)}`;
+
+      return provider.send("eth_sendTransaction", [tx]);
+    }
+
+    function encodeValue(val: number) {
+      return Quantity.from(val).toBuffer().toString("hex").padStart(64, "0");
+    }
+
+    function makeTxForSet(key: number, value: number) {
+      const encodedKey = encodeValue(key);
+      const encodedValue = encodeValue(value);
+
+      return {
+        from: remoteAccounts[0],
+        to: contractAddress,
+        data: `0x${
+          methods[`setValueFor(uint8,uint256)`]
+        }${encodedKey}${encodedValue}`
+      };
     }
 
     async function getBlockNumber(provider: EthereumProvider) {
@@ -930,6 +932,43 @@ describe("forking", function () {
         }
       }
     });
+
+    describe("gas estimation", () => {
+      it("should not affect live state", async () => {
+        const { localProvider } = await startLocalChain(PORT, {
+          disableCache: true
+        });
+        const blockNum = await getBlockNumber(localProvider);
+
+        // calling eth_estimateGas shouldn't change actual state, which is `2`
+        const expectedValue = 2;
+        const testValue = 0;
+        assert.strictEqual(
+          testValue,
+          0,
+          "the test value must be 0 in order to make sure the change doesn't get stuck in the delete cache"
+        );
+        const actualValueBefore = parseInt(
+          await get(localProvider, "value1", blockNum)
+        );
+        assert.strictEqual(actualValueBefore, expectedValue);
+
+        // make a tx that sets a value 1 to 0, we'll only use this for gas
+        // estimation
+        const tx = makeTxForSet(1, testValue);
+        const est = await localProvider.request({
+          method: "eth_estimateGas",
+          params: [tx]
+        });
+        assert.notStrictEqual(est, "0x");
+
+        // make sure the call to eth_estimateGas didn't change anything!
+        const actualValueAfter = parseInt(
+          await get(localProvider, "value1", blockNum)
+        );
+        assert.strictEqual(actualValueAfter, expectedValue);
+      });
+    });
   });
 
   describe("blocks", () => {