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

Error IDs should only be four bytes long #29973

Open
shoenseiwaso opened this issue Jun 11, 2024 · 5 comments
Open

Error IDs should only be four bytes long #29973

shoenseiwaso opened this issue Jun 11, 2024 · 5 comments
Labels

Comments

@shoenseiwaso
Copy link
Contributor

shoenseiwaso commented Jun 11, 2024

System information

Geth
Version: 1.13.14-stable
Git Commit: 2bd6bd0
Architecture: arm64
Go Version: go1.22.1
Operating System: darwin

Expected behaviour

The ABI Error type's ID field should be four bytes long instead of a full hash, similar to Method, but different from Event, which uses the full hash.

Consider the following Solidity code:

// SPDX-License-Identifier: MIT
pragma solidity ~0.8.0;

contract FourbyteEventErrorClash {
    event C8oTw6suLJ(uint256 a); // 72c3d9f62ad3bd447d142486ab15ad03084b3a8e60bedd4b5eb858b56a4facf9
    event UesJkLtTeG(uint256 b); // 72c3d9f63412bbfa54e1d173d8f05aab6a55a47afc4249497933fe3cbb23ed8a

    error EZCGQCzJGG(uint256); // 8e3b9a57b2eb3d6689c28d245256af6f383078e2a54052548410bf650cf73fc7
    error cQYHroCUn9(uint256); // 8e3b9a57c90b37a1aa8ec1f182dd6d9ab5263da6368670ae8cb78eacc63caa36

    function emitEvents() public {
        emit C8oTw6suLJ(123);
        emit UesJkLtTeG(456);
    }
}

As expected, attempting to compile it does yield an error for the two errors which clash on the first four bytes of the hash (but not the full hash), and does not yield an error for the events, which also clash on the first four bytes of the hash (but not the full hash).

$ solc-0.8.26 --combined-json bin,abi FourbyteEventErrorClash.sol

Error: Error signature hash collision for cQYHroCUn9(uint256)
 --> FourbyteEventErrorClash.sol:9:11:
  |
9 |     error cQYHroCUn9(uint256); // 8e3b9a57c90b37a1aa8ec1f182dd6d9ab5263da6368670ae8cb78eacc63caa36
  |           ^^^^^^^^^^
Note: This error has a different signature but the same hash:
 --> FourbyteEventErrorClash.sol:8:5:
  |
8 |     error EZCGQCzJGG(uint256); // 8e3b9a57b2eb3d6689c28d245256af6f383078e2a54052548410bf650cf73fc7
  |     ^^^^^^^^^^^^^^^^^^^^^^^^^^

Therefore, the ABI Error type's ID field should be changed to []byte and be equal to the first four bytes of the hash (id := crypto.Keccak256([]byte(sig))[:4]), the same as Function.

@gerceboss
Copy link

I belive that the problem is here . Can you assign me this issue @shoenseiwaso ?

@fjl
Copy link
Contributor

fjl commented Jun 13, 2024

We can't change the type definition of Error because it would be backwards-incompatible. I'm kind of curious if this issue has any practical impact. @shoenseiwaso can you share your thoughts on that please.

@shoenseiwaso
Copy link
Contributor Author

We can't change the type definition of Error because it would be backwards-incompatible. I'm kind of curious if this issue has any practical impact. @shoenseiwaso can you share your thoughts on that please.

It's true, I'm having a hard time finding a non-contrived case where this is an issue. How about at least documenting that it should be the first four bytes of the ID? abi.ErrorByID() already requires this. Happy to put in a tiny PR for this.

Here's a (slightly naive) example where a manual search for the error ID fails, whereas a manual search for the event ID succeeds.

package main

import (
	"context"
	"encoding/json"
	"errors"
	"fmt"
	"log"
	"math/big"

	"github.com/ethereum/go-ethereum"
	"github.com/ethereum/go-ethereum/accounts/abi"
	"github.com/ethereum/go-ethereum/common"
	"github.com/ethereum/go-ethereum/common/hexutil"
	"github.com/ethereum/go-ethereum/ethclient"
	"github.com/ethereum/go-ethereum/rpc"
)

const (
	rpcEndpoint = "" // for Sepolia - NEEDS TO BE SET FOR THE EXAMPLE TO WORK

	// JSON string representing the ABI of the contract
	jsonABI = `[{"inputs":[{"internalType":"uint256","name":"","type":"uint256"}],"name":"EZCGQCzJGG","type":"error"},{"anonymous":false,"inputs":[{"indexed":false,"internalType":"uint256","name":"a","type":"uint256"}],"name":"C8oTw6suLJ","type":"event"},{"inputs":[{"internalType":"uint256","name":"a","type":"uint256"}],"name":"NwiFBi5v2i","outputs":[{"internalType":"uint256","name":"","type":"uint256"}],"stateMutability":"pure","type":"function"},{"inputs":[],"name":"emitEvent","outputs":[],"stateMutability":"nonpayable","type":"function"}]`

	contractAddressStr = "0xd50147B4Dd5F23e3Fff0D73119e4DC446FD7f0b3"                         // on Sepolia
	functionName       = "NwiFBi5v2i"                                                         // function to call, note that this function will always revert with the error EZCGQCzJGG()
	errorName          = "EZCGQCzJGG"                                                         // name of the error
	txHash             = "0xedaa4f5dd8d70b15c4fa2e4cd20cc457a79d98ea6adbd355f54eb95f578b5684" // on Sepolia, transaction that emits an event
)

