Skip to content
This repository has been archived by the owner on Feb 26, 2024. It is now read-only.

fix: add debug_traceTransaction RPC method #614

Merged
merged 15 commits into from
Nov 20, 2020
Merged

fix: add debug_traceTransaction RPC method #614

merged 15 commits into from
Nov 20, 2020

Conversation

tcoulter
Copy link
Contributor

@tcoulter tcoulter commented Aug 20, 2020

Two important notes:

  • The port does not include code from the original version meant for forking. Those changes will need to be added when forking is added to ts.
  • There were a few tests in the old version that appeared to do the same thing multiple times, like setting a value. It didn't appear as though each repetition was a novel test, so I instead opted to merge what I thought was novel into a single test. Happy to add things back if the test is more nuanced than it appears.

Note that I copied over many of the original comments to this version. I also added some comments of my own.

@tcoulter
Copy link
Contributor Author

Ah, TODO before merging: I need to test what happens on a revert and see if that's handled correctly. Don't merge yet - I'll add that.

Copy link
Contributor

@gnidan gnidan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's a few comments

src/chains/ethereum/__tests__/api/debug/debug.test.ts Outdated Show resolved Hide resolved
src/chains/ethereum/__tests__/api/debug/debug.test.ts Outdated Show resolved Hide resolved
src/chains/ethereum/src/blockchain.ts Outdated Show resolved Hide resolved
@tcoulter
Copy link
Contributor Author

This is merge'able now! \o/

The biggest changes came from the memory optimization work, and the TraceData and TraceStorageMap objects. These objects help use objects to store data used to create the trace, rather than copying thousands of strings around. In our tests which runs a loop in Solidity 100 times, these optimizations were able to reduce the number of objects used to create the trace by 47% (22844 objects vs the original count of 43693).

This needs some testing against more intense transactions, but so far basic testing with truffle-debugger is A-Ok. Would love more feedback if any!

@tcoulter tcoulter changed the title Direct port of debug_traceTransaction into TypeScript Not-a-direct-port-anymore of debug_traceTransaction into TypeScript Aug 25, 2020
Base automatically changed from ts to next November 16, 2020 21:30
@davidmurdoch davidmurdoch changed the base branch from next to ts-docs November 16, 2020 23:29
@davidmurdoch davidmurdoch changed the base branch from ts-docs to next November 16, 2020 23:30
@davidmurdoch davidmurdoch changed the base branch from next to ts-trace-2 November 16, 2020 23:47
@davidmurdoch davidmurdoch changed the base branch from ts-trace-2 to next November 16, 2020 23:47
1. Clean up errors caught by compled solc. Bad solc.
2. Add a debug contract that can create a lot of instructions in a loop
3. Add a test that tries to check memory allocation of the trace function
Optimize debug_traceTransaction for memory usage by using objects to track data within the trace. Most of the optimization occurs in the TraceData class itself, where values are memoized and only create a new TraceData object if one isn't created for that value yet. In the associated test, this reduces the number of objects used to create the trace by 47%.
@davidmurdoch davidmurdoch changed the title Not-a-direct-port-anymore of debug_traceTransaction into TypeScript fix: add debug_traceTransaction RPC method Nov 20, 2020
@davidmurdoch davidmurdoch merged commit ff65d1f into next Nov 20, 2020
@davidmurdoch davidmurdoch deleted the ts-trace branch November 20, 2020 20:13
davidmurdoch added a commit that referenced this pull request Jan 12, 2021
davidmurdoch added a commit that referenced this pull request Jan 19, 2021
sambacha pushed a commit to contractshark/ganache-core that referenced this pull request Apr 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants