Could you apply the ABIMetadata structure of geth?

[brew0722]

Is your request related to a problem? Please describe.

ethereum/go-ethereum#22583

I saw the result of the generated source which is a little different from the abigen of geth if I use abigen of klaytn.
One of them is the ABIMetadata structure.
Obviously, klaytn's abigen also has all the technically necessary elements.
However, unlike the generation result of geth, if there is any open source using the binding result, it cannot be compiled by redirection alone without modification.

If there is no difference between geth and klaytn that binding generation results, I expect to be able to compile only with redirection without source modification.

Could you please review that apply the ABIMetadata structure of geth?

[hyunsooda]

Hi @brew0722,

Could you elaborate on what it means?

However, unlike the generation result of geth, if there is any open source using the binding result, it cannot be compiled by redirection alone without modification.

What do you mean the open source and redirection?

I checked out the changes of the PR linked, there seems not a feature improvement and its change seems to aim to avoid unnecessary compile costs.

The newly introduced type of MetaData actually seems combined one, which aggregated ABI, BIN, and function signatures. Those are represented by followings in the Klaytn's one:
XXXABI,
XXXBinRuntime,
XXXFuncSigs.

[brew0722]

Hi @hyunsooda,

The newly introduced type of MetaData actually seems combined one, which aggregated ABI, BIN, and function signatures. Those are represented by followings in the Klaytn's one:
XXXABI,
XXXBinRuntime,
XXXFuncSigs.

GetABI member function in Metadata, it provides an Unmarshaled JSON.
If you have a location that uses existing GetAbi, Just looking at xxxABI isn't enough.
Because abigen of klaytn does not provide this, unmasal must be implemented directly from the client every time.

func (m *MetaData) GetAbi() (*abi.ABI, error) {
	m.mu.Lock()
	defer m.mu.Unlock()
	if m.ab != nil {
		return m.ab, nil
	}
	if parsed, err := abi.JSON(strings.NewReader(m.ABI)); err != nil {
		return nil, err
	} else {
		m.ab = &parsed
	}
	return m.ab, nil
}

it cannot be compiled by redirection alone without modification.

For example, if have a client code that uses the abigen generation result of geth as follows,
I'm going to port this code repo from geth-based to Klaytn-based.

        import (
                "github.com/ethereum/go-ethereum/accounts/abi/bind/backends"
                "contracts/bindings"
        )
	contractABI, err := bindings.SomeContractMetaData.GetAbi()
	if err != nil {
		return nil, err
	}
	calldata, err := contractABI.Pack(
		"initialize",
                ....
	)
        backend := backends.NewSimulatedBackend(...)
        tx, err := NewTxWithBackend(backend, calldata)
       ...

However, I don't use replace in go.mod because some other codes must remain as the geth client role.
So if I explicitly change the import, I should also change the generated bindings to the abigen result of klaytn.

        import (
                "github.com/klaytn/klaytn/accounts/abi/bind/backends"
                "contracts/klaytn_bindings"
        )

I want it to be compiled as long as I change only the import to redirect.
However, I must modify this code because of the result of abigen is different.

        import (
                "github.com/klaytn/klaytn/accounts/abi/bind/backends"
                "github.com/klaytn/klaytn/accounts/abi"
                "contracts/klaytn_bindings"
        )

	contractABI, err := abi.JSON(strings.NewReader(klaytn_bindings.xxxABI));
        if err != nil {
		return nil, err
	}
         calldata, err := contractABI.Pack(
		"initialize",
                ....
	)

Of course, I don't think klaytn must match the code with geth. And this is obviously a very minor change.
However, unless it is a change due to some functional difference or refectoring, I think it is helpful for minor usability to match upstream geth as much as possible.
I thought abigen was like that, so I created an issue. I think there is no problem in providing equivalence to geth.

BTW, it is a very minor issue, so please you can close it freely if you think it is unreasonable to match the equivalence with the abigen of geth after reviewing it.

