Skip to content

cmd/abigen, accounts/abi/bind: implement abigen version 2 #26782

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

Closed
wants to merge 204 commits into from

Conversation

s1na
Copy link
Contributor

@s1na s1na commented Feb 28, 2023

This PR adds a new version of abigen which will co-exist parallel to the existing version for a while. To generate v2 bindings provide the --v2 flag to abigen cmd.

Summary

The main point of abigen v2 is having a lightweight generated binding which allows for more composability. This is possible now thanks to Go generics. "only" methods to pack and unpack call and return data are generated for a contract. As well as unpacking of logs.

To interact with the contract a library is available with functions such as Call, Transact, FilterLogs, etc. These take in the packed calldata, or a function pointer to unpack return data.

Features

The new version is at feature-parity with v1 at a much lower generated binding size. The only missing feature as of now is sessions.

Example

V1 and v2 bindings for a sample contract are available here: https://gist.github.com/s1na/05f2d241b07372b41ba1747ce6e098b7. A sample script using v2 is available in main.go.

@s1na s1na changed the title cmd,accounts/abi: abigen v2 cmd, accounts/abi: abigen v2 Feb 28, 2023
Copy link
Member

@MariusVanDerWijden MariusVanDerWijden left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks pretty cool! I don't like the *2 naming, but I guess Felix likes it, so I won't argue to hard against it. I'm going to do some more tests and then approve

@MariusVanDerWijden
Copy link
Member

MariusVanDerWijden commented Mar 1, 2023

Something is broken here:

func (_{{$contract.Type}} *{{$contract.Type}}) Unpack{{.Normalized.Name}}(data []byte) ({{if .Structured}}struct{ {{range .Normalized.Outputs}}{{.Name}} {{bindtype .Type $structs}};{{end}} },{{else}}{{range .Normalized.Outputs}}{{bindtype .Type $structs}},{{end}}{{end}} error) {
			out, err := _{{$contract.Type}}.abi.Unpack("{{.Original.Name}}", data)
			{{if .Structured}}
			outstruct := new(struct{ {{range .Normalized.Outputs}} {{.Name}} {{bindtype .Type $structs}}; {{end}} })
			if err != nil {
				return *outstruct, err
			}
			{{range $i, $t := .Normalized.Outputs}} 
			outstruct.{{.Name}} = *abi.ConvertType(out[{{$i}}], new({{bindtype .Type $structs}})).(*{{bindtype .Type $structs}}){{end}}

			return *outstruct, err
			{{else}}
			if err != nil {
				return {{range $i, $_ := .Normalized.Outputs}}*new({{bindtype .Type $structs}}), {{end}} err
			}
			{{range $i, $t := .Normalized.Outputs}}
			out{{$i}} := *abi.ConvertType(out[{{$i}}], new({{bindtype .Type $structs}})).(*{{bindtype .Type $structs}}){{end}}
			
			return {{range $i, $t := .Normalized.Outputs}}out{{$i}}, {{end}} err
			{{end}}
		}

With this contract it produces invalid code:

contract Eventer {
   
    event TestInt8(int8 indexed out1, int8 indexed out2);
    event AnonEvent(address, address);
    
    function getEvent() public {
        // set to 2,3 for functioning filter
        emit TestInt8(-2, -3);
    }

    function anonEvent() public {
        emit AnonEvent(msg.sender, msg.sender);
    }

    function fail() public {
        require(false, "error string");
    }
}

Produced code:

func (_Eventer *Eventer) UnpackGetEvent(data []byte) (struct{}, error) {
	out, err := _Eventer.abi.Unpack("getEvent", data)

	outstruct := new(struct{})
	if err != nil {
		return *outstruct, err
	}

	return *outstruct, err

}

(out is never used)

I think the issue is that you generate Unpacking functions for solidity functions that have no return value.
I think those functions can just be skipped

@MariusVanDerWijden
Copy link
Member

/home/matematik/go/src/github.com/go-snippets/geth-test/array.go:8:2: "fmt" imported and not used

You should probably do with fmt the same thing as with the other imports:
_ = fmt.Printf so that go doesn't break if fmt is not used

@s1na
Copy link
Contributor Author

s1na commented Mar 1, 2023

Something is broken here:

Aha, we don't need an unpack method when there are no return params.

Copy link
Contributor

@fjl fjl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add some tests, and more documentation.

@geoknee
Copy link

geoknee commented Jul 19, 2023

@s1na are you still actively working on this PR? It is of interest to our project so we would be willing to devote some time to helping getting it over the line.

Our use case is: we are building a state channel client in Go. Currently, it manages a private key, and acts as a signer. It uses the abigen output to sign and launch transactions. What we want is to optionally have our client not manage a private key, and instead simply prepare (or "pack") the transaction and give it to the user to sign and send. Since we would like to maintain the coupling between or .sol source files are our go code, a neat way to achieve this is some changes to abigen such as those proposed on this PR.