func main() {
        if rpcEndpoint == "" {
		log.Fatalf("rpcEndpoint not set - please edit the constant in the source code")
	}

	// Connect to an Ethereum node
	client, err := ethclient.Dial(rpcEndpoint)
	if err != nil {
		log.Fatalf("Failed to connect to the Ethereum client: %v", err)
	}

	// Convert JSON string to ABI
	var contractABI abi.ABI
	err = json.Unmarshal([]byte(jsonABI), &contractABI)
	if err != nil {
		log.Fatalf("Failed to unmarshal ABI: %v", err)
	}

	// Address of the contract
	contractAddress := common.HexToAddress(contractAddressStr)

	// Prepare the transaction data
	param := big.NewInt(42)                            // parameter value to pass, can be any uiint256
	data, err := contractABI.Pack(functionName, param) // function to call
	if err != nil {
		log.Fatalf("Failed to pack function call data: %v", err)
	}

	// Create a call message
	msg := ethereum.CallMsg{
		To:   &contractAddress,
		Data: data,
	}

	// Call the contract function
	ctx := context.Background()
	result, err := client.CallContract(ctx, msg, nil)
	if err != nil {
		// This is expected as we are calling a function that emits an error
		fmt.Printf("Error calling contract, as expected: %v\n", err)
	}

	fmt.Println("---")

	// If there's no error, parse the result - this should not happen
	if len(result) > 0 {
		log.Fatalf("Function call result with no error, unexpected: %s\n", result)
	}

	// Decode the error data from the RPC error
	var rpcErr rpc.DataError
	ok := errors.As(err, &rpcErr)
	if !ok {
		log.Fatalf("Failed to convert error to rpc.DataError: %v", err)
	}
	blockchainErrData, ok := rpcErr.ErrorData().(string)
	if !ok {
		log.Fatalf("Failed to convert error data to string: %v", rpcErr.ErrorData())
	}
	blockchainErr, err := hexutil.Decode(blockchainErrData)
	if err != nil {
		log.Fatalf("Failed to decode error data: %v", err)
	}

	// Option 1: Decode the error using contractABI.ErrorByID()
	errorID := [4]byte(blockchainErr[:4])
	errorDescription, err := contractABI.ErrorByID(errorID)
	if err != nil {
		fmt.Printf("Failed to decode error: %v\n", err)
	} else {
		fmt.Printf("Decoded error via ErrorByID(): %s\n", errorDescription)
	}

	fmt.Println("---")

	// Option 2: Decode the error by searching for the error ID in the ABI

	// Naively (?) taking the first 32 bytes of the error data as the ID, which matches the size of common.Hash in the decoded ABI
	blockchainErrID := common.Hash(blockchainErr[:32])

	found := false
	for _, abiError := range contractABI.Errors {
		fmt.Printf("Checking ABI error ID %s vs. blockchain error ID %s\n", abiError.ID.String(), blockchainErrID.String())
		if abiError.ID == blockchainErrID {
			fmt.Printf("FOUND! Decoded error via ABI lookup: %s\n", abiError.Name)
			found = true
			break
		}
	}

	if !found {
		fmt.Printf("NOT FOUND! Error ID %s not found in ABI\n", blockchainErrID)
	}

	fmt.Println("---")

	// Now for comparison, retrieve a transaction receipt that emits an event
	receipt, err := client.TransactionReceipt(ctx, common.HexToHash(txHash))
	if err != nil {
		log.Fatalf("Failed to retrieve transaction receipt: %v", err)
	}

	if len(receipt.Logs) == 0 || len(receipt.Logs[0].Topics) == 0 {
		log.Fatalf("No event logs found in the transaction receipt")
	}
	eventID := receipt.Logs[0].Topics[0] // event ID

	// Search for the event in the ABI
	found = false
	for _, abiEvent := range contractABI.Events {
		fmt.Printf("Checking ABI event ID %s vs. blockchain event ID %s\n", abiEvent.ID.String(), eventID)
		if abiEvent.ID == eventID {
			fmt.Printf("FOUND! Event that emits the error: %s\n", abiEvent.Name)
			found = true
			break
		}
	}

	if !found {
		fmt.Printf("NOT FOUND! Event ID %s not found in ABI\n", eventID)
	}
}

Output:

$ go run .

Error calling contract, as expected: execution reverted
---
Decoded error via ErrorByID(): error EZCGQCzJGG(uint256 arg0)
---
Checking ABI error ID 0x8e3b9a57b2eb3d6689c28d245256af6f383078e2a54052548410bf650cf73fc7 vs. blockchain error ID 0x8e3b9a5700000000000000000000000000000000000000000000000000000000
NOT FOUND! Error ID 0x8e3b9a5700000000000000000000000000000000000000000000000000000000 not found in ABI
---
Checking ABI event ID 0x72c3d9f62ad3bd447d142486ab15ad03084b3a8e60bedd4b5eb858b56a4facf9 vs. blockchain event ID 0x72c3d9f62ad3bd447d142486ab15ad03084b3a8e60bedd4b5eb858b56a4facf9
FOUND! Event that emits the error: C8oTw6suLJ

@lightclient
Copy link
Member

Adding a comment seems reasonable. Do you want to submit a PR @shoenseiwaso?

@shoenseiwaso
Copy link
Contributor Author

Adding a comment seems reasonable. Do you want to submit a PR @shoenseiwaso?

Yes, would be happy to! Will do so in the next few days.

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

No branches or pull requests

4 participants