Skip to content
This repository has been archived by the owner on Jul 9, 2021. It is now read-only.

Sol tracing fixes #1498

Merged
merged 36 commits into from
Jan 15, 2019
Merged

Sol tracing fixes #1498

merged 36 commits into from
Jan 15, 2019

Conversation

recmo
Copy link
Contributor

@recmo recmo commented Jan 9, 2019

Description

This PR fixes some of the issues in trace-based tools that we've discovered.

  1. We now use custom JS tracer on nodes that support it (Geth only)
  2. Before we assumed that source file ids are in order. That's not true and led to incorrect source map parsing. We now fixed that, but it required us to change the format of the ContractData and it's used by custom artifact adapters, so that's a breaking change.
  3. Added support for assembly

Testing instructions

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • Prefix PR title with [WIP] if necessary.
  • Add tests to cover changes as needed.
  • Update documentation as needed.
  • Add new entries to the relevant CHANGELOG.jsons.

@coveralls
Copy link

coveralls commented Jan 11, 2019

Coverage Status

Coverage decreased (-0.06%) to 85.389% when pulling a662901 on fix/sol-cov into 797d7c7 on development.

@LogvinovLeon LogvinovLeon changed the title [WIP] Fix/sol cov Sol tracing fixes Jan 14, 2019
packages/sol-tracing-utils/CHANGELOG.json Outdated Show resolved Hide resolved
packages/sol-tracing-utils/CHANGELOG.json Outdated Show resolved Hide resolved
packages/sol-tracing-utils/CHANGELOG.json Outdated Show resolved Hide resolved
packages/sol-tracing-utils/src/source_maps.ts Outdated Show resolved Hide resolved
packages/sol-tracing-utils/src/source_maps.ts Outdated Show resolved Hide resolved
packages/sol-tracing-utils/src/source_maps.ts Outdated Show resolved Hide resolved
packages/sol-tracing-utils/src/trace_info_subprovider.ts Outdated Show resolved Hide resolved
}
}
return wrappedAsync;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This hack is going in? I was hoping we would come up with something better :)

Copy link
Contributor

Choose a reason for hiding this comment

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

You're absolutely right. I've figured out how to fix it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oops, I was wrong and I from the past was right. Not about the fact that we shouldn't handle this but about the fact that there is no elegant way to do so.

@LogvinovLeon LogvinovLeon merged commit 1808458 into development Jan 15, 2019
@LogvinovLeon LogvinovLeon deleted the fix/sol-cov branch January 15, 2019 13:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants