Custom errors support in FFI contract invocations and queries #1125

jimthematrix · 2023-01-01T21:48:02Z

For a test solidity contract that declares custom errors like the following :

pragma solidity ^0.8.0;

error CustomError1(uint256 x, uint256 y);
error CustomError2(string x);

contract SimpleStorage {
    uint256 storedData;

    function set(uint256 x) public {
        if (x == 0) {
            revert CustomError1({x: x, y: x});
        }
        if (x == 1) {
            revert CustomError2({x: "This is a custom error"});
        }
        require(x > 2, "Can not equal to 2");
        storedData = x;
    }
}

Use the POST /contracts/interfaces/generate endpoint to generate the corresponding FFI:

{
    "namespace": "default",
    "name": "simple-storage-custom-errors",
    "description": "simple-storage-custom-errors",
    "version": "1.0.0",
    "methods": [
        {
            "name": "set",
            "pathname": "",
            "description": "",
            "params": [
                {
                    "name": "x",
                    "schema": {
                        "oneOf": [
                            {
                                "type": "string"
                            },
                            {
                                "type": "integer"
                            }
                        ],
                        "details": {
                            "type": "uint256",
                            "internalType": "uint256"
                        },
                        "description": "An integer. You are recommended to use a JSON string. A JSON number can be used for values up to the safe maximum."
                    }
                }
            ],
            "returns": [],
            "details": {
                "stateMutability": "nonpayable"
            }
        }
    ],
    "errors": [
        {
            "signature": "",
            "name": "CustomError1",
            "description": "",
            "params": [
                {
                    "name": "x",
                    "schema": {
                        "oneOf": [
                            {
                                "type": "string"
                            },
                            {
                                "type": "integer"
                            }
                        ],
                        "details": {
                            "type": "uint256",
                            "internalType": "uint256"
                        },
                        "description": "An integer. You are recommended to use a JSON string. A JSON number can be used for values up to the safe maximum."
                    }
                },
                {
                    "name": "y",
                    "schema": {
                        "oneOf": [
                            {
                                "type": "string"
                            },
                            {
                                "type": "integer"
                            }
                        ],
                        "details": {
                            "type": "uint256",
                            "internalType": "uint256"
                        },
                        "description": "An integer. You are recommended to use a JSON string. A JSON number can be used for values up to the safe maximum."
                    }
                }
            ]
        }
    ]
}

Then when invoking the generated API:

matching custom error

curl --location --request POST 'http://127.0.0.1:5000/api/v1/namespaces/default/apis/simple-storage/invoke/set' \
--header 'Content-Type: application/json' \
--data-raw '{
  "input": {
    "x": 0
  }
}'

response:

{
    "error": "FF10111: Error from ethereum connector: FF23021: EVM reverted: CustomError1(\"0\", \"0\")"
}

no matching custom error (raw error data returned):

curl --location --request POST 'http://127.0.0.1:5000/api/v1/namespaces/default/apis/simple-storage/invoke/set' \
--header 'Content-Type: application/json' \
--data-raw '{
  "input": {
    "x": 1
  }
}'

response:

{
    "error": "FF10111: Error from ethereum connector: FF23022: EVM reverted: 0x60b7d47e0000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000001654686973206973206120637573746f6d206572726f7200000000000000000000"
}

regular error from the require()

curl --location --request POST 'http://127.0.0.1:5000/api/v1/namespaces/default/apis/simple-storage/invoke/set' \
--header 'Content-Type: application/json' \
--data-raw '{
  "input": {
    "x": 2
  }
}'

response:

{
    "error": "FF10111: Error from ethereum connector: FF23021: EVM reverted: Can not equal to 2"
}

Signed-off-by: Jim Zhang <jim.zhang@kaleido.io>

nguyer

Looks good. Just a couple of minor suggestions, and it looks like we may have some Go build issues to work out. Thanks!

nguyer · 2023-01-03T13:54:26Z

internal/apiserver/swagger_check_test.go

