From 7043174d6215ae9c8d08ed09cd73171b8589d150 Mon Sep 17 00:00:00 2001 From: yann300 Date: Thu, 13 Feb 2025 13:06:18 +0100 Subject: [PATCH] fix revert --- libs/remix-debug/src/trace/traceAnalyser.ts | 1 - libs/remix-debug/src/trace/traceCache.ts | 17 ++- .../contracts/revert-state-sub-call.ts | 39 +++++ .../test/decoder/contracts/revert-state.ts | 20 +++ .../stateTests/revert-state-sub-call.ts | 138 ++++++++++++++++++ .../test/decoder/stateTests/revert-state.ts | 103 +++++++++++++ .../test/decoder/storageDecoder.ts | 11 +- 7 files changed, 322 insertions(+), 7 deletions(-) create mode 100644 libs/remix-debug/test/decoder/contracts/revert-state-sub-call.ts create mode 100644 libs/remix-debug/test/decoder/contracts/revert-state.ts create mode 100644 libs/remix-debug/test/decoder/stateTests/revert-state-sub-call.ts create mode 100644 libs/remix-debug/test/decoder/stateTests/revert-state.ts diff --git a/libs/remix-debug/src/trace/traceAnalyser.ts b/libs/remix-debug/src/trace/traceAnalyser.ts index 24491c9d77..07d6fbbafc 100644 --- a/libs/remix-debug/src/trace/traceAnalyser.ts +++ b/libs/remix-debug/src/trace/traceAnalyser.ts @@ -113,7 +113,6 @@ export class TraceAnalyser { this.traceCache.pushStoreChanges(index + 1, context.storageContext[context.storageContext.length - 1]) } else if (traceHelper.isRevertInstruction(step)) { context.storageContext.pop() - this.traceCache.resetStoreChanges() } return context } diff --git a/libs/remix-debug/src/trace/traceCache.ts b/libs/remix-debug/src/trace/traceCache.ts index d43d48cc70..8fc0c26911 100644 --- a/libs/remix-debug/src/trace/traceCache.ts +++ b/libs/remix-debug/src/trace/traceCache.ts @@ -125,7 +125,8 @@ export class TraceCache { address: address, key: key, value: value, - hashedKey: key && sha3_256(key) + hashedKey: key && sha3_256(key), + contextCall: this.currentCall } this.storageChanges.push(index) } @@ -138,10 +139,16 @@ export class TraceCache { return ret } const sstore = this.sstore[changesIndex] - if (sstore.address === address && sstore.key) { - ret[sstore.hashedKey] = { - key: sstore.key, - value: sstore.value + if (sstore.address?.toLowerCase() === address?.toLowerCase() && sstore.key) { + if (sstore.contextCall?.call?.return < index && sstore.contextCall?.call?.reverted) { + // this is a previous call which has reverted. state changes aren't kept. + console.log('reverted state changes', changesIndex) + continue + } else { + ret[sstore.hashedKey] = { + key: sstore.key, + value: sstore.value + } } } } diff --git a/libs/remix-debug/test/decoder/contracts/revert-state-sub-call.ts b/libs/remix-debug/test/decoder/contracts/revert-state-sub-call.ts new file mode 100644 index 0000000000..f986a7fd48 --- /dev/null +++ b/libs/remix-debug/test/decoder/contracts/revert-state-sub-call.ts @@ -0,0 +1,39 @@ +module.exports = { + contract: ` + + pragma solidity ^0.8; + + contract MyContract { + OtherContract myCall; + constructor () { + myCall = new OtherContract(); + } + function callOther() public { + + // Call 'doSomething' in the other contract + bool result; + (result, ) = address(myCall).call(abi.encodeWithSignature("doSomething()")); + + result; + (result, ) = address(myCall).call(abi.encodeWithSignature("doSomething()")); + } + + function callOther2() public { + // Call 'doSomething' in the other contract + myCall.doSomething(); + myCall.doSomething(); + } + } + + // Assuming 'doSomething' in the other contract's code reverts due to some error... + contract OtherContract { + uint p; + function doSomething() public returns(bool) { + p = 234; + revert("revert"); + } + + function v() public returns (uint) { return p;} + } + ` +} diff --git a/libs/remix-debug/test/decoder/contracts/revert-state.ts b/libs/remix-debug/test/decoder/contracts/revert-state.ts new file mode 100644 index 0000000000..f765a30f78 --- /dev/null +++ b/libs/remix-debug/test/decoder/contracts/revert-state.ts @@ -0,0 +1,20 @@ +module.exports = { + contract: ` +contract MyContract { + uint public data; + + function foo(uint _data) external { + bar(_data); + baz(); + } + + function bar(uint _data) internal { + data = _data; + } + + function baz() internal { + revert("oops...."); + } +} + ` +} diff --git a/libs/remix-debug/test/decoder/stateTests/revert-state-sub-call.ts b/libs/remix-debug/test/decoder/stateTests/revert-state-sub-call.ts new file mode 100644 index 0000000000..9c587ca555 --- /dev/null +++ b/libs/remix-debug/test/decoder/stateTests/revert-state-sub-call.ts @@ -0,0 +1,138 @@ +import { CompilerAbstract } from '@remix-project/remix-solidity' +import { EventManager } from '../../../src/eventManager' +import { compilerInput } from '../../helpers/compilerHelper' +import { TraceManager } from '../../../src/trace/traceManager' +import { CodeManager } from '../../../src/code/codeManager' +import { compile } from 'solc' +import * as stateDecoder from '../../../src/solidity-decoder/stateDecoder' +import { SolidityProxy } from '../../../src/solidity-decoder/solidityProxy' +import { InternalCallTree } from '../../../src/solidity-decoder/internalCallTree' +import * as vmCall from '../../vmCall' +import { StorageResolver } from '../../../src/storage/storageResolver' +import { StorageViewer } from '../../../src/storage/storageViewer' +import { Address, bytesToHex } from '@ethereumjs/util' + +module.exports = async function testMappingStorage (st, cb) { + const revertStateContract = require('../contracts/revert-state-sub-call.ts') + const privateKey = Buffer.from('503f38a9c967ed597e47fe25643985f032b072db8075426a92110f82df48dfcb', 'hex') + let output = compile(compilerInput(revertStateContract.contract)) + output = JSON.parse(output); + const sources = { + target: 'test.sol', + sources: { 'test.sol': { content: revertStateContract.contract } } + } + const compilationResults = new CompilerAbstract('json', output, sources) + const web3 = await (vmCall as any).getWeb3(); + (vmCall as any).sendTx(web3, { nonce: 0, privateKey: privateKey }, undefined, 0, output.contracts['test.sol']['MyContract'].evm.bytecode.object, function (error, hash) { + if (error) { + console.log(error) + st.end(error) + } else { + web3.eth.getTransactionReceipt(hash) + .then(tx => { + // const storage = await this.vm.stateManager.dumpStorage(data.to) + // web3.eth.getCode(tx.contractAddress).then((code) => console.log('code:---', code)) + // (vmCall as any).web3().debug.traceTransaction(hash).then((code) => console.log('trace:', code)) + testRevertStateSubCall(st, privateKey, tx.contractAddress, output, compilationResults, web3, cb) + } + // st.end() + ) + .catch(error => { + st.end(error) + }) + } + }) +} + +function testRevertStateSubCall (st, privateKey, contractAddress, output, compilationResults, web3, cb) { + // call to foo(22) + (vmCall as any).sendTx(web3, { nonce: 1, privateKey: privateKey }, contractAddress, 0, '8e0bf849', + function (error, hash) { + if (error) { + console.log(error) + st.end(error) + } else { + web3.eth.getTransaction(hash) + .then(tx => { + const traceManager = new TraceManager({ web3 }) + const codeManager = new CodeManager(traceManager) + codeManager.clear() + const solidityProxy = new SolidityProxy({ + getCurrentCalledAddressAt: traceManager.getCurrentCalledAddressAt.bind(traceManager), + getCode: codeManager.getCode.bind(codeManager), + compilationResult: () => compilationResults + }) + const debuggerEvent = new EventManager() + const callTree = new InternalCallTree(debuggerEvent, traceManager, solidityProxy, codeManager, { includeLocalVariables: true }) + callTree.event.register('callTreeBuildFailed', (error) => { + st.fail(error) + }) + callTree.event.register('callTreeNotReady', (reason) => { + st.fail(reason) + }) + callTree.event.register('callTreeReady', (scopes, scopeStarts) => { + const storageViewerMyContract = new StorageViewer({ + stepIndex: 29, + tx: tx, + address: contractAddress + }, new StorageResolver({ web3 }), traceManager) + + const stateVarsMyContract = stateDecoder.extractStateVariables('MyContract', output.sources) + stateDecoder.decodeState(stateVarsMyContract, storageViewerMyContract).then((result) => { + const contractAddressOtherContract = result['myCall'].value + const storageViewerOtherContract1 = new StorageViewer({ + stepIndex: 300, + tx: tx, + address: contractAddressOtherContract + }, new StorageResolver({ web3 }), traceManager) + + const storageViewerOtherContract2 = new StorageViewer({ + stepIndex: 550, + tx: tx, + address: contractAddressOtherContract + }, new StorageResolver({ web3 }), traceManager) + + const storageViewerOtherContract3 = new StorageViewer({ + stepIndex: 556, + tx: tx, + address: contractAddressOtherContract + }, new StorageResolver({ web3 }), traceManager) + + const stateVars = stateDecoder.extractStateVariables('OtherContract', output.sources) + stateDecoder.decodeState(stateVars, storageViewerOtherContract1).then((result) => { + // value should be set + st.equal(result['p'].value, '234') + stateDecoder.decodeState(stateVars, storageViewerOtherContract2).then((result) => { + // in the other sub call, the value is reverted + st.equal(result['p'].value, '0') + stateDecoder.decodeState(stateVars, storageViewerOtherContract3).then((result) => { + // and reset back to 234 + st.equal(result['p'].value, '234') + cb() + }, (reason) => { + console.log('fail') + st.end(reason) + }) + }) + }, (reason) => { + console.log('fail') + st.end(reason) + }) + }) + }, (reason) => { + console.log('fail') + st.end(reason) + }) + traceManager.resolveTrace(tx).then(() => { + debuggerEvent.trigger('newTraceLoaded', [traceManager.trace]) + }).catch((error) => { + st.fail(error) + }) + }) + .catch(error => { + console.log(error) + st.end(error) + }) + } + }) +} diff --git a/libs/remix-debug/test/decoder/stateTests/revert-state.ts b/libs/remix-debug/test/decoder/stateTests/revert-state.ts new file mode 100644 index 0000000000..97662bd5bb --- /dev/null +++ b/libs/remix-debug/test/decoder/stateTests/revert-state.ts @@ -0,0 +1,103 @@ +import { CompilerAbstract } from '@remix-project/remix-solidity' +import { EventManager } from '../../../src/eventManager' +import { compilerInput } from '../../helpers/compilerHelper' +import { TraceManager } from '../../../src/trace/traceManager' +import { CodeManager } from '../../../src/code/codeManager' +import { compile } from 'solc' +import * as stateDecoder from '../../../src/solidity-decoder/stateDecoder' +import { SolidityProxy } from '../../../src/solidity-decoder/solidityProxy' +import { InternalCallTree } from '../../../src/solidity-decoder/internalCallTree' +import * as vmCall from '../../vmCall' +import { StorageResolver } from '../../../src/storage/storageResolver' +import { StorageViewer } from '../../../src/storage/storageViewer' +import { Address, bytesToHex } from '@ethereumjs/util' + +module.exports = async function testMappingStorage (st, cb) { + const revertStateContract = require('../contracts/revert-state.ts') + const privateKey = Buffer.from('503f38a9c967ed597e47fe25643985f032b072db8075426a92110f82df48dfcb', 'hex') + let output = compile(compilerInput(revertStateContract.contract)) + output = JSON.parse(output); + const sources = { + target: 'test.sol', + sources: { 'test.sol': { content: revertStateContract.contract } } + } + const compilationResults = new CompilerAbstract('json', output, sources) + const web3 = await (vmCall as any).getWeb3(); + (vmCall as any).sendTx(web3, { nonce: 0, privateKey: privateKey }, undefined, 0, output.contracts['test.sol']['MyContract'].evm.bytecode.object, function (error, hash) { + if (error) { + console.log(error) + st.end(error) + } else { + web3.eth.getTransactionReceipt(hash) + .then(tx => { + // const storage = await this.vm.stateManager.dumpStorage(data.to) + // web3.eth.getCode(tx.contractAddress).then((code) => console.log('code:---', code)) + // (vmCall as any).web3().debug.traceTransaction(hash).then((code) => console.log('trace:', code)) + testRevertState(st, privateKey, tx.contractAddress, output, compilationResults, web3, cb) + } + // st.end() + ) + .catch(error => { + st.end(error) + }) + } + }) +} + +function testRevertState (st, privateKey, contractAddress, output, compilationResults, web3, cb) { + // call to foo(22) + (vmCall as any).sendTx(web3, { nonce: 1, privateKey: privateKey }, contractAddress, 0, '2fbebd380000000000000000000000000000000000000000000000000000000000000016', + function (error, hash) { + if (error) { + console.log(error) + st.end(error) + } else { + web3.eth.getTransaction(hash) + .then(tx => { + const traceManager = new TraceManager({ web3 }) + const codeManager = new CodeManager(traceManager) + codeManager.clear() + console.log(compilationResults) + const solidityProxy = new SolidityProxy({ + getCurrentCalledAddressAt: traceManager.getCurrentCalledAddressAt.bind(traceManager), + getCode: codeManager.getCode.bind(codeManager), + compilationResult: () => compilationResults + }) + const debuggerEvent = new EventManager() + const callTree = new InternalCallTree(debuggerEvent, traceManager, solidityProxy, codeManager, { includeLocalVariables: true }) + callTree.event.register('callTreeBuildFailed', (error) => { + st.fail(error) + }) + callTree.event.register('callTreeNotReady', (reason) => { + st.fail(reason) + }) + callTree.event.register('callTreeReady', (scopes, scopeStarts) => { + const storageViewer = new StorageViewer({ + stepIndex: 120, + tx: tx, + address: contractAddress + }, new StorageResolver({ web3 }), traceManager) + const stateVars = stateDecoder.extractStateVariables('MyContract', output.sources) + stateDecoder.decodeState(stateVars, storageViewer).then((result) => { + // even if the call is reverted, the value still persist during the call exeecution + st.equal(result['data'].value, '22') + cb() + }, (reason) => { + console.log('fail') + st.end(reason) + }) + }) + + traceManager.resolveTrace(tx).then(() => { + debuggerEvent.trigger('newTraceLoaded', [traceManager.trace]) + }).catch((error) => { + st.fail(error) + }) + }) + .catch(error => { + console.log(error) + st.end(error) + }) + } + }) +} diff --git a/libs/remix-debug/test/decoder/storageDecoder.ts b/libs/remix-debug/test/decoder/storageDecoder.ts index 9c1e3b0535..39db8d0735 100644 --- a/libs/remix-debug/test/decoder/storageDecoder.ts +++ b/libs/remix-debug/test/decoder/storageDecoder.ts @@ -5,6 +5,9 @@ import * as stateDecoder from '../../src/solidity-decoder/stateDecoder' import { MockStorageResolver } from './mockStorageResolver' import { compilerInput } from '../helpers/compilerHelper' const testMappingStorage = require('./stateTests/mapping') +const testRevertState = require('./stateTests/revert-state') +const testRevertStateSubCall = require('./stateTests/revert-state-sub-call') + tape('solidity', function (t) { t.test('storage decoder', function (st) { @@ -16,7 +19,13 @@ tape('solidity', function (t) { testStructArrayStorage(st, function () { console.log('test mapping storage') testMappingStorage(st, function () { - st.end() + console.log('test revert state') + testRevertState(st, function () { + console.log('test revert state sub call') + testRevertStateSubCall(st, function () { + st.end() + }) + }) }) }) })