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

try/catch for abi.decode() or abi.tryDecode #10381

Open
gitpusha opened this issue Nov 24, 2020 · 13 comments
Open

try/catch for abi.decode() or abi.tryDecode #10381

gitpusha opened this issue Nov 24, 2020 · 13 comments
Labels
high effort A lot to implement but still doable by a single person. The task is large or difficult. high impact Changes are very prominent and affect users or the project in a major way. language design :rage4: Any changes to the language, e.g. new features must have Something we consider an essential part of Solidity 1.0. needs design The proposal is too vague to be implemented right away

Comments

@gitpusha
Copy link
Contributor

gitpusha commented Nov 24, 2020

Hi,

I am referencing the conversation #10317 .

It would be nice to have a way to catch errors thrown during abi.decode operations.

Quoting @chriseth:

Instead of try/catch we should maybe rather have abi.tryDecode? The problem here is how to return the data in the two cases

Also try catch might be useful for functions other than abi.decode but I will first need to find concrete examples and will report them here in this issue.


Preliminary implementation: #10883

@cameel cameel added feature language design :rage4: Any changes to the language, e.g. new features waiting for more input Issues waiting for more input by the reporter labels Nov 24, 2020
@cameel cameel added this to New issues in Solidity via automation Nov 24, 2020
@chriseth chriseth moved this from New issues to Design Backlog in Solidity Dec 1, 2020
@chriseth
Copy link
Contributor

chriseth commented Feb 3, 2021

I'm starting work on the underlying mechanism for use in catch. The problem of exposing this to the user is still how to deal with the actual values in case of an error. We can use a success flag:

(bool success, uint value) = abi.tryDecode(ms.data, (uint));

but this would make value accessible even in case of failure.

@ekpyron
Copy link
Member

ekpyron commented Feb 10, 2021

I'm starting work on the underlying mechanism for use in catch. The problem of exposing this to the user is still how to deal with the actual values in case of an error. We can use a success flag:

(bool success, uint value) = abi.tryDecode(ms.data, (uint));

but this would make value accessible even in case of failure.