@@ -61,7 +61,7 @@ func TestDiffSwaggerYAML(t *testing.T) {
 	var existingSwaggerBytes []byte
 	existingSwaggerBytes, err = os.ReadFile(filepath.Join("..", "..", "docs", "swagger", "swagger.yaml"))
 	assert.NoError(t, err)
-
+	fmt.Println(string(b))


I'm guessing this was left over from testing something?

nguyer · 2023-01-03T15:00:46Z

internal/blockchain/ethereum/ethereum.go

 	if err != nil {
 		return err
 	}
+	options["errors"] = errorsAbi


I think this ends up working the way we expect it to, but the intention is that options is a set of fields passed in by the user.

Maybe invokeContractMethod should also take in an errors param as well, instead of piggybacking on the options map?

in fact that was the route I took originally but then realized that the errors as a construct in the interface definition doesn't apply universally to other protocols such as Fabric. So instead of forcing that as part of the plugin API, I decided to use the options so it fits with this being an "optional" feature. But if you feely strongly about this, I can certainly move to making it part of the InvokeContract() signature out of the options parameter

upon further discussing this with @nguyer and @peterbroadhurst, I realized that the errors construct is already part of the standard FFI API because of the earlier work in FFTM: https://github.com/hyperledger/firefly-transaction-manager/blob/main/pkg/ffcapi/api.go#L202. so I'll make the update as suggested above by Nicko

nguyer · 2023-01-03T15:03:03Z

internal/contracts/manager.go

+		if req.Options == nil {
+			req.Options = map[string]interface{}{
+				"errors": errors,
+			}
+		} else {
+			req.Options["errors"] = errors
+		}


Perhaps instead of setting req.Options["errors"] here, we should add an errors field to the ContractCallRequest struct itself.

nguyer · 2023-01-03T15:05:26Z

internal/coremsgs/en_error_messages.go

@@ -33,7 +33,7 @@ var (
 	MsgWebsocketClientError               = ffe("FF10108", "Error received from WebSocket client: %s")
 	Msg404NotFound                        = ffe("FF10109", "Not found", 404)
 	MsgUnknownBlockchainPlugin            = ffe("FF10110", "Unknown blockchain plugin: %s")
-	MsgEthconnectRESTErr                  = ffe("FF10111", "Error from ethconnect: %s")
+	MsgEthconnectRESTErr                  = ffe("FF10111", "Error from ethereum connector: %s")


Should we also rename the key itself? (not the error code)

nguyer · 2023-01-03T15:12:38Z

internal/coremsgs/en_error_messages.go

@@ -271,4 +271,6 @@ var (
 	MsgIdempotencyKeyDuplicateMessage     = ffe("FF10430", "Idempotency key '%s' already used for message '%s'", 409)
 	MsgIdempotencyKeyDuplicateTransaction = ffe("FF10431", "Idempotency key '%s' already used for transaction '%s'", 409)
 	MsgNonIdempotencyKeyConflictTxInsert  = ffe("FF10432", "Conflict on insert of transaction '%s'. No existing transaction matching idempotency key '%s' found", 409)
+	MsgErrorNameMustBeSet                 = ffe("FF10433", "Error name must be set", 400)


Seeing this out of context confused me for a little bit as to the meaning of the message.

I was originally reading it as: Error. Name must be set. But I realized that's not actually what it says 🙂

Would it make more sense to say: The name of the error must be set ?

Signed-off-by: Jim Zhang <jim.zhang@kaleido.io>

jimthematrix · 2023-01-03T20:18:29Z

@nguyer , should be all addressed in the latest commit. I believe the build errors should be taken care of once the dependency components are updated with a new release (and the dependency version in go.mod is updated)

Signed-off-by: Jim Zhang <jim.zhang@kaleido.io>

codecov-commenter · 2023-01-05T17:59:55Z

Codecov Report

Merging #1125 (2f08032) into main (6565f7c) will decrease coverage by 0.49%.
The diff coverage is 34.01%.

@@            Coverage Diff             @@
##             main    #1125      +/-   ##
==========================================
- Coverage   99.82%   99.33%   -0.50%     
==========================================
  Files         302      303       +1     
  Lines       19506    19617     +111     
==========================================
+ Hits        19472    19486      +14     
- Misses         29      122      +93     
- Partials        5        9       +4

Impacted Files	Coverage Δ
internal/blockchain/ethereum/address_resolver.go	`100.00% <ø> (ø)`
internal/database/sqlcommon/ffi_errors_sql.go	`0.00% <0.00%> (ø)`
internal/definitions/handler_contracts.go	`95.52% <0.00%> (-4.48%)`	⬇️
internal/definitions/sender_contracts.go	`94.11% <0.00%> (-5.89%)`	⬇️
pkg/core/contracts.go	`100.00% <ø> (ø)`
internal/contracts/manager.go	`95.83% <21.73%> (-4.17%)`	⬇️
internal/blockchain/fabric/fabric.go	`99.60% <50.00%> (-0.40%)`	⬇️
internal/blockchain/ethereum/ethereum.go	`100.00% <100.00%> (ø)`
internal/blockchain/ethereum/eventstream.go	`100.00% <100.00%> (ø)`
internal/contracts/operations.go	`100.00% <100.00%> (ø)`
... and 2 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

jimthematrix added 5 commits January 1, 2023 16:04

Add FFI errors support in the local db

84b62d1

Signed-off-by: Jim Zhang <jim.zhang@kaleido.io>

Add custom errors to FFI generation, conversion to ABI

98917f3

Signed-off-by: Jim Zhang <jim.zhang@kaleido.io>

Update swagger and reference docs

f31a01d

Signed-off-by: Jim Zhang <jim.zhang@kaleido.io>

Update mocks

107674f

Signed-off-by: Jim Zhang <jim.zhang@kaleido.io>

Update dependencies

5626e5b

Signed-off-by: Jim Zhang <jim.zhang@kaleido.io>

jimthematrix requested review from peterbroadhurst, nguyer, awrichar, shorsher and nickgaski as code owners January 1, 2023 21:48

nguyer approved these changes Jan 3, 2023

View reviewed changes

jimthematrix added 4 commits January 3, 2023 15:04

Move the errors spec out of the 'options' to the API parameter list

259e8b0

Signed-off-by: Jim Zhang <jim.zhang@kaleido.io>

Update swagger docs

189e650

Signed-off-by: Jim Zhang <jim.zhang@kaleido.io>

Update translation key

1d1eddb

Signed-off-by: Jim Zhang <jim.zhang@kaleido.io>

Update error string

d9751f1

Signed-off-by: Jim Zhang <jim.zhang@kaleido.io>

jimthematrix added 5 commits January 3, 2023 15:24

Update go.sum

a35484b

Signed-off-by: Jim Zhang <jim.zhang@kaleido.io>

Update dependency firefly-signer to v1.1.5

d139b8c

Signed-off-by: Jim Zhang <jim.zhang@kaleido.io>

Fix linter errors

c349984

Signed-off-by: Jim Zhang <jim.zhang@kaleido.io>

Fix linter errors

946c38a

Signed-off-by: Jim Zhang <jim.zhang@kaleido.io>

Update golang docs header with year 2023

2f08032

Signed-off-by: Jim Zhang <jim.zhang@kaleido.io>

jimthematrix merged commit 7342adc into hyperledger:main Jan 5, 2023

jimthematrix deleted the custom-errors branch January 5, 2023 18:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Custom errors support in FFI contract invocations and queries #1125

Custom errors support in FFI contract invocations and queries #1125

jimthematrix commented Jan 1, 2023 •

edited

nguyer left a comment

nguyer Jan 3, 2023

nguyer Jan 3, 2023

jimthematrix Jan 3, 2023 •

edited

jimthematrix Jan 3, 2023

nguyer Jan 3, 2023

nguyer Jan 3, 2023

nguyer Jan 3, 2023

jimthematrix commented Jan 3, 2023

codecov-commenter commented Jan 5, 2023

Custom errors support in FFI contract invocations and queries #1125

Custom errors support in FFI contract invocations and queries #1125

Conversation

jimthematrix commented Jan 1, 2023 • edited

matching custom error

no matching custom error (raw error data returned):

regular error from the require()

nguyer left a comment

Choose a reason for hiding this comment

nguyer Jan 3, 2023

Choose a reason for hiding this comment

nguyer Jan 3, 2023

Choose a reason for hiding this comment

jimthematrix Jan 3, 2023 • edited

Choose a reason for hiding this comment

jimthematrix Jan 3, 2023

Choose a reason for hiding this comment

nguyer Jan 3, 2023

Choose a reason for hiding this comment

nguyer Jan 3, 2023

Choose a reason for hiding this comment

nguyer Jan 3, 2023

Choose a reason for hiding this comment

jimthematrix commented Jan 3, 2023

codecov-commenter commented Jan 5, 2023

Codecov Report

jimthematrix commented Jan 1, 2023 •

edited

jimthematrix Jan 3, 2023 •

edited