One suggestion I would have is to maintain backward compatibility with the current version of abigen by simply adding the extra PackFoo (etc) functions to the existing output? Then you wouldn't need to worry about putting it all under a "v2" namespace or even adding those Transact helper functions...

@s1na
Copy link
Contributor Author

s1na commented Jul 19, 2023

Hi @geoknee, great to see your interest in this PR. I'm not actively working on this, although I plan to finish it at some point. Your help is appreciated.

I want to first address

One suggestion I would have is to maintain backward compatibility with the current version of abigen by simply adding the extra PackFoo (etc) functions to the existing output? Then you wouldn't need to worry about putting it all under a "v2" namespace or even adding those Transact helper functions...

We really want to get v2 out instead of packing more features into v1.

I saw you have already generated v2 bindings for your contract. It'd be already great to hear if things are working and you've had any friction points.

The biggest outstanding point is to add a test suite for v2.

@geoknee
Copy link

geoknee commented Jul 20, 2023

We really want to get v2 out instead of packing more features into v1.

Fair enough, we can use both side by side in our project without too much pain.

I saw you have already generated v2 bindings for your contract. It'd be already great to hear if things are working and you've had any friction points.

I did spot a couple of problems:

  1. One which I suspect is a bug -- some unexpected db field which looks like it could be from your example contract sneaking into our bindings:

statechannels/go-nitro@a1e9dd7

I pushed a fix here s1na#10.

  1. I get an error when trying to generate bindings for an ERC20 Token that is part of our setup. This has also been thrown up in some tests I started to sketch out https://github.com/s1na/go-ethereum/pull/11/files#r1269362198

The biggest outstanding point is to add a test suite for v2.

How should this look like? Is there an existing test for v1 which we can emulate?
I started hacking with the existing tests to extend their coverage. Keen to get feedback on that approach before devoting the time to finishing that off.

"context"
"encoding/json"
"github.com/ethereum/go-ethereum/accounts/abi/bind/backends"
"github.com/ethereum/go-ethereum/accounts/abi/bind/testdata/v2_generated_testcase"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Build fails on this import atm

@jwasinger
Copy link
Contributor

@MariusVanDerWijden I have a few things I am trying to wrap with this. It's in a bit of a messy state, and I'd ask to hold off on review until I can get this finished (couple of hours).

@rjl493456442
Copy link
Member

The fallback and receive function are not rendered in the binding, which I think should be useful.

A contract can have at most one fallback function, declared using either fallback () external [payable] or fallback (bytes calldata input) external [payable] returns (bytes memory output)

Especially for fallback, it can support bytes input. It would be very useful to have the fallback encoder and combine it with other function encoder output.

@fjl fjl modified the milestones: 1.15.2, 1.15.3 Feb 17, 2025
@jwasinger
Copy link
Contributor

jwasinger commented Feb 17, 2025

Especially for fallback, it can support bytes input. It would be very useful to have the fallback encoder and combine it with other function encoder output.

So in this case, the input to fallback is not abi-encoded, so generating an encoder in the bindings is not strictly required.

That being said, I'm fine with generating packing functions when fallback/receive are defined (and an unpack if fallback is of the signature (bytes calldata input) external [payable] returns (bytes memory output), which is the only form allowed other than () external [payable]). It makes the code for interacting with these methods more consistent with how it is done with other custom named methods.

@jwasinger
Copy link
Contributor

jwasinger commented Feb 18, 2025

I found out that if a fallback is specified, its signature is not specified in the ABI definition (with the exception of whether it is payable or not).

Even if this could be fixed in solidity, we still have to deal with the current behavior.

Accordingly, I think the best option for us will be to provide generic pack/unpack for fallback from lib.go. These will take/return []byte without doing any encoding/decoding. Then we note in the docs that interaction via the fallback can optionally use these based on which variant of the fallback was defined (noting the lack of distinction in the ABI definition).

@fjl fjl modified the milestones: 1.15.3, 1.15.4, 1.15.5 Feb 25, 2025
@fjl fjl modified the milestones: 1.15.5, 1.15.6 Mar 5, 2025
@wisehalvdan
Copy link

Is this ready to be merged ? or will it be merged any time soon ?

…pdate any bindings that were stale (all changed because I used a newer version of solc)
@fjl
Copy link
Contributor

fjl commented Mar 7, 2025

@wisehalvdan soon!

jwasinger and others added 4 commits March 8, 2025 10:19
… utilties. Better documentation indicating the intended use of DefaultDeployer and the deployment options available.
… be used with contract bindings generated with --v2. Simplify Transact impl
@jwasinger
Copy link
Contributor

Before we merge this, I want to reopen so that I get main authorship of the PR.

@jwasinger jwasinger closed this Mar 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants