Skip to content
This repository was archived by the owner on Apr 11, 2021. It is now read-only.

feat: reorgs first pass #271

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

feat: reorgs first pass #271

wants to merge 3 commits into from

Conversation

tynes
Copy link
Collaborator

@tynes tynes commented Mar 10, 2021

Description

WIP: reorgs

Metadata

Fixes

  • Fixes # [Link to Issue]

Contributing Agreement

@@ -66,7 +66,8 @@ func run(evm *EVM, contract *Contract, input []byte, readOnly bool) ([]byte, err
} else {
log.Debug("Calling Known Contract", "ID", evm.Id, "Name", name, "Method", method.RawName)
if method.RawName == "ovmREVERT" {
log.Debug("Contract Threw Exception", "ID", evm.Id, "asciified", string(input))
hex := hexutil.Encode(input)
Copy link
Contributor

Choose a reason for hiding this comment

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

FWIW I intentionally put this as asciified and not hex because it is usually a solidity revert string which is readable ascii. @smartcontracts and I ran into a case the other day where we wanted raw hex though, could be worth having both.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I noticed it printing garbage characters so there must be a case where it does not include ascii

Copy link
Contributor

Choose a reason for hiding this comment

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

It prepends the methodID hash for Error(string), so there are consistently those garbage characters at the front. There are definitely cases where there will be raw hex here but almost all normal contract reversions will be ascii. I'd recommend that we have both.

@tynes tynes mentioned this pull request Mar 27, 2021
1 task
@tynes tynes mentioned this pull request Apr 5, 2021
1 task
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.

2 participants