Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

hevm: invalid character at offset #802

Closed
shazow opened this issue Sep 19, 2021 · 13 comments · Fixed by #804
Closed

hevm: invalid character at offset #802

shazow opened this issue Sep 19, 2021 · 13 comments · Fixed by #804

Comments

@shazow
Copy link

shazow commented Sep 19, 2021

Hiya, I'm just giving dapptools a try, I wrote a basic contract and some tests. I have my nix shell pinned at the hevm/0.48.1 tag.

I'm getting a difficult-to-understand/debug error, is this something I'm doing wrong?

hevm: invalid character at offset: 4908
CallStack (from HasCallStack):
  error, called at src/EVM/Solidity.hs:538:13 in hevm-0.48.1-KdjyJ1kXYK7IQeiBwTCoFo:EVM.Solidity

I'm guessing this is some problem with my code [edit: though the offset doesn't seem to change if I add more code to the contract/tests], so I guess this is a request to produce a more actionable error if possible. :)

Would also appreciate any tips for how to debug this productively.

Versions:

  /nix/store/6hf8z25vi0h0k8biswmbh5x0c4xwj1d6-hevm-0.48.1.drv
  /nix/store/gi39gmmjhb5q3dk3gp70fn606vw7r4ql-ethsign-0.17.0-go-modules.drv
  /nix/store/h1z3nhhxkaaq452zz5d72g6xyw6bw0f7-ethsign-0.17.0.drv
  /nix/store/31bw7f8bj8ivj6wh48dkngmv1yrvsz54-seth-0.10.1.drv
  /nix/store/1z25gzb3i71hvmr8pq50kn2piljgdgnq-dapp-0.34.0.drv

Full output:

$ make test
dapp test # --ffi # enable if you need the `ffi` cheat code on HEVM
+ dapp clean
+ rm -rf out
hevm: invalid character at offset: 4908
CallStack (from HasCallStack):
  error, called at src/EVM/Solidity.hs:538:13 in hevm-0.48.1-KdjyJ1kXYK7IQeiBwTCoFo:EVM.Solidity
make: *** [Makefile:20: test] Error 1
@transmissions11
Copy link
Contributor

can you upload and send over a link to your repo? hard to debug without any info

@shazow
Copy link
Author

shazow commented Sep 19, 2021

Er sorry, I just tracked it down the minimal reproduction: https://github.com/shazow/dapptools-bug-repro

make test in that repo should reproduce it.

Looks like the test runner doesn't like these lines (build compiles fine):

    function cmp(string calldata a, string calldata b) pure public returns (bool) {
        return keccak256(abi.encodePacked(a)) == keccak256(abi.encodePacked(b));
    }

@d-xo
Copy link
Contributor

d-xo commented Sep 21, 2021

Thanks for the report @shazow.

It seems as though we were passing the libraries to solc in a format that was no longer accepted post 0.7. Should be fixed once #804 is merged.

@d-xo d-xo closed this as completed in #804 Sep 21, 2021
@shazow
Copy link
Author

shazow commented Sep 21, 2021

Thanks for the quick fix!

@shazow
Copy link
Author

shazow commented Sep 21, 2021

I confirmed the fix did work for the minimal reproduction, but unfortunately the error still exists in my larger contract repo.

The full thing is here: https://github.com/NonFungibleSequences/memenumbers-contract

make test reproduces it.

$ make test
dapp test # --ffi # enable if you need the `ffi` cheat code on HEVM
+ dapp clean
+ rm -rf out
hevm: invalid character at offset: 4908
CallStack (from HasCallStack):
  error, called at src/EVM/Solidity.hs:538:13 in hevm-0.48.1-KdjyJ1kXYK7IQeiBwTCoFo:EVM.Solidity
make: *** [Makefile:20: test] Error 1

@d-xo d-xo reopened this Sep 21, 2021
@d-xo
Copy link
Contributor

d-xo commented Sep 22, 2021

You have library linking disabled in your .dapprc: https://github.com/NonFungibleSequences/memenumbers-contract/blob/master/.dapprc#L7.

Commenting out that line fixes the issue for me.

@d-xo d-xo closed this as completed Sep 22, 2021
@shazow
Copy link
Author

shazow commented Sep 22, 2021

Aha good find! Thank you.

This was all part of this template repo: https://github.com/gakonst/dapptools-template/blob/master/.dapprc

@d-xo
Copy link
Contributor

d-xo commented Sep 22, 2021

The library linking procedure is a little crude, and sometimes ends up deploying libraries that aren't needed (if e.g. all library methods are marked as internal), so switching it off lets you speed up testing in that case.

btw, you can save gas and avoid the need for the linking step by marking cmp as internal (and changing the args from calldata to memory), this way the compiler will directly inline cmp into the contracts where it is used and you will avoid the need for a delegatecall (and associated abi encoding / decoding costs).

@shazow
Copy link
Author

shazow commented Sep 22, 2021

@d-xo Thanks for the tips! This is all just rough brainstorming/sketch ideas, probably going to avoid doing cmp the way it is now anyway and instead compare the first byte which in my case is sufficient. Will definitely use internal for the reasons you mentioned, thanks again!

@transmissions11
Copy link
Contributor

Could we make this error message less opaque to help out future users? “Invalid character at offset” is about as generic as it gets lolol

@d-xo
Copy link
Contributor

d-xo commented Sep 22, 2021

Yeah for sure. We would need to scan for unresolved linker references in the bytecode and error out with a nice message if we try to deploy code with holes.

@transmissions11
Copy link
Contributor

transmissions11 commented Sep 22, 2021

Sounds complex heh. Wdyt about just overriding the error with a hint like Invalid character at offset. This might be because there are unlinked libraries used in your contracts!

Even though this may be unhelpful sometimes still better than nothing?

@d-xo
Copy link
Contributor

d-xo commented Sep 23, 2021

@transmissions11 it was fairly simple to detect the linker holes with a regex, implemented in #816.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants