Skip to content

Conversation

@royalfork
Copy link

This PR fixes a panic which could occur in an abigen'd output file. This is the currently generated code:

// Tuple is a free data retrieval call binding the contract method 0xea8ca604.
//
// Solidity: function tuple(bool reverts) view returns(string a, int256 b, bytes32 c)
func (_Tupler *TuplerCaller) Tuple(opts *bind.CallOpts, reverts bool) (struct {
	A string
	B *big.Int
	C [32]byte
}, error) {
	var out []interface{}
	err := _Tupler.contract.Call(opts, &out, "tuple", reverts)

	outstruct := new(struct {
		A string
		B *big.Int
		C [32]byte
	})

	outstruct.A = out[0].(string)
	outstruct.B = out[1].(*big.Int)
	outstruct.C = out[2].([32]byte)

	return *outstruct, err

}

When _Tupler.contract.Call returns a non-nil err, out will not be populated with result data, and the outstruct.A = out[0].(string) assignment will panic with an "index out of range" runtime error.

This PR adds an if err != nil check to the generated code so that the contract.Call error is properly returned before any outstruct assignments.

// Tuple is a free data retrieval call binding the contract method 0xea8ca604.
//
// Solidity: function tuple(bool reverts) view returns(string a, int256 b, bytes32 c)
func (_Tupler *TuplerCaller) Tuple(opts *bind.CallOpts, reverts bool) (struct {
	A string
	B *big.Int
	C [32]byte
}, error) {
	var out []interface{}
	err := _Tupler.contract.Call(opts, &out, "tuple", reverts)

	outstruct := new(struct {
		A string
		B *big.Int
		C [32]byte
	})
	if err != nil {
		return *outstruct, err
	}

	outstruct.A = out[0].(string)
	outstruct.B = out[1].(*big.Int)
	outstruct.C = out[2].([32]byte)

	return *outstruct, nil

}

@royalfork royalfork changed the title accounts/abi/bind: Reverted call with tuple return doesn't panic accounts/abi/bind: Remove panic in abigen generated code Dec 1, 2020
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.

LGTM, thank you for picking this up!

Copy link
Contributor

@holiman holiman left a comment

Choose a reason for hiding this comment

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

LGTM

@holiman
Copy link
Contributor

holiman commented Dec 12, 2020

Apologies!

I thought this had been merged already, and when #22005 came in, I thought that was handling the same error in a different place, and merged that one. So now although this one was first, the other one got merged instead, and I just now saw that this had been left hanging.

Thanks for the PR, sorry about the screw-up, my bad!

@holiman holiman closed this Dec 12, 2020
@royalfork
Copy link
Author

No worries, just happy a fix has been merged successfully. Thanks!

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.

3 participants