Result/Error improvements

[vdrg]

We've been discussing a few ideas for improving the way Errors are structured within our new "Rust-like" results implementation. Some ideas:

1. Current way of handling errors

Base Error: struct Error { id, message, data? }

For each module, we define:

library LibFooError {
    bytes32 constant ERROR1_ID = ...;
    bytes32 constant ERROR2_ID = ...;

    function error1(...args) internal pure returns(FooResult memory) { }
    function error2(...args) internal pure returns(FooResult memory) { }
}

Usage:

// To return the error
return LibFooError.error1(...args);

// To handle the error
if (res.isError(LibFooError.ERROR1_ID)) {
    (...decodedArgs) = abi.decode(res.toError().data, (...argTypes));
}

Pros:

• Simple

Cons:

• Error id definitions are separate from the other stuff related to the error (getting ids and creating the error are not standardized)
• No type safety, users have to manually decode
• Encoding/decoding must be simple (abi.decode)

2. Errors as micro libraries

Base Error: struct Error { id, message, data? }

For each specific error, we define:

library FooError {
    bytes32 constant id = ...;
    function create(...args) internal pure returns(Result memory) { } // Alternatively this could return the specific result we want
    function decode(Error memory) internal pure returns(...decodedArgs) { } // Only if Error encodes data
}

Usage:

    library someModule {
        function returnErrors() internal returns (ModuleResult memory) {
            return ModuleResult(FooError.create(bytes32("Hello, World!")));
        }
    }

    function foo() external {
        ModuleResult memory result0 = someModule.returnErrors();

        if (result0.isOk()) {
            // Do something
        }

        if (result0.isError(FooError.id) {
             bytes32 payload = FooError.decode(result0.toError());

             expect(payload).toEqual(bytes32("Hello, World!")); // true
        }
    }

Pros:

• Full type safety
• The error id "is part" of the error library definition (standardized way of checking ids)
• Internal encoding/decoding can be as complex as we need as long as we don't change the api

Cons:

• We have to define a library for each error
• The library functions are just a convention, as libs can't inherit (all methods are conventions anyways)
• Add more

3. Using custom errors

Base Error: struct Error { message, data }

We modify the Error struct functions and we add

    function extract(Error memory self) internal pure returns (bytes4 selector, bytes memory payload) {
        (selector, payload) = extractSelector(self.data);
    }

Our module specific error library will look something like

library ModuleErrors {
    error Error0(bytes32);

    // `data` is the additional information we want to store on the error
    function error0(bytes32 data) internal pure returns (TestResult memory) {
        return TestResult(Error("error0", abi.encodeWithSelector(Error0.selector, data)).toResult());
    }
}

Usage:

    library someModule {
        function returnErrors() internal returns (ModuleResult memory) {
            return ModuleErrors.error0(bytes32("Hello, World!"));
        }
    }

    function foo() external {
        TestResult memory result0 = someModule.returnErrors();

        if (result0.isError()) {
            Error memory err = result0.toError();

            (bytes4 selector, bytes memory payload) = err.extract();

            if (selector == TestErrors.Error0.selector) {
                bytes32 payload = abi.decode(payload, (bytes32));

                expect(payload).toEqual(bytes32("Hello, World!")); // true
            }
        }
    }

Pros:

• We don't need to maintain custom ids since we can use selectors.
• Custom errors encapsulate and describes the additional data we want to transmit with the error.
• There is less boilerplate since the extract function will return the selector and payload directly from the Error struct so we don't need to add special functions to the module libraries

Cons:

• The payload needs to be decoded with the correct types so bugs can be added when decoding
• There is no type safety when storing the custom error in the Error struct

4. Errors as functions

Base Error: type Error is bytes32

For each specific error we define a function:

    using LibError for *;

    function FooError(bytes32 value) internal pure returns (Error) {
        return FooError.encodeError("Foo error message", value);
    }

Usage:

    library someModule {
        function returnErrors() internal returns (ModuleResult memory) {
            return ModuleResult(FooError(bytes32("Hello, World!")).toResult());
        }
    }

    function foo() external {
        ModuleResult memory result0 = someModule.returnErrors();

        if (result0.isOk()) {
            // Do something
        }

        Error err = result0.toError();
        if (err.matches(FooError) {
             bytes32 payload = err.decodeAs(FooError);

             expect(payload).toEqual(bytes32("Hello, World!")); // true
        }
    }

Pros:

• Full type safety
• The error id "is part" of the error (it is just the encoded internal function)
• Clean syntax as the error functions can be passed as arguments to other functions
• Once we have the main setup ready for multiple function types, defining new error functions is extremely easy

Cons:

• We have to define a large LibError library that defines matches, encodeError and decodeAs for each function type we want to support (this could be automated though).
• internal function encoding is actually not documented and is considered an implementation detail, so it could change at some point. However, other projects use function pointers so we should probably be fine.

[vdrg]

We decided to go for 4. implemented in #189