casper-node icon indicating copy to clipboard operation
casper-node copied to clipboard

Provide more detailed debug info for smart contract execution errors

Open RitaMAllenCA opened this issue 2 years ago • 3 comments

In an attempt to provide more detailed info for the smart contract execution failure I created this PR (now closed). Initially it was only about printing the stack height on Trap::Unreachable, but as a result of this discussion I attempted to provide more info. However, it turned out that there are some issues that prevent doing it in a straightforward way (comment). Since the info we can potentially provide to the users is valuable, we should consider pursuing this in a separate ticket, since this is out of scope for #4003.

After discussion with @EdHastingsCasperLabs, the following approach is proposed:

decouple the performance regression check from tests (long term) -- currently, our tests may fail not because the system is broken, but because the performance changed; these tests will even fail on performance improvement -- our target is for the tests to fail if we broke EE, have some missing FFI, etc. and for the performance to be monitored on CI (there should be a gas cost/file size baseline, and CI should inform us when calculated cost/size of the PR varies too much) remove the need to use print (especially in panic handler) by improving the error reporting (long term) -- having the conditionally compiled print might be confusing (for example, people may try to chase an error in production, they will enable test-support, which will result in an error in different place, because of wasm structure change) -- we shall be able to provide more detailed reason of failure (aka, more fine grained error than just Trap::Unreachable) so that users can reason about what happened split the host-function-metric contract in two (orthogonal) -- one part should just call different FFIs (as in the top of the file) -- second part should exercise the extern with long names

RitaMAllenCA avatar Jul 05 '23 14:07 RitaMAllenCA

@rafal - need an estimate

RitaMAllenCA avatar Aug 15 '23 19:08 RitaMAllenCA

@rafal-ch @mpapierski Does it make sense to do this in 1.6 if we are making substantial changes to the ee in 2.0? My thought is, the temporary solution will suffice until then - WDYT?

RitaMAllenCA avatar Aug 15 '23 19:08 RitaMAllenCA

@RitaMAllenCA Yes, I think we don't need to rush with this issue.

rafal-ch avatar Aug 16 '23 06:08 rafal-ch

This issue has been migrated to Jira. https://casper-association.atlassian.net/browse/CORE-44. Therefore, closing it here.

devendran-m avatar Apr 28 '25 12:04 devendran-m