fuels-ts icon indicating copy to clipboard operation
fuels-ts copied to clipboard

feat: log out custom require messages for error enums when tx reverts

Open theausicist opened this issue 1 year ago • 12 comments
trafficstars

Unfortunately, the SDK does not support debugging revert errors with string messages - at least temporarily. This is because the SDK does not support decoding of Sway str slices in the v0 Sway encoding scheme. This problem will be fixed soon once the v1 Sway encoding scheme is adopted.

But for now, if your Sway contract has a revert error with a string message like this:

fn test_function() -> bool {
    require(false, "This is a revert error");
    true
}

The SDK will only log out a generic error: String slices can not be decoded from logs.

This PR enables the logging of the messages associated with custom error enums in Sway, and recommends users to use these enums to debug contracts that require multiple require statements. After this PR, require reverts using error enums will be logged out like this:

The script reverted with reason RequireFailed. (Reason: "InvalidInput")

theausicist avatar Mar 06 '24 18:03 theausicist

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Mar 06 '24 18:03 CLAassistant

@theausicist thanks for this. I had just started investigating the problem. Your PR definitely has some useful insights that will help me. Did you verify that this works though? I tried copying your solution to a local test environment that I have and I did not see any changes. Just curious if you had tested this by any chance :D

Dhaiwat10 avatar Mar 06 '24 20:03 Dhaiwat10

The culprit that I've found from my investigation is the fact that the TS SDK does not support decoding str slices in the v0 encoding scheme. Until we incorporate the v1 encoding scheme, the only way to fix this problem is by somehow adding decoding support for them. cc @danielbate @FuelLabs/sdk-ts any thoughts?

Dhaiwat10 avatar Mar 06 '24 20:03 Dhaiwat10

@theausicist thanks for this. I had just started investigating the problem. Your PR definitely has some useful insights that will help me. Did you verify that this works though? I tried copying your solution to a local test environment that I have and I did not see any changes. Just curious if you had tested this by any chance :D

I directly modified/patched the built index.js within the node_modules of the @fuel-ts package, and tried to replicate the changes within the raw code here. It's possible I missed something. But like I said, it is hacky! Here's the full index.js file (in node_modules/@fuel-ts/program/dist/index.js) and the original build

theausicist avatar Mar 07 '24 05:03 theausicist

Relates to #1556

petertonysmith94 avatar Mar 07 '24 06:03 petertonysmith94

@theausicist that's actually super helpful. Can I ask what version of the SDK that code is from?

Dhaiwat10 avatar Mar 11 '24 04:03 Dhaiwat10

@theausicist that's actually super helpful. Can I ask what version of the SDK that code is from?

v0.76.0

theausicist avatar Mar 11 '24 04:03 theausicist

@Dhaiwat10 IMO regardless of where this issue was coming from (i.e. adding str slice support), adding logs to an error message, probably with some formatting or interpolation, is a good idea. Are you implementing this yourself or should we review this PR?

danielbate avatar Mar 11 '24 11:03 danielbate

@danielbate for now I'm just trying to tackle a very specific problem first: making sure we log out custom require revert messages from Sway programs. I haven't gotten to the broader problems which would be str slice support and error logs like you mentioned.

This PR, while super useful in pointing us in a correct direction, is not ready for review I believe like @theausicist pointed out - it's a hack more or less. We would probably want to make a substantial amount of changes to this PR or create a new one to incorporate these changes. I recommend tackling them once we get custom require revert messages working first. Maybe we could file issues for all three problems to track them, though. Does that sound fair?

Dhaiwat10 avatar Mar 11 '24 12:03 Dhaiwat10

@theausicist I tried replacing my node_modules/@fuel-ts/program/dist/index.js with the file that you provided and I'm still only seeing this error:

FuelError: String slices can not be decoded from logs. Convert the slice to `str[N]` with `__to_str_array`

:( I'm gonna continue investigating to see if there is some other way to log out the reason string, but the approach you suggested doesn't seem to be working. v1 encoding solves this anyways, but that's still several weeks away.

Dhaiwat10 avatar Mar 12 '24 06:03 Dhaiwat10

@theausicist I tried replacing my node_modules/@fuel-ts/program/dist/index.js with the file that you provided and I'm still only seeing this error:

FuelError: String slices can not be decoded from logs. Convert the slice to `str[N]` with `__to_str_array`

:( I'm gonna continue investigating to see if there is some other way to log out the reason string, but the approach you suggested doesn't seem to be working. v1 encoding solves this anyways, but that's still several weeks away.

you'll get this error because you're using raw error strings in the require method which fails with the error you mentioned above. If you use an Error enum, fuel-ts can decode this nicely, following which this PR can help.

instead of:

require(false, "Raw Error");

do:

enum Error {
    DecodeableError: ()
}

...
require(false, Error::DecodeableError);

theausicist avatar Mar 12 '24 07:03 theausicist

@theausicist I see what you mean. I just tried this out and it works. :)

@FuelLabs/sdk-ts Adding support for decoding str slices in the v0 scheme doesn't seem to be all that feasible, especially since we are gonna move to v1 very soon. I think a fair compromise would be supporting at least error enums as mentioned in @theausicist's comment above, and recommending the usage of error enums until v1 support drops with a notice in our docs.

Please let me know if this sounds fair, or if anyone has any other ideas.

Dhaiwat10 avatar Mar 13 '24 07:03 Dhaiwat10