[hyunsooda]

Thanks for such a kind description.

If you don't start with the latest abigen of geth, I think there seems to be no problem. Then you can't have a GetAbi() wrapper, which is a trivial implementation.

As far as I know, the Klaytn team did not add any features that fits to Klaytn's one. I anticipate this was not considered one of the following compatibility updates with Ether. its minor compatibility issue may be considered later.

BTW, I wonder why you've been using the geth's one? Just for GetAbi() function? The mannual effort is much bigger than getting its implementation... Was there other obstacles except for this one?

[brew0722]

its minor compatibility issue may be considered later.

I agree. If it's an issue that going to work on very later, I'll leave it opened.

BTW, I wonder why you've been using the geth's one? Just for GetAbi() function? The mannual effort is much bigger than getting its implementation... Was there other obstacles except for this one?

of course, If I developed it myself, I wouldn't use geth. But... 😅
This is necessary in the process of porting some open source project related to ethereum. It uses various types of geth, such as SimulatedBackend and StateDB, Genesis, ... So, In addition to this issue, there are a few compatibility issues encountered due to differences in the interface and implementation between the two projects.
Most issues can be modified locally myself. As long as it is not related to the CN of klaytn (like as tx size limitation).

[hyunsooda]

I think, in that case, it seems better to use the appropriate abigen for the Ether-based connector or Klaytn-based connector. As they have different implementation (logic, types, and so on), the common one would not exist that guarantees works on both of them, as well as the case of GetAbi() function. To get a work, it needs changes in the lower layers which are breaking changes.

Let me conclude this thread:

For the different implementation connect, the corresponding abigen should be used as the dedicated one of course.
For the following updates of Ether's upstream (abigen), its consideration may be pushed later.

[brew0722]

As they have different implementation (logic, types, and so on), the common one would not exist that guarantees works on both of them

I understand your concern about this part. This is the right way to deal with it properly at the time of client development.

However, as I said earlier, it is an issue proposed because abigen does not seem to have any customized changes in klaytn. I just suggested it because there seems to be no reason not to follow up on the upstream changes.

[hyunsooda]

I meant, if Klaytn synced the latest abigen implementation, it might not work as expected because the stub code still depends on the implementation of its core logic (Klaytn or Ether). I'm not sure it still works with only the fix of header file locations. The update of abigen and its guarantees of working on both of them and seem different problems (The former requires a lot of work to test and may be considered as out of scope) That's why mentioned as:

For the different implementation connect, the corresponding abigen should be used as the dedicated one of course.
For the following updates of Ether's upstream (abigen), its consideration may be pushed later.

Please let me know if you have different aspect of it.

[brew0722]

The update of abigen and its guarantees of working on both of them and seem different problems (The former requires a lot of work to test and may be considered as out of scope) That's why mentioned as:

That's right. I agree perfectly. I don't want all the porting to be solved easily through this issue.
Other side effects by import redirection are not a matter to be concerned here. It was just to explain why there was a discrepancy about abigen result in my situation.

I don't want guarantees that there is haven't any problem after redirected.
I just hope the interface shapes are the same between abigen of klaytn and abigen of geth.

There seems to be a lot of confusion due to my lack of English expression. Please understand.

[hyunsooda]

Yes. Now let me note a brief conclusion for the following readers:

For a consistent update of the upstream abigen implementation, Having the same interface and architecture would make other convenient stuff, but not critical for now.

There seems to be a lot of confusion due to my lack of English expression. Please understand.

Same for me. Thanks.

[exalate-issue-sync]

Exalate commented: Issue Created by: brew0722

[exalate-issue-sync]

aidan.kwon commented: [https://github.com//pull/1879|https://github.com//pull/1879|smart-link]

[exalate-issue-sync]

jay s commented: reopend PR : [https://github.com//pull/1881|https://github.com//pull/1881|smart-link]

[aidan-kwon]

Closed via #1881