Algebraic datatypes (#909) could be a way around that...

@chriseth
Copy link
Contributor

Until we have algebraic datatypes, returning an additional bool might be a good solution. All our types should have a sane zero-value, at least those that can be abi-decoded.

@daltonclaybrook
Copy link

Any update on this issue? Would love to be able to try abi.decode without fear of panic.

@fredlacs
Copy link

Would also love support for abi.tryDecode that returns a bool

@giuseppeg
Copy link

What is the current solution to this? Just let it revert without reason?

@CJ42
Copy link
Contributor

CJ42 commented Jul 1, 2022

It would be useful especially in case when trying to decode abi-encoded arrays, where the bytes data passed as an argument to abi.decode(...) is not a properly encoded array of valueTypes[], according to the ABI specs.

I encountered the issue on my side, and the only way was to write some custom functions that analyse the data to check for valid offsets, length, etc...

For reference, see https://github.com/lukso-network/lsp-smart-contracts/blob/53e33cc5359244cbc6f9f1db4ffc7c5b1348ad67/contracts/LSP2ERC725YJSONSchema/LSP2Utils.sol#L135-L197

using BytesLib for uint256;

function isEncodedArray(bytes memory data) internal pure returns (bool) {
    uint256 nbOfBytes = data.length;

    // there must be at least 32 x length bytes after offset
    uint256 offset = uint256(bytes32(data));
    if (nbOfBytes < offset + 32) return false;
    uint256 arrayLength = data.toUint256(offset);

    //   32 bytes word (= offset)
    // + 32 bytes word (= array length)
    // + remaining bytes that make each element of the array
    if (nbOfBytes < (offset + 32 + (arrayLength * 32))) return false;

    return true;
}

function isEncodedArrayOfAddresses(bytes memory data) internal pure returns (bool) {
    if (!isEncodedArray(data)) return false;

    uint256 offset = uint256(bytes32(data));
    uint256 arrayLength = data.toUint256(offset);

    uint256 pointer = offset + 32;

    for (uint256 ii = 0; ii < arrayLength; ii++) {
        bytes32 key = data.toBytes32(pointer);

        // check that the leading bytes are zero bytes "00"
        // NB: address type is padded on the left (unlike bytes20 type that is padded on the right)
        if (bytes12(key) != bytes12(0)) return false;

        // increment the pointer
        pointer += 32;
    }

     return true;
}

function isBytes4EncodedArray(bytes memory data) internal pure returns (bool) {
    if (!isEncodedArray(data)) return false;

    uint256 offset = uint256(bytes32(data));
    uint256 arrayLength = data.toUint256(offset);
    uint256 pointer = offset + 32;

    for (uint256 ii = 0; ii < arrayLength; ii++) {
        bytes32 key = data.toBytes32(pointer);

        // check that the trailing bytes are zero bytes "00"
        if (uint224(uint256(key)) != 0) return false;

        // increment the pointer
        pointer += 32;
    }

    return true;
}

a tryDecode would be useful in such case and apply well. For instance:

function isAbiEncodedArrayOfAddresses(bytes memory data) public returns (bool) {
    try abi.tryDecode(data, (address[]) returns (address[] memory) {
        // ...
        return true;
    } catch {
        // ...
        return false;
    }
}

Or in a more simpler using @chriseth example:

function isAbiEncodedArrayOfAddresses(bytes memory data) public returns (bool) {
    (bool success,) = abi.tryDecode(ms.data, (address[]));
    return success;
}

For arrays, the tryDecode could analyze the structure of the bytes given as input. For instance for abi-encoded array of bytes:

  • if the offset is valid (= not more than uint64, being the maximum size of the memory allowed)
  • if the offset actually point to some bytes offset, and does not point to far (an "empty space where there is nothing")
  • if there is the right number of values according to the encoded length.

For simple types like uint256, uint128, bytesN, etc..., I guess it could check for things like out of range value, the right number of bytes, etc...

@cameel cameel added high effort A lot to implement but still doable by a single person. The task is large or difficult. high impact Changes are very prominent and affect users or the project in a major way. must have Something we consider an essential part of Solidity 1.0. needs design The proposal is too vague to be implemented right away and removed waiting for more input Issues waiting for more input by the reporter labels Sep 26, 2022
@novaknole
Copy link

Hi all.

Any news on this ?

@cameel
Copy link
Member

cameel commented Oct 20, 2022

Not yet, other that we do consider this pretty important so it's on our roadmap, along with other high-impact features. It's waiting for its turn.

@jefflau
Copy link

jefflau commented Nov 14, 2022

Hi all. I'm also looking for this feature, would be nice to be able to catch and throw a specific error

@chriseth
Copy link
Contributor

chriseth commented Feb 8, 2023

Preliminary implementation: #10883

@cameel
Copy link
Member

cameel commented Feb 12, 2023

We have too many duplicates of this issue. I'm going to close it in favor of #13869, which has a syntax proposal that could use some feedback.

@cameel cameel closed this as not planned Won't fix, can't repro, duplicate, stale Feb 12, 2023
Solidity automation moved this from Design Backlog to Done Feb 12, 2023
@cameel
Copy link
Member

cameel commented Feb 12, 2023

Ah, wait. This is actually slightly different. #13869 would still not allow you to catch reverts from explicit abi.decode() calls. Just from the implicit decoding that the compiler does. Still, maybe allowing abi.decode() in try/catch and using the syntax from #13869 (comment) would be a good way to solve this.

@cameel cameel reopened this Feb 12, 2023
Solidity automation moved this from Done to In progress Feb 12, 2023
@cameel cameel changed the title feature request: try catch for abi.decode try/catch for abi.decode() Feb 12, 2023
@cameel cameel removed this from In progress in Solidity Feb 12, 2023
@cameel cameel removed the feature label Feb 12, 2023
@ekpyron ekpyron changed the title try/catch for abi.decode() try/catch for abi.decode() or abi.tryDecode Feb 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
high effort A lot to implement but still doable by a single person. The task is large or difficult. high impact Changes are very prominent and affect users or the project in a major way. language design :rage4: Any changes to the language, e.g. new features must have Something we consider an essential part of Solidity 1.0. needs design The proposal is too vague to be implemented right away
Projects
None yet
Development

No branches or pull requests